Cap gas estimation at the configured per-tx gas limit#4584
Conversation
|
Claude finished @squadgazzz's task in 2m 22s —— View job PR Review: Cap gas estimation at the configured per-tx gas limit
The change itself is correct and cleanly mirrors
So the winning solution passes the now-capped pre-comp sim, then the uncapped mempools re-sim still searches up to the block gas limit every block. Worth applying the same cap there to fully close the vector. Details in the inline comment. Otherwise LGTM — the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // Cap the search at the per-tx gas limit so a rogue solution can't force | ||
| // the node to binary-search up to the block gas limit. `Gas::new` rejects | ||
| // anything above this cap anyway, so no accepted solution is affected. | ||
| let gas_limit = self.inner.tx_gas_limit.try_into().map_err(|err| { | ||
| Error::GasPrice(anyhow!("failed to convert gas_limit to u64: {err:?}")) | ||
| })?; | ||
| let tx = tx.into().with_gas_limit(gas_limit); |
There was a problem hiding this comment.
This caps the simulator crate's estimate_gas, which is on the pre-competition solution-simulation path (Simulator::gas). But the DoS scenario the PR describes — "burns gas in a loop… repeated every block during re-simulation" — flows through a different, still-uncapped estimate_gas:
crates/driver/src/infra/blockchain/mod.rs:233(driver::infra::blockchain::Ethereum::estimate_gas) applies no gas ceiling.- It's called in the per-block submission loop at
crates/driver/src/domain/mempools.rs:272(and pre-submission atmempools.rs:148) — the exact "every block during re-simulation" path. - That
Ethereumalready carriestx_gas_limit(mod.rs:98), so the fix can be applied there too.
Net effect: a winning solution now passes the capped pre-competition sim, wins, and then the uncapped mempools re-sim runs eth_estimateGas searching up to the block limit every block anyway — so the described attack vector isn't fully closed. Consider applying the same cap to the driver's estimate_gas.
| { | ||
| let tx = tx.into(); | ||
| // Cap the search at the per-tx gas limit so a rogue solution can't force | ||
| // the node to binary-search up to the block gas limit. `Gas::new` rejects |
There was a problem hiding this comment.
gas simulation is done with binary search? where can i learn more about this?
There was a problem hiding this comment.
The same applies to geth and others.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request caps the transaction gas limit search in the simulator to prevent rogue solutions from forcing a binary search up to the block gas limit. The reviewer correctly identified that unconditionally setting the gas limit overwrites any pre-existing lower gas limit on the transaction request, and provided an actionable suggestion to cap it by taking the minimum of the existing gas limit and the configured transaction gas limit instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Claude finished @squadgazzz's task in 2m 51s —— View job PR Review: Cap gas estimation at the configured per-tx gas limitThe follow-up commit ( I left 2 inline comments, both on
Otherwise LGTM. |
There was a problem hiding this comment.
Code Review
This pull request introduces gas limit capping based on the per-transaction gas limit in both the blockchain driver and the Ethereum simulator. This change prevents rogue solutions from forcing expensive binary searches up to the block gas limit. There are no critical issues or feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Cap the search at the per-tx gas limit so a rogue solution can't force | ||
| // the node to binary-search up to the block gas limit while we re-check | ||
| // the settlement on every block during submission. | ||
| let gas_limit = u64::try_from(self.inner.tx_gas_limit.0).unwrap_or(u64::MAX); |
There was a problem hiding this comment.
Minor: this silently falls back to u64::MAX on conversion failure, which disables the cap entirely — on the exact path this PR is meant to harden. The simulator version (simulator/src/ethereum/mod.rs:121) instead returns an error. tx_gas_limit is a U256 operator config that realistically fits in u64, so this is unlikely to bite, but the two implementations diverging is a small footgun. Consider mirroring the simulator's try_into().map_err(..)? here for consistency.
|
Looks like reth (and probably all major nodes) already pick a reasonable maximum gas limit to avoid this issue: https://github.com/paradigmxyz/reth/blob/00aa375238e8f03b3e824dad57f47ade6b05bec7/crates/rpc/rpc-eth-api/src/helpers/estimate.rs#L102. |
Nice, thanks! But does it still make sense to be on the safer side? Looks like zero cost from our side. |
3 participants
Description
When the driver needs to know how much gas a settlement uses, it asks the node via
eth_estimateGas. The node answers by running the transaction and binary-searching for the smallest gas that succeeds, and with no ceiling set it searches all the way up to the block gas limit. A solver can return a settlement that burns gas in a loop to make every step of that search maximally expensive. Solver responses are untrusted on the non-colocated path (KYC, no staked COW), so this is a real way to overload the node the driver depends on, and it is worse on high-gas-limit chains.Two paths ran the estimation with no ceiling:
Simulator::gas), run for every solution and re-run every block byresimulate_until_revert,blockchain::Ethereum::estimate_gas), run every block while the settlement sits in the mempool.No new config is needed. Operators already set
tx_gas_limit, and it was already applied increate_access_list, just not in eitherestimate_gas.Changes
eth_estimateGassearch at the configuredtx_gas_limiton both the simulation and submission paths.tx_gas_limitdoes not fit au64.How to test
Existing tests. On the competition path,
Gas::newalready rejects any solution estimated abovemin(block_limit / 2, tx_gas_limit), so bounding the search bytx_gas_limitreturns the same estimate for every accepted solution, and only solutions that would be rejected now fail at the RPC instead of the post-check. On the submission path the submitted tx's gas limit is already<= tx_gas_limit, so a settlement that needs more only shifts from a doomed on-chain revert to an earlier pre-submission abort.