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.

Sign up for GitHub

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

Jump to bottom

Add starknet_estimateMessageFee #1139

Merged
merged 22 commits into from
Jun 21, 2023
Merged

Add starknet_estimateMessageFee #1139

merged 22 commits into from
Jun 21, 2023

Conversation

sergey-melnychuk
Copy link
Contributor

@sergey-melnychuk sergey-melnychuk commented Jun 14, 2023 •
edited
Loading

Closes #1113

@sergey-melnychuk sergey-melnychuk marked this pull request as ready for review June 16, 2023 11:00
@sergey-melnychuk sergey-melnychuk requested a review from a team as a code owner June 16, 2023 11:00
Mirko-von-Leipzig
Mirko-von-Leipzig suggested changes Jun 16, 2023
View reviewed changes
Copy link
Contributor

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

LGTM, just the storage changes. Can't comment much on the python - leaving that to the more knowledgeable people :)

crates/storage/src/connection.rs Outdated Show resolved Hide resolved
@sergey-melnychuk sergey-melnychuk force-pushed the sm/est-msg-fee branch 2 times, most recently from be6df04 to 90897c8 Compare June 16, 2023 11:59
@Mirko-von-Leipzig
Copy link
Contributor

Oh and maybe add it to our CHANGELOG.md! We should really add a PR template that reminds us :D

@sergey-melnychuk
Copy link
Contributor Author

Oh and maybe add it to our CHANGELOG.md! We should really add a PR template that reminds us :D

Done: 42192fe

kkovaacs
kkovaacs reviewed Jun 19, 2023
View reviewed changes
Copy link
Contributor

@kkovaacs kkovaacs left a comment

Choose a reason for hiding this comment

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

Looks great except for the sender_address thingy we should double-check.

crates/rpc/src/v03/method/estimate_message_fee.rs Show resolved Hide resolved
crates/rpc/src/v03/method/estimate_message_fee.rs Outdated Show resolved Hide resolved
kkovaacs
kkovaacs approved these changes Jun 21, 2023
View reviewed changes
Copy link
Contributor

@kkovaacs kkovaacs left a comment

Choose a reason for hiding this comment

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

LGTM!

@sergey-melnychuk sergey-melnychuk merged commit 2ae47e7 into main Jun 21, 2023
@sergey-melnychuk sergey-melnychuk deleted the sm/est-msg-fee branch June 21, 2023 13:02
@sergey-melnychuk
Copy link
Contributor Author

starkware-libs/starknet-specs#100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mirko-von-Leipzig Mirko-von-Leipzig Mirko-von-Leipzig approved these changes

@kkovaacs kkovaacs kkovaacs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support starknet_estimateMessageFee
3 participants
@sergey-melnychuk @Mirko-von-Leipzig @kkovaacs