diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index d3ea1a204c..f9f9de0333 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -1,6 +1,5 @@ //! The EVM circuit implementation. -#![allow(missing_docs)] use halo2_proofs::{ circuit::{Layouter, SimpleFloorPlanner, Value}, plonk::*, @@ -41,6 +40,7 @@ pub struct EvmCircuitConfig { fixed_table: [Column; 4], u8_table: UXTable<8>, u16_table: UXTable<16>, + /// The execution config pub execution: Box>, // External tables tx_table: TxTable, @@ -183,17 +183,19 @@ impl EvmCircuit { fixed_table_tags: FixedTableTag::iter().collect(), } } - - pub fn get_test_cicuit_from_block(block: Block) -> Self { + #[cfg(feature = "test-circuits")] + /// Construct the EvmCircuit with only subset of Fixed table tags required by tests to save + /// testing time + pub(crate) fn get_test_circuit_from_block(block: Block) -> Self { let fixed_table_tags = detect_fixed_table_tags(&block); Self { block: Some(block), fixed_table_tags, } } - + #[cfg(feature = "test-circuits")] /// Calculate which rows are "actually" used in the circuit - pub fn get_active_rows(block: &Block) -> (Vec, Vec) { + pub(crate) fn get_active_rows(block: &Block) -> (Vec, Vec) { let max_offset = Self::get_num_rows_required(block); // some gates are enabled on all rows let gates_row_ids = (0..max_offset).collect(); @@ -201,8 +203,9 @@ impl EvmCircuit { let lookup_row_ids = (0..max_offset).collect(); (gates_row_ids, lookup_row_ids) } - - pub fn get_num_rows_required(block: &Block) -> usize { + /// Get the minimum number of rows required to process the block + /// If unspecified, then compute it + pub(crate) fn get_num_rows_required(block: &Block) -> usize { let evm_rows = block.circuits_params.max_evm_rows; if evm_rows == 0 { Self::get_min_num_rows_required(block) @@ -211,8 +214,8 @@ impl EvmCircuit { block.circuits_params.max_evm_rows + 1 } } - - pub fn get_min_num_rows_required(block: &Block) -> usize { + /// Compute the minimum number of rows required to process the block + fn get_min_num_rows_required(block: &Block) -> usize { let mut num_rows = 0; for transaction in &block.txs { for step in transaction.steps() { @@ -343,8 +346,8 @@ pub(crate) mod cached { } impl EvmCircuitCached { - pub fn get_test_cicuit_from_block(block: Block) -> Self { - Self(EvmCircuit::::get_test_cicuit_from_block(block)) + pub(crate) fn get_test_circuit_from_block(block: Block) -> Self { + Self(EvmCircuit::::get_test_circuit_from_block(block)) } } } @@ -495,7 +498,7 @@ mod evm_circuit_stats { let block = block_convert::(&builder).unwrap(); let k = block.get_test_degree(); - let circuit = EvmCircuit::::get_test_cicuit_from_block(block); + let circuit = EvmCircuit::::get_test_circuit_from_block(block); let prover1 = MockProver::::run(k, &circuit, vec![]).unwrap(); let code = bytecode! { @@ -516,7 +519,7 @@ mod evm_circuit_stats { .unwrap(); let block = block_convert::(&builder).unwrap(); let k = block.get_test_degree(); - let circuit = EvmCircuit::::get_test_cicuit_from_block(block); + let circuit = EvmCircuit::::get_test_circuit_from_block(block); let prover2 = MockProver::::run(k, &circuit, vec![]).unwrap(); assert_eq!(prover1.fixed(), prover2.fixed()); diff --git a/zkevm-circuits/src/evm_circuit/param.rs b/zkevm-circuits/src/evm_circuit/param.rs index ff9617e132..d12b023111 100644 --- a/zkevm-circuits/src/evm_circuit/param.rs +++ b/zkevm-circuits/src/evm_circuit/param.rs @@ -1,3 +1,4 @@ +//! Constants and parameters for the EVM circuit use super::table::Table; use crate::evm_circuit::{step::ExecutionState, EvmCircuit}; use halo2_proofs::{ @@ -22,12 +23,14 @@ pub const N_PHASE2_COLUMNS: usize = 1; pub const N_PHASE1_COLUMNS: usize = STEP_WIDTH - EVM_LOOKUP_COLS - N_PHASE2_COLUMNS - N_COPY_COLUMNS - N_U8_LOOKUPS - N_U16_LOOKUPS; -// Number of copy columns +/// Number of copy columns pub const N_COPY_COLUMNS: usize = 2; +/// Number of columns reserved for u8 lookup pub const N_U8_LOOKUPS: usize = 24; // TODO shift #column/2 from u8 to u16 +/// Number of columns reserved for u16 lookup pub const N_U16_LOOKUPS: usize = 0; /// Amount of lookup columns in the EVM circuit dedicated to lookups. diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index aaa99e9ceb..b87277eefd 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -1,3 +1,8 @@ +//! EVM execution state. We model the EVM execution as a finite state machine. The execution of a +//! EVM block goes from one state to another, but never at an undefined state. The EVM circuit +//! enables the selectors and thus activates the constraints for the state that the execution +//! reaches. + use super::{ param::MAX_STEP_HEIGHT, util::{evm_cm_distribute_advice, CachedRegion, Cell, CellType}, @@ -27,8 +32,11 @@ use std::{fmt::Display, iter}; use strum::IntoEnumIterator; use strum_macros::EnumIter; +#[allow(missing_docs, reason = "some docs here are tedious and not helpful")] #[allow(non_camel_case_types)] #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, EnumIter)] +/// All the possible execution states that the computation of EVM can arrive. +/// Some states are shared by multiple opcodes. pub enum ExecutionState { // Internal state BeginTx, @@ -36,18 +44,25 @@ pub enum ExecutionState { EndBlock, // Opcode successful cases STOP, - ADD_SUB, // ADD, SUB - MUL_DIV_MOD, // MUL, DIV, MOD - SDIV_SMOD, // SDIV, SMOD - SHL_SHR, // SHL, SHR + /// ADD and SUB opcodes share this state + ADD_SUB, + /// MUL, DIV, MOD + MUL_DIV_MOD, + /// SDIV, SMOD + SDIV_SMOD, + /// SHL, SHR + SHL_SHR, ADDMOD, MULMOD, EXP, SIGNEXTEND, - CMP, // LT, GT, EQ - SCMP, // SLT, SGT + /// LT, GT, EQ + CMP, + /// SLT, SGT + SCMP, ISZERO, - BITWISE, // AND, OR, XOR + /// AND, OR, XOR + BITWISE, NOT, BYTE, SAR, @@ -69,11 +84,13 @@ pub enum ExecutionState { RETURNDATACOPY, EXTCODEHASH, BLOCKHASH, - BLOCKCTX, // TIMESTAMP, NUMBER, GASLIMIT, COINBASE, DIFFICULTY, BASEFEE + /// TIMESTAMP, NUMBER, GASLIMIT, COINBASE, DIFFICULTY, BASEFEE + BLOCKCTX, CHAINID, SELFBALANCE, POP, - MEMORY, // MLOAD, MSTORE, MSTORE8 + /// MLOAD, MSTORE, MSTORE8 + MEMORY, SLOAD, SSTORE, JUMP, @@ -82,13 +99,18 @@ pub enum ExecutionState { MSIZE, GAS, JUMPDEST, - PUSH, // PUSH1, PUSH2, ..., PUSH32 - DUP, // DUP1, DUP2, ..., DUP16 - SWAP, // SWAP1, SWAP2, ..., SWAP16 - LOG, // LOG0, LOG1, ..., LOG4 + /// PUSH1, PUSH2, ..., PUSH32 + PUSH, + /// DUP1, DUP2, ..., DUP16 + DUP, + /// SWAP1, SWAP2, ..., SWAP16 + SWAP, + /// LOG0, LOG1, ..., LOG4 + LOG, CREATE, - CALL_OP, // CALL, CALLCODE, DELEGATECALL, STATICCALL - RETURN_REVERT, // RETURN, REVERT + /// CALL, CALLCODE, DELEGATECALL, STATICCALL + CALL_OP, + RETURN_REVERT, CREATE2, SELFDESTRUCT, // Error cases @@ -283,7 +305,7 @@ impl From<&ExecStep> for ExecutionState { } } -pub trait HasExecutionState { +pub(crate) trait HasExecutionState { fn execution_state(&self) -> ExecutionState; } @@ -334,6 +356,7 @@ impl ExecutionState { || self.halts_in_exception() } + /// Get the opocdes that are related to the execution state pub fn responsible_opcodes(&self) -> Vec { if matches!(self, Self::ErrorStack) { return OpcodeId::valid_opcodes() @@ -498,11 +521,12 @@ impl ExecutionState { .collect() } + /// Get the state hight pub fn get_step_height_option(&self) -> Option { EXECUTION_STATE_HEIGHT_MAP.get(self).copied() } - pub fn get_step_height(&self) -> usize { + pub(crate) fn get_step_height(&self) -> usize { self.get_step_height_option() .unwrap_or_else(|| panic!("Execution state unknown: {:?}", self)) } @@ -525,6 +549,7 @@ impl From for ResponsibleOp { } impl ResponsibleOp { + /// Get the opcode pub fn opcode(&self) -> OpcodeId { *match self { ResponsibleOp::Op(opcode) => opcode, diff --git a/zkevm-circuits/src/evm_circuit/table.rs b/zkevm-circuits/src/evm_circuit/table.rs index f9fd4e468e..38557ca6fe 100644 --- a/zkevm-circuits/src/evm_circuit/table.rs +++ b/zkevm-circuits/src/evm_circuit/table.rs @@ -1,3 +1,5 @@ +//! Fixed lookup tables and dynamic lookup tables for the EVM circuit + use crate::{ evm_circuit::step::{ExecutionState, ResponsibleOp}, impl_expr, @@ -11,28 +13,46 @@ use strum::IntoEnumIterator; use strum_macros::EnumIter; #[derive(Clone, Copy, Debug, EnumIter)] +/// Tags for different fixed tables pub enum FixedTableTag { + /// x == 0 Zero = 0, + /// 0 <= x < 5 Range5, + /// 0 <= x < 16 Range16, + /// 0 <= x < 32 Range32, + /// 0 <= x < 64 Range64, + /// 0 <= x < 128 Range128, + /// 0 <= x < 256 Range256, + /// 0 <= x < 512 Range512, + /// 0 <= x < 1024 Range1024, + /// -128 <= x < 128 SignByte, + /// bitwise AND BitwiseAnd, + /// bitwise OR BitwiseOr, + /// bitwise XOR BitwiseXor, + /// lookup for corresponding opcode ResponsibleOpcode, + /// power of 2 Pow2, + /// Lookup constant gas cost for opcodes ConstantGasCost, } impl_expr!(FixedTableTag); impl FixedTableTag { - pub fn build(&self) -> Box> { + /// build up the fixed table row values + pub(crate) fn build(&self) -> Box> { let tag = F::from(*self as u64); match self { Self::Zero => Box::new((0..1).map(move |_| [tag, F::ZERO, F::ZERO, F::ZERO])), @@ -122,33 +142,57 @@ impl FixedTableTag { } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, EnumIter)] +/// Each item represents the lookup table to query pub enum Table { + /// The range check table for u8 U8, + /// The range check table for u16 U16, + /// The rest of the fixed table. See [`FixedTableTag`] Fixed, + /// Lookup for transactions Tx, + /// Lookup for read write operations Rw, + /// Lookup for bytecode table Bytecode, + /// Lookup for block constants Block, + /// Lookup for copy table Copy, + /// Lookup for keccak table Keccak, + /// Lookup for exp table Exp, } #[derive(Clone, Debug)] -pub struct RwValues { - pub id: Expression, - pub address: Expression, - pub field_tag: Expression, - pub storage_key: Word>, - pub value: Word>, - pub value_prev: Word>, - pub init_val: Word>, +/// Read-Write Table fields +pub(crate) struct RwValues { + /// The unique identifier for the Read or Write. Depending on context, this field could be used + /// for Transaction ID or call ID + id: Expression, + /// The position to Stack, Memory, or account, where the read or write takes place, depending + /// on the cell value of the [`bus_mapping::operation::Target`]. + address: Expression, + /// Could be [`crate::table::CallContextFieldTag`], [`crate::table::AccountFieldTag`], + /// [`crate::table::TxLogFieldTag`], or [`crate::table::TxReceiptFieldTag`] depending on + /// the cell value of the [`bus_mapping::operation::Target`] + field_tag: Expression, + /// Storage key of two limbs + storage_key: Word>, + /// The current storage value + value: Word>, + /// The previous storage value + value_prev: Word>, + /// The initial storage value before the current transaction + init_val: Word>, } impl RwValues { + /// Constructor for RwValues #[allow(clippy::too_many_arguments)] - pub fn new( + pub(crate) fn new( id: Expression, address: Expression, field_tag: Expression, @@ -167,6 +211,15 @@ impl RwValues { init_val, } } + + pub(crate) fn revert_value(&self) -> Self { + let new_self = self.clone(); + Self { + value_prev: new_self.value, + value: new_self.value_prev, + ..new_self + } + } } #[derive(Clone, Debug)] diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 21415874d2..f62c46bd79 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -814,11 +814,7 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> { reversion_info.rw_counter_of_reversion(reversible_write_counter_inc_selector), true.expr(), tag, - RwValues { - value_prev: values.value, - value: values.value_prev, - ..values - }, + values.revert_value(), ) }); } @@ -1302,15 +1298,15 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> { counter, 0.expr(), Target::Start, - RwValues { - id: 0.expr(), - address: 0.expr(), - field_tag: 0.expr(), - storage_key: Word::zero(), - value: Word::zero(), - value_prev: Word::zero(), - init_val: Word::zero(), - }, + RwValues::new( + 0.expr(), + 0.expr(), + 0.expr(), + Word::zero(), + Word::zero(), + Word::zero(), + Word::zero(), + ), ); } diff --git a/zkevm-circuits/src/lib.rs b/zkevm-circuits/src/lib.rs index f093c5add4..a1de8df42f 100644 --- a/zkevm-circuits/src/lib.rs +++ b/zkevm-circuits/src/lib.rs @@ -16,8 +16,6 @@ #![deny(missing_docs)] #![deny(unsafe_code)] #![deny(clippy::debug_assert_with_mut_call)] -#![allow(deprecated)] -#![allow(unreachable_code)] pub mod bytecode_circuit; pub mod copy_circuit; diff --git a/zkevm-circuits/src/table/keccak_table.rs b/zkevm-circuits/src/table/keccak_table.rs index 670aaba375..7f36be98a1 100644 --- a/zkevm-circuits/src/table/keccak_table.rs +++ b/zkevm-circuits/src/table/keccak_table.rs @@ -11,9 +11,6 @@ pub struct KeccakTable { pub input_len: Column, /// Output hash word pub output: word::Word>, - #[deprecated] - /// RLC of the hash result - pub output_rlc: Column, } impl LookupTable for KeccakTable { @@ -46,7 +43,6 @@ impl KeccakTable { input_rlc: meta.advice_column_in(SecondPhase), input_len: meta.advice_column(), output: word::Word::new([meta.advice_column(), meta.advice_column()]), - output_rlc: meta.advice_column_in(SecondPhase), } } diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 58d88d4971..0cffa92f9a 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -207,7 +207,7 @@ impl CircuitTestBuilder { let (active_gate_rows, active_lookup_rows) = EvmCircuit::::get_active_rows(&block); - let circuit = EvmCircuitCached::get_test_cicuit_from_block(block.clone()); + let circuit = EvmCircuitCached::get_test_circuit_from_block(block.clone()); let prover = MockProver::::run(k, &circuit, vec![]).unwrap(); self.evm_checks.as_ref()(prover, &active_gate_rows, &active_lookup_rows) diff --git a/zkevm-circuits/src/witness/rw.rs b/zkevm-circuits/src/witness/rw.rs index c07853ac0c..859c25e924 100644 --- a/zkevm-circuits/src/witness/rw.rs +++ b/zkevm-circuits/src/witness/rw.rs @@ -1,4 +1,4 @@ -#![allow(missing_docs)] +//! The Read-Write table related structs use std::collections::HashMap; use bus_mapping::{ @@ -152,6 +152,10 @@ impl RwMap { } } +#[allow( + missing_docs, + reason = "Some of the docs are tedious and can be found at https://github.com/privacy-scaling-explorations/zkevm-specs/blob/master/specs/tables.md" +)] /// Read-write records in execution. Rws are used for connecting evm circuit and /// state circuits. #[derive(Clone, Copy, Debug)] @@ -236,11 +240,12 @@ pub enum Rw { tx_id: usize, log_id: u64, // pack this can index together into address? field_tag: TxLogFieldTag, - // topic index (0..4) if field_tag is TxLogFieldTag:Topic - // byte index if field_tag is TxLogFieldTag:Data - // 0 for other field tags + /// index has 3 usages depends on [`crate::table::TxLogFieldTag`] + /// - topic index (0..4) if field_tag is TxLogFieldTag::Topic + /// - byte index if field_tag is TxLogFieldTag:Data + /// - 0 for other field tags index: usize, - // when it is topic field, value can be word type + /// when it is topic field, value can be word type value: Word, }, /// TxReceipt