Skip to content

Commit 550de88

Browse files
fselmofselmo
committed
fix(spec-specs): Calculate all gas we can before accessing state
- Calculate all gas that we can without state access and check this gas before ever accessing state. This is the most sensible way for an implementation to behave and indeed was revealed to be the way clients are behaving, which differed from the specs. - Use fork.gas_costs to calculate gas costs, NOT hard-coded values. - Create a BAL expectation for the test that yielded discrepancies between clients and specs so this doesn't slip through again. Document this test in `test_cases.md`.
1 parent cd46d89 commit 550de88

File tree

3 files changed

+187
-89
lines changed

3 files changed

+187
-89
lines changed

‎src/ethereum/forks/amsterdam/vm/instructions/system.py‎

Lines changed: 93 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -395,50 +395,59 @@ def call(evm: Evm) -> None:
395395
access_gas_cost = (
396396
GAS_COLD_ACCOUNT_ACCESS if is_cold_access else GAS_WARM_ACCESS
397397
)
398+
399+
transfer_gas_cost = Uint(0) if value == 0 else GAS_CALL_VALUE
400+
401+
check_gas(
402+
evm,
403+
access_gas_cost + transfer_gas_cost + extend_memory.cost,
404+
)
405+
406+
# need to access account to check if account is alive, check gas before
407+
create_gas_cost = GAS_NEW_ACCOUNT
408+
if value == 0 or is_account_alive(evm.message.block_env.state, to):
409+
create_gas_cost = Uint(0)
410+
398411
if is_cold_access:
399412
evm.accessed_addresses.add(to)
400413

401-
# check gas for base access before reading `to` account
402-
base_gas_cost = extend_memory.cost + access_gas_cost
403-
check_gas(evm, base_gas_cost)
404-
405-
# read `to` account and assess delegation cost
406414
(
407415
is_delegated,
408416
original_address,
409417
delegated_address,
410418
delegation_gas_cost,
411419
) = calculate_delegation_cost(evm, to)
412420

413-
# check gas again for delegation target access before reading it
414421
if is_delegated and delegation_gas_cost > Uint(0):
415-
check_gas(evm, base_gas_cost + delegation_gas_cost)
416-
417-
if is_delegated:
418422
assert delegated_address is not None
423+
message_call_gas = calculate_message_call_gas(
424+
value,
425+
gas,
426+
Uint(evm.gas_left),
427+
extend_memory.cost,
428+
access_gas_cost
429+
+ transfer_gas_cost
430+
+ create_gas_cost
431+
+ delegation_gas_cost,
432+
)
433+
check_gas(evm, message_call_gas.cost + extend_memory.cost)
419434
code = read_delegation_target(evm, delegated_address)
420435
final_address = delegated_address
421436
else:
437+
message_call_gas = calculate_message_call_gas(
438+
value,
439+
gas,
440+
Uint(evm.gas_left),
441+
extend_memory.cost,
442+
access_gas_cost + create_gas_cost + transfer_gas_cost,
443+
)
444+
check_gas(evm, message_call_gas.cost + extend_memory.cost)
422445
code = get_account(evm.message.block_env.state, to).code
423446
final_address = to
424447

425-
access_gas_cost += delegation_gas_cost
426-
427448
code_address = final_address
428449
disable_precompiles = is_delegated
429450

430-
create_gas_cost = GAS_NEW_ACCOUNT
431-
if value == 0 or is_account_alive(evm.message.block_env.state, to):
432-
create_gas_cost = Uint(0)
433-
transfer_gas_cost = Uint(0) if value == 0 else GAS_CALL_VALUE
434-
message_call_gas = calculate_message_call_gas(
435-
value,
436-
gas,
437-
Uint(evm.gas_left),
438-
extend_memory.cost,
439-
access_gas_cost + create_gas_cost + transfer_gas_cost,
440-
)
441-
442451
charge_gas(evm, message_call_gas.cost + extend_memory.cost)
443452
if evm.message.is_static and value != U256(0):
444453
raise WriteInStaticContext
@@ -509,44 +518,49 @@ def callcode(evm: Evm) -> None:
509518
if is_cold_access:
510519
evm.accessed_addresses.add(code_address)
511520

512-
# check gas for base access before reading `code_address` account
513-
base_gas_cost = extend_memory.cost + access_gas_cost
514-
check_gas(evm, base_gas_cost)
521+
transfer_gas_cost = Uint(0) if value == 0 else GAS_CALL_VALUE
522+
523+
check_gas(
524+
evm,
525+
access_gas_cost + extend_memory.cost + transfer_gas_cost,
526+
)
515527

516-
# read code_address account and assess delegation cost
528+
# need to access account to get delegation code, check gas before
517529
(
518530
is_delegated,
519531
original_address,
520532
delegated_address,
521533
delegation_gas_cost,
522534
) = calculate_delegation_cost(evm, code_address)
523535

524-
# check gas again for delegation target access before reading it
525536
if is_delegated and delegation_gas_cost > Uint(0):
526-
check_gas(evm, base_gas_cost + delegation_gas_cost)
527-
528-
if is_delegated:
529537
assert delegated_address is not None
538+
# Recalculate with delegation cost and check gas
539+
message_call_gas = calculate_message_call_gas(
540+
value,
541+
gas,
542+
Uint(evm.gas_left),
543+
extend_memory.cost,
544+
access_gas_cost + transfer_gas_cost + delegation_gas_cost,
545+
)
546+
check_gas(evm, message_call_gas.cost + extend_memory.cost)
530547
code = read_delegation_target(evm, delegated_address)
531548
final_address = delegated_address
532549
else:
550+
message_call_gas = calculate_message_call_gas(
551+
value,
552+
gas,
553+
Uint(evm.gas_left),
554+
extend_memory.cost,
555+
access_gas_cost + transfer_gas_cost,
556+
)
557+
check_gas(evm, message_call_gas.cost + extend_memory.cost)
533558
code = get_account(evm.message.block_env.state, code_address).code
534559
final_address = code_address
535560

536-
access_gas_cost += delegation_gas_cost
537-
538561
code_address = final_address
539562
disable_precompiles = is_delegated
540563

541-
transfer_gas_cost = Uint(0) if value == 0 else GAS_CALL_VALUE
542-
message_call_gas = calculate_message_call_gas(
543-
value,
544-
gas,
545-
Uint(evm.gas_left),
546-
extend_memory.cost,
547-
access_gas_cost + transfer_gas_cost,
548-
)
549-
550564
charge_gas(evm, message_call_gas.cost + extend_memory.cost)
551565

552566
# OPERATION
@@ -688,44 +702,46 @@ def delegatecall(evm: Evm) -> None:
688702
access_gas_cost = (
689703
GAS_COLD_ACCOUNT_ACCESS if is_cold_access else GAS_WARM_ACCESS
690704
)
691-
692-
# check gas for base access before reading `code_address` account
693-
base_gas_cost = extend_memory.cost + access_gas_cost
694-
check_gas(evm, base_gas_cost)
695-
696705
if is_cold_access:
697706
evm.accessed_addresses.add(code_address)
698707

699-
# read `code_address` account and assess delegation cost
708+
check_gas(evm, access_gas_cost + extend_memory.cost)
709+
710+
# need to access account to get delegation code, check gas before
700711
(
701712
is_delegated,
702713
original_address,
703714
delegated_address,
704715
delegation_gas_cost,
705716
) = calculate_delegation_cost(evm, code_address)
706717

707-
# check gas again for delegation target access before reading it
708718
if is_delegated and delegation_gas_cost > Uint(0):
709-
check_gas(evm, base_gas_cost + delegation_gas_cost)
710-
711-
# Now safe to read delegation target since we verified gas
712-
if is_delegated:
713719
assert delegated_address is not None
720+
message_call_gas = calculate_message_call_gas(
721+
U256(0),
722+
gas,
723+
Uint(evm.gas_left),
724+
extend_memory.cost,
725+
access_gas_cost + delegation_gas_cost,
726+
)
727+
check_gas(evm, message_call_gas.cost + extend_memory.cost)
714728
code = read_delegation_target(evm, delegated_address)
715729
final_address = delegated_address
716730
else:
731+
message_call_gas = calculate_message_call_gas(
732+
U256(0),
733+
gas,
734+
Uint(evm.gas_left),
735+
extend_memory.cost,
736+
access_gas_cost,
737+
)
738+
check_gas(evm, message_call_gas.cost + extend_memory.cost)
717739
code = get_account(evm.message.block_env.state, code_address).code
718740
final_address = code_address
719741

720-
access_gas_cost += delegation_gas_cost
721-
722742
code_address = final_address
723743
disable_precompiles = is_delegated
724744

725-
message_call_gas = calculate_message_call_gas(
726-
U256(0), gas, Uint(evm.gas_left), extend_memory.cost, access_gas_cost
727-
)
728-
729745
charge_gas(evm, message_call_gas.cost + extend_memory.cost)
730746

731747
# OPERATION
@@ -785,44 +801,43 @@ def staticcall(evm: Evm) -> None:
785801
if is_cold_access:
786802
evm.accessed_addresses.add(to)
787803

788-
# check gas for base access before reading `to` account
789-
base_gas_cost = extend_memory.cost + access_gas_cost
790-
check_gas(evm, base_gas_cost)
804+
check_gas(evm, access_gas_cost + extend_memory.cost)
791805

792-
# read `to` account and assess delegation cost
806+
# need to access account to get delegation code, check gas before
793807
(
794808
is_delegated,
795809
original_address,
796810
delegated_address,
797811
delegation_gas_cost,
798812
) = calculate_delegation_cost(evm, to)
799813

800-
# check gas again for delegation target access before reading it
801814
if is_delegated and delegation_gas_cost > Uint(0):
802-
check_gas(evm, base_gas_cost + delegation_gas_cost)
803-
804-
# Now safe to read delegation target since we verified gas
805-
if is_delegated:
806815
assert delegated_address is not None
816+
message_call_gas = calculate_message_call_gas(
817+
U256(0),
818+
gas,
819+
Uint(evm.gas_left),
820+
extend_memory.cost,
821+
access_gas_cost + delegation_gas_cost,
822+
)
823+
check_gas(evm, message_call_gas.cost + extend_memory.cost)
807824
code = read_delegation_target(evm, delegated_address)
808825
final_address = delegated_address
809826
else:
827+
message_call_gas = calculate_message_call_gas(
828+
U256(0),
829+
gas,
830+
Uint(evm.gas_left),
831+
extend_memory.cost,
832+
access_gas_cost,
833+
)
834+
check_gas(evm, message_call_gas.cost + extend_memory.cost)
810835
code = get_account(evm.message.block_env.state, to).code
811836
final_address = to
812837

813-
access_gas_cost += delegation_gas_cost
814-
815838
code_address = final_address
816839
disable_precompiles = is_delegated
817840

818-
message_call_gas = calculate_message_call_gas(
819-
U256(0),
820-
gas,
821-
Uint(evm.gas_left),
822-
extend_memory.cost,
823-
access_gas_cost,
824-
)
825-
826841
charge_gas(evm, message_call_gas.cost + extend_memory.cost)
827842

828843
# OPERATION

‎tests/amsterdam/eip7928_block_level_access_lists/test_cases.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,4 @@
9393
| `test_init_collision_create_tx` | Ensure BAL tracks CREATE collisions correctly (pre-Amsterdam test with BAL) | CREATE transaction targeting address with existing storage aborts | BAL **MUST** show empty expectations for collision address (no changes occur due to abort) | ✅ Completed |
9494
| `test_call_to_pre_authorized_oog` | Ensure BAL handles OOG during EIP-7702 delegation access (pre-Amsterdam test with BAL) | Call to delegated account that OOGs before accessing delegation contract | BAL **MUST** include auth_signer (code read for delegation check) but **MUST NOT** include delegation contract (OOG before access) | ✅ Completed |
9595
| `test_selfdestruct_created_in_same_tx_with_revert` | Ensure BAL tracks selfdestruct with revert correctly (pre-Amsterdam test with BAL) | Contract created and selfdestructed in same tx with nested revert | BAL **MUST** track storage reads and balance changes for selfdestruct even with reverts | ✅ Completed |
96+
| `test_value_transfer_gas_calculation` | Ensure BAL correctly tracks OOG scenarios for CALL/CALLCODE/DELEGATECALL/STATICCALL (pre-Amsterdam test with BAL) | Nested calls with precise gas limits to test OOG behavior. For CALL with OOG: target account read for `is_account_alive` check. For CALLCODE/DELEGATECALL/STATICCALL with OOG: target account **NOT** read (OOG before state access) | For CALL: target in BAL even with OOG. For CALLCODE/DELEGATECALL/STATICCALL: target **NOT** in BAL when OOG (state access deferred until after gas check) | ✅ Completed |

0 commit comments

Comments
 (0)