Skip to content

Add --estimate-tip flag #3636

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
Merged
merged 38 commits into from
Aug 22, 2025
Merged

Add --estimate-tip flag #3636

merged 38 commits into from
Aug 22, 2025

Conversation

ddoktorski
Copy link
Contributor

Closes #3554

ddoktorski added 24 commits July 11, 2025 12:11
@ddoktorski
Support RPC 0.9.0
ea52319
@ddoktorski
Merge branch 'master' into rpc-0-9-0
bbfb650
@ddoktorski
Rename preconfirmed to pre_confirmed
d5b0302
@ddoktorski
Update version to 0.9 in node url
a0d4f6a
@ddoktorski
Restore ignored tests
2c20aca
@ddoktorski
Add backticks
2c599e1
@ddoktorski
Update changelog
3d1c5aa
@ddoktorski
Fix test_happy_case_deployment_fee_message test
802462b
@ddoktorski
Add dash to pre confirmed
764611e
@ddoktorski
Bump devnet to rc.2
7dc04ca
@ddoktorski
Resolve merge conflicts
f0bbc6e
@ddoktorski
Address PR comments
808625a
@ddoktorski
Use always underscore for pre_confirmed input
d1721df
@ddoktorski
Revert refactor in check_if_legacy_contract
38ce1d5
@ddoktorski
Resolve merge conflicts
c8ad731
@ddoktorski
Bump starknet-rs to rc.2; bump devnet to rc.4
35ab8a2
@ddoktorski
Fix changelog
8b28a91
@ddoktorski
Fix e2e::forking::with_cache test
34bbd0c
@ddoktorski
Resolve merge conflicts
68329c7
@ddoktorski
Remove rc from EXPECTED_RPC_VERSION
3a0a505
@ddoktorski
Fix cache test
28915e5
@ddoktorski
Update free provider url
dfb1f53
@ddoktorski
Add --tip flag
bf949ba
@ddoktorski
Add --estimate-tip flag
c58f89b
@ddoktorski ddoktorski requested a review from a team as a code owner August 12, 2025 10:38
@ddoktorski ddoktorski requested review from franciszekjob, MKowalski8 and cptartur and removed request for a team August 12, 2025 10:38
franciszekjob
franciszekjob reviewed Aug 12, 2025
View reviewed changes
Copy link
Contributor

@franciszekjob franciszekjob left a comment

Choose a reason for hiding this comment

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

Could we add a test in which we use --estimate-tip flag and later check the tip of transaction (that it's not 0)?

cptartur
cptartur reviewed Aug 12, 2025
View reviewed changes
Copy link
Member

@cptartur cptartur left a comment

Choose a reason for hiding this comment

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

Left some comments, but otherwise looks good

cptartur
cptartur reviewed Aug 12, 2025
View reviewed changes
Copy link
Member

@cptartur cptartur left a comment

Choose a reason for hiding this comment

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

Maybe some test with mocks validating that tip estimation is actually doing anything would be good?

@ddoktorski
Copy link
Contributor Author

Maybe some test with mocks validating that tip estimation is actually doing anything would be good?

Agree, but it's a bit problematic to mock a single call while serving the rest through the actual running devnet. I'll update if I manage to add it.

franciszekjob
franciszekjob approved these changes Aug 21, 2025
View reviewed changes
cptartur
cptartur approved these changes Aug 21, 2025
View reviewed changes
Base automatically changed from rpc-0-9-0-tip to master August 22, 2025 08:17
@ddoktorski ddoktorski added this pull request to the merge queue Aug 22, 2025
Merged via the queue into master with commit 941979d Aug 22, 2025
26 checks passed
@ddoktorski ddoktorski deleted the rpc-0-9-0-estimate-tip branch August 22, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cptartur cptartur cptartur approved these changes

@franciszekjob franciszekjob franciszekjob approved these changes

@MKowalski8 MKowalski8 Awaiting requested review from MKowalski8 MKowalski8 is a code owner automatically assigned from foundry-rs/starknet-foundry

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

Successfully merging this pull request may close these issues.

Suggested tip functionality
3 participants
@ddoktorski @cptartur @franciszekjob