enable BPO1 and BPO2 for Amsterdam #1908
Open
Conversation
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
fix(tests): Fix Amsterdam filling after rebase fix(specs): Fix issues with new ruff + mypy rules after rebase - bal -> block_access_list; re-add custom rlp encoding for block access list - bytes to uint - move away from method-style - Update EIP-7928 implementation: system contracts at index 0, migrate to RLP - System contracts (parent hash, beacon root) now use block_access_index 0 - Transactions use block_access_index 1 to len(transactions) - Post-execution changes use block_access_index len(transactions) + 1 - Migrated from SSZ to RLP encoding as per updated EIP-7928 spec - Updated all tests to match new API and structure - Replaced tx_index with block_access_index throughout codebase - add system contract logic - add markdown docstrings - update BAL format; address comments - ssz encoding and bal validation - six ssz types - bal tests - balspecs fix: do not track setting empty code to a new account (ethereum#19) fix: track implicit SLOAD within SSTORE for OOG cases (ethereum#18) refactor: Put back explicit acct tracking outside 7702 delegation path (ethereum#17) fix non-tracked 7702 authority for invalid delegations (ethereum#16) * fix non-tracked 7702 authority for invalid delegations * fix: lint issues * fix: track delegation target when loaded as call target * fix: track delegation target when loaded as call target from call opcodes * chore: fix issues with documentation generation Fix self-destruct cases with pre-execution balance cache / tracking * fix self-destruct implementation * fix self-destruct tracking balance * fix it in the bal finalization by filtering * add balance reset and fix tests * simplify pre-balance tracking not using snapshots fix duplicated code entries for in transaction self destruct fix self destruct in same transaction bug fix call/delagate call tracking bug fix zero-value transfer tracking (ethereum#6) * fix zero-value transfer tracking * fix reverted frame tracking * rename variables * fix missing addresses bug * fix: docs run & move imports to top of file refactor: move rlp_utils to block_access_lists; bal -> block_access_lists Some remaining fixes due to large refactor in `forks/osaka`: - Move BALs from amsterdam -> forks/amsterdam - rename: build -> build_block_access_list - fix docc issues move state change tracker to State correct system contract addresses Fixes to communicate with BALs EEST branch: - fix(bal): Initialize the state tracker before system contract calls - We were missing system contract calls to beacon roots and history contracts. This change initializes the state tracker before system contract calls and passes the tracker to these calls if post-Amsterdam. - fix(docs): Fix issues with toxenvs: lint, doc, json_infra - fix(t8n): Only initialize the bal_change_tracker for amsterdam - feat(fork criteria): Index upcoming forks for better ordering / fix issues - chore(forks): Fix issues from lint after rebase with Osaka latest - fix(setuptools): Update packages to include amsterdam - chore(lint): Fix 'tox -e static' issues - Fix bug in tracker Manually cherry-picked from e72991b Author: nerolation - chore(tests): Attempt to resolve issues with CI tests - chore(lint): fix issues from running ``tox -e static`` locally - refactor(bal): Send BAL as a list over t8n tool - fix(amsterdam): Add change tracker to state test in t8n - chore(lint,tests): Fix tests after moving bal from osaka -> amsterdam - chore(forks): Move bals from Osaka to Amsterdam - chore(lint): Fix lint issues - refactor(bal): Send the full bal object and bal_hash over t8n - If we send the full object over JSON, we can model_validate() on ESST. - If we send the hash, once we fill the pydantic model, we can get the rlp and the hash and validate that our objects match while only really validating the parts of the BAL we are interested in for each test. - chore: point to working eest branch - chore(bals): Remove unused SSZ utils.py The SSZ implementation is no longer needed as we are now using RLP - refactor(bals): Clean up BAL module types and imports - Bytes -> Bytes32 type for storage slots - Remove unused imports / fix imports / fix linting - Update function signatures to match tracker - fix(bals-tx-index): Track bal indexes in t8n Keep track of BAL index state in t8n
* fix(specs): Fix zero value withdrawals BAL tracking * docs(specs): rename 'finalize' to 'normalize' in comments * docs(specs): remove reference to uint128 for balance tracking --------- Co-authored-by: Toni Wahrstätter <51536394+nerolation@users.noreply.github.com> Co-authored-by: Toni Wahrstätter <info@toniwahrstaetter.com>
refactor(tests): Prevent skips by splitting tests appropriately fix(tests): Use valid inputs to precompile tests chore(tests): linting fixes feat(tests): EIP-7928 test_bal_storage_write_read_cross_frame feat(tests): EIP-7928 test_bal_storage_write_read_same_frame feat(tests): EIP-7928 test_bal_nonexistent_account_access feat(tests): EIP-7928 test_bal_nonexistent_value_transfer feat(tests): EIP-7928 test_bal_precompiles feat(tests): EIP-7928 test_bal_withdrawal_to_coinbase_empty_block feat(tests): EIP-7928 test_bal_withdrawal_to_coinbase feat(tests): EIP-7928 test_bal_withdrawal_largest_amount feat(tests): EIP-7928 test_bal_withdrawal_to_precompiles fix(tests): expectation for nonexistent account in post fix(specs,tests): Fix withdrawal tests for BALs issue with idx==0 - `self.txs.successfully_parsed` is a list of transaction indexes, not transactions. The "if tx" check here would then check `if 0` which parses as a boolean ``False``. This means we would skip counting the tx if index=0 was successful. - Fixes some test expectations where `post_code` was being checked instead of ``new_code``. feat(tests): EIP-7928 test_bal_zero_withdrawal feat(tests): EIP-7928 test_bal_withdrawal_and_new_contract feat(tests): EIP-7928 test_bal_withdrawal_and_selfdestruct feat(tests): EIP-7928 test_bal_multiple_withdrawals_same_address feat(tests): EIP-7928 withdrawal_and_value_transfer_same_address feat(tests): EIP-7928 withdrawal_and_state_access_same_account feat(tests): EIP-7928 test_bal_withdrawal_no_evm_execution feat(tests): EIP-7928 test_bal_withdrawal_to_nonexistent_account feat(tests): EIP-7928 test_bal_withdrawal_empty_block feat(tests): EIP-7928 test_bal_withdrawal_with_transaction feat(tests): EIP-7928 coinbase
…1743) - Perform a similar check to balance changes and other tracker methods and keep only the last write.
ethereum#1742) - Validate static checks on the t8n BAL if it exists - IF the expectation also exists, validate against the expectation Keep these checks separate as this helps validation now that we fill for all tests, regardless if they have an expectation or not.
Co-authored-by: spencer spencer.taylor-brown@ethereum.org
- add commit_transaction_frame() - no net-zero filtering for cross-tx changes - keep max nonce per transaction when building BAL, remove block-level code filtering - filter net-zero code changes at tracking time (for 7702 txs) - use commit_transaction_frame() instead of merge_on_success() for tx->block commits
- Calculate all gas that we can without state access and check this gas before ever accessing state. This is the most sensible way for an implementation to behave and indeed was revealed to be the way clients are behaving, which differed from the specs. - Use fork.gas_costs to calculate gas costs, NOT hard-coded values. - Create a BAL expectation for the test that yielded discrepancies between clients and specs so this doesn't slip through again. Document this test in `test_cases.md`.
…ereum#1841) * refactor(spec-specs): Refactor state changes and their frames * chore(spec-specs): cleanup BAL logic; organize gas check for SSTORE * refactor(spec-specs): Changes from comments on PR ethereum#1841 * enhance: don't set defaults for state_changes
* fix(spec): update BAL type * fix(tests): run amsterdam jsons * fix(test-specs): Add BAL to genesis beyond Amsterdam fork --------- Co-authored-by: fselmo <fselmo2@gmail.com>
* rebase onto upstream * chore(fix) balance check in call before target access * merge test cases with usptream * merge test cases with usptream * chore(fix) format with ruff * chore(fix) revert call changes and add target to bal * merge test cases with usptream * improve wording * chore(formate) fix formatting and line length * refactor(test-tests): Use pre API where possible; explicit check for none in BAL * refactor(test-tests): Refactor opcode tests to bal opcodes test file --------- Co-authored-by: fselmo <fselmo2@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## eips/amsterdam/eip-7928 #1908 +/- ##
========================================================
Coverage 83.85% 83.85%
========================================================
Files 402 402
Lines 25105 25105
Branches 2287 2287
========================================================
Hits 21051 21051
Misses 3615 3615
Partials 439 439
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:
|
Contributor
|
I'd like to wait on this until maybe next release. We are just now moving into a new system for filling tests with BPOs and this seems like it's just a trick to please Geth and no other client. So I'd rather wait to include this until we align everything on the testing side and with all client expectations for the BAL tests. |
fselmo
force-pushed
the
eips/amsterdam/eip-7928
branch
2 times, most recently
from
be73122 to
ba0eec1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Geth is failing many tests as the BPOs 1 and 2 are hard coded