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

feat(sim): skip estimation if gas field is provided in op #683

Merged
merged 1 commit into from
Apr 17, 2024
Merged

feat(sim): skip estimation if gas field is provided in op #683

merged 1 commit into from
Apr 17, 2024

Conversation

dancoombs
Copy link
Collaborator

Closes #671

@dancoombs dancoombs force-pushed the danc/skip-estimation branch from fad59e7 to 4969b23 Compare April 16, 2024 20:01
@dancoombs dancoombs force-pushed the danc/skip-estimation branch 2 times, most recently from 5a96d7a to 18ce6b2 Compare April 17, 2024 01:23
@dancoombs dancoombs force-pushed the danc/skip-estimation branch 4 times, most recently from e343b20 to 3ba2ccb Compare April 17, 2024 14:17
@dancoombs
Copy link
Collaborator Author

dancoombs commented Apr 17, 2024 •
edited
Loading

TODO: unit tests

@dancoombs dancoombs force-pushed the danc/skip-estimation branch 2 times, most recently from 8a4e2bf to cfd593f Compare April 17, 2024 15:35
0xfourzerofour
0xfourzerofour previously approved these changes Apr 17, 2024
View reviewed changes
Copy link
Collaborator

@0xfourzerofour 0xfourzerofour left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good just some overflow checks and nit test stuff

crates/sim/src/estimation/v0_6.rs Show resolved Hide resolved
crates/sim/src/estimation/v0_6.rs Show resolved Hide resolved
crates/sim/src/estimation/v0_6.rs Outdated Show resolved Hide resolved
crates/sim/src/estimation/v0_7.rs Show resolved Hide resolved
crates/types/src/user_operation/mod.rs Show resolved Hide resolved
@dancoombs dancoombs force-pushed the danc/skip-estimation branch from cfd593f to 9a98580 Compare April 17, 2024 17:52
0xfourzerofour
0xfourzerofour previously approved these changes Apr 17, 2024
View reviewed changes
dphilipson
dphilipson previously approved these changes Apr 17, 2024
View reviewed changes
Copy link
Collaborator

@dphilipson dphilipson 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! We can look into reducing DRY later.

crates/types/src/user_operation/v0_7.rs Outdated Show resolved Hide resolved
crates/types/src/user_operation/mod.rs Outdated Show resolved Hide resolved
crates/types/contracts/src/utils/CallGasEstimationProxy.sol Show resolved Hide resolved
@dancoombs dancoombs dismissed stale reviews from dphilipson and 0xfourzerofour via 0759f03 April 17, 2024 18:40
@dancoombs dancoombs force-pushed the danc/skip-estimation branch from 9a98580 to 0759f03 Compare April 17, 2024 18:40
@dancoombs dancoombs merged commit 72331f6 into feat/v0_7 Apr 17, 2024
7 checks passed
@dancoombs dancoombs deleted the danc/skip-estimation branch April 17, 2024 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dphilipson dphilipson dphilipson left review comments

@0xfourzerofour 0xfourzerofour 0xfourzerofour approved these changes

@alex-miao alex-miao Awaiting requested review from alex-miao alex-miao is a code owner

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

Successfully merging this pull request may close these issues.
3 participants
@dancoombs @dphilipson @0xfourzerofour