Skip to content

feat(specs,tests): Implement EIP-2780#2175

Merged
gurukamath merged 1 commit into
ethereum:eips/amsterdam/eip-2780from
gurukamath:eip-2780/implement-eip-final
Apr 24, 2026
Merged

feat(specs,tests): Implement EIP-2780#2175
gurukamath merged 1 commit into
ethereum:eips/amsterdam/eip-2780from
gurukamath:eip-2780/implement-eip-final

Conversation

@gurukamath

@gurukamath gurukamath commented Feb 9, 2026

Copy link
Copy Markdown
Contributor

🗒️ Description

Implement EIP-2780 and some basic tests.

Note: This PR contains some testing framework changes which are not compatible with other Amsterdam tests. This PR however, does not focus on fixing those. Those should be fixed in a future PR.
As a result, the CI is not likely to pass for Amsterdam

🔗 Related Issues or PRs

issue #1940

✅ Checklist

Cute Animal Picture

TBD

@gurukamath gurukamath marked this pull request as draft February 9, 2026 16:15
RazorClient pushed a commit to RazorClient/execution-specs that referenced this pull request Feb 11, 2026
…#2175)

* refactor(bal): absence validators -> BalAccountAbsentValues

* feat: raise for empty lists on BAL absence checks

* refactor: changes from comments on PR ethereum#2175
RazorClient pushed a commit to RazorClient/execution-specs that referenced this pull request Feb 11, 2026
…#2175)

* refactor(bal): absence validators -> BalAccountAbsentValues

* feat: raise for empty lists on BAL absence checks

* refactor: changes from comments on PR ethereum#2175

@SamWilsn SamWilsn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Quick, incomplete review

Comment thread src/ethereum/forks/amsterdam/vm/gas.py Outdated
Comment thread src/ethereum/forks/amsterdam/vm/gas.py Outdated
Comment thread src/ethereum/forks/amsterdam/fork.py
Comment thread src/ethereum/forks/amsterdam/vm/instructions/environment.py Outdated
@marioevz

Copy link
Copy Markdown
Member

There's a possibility this becomes a blocker for Devnet-3 since it's listed as requirement for https://eips.ethereum.org/EIPS/eip-8037 which is included.

Spencer checked with Maria, and it seems like it's not a hard requirement but it might be a good idea to get this ready to merge.

@gurukamath

gurukamath commented Feb 17, 2026

Copy link
Copy Markdown
Contributor Author

Spencer checked with Maria, and it seems like it's not a hard requirement but it might be a good idea to get this ready to merge.

To my mind, this needs further clarity, especially when it comes to the EIP-7702 interactions. @jochem-brouwer has nicely summarised some of the open questions in this eth magicians thread

@gurukamath

Copy link
Copy Markdown
Contributor Author

Raised a PR on the EIPs repo to clarify some interactions with EIP-7702 and EIP-7928. See here

We can proceed to write more tests specific to these cases once we agree on these points.

@gurukamath gurukamath force-pushed the eip-2780/implement-eip-final branch 3 times, most recently from 3f816ea to 0d197cc Compare February 19, 2026 15:51
@gurukamath gurukamath marked this pull request as ready for review February 19, 2026 15:52
@gurukamath gurukamath marked this pull request as draft February 19, 2026 17:49
@gurukamath gurukamath force-pushed the eip-2780/implement-eip-final branch from 044f9b3 to 4c9e3e4 Compare February 20, 2026 13:45
@codecov

codecov Bot commented Feb 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.40%. Comparing base (e8e9c38) to head (485232b).

Additional details and impacted files
@@                     Coverage Diff                     @@
##           eips/amsterdam/eip-2780    #2175      +/-   ##
===========================================================
+ Coverage                    86.47%   88.40%   +1.92%     
===========================================================
  Files                          599      524      -75     
  Lines                        37632    31721    -5911     
  Branches                      3795     3036     -759     
===========================================================
- Hits                         32542    28042    -4500     
+ Misses                        4526     3164    -1362     
+ Partials                       564      515      -49     
Flag Coverage Δ
unittests 88.40% <ø> (+1.92%) ⬆️

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.

@gurukamath gurukamath marked this pull request as ready for review February 24, 2026 10:59
@gurukamath

Copy link
Copy Markdown
Contributor Author

Converting back to draft so I can re-base this on the latest forks/amsterdam

@gurukamath gurukamath marked this pull request as draft February 24, 2026 14:10
@gurukamath gurukamath force-pushed the eip-2780/implement-eip-final branch 2 times, most recently from aa8045c to f04a782 Compare February 24, 2026 15:44
@gurukamath gurukamath marked this pull request as ready for review February 24, 2026 15:45
@gurukamath

gurukamath commented Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

@danceratopz This got a bit out of hand in terms of PR size. If this is too much, I'd be happy to split this up into the following 2 PRs.

  1. Implementation + Framework updates and EIP-specific tests (in this PR)
  2. Fix broken legacy tests including BALs and other older fork tests

Do let me know what you think

@SamWilsn SamWilsn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From the spec side, my review is complete.

Comment thread src/ethereum/forks/amsterdam/vm/gas.py Outdated
Comment thread src/ethereum/forks/amsterdam/vm/gas.py Outdated
Comment thread src/ethereum/forks/amsterdam/vm/instructions/environment.py
Comment thread src/ethereum/forks/amsterdam/vm/instructions/environment.py Outdated
Comment thread src/ethereum/forks/amsterdam/vm/instructions/system.py Outdated
Comment thread src/ethereum/forks/amsterdam/fork.py Outdated
Comment thread src/ethereum/forks/amsterdam/fork.py
@gurukamath gurukamath force-pushed the eip-2780/implement-eip-final branch from 33a093e to 354ecc8 Compare March 13, 2026 10:25
@gurukamath gurukamath closed this Mar 13, 2026
@gurukamath gurukamath reopened this Mar 13, 2026

@danceratopz danceratopz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @gurukamath, here's some initial feedback. I must admit, I need to go deeper and improve my understanding to do this justice in a second round. I haven't looked at the tests updated for interactions yet. Will come back to this.

Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/__init__.py Outdated
Comment thread packages/testing/src/execution_testing/forks/gas_costs.py Outdated
Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/spec.py Outdated
Comment thread src/ethereum/forks/amsterdam/vm/gas.py Outdated
Comment thread packages/testing/src/execution_testing/forks/forks/forks.py Outdated
Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_value_moving_transactions.py Outdated
Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_value_moving_transactions.py Outdated
Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_value_moving_transactions.py Outdated
Comment thread src/ethereum/forks/amsterdam/vm/gas.py Outdated
Comment thread src/ethereum/forks/amsterdam/vm/instructions/system.py Outdated
Comment thread src/ethereum/forks/amsterdam/vm/gas.py Outdated
Comment thread src/ethereum/forks/amsterdam/fork.py
@gurukamath gurukamath force-pushed the eip-2780/implement-eip-final branch from 8698153 to 1ae2024 Compare March 17, 2026 15:21
@danceratopz

Copy link
Copy Markdown
Member

@danceratopz This got a bit out of hand in terms of PR size. If this is too much, I'd be happy to split this up into the following 2 PRs.

  1. Implementation + Framework updates and EIP-specific tests (in this PR)

  2. Fix broken legacy tests including BALs and other older fork tests

That's a good idea, @gurukamath, I think splitting the older fork tests out would be a great idea to give more targeted reviews.

@danceratopz danceratopz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @gurukamath, here's something that stood out really quickly upon taking another look at these.

Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_value_moving_transactions.py Outdated
Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_value_moving_transactions.py Outdated
Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_value_moving_transactions.py Outdated
Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_value_moving_transactions.py Outdated
Comment thread tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_value_moving_transactions.py Outdated

@danceratopz danceratopz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @gurukamath, here's a second round after reviewing tests/amsterdam/eip7928_block_level_access_lists/.

Comment thread tests/amsterdam/eip7928_block_level_access_lists/test_block_access_lists.py Outdated
@pytest.mark.parametrize(
"fails_at_extcodesize",
[True, False],
ids=["oog_at_extcodesize", "successful_extcodesize"],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think "successful_extcodesize" as-is deviates from the original test's intent; it should be renamed to oog_phase2_at_extcodesize, in which case, how about?

Suggested change
ids=["oog_phase1_at_extcodesize", "oog_phase2_at_extcodesize"],

The question is do we need to add a new test case here "successful_extcodesize", or is it covered elsewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did not include a case here since it is covered specifically in tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/test_account_access_opcodes.py::test_account_access_opcode. The original idea was to keep the implementation (and tests) of EIP-7928 and EIP-2780 as independent as possible. Not sure how essential this separation is anymore after the debates we've been having about merging stuff to forks/amsterdam

Op.PUSH20(target_contract) # Target contract address
+ Op.EXTCODESIZE(address_warm=False) # Check code size (cold access)
+ Op.EXTCODESIZE(
address_warm=False, address_has_code=False

@danceratopz danceratopz Mar 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We only have an Op.STOP, but unless this changes too, we should rename "successful_extcodesize" to "phase2_oog_at_extcodesize". We could consider adding a third successful case but I think the happy case is covered here https://github.com/gurukamath/execution-specs/blob/eip-2780/implement-eip-final/tests/amsterdam/eip7928_block_level_access_lists/test_block_access_lists.py#L246.

Suggested change
address_warm=False, address_has_code=False
address_warm=False, address_has_code=True

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the test ids are a bit misleading and might need to change. The test essentially looks at the target being in BAL vs target not being in BAL. Pre 2780 these two scenarios exactly coincide with Success vs OOG of the opcode. Post 2780, they don't. Which is also why there is a disconnect between the target having code and us using address_has_code=False.

@danceratopz danceratopz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed all other non-specific Amsterdam tests for hard-coded constants. Didn't add inline comments here, just Clauded these up.Each addresses a few missing hard-coded gas constants from a single fork (Cancun, Prague, Osaka). I still need to self-review them:

Squashes the previous EIP-2780 branch onto the current forks/amsterdam
tip to account for the upstream GasCosts class refactor (module-level
GAS_* constants consolidated into GasCosts.* class attributes).

Spec changes (src/ethereum/forks/amsterdam/):
- Add COLD_ACCOUNT_COST_CODE (2600), COLD_ACCOUNT_COST_NO_CODE (500),
  STATE_UPDATE (1000) to GasCosts
- Reduce TX_BASE from 21000 to 4500
- Split cold account access in BALANCE/EXTCODESIZE/EXTCODECOPY/
  EXTCODEHASH/CALL/CALLCODE/DELEGATECALL/STATICCALL by code presence
- Restructure CALL value transfer: STATE_UPDATE + NEW_ACCOUNT for
  empty targets, 2 * STATE_UPDATE for non-empty
- Add compute_recipient_cost helper in process_transaction

Testing framework (packages/testing/):
- Add RecipientType enum and propagate sends_value / recipient_type /
  recipient_is_warm / recipient_delegation_is_warm through the
  TransactionIntrinsicCostCalculator signature in EIP2, EIP2930,
  EIP7623, EIP7702 and Frontier base
- Add EIP2780 class with gas_costs, transaction_intrinsic_cost_calculator,
  _calculate_call_gas and _with_account_access overrides
- Extend GasCosts dataclass with COLD_ACCOUNT_COST_CODE,
  COLD_ACCOUNT_COST_NO_CODE, STATE_UPDATE (default 0 for pre-Amsterdam)

Tests:
- Add tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/ covering value-
  moving transactions, calls, precompile transfers and account-access
  opcodes
- Update blob, 7623, 7702, BAL, and tx-gas-limit tests to use the
  extended intrinsic cost calculator signature
@gurukamath gurukamath force-pushed the eip-2780/implement-eip-final branch from 3ca1df3 to 485232b Compare April 21, 2026 09:49
@gurukamath gurukamath closed this Apr 21, 2026
@gurukamath gurukamath reopened this Apr 21, 2026
@gurukamath

Copy link
Copy Markdown
Contributor Author

@danceratopz I have merged the changes you suggested in the 3 PRs as well as rebased this on the latest forks/amsterdam since the base was starting to get pretty stale. I made a call to squash all the 39 PR commits into one. Otherwise, the rebase would just be very painful. One more quick look and I think we are ready to merge this.

Next steps would be to write tests for more scenarios as well as fix the fails in the ported_static tests. (Separate PRs of course)

@danceratopz danceratopz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @gurukamath for the rebase and merging the test updates in.

Two comments below. Even if they're valid, I don't want to block a merge, so giving an approve! We can tackle other fixes in follow-up PRs. But as you like!

Comment on lines +500 to +502
call_value_cost = (
Uint(0) if value == 0 else Uint(2) * GasCosts.STATE_UPDATE
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't this count as "self-call" as caller == to in CALLCODE? So there should only be one state update:

Suggested change
call_value_cost = (
Uint(0) if value == 0 else Uint(2) * GasCosts.STATE_UPDATE
)
call_value_cost = Uint(0) if value == 0 else GasCosts.STATE_UPDATE

If you agree, we should update the transaction_intrinsic_cost_calculator() and potentially tests this touches.

Also happy to do/discuss this as a follow-up!

Comment on lines +405 to +408
if call_target == EMPTY_ACCOUNT:
call_value_cost += GasCosts.STATE_UPDATE + GasCosts.NEW_ACCOUNT
else:
call_value_cost += Uint(2) * GasCosts.STATE_UPDATE

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we need to check whether we're in a "self-call" context here and only charge GasCosts.STATE_UPDATE in that case.

@gurukamath

Copy link
Copy Markdown
Contributor Author

Thanks @gurukamath for the rebase and merging the test updates in.

Two comments below. Even if they're valid, I don't want to block a merge, so giving an approve! We can tackle other fixes in follow-up PRs. But as you like!

Thanks for the review Dan.

Yes. The changes are relevant. The self-call semantics weren't part of he EIP when we implemented this PR and have only been added to the EIP recently, along with the transfer log costs associated with 7708. I am planning to add both of those in a follow-up PR. So I will go ahead and merge this for now.

@gurukamath gurukamath merged commit 8af3496 into ethereum:eips/amsterdam/eip-2780 Apr 24, 2026
37 of 51 checks passed
gurukamath added a commit that referenced this pull request May 5, 2026
)

Squashes the previous EIP-2780 branch onto the current forks/amsterdam
tip to account for the upstream GasCosts class refactor (module-level
GAS_* constants consolidated into GasCosts.* class attributes).

Spec changes (src/ethereum/forks/amsterdam/):
- Add COLD_ACCOUNT_COST_CODE (2600), COLD_ACCOUNT_COST_NO_CODE (500),
  STATE_UPDATE (1000) to GasCosts
- Reduce TX_BASE from 21000 to 4500
- Split cold account access in BALANCE/EXTCODESIZE/EXTCODECOPY/
  EXTCODEHASH/CALL/CALLCODE/DELEGATECALL/STATICCALL by code presence
- Restructure CALL value transfer: STATE_UPDATE + NEW_ACCOUNT for
  empty targets, 2 * STATE_UPDATE for non-empty
- Add compute_recipient_cost helper in process_transaction

Testing framework (packages/testing/):
- Add RecipientType enum and propagate sends_value / recipient_type /
  recipient_is_warm / recipient_delegation_is_warm through the
  TransactionIntrinsicCostCalculator signature in EIP2, EIP2930,
  EIP7623, EIP7702 and Frontier base
- Add EIP2780 class with gas_costs, transaction_intrinsic_cost_calculator,
  _calculate_call_gas and _with_account_access overrides
- Extend GasCosts dataclass with COLD_ACCOUNT_COST_CODE,
  COLD_ACCOUNT_COST_NO_CODE, STATE_UPDATE (default 0 for pre-Amsterdam)

Tests:
- Add tests/amsterdam/eip2780_reduce_intrinsic_tx_gas/ covering value-
  moving transactions, calls, precompile transfers and account-access
  opcodes
- Update blob, 7623, 7702, BAL, and tx-gas-limit tests to use the
  extended intrinsic cost calculator signature
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.

4 participants