Skip to content

fix(test-focil): omit inclusion list result for invalid payloads - #3445

Closed
chugarchugarr wants to merge 1 commit into
ethereum:devnets/focil/0from
chugarchugarr:fix/focil-invalid-payload-metadata
Closed

chugarchugarr wants to merge 1 commit into
ethereum:devnets/focil/0from
chugarchugarr:fix/focil-invalid-payload-metadata

Conversation

@chugarchugarr

@chugarchugarr chugarchugarr commented Aug 26, 2026 •

Copy link
Copy Markdown

Description

Fixes #3436.

Engine API payload metadata only has a meaningful inclusionListSatisfied result when the payload itself is valid. The FOCIL filler previously carried the transition-tool result into engine_newPayload fixtures even when the same BuiltBlock was marked invalid.

BuiltBlock.get_fixture_engine_new_payload() now:

  • preserves True and False for valid payloads;
  • emits None for invalid payloads, as the Engine API requires.

The focused regression covers all four valid/invalid and satisfied/unsatisfied combinations.

Verification

  • uv run pytest -q packages/testing/src/execution_testing/specs/tests/test_focil_payload_metadata.py — 4 passed
  • Ruff check and format check — passed
  • git diff --check — passed

Scope

  • no change to inclusion-list evaluation
  • no change to valid-payload metadata
  • no production fork behavior change

Base

devnets/focil/0 at 23b0358c3513c8d22324faea66b41cec4b0b6446.

Copy link
Copy Markdown
Author

Closing this contribution to keep my active upstream review surface focused on #3443. This PR received no maintainer review and its fork workflows remained authorization-gated, so I am not asking the project to spend review bandwidth on it.

The branch and diff remain available and can be reopened if a maintainer specifically wants to continue the invalid-payload metadata fix.

ilitteri added a commit to lambdaclass/execution-specs that referenced this pull request Aug 31, 2026
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.
fselmo added a commit that referenced this pull request Sep 15, 2026
…ration (#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>
fselmo added a commit that referenced this pull request Sep 16, 2026
…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>
fselmo added a commit that referenced this pull request Sep 23, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant