Skip to content

Conversation

fselmo
Copy link
Contributor

@fselmo fselmo commented Aug 21, 2025

What was wrong?

This builds on #1338 but moves Block Access List (BAL) support to Amsterdam. I worked on this branch in conjunction with the EEST branch (ethereum/execution-spec-tests#2067) to get them talking to each other and they are currently working well. This is meant to be an initial implementation for BAL and for Amsterdam support in general. I fully expect that we will iterate on these changes as new PRs on top of forks/amsterdam.

How was it fixed?

Notable updates from #1338:

  • Added the index tracking to t8n tool
  • Passes the BAL object via t8n to be consumed by an appropriate pydantic model on EEST side
  • Passes the BAL hash over t8n
  • Properly creates Amsterdam fork and moves BAL related logic into Amsterdam

📣 Some notable changes for ease of review 📣

Some things to note since the PR is quite large (sorry for this):

  • Since we currently have two development forks underway, and I imagine this will be more common moving forward with faster shipping speeds, I had to tweak how the ForkCriteria is read by adding an index arg to Unscheduled here. This leaves Osaka with the default of 0 and I configured Amsterdam with the next index here. This keeps the correct expected order when the forks are collected. Without this change the is_after_fork logic starts to fall apart as "Amsterdam" comes before "Osaka" in the list.
  • From a similar conversation that I caught here, the state tracker is only passed post-amsterdam (examples 1, 2, etc...). I do think loading the args and appending impacts readability. If we want to refactor this to be for every fork, we can do so either here or create an issue and track in another PR once this is merged for easier tracking.
  • There is a long reference that I needed to add a file-specific line-too-long ignore for here. We may want to manage these later via a ruff.toml for better readability if this list grows long.

Cute Animal Picture

Screenshot 2025-08-27 at 16 11 26

@fselmo fselmo changed the base branch from forks/osaka to forks/amsterdam August 21, 2025 18:59
@fselmo fselmo force-pushed the feat/amsterdam-fork-and-block-access-lists branch 8 times, most recently from 16ca33d to 72cb7e0 Compare August 27, 2025 22:04
fselmo and others added 19 commits August 29, 2025 13:19
…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
Keep track of BAL index state in t8n as is done in the Osaka ``fork.py`` implementation.
  - Bytes -> Bytes32 type for storage slots
  - Remove unused imports / fix imports / fix linting
  - Update function signatures to match tracker
  The SSZ implementation is no longer needed as we are now using RLP
@fselmo fselmo force-pushed the feat/amsterdam-fork-and-block-access-lists branch from f4047ce to 39dcb41 Compare August 29, 2025 19:23
@fselmo fselmo deleted the branch ethereum:forks/amsterdam August 29, 2025 19:27
@fselmo fselmo closed this Aug 29, 2025
@fselmo fselmo reopened this Aug 29, 2025
@fselmo fselmo force-pushed the feat/amsterdam-fork-and-block-access-lists branch 2 times, most recently from ce4b394 to a817b8e Compare August 30, 2025 18:16
@fselmo fselmo force-pushed the feat/amsterdam-fork-and-block-access-lists branch from a817b8e to a5c7b29 Compare August 30, 2025 18:18
@fselmo fselmo marked this pull request as ready for review August 30, 2025 19:42
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 43.60576% with 2077 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.01%. Comparing base (51cabd8) to head (a5c7b29).

Files with missing lines Patch % Lines
src/ethereum/amsterdam/fork.py 20.06% 247 Missing ⚠️
src/ethereum/amsterdam/vm/instructions/system.py 10.13% 204 Missing ⚠️
.../ethereum/amsterdam/vm/instructions/environment.py 17.29% 153 Missing ⚠️
src/ethereum/amsterdam/state.py 32.27% 106 Missing and 1 partial ⚠️
src/ethereum/amsterdam/transactions.py 65.44% 94 Missing ⚠️
...c/ethereum/amsterdam/vm/instructions/arithmetic.py 16.21% 93 Missing ⚠️
src/ethereum/amsterdam/vm/interpreter.py 25.80% 92 Missing ⚠️
...um/amsterdam/vm/precompiled_contracts/alt_bn128.py 15.05% 79 Missing ⚠️
...dam/vm/precompiled_contracts/bls12_381/__init__.py 29.67% 64 Missing ⚠️
src/ethereum/amsterdam/vm/instructions/bitwise.py 17.33% 62 Missing ⚠️
... and 37 more
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@                 Coverage Diff                 @@
##           forks/amsterdam    #1381      +/-   ##
===================================================
- Coverage            94.94%   90.01%   -4.94%     
===================================================
  Files                  583      637      +54     
  Lines                34665    38346    +3681     
  Branches              3070     3470     +400     
===================================================
+ Hits                 32914    34518    +1604     
- Misses                1198     3267    +2069     
- Partials               553      561       +8     
Flag Coverage Δ
unittests 90.01% <43.60%> (-4.94%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants