Merge remote-tracking branch 'origin/main' into fix/call_data

bus-mapping/src/evm/opcodes/callop.rs

@@ -100,7 +100,8 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
         )?;
 
         let callee_code_hash = call.code_hash;
-        let callee_exists = !state.sdb.get_account(&callee_address).1.is_empty();
+        let callee = state.sdb.get_account(&callee_address).1.clone();
+        let callee_exists = !callee.is_empty();
 
         let (callee_code_hash_word, is_empty_code_hash) = if callee_exists {
             (
@@ -145,11 +146,13 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
         debug_assert!(found);
 
         let caller_balance = sender_account.balance;
+
         let is_call_or_callcode = call.kind == CallKind::Call || call.kind == CallKind::CallCode;
+        let is_sufficient = caller_balance >= call.value;
+        let is_valid_depth = geth_step.depth < 1025;
 
         // Precheck is OK when depth is in range and caller balance is sufficient
-        let is_precheck_ok =
-            geth_step.depth < 1025 && (!is_call_or_callcode || caller_balance >= call.value);
+        let is_precheck_ok = is_valid_depth && (is_sufficient || !is_call_or_callcode);
 
         log::debug!(
             "is_precheck_ok: {}, call type: {:?}, sender_account: {:?} ",
@@ -173,9 +176,11 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
         let is_precompile = code_address
             .map(|ref addr| is_precompiled(addr))
             .unwrap_or(false);
-        // TODO: What about transfer for CALLCODE?
-        // Transfer value only for CALL opcode, is_precheck_ok = true.
-        if call.kind == CallKind::Call && is_precheck_ok {
+        // Transfer value only when all these conditions met:
+        // - The opcode is CALL
+        // - The precheck passed
+        // - The value to send is not zero
+        if call.kind == CallKind::Call && is_precheck_ok && !call.value.is_zero() {
             state.transfer(
                 &mut exec_step,
                 call.caller_address,
@@ -221,9 +226,9 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
 
         // There are 4 branches from here.
         // add failure case for insufficient balance or error depth in the future.
-        match (!is_precheck_ok, is_precompile, is_empty_code_hash) {
+        match (is_precheck_ok, is_precompile, is_empty_code_hash) {
             // 1. Call to precompiled.
-            (false, true, _) => {
+            (true, true, _) => {
                 assert!(call.is_success, "call to precompile should not fail");
                 let caller_ctx = state.caller_ctx_mut()?;
                 let code_address = code_address.unwrap();
@@ -275,7 +280,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                 Ok(vec![exec_step])
             }
             // 2. Call to account with empty code.
-            (false, _, true) => {
+            (true, _, true) => {
                 for (field, value) in [
                     (CallContextField::LastCalleeId, 0.into()),
                     (CallContextField::LastCalleeReturnDataOffset, 0.into()),
@@ -287,7 +292,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                 Ok(vec![exec_step])
             }
             // 3. Call to account with non-empty code.
-            (false, _, false) => {
+            (true, _, false) => {
                 for (field, value) in [
                     (CallContextField::ProgramCounter, (geth_step.pc + 1).into()),
                     (
@@ -349,7 +354,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
             }
 
             // 4. insufficient balance or error depth cases.
-            (true, _, _) => {
+            (false, _, _) => {
                 for (field, value) in [
                     (CallContextField::LastCalleeId, 0.into()),
                     (CallContextField::LastCalleeReturnDataOffset, 0.into()),

zkevm-circuits/src/evm_circuit/execution/balance.rs

@@ -135,15 +135,15 @@ impl<F: Field> ExecutionGadget<F> for BalanceGadget<F> {
         self.is_warm
             .assign(region, offset, Value::known(F::from(is_warm as u64)))?;
 
-        let code_hash = block.get_rws(step, 5).account_value_pair().0;
+        let code_hash = block.get_rws(step, 5).account_codehash_pair().0;
         self.code_hash
             .assign_u256(region, offset, code_hash.to_word())?;
         self.not_exists
             .assign_value(region, offset, Value::known(Word::from(code_hash)))?;
         let balance = if code_hash.is_zero() {
             eth_types::Word::zero()
         } else {
-            block.get_rws(step, 6).account_value_pair().0
+            block.get_rws(step, 6).account_balance_pair().0
         };
         self.balance.assign_u256(region, offset, balance)?;
 

zkevm-circuits/src/evm_circuit/execution/begin_tx.rs

@@ -514,20 +514,20 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
         let is_coinbase_warm = rws.next().tx_access_list_value_pair().1;
         let mut callee_code_hash = zero;
         if !is_precompiled(&tx.to_or_contract_addr()) && !tx.is_create() {
-            callee_code_hash = rws.next().account_value_pair().1;
+            callee_code_hash = rws.next().account_codehash_pair().1;
         }
         let callee_exists = is_precompiled(&tx.to_or_contract_addr())
             || (!tx.is_create() && !callee_code_hash.is_zero());
-        let caller_balance_sub_fee_pair = rws.next().account_value_pair();
+        let caller_balance_sub_fee_pair = rws.next().account_balance_pair();
         let must_create = tx.is_create();
         if (!callee_exists && !tx.value.is_zero()) || must_create {
-            callee_code_hash = rws.next().account_value_pair().1;
+            callee_code_hash = rws.next().account_codehash_pair().1;
         }
         let mut caller_balance_sub_value_pair = (zero, zero);
         let mut callee_balance_pair = (zero, zero);
         if !tx.value.is_zero() {
-            caller_balance_sub_value_pair = rws.next().account_value_pair();
-            callee_balance_pair = rws.next().account_value_pair();
+            caller_balance_sub_value_pair = rws.next().account_balance_pair();
+            callee_balance_pair = rws.next().account_balance_pair();
         };
 
         self.tx_id

zkevm-circuits/src/evm_circuit/execution/callop.rs

@@ -253,8 +253,7 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
                 // caller address and value (+2).
                 //
                 // No extra lookups for STATICCALL opcode.
-                let transfer_rwc_delta =
-                    is_call.expr() * not::expr(transfer.value_is_zero.expr()) * 2.expr();
+                let transfer_rwc_delta = is_call.expr() * transfer.reversible_w_delta();
                 let rw_counter_delta = 21.expr()
                     + is_call.expr() * 1.expr()
                     + transfer_rwc_delta.clone()
@@ -484,7 +483,7 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
         let depth = rws.next().call_context_value();
         let current_callee_address = rws.next().call_context_value();
 
-        let is_error_depth = depth.low_u64() > 1024;
+        let is_valid_depth = depth.low_u64() < 1025;
         self.is_depth_ok
             .assign(region, offset, F::from(depth.low_u64()), F::from(1025))?;
         // This offset is used to change the index offset of `step.rw_indices`.
@@ -513,7 +512,7 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
         let rd_length = rws.next().stack_value();
         let is_success = rws.next().stack_value();
 
-        let callee_code_hash = rws.next().account_value_pair().0;
+        let callee_code_hash = rws.next().account_codehash_pair().0;
         let callee_exists = !callee_code_hash.is_zero();
 
         let (is_warm, is_warm_prev) = rws.next().tx_access_list_value_pair();
@@ -523,24 +522,12 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
 
         // check if it is insufficient balance case.
         // get caller balance
-        let caller_balance = rws.next().account_value_pair().0;
+        let caller_balance = rws.next().account_balance_pair().0;
         self.caller_balance
             .assign_u256(region, offset, caller_balance)?;
         self.is_insufficient_balance
             .assign(region, offset, caller_balance, value)?;
 
-        let is_insufficient = (value > caller_balance) && (is_call || is_callcode);
-        // only call opcode do transfer in sucessful case.
-        let [caller_balance_pair, callee_balance_pair] =
-            if is_call && !is_insufficient && !is_error_depth && !value.is_zero() {
-                [
-                    rws.next().account_value_pair(),
-                    rws.next().account_value_pair(),
-                ]
-            } else {
-                [(U256::zero(), U256::zero()), (U256::zero(), U256::zero())]
-            };
-
         self.opcode
             .assign(region, offset, Value::known(F::from(opcode.as_u64())))?;
         self.is_call.assign(
@@ -612,8 +599,19 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
             callee_rw_counter_end_of_reversion.low_u64() as usize,
             callee_is_persistent.low_u64() != 0,
         )?;
+
+        let is_call_or_callcode = is_call || is_callcode;
+        let is_sufficient = caller_balance >= value;
+        let is_precheck_ok = is_valid_depth && (is_sufficient || !is_call_or_callcode);
+
         // conditionally assign
-        if !is_insufficient && !is_error_depth && !value.is_zero() {
+        if is_call && is_precheck_ok && !value.is_zero() {
+            if !callee_exists {
+                rws.next().account_codehash_pair(); // callee hash
+            }
+
+            let caller_balance_pair = rws.next().account_balance_pair();
+            let callee_balance_pair = rws.next().account_balance_pair();
             self.transfer.assign(
                 region,
                 offset,
@@ -695,15 +693,13 @@ mod test {
         }
     }
 
-    #[ignore]
     #[test]
     fn callop_recursive() {
         for opcode in TEST_CALL_OPCODES {
             test_recursive(opcode);
         }
     }
 
-    #[ignore]
     #[test]
     fn callop_simple() {
         let stacks = [

zkevm-circuits/src/evm_circuit/execution/create.rs

@@ -497,8 +497,8 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
         let rw_offset = if is_create2 { 8 } else { 7 };
 
         // Pre-check: call depth, user's nonce and user's balance
-        let (caller_balance, _) = block.get_rws(step, rw_offset + 1).account_value_pair();
-        let (caller_nonce, _) = block.get_rws(step, rw_offset + 2).account_value_pair();
+        let (caller_balance, _) = block.get_rws(step, rw_offset + 1).account_balance_pair();
+        let (caller_nonce, _) = block.get_rws(step, rw_offset + 2).account_nonce_pair();
         let is_precheck_ok =
             if call.depth < 1025 && caller_balance >= value && caller_nonce.as_u64() < u64::MAX {
                 1
@@ -513,7 +513,7 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
                 .get_rws(step, rw_offset + 4)
                 .tx_access_list_value_pair();
             let (callee_prev_code_hash, _) =
-                block.get_rws(step, rw_offset + 5).account_value_pair();
+                block.get_rws(step, rw_offset + 5).account_codehash_pair();
             (callee_prev_code_hash, was_warm)
         } else {
             (U256::from(0), false)
@@ -586,7 +586,7 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
                     rw_offset + copy_rw_increase + 14,
                     rw_offset + copy_rw_increase + 15,
                 ]
-                .map(|i| block.get_rws(step, i).account_value_pair())
+                .map(|i| block.get_rws(step, i).account_balance_pair())
             } else {
                 [(0.into(), 0.into()), (0.into(), 0.into())]
             };

zkevm-circuits/src/evm_circuit/execution/end_tx.rs

@@ -228,8 +228,8 @@ impl<F: Field> ExecutionGadget<F> for EndTxGadget<F> {
     ) -> Result<(), Error> {
         let gas_used = tx.gas() - step.gas_left;
         let (refund, _) = block.get_rws(step, 2).tx_refund_value_pair();
-        let (caller_balance, caller_balance_prev) = block.get_rws(step, 3).account_value_pair();
-        let (coinbase_code_hash_prev, _) = block.get_rws(step, 4).account_value_pair();
+        let (caller_balance, caller_balance_prev) = block.get_rws(step, 3).account_balance_pair();
+        let (coinbase_code_hash_prev, _) = block.get_rws(step, 4).account_codehash_pair();
         let (coinbase_balance, coinbase_balance_prev) = block
             .get_rws(
                 step,
@@ -239,7 +239,7 @@ impl<F: Field> ExecutionGadget<F> for EndTxGadget<F> {
                     5
                 },
             )
-            .account_value_pair();
+            .account_balance_pair();
 
         self.tx_id
             .assign(region, offset, Value::known(F::from(tx.id)))?;

zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs

@@ -129,7 +129,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGCallGadget<F> {
 
         let callee_code_hash = block
             .get_rws(step, 9 + is_call_or_callcode)
-            .account_value_pair()
+            .account_codehash_pair()
             .0;
         let callee_exists = !callee_code_hash.is_zero();
 

zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs

@@ -203,7 +203,7 @@ impl<F: Field> ExecutionGadget<F> for ExtcodecopyGadget<F> {
         self.is_warm
             .assign(region, offset, Value::known(F::from(is_warm as u64)))?;
 
-        let code_hash = block.get_rws(step, 8).account_value_pair().0;
+        let code_hash = block.get_rws(step, 8).account_codehash_pair().0;
         self.code_hash.assign_u256(region, offset, code_hash)?;
         self.not_exists.assign_u256(region, offset, code_hash)?;
 

zkevm-circuits/src/evm_circuit/execution/extcodehash.rs

@@ -122,7 +122,7 @@ impl<F: Field> ExecutionGadget<F> for ExtcodehashGadget<F> {
         self.is_warm
             .assign(region, offset, Value::known(F::from(is_warm as u64)))?;
 
-        let code_hash = block.get_rws(step, 5).account_value_pair().0;
+        let code_hash = block.get_rws(step, 5).account_codehash_pair().0;
         self.code_hash.assign_u256(region, offset, code_hash)?;
 
         Ok(())

zkevm-circuits/src/evm_circuit/execution/extcodesize.rs

@@ -140,7 +140,7 @@ impl<F: Field> ExecutionGadget<F> for ExtcodesizeGadget<F> {
         self.is_warm
             .assign(region, offset, Value::known(F::from(is_warm as u64)))?;
 
-        let code_hash = block.get_rws(step, 5).account_value_pair().0;
+        let code_hash = block.get_rws(step, 5).account_codehash_pair().0;
         self.code_hash.assign_u256(region, offset, code_hash)?;
         self.not_exists
             .assign(region, offset, Word::from(code_hash))?;

zkevm-circuits/src/evm_circuit/execution/shl_shr.rs

@@ -211,7 +211,8 @@ impl<F: Field> ExecutionGadget<F> for ShlShrGadget<F> {
         self.remainder_is_zero
             .assign(region, offset, Word::from(remainder))?;
         self.remainder_lt_divisor
-            .assign(region, offset, remainder, divisor)
+            .assign(region, offset, remainder, divisor)?;
+        Ok(())
     }
 }
 

zkevm-circuits/src/evm_circuit/util/common_gadget.rs

@@ -514,7 +514,7 @@ impl<F: Field> TransferWithGasFeeGadget<F> {
         or::expr([
             not::expr(self.value_is_zero.expr()) * not::expr(self.receiver.receiver_exists.expr()),
             self.receiver.must_create.clone()]
-        ) * 1.expr() +
+        ) +
         // +1 Write Account (sender) Balance
         // +1 Write Account (receiver) Balance
         not::expr(self.value_is_zero.expr()) * 2.expr()

zkevm-circuits/src/evm_circuit/util/math_gadget/lt_word.rs

@@ -42,22 +42,22 @@ impl<F: Field> LtWordGadget<F> {
         offset: usize,
         lhs: Word,
         rhs: Word,
-    ) -> Result<(), Error> {
+    ) -> Result<F, Error> {
         let (lhs_lo, lhs_hi) = split_u256(&lhs);
         let (rhs_lo, rhs_hi) = split_u256(&rhs);
-        self.comparison_hi.assign(
+        let (hi_lt, hi_eq) = self.comparison_hi.assign(
             region,
             offset,
             F::from_u128(lhs_hi.as_u128()),
             F::from_u128(rhs_hi.as_u128()),
         )?;
-        self.lt_lo.assign(
+        let (lt_lo, _) = self.lt_lo.assign(
             region,
             offset,
             F::from_u128(lhs_lo.as_u128()),
             F::from_u128(rhs_lo.as_u128()),
         )?;
-        Ok(())
+        Ok(hi_lt + hi_eq * lt_lo)
     }
 }
 

zkevm-circuits/src/witness/block.rs

@@ -60,9 +60,11 @@ impl<F: Field> Block<F> {
         for (tx_idx, tx) in self.txs.iter().enumerate() {
             println!("tx {}", tx_idx);
             for step in tx.steps() {
-                println!(" step {:?} rwc: {}", step.exec_state, step.rwc.0);
+                println!("> Step {:?}", step.exec_state);
                 for rw_idx in 0..step.bus_mapping_instance.len() {
-                    println!("  - {:?}", self.get_rws(step, rw_idx));
+                    let rw = self.get_rws(step, rw_idx);
+                    let rw_str = if rw.is_write() { "READ" } else { "WRIT" };
+                    println!("  {} {} {:?}", rw.rw_counter(), rw_str, rw);
                 }
             }
         }