Enhance Ported Test

Enhance Ported Test

Future-proof and clean up a test under tests/ported_static/. These tests were machine-ported from the legacy ethereum/tests static fillers (YAML/JSON) and carry a lot of boilerplate, hardcoded values, and weak/incomplete post-state checks. This skill is the ordered methodology for turning one into idiomatic, robust Python.

This skill is a living document: it captures the cases we have validated so far. Real tests will hit shapes not covered here — that is expected. When you find one, solve it, then add the new case/step to this file.

Goal

The end state is a test that passes on every fork from its valid_from onward (not just the baseline), expresses its intent explicitly, and has no fragile hardcoded constants. "Future-proof" = a later fork that re-prices gas, adds state costs, or changes account rules should not silently break it.

Core loop (subtractive)

Most of the work is removing boilerplate one piece at a time and proving the test still passes after each removal:

1. Baseline first. Before touching anything, fill the test and confirm it is green: uv run fill <path> --fork=<valid_from-fork> -q --clean.
2. Make one change.
3. Fill again (same fast command). Green → keep, move on.
4. Red → roll back that one change and analyze. A break is information: it tells you the thing you removed was load-bearing. Understand why before deciding whether to keep it, replace it with a dynamic equivalent, or leave it. Never paste a new expected value just to make red go green without understanding the change (see "Re-pinning" below).

Do low-risk, independent removals in small batches if you like, but anything that can plausibly interact (addresses, contracts, gas) goes one at a time so a failure is attributable.

Verification cadence

• Iterating: --fork=<baseline> (usually Cancun) — fast.
• Checkpoint / done: fill the whole valid_from range (omit --fork) so all deployed forks are exercised.
• Probe the future fork: explicitly --fork Amsterdam (or the latest fork that enables new EIPs). A gas/state-cost change there is the most likely future breakage. (Historical note: broken tests used to be parked in a tests/ported_static/amsterdam_skip_list.txt consumed by a local conftest; the list was emptied and both were removed. If a future fork's repricing breaks tests en masse, the same parking pattern — a substring-matched skip list plus a pytest_collection_modifyitems hook — is in git history.)
• fill output: writes to ./fixtures (--clean resets it), or pass --output <dir> for a scratch location. Do not use -o — that is pytest's --override-ini, not the output dir.

Ordered steps

Do them roughly in this order. Earlier steps unblock later ones (notably: audit the bytecode before touching gas, since restoring elided opcodes moves the budget; and max out gas before strengthening post-state, so added opcodes don't hit a gas ceiling).

0. Audit the bytecode against its # Source: yul comment

The # Source: yul blocks are the filler's source; the bytecode beside them is what solc emitted, and the optimizer is free to delete operations the test depends on. A port that faithfully reproduces the compiled bytecode therefore faithfully reproduces the hole the optimizer left. Do this first — restoring elided operations changes gas, so it must precede any budget work (steps 2 / 10).

The canonical fold: a self-cancelling SSTORE pair. Refund tests set a slot then clear it (sstore(k, 1); sstore(k, 0)) to earn a refund. In a fresh CREATE frame slot k is already zero, so solc folds the pair down to sstore(k, 0) — a no-op that generates no refund at all, leaving the test vacuous while still passing. Validated on test_create_oog_from_call_refunds, where 2 of 24 init codes had lost their sstore(1, 1): the OoG arms assert the sender's balance reaches exactly zero, which is the "refund earned inside a reverted frame must be discarded" check — and it was asserting nothing.

How to check. Disassemble every bytecode blob and diff it against the comment above it. Comparing opcode counts per mnemonic (sstore( in the Yul vs. SSTORE in the asm) catches the whole class in one pass. A throwaway script that ast-parses the test, evals each Op... assignment against a namespace of the test's constants, and walks bytes(...) through a PUSH*-aware opcode table is enough — there is no disassembler in execution_testing.

Tells in the ported source. Dense DUP/SWAP juggling (Op.SSTORE(key=Op.DUP2, value=Op.DUP2), a bare Op.PUSH1[0x1] + Op.PUSH1[0x0] prologue, a trailing argument-less Op.RETURN) is solc's stack reuse — the shape most likely to hide a fold, and unreadable regardless. Rewrite those from the Yul into explicit Op.SSTORE(key=..., value=...) / Op.RETURN(offset=..., size=...) form: it restores the intent and makes the next audit trivial.

Benign deviations — do not "fix" them. solc drops a POP before a terminator (pop(call(...)); return(0, 1) compiles without the POP, as RETURN ignores leftover stack) and encodes repeated literal zeros as DUP1 chains (Op.CALL(..., args_offset=Op.DUP1, ...)). Both are semantically identical to the Yul. Only a missing or added state-changing operation is a real deviation.

Expect to re-budget afterwards. Restoring an elided op adds its cost — a zero->non-zero SSTORE is ~22.1k pre-EIP-8037 and ~97.9k of state gas on Amsterdam — so a test with a hardcoded gas_limit may now OOG. That is usually not a regression you introduced: it reveals that the sibling cases which never lost their op were already failing on the future fork for the same reason. Establish this before re-budgeting by copying the pre-change file aside under a different test name, filling both, and diffing the failure sets — in the validated case that separated 12 pre-existing Amsterdam failures from the 3 the fix added.

Verify the restoration is observable, not merely green. Fill before and after and reconcile the gas delta. Above, consumption moved 77731 -> 97857 (the added cold SSTORE, minus the reset dropping to a warm 100) and the 19900 refund was capped by EIP-3529 at 97857 // 5 = 19571, giving the reported 78286 exactly. A delta you cannot account for means the rewrite changed the program (see "Re-pinning" below).

1. Remove env

Delete the Environment(...) block, the env=env arg to state_test, and any now-orphaned vars (coinbase) and the Environment import. The framework supplies sensible defaults. Keep env only if the post asserts on the coinbase/fee_recipient balance, or the bytecode reads block fields (NUMBER, TIMESTAMP, PREVRANDAO, BASEFEE, GASLIMIT, COINBASE). fee_recipient=sender alone is not a reason to keep it.

2. Remove gas_limit from the transaction (if gas is not the subject)

This is the common case and belongs early. Omitting gas_limit maxes out the gas the tx receives, so the body executes fully. See the [Transactions section] (../write-test/SKILL.md#transactions) of the write-test skill.

• Remove it when the test is about behavior and just needs to run to completion. This also lets you delete any per-fork gas band-aids (e.g. fork.is_eip_enabled(8037) budget bumps) and often the fork param itself.
• Keep it only for genuinely gas-sensitive tests (OOG boundaries, intrinsic-gas, code-deposit limits, or gas metering) — see step 10.
• Gas-snapshot tests are gas-sensitive. If the post asserts a stored GAS reading or a SUB(@gas_before, GAS) delta (legacy slots 0 / 0x64), the test measures gas — handle it under step 10 (preserve via CodeGasMeasure), do not just strip gas_limit. This was the dominant skip-list shape: the stored gas value is exactly what EIP-8037 re-prices and breaks.
• EIP-8037 caveat: when you omit gas_limit on a test that measures an operation incurring state gas (account creation, storage writes), add state_gas_reservoir=0 to the tx, or that state gas is silently dropped from the measurement on EIP-8037 forks (see step 10). Pure-execution opcodes (e.g. PUSH0, arithmetic) have no state gas and do not need it.
• Do not add a comment explaining the absence of gas_limit; omission is the default.

3. Remove hardcoded contract nonce

Drop nonce=0 from pre.deploy_contract(...). If a compute_create_address(..., nonce=N) in the post depends on it, keep them consistent.

4. Remove hardcoded addresses (one contract at a time)

Two sub-cases:

• Value discarded: a contract = Address(0x...) literal that is immediately overwritten by pre.deploy_contract(...) (no address=). Just delete the literal; the deploy returns a fill-generated address.
• Value passed to address=: remove both the literal and the address= argument, per contract, filling after each.
• No-op case: to=None creation tests often have no hardcoded address at all (the created address is compute_create_address(sender, nonce=0)). Confirm by grepping for Address(0x / address=.
• On break: some bytecode hardcodes that address as a CALL/CREATE target (or the tx to/data). Thread the dynamic address through the caller and the tx entry point instead.
• Self-reference: a contract that hardcodes its own deploy address (e.g. Op.BALANCE(0xF172…) where 0xF172… is its own address=). Threading a fill-generated address in is impossible (chicken-and-egg), so replace the self-reference with the opcode that yields it at runtime — Op.BALANCE(Op. ADDRESS). Don't substitute a different opcode that happens to be shorter (e.g. Op.SELFBALANCE) if it changes what the test exercises.
• Remove @pytest.mark.pre_alloc_mutable once the test no longer hardcodes addresses/nonces or assigns pre[...] directly — i.e. all allocation now goes through fund_eoa / deploy_contract / nonexistent_account. Fill to confirm.

5. Remove easy boilerplate values

Independent and usually safe (batchable): pre.fund_eoa(amount=...) → fund_eoa(); tx value; tx data when it is empty (Bytes("")); explicit gas price fields. Keep any of these that the post actually checks or that triggers the behavior under test.

• Drop opcode args that just pass their default. Ported bytecode often spells out zero operands that are already the default, e.g. Op.CALL(..., args_offset=0, args_size=0, ret_offset=0, ret_size=0) — all four are 0 by default. Removing them is a no-op on the assembled bytecode (verify once with bytes(a) == bytes(b)) and cuts noise. Applies to any opcode arg equal to its default.
• Drop the hardcoded subcall gas operand — this is a correctness fix, not cosmetics. Op.CALL/CALLCODE/DELEGATECALL/STATICCALL default gas to Op.GAS (forward all remaining). Ported fillers hardcode a constant (gas=0xEA60, gas=0x186A0) that was sized for the old gas schedule; once EIP-8037 inflates the callee's state gas (e.g. a zero→non-zero SSTORE jumps to ~97920), that fixed budget no longer covers the callee and the subcall OOGs on Amsterdam — a common reason a pure-behavior test lands on the skip list. Omit the operand so it forwards everything. Caveat: forwarding all gas via Op.GAS misbehaves on pre-EIP-150 (Homestead) — the sweep (step 11) fails only there, so such tests floor at TangerineWhistle. Keep an explicit gas operand only when the amount forwarded is the subject (an OOG-boundary test). Budget vs. subject: before dropping the operand, ask why the constant has its value. A mid-sized constant (0xEA60) is a budget sized for the old schedule — drop it. An absurd or boundary constant (2**256 - 20) is the subject: it exercises the 63/64 clamp on an oversized ask (a client that computed e.g. requested + stipend in wrapping arithmetic would forward almost nothing and fail). Keep it, name it (OVERSIZED_GAS_ASK), and state the intent in a comment. Validated on test_make_money.
• A codeless / absent call target is pre.nonexistent_account(), not pre.fund_eoa(amount=0). It yields an address guaranteed to hold no code and no state, which is what "call an empty contract" tests mean.
• Drop a stale # noqa: F841 on contract = pre.deploy_contract(...) once the variable is actually used (in to= / the post); leaving it triggers RUF100.

6. (Parametrized tests) Analyze what the data parameter is

Look at tx.data / tx.to:

• Scenario A — data is a target contract address: the tx lands in a thin entry-point contract that just CALLs the address from calldata. Usually you can delete the entry-point and call the target directly, and the N targets are near-identical → replace N bytecode copies with a dynamic generator parameterized by the small difference. When the targets are gas-measurement contracts differing only by the measured opcode, the dedup collapses all the way to a single CodeGasMeasure(code=opcode) parametrized on the opcode (step 10) — the entry-point's CALL was only a delivery mechanism. Validated on test_push0_gas2 (PUSH0 vs PUSH1 0x00).
• Scenario B — data is initcode: spotted by to=None. Decide whether running inside initcode is required by the test (e.g. the test is about initcode-context behavior, per its title/docstring) or just an artifact of the static-filler format (most common — then the logic can move to a normal deployed contract). If required, convert the tx_data array into an initcode(d) generator function: even when variants are genuinely different programs, the function form lets each branch be labeled by intent, surfacing the one thing that varies.

7. (Parametrized tests) Simplify expect_entries_ / resolve_expect_post

First, identify which index actually discriminates — it is not always d. Ported tests also key on g (gas) or v (value); check both the expect_entries_ indexes (which axis is non--1) and which of tx_data[d]/tx_gas[g]/tx_value[v] is the list with >1 entry. The other two indexes are pinned/wildcard. (Example: test_add_non_const varies v — d/g are fixed at 0 and the indexes match on "value".) Precondition (to collapse to a per-case form): every entry's network is implied by valid_from and there is no expect_exception. Then the post is a pure function of the discriminating index.

• Convert expect_entries_ into a plain list of result dicts indexed by the discriminator — duplicating identical entries (e.g. data [0,1] → two slots) is fine and preferred; an explicit flat list is easiest to reason about.
• When the discriminator is a real quantity (the tx value or gas), parametrize directly on that quantity (parametrize("tx_value", [0, 1])) rather than an opaque index, feed it straight into the Transaction, and express the post as a function of it. A clean closed form is ideal — e.g. Account(storage={0: 2 * tx_value}) for a contract that stores ADD(BALANCE, BALANCE) of a balance equal to the sent value (this is the "encode relationships" idea from step 9 applied to the post).
• Cascade: delete the resolve_expect_post import, the _exc it returned, and the tx's error=_exc.
• Optionally merge the data-generator and the post-list into one if/elif/else on d that sets both initcode and post per case. This co-locates each case's bytecode with its expected state — the strongest readability win, and it tends to reveal incomplete verification. Use a final else so every branch binds both vars; declare initcode: Bytecode and post: dict above the switch. Prefer the array form when cases are many or the switch would be unwieldy; this is a judgment call.
• Clean up the parametrize signature. The ported "d, g, v" triple is usually overkill: drop the pinned/unused indexes from both the parametrize and the function signature, keep the discriminator, and rename it to something meaningful (and fork too, if no longer used). Parametrize on the renamed axis:
  • String values (e.g. parametrize("opcode", ["calldataload", "calldatacopy", "codecopy"])) read best when the cases are distinct programs; pytest derives the test ids straight from the strings (matching the old id=s), and the switch branches become if opcode == "calldataload".
  • Op values (e.g. parametrize("opcode", [Op.SLOAD, Op.TLOAD])) are cleaner only when the opcode plugs directly into a shared bytecode template; avoid forcing it when each case needs structurally different code.
  • Drop the verbose pytest.param(..., id=...) wrapping when the bare values already give good ids.

7b. Consolidate near-identical sibling files

Ported fillers often arrive as a fan of files with near-identical names that differ in one axis — test_non_zero_value_{call,callcode,delegatecall} × {,_to_empty,_to_one_storage_key,…}. Once enhanced to the same shape, join them into one parametrized test (parametrize("opcode, target_kind", …) with ids matching the old filenames), set up the varying piece (call op, target pre-state) from the params, and merge every source into a single ported_from list. One readable file replaces N. Validated: 10 NonZeroValue_* files → test_non_zero_value.py.

8. Strengthen post-state verification

Co-locating bytecode and post (step 7) often exposes that the ported test barely verifies anything. Improve coupling and observability:

• Couple the expectation to the bytecode. If a contract returns its own code (CODECOPY+RETURN), assert code=initcode instead of a hand-copied bytes.fromhex(...) — change the bytecode and the expectation follows.
• Make no-op results observable. Storing 0 is indistinguishable from not storing (and storage={} already asserts "all slots zero" — see Storage.must_be_equal). To genuinely prove a read returned zero, store a derived non-zero value (e.g. Op.ADD(Op.CALLDATALOAD(0), 1) → assert 1).
• Zero source data makes offset tests vacuous. A test that asserts an out-of-bounds read yields zeros proves nothing if the in-bounds data is also all zeros — any offset, right or wrong, reads zero. Supply non-zero source bytes (e.g. data=bytes(range(1, 33)) for a CALLDATACOPY test) so a client reading from a wrong in-bounds offset produces a visible mismatch. Ported fillers often ship all-zero calldata; the rewrite is the moment to fix it. Validated on test_copy_offset.
• Preserve every assertion the legacy filler made — count its slots. A ported post often pins two observables (e.g. the ask fillers stored both the callee-observed gas and the caller's net gas, which proves unused forwarded gas is credited back). When reframing, it is easy to carry over the headline assertion and silently drop the second. Diff the old post's slots against the new one and re-express each dropped slot dynamically (or justify its removal explicitly). Validated on test_raw_call_gas_ask (the caller reports its remaining gas up the stack as a second return word).
• Add a canary. Write a distinctive non-zero sentinel to an extra slot as the final step (e.g. Op.SSTORE(0x2, 0xC0DE)), and assert it. If creation reverts or the code doesn't run to completion, the slot stays zero and the test fails loudly instead of silently passing on a coincidentally-matching (often empty) account.
• Adding SSTOREs costs gas — this is why step 2 (max out gas) comes first.
• Spot a degraded port and restore its stated intent. A ported test whose name/source promises a scenario its values don't actually exercise is a bug in the port, not something to preserve faithfully. Classic tell: a *_after_value_transfer / *_with_value test that sends value=0, so the observable it names (a callee's CALLVALUE, a recipient's balance) is vacuously zero and would pass even if the behavior were broken. Fix it by supplying the missing ingredient (a non-zero tx value) and asserting the now-meaningful result (CALLVALUE == transferred, recipient balance moved) — note the restoration in the @manually-enhanced line. Validated on test_deleagate_call_after_value_transfer (DELEGATECALL preserves the enclosing frame's value). Read the test's name and source comment against what it actually checks; the gap is the enhancement. The compiler-optimized init code of step 0 is the same family, one level down: there the bytecode stopped matching the scenario its own Yul comment describes.

Engine X packing can introduce an intentional collision into another test. Two tests that deploy identical creator bytecode derive the same CREATE address. If one predeploys a collider there and the other requires it absent, their pre-allocations must not share a packed genesis. Give the collision test a named @pytest.mark.pre_alloc_group(...), then fill both with --generate-all-formats and verify Engine X matches its ordinary engine sibling. Validated on the stRevertTest CREATE collision and CREATE-OOG pair.

9. Introduce variables that encode relationships

Whenever a literal carries intent or two literals are logically linked, lift them into named variables that express the relationship, not just the value. E.g. create_value = 0xB fed to both Op.CREATE(value=create_value, ...) and the tx value=create_value - 1 documents an intentional off-by-one (insufficient balance) and keeps the two coupled so a future edit can't desync them. Same idea ties a CREATE's size operand to the memory/gas math that depends on it.

• Post-state derived from gas/fees. When the asserted value is a function of the gas charge (e.g. an origin BALANCE read mid-execution equals sender_balance - gas_limit * effective_gas_price), express it as that formula rather than a hardcoded number. Such a test is gas-sensitive — keep an explicit gas_limit (step 10), since the observable depends on it, but derive that gas_limit too — fork.transaction_intrinsic_cost_calculator()() + code.gas_cost(fork) + buffer (conservative metadata so it can't undershoot) — so it is neither a magic number nor fork-fragile. Validated on test_sender_balance (EIP-1559 effective-vs-max price).
• But first ask whether the gas-derived value is the subject or just noise. A ported test often pins the sender balance to initial − value − gas_used * price — pure filler bookkeeping, not what the test is about. If the real subject is a gas-independent fact (a value flow tx → caller → callee, a storage write, a created account), drop the gas_limit (step 2), drop the fragile sender-balance assertion, and instead assert the gas-independent facts, encoding them as a relationship (caller: INITIAL + tx_value - call_value, callee: INITIAL + call_value). Only reach for the "derive the fee formula" machinery above when the fee itself is the observable. Validated on test_make_money.
• A budget bounded on both sides needs a guard, not just a comment. When a test's OOG mechanism is itself gas-priced — the classic being an oversized code deposit, return(0, 5000) — raising gas_limit to fit the successful cases can quietly fund the failing ones, flipping them to success. That direction fails silently, because a passing test is the failure mode. Name the quantity (oversized_code_size = 5000), feed it to both the Op.RETURN operands and a guard: assert tx_gas[g] < oversized_code_size * fork.gas_costs().CODE_DEPOSIT_PER_BYTE. That constant is the last-resort form (see step 10's code_deposit_size note); it is legitimate here only because it understates the deposit on 8037 forks, keeping an "is this unaffordable?" guard conservative. Prefer Op.RETURN's metadata whenever the comparison can be expressed against the init code's own gas_cost(fork). Couple the sender's balance to the budget in the same breath — balance=tx_gas[g] * tx_gas_price — whenever the post asserts it reaches exactly zero; a hardcoded 0x3D0900 silently desyncs the moment the budget moves, and "burned the whole allowance" stops meaning anything. Validated on test_create_oog_from_call_refunds.