From 44915134f95fc12c8d07c0016a18f4968e03eaca Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 16 Nov 2023 11:22:29 +0800 Subject: [PATCH 1/2] Enable `BlockCtxU256Gadget` for `BASEFEE` opcode in `scroll` feature. --- zkevm-circuits/src/evm_circuit.rs | 4 --- zkevm-circuits/src/evm_circuit/execution.rs | 10 ++++++ .../src/evm_circuit/execution/block_ctx.rs | 32 +++++++++++-------- zkevm-circuits/src/evm_circuit/step.rs | 7 ++-- zkevm-circuits/src/witness/step.rs | 9 +++++- 5 files changed, 41 insertions(+), 21 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index 7d999e1b58..0f83c7f3c8 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -283,11 +283,7 @@ impl EvmCircuit { } } -// the diff is from the num of valid opcodes: self-destruct? -#[cfg(not(feature = "scroll"))] const FIXED_TABLE_ROWS_NO_BITWISE: usize = 3647; -#[cfg(feature = "scroll")] -const FIXED_TABLE_ROWS_NO_BITWISE: usize = 3646; const FIXED_TABLE_ROWS: usize = FIXED_TABLE_ROWS_NO_BITWISE + 3 * 65536; impl SubCircuit for EvmCircuit { diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 9801ef2fa6..704610a6e9 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -146,6 +146,8 @@ use address::AddressGadget; use balance::BalanceGadget; use begin_tx::BeginTxGadget; use bitwise::BitwiseGadget; +#[cfg(feature = "scroll")] +use block_ctx::DifficultyGadget; use block_ctx::{BlockCtxU160Gadget, BlockCtxU256Gadget, BlockCtxU64Gadget}; use blockhash::BlockHashGadget; use byte::ByteGadget; @@ -328,6 +330,8 @@ pub(crate) struct ExecutionConfig { block_ctx_u64_gadget: Box>, block_ctx_u160_gadget: Box>, block_ctx_u256_gadget: Box>, + #[cfg(feature = "scroll")] + difficulty_gadget: Box>, // error gadgets error_oog_call: Box>, error_oog_precompile: Box>, @@ -606,6 +610,8 @@ impl ExecutionConfig { block_ctx_u64_gadget: configure_gadget!(), block_ctx_u160_gadget: configure_gadget!(), block_ctx_u256_gadget: configure_gadget!(), + #[cfg(feature = "scroll")] + difficulty_gadget: configure_gadget!(), // error gadgets error_oog_constant: configure_gadget!(), error_oog_static_memory_gadget: configure_gadget!(), @@ -1439,6 +1445,10 @@ impl ExecutionConfig { ExecutionState::BLOCKCTXU64 => assign_exec_step!(self.block_ctx_u64_gadget), ExecutionState::BLOCKCTXU160 => assign_exec_step!(self.block_ctx_u160_gadget), ExecutionState::BLOCKCTXU256 => assign_exec_step!(self.block_ctx_u256_gadget), + ExecutionState::DIFFICULTY => { + #[cfg(feature = "scroll")] + assign_exec_step!(self.difficulty_gadget) + } ExecutionState::BLOCKHASH => assign_exec_step!(self.blockhash_gadget), ExecutionState::SELFBALANCE => assign_exec_step!(self.selfbalance_gadget), ExecutionState::CREATE => assign_exec_step!(self.create_gadget), diff --git a/zkevm-circuits/src/evm_circuit/execution/block_ctx.rs b/zkevm-circuits/src/evm_circuit/execution/block_ctx.rs index 58e06efd22..a388529a3f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/block_ctx.rs +++ b/zkevm-circuits/src/evm_circuit/execution/block_ctx.rs @@ -147,13 +147,16 @@ impl ExecutionGadget for BlockCtxU160Gadget { } } -#[cfg(not(feature = "scroll"))] +// TODO: +// This gadget is used for `BASEFEE` opcode. With current `scroll` feature, it's +// disabled by l2geth and converted to an invalid opcode. +// +// So need to test it after `BASEFEE` opcode is enabled in scroll l2geth. #[derive(Clone, Debug)] pub(crate) struct BlockCtxU256Gadget { value_u256: BlockCtxGadget, } -#[cfg(not(feature = "scroll"))] impl ExecutionGadget for BlockCtxU256Gadget { const NAME: &'static str = "BLOCKCTXU256"; @@ -174,6 +177,11 @@ impl ExecutionGadget for BlockCtxU256Gadget { _: &Call, step: &ExecStep, ) -> Result<(), Error> { + if cfg!(feature = "scroll") { + panic!("BASEFEE is disabled by scroll for now"); + } + log::debug!("BlockCtxU256Gadget assign for {:?}", step.opcode); + self.value_u256 .same_context .assign_exec_step(region, offset, step)?; @@ -188,17 +196,20 @@ impl ExecutionGadget for BlockCtxU256Gadget { } } +#[cfg(feature = "scroll")] #[derive(Clone, Debug)] -pub(crate) struct DifficulityGadget { +pub(crate) struct DifficultyGadget { same_context: SameContextGadget, } -impl ExecutionGadget for DifficulityGadget { - const NAME: &'static str = "DIFFICULITY"; +#[cfg(feature = "scroll")] +impl ExecutionGadget for DifficultyGadget { + const NAME: &'static str = "DIFFICULTY"; - const EXECUTION_STATE: ExecutionState = ExecutionState::BLOCKCTXU256; + const EXECUTION_STATE: ExecutionState = ExecutionState::DIFFICULTY; fn configure(cb: &mut EVMConstraintBuilder) -> Self { + // Always returns 0 for scroll. cb.stack_push(0.expr()); let opcode = cb.query_cell(); // State transition @@ -223,17 +234,10 @@ impl ExecutionGadget for DifficulityGadget { _: &Call, step: &ExecStep, ) -> Result<(), Error> { - self.same_context.assign_exec_step(region, offset, step)?; - - Ok(()) + self.same_context.assign_exec_step(region, offset, step) } } -// notice in scroll ,the BASEFEE is invalid and difficulity has been -// changed -#[cfg(feature = "scroll")] -pub(crate) use DifficulityGadget as BlockCtxU256Gadget; - #[cfg(test)] mod test { use crate::test_util::CircuitTestBuilder; diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index b26236f4e9..a500693d58 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -80,7 +80,8 @@ pub enum ExecutionState { BLOCKHASH, BLOCKCTXU64, // TIMESTAMP, NUMBER, GASLIMIT BLOCKCTXU160, // COINBASE - BLOCKCTXU256, // DIFFICULTY, BASEFEE + BLOCKCTXU256, // BASEFEE + DIFFICULTY, // DIFFICULTY CHAINID, SELFBALANCE, POP, @@ -270,11 +271,13 @@ impl ExecutionState { Self::BLOCKCTXU160 => vec![OpcodeId::COINBASE], Self::BLOCKCTXU256 => { if cfg!(feature = "scroll") { - vec![OpcodeId::DIFFICULTY] + vec![OpcodeId::BASEFEE] } else { vec![OpcodeId::DIFFICULTY, OpcodeId::BASEFEE] } } + #[cfg(feature = "scroll")] + Self::DIFFICULTY => vec![OpcodeId::DIFFICULTY], Self::CHAINID => vec![OpcodeId::CHAINID], Self::SELFBALANCE => vec![OpcodeId::SELFBALANCE], Self::POP => vec![OpcodeId::POP], diff --git a/zkevm-circuits/src/witness/step.rs b/zkevm-circuits/src/witness/step.rs index a7336a591a..c589d91522 100644 --- a/zkevm-circuits/src/witness/step.rs +++ b/zkevm-circuits/src/witness/step.rs @@ -187,7 +187,14 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState { ExecutionState::BLOCKCTXU64 } OpcodeId::COINBASE => ExecutionState::BLOCKCTXU160, - OpcodeId::DIFFICULTY | OpcodeId::BASEFEE => ExecutionState::BLOCKCTXU256, + OpcodeId::BASEFEE => ExecutionState::BLOCKCTXU256, + OpcodeId::DIFFICULTY => { + if cfg!(feature = "scroll") { + ExecutionState::DIFFICULTY + } else { + ExecutionState::BLOCKCTXU256 + } + } OpcodeId::GAS => ExecutionState::GAS, OpcodeId::SAR => ExecutionState::SAR, OpcodeId::SELFBALANCE => ExecutionState::SELFBALANCE, From a511ed4157341a9555ef28297914000a935287c5 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Tue, 21 Nov 2023 22:17:47 +0800 Subject: [PATCH 2/2] Fix to restrict `DIFFICULTY` execution state only for scroll (to avoid soundness). --- zkevm-circuits/src/evm_circuit/execution.rs | 6 ++---- zkevm-circuits/src/evm_circuit/step.rs | 5 +++-- zkevm-circuits/src/witness/step.rs | 11 ++++------- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 704610a6e9..1954114746 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -1445,10 +1445,8 @@ impl ExecutionConfig { ExecutionState::BLOCKCTXU64 => assign_exec_step!(self.block_ctx_u64_gadget), ExecutionState::BLOCKCTXU160 => assign_exec_step!(self.block_ctx_u160_gadget), ExecutionState::BLOCKCTXU256 => assign_exec_step!(self.block_ctx_u256_gadget), - ExecutionState::DIFFICULTY => { - #[cfg(feature = "scroll")] - assign_exec_step!(self.difficulty_gadget) - } + #[cfg(feature = "scroll")] + ExecutionState::DIFFICULTY => assign_exec_step!(self.difficulty_gadget), ExecutionState::BLOCKHASH => assign_exec_step!(self.blockhash_gadget), ExecutionState::SELFBALANCE => assign_exec_step!(self.selfbalance_gadget), ExecutionState::CREATE => assign_exec_step!(self.create_gadget), diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index a500693d58..89026375f1 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -80,8 +80,9 @@ pub enum ExecutionState { BLOCKHASH, BLOCKCTXU64, // TIMESTAMP, NUMBER, GASLIMIT BLOCKCTXU160, // COINBASE - BLOCKCTXU256, // BASEFEE - DIFFICULTY, // DIFFICULTY + BLOCKCTXU256, // BASEFEE, DIFFICULTY (for non-scroll) + #[cfg(feature = "scroll")] + DIFFICULTY, // DIFFICULTY CHAINID, SELFBALANCE, POP, diff --git a/zkevm-circuits/src/witness/step.rs b/zkevm-circuits/src/witness/step.rs index c589d91522..c1a0349174 100644 --- a/zkevm-circuits/src/witness/step.rs +++ b/zkevm-circuits/src/witness/step.rs @@ -188,13 +188,10 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState { } OpcodeId::COINBASE => ExecutionState::BLOCKCTXU160, OpcodeId::BASEFEE => ExecutionState::BLOCKCTXU256, - OpcodeId::DIFFICULTY => { - if cfg!(feature = "scroll") { - ExecutionState::DIFFICULTY - } else { - ExecutionState::BLOCKCTXU256 - } - } + #[cfg(not(feature = "scroll"))] + OpcodeId::DIFFICULTY => ExecutionState::BLOCKCTXU256, + #[cfg(feature = "scroll")] + OpcodeId::DIFFICULTY => ExecutionState::DIFFICULTY, OpcodeId::GAS => ExecutionState::GAS, OpcodeId::SAR => ExecutionState::SAR, OpcodeId::SELFBALANCE => ExecutionState::SELFBALANCE,