core: fix EIP-7778 block gas accounting to exclude refunds#33754
core: fix EIP-7778 block gas accounting to exclude refunds#33754qu0b wants to merge 1 commit intoethereum:bal-devnet-2from
Conversation
qu0b
force-pushed
the
qu0b/fix/eip-7778-block-gas-accounting
branch
from
95334ea to
514bde1
Compare
EIP-7778 requires block-level gas accounting to exclude refunds, but receipts must continue using post-refund CumulativeGasUsed. The previous code used a single gas value for both purposes, causing: 1. Miner's header.GasUsed (post-refund) diverging from validator's expected value (pre-refund), rejecting all refund-heavy blocks. 2. Receipt root hash mismatches with other clients (e.g., Besu) since CumulativeGasUsed was incorrectly set to pre-refund gas. Fix by returning two gas values from ApplyTransactionWithEVM: - receipt: contains post-refund CumulativeGasUsed (unchanged behavior) - blockGasUsed: pre-refund gas for Amsterdam, post-refund otherwise All callers updated: state_processor, chain_makers, parallel processor, miner/worker, t8ntool, simulate, tracers. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
qu0b
force-pushed
the
qu0b/fix/eip-7778-block-gas-accounting
branch
from
514bde1 to
b6c983d
Compare
| snapshot = statedb.Snapshot() | ||
| prevGas = gaspool.Gas() | ||
| ) | ||
| receipt, err := core.ApplyTransactionWithEVM(msg, gaspool, statedb, vmContext.BlockNumber, blockHash, pre.Env.Timestamp, tx, &gasUsed, evm) |
There was a problem hiding this comment.
The receipt.GasUsed contains the gas used as it counts towards the block gas limit (the gas charged to the account + any gas that was returned via refunds). Thus, I think there's no need to return this value separately and we can simplify the fix that this PR makes.
Going to try to see if I can make the fix accordingly.
There was a problem hiding this comment.
Feel free to close this PR if you have a better solution, but please keep in mind: ethereum/EIPs#11191
There was a problem hiding this comment.
no. I'm wrong receipt.GasUsed subtracts refunds.
There was a problem hiding this comment.
Yeah, the issue right now is afaik is in the block building. On the verification side, Geth seems to be correct (we pass all the tests for 7778). This PR does fix other spots (chain makers, t8n), and we want to incorporate a fix into those areas.
|
I think we can make this fix in a way that decreases the size of the diff: include the block gas used in the |
|
I'm gonna go with #33761 . Thanks for this, but I feel like the other PR is a simpler and cleaner solution. |
alternative to #33754 Adds an extra non-consensus field on the `Receipt`: `BlockGasUsed`. This reflects the gas used as it counts towards the block gas limit, whereas the existing field `GasUsed` is the consensus field: the gas used with refunds subtracted. Changes an incorrect comment describing the field `ExecutionResult.UsedGas`.
2 participants
Summary
CumulativeGasUsed) from pre-refund gas (for blockheader.GasUsed) in EIP-7778 (Block Gas Accounting Without Refunds)ApplyTransactionWithEVMnow returns(receipt, blockGasUsed, error)— receipt uses post-refund gas,blockGasUseduses pre-refund for AmsterdamProblem
EIP-7778 requires block-level gas to exclude refunds, but receipts must continue using post-refund
CumulativeGasUsed. The previous code used a single gas value for both purposes, causing:header.GasUsed(post-refund) diverging from validator's expected value (pre-refund), rejecting all refund-heavy blocksCumulativeGasUsedwas incorrectly set to pre-refund gasTest plan
invalid gas usedor receipt root hash mismatch errors after Gloas fork🤖 Generated with Claude Code