Skip to content

Conversation

@gurukamath
Copy link
Contributor

๐Ÿ—’๏ธ 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.

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.

@gurukamath
Copy link
Contributor Author

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

LGTM! However, the genesis RLP check is enabled now for json_infra but the fixtures don't yet have the changes you made. So the json_infra blockchain tests will continue to fail until we upgrade to a newer release which has the genesis fix. I'm ok with merging this in and then updating to a new release in another PR

@fselmo
Copy link
Contributor

fselmo commented Dec 8, 2025

I'm ok with merging this in and then updating to a new release in another PR

Right, this was my expectation here. We plan to make a new release this week ๐Ÿ‘๐Ÿผ

@fselmo fselmo merged commit 323d882 into ethereum:eips/amsterdam/eip-7928 Dec 8, 2025
12 of 14 checks passed
fselmo added a commit that referenced this pull request Dec 9, 2025
* 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>
fselmo added a commit that referenced this pull request Dec 9, 2025
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-eip Category: tracking implementation of an EIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants