Conversation
…thereum#3406) The inclusion-list variant moves the last transaction of the last block out of the block body and into the inclusion list, then clears the expectations derived from it: header_verify, expected_gas_used and expected_block_access_list. rlp_modifier was left in place. rlp_modifier force-writes header fields onto the built block after the transition tool has run, and tests compute it from the pre-move transaction list. The moved transaction therefore keeps contributing to the header even though it no longer executes in the block, producing fixtures whose header contradicts their own body. Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com>
…ts in The test pinned the block gas limit to 21000 with a 21001-gas transaction to trigger GAS_ALLOWANCE_EXCEEDED. On forks with the EIP-7928 block-access-list item budget (gas_limit // 2000) a 21000-gas-limit block allows 10 items, which is below the protocol-level writes of even an empty block, so no block in this environment can be valid. The test itself never noticed - its block is expected invalid - but its auto-generated inclusion-list variant moves the failing transaction into the inclusion list and expects the emptied block to be VALID, producing a fixture that the spec's own state transition rejects (tests-focil-devnet@v0.2.0, blockchain_test_engine_inclusion_list, the one non-blob entry in the 127 broken engine fixtures). Scale the numbers to 100_000/100_001: the allowance check still fires the same way, and the item budget (50) now accommodates an empty block, so the inclusion-list variant fills to a genuinely valid block.
The regression test from ethereum#3445 constructs a BuiltBlock through model_construct with object() sentinels for fields that get_fixture_engine_new_payload only forwards, which the typechecker rejects for missing and mistyped named arguments. model_construct skips validation and the sentinels are never read, so splat them from a dict[str, Any] instead of passing them as checked named arguments.
|
Thanks @ilitteri! We are working on wrapping up some Glamsterdam things but FOCIL PRs and issues will be prioritized shortly after that. Thanks for the patience, we'll get to these soon! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devnets/focil/0 #3471 +/- ##
==================================================
Coverage ? 91.34%
==================================================
Files ? 624
Lines ? 37099
Branches ? 3397
==================================================
Hits ? 33888
Misses ? 2756
Partials ? 455
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:
|
Byte-identical to the forks/amsterdam hunk so the next focil rebase resolves cleanly; fixture output is unchanged.
fselmo
left a comment
There was a problem hiding this comment.
Thanks @ilitteri! lgtm. Regression unit test was very appreciated as well 👌🏼. I just pushed a byte-identical fix for the same issue that is going into forks/amsterdam in #3566. If that changes before it gets merged it might end up giving some small conflict when we rebase devnets/focil/0 on top of forks/amsterdam but otherwise it should just move smoothly through the rebase 🤞🏼. Either way, we'll get it ironed out :)
…es to eip-7805 (#3595) * fix(github): EIP-7805 Devnet config Backported without the whitespace-only runs-on reformatting. (cherry picked from commit 5f46130) * fix(consume): require null inclusionListSatisfied on non-VALID payloads (#3470) Port the engine simulator's inclusionListSatisfied guard from #3410 to the devnets/focil/0 simulator. The check previously asserted a non-null response field whenever the fixture stamped one, regardless of the payload's expected status; execution-apis bogota.md requires the field to be null for any payload not deemed VALID, so the check demanded a spec violation. tests-focil-devnet v0.2.0 stamps the field on 5,489 expected-INVALID engine fixtures, which fail for every client on the Hive focil board with "expected inclusion_list_satisfied in response". The ported guard is verbatim from #3410: a payload not deemed VALID must report null, and only VALID payloads have the fixture's expected verdict enforced. Fixes #3436 for the consume path independently of a fixture refill. (cherry picked from commit 67a9345) * fix(tests,test-specs): fix invalid inclusion-list engine fixture generation (#3471) * fix(tests): clear rlp_modifier when building inclusion-list variants (#3406) The inclusion-list variant moves the last transaction of the last block out of the block body and into the inclusion list, then clears the expectations derived from it: header_verify, expected_gas_used and expected_block_access_list. rlp_modifier was left in place. rlp_modifier force-writes header fields onto the built block after the transition tool has run, and tests compute it from the pre-move transaction list. The moved transaction therefore keeps contributing to the header even though it no longer executes in the block, producing fixtures whose header contradicts their own body. Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com> * fix(focil): omit inclusion list result for invalid payloads * fix(tests): give test_tx_gas_limit a block gas limit a valid block fits in The test pinned the block gas limit to 21000 with a 21001-gas transaction to trigger GAS_ALLOWANCE_EXCEEDED. On forks with the EIP-7928 block-access-list item budget (gas_limit // 2000) a 21000-gas-limit block allows 10 items, which is below the protocol-level writes of even an empty block, so no block in this environment can be valid. The test itself never noticed - its block is expected invalid - but its auto-generated inclusion-list variant moves the failing transaction into the inclusion list and expects the emptied block to be VALID, producing a fixture that the spec's own state transition rejects (tests-focil-devnet@v0.2.0, blockchain_test_engine_inclusion_list, the one non-blob entry in the 127 broken engine fixtures). Scale the numbers to 100_000/100_001: the allowance check still fires the same way, and the item budget (50) now accommodates an empty block, so the inclusion-list variant fills to a genuinely valid block. * fix(test-specs): splat the metadata regression block from untyped kwargs The regression test from #3445 constructs a BuiltBlock through model_construct with object() sentinels for fields that get_fixture_engine_new_payload only forwards, which the typechecker rejects for missing and mistyped named arguments. model_construct skips validation and the sentinels are never read, so splat them from a dict[str, Any] instead of passing them as checked named arguments. * fix(tests): align test_tx_gas_limit with its #3566 counterpart Byte-identical to the forks/amsterdam hunk so the next focil rebase resolves cleanly; fixture output is unchanged. --------- Co-authored-by: Marc <Marchhill@users.noreply.github.com> Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com> Co-authored-by: chugarchugarr <josephlerma19@gmail.com> Co-authored-by: Felipe Selmo <fselmo2@gmail.com> (cherry picked from commit 65f4222) --------- Co-authored-by: marioevz <marioevz@gmail.com> Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> Co-authored-by: Marc <Marchhill@users.noreply.github.com> Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com> Co-authored-by: chugarchugarr <josephlerma19@gmail.com>
Description
Three fixture-side fixes covering the 127
blockchain_tests_enginefixtures thattests-focil-devnet@v0.2.0ships with expectations EELS itself rejects, plus the invalid-payloadinclusionListSatisfiedstamping reported in #3436:eips/amsterdam/eip-7805): clearBlock.rlp_modifierwhen building inclusion-list variants. Without it the variant's emptied block keeps the moved transaction'sblobGasUsedin its header. Accounts for 126 of the 127.BuiltBlock.get_fixture_engine_new_payload()emitsNoneforinclusion_list_satisfiedwhen the block has an expected exception, perbogota.md. Refilled fixtures then stop stamping a verdict on expected-INVALID payloads and pass even under the currently deployed simulator check.test_tx_gas_limit: raise the pinned block gas limit from 21000 to 100000 (tx gas 21001 to 100001). At 21000 the EIP-7928 item budget (gas_limit // 2000= 10) is below the protocol-level writes of an empty block, so no block in that environment can be valid, while the auto-generated inclusion-list variant expects its emptied block to be VALID. At 100000 the budget is 50 and the variant fills to a valid block. Accounts for the 127th.A refill and release after this merges clears the remaining 127 failures on the Hive focil board.
Related Issues or PRs
Fixes #3436 (fixture side). Ports #3406. Revives #3445.
Verification
uv run fill tests/frontier/validation/test_transaction.py::test_tx_gas_limit --fork Bogota: the inclusion-list variant fills withgasLimit=100000, 0 txs, novalidationError, and passes ethrex's engine fixture runner (previously rejected for exceeding the BAL item budget).uv run fill tests/cancun/eip4844_blobs/test_blob_txs.py::test_insufficient_balance_blob_tx --fork Bogota: 144 inclusion-list variants refilled, none with the pinnedblobGasUsed; all 504 fixtures in the file pass ethrex's runner.uv run pytest packages/testing/src/execution_testing/specs/tests/test_focil_payload_metadata.py: 4 passed.