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

Respect block gas limit in gasEstimate buffer #2353

Merged
merged 4 commits into from
Feb 6, 2024
Merged

Conversation

fleupold
Copy link
Contributor

@fleupold fleupold commented Feb 2, 2024

Description

When looking at driver native solution submissions on Gnosis Chain, I notice that we sometimes fail due to:

Error { code: ServerError(-32010), message: "GasLimitExceeded, Gas limit: 17000000, gas limit of rejected tx: 17728674", data: None }

This stems from the fact that we multiply the gasEstimate we receive during simulation by 2 (to account for inaccuracies, changes in the execution trace, refunds, etc.)
This PR makes is so that we upper bound this buffered estimate by the current block size.

Changes

  • Introduce method on infra::eth to fetch the current gas limit
  • Use this value when creating the gas estimate

Thanks to @MartinquaXD great suggestion I moved the gas limit fetch into the Fetch.sol helper contract, which even allows this call to be infallible and synchronous 🙌 .

How to test

Added a driver integration test

@fleupold fleupold requested a review from a team as a code owner February 2, 2024 10:32
@MartinquaXD
Copy link
Contributor

Note, that the call to get the current gas limit isn't cached and can potentially lead to a lot of expensive getBlock calls on our node. I'm not sure what a good and simple caching strategy is. Any suggestions?

The most straight forward solution would probably to adjust FetchBlock.sol to also return the gas limit and access that value from the current block stream. That way we don't add any additional calls and get caching for free.

Copy link
Contributor

@MartinquaXD MartinquaXD left a comment

Choose a reason for hiding this comment

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

Just a nit on the test function.

@fleupold fleupold enabled auto-merge (squash) February 6, 2024 09:20
@fleupold fleupold merged commit ab1143c into main Feb 6, 2024
7 of 8 checks passed
@fleupold fleupold deleted the respect_block_gas_limit branch February 6, 2024 09:25
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants