Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

Commit

Permalink
Merge Transfer and TransferWithGas gadget (#1777)
Browse files Browse the repository at this point in the history
### Description

- Transfer and TransferWithGas gadgets are mostly the same. We can merge
them and duplicate them.
- There are duplicated logics for creating the receiver account (writing
empty hash). We remove the duplicated one.
- The update balance gadget is vague, it is hard to understand whether
we want to add or subtract balance. We extract a helpful method for
that.
- deduplicate the transfer_to logic in busmapping.
- Fix the receiver creation condition on busmapping.
- Fix the order of the sender account read-write in begin tx assignment.

Thank @curryrasul for initializing this conversation.


### Issue Link


### Type of change

Refactor (no updates to logic)

### Content

### Test

```
cargo test -p zkevm-circuits create
cargo test -p zkevm-circuits begin_tx
cargo test -p zkevm-circuits end_tx 
```
  • Loading branch information
ChihChengLiang authored Mar 5, 2024
1 parent 3bbc757 commit 41e3408
Show file tree
Hide file tree
Showing 10 changed files with 171 additions and 361 deletions.
69 changes: 11 additions & 58 deletions bus-mapping/src/circuit_input_builder/input_state_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion bus-mapping/src/evm/opcodes/begin_end_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,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,
Expand Down
1 change: 1 addition & 0 deletions bus-mapping/src/evm/opcodes/callop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
callee_exists,
false,
call.value,
None,
)?;
}

Expand Down
3 changes: 2 additions & 1 deletion bus-mapping/src/evm/opcodes/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 15 additions & 28 deletions zkevm-circuits/src/evm_circuit/execution/begin_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -14,7 +14,7 @@ use crate::{
math_gadget::{
ContractCreateGadget, IsEqualWordGadget, IsZeroWordGadget, RangeCheckGadget,
},
not, or,
not,
tx::{BeginTxHelperGadget, TxDataGadget},
AccountAddress, CachedRegion, Cell, StepRws,
},
Expand All @@ -41,7 +41,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>,
Expand Down Expand Up @@ -170,17 +170,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();
Expand Down Expand Up @@ -467,18 +466,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)?;

Expand All @@ -502,15 +498,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(
Expand Down
22 changes: 6 additions & 16 deletions zkevm-circuits/src/evm_circuit/execution/callop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() +
Expand Down Expand Up @@ -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
Expand Down
22 changes: 8 additions & 14 deletions zkevm-circuits/src/evm_circuit/execution/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
)?;
}

Expand Down
42 changes: 16 additions & 26 deletions zkevm-circuits/src/evm_circuit/execution/end_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
MulWordByU64Gadget,
},
tx::EndTxHelperGadget,
CachedRegion, Cell,
CachedRegion, Cell, StepRws,
},
witness::{Block, Call, ExecStep, Transaction},
},
Expand Down Expand Up @@ -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,
);

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)))?;
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 41e3408

Please sign in to comment.