Skip to content

Commit 97d7e9c

Browse files
committed
refactor(types): Refactor BAL checks for explicit exclusion of acct changes
- From comments on PR ethereum#2067, we should allow for an explicit exclusion case for an entire set of account changes. If a test case warrants a sanity check that some account might have been impacted but shouldn't be accounted for, we should be able to specify that explicitly.
1 parent e4d2c4b commit 97d7e9c

File tree

4 files changed

+91
-71
lines changed

4 files changed

+91
-71
lines changed

src/ethereum_test_fixtures/blockchain.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
from ethereum_test_exceptions import EngineAPIError, ExceptionInstanceOrList
3939
from ethereum_test_forks import Fork, Paris
4040
from ethereum_test_types import (
41+
BlockAccessList,
4142
Environment,
4243
Requests,
4344
Transaction,
@@ -234,7 +235,9 @@ def genesis(cls, fork: Fork, env: Environment, state_root: Hash) -> "FixtureHead
234235
extras = {
235236
"state_root": state_root,
236237
"requests_hash": Requests() if fork.header_requests_required(0, 0) else None,
237-
"block_access_list_hash": Hash([]) if fork.header_bal_hash_required(0, 0) else None,
238+
"block_access_list_hash": (
239+
BlockAccessList().rlp_hash if fork.header_bal_hash_required(0, 0) else None
240+
),
238241
"fork": fork,
239242
}
240243
return FixtureHeader(**environment_values, **extras)

src/ethereum_test_specs/blockchain.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ def get_fixture_block(self) -> FixtureBlock | InvalidFixtureBlock:
338338
if self.withdrawals is not None
339339
else None
340340
),
341-
block_access_list=self.block_access_list.rlp() if self.block_access_list else None,
341+
block_access_list=self.block_access_list.rlp if self.block_access_list else None,
342342
fork=self.fork,
343343
).with_rlp(txs=self.txs)
344344

@@ -611,7 +611,7 @@ def generate_block_data(
611611
"by the transition tool"
612612
)
613613

614-
rlp = transition_tool_output.result.block_access_list.rlp()
614+
rlp = transition_tool_output.result.block_access_list.rlp
615615
computed_bal_hash = Hash(rlp.keccak256())
616616
assert computed_bal_hash == header.block_access_list_hash, (
617617
"Block access list hash in header does not match the "

src/ethereum_test_types/block_access_list/__init__.py

Lines changed: 66 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
these are simple data classes that can be composed together.
66
"""
77

8-
from typing import Any, Callable, ClassVar, List
8+
from functools import cached_property
9+
from typing import Any, Callable, ClassVar, Dict, List
910

1011
import ethereum_rlp as eth_rlp
1112
from pydantic import Field
@@ -138,10 +139,40 @@ def to_list(self) -> List[Any]:
138139
"""Return the list for RLP encoding per EIP-7928."""
139140
return to_serializable_element(self.root)
140141

142+
@cached_property
141143
def rlp(self) -> Bytes:
142144
"""Return the RLP encoded block access list for hash verification."""
143145
return Bytes(eth_rlp.encode(self.to_list()))
144146

147+
@cached_property
148+
def rlp_hash(self) -> Bytes:
149+
"""Return the hash of the RLP encoded block access list."""
150+
return self.rlp.keccak256()
151+
152+
153+
class BalAccountExpectation(CamelModel):
154+
"""
155+
Represents expected changes to a specific account in a block.
156+
157+
Same as BalAccountChange but without the address field, used for expectations.
158+
"""
159+
160+
nonce_changes: List[BalNonceChange] = Field(
161+
default_factory=list, description="List of expected nonce changes"
162+
)
163+
balance_changes: List[BalBalanceChange] = Field(
164+
default_factory=list, description="List of expected balance changes"
165+
)
166+
code_changes: List[BalCodeChange] = Field(
167+
default_factory=list, description="List of expected code changes"
168+
)
169+
storage_changes: List[BalStorageSlot] = Field(
170+
default_factory=list, description="List of expected storage changes"
171+
)
172+
storage_reads: List[StorageKey] = Field(
173+
default_factory=list, description="List of expected read storage slots"
174+
)
175+
145176

146177
class BlockAccessListExpectation(CamelModel):
147178
"""
@@ -151,22 +182,23 @@ class BlockAccessListExpectation(CamelModel):
151182
- Partial validation (only checks explicitly set fields)
152183
- Convenient test syntax with named parameters
153184
- Verification against actual BAL from t8n
185+
- Explicit exclusion of addresses (using None values)
154186
155187
Example:
156188
# In test definition
157189
expected_block_access_list = BlockAccessListExpectation(
158-
account_changes=[
159-
BalAccountChange(
160-
address=alice,
190+
account_expectations={
191+
alice: BalAccountExpectation(
161192
nonce_changes=[BalNonceChange(tx_index=1, post_nonce=1)]
162-
)
163-
]
193+
),
194+
bob: None, # Bob should NOT be in the BAL
195+
}
164196
)
165197
166198
"""
167199

168-
account_changes: List[BalAccountChange] = Field(
169-
default_factory=list, description="Expected account changes to verify"
200+
account_expectations: Dict[Address, BalAccountExpectation | None] = Field(
201+
default_factory=dict, description="Expected account changes or exclusions to verify"
170202
)
171203

172204
modifier: Callable[["BlockAccessList"], "BlockAccessList"] | None = Field(
@@ -191,7 +223,7 @@ def modify(
191223
from ethereum_test_types.block_access_list.modifiers import remove_nonces
192224
193225
expectation = BlockAccessListExpectation(
194-
account_changes=[...]
226+
account_expectations={...}
195227
).modify(remove_nonces(alice))
196228
197229
"""
@@ -213,8 +245,7 @@ def to_fixture_bal(self, t8n_bal: "BlockAccessList") -> "BlockAccessList":
213245
The potentially transformed BlockAccessList for the fixture
214246
215247
"""
216-
# Only validate if we have expectations
217-
if self.account_changes:
248+
if self.account_expectations:
218249
self.verify_against(t8n_bal)
219250

220251
# Apply modifier if present (for invalid tests)
@@ -235,49 +266,42 @@ def verify_against(self, actual_bal: "BlockAccessList") -> None:
235266
236267
"""
237268
actual_accounts_by_addr = {acc.address: acc for acc in actual_bal.root}
238-
expected_accounts_by_addr = {acc.address: acc for acc in self.account_changes}
239269

240-
# Check for missing accounts
241-
missing_accounts = set(expected_accounts_by_addr.keys()) - set(
242-
actual_accounts_by_addr.keys()
243-
)
244-
if missing_accounts:
245-
raise Exception(
246-
"Expected accounts not found in actual BAL: "
247-
f"{', '.join(str(a) for a in missing_accounts)}"
248-
)
249-
250-
# Verify each expected account
251-
for address, expected_account in expected_accounts_by_addr.items():
252-
actual_account = actual_accounts_by_addr[address]
253-
254-
try:
255-
self._compare_account_changes(expected_account, actual_account)
256-
except AssertionError as e:
257-
raise Exception(f"Account {address}: {str(e)}") from e
258-
259-
def _compare_account_changes(
260-
self, expected: BalAccountChange, actual: BalAccountChange
270+
for address, expectation in self.account_expectations.items():
271+
if expectation is None:
272+
# check explicit exclusion of address when set to `None`
273+
if address in actual_accounts_by_addr:
274+
raise Exception(f"Address {address} should not be in BAL but was found")
275+
else:
276+
# Address should be in BAL with expected values
277+
if address not in actual_accounts_by_addr:
278+
raise Exception(f"Expected address {address} not found in actual BAL")
279+
280+
actual_account = actual_accounts_by_addr[address]
281+
try:
282+
self._compare_account_expectations(expectation, actual_account)
283+
except AssertionError as e:
284+
raise Exception(f"Account {address}: {str(e)}") from e
285+
286+
def _compare_account_expectations(
287+
self, expected: BalAccountExpectation, actual: BalAccountChange
261288
) -> None:
262289
"""
263-
Compare two BalAccountChange models with detailed error reporting.
290+
Compare expected account changes with actual BAL account entry.
264291
265292
Only validates fields that were explicitly set in the expected model,
266293
using model_fields_set to determine what was intentionally specified.
267294
"""
268295
# Only check fields that were explicitly set in the expected model
269296
for field_name in expected.model_fields_set:
270-
if field_name == "address":
271-
continue # Already matched by account lookup
272-
273297
expected_value = getattr(expected, field_name)
274298
actual_value = getattr(actual, field_name)
275299

276-
# If we explicitly set a field to None, verify it's None/empty
300+
# explicit check for None
277301
if expected_value is None:
278302
if actual_value is not None and actual_value != []:
279303
raise AssertionError(
280-
f"Expected {field_name} to be empty/None but found: {actual_value}"
304+
f"Expected {field_name} to be `None` but found: {actual_value}"
281305
)
282306
continue
283307

@@ -314,7 +338,8 @@ def _compare_account_changes(
314338
# The comparison method will raise with details
315339
pass
316340

317-
def _compare_change_lists(self, field_name: str, expected: List, actual: List) -> bool:
341+
@staticmethod
342+
def _compare_change_lists(field_name: str, expected: List, actual: List) -> bool:
318343
"""Compare lists of change objects using set operations for better error messages."""
319344
if field_name == "storage_changes":
320345
# Storage changes are nested (slot -> changes)
@@ -390,6 +415,7 @@ def _compare_change_lists(self, field_name: str, expected: List, actual: List) -
390415
# Core models
391416
"BlockAccessList",
392417
"BlockAccessListExpectation",
418+
"BalAccountExpectation",
393419
# Change types
394420
"BalAccountChange",
395421
"BalNonceChange",

tests/amsterdam/eip7928_block_level_access_lists/test_block_access_lists.py

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
)
1414
from ethereum_test_tools.vm.opcode import Opcodes as Op
1515
from ethereum_test_types.block_access_list import (
16-
BalAccountChange,
16+
BalAccountExpectation,
1717
BalBalanceChange,
1818
BalCodeChange,
1919
BalNonceChange,
@@ -53,12 +53,11 @@ def test_bal_nonce_changes(
5353
bob: Account(balance=100),
5454
},
5555
expected_block_access_list=BlockAccessListExpectation(
56-
account_changes=[
57-
BalAccountChange(
58-
address=alice,
56+
account_expectations={
57+
alice: BalAccountExpectation(
5958
nonce_changes=[BalNonceChange(tx_index=1, post_nonce=1)],
6059
),
61-
]
60+
}
6261
),
6362
)
6463

@@ -105,20 +104,17 @@ def test_bal_balance_changes(
105104
bob: Account(balance=100),
106105
},
107106
expected_block_access_list=BlockAccessListExpectation(
108-
account_changes=[
109-
BalAccountChange(
110-
address=alice,
107+
account_expectations={
108+
alice: BalAccountExpectation(
111109
nonce_changes=[BalNonceChange(tx_index=1, post_nonce=1)],
112110
balance_changes=[
113111
BalBalanceChange(tx_index=1, post_balance=alice_final_balance)
114112
],
115113
),
116-
BalAccountChange(
117-
address=bob,
114+
bob: BalAccountExpectation(
118115
balance_changes=[BalBalanceChange(tx_index=1, post_balance=100)],
119116
),
120-
# TODO: Validate coinbase
121-
]
117+
}
122118
),
123119
)
124120

@@ -153,17 +149,16 @@ def test_bal_storage_writes(
153149
storage_contract: Account(storage={0x01: 0x42}),
154150
},
155151
expected_block_access_list=BlockAccessListExpectation(
156-
account_changes=[
157-
BalAccountChange(
158-
address=storage_contract,
152+
account_expectations={
153+
storage_contract: BalAccountExpectation(
159154
storage_changes=[
160155
BalStorageSlot(
161156
slot=0x01,
162157
slot_changes=[BalStorageChange(tx_index=1, post_value=0x42)],
163158
)
164159
],
165160
),
166-
]
161+
}
167162
),
168163
)
169164

@@ -196,12 +191,11 @@ def test_bal_storage_reads(
196191
storage_contract: Account(storage={0x01: 0x42}),
197192
},
198193
expected_block_access_list=BlockAccessListExpectation(
199-
account_changes=[
200-
BalAccountChange(
201-
address=storage_contract,
194+
account_expectations={
195+
storage_contract: BalAccountExpectation(
202196
storage_reads=[0x01],
203197
),
204-
]
198+
}
205199
),
206200
)
207201

@@ -266,19 +260,16 @@ def test_bal_code_changes(
266260
),
267261
},
268262
expected_block_access_list=BlockAccessListExpectation(
269-
account_changes=[
270-
BalAccountChange(
271-
address=alice,
263+
account_expectations={
264+
alice: BalAccountExpectation(
272265
nonce_changes=[BalNonceChange(tx_index=1, post_nonce=1)],
273266
),
274-
BalAccountChange(
275-
address=factory_contract,
267+
factory_contract: BalAccountExpectation(
276268
nonce_changes=[BalNonceChange(tx_index=1, post_nonce=2)],
277269
),
278-
BalAccountChange(
279-
address=created_contract,
270+
created_contract: BalAccountExpectation(
280271
code_changes=[BalCodeChange(tx_index=1, new_code=runtime_code_bytes)],
281272
),
282-
]
273+
}
283274
),
284275
)

0 commit comments

Comments
 (0)