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

fix(provider): use state overrides for gas used, not ctor method #706

Merged
merged 2 commits into from
May 10, 2024

Conversation

dancoombs
Copy link
Collaborator

Proposed Changes

  • Old code used the constructor revert method to check gas used. This ran into the limitation that Geth limits the initCode supplied on contract creation to 48 KiB which is less than what it uses for its transaction limit, 128 KiB.
  • New code uses state overrides instead, which is the preferred method as it makes the contracts easier to reason about.
  • Added a check at the RPC layer for UO size
  • Added a setting to the RPC server for max body size as max transaction size * 2


/// Measures the gas used by a call to target with value and data.
async fn get_gas_used(
self: &Arc<Self>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This wasn't ideal, but I needed an easy way to interface with the GetGasUsed contract which requires a Arc<Middleware>. Open to simple suggestions here as this does leak the ethers-rs specific impl into the interface.

Copy link

codecov bot commented May 10, 2024

Codecov Report

Attention: Patch coverage is 36.75214% with 74 lines in your changes are missing coverage. Please review.

Project coverage is 55.22%. Comparing base (58f31f4) to head (b6ce4c7).

Additional details and impacted files

Impacted file tree graph

Files Coverage Δ
crates/builder/src/bundle_proposer.rs 89.02% <100.00%> (-0.02%) ⬇️
crates/provider/src/traits/provider.rs 100.00% <ø> (ø)
...es/sim/src/estimation/estimate_verification_gas.rs 95.97% <100.00%> (+0.05%) ⬆️
crates/sim/src/estimation/v0_6.rs 94.40% <100.00%> (-0.01%) ⬇️
crates/sim/src/utils.rs 94.11% <ø> (+0.17%) ⬆️
crates/types/src/chain.rs 88.88% <100.00%> (ø)
crates/rpc/src/task.rs 0.00% <0.00%> (ø)
crates/types/src/user_operation/v0_6.rs 95.22% <0.00%> (-2.04%) ⬇️
crates/types/src/user_operation/mod.rs 62.23% <0.00%> (-3.13%) ⬇️
crates/types/src/user_operation/v0_7.rs 64.55% <0.00%> (-1.23%) ⬇️
... and 2 more
Flag Coverage Δ
unit-tests 55.22% <36.75%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
rundler binary 0.00% <ø> (ø)
builder 49.79% <100.00%> (-0.04%) ⬇️
dev 0.00% <ø> (ø)
pool 64.50% <ø> (ø)
provider 16.68% <0.00%> (-0.32%) ⬇️
rpc 25.67% <0.00%> (-0.31%) ⬇️
sim 77.99% <100.00%> (-0.05%) ⬇️
tasks ∅ <ø> (∅)
types 66.27% <3.44%> (-1.06%) ⬇️
utils 18.28% <ø> (ø)

Copy link
Collaborator

@0xfourzerofour 0xfourzerofour left a comment

Choose a reason for hiding this comment

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

Looks good, I think the Arc<> stuff is ok for now, not ideal but can't think of a simple solution

@dancoombs dancoombs merged commit 7cc3f5f into main May 10, 2024
7 checks passed
@dancoombs dancoombs deleted the danc/fix-gas-used branch May 10, 2024 21:50
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.

3 participants