Skip to content

Comments

fix(forge): prevent gas underflow in delete operations on Cancun#12861

Closed
subwaycookiecrunch wants to merge 6 commits intofoundry-rs:masterfrom
subwaycookiecrunch:fix/gas-underflow-cancun
Closed

fix(forge): prevent gas underflow in delete operations on Cancun#12861
subwaycookiecrunch wants to merge 6 commits intofoundry-rs:masterfrom
subwaycookiecrunch:fix/gas-underflow-cancun

Conversation

@subwaycookiecrunch
Copy link
Contributor

Fixes a gas underflow issue that occurs when using delete operations within a pauseGasMetering block 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.

Copy link
Member

Choose a reason for hiding this comment

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

please revert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@zerosnacks
Copy link
Member

Closing in favor of #13157, added you as a co-author @subwaycookiecrunch ; thanks for your work

@zerosnacks zerosnacks closed this Jan 20, 2026
@github-project-automation github-project-automation bot moved this to Done in Foundry Jan 20, 2026
@nbaztec
Copy link
Contributor

nbaztec commented Jan 21, 2026

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 - wrapping vs saturating, stripped it of context, and implemented the code verbatim from it.

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.

@zerosnacks
Copy link
Member

zerosnacks commented Jan 21, 2026

@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.

@nbaztec
Copy link
Contributor

nbaztec commented Jan 21, 2026

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bug(forge): gas underflow for delete operations on Cancun

4 participants