Skip to content

fix(spec,tests): Change BAL to List[AccountChange] #1844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
gurukamath wants to merge 3 commits into
base: eips/amsterdam/eip-7928
Choose a base branch
from
Open

fix(spec,tests): Change BAL to List[AccountChange] #1844

gurukamath wants to merge 3 commits into from
+48 −39

Conversation

@gurukamath
Copy link
Contributor

@gurukamath gurukamath commented Dec 4, 2025

🗒️ Description

The EIP does the follwoing updates

  1. Defines BlockAccessList = List[AccountChanges]. The specs currently have block access list as a dataclass which has an attribute calles account changes which is this list. This inconsistency creates issue when we try to decode amsterdam block rlp to block.
  2. The EIP also consumes the newly created amsterdam json fixtures in json_infra

✅ Checklist

Cute Animal Picture

Cute Animals - 12 of 12

@gurukamath gurukamath changed the title Amsterdam json infra fix(spec,tests): Change BAL to List[AccountChange] Dec 4, 2025
@gurukamath gurukamath requested a review from fselmo December 4, 2025 14:47
@gurukamath gurukamath added the C-eip Category: tracking implementation of an EIP label Dec 4, 2025
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (eips/amsterdam/eip-7928@67c1983). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...um/forks/amsterdam/block_access_lists/rlp_utils.py 25.00% 3 Missing ⚠️
Additional details and impacted files 
@@                    Coverage Diff                     @@
##             eips/amsterdam/eip-7928    #1844   +/-   ##
==========================================================
  Coverage                           ?   86.62%           
==========================================================
  Files                              ?      797           
  Lines                              ?    47984           
  Branches                           ?     4341           
==========================================================
  Hits                               ?    41566           
  Misses                             ?     5843           
  Partials                           ?      575
Flag Coverage Δ
unittests 86.62% <72.72%> (?)

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.

fselmo
fselmo approved these changes Dec 5, 2025
View reviewed changes
Copy link
Contributor

@fselmo fselmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gurukamath I added the bal in the genesis here and fixed the fill with your changes. Lmk what you think but this lgtm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fselmo fselmo fselmo approved these changes

Assignees

No one assigned

Labels

C-eip Category: tracking implementation of an EIP

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gurukamath @fselmo