Conversation
`f834f000…` no longer exists in ethereum/EIPs: it was a pre-force-push commit of EIPs#12277, which squash-merged on 2026-09-09 as d2a64c2d4cc44f2f507577d0ebfb110dcc21d358 (rationale-only wording change). The stateful benchmark refresh copied the dead hash and the compute benchmark test still carried aca88aa0. PR CI only collects the version checks, so none of the three were flagged.
The `with_all_precompiles` marker yields an `Address`, which is what `test_bal_precompile_funded` already declares; match it in the two other tests that consume the same parameter.
The coinbase lands in the BAL because the priority-fee credit touches it even when the fee is zero (`create_ether(coinbase, transaction_fee)` in `process_transaction`), not because EIP-3651 warms it: warmth is a gas property and records no access. Reword both gas-limit boundary comments.
…_ACCESS_LIST reth rejects a non-minimally encoded BAL with "failed to decode block access list: leading zero". The mapper had no pattern for it, so six `test_bal_invalid_non_minimal_scalar_encoding` engine fixtures failed as "Undefined exception message" although reth rejects the block.
…oads A client that hashes the payload's BAL bytes into its reconstructed header before validating the encoding (reth) answers INVALID_BLOCK_HASH where the fixtures only accepted INVALID_BLOCK_ACCESS_LIST. EIP-7928 fixes neither order, and both verdicts reject the block, so `test_bal_invalid_engine_payload_encoding` and `test_bal_invalid_non_minimal_scalar_encoding` now accept either. Nine reth consume-engine failures on the 2026-09-14 glamsterdam run.
With a 21000 block gas limit the BAL item cap is 10, but an empty Amsterdam block already carries 25 items of system-contract reads and writes, so the engine payload is invalid twice over. besu, geth, reth and nimbus report the cap before the transaction's gas allowance (ethereum#3439) while EELS reports the allowance first. A 100000 limit keeps the transaction the only defect; every fork still fills with GAS_ALLOWANCE_EXCEEDED.
spencer-tb
marked this pull request as ready for review
September 15, 2026 11:08
spencer-tb
changed the title
spencer-tb
force-pushed
the
pr3566-review-fixes
branch
from
September 15, 2026 11:34
799d9a2 to
b47945a
Compare
Owner
|
Thanks! I've got some minor tweaks to this but merging and will slap it on top |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-ups to ethereum#3566: fix BAL fixture failures, strengthen encoding and zero-value coverage, and refresh the EIP reference pins. Each commit is a separate change.
Commits
6ae21cbfe5— Refresh EIP-7928 pins. Replace the unavailable reference commit and outdated compute-benchmark pin with the mergedd2a64c2dcommit in all three locations.5ee4f2d758— Correct precompile parameter types. UseAddressin the two tests receiving values fromwith_all_precompiles.0d282c920a— Explain coinbase inclusion. Correct two comments: the coinbase enters the BAL through the fee credit, including a zero credit.4f2daa9ef1— Map Reth's BAL decoding error. Recognizefailed to decode block access listasINVALID_BLOCK_ACCESS_LISTinstead of reporting an undefined exception.437bf1503d— Accept the block-hash diagnosis. AllowINVALID_BLOCK_HASHalongsideINVALID_BLOCK_ACCESS_LISTin the malformed-payload and non-minimal-integer tests when clients reject during header reconstruction. Update the test-case documentation.b3b2b999cb— Fix the gas-limit fixture's BAL cap conflict. Raise the block limit to 100,000 and keep the transaction one gas above it. The system-contract BAL fits the cap, leaving the transaction gas allowance as the intended failure.af813764cf— Add matching-header malformed-BAL cases. Test empty bytes, a non-list RLP value, and a truncated list with both header commitments. The new matching-header cases requireINVALID_BLOCK_ACCESS_LIST; mismatched-header cases retain either diagnosis.f44bd11d44— Strengthen zero-value assertions. Require all recipient BAL change lists to be empty when no change is expected. Document why pre-existing empty-account deletion from EIP-161: Sending zero to an empty account ethereum/execution-specs#3341 is outside valid Amsterdam test states.b47945ae67— Isolate pre-fork hash rejection from API validation. Stop injecting a BAL payload field when only the header hash is corrupted, and retain explicit payload overrides. Keep the extra-field test active with-32602, as required by the strict-field rule inherited from Cancun. Preserve the general header-to-payload docstring explanation.Issue resolution
Pre-fork BAL-field responses
The final commit
b47945ae67separates the two cases:blockAccessList, leaving valid pre-fork parameters and an incorrect block hash. The RLP fixture retains the illegal header field.-32602. Prague newPayloadV4 inherits Cancun's strict parameter and field matching rule; ExecutionPayloadV3 has noblockAccessListfield. The client fix is proposed in reth#27248: reject non-nullblockAccessListandslotNumberfields onnewPayloadV3/V4with-32602, while treatingnullas absent and preserving V5 handling. The EELS test stays enabled.