Skip to content

fix(EL): payload builder no longer panics on cumulative-gas checked_add overflow - #461

Closed
UzairRehman4 wants to merge 1 commit into
circlefin:mainfrom
UzairRehman4:fix/payload-builder-gas-overflow-panic
Closed

UzairRehman4 wants to merge 1 commit into
circlefin:mainfrom
UzairRehman4:fix/payload-builder-gas-overflow-panic

Conversation

@UzairRehman4

Copy link
Copy Markdown

Summary

Both cumulative-gas accounting sites in crates/execution-payload/src/payload.rs used checked_add(...).expect("total gas shouldn't overflow"), which aborts the process on overflow instead of failing through the structured error path this function already uses everywhere else.

  • Pre-execution capacity check: an overflowing cumulative_gas_used + pool_tx.gas_limit() can never fit in a bounded block, so it's now treated the same as "doesn't fit" — the tx is marked invalid via ExceedsGasLimit, evicted from the pool if permanently un-includable, and the loop continues. No panic.
  • Post-execution cumulative update: now returns a PayloadBuilderError (wrapping a new CumulativeGasOverflowError, following the existing UnprocessableTransactionError pattern in builder.rs) via ? instead of panicking, so the build fails cleanly through the existing Result path.

No hardfork gate — this is builder-local error handling only, as scoped in the issue.

Fixes #452

Acceptance criteria (from the issue)

  • Both .expect("total gas shouldn't overflow") call sites in payload.rs are removed
  • Overflow / non-fit paths return errors (no panic)
  • cargo test -p arc-execution-payload passes — I was not able to run this locally (no Rust toolchain available in my environment). I reviewed the change carefully against the existing code patterns (the ExceedsGasLimit branch is reused unchanged, and CumulativeGasOverflowError mirrors UnprocessableTransactionError exactly), but please run the test suite / CI before merging and let me know if anything needs adjusting.

Test plan

  • Manual code review against the existing patterns in this file (see above)
  • CI / cargo test -p arc-execution-payload should be run by a maintainer or CI, since I couldn't run it locally

…dd overflow

Both cumulative-gas accounting sites in the payload builder used
checked_add(...).expect("total gas shouldn't overflow"), which aborts
the process if the addition overflows u64 instead of failing through
the structured error path already used elsewhere in this function.

- Pre-execution capacity check: an overflowing add can never fit in a
  bounded block, so treat it the same as "doesn't fit" (mark the tx
  invalid via ExceedsGasLimit, evict if permanently un-includable,
  continue) instead of panicking.
- Post-execution cumulative update: return a PayloadBuilderError
  (wrapping a new CumulativeGasOverflowError) instead of panicking, so
  the build fails cleanly through the existing Result-based path.

No hardfork gate; this is builder-local error handling only.

Fixes circlefin#452

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Unsigned Commits Detected

The following commits are missing a verified signature:

  • 3f39a85 by Uzair Rehman

How to fix: Sign your commits.

@github-actions

Copy link
Copy Markdown
Contributor

Hi @UzairRehman4,

Thank you for your interest in contributing to Arc Node.

This PR has been automatically closed because you are not assigned to issue #452. We require contributors to be explicitly assigned to an issue before submitting a PR.

To contribute properly:

  1. Comment on issue bug(EL): payload builder still panics on cumulative-gas checked_add overflow #452 requesting assignment
  2. Wait for maintainer approval
  3. Only submit a PR after you have been assigned

Please see our CONTRIBUTING.md for more details.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(EL): payload builder still panics on cumulative-gas checked_add overflow

1 participant