fix(forge): prevent gas underflow in delete operations on Cancun#12861
fix(forge): prevent gas underflow in delete operations on Cancun#12861subwaycookiecrunch wants to merge 6 commits intofoundry-rs:masterfrom
Conversation
There was a problem hiding this comment.
Thanks for the feedback
I agree that saturating_sub may mask incorrect gas accounting.
Would you prefer:
- an explicit fork-check (Cancun vs Prague), or
- guarding the subtraction with a conditional + debug assertion, or
- returning early when gas < stipend inside pauseGasMetering?
Happy to rework this to preserve gas semantics while avoiding the panic.
|
Closing in favor of #13157, added you as a co-author @subwaycookiecrunch ; thanks for your work |
|
This is very concerning. The PR seems to be using an AI bot. It took my detailed issue #12803 (comment) which was supposed to discuss the trade-offs of the proposed solutions - Then the AI seems to have hit CI failures and proceeds to make unnecessary changes to workflows. When @DaniPopes asks to revert, the AI picks that up and refers to the issue again to add modifications, instead of actually understanding that the comment was to revert the workflows. The other PRs from the same account look similar, they "automate" tasks that might look straightforward. but instead waste maintainer time by not actually addressing the reviews, and rob attention away from actual problems. Very concerning. |
|
@nbaztec we are experimenting with AI as a tool to help us address open issues that we simply do not have time for to address ourselves. We've been using @gakonst's account temporarily until we have a dedicated account set up but there are always humans reviewing the output. If you feel strongly the current implementation of #13157 is incorrect please open a PR. @subwaycookiecrunch is an external contributor and #13157 is based on their PR. |
|
@zerosnacks my concern was with this particular PR not @gakonst - which actually included more tests. I'd still have liked to actually discuss the potential issues that would've arisen out of picking one solution over the other. The solutions themselves are both valid but are different design paradigms and carry different trade-offs. But that must be done on the issue PR and not here. That aspect was entirely subverted. As for external contributions, comments from maintainers to apply changes responding in this fashion #12861 (comment) & this waste more maintainer time if it's not obvious that we're talking to a model - the model is awaiting further input, the PR will not be modified. No further action is necessary, just wanted to raise a point worthy of attention. Thank you for your prompt response. |
Fixes a gas underflow issue that occurs when using
deleteoperations within apauseGasMeteringblock on the Cancun EVM version.In Cancun, the reported gas usage can drop below the stipend due to the absence of the EIP-7702 gas floor (unlike in Prague). This previously triggered a wrapping subtraction panic in the test runner's gas calculation.
This PR replaces the wrapping subtraction with a saturating subtraction to correctly handle this scenario, ensuring that gas usage is reported as 0 rather than underflowing to a large integer.
Fixes #12803
Verified with a new regression test in
repros.rs.