Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions src/ethereum/forks/amsterdam/fork.py
Original file line number Diff line number Diff line change
Expand Up @@ -1050,6 +1050,10 @@ def process_transaction(

tx_output = process_message_call(message)

if tx_output.error is not None:
tx_output.state_gas_left += tx_output.state_gas_used
tx_output.state_gas_used = Uint(0)
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 believe this is incorrect: if an EIP-7702 account is delegated and the account we are delegating already exists, we get a state gas refund which should propagate here. AFAIK all other state updates are triggered by execution, so indeed if there is an error this should be subtracted.

The gas calculation for the block is now incorrect: in case of a EIP-7702 delegation the state gas used of the block should increment. This is here set to 0.

For instance the case where the tx fails vs tx does not fail (either call an account with INVALID/REVERT(0,0) as opcode and where the tx creates an EIP-7702 account PER_EMPTY_ACCOUNT_COST is paid in state gas (or at least the PER_AUTH_BASE_COST). However via current code this sets block state gas to 0 and execution gas to the intrinsic gas cost (plus any cost for the revert / or all gas in case of invalid). This fails in case state gas > execution gas.

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.

Ok I see where this comes from (from EIP-8037)

On child exceptional halt, the child’s gas_left is additionally consumed (zeroed).

This is incorrect in the EIP-7702 case, see EIP-7702 (see Behavior section):

Note, if transaction execution results in failure (e.g. any exceptional condition or code reverting), the processed delegation indicators is not rolled back.

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.

@jochem-brouwer I think this is right. The 7702 interactions are handled in forks/amsterdam/vm/eoa_delegation.py. I put up PR #2722 to add some tests around these scenarios. Let me know here or there if those look wrong or if I misunderstood something!


tx_gas_used_before_refund = (
tx.gas - tx_output.gas_left - tx_output.state_gas_left
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
Storage,
Transaction,
TransactionException,
TransactionReceipt,
)
from execution_testing.checklists import EIPChecklist

Expand Down Expand Up @@ -415,3 +416,319 @@ def test_create_tx_reservoir(
)

state_test(env=env, pre=pre, post={}, tx=tx)


@pytest.mark.parametrize(
"failure_mode",
[
pytest.param("revert", id="revert"),
pytest.param("halt", id="halt"),
pytest.param("oog", id="oog"),
],
)
@pytest.mark.valid_from("EIP8037")
def test_top_level_failure_refunds_execution_state_gas(
state_test: StateTestFiller,
pre: Alloc,
fork: Fork,
failure_mode: str,
) -> None:
"""
Verify top level tx failure returns execution state gas to the
reservoir across revert, exceptional halt, and out of gas paths.

On top level failure no state was created, so execution state gas
is credited back to the reservoir and `state_gas_used` is zeroed.
The billing formula `tx.gas - gas_left - state_gas_left` sees a
restored reservoir and refunds the sender. Without the refund the
receipt would bill the consumed state gas despite the failure.
"""
gas_limit_cap = fork.transaction_gas_limit_cap()
assert gas_limit_cap is not None
sstore_state_gas = fork.sstore_state_gas()
intrinsic_cost = fork.transaction_intrinsic_cost_calculator()()

if failure_mode == "revert":
code = Op.SSTORE(0, 1) + Op.REVERT(0, 0)
elif failure_mode == "halt":
code = Op.SSTORE(0, 1) + Op.INVALID
else:
# OOG: perform the SSTORE then spin with JUMPDEST loop until
# gas runs out.
code = Op.SSTORE(0, 1) + Op.JUMPDEST + Op.JUMP(0x5)
contract = pre.deploy_contract(code=code)

tx_gas = gas_limit_cap + sstore_state_gas

if failure_mode == "revert":
# REVERT preserves unused gas_left.
expected_cumulative = (
intrinsic_cost + code.gas_cost(fork) - sstore_state_gas
)
else:
# Exceptional halt and out of gas zero gas_left.
expected_cumulative = tx_gas - sstore_state_gas

tx = Transaction(
to=contract,
gas_limit=tx_gas,
sender=pre.fund_eoa(),
expected_receipt=TransactionReceipt(
cumulative_gas_used=expected_cumulative,
),
)

state_test(pre=pre, post={contract: Account(storage={})}, tx=tx)


@pytest.mark.parametrize(
"failure_mode",
[
pytest.param("revert", id="revert"),
pytest.param("halt", id="halt"),
pytest.param("oog", id="oog"),
],
)
@pytest.mark.valid_from("EIP8037")
def test_top_level_failure_zeros_block_state_gas(
blockchain_test: BlockchainTestFiller,
pre: Alloc,
fork: Fork,
failure_mode: str,
) -> None:
"""
Verify the block header reflects zero execution state gas after a
top level failure.

With `state_gas_used` zeroed on failure, `block_state_gas_used`
excludes any state gas consumed during the failed transaction and
the block header `gas_used` falls back to the regular gas
component alone.
"""
gas_limit_cap = fork.transaction_gas_limit_cap()
assert gas_limit_cap is not None
sstore_state_gas = fork.sstore_state_gas()
intrinsic_cost = fork.transaction_intrinsic_cost_calculator()()

if failure_mode == "revert":
code = Op.SSTORE(0, 1) + Op.REVERT(0, 0)
elif failure_mode == "halt":
code = Op.SSTORE(0, 1) + Op.INVALID
else:
code = Op.SSTORE(0, 1) + Op.JUMPDEST + Op.JUMP(0x5)
contract = pre.deploy_contract(code=code)

tx_gas = gas_limit_cap + sstore_state_gas
tx = Transaction(
to=contract,
gas_limit=tx_gas,
sender=pre.fund_eoa(),
)

if failure_mode == "revert":
expected_block_regular = (
intrinsic_cost + code.gas_cost(fork) - sstore_state_gas
)
else:
# Exceptional halt and out of gas zero gas_left.
expected_block_regular = tx_gas - sstore_state_gas

blockchain_test(
pre=pre,
blocks=[
Block(
txs=[tx],
header_verify=Header(gas_used=expected_block_regular),
),
],
post={contract: Account(storage={})},
)


@pytest.mark.valid_from("EIP8037")
def test_creation_tx_failure_preserves_intrinsic_state_gas(
blockchain_test: BlockchainTestFiller,
pre: Alloc,
fork: Fork,
) -> None:
"""
Regression test for the creation tx failure path.

A creation tx (to=None) whose initcode halts exercises both the
intrinsic state gas for the new account and the top level failure
refund of execution state gas. The test asserts the block header
`gas_used` equals `max(block_regular, intrinsic_state_gas)`,
guarding that the failure path does not raise and that block
accounting does not underflow when the refund is applied.
"""
gas_limit_cap = fork.transaction_gas_limit_cap()
assert gas_limit_cap is not None

create_intrinsic_state = fork.transaction_intrinsic_state_gas(
contract_creation=True,
)
sstore_state_gas = fork.sstore_state_gas()
tx_gas = gas_limit_cap + create_intrinsic_state + sstore_state_gas

tx = Transaction(
to=None,
data=Op.SSTORE(0, 1) + Op.INVALID,
gas_limit=tx_gas,
sender=pre.fund_eoa(),
)

block_regular = tx_gas - create_intrinsic_state - sstore_state_gas
expected_gas_used = max(block_regular, create_intrinsic_state)

blockchain_test(
pre=pre,
blocks=[
Block(
txs=[tx],
header_verify=Header(gas_used=expected_gas_used),
),
],
post={},
)


@pytest.mark.valid_from("EIP8037")
def test_subcall_failure_does_not_zero_top_level_state_gas(
blockchain_test: BlockchainTestFiller,
pre: Alloc,
fork: Fork,
) -> None:
"""
Verify a subcall failure does not zero the top level execution
state gas.

The top level tx succeeds end to end even though a subcall
reverts, so the top level failure refund does not apply. The
parent's own SSTORE contributes state gas that appears in
`block_state_gas_used`.
"""
gas_limit_cap = fork.transaction_gas_limit_cap()
assert gas_limit_cap is not None
sstore_state_gas = fork.sstore_state_gas()

child = pre.deploy_contract(code=Op.REVERT(0, 0))
parent_storage = Storage()
parent = pre.deploy_contract(
code=(
Op.POP(Op.CALL(gas=Op.GAS, address=child))
+ Op.SSTORE(parent_storage.store_next(1, "parent_sstore"), 1)
),
)

tx = Transaction(
to=parent,
gas_limit=gas_limit_cap + sstore_state_gas,
sender=pre.fund_eoa(),
)

# Parent's SSTORE state gas dominates tx_regular and surfaces in
# the block header, proving the top level refund is scoped to
# top level failures and not child reverts.
blockchain_test(
pre=pre,
blocks=[
Block(
txs=[tx],
header_verify=Header(gas_used=sstore_state_gas),
),
],
post={parent: Account(storage=parent_storage)},
)


@pytest.mark.valid_from("EIP8037")
def test_top_level_failure_refunds_spilled_state_gas(
state_test: StateTestFiller,
pre: Alloc,
fork: Fork,
) -> None:
"""
Verify the top level failure refund covers state gas that
spilled from the reservoir into gas_left.

When the reservoir is smaller than the state gas charge, the
overflow spills and is drawn from gas_left. On top level failure
the full consumed state gas (reservoir portion plus spilled
portion) is credited back to the reservoir so the sender is not
billed for any of it.
"""
gas_limit_cap = fork.transaction_gas_limit_cap()
assert gas_limit_cap is not None
sstore_state_gas = fork.sstore_state_gas()
intrinsic_cost = fork.transaction_intrinsic_cost_calculator()()

code = Op.SSTORE(0, 1) + Op.REVERT(0, 0)
contract = pre.deploy_contract(code=code)

# Reservoir sized to cover only half the SSTORE state gas; the
# other half must spill into gas_left.
tx_gas = gas_limit_cap + sstore_state_gas // 2
expected_cumulative = (
intrinsic_cost + code.gas_cost(fork) - sstore_state_gas
)

tx = Transaction(
to=contract,
gas_limit=tx_gas,
sender=pre.fund_eoa(),
expected_receipt=TransactionReceipt(
cumulative_gas_used=expected_cumulative,
),
)

state_test(pre=pre, post={contract: Account(storage={})}, tx=tx)


@pytest.mark.valid_from("EIP8037")
def test_top_level_failure_refunds_state_gas_propagated_from_child(
state_test: StateTestFiller,
pre: Alloc,
fork: Fork,
) -> None:
"""
Verify the top level failure refund catches state gas propagated
from a successful subcall.

The parent calls a child that runs SSTORE and returns. The
child's state gas usage is folded into the parent frame via the
success path. When the parent then reverts at the top level, the
full propagated state gas must be refunded so the sender fee
excludes it.
"""
gas_limit_cap = fork.transaction_gas_limit_cap()
assert gas_limit_cap is not None
sstore_state_gas = fork.sstore_state_gas()
intrinsic_cost = fork.transaction_intrinsic_cost_calculator()()

child_code = Op.SSTORE(0, 1)
child = pre.deploy_contract(code=child_code)
parent_code = Op.POP(Op.CALL(gas=Op.GAS, address=child)) + Op.REVERT(0, 0)
parent = pre.deploy_contract(code=parent_code)

# Reservoir sized for the child's SSTORE. After the propagated
# state gas is refunded, the sender is billed only the regular
# gas: parent + CALL dispatch + child regular (SSTORE minus its
# state component).
tx_gas = gas_limit_cap + sstore_state_gas
expected_cumulative = (
intrinsic_cost
+ parent_code.gas_cost(fork)
+ child_code.gas_cost(fork)
- sstore_state_gas
)

tx = Transaction(
to=parent,
gas_limit=tx_gas,
sender=pre.fund_eoa(),
expected_receipt=TransactionReceipt(
cumulative_gas_used=expected_cumulative,
),
)

state_test(pre=pre, post={child: Account(storage={})}, tx=tx)
14 changes: 9 additions & 5 deletions tests/cancun/create/test_create_oog_from_eoa_refunds.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,12 +326,16 @@ def test_create_oog_from_eoa_refunds(
)
post[sender] = Account(nonce=1)
else:
# OOG case: contract not created, sender balance is fully consumed
# OOG case: contract not created
post[created_address] = Account.NONEXISTENT
post[sender] = Account(
nonce=1,
balance=0,
)
if fork.is_eip_enabled(8037):
# EIP-8037: execution state gas is returned to the
# reservoir on top-level failure, so the sender retains
# some balance (the refunded state gas × gas_price).
post[sender] = Account(nonce=1)
else:
# Pre-EIP-8037: sender balance is fully consumed
post[sender] = Account(nonce=1, balance=0)

if refund_type == RefundType.SELFDESTRUCT:
selfdestruct_code = Op.SELFDESTRUCT(Op.ORIGIN) + Op.STOP
Expand Down
10 changes: 9 additions & 1 deletion tests/cancun/eip5656_mcopy/test_mcopy_memory_expansion.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,22 @@ def tx( # noqa: D103
initial_memory: bytes,
tx_gas_limit: int,
tx_access_list: List[AccessList],
fork: Fork,
successful: bool,
) -> Transaction:
# EIP-8037: on top-level OOG, execution state gas is returned to the
# reservoir and not billed. The callee's SSTORE contributes state
# gas that gets refunded on failure.
expected_gas = tx_gas_limit
if not successful and fork.is_eip_enabled(8037):
expected_gas -= fork.sstore_state_gas()
return Transaction(
sender=sender,
to=caller_address,
access_list=tx_access_list,
data=initial_memory,
gas_limit=tx_gas_limit,
expected_receipt=TransactionReceipt(cumulative_gas_used=tx_gas_limit),
expected_receipt=TransactionReceipt(cumulative_gas_used=expected_gas),
)


Expand Down
Loading