fix(test-specs): give invalid-transaction blocks a nonzero header gas used - #3599
Draft
danceratopz wants to merge 2 commits into
Draft
danceratopz wants to merge 2 commits into
danceratopz wants to merge 2 commits into
Conversation
… used A block whose only transaction is rejected by the transition tool gets a header with gas used zero. A header that claims zero gas while the block carries transactions is inconsistent on its own, and some clients, Nimbus among them, reject such a block before validating any transaction. The fixture's expected transaction exception is then never exercised. When every exception a block expects is a transaction exception, the block has transactions and the transition tool reports zero gas used, set the header gas used to the sum of the transaction gas limits, capped at the block gas limit and at least one, so a transaction with a zero gas limit is covered too. Any nonzero value leaves the block invalid only because of its transaction; the sum is a plausible claim that stays within the header rules. A test-supplied `rlp_modifier` is applied afterwards and still wins, and blocks with block-level exceptions keep the transition tool's value. The engine payload is built from the same header, so the payload gas used changes with it. Add a unit test for the default, the list form of the exception, a zero gas limit, the modifier override and both block-level cases. The existing golden fixtures are unchanged because their invalid blocks also carry a valid transaction.
Recognize the errors Nimbus reports when a transaction's gas limit is below its intrinsic or floor gas requirement, when either of those exceeds the transaction gas limit cap on the Amsterdam path, and when the gas limit itself exceeds the EIP-7825 cap. Nimbus uses one message for the intrinsic and floor checks, so both exceptions map to it, and the specification classifies the Amsterdam cap check as insufficient intrinsic gas, so that message maps to the same pair. Remove the mappings of "zero gasUsed but transactions present" to the intrinsic gas and gas limit cap exceptions. They only existed to accept Nimbus rejecting a block with transactions and a zero header gas used before validating the transactions. Such blocks now claim the gas their transactions could have used, so Nimbus reaches transaction validation and reports the mapped errors instead.
danceratopz
added
C-bug
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #3599 +/- ##
================================================
Coverage 94.44% 94.44%
================================================
Files 624 624
Lines 36928 36928
Branches 3326 3326
================================================
Hits 34875 34875
Misses 1450 1450
Partials 603 603
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
danceratopz
changed the title
2 tasks
This branch has not been deployed
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
1 participant
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.
Description
Important
Requires #3537.
Fix how the test framework builds blocks containing invalid transactions. These tests should check why a transaction is invalid, but Nimbus rejects some generated blocks before checking the transaction because their headers claim zero gas used.
The transition tool excludes rejected transactions from its gas accounting. If a block contains only a rejected transaction, the generated header therefore gets
gasUsed = 0. Nimbus rejects this with "zero gasUsed but transactions present". Our exception mapper treated that header error as an intrinsic-gas or transaction-gas-cap error, so those tests could pass without exercising the intended check. Tests expecting the calldata-floor error failed exception matching instead.When a block contains transactions, expects only transaction exceptions, and has zero computed gas used, claim the sum of its transaction gas limits, capped at the block gas limit and raised to at least one. This also covers a transaction with a zero gas limit. Explicit
rlp_modifieroverrides still apply afterwards. Blocks with nonzero computed gas used or block-level expected exceptions keep their existing values.Update the Nimbus mapper to recognize the actual intrinsic/floor and transaction-cap errors, including both the current "Intrinsic execution" and older "Intrinsic regular" wording. Remove both mappings that accepted the early header rejection. Note that exception mapping is currently disabled for Nimbus by default, we should change that:
Add 12 framework regression cases covering six scenarios in both blockchain and engine formats. Existing consensus tests gain a check of the intended transaction rejection; this PR adds no new consensus test scenarios.
Affected fixtures change their header, block hash and encoded block or engine payload when regenerated, unless an explicit modifier overrides the new value. Expected exceptions stay the same.
Validation:
just staticpasses after the final amend.--until Amsterdam --generate-all-formatspasses all 803 cases and produces 211 enginex fixtures.v0.4.1-0d06e05a, using enginex with strict exception matching: all 124 EIP-7981 cases pass. The full 211-case run initially had five failures; after correcting the current intrinsic-cap wording, both affected cases pass on a targeted rerun. The other three failures are unrelated missingGAS_ALLOWANCE_EXCEEDEDmappings and are reserved for a separate PR.Related Issues or PRs
Checklist
just static<type>(<area>): <title>, where<type>and<area>come from an appropriateC-<type>, respectivelyA-<area>, label. The title should match the target squash commit message.Cute Animal Picture