feat(tests): adds EIP-7981 test and required framework changes by felix314159 · Pull Request #2144 · ethereum/execution-specs

felix314159 · 2026-02-05T11:24:52Z

🗒️ Description

uv run fill -v -s --clean ./tests/amsterdam/eip7981_increase_access_list_cost/ --until=amsterdam
TODO: test against clients

🔗 Related Issues or PRs

Solves Implementation Tracker
EIP 7981
Toni PR

✅ Checklist

All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
```
uvx tox -e static
```
All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
All: Considered updating the online docs in the ./docs/ directory.
All: Set appropriate labels for the changes (only maintainers can apply labels).
Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.
Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
Ported Tests: All converted JSON/YML tests from ethereum/tests or tests/static have been assigned @ported_from marker.

Cute Animal Picture

felix314159 · 2026-02-05T11:27:41Z

@gurukamath feel free to make adjustments / add more tests

gurukamath · 2026-02-05T11:37:10Z

@gurukamath feel free to make adjustments / add more tests

Sure. Will take a look

spencer-tb

Nice work. I know this is in draft but just to keep the ball rolling.

I think it would be good to check we don't duplicate tests here that are already present in berlin/eip2930_access_list or prague/eip7623_increase_calldata_cost. If we update the EIPs here to include the access list in the fork aware floor cost calculation we should be able to remove some of the tests from this PR. For example:

tests/berlin/eip2930_access_list/test_tx_intrinsic_gas.py we can add access_list to: data_floor_gas_cost_calculator(data=data, ...), here.
tests/prague/eip7623_increase_calldata_cost/conftest.py there are two instances we can add the access_list to: here & here.

As these methods are fork aware as long we fill these tests for Amsterdam, I think we should be able to remove test_transactions_without_access_list with some certainty.

Fork transition tests too! ;)

spencer-tb · 2026-02-05T12:46:03Z

tests/amsterdam/eip7981_increase_access_list_cost/conftest.py

+    )
+
+    # Calculate calldata floor cost
+    return fork_data_floor_cost_calculator(data=tx_data)


Does this need the access list here?

Suggested change

return fork_data_floor_cost_calculator(data=tx_data)

return fork_data_floor_cost_calculator(data=tx_data, access_list=access_list)

spencer-tb · 2026-02-05T12:49:39Z

tests/amsterdam/eip7981_increase_access_list_cost/helpers.py

+    return zero_bytes + nonzero_bytes * 4
+
+
+def find_floor_cost_threshold(


I dont think this helper is used anywhere?

spencer-tb · 2026-02-05T13:16:52Z

tests/amsterdam/eip7981_increase_access_list_cost/test_transaction_validity.py

+
+pytestmark = pytest.mark.valid_at("Amsterdam")
+
+


Should we param with @pytest.mark.with_all_tx_types across most of these tests. It would be good to have blob and type 4 txs tested.

spencer-tb · 2026-02-05T13:26:20Z

tests/amsterdam/eip7981_increase_access_list_cost/test_access_list_cost.py

+    According to EIP-7981:
+    - total_data_tokens = tokens_in_calldata + tokens_in_access_list
+    - floor_gas = TX_BASE_COST + total_data_tokens * TOTAL_COST_FLOOR_PER_TOKEN
+    """


We could add these to tests, to help aid debugging between framework/specs calculations on a gas mismatch. Not essential imo.

Suggested change

"""

"""

tx.expected_receipt = TransactionReceipt(

cumulative_gas_used=tx_intrinsic_gas_cost_including_floor_data_cost

)

gurukamath · 2026-02-05T12:35:16Z

src/ethereum/forks/amsterdam/transactions.py

@@ -603,10 +608,6 @@ def calculate_intrinsic_cost(tx: Transaction) -> Tuple[Uint, Uint]:
            zero_bytes += 1

    tokens_in_calldata = Uint(zero_bytes + (len(tx.data) - zero_bytes) * 4)


I think we should refactor the token data counting to a separate helper function and just call that here and in the access list gas section. I am thinking something like

def count_tokens_in_data(data: bytes) -> Uint: """ Count the tokens in an arbitrary input data. """ zero_bytes = 0 for byte in data: if byte == 0: zero_bytes += 1 return Uint(zero_bytes + (len(data) - zero_bytes) * 4)

gurukamath · 2026-02-05T12:37:24Z

src/ethereum/forks/amsterdam/transactions.py

            + auth_cost
        ),
-        calldata_floor_gas_cost,
+        floor_gas_cost,


We should also change the variable name in the validate_transaction function. The variable there continues to be called calldata_floor_gas_cost

spencer-tb · 2026-02-05T14:58:21Z

Can we point the PR to the eips branch: gh pr edit <PR_NUMBER> --base eips/amsterdam/eip-7981

felix314159 · 2026-02-06T11:40:25Z

Thanks for all the feedback @spencer-tb and @gurukamath. I implemented it, and after lunch I will try to rebase this PR on forks/amsterdam so that i can change the target fork to the EIP branch (which is based on forks/amsterdam). Right now this PR is based on the devnet2 branch (my fault), so if I now would just change the target branch it would pick up changes from our devnet branch that would make it hard to see what this PR does / hard to review / hard to run against clients due to too many changes files. From now on I will base new EIPs off of forks/amsterdam and when i want to run tests i locally run our workflow that creates the devnet branch.

Edit: Done! We now target the EIP branch

SamWilsn · 2026-02-06T16:44:07Z

src/ethereum/forks/amsterdam/transactions.py

+def count_tokens_in_data(data: bytes) -> Uint:
+    """
+    Count the tokens in an arbitrary input data.
+    """
+    zero_bytes = 0
+    for byte in data:
+        if byte == 0:
+            zero_bytes += 1
+
+    return Uint(zero_bytes + (len(data) - zero_bytes) * 4)
+
+


This should be backported to previous forks.

SamWilsn · 2026-02-06T16:45:26Z

src/ethereum/forks/amsterdam/transactions.py

+        if byte == 0:
+            zero_bytes += 1
+
+    return Uint(zero_bytes + (len(data) - zero_bytes) * 4)


Should this be something like:

Suggested change

return Uint(zero_bytes + (len(data) - zero_bytes) * 4)

return Uint(zero_bytes + (len(data) - zero_bytes) * CALLDATA_TOKENS_PER_NONZERO_BYTE)

?

SamWilsn · 2026-02-06T16:46:43Z

src/ethereum/forks/amsterdam/transactions.py

+    zero_bytes = 0
+    for byte in data:
+        if byte == 0:
+            zero_bytes += 1


Should the 1 be in a named constant? Something like CALLDATA_TOKENS_PER_ZERO_BYTE?

SamWilsn · 2026-02-06T16:50:00Z

src/ethereum/forks/amsterdam/transactions.py

+    zero_bytes = 0
+    for byte in data:
+        if byte == 0:
+            zero_bytes += 1
+
+    return Uint(zero_bytes + (len(data) - zero_bytes) * 4)


If we're iterating through all of data anyway, how about:

Suggested change

zero_bytes = 0

for byte in data:

if byte == 0:

zero_bytes += 1

return Uint(zero_bytes + (len(data) - zero_bytes) * 4)

return sum(Uint(4) if byte else Uint(1) for byte in data)

Or if that's too Pythonic:

Suggested change

zero_bytes = 0

for byte in data:

if byte == 0:

zero_bytes += 1

return Uint(zero_bytes + (len(data) - zero_bytes) * 4)

return sum(Uint(1) if byte == 0 else Uint(4) for byte in data)

SamWilsn · 2026-02-06T16:53:55Z

src/ethereum/forks/amsterdam/transactions.py

+    in the access list (addresses and storage keys), with zero bytes costing
+    1 token and non-zero bytes costing 4 tokens.


We should avoid putting constant values in docstrings. It's too easy to update the code and forget to update the docs. Instead, link to the relevant constant.

SamWilsn · 2026-02-06T16:54:21Z

src/ethereum/forks/amsterdam/transactions.py

        ),
    ):
        for access in tx.access_list:
+            # Storage access costs (EIP-2930)


You'll need to backport the comment as well.

SamWilsn · 2026-02-06T17:03:12Z

src/ethereum/forks/amsterdam/transactions.py

    else:
        create_cost = Uint(0)

+    # EIP-7981: Calculate access list tokens and costs


I'm not sure we should establish the pattern of commenting with an EIP number. If we take this approach, we'll end up with piles of comments where a block of code is introduced under EIP-X, then in a later fork EIP-Y will modify that block, making it misleading which EIP a sequence of lines belongs to. For example:

Fork T Fork T+1

# EIP-1234: The dingus is the rate of fleep dingus = a + b dingus += c ^ d dingus /= fleep(e)

# EIP-1234: The dingus is the rate of fleep dingus = a + b # EIP-4567: Frobulate the dingus dingus = frobulate(dingus) dingus += c ^ d # <- dingus /= fleep(e) # <-

The marked lines (<-) are now incorrectly attributed to EIP-4567 in Fork+1. Instead, I'd recommend omitting the EIP identifier in the comments, and instead describe the changes introduced by the EIP in the function's docstrings. The rendered diffs will make it pretty obvious what's changed.

Perhaps I'm being overly cautious, however.

felix314159 · 2026-02-16T14:23:24Z

Thanks for the feedback, I 'backported' all the way to Berlin (2930 introduced access lists). This code duplication is a feature right, so that each fork is standalone?

…, do we really need this check

spencer-tb reviewed Feb 5, 2026

View reviewed changes

gurukamath requested changes Feb 5, 2026

View reviewed changes

felix314159 force-pushed the eip7981 branch from f566236 to df0a728 Compare February 6, 2026 11:30

felix314159 force-pushed the eip7981 branch from df0a728 to aacac5f Compare February 6, 2026 15:25

felix314159 changed the base branch from devnets/bal/2 to eips/amsterdam/eip-7981 February 6, 2026 15:25

feat: adds 7981 src changes + tests

c01f95e

felix314159 force-pushed the eip7981 branch from aacac5f to c01f95e Compare February 6, 2026 15:27

felix314159 marked this pull request as ready for review February 6, 2026 15:28

felix314159 mentioned this pull request Feb 6, 2026

feat(specs): add EIP-7981 #2133

Closed

7 tasks

SamWilsn reviewed Feb 6, 2026

View reviewed changes

fix: sam feedback

b1ff7f7

fix: ethereum-spec-lint: 'the item .. has changed relative positions'…

c59096e

…, do we really need this check

	return fork_data_floor_cost_calculator(data=tx_data)
	return fork_data_floor_cost_calculator(data=tx_data, access_list=access_list)

		return zero_bytes + nonzero_bytes * 4


		def find_floor_cost_threshold(

-    """
+    """
+    tx.expected_receipt = TransactionReceipt(
+          cumulative_gas_used=tx_intrinsic_gas_cost_including_floor_data_cost
+    )

		@@ -603,10 +608,6 @@ def calculate_intrinsic_cost(tx: Transaction) -> Tuple[Uint, Uint]:
		zero_bytes += 1

		tokens_in_calldata = Uint(zero_bytes + (len(tx.data) - zero_bytes) * 4)

	return Uint(zero_bytes + (len(data) - zero_bytes) * 4)
	return Uint(zero_bytes + (len(data) - zero_bytes) * CALLDATA_TOKENS_PER_NONZERO_BYTE)

		in the access list (addresses and storage keys), with zero bytes costing
		1 token and non-zero bytes costing 4 tokens.


		pytestmark = pytest.mark.valid_at("Amsterdam")

Conversation

felix314159 commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗒️ Description

🔗 Related Issues or PRs

✅ Checklist

Cute Animal Picture

Uh oh!

felix314159 commented Feb 5, 2026

Uh oh!

gurukamath commented Feb 5, 2026

Uh oh!

spencer-tb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spencer-tb commented Feb 5, 2026

Uh oh!

felix314159 commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felix314159 commented Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

felix314159 commented Feb 5, 2026 •

edited

Loading

spencer-tb left a comment •

edited

Loading

felix314159 commented Feb 6, 2026 •

edited

Loading