feat(scripts): use dynamic addresses in ported static tests#2695
feat(scripts): use dynamic addresses in ported static tests#2695leolara wants to merge 13 commits intoethereum:forks/amsterdamfrom
Conversation
8c0f890 to
f74f2a3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2695 +/- ##
================================================
Coverage 86.26% 86.26%
================================================
Files 599 599
Lines 37038 37038
Branches 3795 3795
================================================
Hits 31949 31949
Misses 4525 4525
Partials 564 564
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Update since the initial descriptionAnalyzer enhancements (round 2)
|
Final verification resultsCompleted the full fill+verify run (
The 117 fill failures are all from the 3 STRUCTURAL tests in Every trace that was successfully generated matches baseline under |
kclowes
left a comment
There was a problem hiding this comment.
This looks good to me @leolara, nice work!! I left a couple comments that claude flagged, so feel free to disregard if they're incorrect. @marioevz suggested to look at the pre_alloc mutable flag, so that one is probably worth addressing here or at least in a new PR shortly after this. I'll approve so I'm not blocking, and I'm happy to implement any of these comments if that's helpful too, just lmk.
| dynamic_eoa_addrs: set[Address | EOA] = set() | ||
| for acct in accounts: | ||
| if acct.is_eoa and acct.use_dynamic and acct.address is not None: | ||
| dynamic_eoa_addrs.add(acct.address) |
There was a problem hiding this comment.
Functionally harmless, so feel free to take or leave or address in a new PR, but:
dynamic_eoa_addrs only collects addresses of dynamic EOAs, so the skip check on this line only filters EOAs. Non-tagged dynamic contracts fall through to line 1696 and
get emitted as contract_N = Address(0x…) — which is then shadowed by pre.deploy_contract(...) in the accounts block. Tagged contracts are correctly handled by the ContractTag skip at
line 1672; this is the gap for the non-tagged path.
Example: tests/ported_static/stRandom/test_random_statetest307.py — line 36 declares contract_0 = Address(0x095E7BAE…), then line 66 rebinds contract_0 = pre.deploy_contract(...) with no
address= kwarg.
A scan of tests/ported_static/ shows this potentially affects 204 files / 360 variable bindings.
Suggested fix:
skip: set[Address | EOA] = set()
for acct in accounts:
if acct.use_dynamic and acct.address is not None:
skip.add(acct.address)Then in lines 1676, and 1686, something like:
if resolved in skip:
continue| {% if has_exceptions and not is_multi_case %} | ||
| @pytest.mark.exception_test | ||
| {% endif %} | ||
| @pytest.mark.pre_alloc_mutable |
There was a problem hiding this comment.
It looks like the majority of tests ported don't need this @pytest.mark.pre_alloc_mutable line (Claude says 1358/2137 don't fwiw).
Consider adding a boolean so tests can be run with the execute plugin:
ir.py — add field:
@dataclass
class IntermediateTestModel:
...
needs_mutable_pre: bool = Falseanalyzer.py — compute it before returning IR (around line 452):
needs_mutable_pre = (
not sender_ir.use_dynamic
or any(a.is_eoa and not a.use_dynamic for a in accounts)
or any(a.oversized_code for a in accounts)
)state_test.py.j2:162 — wrap the mark:
{% if needs_mutable_pre %}
@pytest.mark.pre_alloc_mutable
{% endif %}
| short_push_refs: set[Address] = set() | ||
| # True when a short-PUSH ref targets an address that is not a | ||
| # deployed contract in the pre-state (e.g. an external tag like | ||
| # <contract:0x...dead> only referenced from bytecode). In that case | ||
| # no contract can be pinned, so fall back to globally disabling | ||
| # dynamic addresses for the whole test. | ||
| short_push_unpinnable = False | ||
| for addr, cb in code_bytes_map.items(): | ||
| refs = _find_address_refs_in_bytecode(cb, all_known_addrs) | ||
| # Track deps on other contracts. Keep self-references — they | ||
| # create self-loops detected as cycles, forcing hardcoded addr. | ||
| deps[addr] = set(refs) & known_contract_addrs | ||
| for ref_addr, push_size in refs.items(): | ||
| if push_size < 20: | ||
| if ref_addr in known_contract_addrs: | ||
| short_push_refs.add(ref_addr) | ||
| else: | ||
| short_push_unpinnable = True |
There was a problem hiding this comment.
Claude generated, feel free to take or leave if incorrect:
At analyzer.py:1094-1111, a short-PUSH ref to a non-contract (e.g. a precompile at 0x01 funded as an EOA) sets short_push_unpinnable=True. The fallback at line 1225-1246 then disables dynamic for contracts only — EOAs escape and land at pre.fund_eoa() random addresses, losing the baseline precompile-funding semantic.
Example: tests/ported_static/stRevertTest/test_revert_precompiled_touch_exact_oog_paris.py emits addr_5 = pre.fund_eoa(amount=1) instead of pinning 0x01. The bytecode's CALL 0x01
hits the empty precompile, so touch semantics diverge from the filler.
Scope: 278 fillers have pre-state accounts at 0x01-0x10; only 4 are in FORCE_HARDCODED_TESTS, leaving ~274 unprotected.
Why trace verification didn't catch it: baseline traces were generated from the same converter output, so both sides share the bug and compare equivalent. An independent baseline (e.g.
retesteth against the original filler) would flag the divergence.
Fix: pin EOA short-PUSH targets individually instead of falling back to the global short_push_unpinnable:
# around analyzer.py:1094
known_eoa_addrs = {a.address for a in raw_accounts
if a.is_eoa and a.address is not None}
short_push_eoa_refs: set[Address] = set()
for addr, cb in code_bytes_map.items():
refs = _find_address_refs_in_bytecode(cb, all_known_addrs)
deps[addr] = set(refs) & known_contract_addrs
for ref_addr, push_size in refs.items():
if push_size < 20:
if ref_addr in known_contract_addrs:
short_push_refs.add(ref_addr)
elif ref_addr in known_eoa_addrs:
short_push_eoa_refs.add(ref_addr)
else:
short_push_unpinnable = True
# around analyzer.py:1146, after the propagation loop
for acct in raw_accounts:
if acct.address in short_push_eoa_refs:
acct.use_dynamic = False
The 4 existing allowlist entries then become candidates for removal pending an independent re-verification.
🗒️ Description
Modify
filler_to_pythonto generate ported static tests that use dynamically-assigned addresses (pre.fund_eoa(),pre.deploy_contract()withoutaddress=) instead of hardcoding every address from the original fillers. This makes the ported tests behave like hand-written EEST tests, where the framework assigns addresses at fill time.What changed
scripts/filler_to_python/analyzer.py— core logic:_bytes_to_op_expr(code_bytes, addr_to_var), so emitted Op expressions use variable names instead of hex literals.compute_create_address(...)expressions.Op.ADD(contract_var, ...)) — sequential-address dispatch tablesaddress=Op.ADD(Op.CALLDATALOAD(...), ...)) — runtime dispatchFORCE_HARDCODED_TESTSallowlist (51 tests) for cases that can't converge underexact-no-stacktrace verification with dynamic addresses: EIP-2929 gas differences, CREATE2 collisions, keccak-derived storage keys, structural tx-rejection differences, etc.scripts/filler_to_python/ir.py—use_dynamicflag onAccountIR,SenderIR,AccessListEntryIR;noncefield onSenderIR.scripts/filler_to_python/templates/state_test.py.j2— conditional rendering:pre.fund_eoa(amount=...)for dynamic senders/EOAs,pre.deploy_contract(...)withoutaddress=for dynamic contracts.scripts/filler_to_python/render.py—format_storagehandles string variable references alongside int values.packages/testing/src/execution_testing/client_clis/trace_comparators.py—exact-no-stacknow also excludesreturn_data(addresses produced by runtime opcodes likeCALLERland on the stack and inreturn_dataafterRETURN/REVERT; excluding only stack just shifts the divergence forward).tests/ported_static/— all 2151 test files regenerated.Trace verification
Verified against baseline traces (generated once from
forks/amsterdamtip) using--verify-traces-comparator exact-no-stack:🔗 Related Issues or PRs
N/A.
✅ Checklist
type(scope):.just static@ported_frommarker.