Repository navigation
bug(EL): payload builder still panics on cumulative-gas checked_add overflow #452
Description
Bug
crates/execution-payload/src/payload.rs still panics on two cumulative-gas accounting paths via:
checked_add(...).expect("total gas shouldn't overflow")- Pre-execution capacity check (
pool_tx.gas_limit()) - Post-execution cumulative update (
gas_used)
If either checked_add returns None, the payload builder aborts instead of failing with a structured PayloadBuilderError / invalid-tx path.
Why this is a bug (not a refactor)
- fix: use checked arithmetic for cumulative gas accounting in payload builder #24 asked for explicit error propagation instead of panic / silent clamp on this accounting.
- fix: use checked arithmetic for cumulative gas accounting in payload builder #42 switched to
checked_addbut left both.expect(...)sites in place, so the panic path remains. - Closed PR fix: complete payload builder gas overflow error propagation #135 proposed finishing that fix; it was closed for process (
Closes: #…/ assignment), not because the change was rejected on technical grounds.
Impact
In production the overflow is extremely unlikely (gas_used / gas_limit are bounded by the block gas limit, far below u64::MAX). The bug is still real: an unreachable panic is the wrong failure mode for block building, and it is inconsistent with the error-propagation pattern already required for this code.
Proposed fix
- Pre-execution: treat add overflow like “does not fit” (
ExceedsGasLimit/ mark invalid), do not panic. - Post-execution: return
PayloadBuilderErroron overflow and fail the build cleanly.
No hardfork gate: builder-local error handling only.
Acceptance criteria
- Both
.expect("total gas shouldn't overflow")call sites inpayload.rsare removed - Overflow / non-fit paths return errors (no panic)
-
cargo test -p arc-execution-payloadpasses
Activity
crazywriter1 commentedon Sep 20, 2026
Hello. If this looks useful, could I please be assigned so I can open a PR with
Closes: #<this>once you're ready?I'd keep the first PR limited to the two
payload.rssites (pre-check → mark invalid / ExceedsGasLimit; post-exec → PayloadBuilderError). Happy to leave the executor path for a separate follow-up if you prefer. @ZhiyuCircleUzairRehman4 commentedon Sep 23, 2026
Hi maintainers, I have a fix ready for both
.expect("total gas shouldn't overflow")sites described here (pre-execution capacity check treated as "doesn't fit" on overflow, post-execution update returns aPayloadBuilderErrorinstead of panicking), following the acceptance criteria in the issue. Could I be assigned to this issue so I can open a PR per the contributing guidelines? Thanks!fix(EL): payload builder no longer panics on cumulative-gas checked_a…
crazywriter1 commentedon Sep 28, 2026
Requesting assignment to @crazywriter1 for this issue, per CONTRIBUTING.md.
forumevi commentedon Oct 27, 2026
Requesting assignment per CONTRIBUTING.md.
I have verified that both
.expect("total gas shouldn't overflow")call sites are present on the currentmainbranch:cumulative_gas_used.checked_add(pool_tx.gas_limit()).expect(...)cumulative_gas_used.checked_add(gas_used).expect(...)Both are in
crates/execution-payload/src/payload.rs.Proposed fix, limited to the two sites:
match; treatNoneas the tx not fitting the block — mark it invalid viaExceedsGasLimitandcontinue, consistent with the existing capacity-exceeded path above it.matchorok_or(); treatNoneas aPayloadBuilderErrorand returnErr, keeping it consistent with theFatalarm inclassify_tx_outcome.No hardfork gate needed. Happy to keep this strictly to the two arithmetic sites and leave any refactor for a separate PR.