Bug: fix to not invoke transfer for call related opcodes (except CALL)

specs/opcode/F1CALL_F2CALLCODE_F4DELEGATECALL_FASTATICCALL.md

@@ -25,7 +25,8 @@ It creates a new sub context as setting caller address to parent caller's and ca
 - For opcode `STATICCALL`:
 It does not allow any state modifying instructions (is_static == 1) or sending ether to callee in the sub context.
 
-And both `DELEGATECALL` and `STATICCALL` opcodes only pop 6 words from stack `gas`, `callee_address`, `call_data_offset`, `call_data_length`, `return_data_offset` and `return_data_length` (except the third popped word `value` for both `CALL` and `CALLCODE` opcodes).
+Both `DELEGATECALL` and `STATICCALL` opcodes only pop 6 words from stack `gas`, `callee_address`, `call_data_offset`, `call_data_length`, `return_data_offset` and `return_data_length` (except the third popped word `value` for both `CALL` and `CALLCODE` opcodes).
+There should be no `transfer` invocation for `CALLCODE`, `DELEGATECALL`and `STATICCALL` (only for CALL).
 
 Before switching call context to the new one, it does several things:
 
@@ -34,7 +35,7 @@ Before switching call context to the new one, it does several things:
 3. Calculate `gas_cost` and check `gas_left` is enough
 4. Calculate `callee_gas_left` for new context by rule in EIP150
 5. Check `depth` is less than `1024`
-6. Check `value` could be transfer (zero for both `DELEGATECALL` and `STATICCALL` opcodes)
+6. Check `value` could be transferred (only for `CALL` and `CALLCODE` opcodes)
 
 The memory size is calculated as follows:
 
@@ -84,7 +85,7 @@ callee_gas_left := min(gas_available - floor(gas_available / 64), gas)
 
 After switching call context, it does:
 
-1. Transfer `value` (zero for both `DELEGATECALL` and `STATICCALL` opcodes)
+1. Transfer `value` (only for `CALL` opcode)
 2. Execution
    1. If `callee_address` is a precompiled, it runs the pre-defined handler
    2. Otherwise, it takes callee's code for execution

src/zkevm_specs/evm/execution/callop.py

@@ -108,27 +108,50 @@ def callop(instruction: Instruction):
     has_value = 1 - instruction.is_zero(value)
     instruction.constrain_zero(has_value * is_static)
 
-    # Verify transfer
-    _, (_, callee_balance_prev) = instruction.transfer(
-        caller_address, callee_address, value, callee_reversion_info
-    )
+    if is_call == 1:
+        # For CALL opcode, verify transfer, and get caller balance before
+        # transfer to constrain it should be greater than or equal to stack
+        # `value`.
+        (_, caller_balance), _ = instruction.transfer(
+            caller_address, callee_address, value, callee_reversion_info
+        )
+    elif is_callcode == 1:
+        # For CALLCODE opcode, get caller balance to constrain it should be
+        # greater than or equal to stack `value`.
+        caller_balance = instruction.account_read(caller_address, AccountFieldTag.Balance)
 
-    # Verify gas cost
-    callee_nonce = instruction.account_read(callee_address, AccountFieldTag.Nonce)
-    callee_code_hash = instruction.account_read(code_address, AccountFieldTag.CodeHash)
-    is_empty_code_hash = instruction.is_equal(
-        callee_code_hash, instruction.rlc_encode(EMPTY_CODE_HASH, 32)
-    )
-    is_account_empty = (
-        instruction.is_zero(callee_nonce)
-        * instruction.is_zero(callee_balance_prev)
-        * is_empty_code_hash
-    )
+    # For both CALL and CALLCODE opcodes, verify caller balance is greater than
+    # or equal to stack `value`.
+    if is_call + is_callcode == 1:
+        value_lt_caller_balance, value_eq_caller_balance = instruction.compare_word(
+            value, caller_balance
+        )
+        instruction.constrain_zero(1 - value_lt_caller_balance - value_eq_caller_balance)
+
+    # Load callee account `exists` value from auxilary witness data.
+    callee_exists = instruction.curr.aux_data
+
+    if callee_exists == 1:
+        # Get callee code hash.
+        callee_code_hash = instruction.account_read(code_address, AccountFieldTag.CodeHash)
+        is_empty_code_hash = instruction.is_equal(
+            callee_code_hash, instruction.rlc_encode(EMPTY_CODE_HASH, 32)
+        )
+    else:  # callee_exists == 0
+        instruction.account_read(code_address, AccountFieldTag.NonExisting)
+        is_empty_code_hash = FQ(1)
+
+    # Verify gas cost.
     gas_cost = (
         instruction.select(
             is_warm_access, FQ(GAS_COST_WARM_ACCESS), FQ(GAS_COST_ACCOUNT_COLD_ACCESS)
         )
-        + has_value * (GAS_COST_CALL_WITH_VALUE + is_account_empty * GAS_COST_NEW_ACCOUNT)
+        + has_value
+        * (
+            GAS_COST_CALL_WITH_VALUE
+            # Only CALL opcode could invoke transfer to make empty account into non-empty.
+            + is_call * (1 - callee_exists) * GAS_COST_NEW_ACCOUNT
+        )
         + memory_expansion_gas_cost
     )
 
@@ -161,10 +184,11 @@ def callop(instruction: Instruction):
                 expected_value,
             )
 
-        # Both CALL and CALLCODE opcodes have an extra stack pop `value`, and
-        # opcode DELEGATECALL has two extra call context lookups - parent caller
-        # address and value.
-        rw_counter_delta = 23 + is_call + is_callcode + is_delegatecall * 2
+        # For CALL opcode, it has an extra stack pop `value` and two account write for `transfer` call (+3).
+        # For CALLCODE opcode, it has an extra stack pop `value` and one account read for caller balance (+2).
+        # For DELEGATECALL opcode, it has two extra call context lookups for current caller address and value (+2).
+        # No extra lookups for STATICCALL opcode.
+        rw_counter_delta = 20 + is_call * 3 + is_callcode * 2 + is_delegatecall * 2
         stack_pointer_delta = 5 + is_call + is_callcode
 
         instruction.constrain_step_state_transition(
@@ -181,7 +205,8 @@ def callop(instruction: Instruction):
             code_hash=Transition.same(),
         )
     else:
-        rw_counter_delta = 43 + is_call + is_callcode + is_delegatecall * 2
+        # Similar as above comment.
+        rw_counter_delta = 40 + is_call * 3 + is_callcode * 2 + is_delegatecall * 2
         stack_pointer_delta = 5 + is_call + is_callcode
 
         # Save caller's call state

tests/evm/test_callop.py

@@ -74,6 +74,7 @@ def memory_size(offset: int, length: int) -> int:
             return 0
         return (offset + length + 31) // 32
 
+    is_call = 1 if opcode == Opcode.CALL else 0
     # Both CALL and CALLCODE opcodes have argument `value` on stack, but no for
     # DELEGATECALL or STATICCALL.
     has_value = stack.value != 0 if opcode in [Opcode.CALL, Opcode.CALLCODE] else False
@@ -87,7 +88,12 @@ def memory_size(offset: int, length: int) -> int:
     ) // 512 + 3 * (next_memory_size - caller_ctx.memory_size)
     gas_cost = (
         (GAS_COST_WARM_ACCESS if is_warm_access else GAS_COST_ACCOUNT_COLD_ACCESS)
-        + has_value * (GAS_COST_CALL_WITH_VALUE + callee.is_empty() * GAS_COST_NEW_ACCOUNT)
+        + has_value
+        * (
+            GAS_COST_CALL_WITH_VALUE
+            # Only CALL opcode could invoke transfer to make empty account into non-empty.
+            + is_call * callee.is_empty() * GAS_COST_NEW_ACCOUNT
+        )
         + memory_expansion_gas_cost
     )
     gas_available = caller_ctx.gas_left - gas_cost
@@ -184,6 +190,9 @@ def test_callop(
     is_call = 1 if opcode == Opcode.CALL else 0
     is_callcode = 1 if opcode == Opcode.CALLCODE else 0
     is_delegatecall = 1 if opcode == Opcode.DELEGATECALL else 0
+    is_staticcall = 1 if opcode == Opcode.STATICCALL else 0
+
+    callee_exists = 0 if callee.is_empty() else 1
 
     # Set `is_static == 1` for both DELEGATECALL and STATICCALL opcodes, or when
     # `stack.value == 0` for both CALL and CALLCODE opcodes.
@@ -266,10 +275,11 @@ def test_callop(
         )
     )
 
-    # Both CALL and CALLCODE opcodes have an extra stack pop `value`, and opcode
-    # DELEGATECALL has two extra call context lookups - parent caller address
-    # and value.
-    call_id = 23 + is_call + is_callcode + is_delegatecall * 2
+    # For CALL opcode, it has an extra stack pop `value` and two account write for `transfer` call (+3).
+    # For CALLCODE opcode, it has an extra stack pop `value` and one account read for caller balance (+2).
+    # For DELEGATECALL opcode, it has two extra call context lookups for current caller address and value (+2).
+    # No extra lookups for STATICCALL opcode.
+    call_id = 20 + is_call * 3 + is_callcode * 2 + is_delegatecall * 2
     rw_counter = call_id
     next_program_counter = 232 if is_call + is_callcode == 1 else 199
     stack_pointer = 1018 - is_call - is_callcode
@@ -329,11 +339,22 @@ def test_callop(
     callee_balance = RLC(callee.balance + value, randomness)
 
     # fmt: off
-    rw_dictionary \
-        .account_write(caller.address, AccountFieldTag.Balance, caller_balance, caller_balance_prev, rw_counter_of_reversion=None if callee_is_persistent else callee_rw_counter_end_of_reversion) \
-        .account_write(callee.address, AccountFieldTag.Balance, callee_balance, callee_balance_prev, rw_counter_of_reversion=None if callee_is_persistent else callee_rw_counter_end_of_reversion - 1) \
-        .account_read(callee.address, AccountFieldTag.Nonce, RLC(callee.nonce, randomness)) \
-        .account_read(code_address, AccountFieldTag.CodeHash, callee_bytecode_hash)
+    if is_call == 1:
+        # For `transfer` invocation.
+        rw_dictionary \
+            .account_write(caller.address, AccountFieldTag.Balance, caller_balance, caller_balance_prev, rw_counter_of_reversion=None if callee_is_persistent else callee_rw_counter_end_of_reversion) \
+            .account_write(callee.address, AccountFieldTag.Balance, callee_balance, callee_balance_prev, rw_counter_of_reversion=None if callee_is_persistent else callee_rw_counter_end_of_reversion - 1)
+    elif is_callcode == 1:
+        # Get caller balance to constrain it should be greater than or equal to stack `value`.
+        rw_dictionary \
+            .account_read(caller.address, AccountFieldTag.Balance, RLC(caller.balance, randomness))
+
+    if callee_exists == 1:
+        rw_dictionary \
+            .account_read(code_address, AccountFieldTag.CodeHash, callee_bytecode_hash)
+    else:
+        rw_dictionary \
+            .account_read(code_address, AccountFieldTag.NonExisting, RLC(1, randomness))
 
     if is_empty_code_hash:
         rw_dictionary \
@@ -395,6 +416,7 @@ def test_callop(
                 gas_left=caller_ctx.gas_left,
                 memory_size=caller_ctx.memory_size,
                 reversible_write_counter=caller_ctx.reversible_write_counter,
+                aux_data=callee_exists,
             ),
             (
                 StepState(