Merge branch 'main' into fix/begintx-missingconstraint

curryrasul · web-flow · commit 3af8f8f73746 · 2024-03-06T21:46:26.000+04:00
diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs
@@ -559,13 +559,13 @@ impl<'a> CircuitInputStateRef<'a> {
     /// balance by `value`. If `fee` is existing (not None), also need to push 1
     /// non-reversible [`AccountOp`] to update `sender` balance by `fee`.
     #[allow(clippy::too_many_arguments)]
-    pub fn transfer_with_fee(
+    pub fn transfer(
         &mut self,
         step: &mut ExecStep,
         sender: Address,
         receiver: Address,
         receiver_exists: bool,
-        must_create: bool,
+        is_create: bool,
         value: Word,
         fee: Option<Word>,
     ) -> Result<(), Error> {
@@ -608,82 +608,35 @@ impl<'a> CircuitInputStateRef<'a> {
             sender_balance_prev,
             sender_balance
         );
-        // If receiver doesn't exist, create it
-        if !receiver_exists && (!value.is_zero() || must_create) {
+
+        if !value.is_zero() {
             self.push_op_reversible(
                 step,
                 AccountOp {
-                    address: receiver,
-                    field: AccountField::CodeHash,
-                    value: CodeDB::empty_code_hash().to_word(),
-                    value_prev: Word::zero(),
+                    address: sender,
+                    field: AccountField::Balance,
+                    value: sender_balance,
+                    value_prev: sender_balance_prev,
                 },
             )?;
         }
-        if value.is_zero() {
-            // Skip transfer if value == 0
-            return Ok(());
-        }
 
-        self.push_op_reversible(
-            step,
-            AccountOp {
-                address: sender,
-                field: AccountField::Balance,
-                value: sender_balance,
-                value_prev: sender_balance_prev,
-            },
-        )?;
-
-        let (_found, receiver_account) = self.sdb.get_account(&receiver);
-        let receiver_balance_prev = receiver_account.balance;
-        let receiver_balance = receiver_account.balance + value;
-        self.push_op_reversible(
-            step,
-            AccountOp {
-                address: receiver,
-                field: AccountField::Balance,
-                value: receiver_balance,
-                value_prev: receiver_balance_prev,
-            },
-        )?;
+        self.transfer_to(step, receiver, receiver_exists, is_create, value, true)?;
 
         Ok(())
     }
 
-    /// Same functionality with `transfer_with_fee` but with `fee` set zero.
-    pub fn transfer(
-        &mut self,
-        step: &mut ExecStep,
-        sender: Address,
-        receiver: Address,
-        receiver_exists: bool,
-        must_create: bool,
-        value: Word,
-    ) -> Result<(), Error> {
-        self.transfer_with_fee(
-            step,
-            sender,
-            receiver,
-            receiver_exists,
-            must_create,
-            value,
-            None,
-        )
-    }
-
     /// Transfer to an address. Create an account if it is not existed before.
     pub fn transfer_to(
         &mut self,
         step: &mut ExecStep,
         receiver: Address,
         receiver_exists: bool,
-        must_create: bool,
+        is_create: bool,
         value: Word,
         reversible: bool,
     ) -> Result<(), Error> {
-        // If receiver doesn't exist, create it
-        if (!receiver_exists && !value.is_zero()) || must_create {
+        if !receiver_exists && (!value.is_zero() || is_create) {
             self.account_write(
                 step,
                 receiver,
diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs
@@ -124,7 +124,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro
     }
 
     // Transfer with fee
-    state.transfer_with_fee(
+    state.transfer(
         &mut exec_step,
         call.caller_address,
         call.address,
diff --git a/bus-mapping/src/evm/opcodes/callop.rs b/bus-mapping/src/evm/opcodes/callop.rs
@@ -195,6 +195,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                 callee_exists,
                 false,
                 call.value,
+                None,
             )?;
         }
 
diff --git a/bus-mapping/src/evm/opcodes/create.rs b/bus-mapping/src/evm/opcodes/create.rs
@@ -217,8 +217,9 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
                 caller.address,
                 callee.address,
                 callee_exists,
-                !callee_exists,
+                true,
                 callee.value,
+                None,
             )?;
 
             // EIP 161, increase callee's nonce
diff --git a/zkevm-circuits/src/evm_circuit/execution/begin_tx.rs b/zkevm-circuits/src/evm_circuit/execution/begin_tx.rs
@@ -5,7 +5,7 @@ use crate::{
         step::ExecutionState,
         util::{
             and,
-            common_gadget::TransferWithGasFeeGadget,
+            common_gadget::TransferGadget,
             constraint_builder::{
                 ConstrainBuilderCommon, EVMConstraintBuilder, ReversionInfo, StepStateTransition,
                 Transition::{Delta, To},
@@ -42,7 +42,7 @@ pub(crate) struct BeginTxGadget<F> {
     call_callee_address: AccountAddress<F>,
     reversion_info: ReversionInfo<F>,
     sufficient_gas_left: RangeCheckGadget<F, N_BYTES_GAS>,
-    transfer_with_gas_fee: TransferWithGasFeeGadget<F>,
+    transfer_with_gas_fee: TransferGadget<F, true>,
     code_hash: WordLoHiCell<F>,
     is_empty_code_hash: IsEqualWordGadget<F, WordLoHi<Expression<F>>, WordLoHi<Expression<F>>>,
     caller_nonce_hash_bytes: Word32Cell<F>,
@@ -174,17 +174,16 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
             AccountFieldTag::CodeHash,
             code_hash.to_word(),
         );
-
         // Transfer value from caller to callee, creating account if necessary.
-        let transfer_with_gas_fee = TransferWithGasFeeGadget::construct(
+        let transfer_with_gas_fee = TransferGadget::construct(
             cb,
             tx.caller_address.to_word(),
             tx.callee_address.to_word(),
             not::expr(callee_not_exists.expr()),
-            or::expr([tx.is_create.expr(), callee_not_exists.expr()]),
+            tx.is_create.expr(),
             tx.value.clone(),
-            tx.mul_gas_fee_by_gas.product().clone(),
             &mut reversion_info,
+            Some(tx.mul_gas_fee_by_gas.product().clone()),
         );
 
         let caller_nonce_hash_bytes = cb.query_word32();
@@ -509,18 +508,15 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
         }
         let callee_exists =
             is_precompiled(&tx.to_or_contract_addr()) || !callee_code_hash.is_zero();
-        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_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_balance_pair();
-            callee_balance_pair = rws.next().account_balance_pair();
-        };
-
+        self.transfer_with_gas_fee.assign(
+            region,
+            offset,
+            &mut rws,
+            callee_exists,
+            tx.value,
+            tx.is_create(),
+            Some(gas_fee),
+        )?;
         self.begin_tx.assign(region, offset, tx)?;
         self.tx.assign(region, offset, tx)?;
 
@@ -544,15 +540,6 @@ impl<F: Field> ExecutionGadget<F> for BeginTxGadget<F> {
         )?;
         self.sufficient_gas_left
             .assign(region, offset, F::from(tx.gas() - step.gas_cost))?;
-        self.transfer_with_gas_fee.assign(
-            region,
-            offset,
-            caller_balance_sub_fee_pair,
-            caller_balance_sub_value_pair,
-            callee_balance_pair,
-            tx.value,
-            gas_fee,
-        )?;
         self.code_hash
             .assign_u256(region, offset, callee_code_hash)?;
         self.is_empty_code_hash.assign_u256(
diff --git a/zkevm-circuits/src/evm_circuit/execution/callop.rs b/zkevm-circuits/src/evm_circuit/execution/callop.rs
@@ -60,7 +60,7 @@ pub(crate) struct CallOpGadget<F> {
     is_warm: Cell<F>,
     is_warm_prev: Cell<F>,
     callee_reversion_info: ReversionInfo<F>,
-    transfer: TransferGadget<F>,
+    transfer: TransferGadget<F, false>,
     // current handling Call* opcode's caller balance
     caller_balance: WordLoHi<Cell<F>>,
     // check if insufficient balance case
@@ -242,9 +242,10 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
                 caller_address.to_word(),
                 callee_address.to_word(),
                 not::expr(call_gadget.callee_not_exists.expr()),
-                0.expr(),
+                false.expr(),
                 call_gadget.value.clone(),
                 &mut callee_reversion_info,
+                None,
             )
         });
         // rwc_delta = 8 + is_delegatecall * 2 + call_gadget.rw_delta() +
@@ -866,20 +867,9 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
             depth.low_u64() < 1025 && (!(is_call || is_callcode) || caller_balance >= value);
 
         // conditionally assign
-        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,
-                caller_balance_pair,
-                callee_balance_pair,
-                value,
-            )?;
+        if is_call && is_precheck_ok {
+            self.transfer
+                .assign(region, offset, &mut rws, callee_exists, value, false, None)?;
         }
 
         self.opcode
diff --git a/zkevm-circuits/src/evm_circuit/execution/create.rs b/zkevm-circuits/src/evm_circuit/execution/create.rs
@@ -63,7 +63,7 @@ pub(crate) struct CreateGadget<F, const IS_CREATE2: bool, const S: ExecutionStat
     callee_nonce: Cell<F>,
     prev_code_hash: WordLoHiCell<F>,
     prev_code_hash_is_zero: IsZeroWordGadget<F, WordLoHi<Expression<F>>>,
-    transfer: TransferGadget<F>,
+    transfer: TransferGadget<F, false>,
     create: ContractCreateGadget<F, IS_CREATE2>,
 
     init_code: MemoryAddressGadget<F>,
@@ -333,10 +333,11 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
                     cb,
                     create.caller_address(),
                     contract_addr.to_word(),
-                    0.expr(),
-                    1.expr(),
+                    false.expr(),
+                    true.expr(),
                     value.clone(),
                     &mut callee_reversion_info,
+                    None,
                 );
 
                 // EIP 161, the nonce of a newly created contract is 1
@@ -638,21 +639,14 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
 
         let code_hash = if is_precheck_ok {
             if !is_address_collision {
-                // transfer
-                if callee_prev_code_hash.is_zero() {
-                    rws.next(); // codehash update
-                }
-                let [caller_balance_pair, callee_balance_pair] = if !value.is_zero() {
-                    [(); 2].map(|_| rws.next().account_balance_pair())
-                } else {
-                    [(0.into(), 0.into()), (0.into(), 0.into())]
-                };
                 self.transfer.assign(
                     region,
                     offset,
-                    caller_balance_pair,
-                    callee_balance_pair,
+                    &mut rws,
+                    !callee_prev_code_hash.is_zero(),
                     value,
+                    true,
+                    None,
                 )?;
             }
 
diff --git a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs
@@ -11,7 +11,7 @@ use crate::{
                 MulWordByU64Gadget,
             },
             tx::EndTxHelperGadget,
-            CachedRegion, Cell,
+            CachedRegion, Cell, StepRws,
         },
         witness::{Block, Call, ExecStep, Transaction},
     },
@@ -75,10 +75,9 @@ impl<F: Field> ExecutionGadget<F> for EndTxGadget<F> {
             tx_gas_price.clone(),
             effective_refund.min() + cb.curr.state.gas_left.expr(),
         );
-        let gas_fee_refund = UpdateBalanceGadget::construct(
-            cb,
+        let gas_fee_refund = cb.increase_balance(
             tx_caller_address.to_word(),
-            vec![mul_gas_price_by_refund.product().clone()],
+            mul_gas_price_by_refund.product().clone(),
             None,
         );
 
@@ -111,7 +110,6 @@ impl<F: Field> ExecutionGadget<F> for EndTxGadget<F> {
             false.expr(),
             mul_effective_tip_by_gas_used.product().clone(),
             None,
-            true,
         );
 
         let end_tx = EndTxHelperGadget::construct(
@@ -152,9 +150,11 @@ impl<F: Field> ExecutionGadget<F> for EndTxGadget<F> {
         step: &ExecStep,
     ) -> 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_balance_pair();
-        let (coinbase_code_hash_prev, _) = block.get_rws(step, 4).account_codehash_pair();
+        let mut rws = StepRws::new(block, step);
+        rws.offset_add(2);
+        let (refund, _) = rws.next().tx_refund_value_pair();
+        let (caller_balance, caller_balance_prev) = rws.next().account_balance_pair();
+        let (coinbase_code_hash_prev, _) = rws.next().account_codehash_pair();
 
         self.tx_id
             .assign(region, offset, Value::known(F::from(tx.id)))?;
@@ -208,24 +208,14 @@ impl<F: Field> ExecutionGadget<F> for EndTxGadget<F> {
             .assign_u256(region, offset, coinbase_code_hash_prev)?;
         self.coinbase_code_hash_is_zero
             .assign_u256(region, offset, coinbase_code_hash_prev)?;
-        if !coinbase_reward.is_zero() {
-            let coinbase_balance_pair = block
-                .get_rws(
-                    step,
-                    if coinbase_code_hash_prev.is_zero() {
-                        6
-                    } else {
-                        5
-                    },
-                )
-                .account_balance_pair();
-            self.coinbase_reward.assign(
-                region,
-                offset,
-                coinbase_balance_pair,
-                effective_tip * gas_used,
-            )?;
-        }
+        self.coinbase_reward.assign(
+            region,
+            offset,
+            &mut rws,
+            !coinbase_code_hash_prev.is_zero(),
+            coinbase_reward,
+            false,
+        )?;
         self.is_persistent.assign(
             region,
             offset,
diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs
diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs

Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@ fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Erro`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`// Transfer with fee`
`127`		`- state.transfer_with_fee(`
	`127`	`+ state.transfer(`
`128`	`128`	`&mut exec_step,`
`129`	`129`	`call.caller_address,`
`130`	`130`	`call.address,`
Original file line number	Diff line number	Diff line change
`@@ -195,6 +195,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {`
`195`	`195`	`callee_exists,`
`196`	`196`	`false,`
`197`	`197`	`call.value,`
	`198`	`+ None,`
`198`	`199`	`)?;`
`199`	`200`	`}`
`200`	`201`