Skip to content

Commit

Permalink
Merge pull request #42 from kroma-network/fix/fix-bug-on-create2
Browse files Browse the repository at this point in the history
fix: fix bug on create2
  • Loading branch information
chokobole authored May 28, 2024
2 parents f534f0e + 9ba1d24 commit 5deea13
Show file tree
Hide file tree
Showing 12 changed files with 405 additions and 488 deletions.
17 changes: 15 additions & 2 deletions bus-mapping/src/circuit_input_builder/input_state_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use super::{
TransactionContext,
};
use crate::{
error::{get_step_reported_error, ExecError},
error::{get_step_reported_error, ExecError, InsufficientBalanceError},
exec_trace::OperationRef,
operation::{
AccountField, AccountOp, CallContextField, CallContextOp, MemoryOp, Op, OpEnum, Operation,
Expand Down Expand Up @@ -1448,7 +1448,20 @@ impl<'a> CircuitInputStateRef<'a> {
return Err(Error::AccountNotFound(sender));
}
if account.balance < value {
return Ok(Some(ExecError::InsufficientBalance));
return Ok(Some(match step.op {
OpcodeId::CALL | OpcodeId::CALLCODE => {
ExecError::InsufficientBalance(InsufficientBalanceError::Call)
}
OpcodeId::CREATE => {
ExecError::InsufficientBalance(InsufficientBalanceError::Create)
}
OpcodeId::CREATE2 => {
ExecError::InsufficientBalance(InsufficientBalanceError::Create2)
}
op => {
unreachable!("insufficient balance error unexpected for opcode: {:?}", op)
}
}));
}

// Address collision
Expand Down
6 changes: 4 additions & 2 deletions bus-mapping/src/circuit_input_builder/tracer_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::{
};
use crate::{
circuit_input_builder::{access::gen_state_access_trace, Access, AccessSet, AccessValue},
error::ExecError,
error::{ExecError, InsufficientBalanceError},
geth_errors::{
GETH_ERR_GAS_UINT_OVERFLOW, GETH_ERR_OUT_OF_GAS, GETH_ERR_STACK_OVERFLOW,
GETH_ERR_STACK_UNDERFLOW,
Expand Down Expand Up @@ -305,7 +305,9 @@ fn tracer_err_insufficient_balance() {
let mut builder = CircuitInputBuilderTx::new(&block, step);
assert_eq!(
builder.state_ref().get_step_err(step, next_step).unwrap(),
Some(ExecError::InsufficientBalance)
Some(ExecError::InsufficientBalance(
InsufficientBalanceError::Call
))
);
}

Expand Down
15 changes: 13 additions & 2 deletions bus-mapping/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,17 @@ pub enum OogError {
SelfDestruct,
}

/// Insufficient balance errors by opcode/state.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum InsufficientBalanceError {
/// Insufficient balance during CALL/CALLCODE opcode.
Call,
/// Insufficient balance during CREATE opcode.
Create,
/// Insufficient balance during CREATE2 opcode.
Create2,
}

/// EVM Execution Error
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum ExecError {
Expand All @@ -113,8 +124,8 @@ pub enum ExecError {
WriteProtection,
/// For CALL, CALLCODE, DELEGATECALL, STATICCALL
Depth,
/// For CALL, CALLCODE
InsufficientBalance,
/// For CALL, CALLCODE, CREATE, CREATE2
InsufficientBalance(InsufficientBalanceError),
/// For CREATE, CREATE2
ContractAddressCollision,
/// contract must not begin with 0xef due to EIP #3541 EVM Object Format
Expand Down
14 changes: 9 additions & 5 deletions bus-mapping/src/evm/opcodes.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Definition of each opcode of the EVM.
use crate::{
circuit_input_builder::{CircuitInputStateRef, ExecStep},
error::{ExecError, OogError},
error::{ExecError, InsufficientBalanceError, OogError},
evm::OpcodeId,
operation::{
AccountField, AccountOp, CallContextField, StorageOp, TxAccessListAccountOp,
Expand Down Expand Up @@ -324,12 +324,16 @@ fn fn_gen_error_state_associated_ops(
ExecError::CodeStoreOutOfGas => Some(ErrorCodeStore::gen_associated_ops),
ExecError::MaxCodeSizeExceeded => Some(ErrorCodeStore::gen_associated_ops),
// call & callcode can encounter InsufficientBalance error, Use pop-7 generic CallOpcode
ExecError::InsufficientBalance => {
if geth_step.op.is_create() {
unimplemented!("insufficient balance for create");
}
ExecError::InsufficientBalance(InsufficientBalanceError::Call) => {
Some(CallOpcode::<7>::gen_associated_ops)
}
// create & create2 can encounter insufficient balance.
ExecError::InsufficientBalance(InsufficientBalanceError::Create) => {
Some(Create::<false>::gen_associated_ops)
}
ExecError::InsufficientBalance(InsufficientBalanceError::Create2) => {
Some(Create::<true>::gen_associated_ops)
}
ExecError::PrecompileFailed => Some(PrecompileFailed::gen_associated_ops),
ExecError::WriteProtection => Some(ErrorWriteProtection::gen_associated_ops),
ExecError::ReturnDataOutOfBounds => Some(ErrorReturnDataOutOfBound::gen_associated_ops),
Expand Down
58 changes: 40 additions & 18 deletions bus-mapping/src/evm/opcodes/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
},
)?;

let caller_balance = state.sdb.get_balance(&callee.caller_address);
state.account_read(
&mut exec_step,
callee.caller_address,
AccountField::Balance,
caller_balance,
);
let insufficient_balance = callee.value > caller_balance;

// TODO: look into when this can be pushed. Could it be done in parse call?
state.push_call(callee.clone());

Expand All @@ -120,24 +129,25 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
}

debug_assert!(state.sdb.get_nonce(&callee.address) == 0);
state.transfer(
&mut exec_step,
callee.caller_address,
callee.address,
true,
true,
callee.value,
)?;

state.push_op_reversible(
&mut exec_step,
AccountOp {
address: callee.address,
field: AccountField::Nonce,
value: 1.into(),
value_prev: 0.into(),
},
)?;
if !callee_exists && !insufficient_balance {
state.transfer(
&mut exec_step,
callee.caller_address,
callee.address,
true,
true,
callee.value,
)?;
state.push_op_reversible(
&mut exec_step,
AccountOp {
address: callee.address,
field: AccountField::Nonce,
value: 1.into(),
value_prev: 0.into(),
},
)?;
}

// Per EIP-150, all but one 64th of the caller's gas is sent to the
// initialization call.
Expand Down Expand Up @@ -169,6 +179,18 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
caller.depth.to_word(),
);

if insufficient_balance {
for (field, value) in [
(CallContextField::LastCalleeId, 0.into()),
(CallContextField::LastCalleeReturnDataOffset, 0.into()),
(CallContextField::LastCalleeReturnDataLength, 0.into()),
] {
state.call_context_write(&mut exec_step, caller.call_id, field, value);
}
state.handle_return(geth_step)?;
return Ok(vec![exec_step]);
}

for (field, value) in [
(CallContextField::CallerId, caller.call_id.into()),
(CallContextField::IsSuccess, callee.is_success.to_word()),
Expand Down
6 changes: 6 additions & 0 deletions bus-mapping/src/state_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,12 @@ impl StateDB {
self.dirty_storage.insert((*addr, *key), *value);
}

/// Get balance of account with the given address.
pub fn get_balance(&self, addr: &Address) -> Word {
let (_, account) = self.get_account(addr);
account.balance
}

/// Get nonce of account with `addr`.
pub fn get_nonce(&self, addr: &Address) -> u64 {
let (_, account) = self.get_account(addr);
Expand Down
12 changes: 4 additions & 8 deletions zkevm-circuits/src/evm_circuit/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ pub(crate) struct ExecutionConfig<F> {
shl_shr_gadget: Box<ShlShrGadget<F>>,
returndatasize_gadget: Box<ReturnDataSizeGadget<F>>,
returndatacopy_gadget: Box<ReturnDataCopyGadget<F>>,
create_gadget: Box<CreateGadget<F>>,
create_gadget: Box<CreateGadget<F, false, { ExecutionState::CREATE }>>,
create_gadget2: Box<CreateGadget<F, true, { ExecutionState::CREATE2 }>>,
selfdestruct_gadget: Box<DummyGadget<F, 1, 0, { ExecutionState::SELFDESTRUCT }>>,
signed_comparator_gadget: Box<SignedComparatorGadget<F>>,
signextend_gadget: Box<SignextendGadget<F>>,
Expand Down Expand Up @@ -361,8 +362,6 @@ pub(crate) struct ExecutionConfig<F> {
error_code_store: Box<ErrorCodeStoreGadget<F>>,
error_oog_self_destruct:
Box<DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasSELFDESTRUCT }>>,
error_insufficient_balance:
Box<DummyGadget<F, 0, 0, { ExecutionState::ErrorInsufficientBalance }>>,
error_nonce_uint_overflow:
Box<DummyGadget<F, 0, 0, { ExecutionState::ErrorNonceUintOverflow }>>,
error_invalid_jump: Box<ErrorInvalidJumpGadget<F>>,
Expand Down Expand Up @@ -625,6 +624,7 @@ impl<F: Field> ExecutionConfig<F> {
returndatasize_gadget: configure_gadget!(),
returndatacopy_gadget: configure_gadget!(),
create_gadget: configure_gadget!(),
create_gadget2: configure_gadget!(),
selfdestruct_gadget: configure_gadget!(),
shl_shr_gadget: configure_gadget!(),
signed_comparator_gadget: configure_gadget!(),
Expand All @@ -651,7 +651,6 @@ impl<F: Field> ExecutionConfig<F> {
error_oog_create2: configure_gadget!(),
error_oog_self_destruct: configure_gadget!(),
error_code_store: configure_gadget!(),
error_insufficient_balance: configure_gadget!(),
error_invalid_jump: configure_gadget!(),
error_invalid_opcode: configure_gadget!(),
error_write_protection: configure_gadget!(),
Expand Down Expand Up @@ -1507,6 +1506,7 @@ impl<F: Field> ExecutionConfig<F> {
ExecutionState::BLOCKHASH => assign_exec_step!(self.blockhash_gadget),
ExecutionState::SELFBALANCE => assign_exec_step!(self.selfbalance_gadget),
ExecutionState::CREATE => assign_exec_step!(self.create_gadget),
ExecutionState::CREATE2 => assign_exec_step!(self.create_gadget2),
// dummy gadgets
ExecutionState::EXTCODECOPY => assign_exec_step!(self.extcodecopy_gadget),
ExecutionState::SELFDESTRUCT => assign_exec_step!(self.selfdestruct_gadget),
Expand Down Expand Up @@ -1562,10 +1562,6 @@ impl<F: Field> ExecutionConfig<F> {
ExecutionState::ErrorStack => {
assign_exec_step!(self.error_stack)
}

ExecutionState::ErrorInsufficientBalance => {
assign_exec_step!(self.error_insufficient_balance)
}
ExecutionState::ErrorInvalidJump => {
assign_exec_step!(self.error_invalid_jump)
}
Expand Down
Loading

0 comments on commit 5deea13

Please sign in to comment.