fix(tests): address review feedback on enhanced ported static tests

.claude/commands/enhance-ported-test.md

@@ -136,6 +136,13 @@ under test.
   `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.
@@ -225,6 +232,21 @@ verifies anything. Improve coupling and observability:
   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
@@ -289,6 +311,19 @@ not drop it.**
   stored value is the opcode's real cost. `extra_stack_items` = items the
   measured code leaves on the stack (`CREATE`/`CALL` leave 1) — wrong value
   corrupts the result. `sstore_key` = the slot the post asserts.
+- **`extra_stack_items=1` silently discards a call's success flag — keep it
+  observable.** `CodeGasMeasure` SWAP/POPs the extra item, and gas alone
+  cannot replace it: a wrongly *failed* call refunds the child gas + stipend,
+  so it measures identically to a *success* into an empty callee, and for
+  `CALLCODE`/`DELEGATECALL` no balance moves either — the whole post-state is
+  then blind to the failure. When the measured op is a call whose success is
+  not otherwise observable, fold the flag into the measured window:
+  `store_code = Op.SSTORE(flag_slot, call_code, key_warm=False,
+  original_value=0, new_value=1)` with `extra_stack_items=0`, assert
+  `flag_slot: 1` in the post, and expect `store_code.gas_cost(fork)` (the
+  SSTORE's cost is now part of the measurement — and a failed call would
+  store 0, shifting the measured gas too, so the failure is doubly loud).
+  Validated on `test_non_zero_value`.
 - **Apply opcode metadata from the test's context** so `gas_cost(fork)` is
   correct (see `docs/writing_tests/opcode_metadata.md`). For `CALL`:
   `address_warm` (is the target pre-accessed?), `value_transfer` (value > 0?),
@@ -470,6 +505,26 @@ its per-directory count header (`# stXxx (N)`) and the `# Total entries:` count.
 Confirm with a full-range fill (`--fork` omitted) with the entry gone — that is
 the definition of done.
 
+**Final sweep checklist** — each of these has been missed in practice; check
+them one by one before calling the test done:
+- `@pytest.mark.pre_alloc_mutable` removed if no hardcoded addresses/
+  nonces/`pre[...]` remain (it silently skips the test in execute mode).
+- No machine-port placeholder docstrings left (`Test_<filename>.`) — the
+  module and function docstrings say what the test verifies, in
+  imperative mood ("Verify/Measure ...", not "Gas cost of ...").
+- Docstrings re-read against the *final* architecture: collapsing a
+  delivery CALL or moving value onto the tx makes "inherited from the
+  enclosing CALL"-style prose stale.
+- Inline magic operands named (`FORWARDED_GAS`, `GAS_SLOT`, ...) —
+  consistent with sibling files in the same directory.
+- Pinned budget constants guarded: anything like
+  `available = BUDGET - code.gas_cost(fork)` gets an
+  `assert available > 0, ...` so a future repricing that outgrows the
+  budget fails loudly at fill time instead of producing a garbage
+  expectation.
+- The old post's slots all accounted for (see step 8's "count its
+  slots").
+
 When done, offer to run `/lint`. Note that pydantic coercion warnings
 (`dict→Alloc/Storage`, `Bytecode→Bytes`, unfilled optional `Transaction` params)
 are false positives from the type checker, not real issues.

tests/ported_static/stArgsZeroOneBalance/test_add_non_const.py

@@ -1,19 +1,22 @@
 """
-Test_add_non_const.
+Verify ADD over non-constant operands: the contract adds its own balance to
+itself, where that balance equals the value sent by the transaction.
 
 Ported from:
 state_tests/stArgsZeroOneBalance/addNonConstFiller.yml
 
 @manually-enhanced: Do not overwrite. Parametrized on the transaction value
 (the real discriminator), the self-referential balance reads use
 `BALANCE(ADDRESS)` instead of a hardcoded address, and the post asserts the
-`2 * tx_value` result directly; env/gas boilerplate removed.
+`2 * tx_value` result directly; env/gas boilerplate removed. A canary slot
+keeps the `tx_value=0` arm observable (its result slot stays zero).
 """
 
 import pytest
 from execution_testing import (
     Account,
     Alloc,
+    Fork,
     StateTestFiller,
     Transaction,
 )
@@ -22,38 +25,43 @@
 REFERENCE_SPEC_GIT_PATH = "N/A"
 REFERENCE_SPEC_VERSION = "N/A"
 
+CANARY = 0xC0DE
+
 
 @pytest.mark.ported_from(
     ["state_tests/stArgsZeroOneBalance/addNonConstFiller.yml"],
 )
-@pytest.mark.valid_from("Cancun")
+@pytest.mark.valid_from("Frontier")
 @pytest.mark.parametrize("tx_value", [0, 1])
-@pytest.mark.pre_alloc_mutable
 def test_add_non_const(
     state_test: StateTestFiller,
     pre: Alloc,
+    fork: Fork,
     tx_value: int,
 ) -> None:
-    """Test_add_non_const."""
+    """Add the contract's own balance to itself and store the result."""
     sender = pre.fund_eoa()
 
     # ADD with non-constant operands: the contract's own balance added to
-    # itself. The balance equals the value sent by the transaction.
+    # itself. The balance equals the value sent by the transaction. The
+    # canary proves the code ran even when the stored result is zero.
     target = pre.deploy_contract(
         code=Op.SSTORE(
             key=0x0,
             value=Op.ADD(Op.BALANCE(Op.ADDRESS), Op.BALANCE(Op.ADDRESS)),
         )
+        + Op.SSTORE(key=0x1, value=CANARY)
         + Op.STOP,
     )
 
     # ADD(BALANCE, BALANCE) over a balance equal to the sent value.
-    post = {target: Account(storage={0: 2 * tx_value})}
+    post = {target: Account(storage={0: 2 * tx_value, 1: CANARY})}
 
     tx = Transaction(
         sender=sender,
         to=target,
         value=tx_value,
+        protected=fork.supports_protected_txs(),
     )
 
     state_test(pre=pre, post=post, tx=tx)

tests/ported_static/stCreateTest/test_create_transaction_call_data.py

@@ -1,7 +1,7 @@
 """
-Tests if CALLDATALOAD, CALLDATACOPY, CODECOPY and CODESIZE work...
-
-call data is always empty in initcode context and "code" is initcode.
+Verify CALLDATALOAD, CALLDATACOPY, CODECOPY and CODESIZE in the initcode
+context of a create transaction: call data is always empty and "code" is the
+initcode itself.
 
 Ported from:
 state_tests/stCreateTest/CreateTransactionCallDataFiller.yml

tests/ported_static/stDelegatecallTestHomestead/test_deleagate_call_after_value_transfer.py

@@ -1,5 +1,6 @@
 """
-Test_deleagate_call_after_value_transfer.
+Verify DELEGATECALL propagates the caller frame's context (CALLVALUE, CALLER,
+CALLDATA) into the delegate, after a value-bearing transaction.
 
 Ported from:
 state_tests/stDelegatecallTestHomestead/deleagateCallAfterValueTransferFiller.json

tests/ported_static/stDelegatecallTestHomestead/test_delegatecall_emptycontract.py

@@ -1,5 +1,6 @@
 """
-Test_delegatecall_emptycontract.
+Verify a DELEGATECALL to a codeless, nonexistent account succeeds without
+creating or touching the target.
 
 Ported from:
 state_tests/stDelegatecallTestHomestead/delegatecallEmptycontractFiller.json
@@ -58,6 +59,11 @@ def test_delegatecall_emptycontract(
         protected=fork.supports_protected_txs(),
     )
 
-    post = {caller: Account(storage={0: 1})}
+    # DELEGATECALL carries no value, so it must not create (or even touch)
+    # the target account.
+    post = {
+        caller: Account(storage={0: 1}),
+        empty: Account.NONEXISTENT,
+    }
 
     state_test(pre=pre, post=post, tx=tx)

tests/ported_static/stEIP150singleCodeGasPrices/test_raw_call_gas.py

@@ -1,5 +1,5 @@
 """
-Gas cost of CALL / CALLCODE / DELEGATECALL measured with CodeGasMeasure,
+Measure the gas cost of CALL / CALLCODE / DELEGATECALL with CodeGasMeasure,
 across value-transfer and memory-expansion variants. The callee records the
 gas it was forwarded.
 

tests/ported_static/stEIP150singleCodeGasPrices/test_raw_call_gas_ask.py

@@ -1,7 +1,7 @@
 """
-Gas forwarded to a subcall that asks for more than is available: the EIP-150
-"all but one 64th" rule, across CALL / CALLCODE / DELEGATECALL and their
-value-transfer and memory-expansion variants.
+Verify the EIP-150 "all but one 64th" rule: a subcall asking for more gas
+than is available receives 63/64 of it, across CALL / CALLCODE / DELEGATECALL
+and their value-transfer and memory-expansion variants.
 
 Ported from:
 state_tests/stEIP150singleCodeGasPrices/RawCallGasAskFiller.json
@@ -20,7 +20,9 @@
 intrinsic). Reframed so an outer call caps the caller frame at a known gas
 budget, the callee returns its observed GAS up to the top frame (no lower-frame
 SSTORE state-gas trap), and the expected value is derived from the fork:
-`all_but_one_64th(caller_gas - call.gas_cost(fork))`.
+`all_but_one_64th(caller_gas - call.gas_cost(fork))`. The caller also reports
+its remaining gas after the subcall, preserving the ported fillers' second
+assertion that unused forwarded gas is credited back.
 """
 
 import pytest
@@ -104,9 +106,10 @@ def test_raw_call_gas_ask(
     sender = pre.fund_eoa()
 
     # Callee returns the gas it observed on entry back to the caller.
-    gas_return_contract = pre.deploy_contract(
-        code=Op.MSTORE(0, Op.GAS) + Op.RETURN(0, 32),
+    gas_return_code = Op.MSTORE(0, Op.GAS, new_memory_size=32) + Op.RETURN(
+        0, 32
     )
+    gas_return_contract = pre.deploy_contract(code=gas_return_code)
 
     mem = MEMORY_SIZE if memory else 0
     ret_size = MEMORY_SIZE if memory else 32  # must fit the 32-byte GAS return
@@ -137,8 +140,11 @@ def test_raw_call_gas_ask(
             account_new=False,
             new_memory_size=new_memory_size,
         )
+    # After the subcall returns, the caller appends its own remaining gas to
+    # the return data, so the top frame can also assert that the unused part
+    # of the 63/64-forwarded grant was credited back to the caller.
     caller = pre.deploy_contract(
-        code=caller_call_code + Op.RETURN(0, 32),
+        code=caller_call_code + Op.MSTORE(32, Op.GAS) + Op.RETURN(0, 64),
         balance=value,
     )
 
@@ -153,20 +159,35 @@ def test_raw_call_gas_ask(
             args_offset=0,
             args_size=0,
             ret_offset=0,
-            ret_size=32,
+            ret_size=64,
         )
-        + Op.SSTORE(1, Op.MLOAD(0)),
+        + Op.SSTORE(1, Op.MLOAD(0))
+        + Op.SSTORE(2, Op.MLOAD(32)),
     )
 
     # EIP-150 forwards "all but one 64th" of the gas left after the call's own
     # cost; a value-bearing call additionally hands the callee the stipend.
     stipend = fork.gas_costs().CALL_STIPEND if value else 0
     available = CALLER_GAS - caller_call_code.gas_cost(fork)
+    assert available > 0, "CALLER_GAS must exceed the call's own cost"
     forwarded = available - available // 64
     expected_gas = forwarded + stipend - Op.GAS.gas_cost(fork)
 
+    # The callee's unconsumed gas returns to the caller: what the caller sees
+    # after the subcall is its budget minus the call's own cost and the
+    # callee's consumption (the stipend nets out on value-bearing calls).
+    expected_caller_gas = (
+        CALLER_GAS
+        - caller_call_code.gas_cost(fork)
+        + stipend
+        - gas_return_code.gas_cost(fork)
+        - Op.GAS.gas_cost(fork)
+    )
+
     tx = Transaction(sender=sender, to=entry)
 
-    post = {entry: Account(storage={0: 1, 1: expected_gas})}
+    post = {
+        entry: Account(storage={0: 1, 1: expected_gas, 2: expected_caller_gas})
+    }
 
     state_test(pre=pre, post=post, tx=tx)

tests/ported_static/stEIP150singleCodeGasPrices/test_raw_create_gas.py

@@ -1,5 +1,5 @@
 """
-Gas cost of CREATE measured with CodeGasMeasure, across value-transfer,
+Measure the gas cost of CREATE with CodeGasMeasure, across value-transfer,
 memory-expansion, and insufficient-balance (failure) variants.
 
 Ported from:

tests/ported_static/stEIP3855_push0/test_push0_gas.py

@@ -1,5 +1,5 @@
 """
-Test_push0_gas.
+Measure the gas cost of the PUSH0 instruction.
 
 Ported from:
 state_tests/Shanghai/stEIP3855_push0/push0GasFiller.yml
@@ -31,7 +31,7 @@ def test_push0_gas(
     pre: Alloc,
     fork: Fork,
 ) -> None:
-    """Test_push0_gas."""
+    """Measure PUSH0's gas cost against the fork-derived expectation."""
     sender = pre.fund_eoa()
 
     push0_code = Op.PUSH0

tests/ported_static/stEIP3855_push0/test_push0_gas2.py

@@ -1,5 +1,6 @@
 """
-Test_push0_gas2.
+Measure the gas cost of PUSH0 and of PUSH1 0x00: each case asserts its own
+fork-derived cost, which together demonstrate PUSH0 is the cheaper encoding.
 
 Ported from:
 state_tests/Shanghai/stEIP3855_push0/push0Gas2Filler.yml
@@ -38,7 +39,7 @@ def test_push0_gas2(
     fork: Fork,
     opcode: Bytecode,
 ) -> None:
-    """Measure that PUSH0 is cheaper than PUSH1 0x00."""
+    """Measure the parametrized push encoding's exact gas cost."""
     sender = pre.fund_eoa()
 
     measured = pre.deploy_contract(

tests/ported_static/stEIP5656_MCOPY/test_mcopy_copy_cost.py

@@ -33,23 +33,13 @@
 
 SRCS = [0x0, 0x1, 0x1F, 0x20]
 SIZES = [0x0, 0x1, 0x1F, 0x20, 0x21, 0xAEDF, 0xAEE0, 0xAEE1]
-_SIZE_LABEL = {
-    0x0: "0",
-    0x1: "1",
-    0x1F: "31",
-    0x20: "32",
-    0x21: "33",
-    0xAEDF: "44767",
-    0xAEE0: "44768",
-    0xAEE1: "44769",
-}
 
 
 @pytest.mark.ported_from(
     ["state_tests/Cancun/stEIP5656_MCOPY/MCOPY_copy_costFiller.yml"],
 )
 @pytest.mark.valid_from("Cancun")
-@pytest.mark.parametrize("size", SIZES, ids=lambda s: f"size{_SIZE_LABEL[s]}")
+@pytest.mark.parametrize("size", SIZES, ids=lambda s: f"size{s}")
 @pytest.mark.parametrize("src", SRCS, ids=lambda s: f"src{s}")
 def test_mcopy_copy_cost(
     state_test: StateTestFiller,

tests/ported_static/stMemoryTest/test_copy_offset.py

@@ -8,7 +8,7 @@
 
 @manually-enhanced: Do not overwrite. CODECOPY/CALLDATACOPY OOB-offset
 zero-fill folded into one parametrize; delivery-CALL dropped; dynamic
-addresses.
+addresses; nonzero tx calldata so a wrong in-bounds offset is observable.
 """
 
 import pytest
@@ -25,11 +25,16 @@
 REFERENCE_SPEC_VERSION = "N/A"
 
 # Copy 16 bytes from a source offset far past the end of code/calldata; the
-# out-of-bounds region reads as zeros, so the low 16 bytes of the pre-filled
-# word are cleared and the high 16 bytes remain 0xFF.
+# out-of-bounds region reads as zeros, which overwrite memory bytes 0..15
+# (the most-significant half of the word MLOAD reads back), leaving only the
+# low 128 bits of the pre-filled word set to 0xFF.
 OOB_OFFSET = 0xFFFF
 COPY_SIZE = 0x10
 EXPECTED = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
+# Nonzero calldata makes the CALLDATACOPY arm discriminate a wrong (in-bounds)
+# source offset from the correct out-of-bounds zero-fill; with empty calldata
+# every offset would read zeros and the assertion would be vacuous.
+TX_DATA = bytes(range(1, 33))
 
 
 @pytest.mark.ported_from(
@@ -63,6 +68,7 @@ def test_copy_offset(
     tx = Transaction(
         sender=pre.fund_eoa(),
         to=contract,
+        data=TX_DATA,
         protected=fork.supports_protected_txs(),
     )