Skip to content

refactor(spec-specs): Refactor state changes and frame hierarchy #1841

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

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

refactor(spec-specs): Refactor state changes and frame hierarchy #1841

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
06d9476
refactor(spec-specs): Refactor state changes and their frames
fselmo Dec 3, 2025
4569117
chore(spec-specs): cleanup BAL logic; organize gas check for SSTORE
fselmo Dec 3, 2025
2478c13
refactor(spec-specs): Changes from comments on PR #1841
fselmo Dec 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 51 additions & 29 deletions src/ethereum/forks/amsterdam/fork.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@
commit_transaction_frame,
create_child_frame,
get_block_access_index,
handle_in_transaction_selfdestruct,
increment_block_access_index,
merge_on_success,
normalize_balance_changes_for_transaction,
track_address,
track_balance_change,
track_nonce_change,
track_selfdestruct,
)
from .transactions import (
AccessListTransaction,
Expand Down Expand Up @@ -647,8 +647,12 @@ def process_system_transaction(
authorizations=(),
index_in_block=None,
tx_hash=None,
state_changes=system_tx_state_changes,
)

# Create call frame as child of tx frame
call_frame = create_child_frame(tx_env.state_changes)

system_tx_message = Message(
block_env=block_env,
tx_env=tx_env,
Expand All @@ -667,14 +671,14 @@ def process_system_transaction(
accessed_storage_keys=set(),
disable_precompiles=False,
parent_evm=None,
transaction_state_changes=system_tx_state_changes,
state_changes=call_frame,
)

system_tx_output = process_message_call(system_tx_message)

# Merge system transaction changes back to block frame
# Commit system transaction changes to block frame
# System transactions always succeed (or block is invalid)
merge_on_success(system_tx_state_changes)
commit_transaction_frame(tx_env.state_changes)

return system_tx_output

Expand Down Expand Up @@ -909,7 +913,9 @@ def process_transaction(
# The frame will read the current block_access_index from the block frame
increment_block_access_index(block_env.block_state_changes)
tx_state_changes = create_child_frame(block_env.block_state_changes)
block_access_index = get_block_access_index(block_env.block_state_changes)

# Capture coinbase pre-balance for net-zero filtering
coinbase_pre_balance = get_account(
block_env.state, block_env.coinbase
).balance
Expand Down Expand Up @@ -947,16 +953,30 @@ def process_transaction(
effective_gas_fee = tx.gas * effective_gas_price

gas = tx.gas - intrinsic_gas
increment_nonce(block_env.state, sender, tx_state_changes)

# Track sender nonce increment
increment_nonce(block_env.state, sender)
sender_nonce_after = get_account(block_env.state, sender).nonce
track_nonce_change(
tx_state_changes, sender, U64(sender_nonce_after), block_access_index
)

# Track sender balance deduction for gas fee
sender_balance_before = get_account(block_env.state, sender).balance
track_address(tx_state_changes, sender)
capture_pre_balance(tx_state_changes, sender, sender_balance_before)

sender_balance_after_gas_fee = (
Uint(sender_account.balance) - effective_gas_fee - blob_gas_fee
)
set_account_balance(
block_env.state,
block_env.state, sender, U256(sender_balance_after_gas_fee)
)
track_balance_change(
tx_state_changes,
sender,
U256(sender_balance_after_gas_fee),
tx_state_changes,
block_access_index,
)

access_list_addresses = set()
Expand Down Expand Up @@ -991,13 +1011,13 @@ def process_transaction(
authorizations=authorizations,
index_in_block=index,
tx_hash=get_transaction_hash(encode_transaction(tx)),
state_changes=tx_state_changes,
)

message = prepare_message(
block_env,
tx_env,
tx,
tx_state_changes,
)

tx_output = process_message_call(message)
Expand Down Expand Up @@ -1027,22 +1047,26 @@ def process_transaction(
sender_balance_after_refund = get_account(
block_env.state, sender
).balance + U256(gas_refund_amount)
set_account_balance(
block_env.state,
set_account_balance(block_env.state, sender, sender_balance_after_refund)
track_balance_change(
tx_env.state_changes,
sender,
sender_balance_after_refund,
tx_state_changes,
block_access_index,
)

coinbase_balance_after_mining_fee = get_account(
block_env.state, block_env.coinbase
).balance + U256(transaction_fee)

set_account_balance(
block_env.state,
block_env.state, block_env.coinbase, coinbase_balance_after_mining_fee
)
track_balance_change(
tx_env.state_changes,
block_env.coinbase,
coinbase_balance_after_mining_fee,
tx_state_changes,
block_access_index,
)

if coinbase_balance_after_mining_fee == 0 and account_exists_and_is_empty(
Expand All @@ -1068,29 +1092,24 @@ def process_transaction(

block_output.block_logs += tx_output.logs

# EIP-7928: Handle in-transaction self-destruct BEFORE normalization
# Destroy accounts first so normalization sees correct post-tx state
# Only accounts created in same tx are in accounts_to_delete per EIP-6780
for address in tx_output.accounts_to_delete:
destroy_account(block_env.state, address)

# EIP-7928: Normalize balance changes for this transaction before merging
# into block frame. Must happen AFTER destroy_account so net-zero filtering
# sees the correct post-transaction balance (0 for destroyed accounts).
normalize_balance_changes_for_transaction(
tx_state_changes,
BlockAccessIndex(
get_block_access_index(block_env.block_state_changes)
),
tx_env.state_changes,
block_access_index,
block_env.state,
)

commit_transaction_frame(tx_state_changes)
commit_transaction_frame(tx_env.state_changes)

# EIP-7928: Handle in-transaction self-destruct normalization AFTER merge
# EIP-7928: Track in-transaction self-destruct normalization AFTER merge
# Convert storage writes to reads and remove nonce/code changes
for address in tx_output.accounts_to_delete:
handle_in_transaction_selfdestruct(
track_selfdestruct(
block_env.block_state_changes,
address,
BlockAccessIndex(
Expand All @@ -1107,6 +1126,9 @@ def process_withdrawals(
"""
Increase the balance of the withdrawing account.
"""
block_access_index = get_block_access_index(block_env.block_state_changes)

# Capture pre-state for withdrawal balance filtering
withdrawal_addresses = {wd.address for wd in withdrawals}
for address in withdrawal_addresses:
pre_balance = get_account(block_env.state, address).balance
Expand All @@ -1129,20 +1151,20 @@ def increase_recipient_balance(recipient: Account) -> None:

new_balance = get_account(block_env.state, wd.address).balance
track_balance_change(
block_env.block_state_changes, wd.address, new_balance
block_env.block_state_changes,
wd.address,
new_balance,
block_access_index,
)

if account_exists_and_is_empty(block_env.state, wd.address):
destroy_account(block_env.state, wd.address)

# EIP-7928: Normalize balance changes after all withdrawals
# Filters out net-zero changes

normalize_balance_changes_for_transaction(
block_env.block_state_changes,
BlockAccessIndex(
get_block_access_index(block_env.block_state_changes)
),
block_access_index,
Copy link
Contributor

@gurukamath gurukamath Dec 4, 2025

Choose a reason for hiding this comment

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

If we intend to call this function beyond a transaction processing, might be best to rename it to normalize_balance_changes instead

Copy link
Contributor Author

@fselmo fselmo Dec 5, 2025

Choose a reason for hiding this comment

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

I think we can normalize all net-zero filtering here once we remove that from merge_on_success as I mentioned before. And yeah, agreed, this is called on withdrawals too so the naming should be better here.

block_env.state,
)

Expand Down
Loading
Loading