Skip to content

fix(tests): stop test_tx_gas_limit opting into an inclusion-list variant - #3503

Draft
Marchhill wants to merge 21 commits into
ethereum:eips/amsterdam/eip-7805from
Marchhill:fix/il-variant-tx-gas-limit-floor
Draft

Marchhill wants to merge 21 commits into
ethereum:eips/amsterdam/eip-7805from
Marchhill:fix/il-variant-tx-gas-limit-floor

Conversation

@Marchhill

Copy link
Copy Markdown
Contributor

What

test_tx_gas_limit no longer opts into the inclusion-list variant.

Why

The test pins the block gas limit to the smallest value that makes its point — the transaction asks for 21,001 and the block allows 21,000, so it is rejected for exceeding the allowance:

tx = Transaction(gas_limit=21001, ..., error=TransactionException.GAS_ALLOWANCE_EXCEEDED)
env.gas_limit = ZeroPaddedHexNumber(21000)

The inclusion-list variant moves that transaction out of the block, leaving an empty block that still carries the 21,000 gas limit. A block's access list may hold at most block_gas_limit / 2000 items, and the mandatory system-contract items alone exceed what 21,000 allows, so the variant asks for VALID on a block no client can accept:

engine_newPayloadV6 returned INVALID, expected VALID.
ValidationError: BlockAccessListGasLimitExceeded: BAL has 25 items,
exceeds limit of 10 (block_gas_limit / 2000)

25 mandatory items need a gas limit of at least 50,000, which is what test_bal_gas_limit_boundary establishes — so the fixture contradicts a sibling test rather than expressing a client disagreement.

The variant is unfillable-as-expected for any consumer, which is why it is currently carried as a known failure downstream rather than gating anything.

Why removing the marker rather than raising the limit

Raising both values (say 100,001 against 100,000) would keep the plain test's meaning and clear the floor, but it rewrites a long-standing frontier fixture for every fork to fix a variant that only exists on one, and the minimal pair is part of what the test documents.

No coverage is lost either way. The scenario the variant would have exercised — an inclusion-list transaction that cannot be included does not make the block non-compliant — is covered purpose-built by test_block_with_failing_included_il_tx_is_valid in tests/bogota/eip7805_focil/, across nine scenarios.

Verification

tests/frontier/validation/test_transaction.py collects 14 tests unchanged, and ruff check / ruff format --check are clean. InclusionListVariantFixtureFormat.discard_fixture_format_by_marks drops the variant when the marker is absent, so this removes the inclusion-list fixtures for this test and leaves its other formats untouched.

I could not re-fill Bogota fixtures locally, so the error above is from the released tests-focil-devnet@v0.2.0 archive rather than from regenerated output.

jihoonsong and others added 21 commits August 13, 2026 15:40
…um#2643)

* feat: continue focil implementation

* feat: tests added
* feat: Bogota fork + EIP-7805 engine_newPayloadV6 bump

The first tests-focil release fixtures were filling FOCIL tests with
`newPayloadVersion: 5` because no EIP class declared the version bump
and there was no fork class to host EIP-7805. As a result `consume engine`
appended `inclusionListTransactions` as a 5th positional arg to V5, which
isn't a wire format defined by any Engine API spec — all clients reject it.

This adds:
- EIP7805 mixin under eips/bogota/ with engine_new_payload_version_bump
  and engine_forkchoice_updated_version_bump
- Bogota fork class (Amsterdam + EIP7805), reusing Amsterdam's t8n
- Switches FOCIL tests' valid_from marker to Bogota

After refilling, fixtures now use engine_newPayloadV6 / forkchoiceUpdatedV5,
matching the canonical Engine API PR (ethereum/execution-apis#626).

* Address review: BogotaEIPs auto-discovery + feature.yaml --from=Bogota

- Add BogotaEIPs sentinel that auto-discovers eip_*.py modules under
  eips/bogota/, mirroring AmsterdamEIPs. Bogota fork class now inherits
  BogotaEIPs instead of referencing eips.EIP7805 explicitly.
- Switch focil entry in .github/configs/feature.yaml from
  --until=Amsterdam to --from=Bogota so the FOCIL job fills against
  the Bogota fork rather than ending at Amsterdam.

---------

Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com>
…msterdam (ethereum#3370)

* fix(tests,tools): make the EIP-7805 FOCIL tests fill on the current Amsterdam

21 of the 24 FOCIL tests do not fill on this branch. Three independent causes,
none of them in the tests' assertions.

t8n passed decoded transactions where raw ones are expected.
check_inclusion_list_transactions takes Tuple[LegacyTransaction | Bytes, ...]
and calls get_transaction_hash, which asserts isinstance(tx, (LegacyTransaction,
Bytes)); the caller passed convert_transaction(...), which returns a decoded
fork transaction, so every typed-transaction case died on AssertionError. Both
tuples now go through encode_transaction.

The scenarios left no gas budget for the block's own access list. They size
block_gas_limit to their transactions alone, but the EIP-7928 budget is
block_gas_limit // GAS_BLOCK_ACCESS_LIST_ITEM, and the system-contract
predeploys touched every block plus senders, recipients and coinbase come to
about 30 items against a limit of about 22.

Raising the limit alone would change what the scenarios assert, because the gas
a pending inclusion-list transaction is allowed comes from block_gas_limit minus
the gas the included transactions use. So build_block raises the limit and
spends exactly the same amount on ballast transactions, leaving remaining_gas --
and therefore every "fits" and "does not fit" boundary -- unchanged. The ballast
is a whole number of empty transfers so the amount is always representable on
the fork's calldata-cost lattice.

One funding transfer predated value-carrying transactions costing extra
intrinsic gas. test_unsatisfied_when_block_tx_funds_pending_il_sender sizes and
funds alice's transfer with calc(), but it carries value, so it needs
calc(sends_value=True) and was failing INTRINSIC_GAS_BELOW_FLOOR_GAS_COST.

Assertions are untouched: of the changed lines in test_focil.py, none touch
expected_status, pytest.param, assertions, sender balances, nonces or
inclusion-list composition. Scenario ids are unchanged and the reference
implementation validates all 24 expected outcomes during fill.

* Apply suggestions from code review

Co-authored-by: Mario Vega <marioevz@gmail.com>

* fix: lint

---------

Co-authored-by: Mario Vega <marioevz@gmail.com>
… existing tests (ethereum#3373)

* feat(test-specs): Add inclusion list satisfied verification to block

* fix(test-specs): fix framework on missing inclusion list

* refactor(tests): refactor existing focil tests

* nit(test-specs): Unnecessary comment

* Apply suggestions from code review

Co-authored-by: spencer <spencer.tb@ethereum.org>
Co-authored-by: Mario Vega <marioevz@gmail.com>

---------

Co-authored-by: spencer <spencer.tb@ethereum.org>
…, implement IL inclusion_test (ethereum#3401)

* fix(specs,test-forks): Fix Bogota filling with EIP-7805

* fix(test-fixtures,test-specs): Update Engine API to latest version

* fix(test-fixtures): Label suffix fix (backport safe)

* fix(test-fixtures): Unit tests (backport safe)

* feat(test-specs): Implement inclusion list variant of inclusion_test marked tests

* bug(specs): Fix type-3, invalid RLP, and invalid chain id txs in inclusion lists

* bug(test-specs): Fix inclusion_test filling

* fix(tests): Blob test
…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>

This branch has not been deployed

No deployments
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.

6 participants