fix(ported_static): Approach-1 + stale-skip cleanup for Amsterdam OoG-by-design tests#2843
Open
leolara wants to merge 4 commits into
Open
fix(ported_static): Approach-1 + stale-skip cleanup for Amsterdam OoG-by-design tests#2843leolara wants to merge 4 commits into
leolara wants to merge 4 commits into
Conversation
OoG-by-design ported_static tests are calibrated to land just below a cost boundary; on Amsterdam each fresh SSTORE-set and CREATE spills its state-gas portion back into regular gas when the per-tx reservoir is empty. Tests that expect "N SSTOREs and M CREATEs complete before OoG" need their gas budget lifted by N * sstore_state_gas + M * create_state_gas to land at the same intermediate state. `Fork.oog_budget_lift(sstores_before_oog=N, creates_before_oog=M)` composes the two existing state-gas helpers and returns 0 on pre-EIP-8037 forks (where both helpers are 0), so callers can apply it unconditionally without a fork guard. Unit-tested on Cancun (zero) and Amsterdam (cumulative spill).
…*/stRevert*/stWallet* tests Eight tests share the OoG-by-design shape `tx_gas = [oog_path, success_path]` where the success_path budget is tuned to barely complete a single CREATE, a CREATE plus a few SSTOREs, or a deploy chain. On Amsterdam EIP-8037 splits NEW_ACCOUNT, fresh SSTORE-set, and code-deposit cost into a regular portion plus a state-gas portion; with an empty reservoir, the state-gas spills back into regular gas and breaks the success_path budget. Apply `Fork.oog_budget_lift` with the right (creates, sstores, deploy_code_size) counts to lift the budget on Amsterdam only. Pre- EIP-8037 forks return 0 from the helper, so the original budget is preserved. Files (skip-list entries cleared): - stCreate2/test_create2_oo_gafter_init_code.py (-g1) - stCreate2/test_create2_oo_gafter_init_code_returndata2.py (-g1) - stCreateTest/test_create_oo_gafter_init_code.py (-g1) - stCreateTest/test_create_oo_gafter_init_code_returndata2.py (-g1) - stRevertTest/test_revert_sub_call_storage_oog.py (-g1-v0) - stRevertTest/test_revert_sub_call_storage_oog2.py (-g1-v0) - stWalletTest/test_wallet_construction_oog.py (-g1) - stWalletTest/test_multi_owned_construction_not_enough_gas_partial.py (-g1) Removes 8 entries from amsterdam_skip_list.txt.
…eTest 16-pair family
22 stSStoreTest files (`sstore_0to*`, `sstore_xto*` excluding `gas`,
`gas_left`, `change_from_external_call_in_init_code`) had 3 skip
entries each (`d{0,1,2}-g1`). Re-running these on Amsterdam with the
skip list disabled shows all 60 fixture variants per file already
pass without any code changes.
The post-state expectation at `g=1` is `contract_2: storage={1: 0}`
(only slot 1, asserted to be zero). On Cancun, ~14 of the 16
SSTORE pairs complete before OoG; on Amsterdam EIP-8037 the state-
gas spill cuts that to ~5 pairs. In both cases slot 1 ends at 0 and
the final `SSTORE(1, 1)` does not run, so the assertion holds on
both forks unchanged.
These were defensive skip entries from an earlier snapshot. Remove
all 66 entries; no test-file changes needed.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devnets/bal/7 #2843 +/- ##
================================================
Coverage ? 85.59%
================================================
Files ? 630
Lines ? 39610
Branches ? 3938
================================================
Hits ? 33903
Misses ? 5083
Partials ? 624
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:
|
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
1 participant
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
Follow-up to #2839 — applies the Approach-1 / Approach-2 strategies
described in
.mb/oog-by-design-amsterdam-approaches.mdto theOoG-by-design subset of Amsterdam-skipped ported_static tests.
Clears 74 skip entries (
tests/ported_static/amsterdam_skip_list.txt897 → 823).
Commits
feat(forks): add Fork.oog_budget_lift helper— new classmethodon
BaseForkthat composessstore_state_gas,create_state_gas,and
code_deposit_state_gasinto a single budget lift for testscalibrated to OoG just below an EIP-8037-affected cost boundary.
Returns 0 on pre-EIP-8037 forks, so callers apply it
unconditionally without a fork guard.
fix(ported_static): Approach-1 fork-conditional OoG lift—8 single-spillable-op tests where
tx_gas[1]is tuned to barelycomplete a CREATE / SSTORE / CREATE+SSTOREs on Cancun but OoGs on
Amsterdam due to state-gas spill. Lift the budget with
Fork.oog_budget_lift(...)using the right(creates, sstores, deploy_code_size) counts:
chore(ported_static): remove stale Amsterdam skip entries for stSStoreTest 16-pair family—22 stSStoreTest files (`sstore_0to*`, `sstore_xto*` excluding
`gas`, `gas_left`, `change_from_external_call_in_init_code`) had
3 skip entries each. Re-running these on Amsterdam with the skip
list disabled shows all 60 fixture variants per file already
pass without any code changes — the post-state assertion
(`contract_2: storage={1: 0}`) holds whether OoG fires at pair 14
(Cancun) or pair 5 (Amsterdam after state-gas spill). Remove all
66 stale entries, no test-file changes.
chore: ruff format oog_budget_lift unit test assertions—cosmetic format pass on the helper's unit test.
Approach 4 (1024-depth CALL family) — deferred
The 3 `call1024_oog` / `callcode1024_oog` files have a per-frame
state-gas interaction that the simple `oog_budget_lift` model
underestimates (depth dropped from 134→44 frames with a 3×SSTORE
lift, vs expected near-zero drop). They need per-frame analysis
beyond the helper's scope. Leaving them skipped; follow-up PR.
Verification
🔗 Related Issues or PRs
Follows #2839 on the same fork. Independent — can merge in either
order.
✅ Checklist
case).
edited tests; `@ported_from` preserved from upstream.