refactor(test-tests): parametrize existing test oog case instead

Author: fselmo

tests/amsterdam/eip7928_block_level_access_lists/test_block_access_lists_opcodes.py

@@ -529,113 +529,53 @@ def test_bal_delegatecall_and_oog(
 
 
 @pytest.mark.parametrize(
-    "fails_at_extcodecopy",
-    [True, False],
-    ids=["oog_at_extcodecopy", "successful_extcodecopy"],
-)
-def test_bal_extcodecopy_and_oog(
-    pre: Alloc,
-    blockchain_test: BlockchainTestFiller,
-    fork: Fork,
-    fails_at_extcodecopy: bool,
-) -> None:
-    """
-    Ensure BAL handles EXTCODECOPY and OOG during EXTCODECOPY appropriately.
-    """
-    alice = pre.fund_eoa()
-    gas_costs = fork.gas_costs()
-
-    # Create target contract with some code
-    target_contract = pre.deploy_contract(
-        code=Bytecode(Op.PUSH1(0x42) + Op.STOP)
-    )
-
-    # Create contract that attempts to copy code from target
-    extcodecopy_contract_code = Bytecode(
-        Op.PUSH1(0)  # size - copy 0 bytes to minimize memory expansion cost
-        + Op.PUSH1(0)  # codeOffset
-        + Op.PUSH1(0)  # destOffset
-        + Op.PUSH20(target_contract)  # address
-        + Op.EXTCODECOPY  # Copy code (cold access + base cost)
-        + Op.STOP
-    )
-
-    extcodecopy_contract = pre.deploy_contract(code=extcodecopy_contract_code)
-
-    intrinsic_gas_calculator = fork.transaction_intrinsic_cost_calculator()
-    intrinsic_gas_cost = intrinsic_gas_calculator()
-
-    # Costs:
-    # - 4 PUSH operations = G_VERY_LOW * 4
-    # - EXTCODECOPY cold = G_COLD_ACCOUNT_ACCESS + (G_COPY * words)
-    #   where words = ceil32(size) // 32 = ceil32(0) // 32 = 0
-    push_cost = gas_costs.G_VERY_LOW * 4
-    extcodecopy_cold_cost = (
-        gas_costs.G_COLD_ACCOUNT_ACCESS
-    )  # + (G_COPY * 0) = 0
-    tx_gas_limit = intrinsic_gas_cost + push_cost + extcodecopy_cold_cost
-
-    if fails_at_extcodecopy:
-        # subtract 1 gas to ensure OOG at EXTCODECOPY
-        tx_gas_limit -= 1
-
-    tx = Transaction(
-        sender=alice,
-        to=extcodecopy_contract,
-        gas_limit=tx_gas_limit,
-    )
-
-    block = Block(
-        txs=[tx],
-        expected_block_access_list=BlockAccessListExpectation(
-            account_expectations={
-                extcodecopy_contract: BalAccountExpectation.empty(),
-                # Target should only appear if EXTCODECOPY succeeded
-                **(
-                    {target_contract: None}
-                    if fails_at_extcodecopy
-                    else {target_contract: BalAccountExpectation.empty()}
-                ),
-            }
-        ),
-    )
-
-    blockchain_test(
-        pre=pre,
-        blocks=[block],
-        post={
-            alice: Account(nonce=1),
-            extcodecopy_contract: Account(),
-            target_contract: Account(),
-        },
-    )
-
-
-@pytest.mark.parametrize(
-    "memory_offset,copy_size,gas_shortfall",
+    "oog_scenario,memory_offset,copy_size",
     [
-        pytest.param(0x10000, 32, "large", id="large_offset"),
-        pytest.param(256, 32, "boundary", id="boundary"),
+        pytest.param("success", 0, 0, id="successful_extcodecopy"),
+        pytest.param("oog_at_cold_access", 0, 0, id="oog_at_cold_access"),
+        pytest.param(
+            "oog_at_memory_large_offset",
+            0x10000,
+            32,
+            id="oog_at_memory_large_offset",
+        ),
+        pytest.param(
+            "oog_at_memory_boundary",
+            256,
+            32,
+            id="oog_at_memory_boundary",
+        ),
     ],
 )
-def test_bal_extcodecopy_oog_at_memory_expansion(
+def test_bal_extcodecopy_and_oog(
     pre: Alloc,
     blockchain_test: BlockchainTestFiller,
     fork: Fork,
+    oog_scenario: str,
     memory_offset: int,
     copy_size: int,
-    gas_shortfall: str,
 ) -> None:
     """
-    Test EXTCODECOPY OOG at memory expansion - target should NOT appear in BAL.
+    Ensure BAL handles EXTCODECOPY and OOG during EXTCODECOPY appropriately.
+
+    Tests various OOG scenarios:
+    - success: EXTCODECOPY completes, target appears in BAL
+    - oog_at_cold_access: OOG before cold access, target NOT in BAL
+    - oog_at_memory_large_offset: OOG at memory expansion (large offset),
+      target NOT in BAL
+    - oog_at_memory_boundary: OOG at memory expansion (boundary case),
+      target NOT in BAL
 
     Gas for all components (cold access + copy + memory expansion) must be
     checked BEFORE recording account access.
     """
     alice = pre.fund_eoa()
     gas_costs = fork.gas_costs()
 
-    target_contract = pre.deploy_contract(code=Bytecode(Op.STOP))
+    # Create target contract with some code
+    target_contract = pre.deploy_contract(
+        code=Bytecode(Op.PUSH1(0x42) + Op.STOP)
+    )
 
     # Build EXTCODECOPY contract with appropriate PUSH sizes
     if memory_offset <= 0xFF:
@@ -647,8 +587,8 @@ def test_bal_extcodecopy_oog_at_memory_expansion(
 
     extcodecopy_contract_code = Bytecode(
         Op.PUSH1(copy_size)
-        + Op.PUSH1(0)
-        + dest_push
+        + Op.PUSH1(0)  # codeOffset
+        + dest_push  # destOffset
         + Op.PUSH20(target_contract)
         + Op.EXTCODECOPY
         + Op.STOP
@@ -659,20 +599,35 @@ def test_bal_extcodecopy_oog_at_memory_expansion(
     intrinsic_gas_calculator = fork.transaction_intrinsic_cost_calculator()
     intrinsic_gas_cost = intrinsic_gas_calculator()
 
+    # Calculate costs
     push_cost = gas_costs.G_VERY_LOW * 4
     cold_access_cost = gas_costs.G_COLD_ACCOUNT_ACCESS
     copy_cost = gas_costs.G_COPY * ((copy_size + 31) // 32)
 
-    if gas_shortfall == "large":
+    if oog_scenario == "success":
+        # Provide enough gas for everything including memory expansion
+        words = (memory_offset + copy_size + 31) // 32
+        memory_cost = (words * gas_costs.G_MEMORY) + (words * words // 512)
+        execution_cost = push_cost + cold_access_cost + copy_cost + memory_cost
+        tx_gas_limit = intrinsic_gas_cost + execution_cost
+        target_in_bal = True
+    elif oog_scenario == "oog_at_cold_access":
+        # Provide gas for pushes but 1 less than cold access cost
+        execution_cost = push_cost + cold_access_cost
+        tx_gas_limit = intrinsic_gas_cost + execution_cost - 1
+        target_in_bal = False
+    elif oog_scenario == "oog_at_memory_large_offset":
         # Provide gas for push + cold access + copy, but NOT memory expansion
         execution_cost = push_cost + cold_access_cost + copy_cost
         tx_gas_limit = intrinsic_gas_cost + execution_cost
-    else:
+        target_in_bal = False
+    else:  # oog_at_memory_boundary
         # Calculate memory cost and provide exactly 1 less than needed
         words = (memory_offset + copy_size + 31) // 32
         memory_cost = (words * gas_costs.G_MEMORY) + (words * words // 512)
         execution_cost = push_cost + cold_access_cost + copy_cost + memory_cost
         tx_gas_limit = intrinsic_gas_cost + execution_cost - 1
+        target_in_bal = False
 
     tx = Transaction(
         sender=alice,
@@ -685,7 +640,11 @@ def test_bal_extcodecopy_oog_at_memory_expansion(
         expected_block_access_list=BlockAccessListExpectation(
             account_expectations={
                 extcodecopy_contract: BalAccountExpectation.empty(),
-                target_contract: None,
+                **(
+                    {target_contract: BalAccountExpectation.empty()}
+                    if target_in_bal
+                    else {target_contract: None}
+                ),
             }
         ),
     )

tests/amsterdam/eip7928_block_level_access_lists/test_cases.md

@@ -49,7 +49,7 @@
 | `test_bal_extcodesize_and_oog` | Ensure BAL handles OOG during EXTCODESIZE opcode execution correctly | Alice calls contract that attempts `EXTCODESIZE` opcode on cold target contract. Parameterized: (1) OOG at EXTCODESIZE opcode (insufficient gas), (2) Successful EXTCODESIZE execution. | For OOG case: BAL **MUST NOT** include target contract (wasn't accessed). For success case: BAL **MUST** include target contract in `account_changes`. | ✅ Completed |
 | `test_bal_call_and_oog` | Ensure BAL handles OOG during CALL opcode execution correctly | Alice calls contract that attempts `CALL` to cold target contract. Parameterized: (1) OOG at CALL opcode (insufficient gas), (2) Successful CALL execution. | For OOG case: BAL **MUST NOT** include target contract (wasn't accessed). For success case: BAL **MUST** include target contract in `account_changes`. | ✅ Completed |
 | `test_bal_delegatecall_and_oog` | Ensure BAL handles OOG during DELEGATECALL opcode execution correctly | Alice calls contract that attempts `DELEGATECALL` to cold target contract. Parameterized: (1) OOG at DELEGATECALL opcode (insufficient gas), (2) Successful DELEGATECALL execution. | For OOG case: BAL **MUST NOT** include target contract (wasn't accessed). For success case: BAL **MUST** include target contract in `account_changes`. | ✅ Completed |
-| `test_bal_extcodecopy_and_oog` | Ensure BAL handles OOG during EXTCODECOPY opcode execution correctly | Alice calls contract that attempts `EXTCODECOPY` from cold target contract. Parameterized: (1) OOG at EXTCODECOPY opcode (insufficient gas), (2) Successful EXTCODECOPY execution. | For OOG case: BAL **MUST NOT** include target contract (wasn't accessed). For success case: BAL **MUST** include target contract in `account_changes`. | ✅ Completed |
+| `test_bal_extcodecopy_and_oog` | Ensure BAL handles OOG during EXTCODECOPY at various failure points | Alice calls contract that attempts `EXTCODECOPY` from cold target contract. Parameterized: (1) Successful EXTCODECOPY, (2) OOG at cold access (insufficient gas for account access), (3) OOG at memory expansion with large offset (64KB offset, gas covers cold access + copy but NOT memory expansion), (4) OOG at memory expansion boundary (256 byte offset, gas is exactly 1 less than needed). | For success case: BAL **MUST** include target contract. For all OOG cases: BAL **MUST NOT** include target contract. Gas for ALL components (cold access + copy + memory expansion) must be checked BEFORE recording account access. | ✅ Completed |
 | `test_bal_oog_7702_delegated_cold_cold` | Ensure BAL handles OOG during EIP-7702 delegated account loading when both accounts are cold | Alice calls cold delegated account Bob (7702) which delegates to cold `TargetContract`  with insufficient gas for second cold load | BAL **MUST** include Bob in `account_changes` (first cold load succeeds) but **MUST NOT** include `TargetContract` (second cold load fails due to OOG) | 🟡 Planned |
 | `test_bal_oog_7702_delegated_warm_cold` | Ensure BAL handles OOG during EIP-7702 delegated account loading when first account is warm, second is cold | Alice calls warm delegated account Bob (7702) which delegates to cold `TargetContract`  with insufficient gas for second cold load | BAL **MUST** include Bob in `account_changes` (warm load succeeds) but **MUST NOT** include `TargetContract` (cold load fails due to OOG) | 🟡 Planned |
 | `test_bal_multiple_balance_changes_same_account` | Ensure BAL tracks multiple balance changes to same account across transactions | Alice funds Bob (starts at 0) in tx0 with exact amount needed. Bob spends everything in tx1 to Charlie. Bob's balance: 0 → funding_amount → 0 | BAL **MUST** include Bob with two `balance_changes`: one at txIndex=1 (receives funds) and one at txIndex=2 (balance returns to 0). This tests balance tracking across two transactions. | ✅ Completed |