Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update(katana): estimate fee on deploy account tx #639

Closed
wants to merge 1 commit into from

Conversation

broody
Copy link
Collaborator

@broody broody commented Jul 12, 2023

No description provided.

@broody broody marked this pull request as draft July 12, 2023 15:00
@broody
Copy link
Collaborator Author

broody commented Jul 12, 2023

@kariy here's that change was talking about, most likely I'm doing something wrong... Estimate fee endpoint returns following error:

Estimate fee error: ContractNotFound(ContractAddress(PatriciaKey(StarkFelt("0x12..34"))))

Copy link
Member

@kariy kariy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but actually we also need to fix the add_deploy_account endpoint because right now it doesnt compute the transaction hash. so it will return validation error when executed.

Comment on lines +109 to +111
let mut inner_vec = calldata.to_vec();
inner_vec.push(class_hash);
inner_vec.push(contract_address_salt);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its actually should be:

[class_hash, salt, constructor_calldata]

based on starknet-rs impl... which is different from the starknet docs

@kariy
Copy link
Member

kariy commented Jul 12, 2023

@broody Do you mind if i takeover this PR?

@broody
Copy link
Collaborator Author

broody commented Jul 12, 2023

@broody Do you mind if i takeover this PR?

Yes plzzz!

@kariy
Copy link
Member

kariy commented Jul 13, 2023

Closing this in favour of #644

@kariy kariy closed this Jul 13, 2023
@tarrencev tarrencev deleted the estfee-deploy-acc-tx branch August 1, 2023 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants