fix(tests,tools): make the EIP-7805 FOCIL tests fill on the current Amsterdam - #3370
Merged
marioevz merged 3 commits intoAug 13, 2026
Merged
Conversation
…msterdam 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.
ilitteri
marked this pull request as ready for review
August 13, 2026 13:54
Member
|
cc @marioevz |
marioevz
self-requested a review
August 13, 2026 15:43
Member
marioevz
approved these changes
Aug 13, 2026
marioevz
left a comment
Member
There was a problem hiding this comment.
LGTM. I'll merge and then rebase on top of latest forks/amsterdam. Thanks!
Co-authored-by: Mario Vega <marioevz@gmail.com>
marioevz
added a commit
that referenced
this pull request
Aug 13, 2026
…msterdam (#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>
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
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
The FOCIL tests on this branch don't fill: 21 of 24 fail. Three independent causes, none of them in the tests' assertions. With these fixed all 24 fill, and the expected outcomes are unchanged.
1.
t8npasses decoded transactions where raw ones are expectedcheck_inclusion_list_transactionstakesTuple[LegacyTransaction | Bytes, ...]and callsget_transaction_hash, which assertsisinstance(tx, (LegacyTransaction, Bytes)). The t8n caller passesconvert_transaction(...), which returns a decoded fork transaction, so every typed-transaction case dies onAssertionError. Both tuples now go throughencode_transaction.2. The scenarios leave no gas budget for the block's own access list
They size
block_gas_limitto their transactions alone. The EIP-7928 budget isblock_gas_limit // GAS_BLOCK_ACCESS_LIST_ITEM, and the system-contract predeploys touched every block plus senders, recipients and coinbase come to ~30 items against a limit of ~22, so filling stops atBlockAccessListGasLimitExceededError.Raising the limit alone would change what the scenarios assert, because the gas a pending inclusion-list transaction is allowed derives from
block_gas_limit - gas_used_by_included_txs. Sobuild_blockraises the limit and spends exactly the same amount on ballast transactions, leavingremaining_gas— and therefore every "fits" / "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.3. One funding transfer predates value-carrying transactions costing extra intrinsic gas
test_unsatisfied_when_block_tx_funds_pending_il_sendersizes and funds alice's transfer withcalc(), but it carries value, so it needscalc(sends_value=True)and was failingINTRINSIC_GAS_BELOW_FLOOR_GAS_COST.Assertions are untouched
Of the changed lines in
test_focil.py, none touchexpected_status,pytest.param, assertions, sender balances, nonces or inclusion-list composition — they are gas-limit plumbing, imports, and thesends_valuefix. Scenario ids are unchanged, and the reference implementation validates all 24 expected outcomes during fill.Deliberately not included
This branch's Amsterdam differs from glamsterdam-devnet-8 (
ACCOUNT_WRITE8000 vs 9000,CREATE_ACCESSderived fromCOLD_STORAGE_ACCESSrather thanCOLD_ACCOUNT_ACCESS), so a contract creation's intrinsic gas is 23000 here against devnet-8's 24000. That is a rebase question rather than a bug, so the gas schedule is left alone — flagging it becauseunsatisfied_with_contract_creating_pending_il_txis the one scenario whose outcome moves with it.Also worth noting: #3307 relocates
evm_tools, so change 1 will need rebasing onto the new path if that lands first.Testing
uv run fill tests/bogota/eip7805_focil --fork Bogota→ 24 passed.The filled fixtures were additionally run against a client implementation (ethrex) as a cross-check.