fix: use live block gas budget for applyPendingChanges() call#282
Conversation
|
LGTM. The implementation correctly mirrors the existing system-call pattern ( |
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The fix is correct and well-scoped. The approach — narrowing the generic in the internal One doc wording issue flagged inline (the |
|
LGTM. Previous feedback on the |
Troublor
left a comment
There was a problem hiding this comment.
Fix is well-scoped and the policy is correctly gated on the explicit opt-in path, so stable specs keep byte-identical behavior. Two comments to address:
- The
transact_apply_pending_changesunit test only exercises gas-limit propagation (no pending role changes), not the OOG scenario the fix targets. A system-level regression test undercrates/mega-evm/tests/rex5/with a heavy SALT-bucket setup would prevent silent re-regression. - The "default system call keeps 30M" test is a structural assertion on
tx.base.gas_limit, not a behavioral check on EIP-2935 / EIP-4788 paths under REX5. The "byte-identical across all stable specs" claim is defended by code review rather than tests.
|
LGTM. Both of @Troublor's concerns from the previous review have been addressed by the new integration test file
|
vincent-k2026
left a comment
There was a problem hiding this comment.
LGTM. Diagnosis is clear, fix is local, and the test design is excellent — particularly:
test_default_system_call_keeps_upstream_30m_gas_limitusing the literal30_000_000instead ofSYSTEM_CALL_GAS_LIMIT_FLOORto deliberately catch upstream drift rather than auto-aligning to it.test_rex5_apply_pending_changes_oogs_under_upstream_30m_capas a negative baseline that will start failing if SALT bucket pricing ever drops back under 30M, prompting a re-examination of whether the fix is still load-bearing.- The EIP-2935 / EIP-4788 isolation tests pinning that REX5 only widens
applyPendingChanges.
Verified separately that SequencerRegistry.sol does not call gasleft(), so the gas_limit change is not bytecode-observable — state/receipts/logs identical for blocks where 30M was already sufficient. REX5 hasn't activated, so no replay impact either.
Two non-blocking nits:
-
Naming:
SYSTEM_CALL_GAS_LIMIT_FLOORlives underconstants::rex5but the value mirrors revm's universal upstream literal, not a REX5 parameter. The REX5-specific decision is which call sites opt into the widened budget, not the floor value itself. Worth hoisting to top-levelconstants(or asystem_callsubmodule named after its provenance) so a future REX spec can reuse it without aliasing. -
PR body mentions related fixes in mega-reth (oracle
close_windowgas limit) and test-client. Could those PRs be linked here? Want to make sure the merge order is coordinated and devnet validation covers both halves before the REX5 activation block. This PR alone is forward-compatible.
|
LGTM. All prior feedback from Troublor has been addressed by the new |
Summary
Fix REX5 pre-block
SequencerRegistry.applyPendingChanges()running out of gas on role-activation blocks.gas_limit, butapplyPendingChanges()rotates roles viaSSTOREs whose actual cost scales with REX dynamic storage gas (SALT bucket capacity) — 30M is no longer guaranteed to be enough.MegaEvm::transact_system_call_with_gas_limit(...)method lets pre-block helpers pick the gas budget explicitly. The defaulttransact_system_callentry point keeps the upstream-fixed 30M, so EIP-2935 / EIP-4788 and every other system call retain byte-level identical behavior across all stable specs.transact_apply_pending_changesopts into the new method, calling withmax(block.gas_limit, SYSTEM_CALL_GAS_LIMIT_FLOOR). The 30M floor matches revm's hardcodedSystemCallTx::new_system_tx_with_callerliteral (revm doesn't export it as apub const, so we mirror it).Note: Related fixes in
mega-reth(oracle update tx gas limit chosen too early inclose_window()) andtest-client(block-driver progress, oracle tx shape assertions) are tracked outside this repo.Test plan
cargo fmt --all --checkpassedcargo clippy --workspace --lib --tests --benches --all-features --lockedpassedcargo test -p mega-evm— all passednpx prettier --check 'docs/**/*.md'passed