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

Feat/CREATE Part B - Error cases that will be handled within opcode gadgets #1425

Merged
merged 8 commits into from
May 26, 2023
35 changes: 32 additions & 3 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,10 @@ use super::{
TransactionContext,
};
use crate::{
error::{get_step_reported_error, ExecError},
error::{
get_step_reported_error, DepthError, ExecError, InsufficientBalanceError,
NonceUintOverflowError,
},
exec_trace::OperationRef,
operation::{
AccountField, AccountOp, CallContextField, CallContextOp, MemoryOp, Op, OpEnum, Operation,
Expand Down Expand Up @@ -1290,7 +1293,14 @@ impl<'a> CircuitInputStateRef<'a> {
&& next_pc != 0
{
if step.depth == 1025 {
return Ok(Some(ExecError::Depth));
return Ok(Some(ExecError::Depth(match step.op {
OpcodeId::CALL | OpcodeId::CALLCODE => DepthError::Call,
OpcodeId::CREATE => DepthError::Create,
OpcodeId::CREATE2 => DepthError::Create2,
op => {
unreachable!("Depth error unexpected for opcode: {:?}", op)
}
})));
}

let sender = self.call()?.address;
Expand All @@ -1299,7 +1309,26 @@ impl<'a> CircuitInputStateRef<'a> {
return Err(Error::AccountNotFound(sender));
}
if account.balance < value {
return Ok(Some(ExecError::InsufficientBalance));
return Ok(Some(ExecError::InsufficientBalance(match step.op {
OpcodeId::CALL | OpcodeId::CALLCODE => InsufficientBalanceError::Call,
OpcodeId::CREATE => InsufficientBalanceError::Create,
OpcodeId::CREATE2 => InsufficientBalanceError::Create2,
op => {
unreachable!("insufficient balance error unexpected for opcode: {:?}", op)
}
})));
}

// Nonce Uint overflow
// If user's nonce is equal u64::MAX, nonce will be overflow in this call
// Nonce is u64 so it's impossible to larger than u64::MAX, that's why we're using `==`
// here.
if account.nonce == u64::MAX {
return Ok(Some(ExecError::NonceUintOverflow(match step.op {
OpcodeId::CREATE => NonceUintOverflowError::Create,
OpcodeId::CREATE2 => NonceUintOverflowError::Create2,
op => unreachable!("Nonce Uint overflow error unexpected for opcode: {:?}", op),
})));
}

// Address collision
Expand Down
8 changes: 5 additions & 3 deletions bus-mapping/src/circuit_input_builder/tracer_tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::*;
use crate::{
circuit_input_builder::access::gen_state_access_trace,
error::{ExecError, OogError},
error::{DepthError, ExecError, InsufficientBalanceError, OogError},
geth_errors::{
GETH_ERR_GAS_UINT_OVERFLOW, GETH_ERR_OUT_OF_GAS, GETH_ERR_STACK_OVERFLOW,
GETH_ERR_STACK_UNDERFLOW,
Expand Down Expand Up @@ -208,7 +208,7 @@ fn tracer_err_depth() {
let mut builder = CircuitInputBuilderTx::new(&block, step);
assert_eq!(
builder.state_ref().get_step_err(step, next_step).unwrap(),
Some(ExecError::Depth)
Some(ExecError::Depth(DepthError::Call))
);
}

Expand Down Expand Up @@ -274,7 +274,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
43 changes: 38 additions & 5 deletions bus-mapping/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,37 @@ 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,
}

/// Nonce uint overflow errors by opcode/state.
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum NonceUintOverflowError {
/// Nonce uint overflow during CREATE opcode.
Create,
/// Nonce uint overflow during CREATE2 opcode.
Create2,
}

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

/// EVM Execution Error
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum ExecError {
Expand All @@ -112,11 +143,11 @@ pub enum ExecError {
/// For SSTORE, LOG0, LOG1, LOG2, LOG3, LOG4, CREATE, CALL, CREATE2,
/// SELFDESTRUCT
WriteProtection,
/// For CALL, CALLCODE, DELEGATECALL, STATICCALL
Depth,
/// For CALL, CALLCODE
InsufficientBalance,
/// For CREATE, CREATE2
/// For CALL, CALLCODE, DELEGATECALL, STATICCALL, CREATE, CREATE2
Depth(DepthError),
/// For CALL, CALLCODE, CREATE, CREATE2
InsufficientBalance(InsufficientBalanceError),
/// For CREATE2
ContractAddressCollision,
/// contract must not begin with 0xef due to EIP #3541 EVM Object Format
/// (EOF)
Expand All @@ -129,6 +160,8 @@ pub enum ExecError {
CodeStoreOutOfGas,
/// For RETURN in a CREATE, CREATE2
MaxCodeSizeExceeded,
/// For CREATE, CREATE2
NonceUintOverflow(NonceUintOverflowError),
}

// TODO: Move to impl block.
Expand Down
40 changes: 26 additions & 14 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, ExecState, ExecStep},
error::{ExecError, OogError},
error::{DepthError, ExecError, InsufficientBalanceError, NonceUintOverflowError, OogError},
evm::OpcodeId,
operation::TxAccessListAccountOp,
Error,
Expand Down Expand Up @@ -271,10 +271,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps {
}
}

fn fn_gen_error_state_associated_ops(
geth_step: &GethExecStep,
error: &ExecError,
) -> Option<FnGenAssociatedOps> {
fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option<FnGenAssociatedOps> {
match error {
ExecError::InvalidJump => Some(InvalidJump::gen_associated_ops),
ExecError::InvalidOpcode => Some(ErrorSimple::gen_associated_ops),
Expand All @@ -287,16 +284,31 @@ fn fn_gen_error_state_associated_ops(
ExecError::StackOverflow => Some(ErrorSimple::gen_associated_ops),
ExecError::StackUnderflow => Some(ErrorSimple::gen_associated_ops),
// call & callcode can encounter InsufficientBalance error, Use pop-7 generic CallOpcode
ExecError::InsufficientBalance => Some(CallOpcode::<7>::gen_associated_ops),
ExecError::InsufficientBalance(InsufficientBalanceError::Call) => {
Some(CallOpcode::<7>::gen_associated_ops)
}
// create & create2 can encounter insufficient balance.
ExecError::InsufficientBalance(InsufficientBalanceError::Create) => {
Some(DummyCreate::<false>::gen_associated_ops)
}
ExecError::InsufficientBalance(InsufficientBalanceError::Create2) => {
Some(DummyCreate::<true>::gen_associated_ops)
}
// only create2 may cause ContractAddressCollision error, so use DummyCreate::<true>.
ExecError::ContractAddressCollision => Some(DummyCreate::<true>::gen_associated_ops),
// create & create2 can encounter nonce uint overflow.
ExecError::NonceUintOverflow(NonceUintOverflowError::Create) => {
Some(DummyCreate::<false>::gen_associated_ops)
}
ExecError::NonceUintOverflow(NonceUintOverflowError::Create2) => {
Some(DummyCreate::<true>::gen_associated_ops)
}
ExecError::WriteProtection => Some(ErrorWriteProtection::gen_associated_ops),
ExecError::ReturnDataOutOfBounds => Some(ErrorReturnDataOutOfBound::gen_associated_ops),
ExecError::Depth => {
let op = geth_step.op;
if !op.is_call() {
evm_unimplemented!("TODO: ErrDepth for CREATE is not implemented yet");
}
Some(fn_gen_associated_ops(&op))
}
// call, callcode, create & create2 can encounter DepthError error,
ExecError::Depth(DepthError::Call) => Some(CallOpcode::<7>::gen_associated_ops),
ExecError::Depth(DepthError::Create) => Some(DummyCreate::<false>::gen_associated_ops),
ExecError::Depth(DepthError::Create2) => Some(DummyCreate::<true>::gen_associated_ops),
// more future errors place here
_ => {
evm_unimplemented!("TODO: error state {:?} not implemented", error);
Expand Down Expand Up @@ -342,7 +354,7 @@ pub fn gen_associated_ops(
// TODO: after more error state handled, refactor all error handling in
// fn_gen_error_state_associated_ops method
// For exceptions that have been implemented
if let Some(fn_gen_error_ops) = fn_gen_error_state_associated_ops(geth_step, &exec_error) {
if let Some(fn_gen_error_ops) = fn_gen_error_state_associated_ops(&exec_error) {
return fn_gen_error_ops(state, geth_steps);
} else {
// For exceptions that fail to enter next call context, we need
Expand Down
9 changes: 0 additions & 9 deletions zkevm-circuits/src/evm_circuit/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1280,9 +1280,6 @@ impl<F: Field> ExecutionConfig<F> {
assign_exec_step!(self.error_stack)
}

ExecutionState::ErrorInsufficientBalance => {
assign_exec_step!(self.call_op_gadget)
}
ExecutionState::ErrorInvalidJump => {
assign_exec_step!(self.error_invalid_jump)
}
Expand All @@ -1292,12 +1289,6 @@ impl<F: Field> ExecutionConfig<F> {
ExecutionState::ErrorWriteProtection => {
assign_exec_step!(self.error_write_protection)
}
ExecutionState::ErrorDepth => {
assign_exec_step!(self.error_depth)
}
ExecutionState::ErrorContractAddressCollision => {
assign_exec_step!(self.error_contract_address_collision)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since ErrDepth will be processed inside create gadget and call gadget, we can remove this code too?

            ExecutionState::ErrorDepth => {
                assign_exec_step!(self.error_depth)
            }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 4147e93

ExecutionState::ErrorInvalidCreationCode => {
assign_exec_step!(self.error_invalid_creation_code)
}
Expand Down
22 changes: 18 additions & 4 deletions zkevm-circuits/src/evm_circuit/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
};
use bus_mapping::{
circuit_input_builder::ExecState,
error::{ExecError, OogError},
error::{DepthError, ExecError, InsufficientBalanceError, NonceUintOverflowError, OogError},
evm::OpcodeId,
};
use eth_types::{evm_unimplemented, Field, ToWord};
Expand Down Expand Up @@ -131,9 +131,23 @@ impl From<&ExecError> for ExecutionState {
ExecError::InvalidOpcode => ExecutionState::ErrorInvalidOpcode,
ExecError::StackOverflow | ExecError::StackUnderflow => ExecutionState::ErrorStack,
ExecError::WriteProtection => ExecutionState::ErrorWriteProtection,
ExecError::Depth => ExecutionState::ErrorDepth,
ExecError::InsufficientBalance => ExecutionState::ErrorInsufficientBalance,
ExecError::ContractAddressCollision => ExecutionState::ErrorContractAddressCollision,
ExecError::Depth(depth_error) => match depth_error {
DepthError::Call => ExecutionState::CALL_OP,
DepthError::Create => ExecutionState::CREATE,
DepthError::Create2 => ExecutionState::CREATE2,
},
ExecError::InsufficientBalance(insufficient_balance_err) => {
match insufficient_balance_err {
InsufficientBalanceError::Call => ExecutionState::CALL_OP,
InsufficientBalanceError::Create => ExecutionState::CREATE,
InsufficientBalanceError::Create2 => ExecutionState::CREATE2,
}
}
ExecError::NonceUintOverflow(nonce_overflow_err) => match nonce_overflow_err {
NonceUintOverflowError::Create => ExecutionState::CREATE,
NonceUintOverflowError::Create2 => ExecutionState::CREATE2,
},
ExecError::ContractAddressCollision => ExecutionState::CREATE2,
ExecError::InvalidCreationCode => ExecutionState::ErrorInvalidCreationCode,
ExecError::InvalidJump => ExecutionState::ErrorInvalidJump,
ExecError::ReturnDataOutOfBounds => ExecutionState::ErrorReturnDataOutOfBound,
Expand Down