From ede39de59ce390ca6d3b1f0568e8661e4868f4fc Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Wed, 10 Jul 2024 07:32:45 +0800 Subject: [PATCH] revert chunk circuit vk to mainnet version (#1361) * add trace jsons to testdata * update mainnet fork height; remove version field in trace * v0.11.2 * fix l1 msg + 1559 * pick some changes from develop * env var SCROLL_MAINNET_CURIE_BLOCK * fix: table kind should not change over the fse code trailing bits * #[serde(default)] for prestate * update mainnet curie block to 7096836 * update l2geth * add vk checking test * add vk checking test * exclude Bernoulli from FixedTable * Revert "add error mcopy test (#1332)" This reverts commit 00cd3e0090a95772b2951ed4de0876f6b37955c2. * try align with develop * enable vk check * Update test.rs --------- Co-authored-by: Rohit Narurkar --- prover/src/zkevm/circuit/builder.rs | 14 +--- zkevm-circuits/src/evm_circuit.rs | 2 +- .../execution/error_oog_memory_copy.rs | 76 +++++++++++-------- zkevm-circuits/src/evm_circuit/table.rs | 27 ++++--- zkevm-circuits/src/super_circuit/test.rs | 30 +++++++- zkevm-circuits/src/witness.rs | 2 +- zkevm-circuits/src/witness/block.rs | 20 ++++- 7 files changed, 112 insertions(+), 59 deletions(-) diff --git a/prover/src/zkevm/circuit/builder.rs b/prover/src/zkevm/circuit/builder.rs index 9d662a6d4c..8f96a7c59a 100644 --- a/prover/src/zkevm/circuit/builder.rs +++ b/prover/src/zkevm/circuit/builder.rs @@ -1,11 +1,7 @@ use crate::{utils::read_env_var, zkevm::SubCircuitRowUsage}; use anyhow::{bail, Result}; -use bus_mapping::circuit_input_builder::{self, CircuitInputBuilder}; -use eth_types::{ - l2_types::BlockTrace, - state_db::{CodeDB, StateDB}, - ToWord, -}; +use bus_mapping::circuit_input_builder::CircuitInputBuilder; +use eth_types::{l2_types::BlockTrace, ToWord}; use itertools::Itertools; use mpt_zktrie::state::ZkTrieHash; use std::sync::LazyLock; @@ -104,11 +100,7 @@ pub fn validite_block_traces(block_traces: &[BlockTrace]) -> Result<()> { pub fn dummy_witness_block() -> Result { log::debug!("generate dummy witness block"); - let builder_block = circuit_input_builder::Blocks::init(*CHAIN_ID, get_super_circuit_params()); - let mut builder: CircuitInputBuilder = - CircuitInputBuilder::new(StateDB::new(), CodeDB::new(), &builder_block); - builder.finalize_building()?; - let witness_block = block_convert(&builder.block, &builder.code_db)?; + let witness_block = zkevm_circuits::witness::dummy_witness_block(*CHAIN_ID); log::debug!("generate dummy witness block done"); Ok(witness_block) } diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index e133f3d930..0a6c6ce1fa 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -285,7 +285,7 @@ impl EvmCircuit { } } -const FIXED_TABLE_ROWS_NO_BITWISE: usize = 3662; +const FIXED_TABLE_ROWS_NO_BITWISE: usize = 3659; 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/error_oog_memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs index 2fb8e12aec..0e0c1d186b 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs @@ -41,6 +41,8 @@ pub(crate) struct ErrorOOGMemoryCopyGadget { external_address: Word, addr_expansion_gadget: MemoryAddrExpandGadget, + // mcopy expansion + memory_expansion_mcopy: MemoryExpansionGadget, // other kind(CALLDATACOPY, CODECOPY, EXTCODECOPY, RETURNDATACOPY) expansion memory_expansion_normal: MemoryExpansionGadget, memory_copier_gas: MemoryCopierGasGadget, @@ -91,12 +93,16 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { cb.stack_pop(external_address.expr()); }); - let addr_expansion_gadget = MemoryAddrExpandGadget::construct(cb, is_mcopy.expr()); + let addr_expansion_gadget = MemoryAddrExpandGadget::construct(cb); cb.stack_pop(addr_expansion_gadget.dst_memory_addr.offset_rlc()); cb.stack_pop(addr_expansion_gadget.src_memory_addr.offset_rlc()); cb.stack_pop(addr_expansion_gadget.dst_memory_addr.length_rlc()); + // for mcopy + let memory_expansion_mcopy = + addr_expansion_gadget.build_memory_expansion_mcopy(cb, is_mcopy.expr()); + // for others (CALLDATACOPY, CODECOPY, EXTCODECOPY, RETURNDATACOPY) let memory_expansion_normal = cb.condition(not::expr(is_mcopy.expr()), |cb| { MemoryExpansionGadget::construct( @@ -107,7 +113,7 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { let memory_expansion_cost = select::expr( is_mcopy.expr(), - addr_expansion_gadget.memory_expansion_mcopy.gas_cost(), + memory_expansion_mcopy.gas_cost(), memory_expansion_normal.gas_cost(), ); let memory_copier_gas = MemoryCopierGasGadget::construct( @@ -160,6 +166,7 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { tx_id, external_address, addr_expansion_gadget, + memory_expansion_mcopy, memory_expansion_normal, memory_copier_gas, insufficient_gas, @@ -227,13 +234,12 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { )?; // assign memory_expansion_mcopy - let (_, memory_expansion_cost_mcopy) = - self.addr_expansion_gadget.memory_expansion_mcopy.assign( - region, - offset, - step.memory_word_size(), - [src_memory_addr, dst_memory_addr], - )?; + let (_, memory_expansion_cost_mcopy) = self.memory_expansion_mcopy.assign( + region, + offset, + step.memory_word_size(), + [src_memory_addr, dst_memory_addr], + )?; let memory_copier_gas = self.memory_copier_gas.assign( region, @@ -291,33 +297,38 @@ pub(crate) struct MemoryAddrExpandGadget { src_memory_addr: MemoryExpandedAddressGadget, /// Destination offset and size to copy dst_memory_addr: MemoryExpandedAddressGadget, - // mcopy expansion - memory_expansion_mcopy: MemoryExpansionGadget, } // construct src_memory_addr, dst_memory_addr and memory_expansion_mcopy. impl MemoryAddrExpandGadget { - fn construct(cb: &mut EVMConstraintBuilder, is_mcopy: Expression) -> Self { + fn construct(cb: &mut EVMConstraintBuilder) -> Self { let dst_memory_addr = MemoryExpandedAddressGadget::construct_self(cb); // src can also be possible to overflow for mcopy. let src_memory_addr = MemoryExpandedAddressGadget::construct_self(cb); - // for mcopy - let memory_expansion_mcopy = cb.condition(is_mcopy.expr(), |cb| { + Self { + src_memory_addr, + dst_memory_addr, + } + } + fn build_memory_expansion_mcopy( + &self, + cb: &mut EVMConstraintBuilder, + is_mcopy: Expression, + ) -> MemoryExpansionGadget { + cb.condition(is_mcopy.expr(), |cb| { cb.require_equal( "mcopy src_address length == dst_address length", - src_memory_addr.length_rlc(), - dst_memory_addr.length_rlc(), + self.src_memory_addr.length_rlc(), + self.dst_memory_addr.length_rlc(), ); MemoryExpansionGadget::construct( cb, - [src_memory_addr.end_offset(), dst_memory_addr.end_offset()], + [ + self.src_memory_addr.end_offset(), + self.dst_memory_addr.end_offset(), + ], ) - }); - Self { - src_memory_addr, - dst_memory_addr, - memory_expansion_mcopy, - } + }) } } @@ -707,6 +718,7 @@ mod tests { #[derive(Clone)] struct ErrOOGMemoryCopyGadgetTestContainer { gadget: MemoryAddrExpandGadget, + memory_expansion_mcopy: MemoryExpansionGadget, is_mcopy: Cell, } @@ -714,9 +726,13 @@ mod tests { fn configure_gadget_container(cb: &mut EVMConstraintBuilder) -> Self { let is_mcopy = cb.query_cell(); cb.require_boolean("is_mcopy is bool", is_mcopy.expr()); - let gadget = MemoryAddrExpandGadget::::construct(cb, is_mcopy.expr()); - - ErrOOGMemoryCopyGadgetTestContainer { gadget, is_mcopy } + let gadget = MemoryAddrExpandGadget::::construct(cb); + let memory_expansion_mcopy = gadget.build_memory_expansion_mcopy(cb, is_mcopy.expr()); + ErrOOGMemoryCopyGadgetTestContainer { + gadget, + memory_expansion_mcopy, + is_mcopy, + } } fn assign_gadget_container( @@ -743,12 +759,8 @@ mod tests { copy_size.into(), )?; // assign memory_expansion_mcopy - self.gadget.memory_expansion_mcopy.assign( - region, - 0, - 0, - [src_memory_addr, dst_memory_addr], - )?; + self.memory_expansion_mcopy + .assign(region, 0, 0, [src_memory_addr, dst_memory_addr])?; Ok(()) } } diff --git a/zkevm-circuits/src/evm_circuit/table.rs b/zkevm-circuits/src/evm_circuit/table.rs index ef42f22f1d..de5ab7a00b 100644 --- a/zkevm-circuits/src/evm_circuit/table.rs +++ b/zkevm-circuits/src/evm_circuit/table.rs @@ -5,6 +5,7 @@ use crate::{ util::Field, }; use bus_mapping::{evm::OpcodeId, precompile::PrecompileCalls}; +use eth_types::forks::HardforkId; use gadgets::util::Expr; use halo2_proofs::plonk::Expression; use strum::IntoEnumIterator; @@ -142,16 +143,22 @@ impl FixedTableTag { F::from(precompile.base_gas_cost().0), ] })), - Self::ChainFork => Box::new(eth_types::forks::hardfork_heights().into_iter().map( - move |(fork, chain_id, height)| { - [ - tag, - F::from(fork as u64), - F::from(chain_id), - F::from(height), - ] - }, - )), + Self::ChainFork => Box::new( + eth_types::forks::hardfork_heights() + .into_iter() + .filter(move |(f, _, _)| { + // other fork info is not needed in circuit now. + *f == HardforkId::Curie + }) + .map(move |(fork, chain_id, height)| { + [ + tag, + F::from(fork as u64), + F::from(chain_id), + F::from(height), + ] + }), + ), } } } diff --git a/zkevm-circuits/src/super_circuit/test.rs b/zkevm-circuits/src/super_circuit/test.rs index ae75012dcc..b670adfd74 100644 --- a/zkevm-circuits/src/super_circuit/test.rs +++ b/zkevm-circuits/src/super_circuit/test.rs @@ -1,4 +1,6 @@ #![allow(unused_imports)] +use crate::witness::dummy_witness_block; + pub use super::*; use bus_mapping::{ circuit_input_builder::CircuitInputBuilder, @@ -7,13 +9,18 @@ use bus_mapping::{ precompile::PrecompileCalls, }; use ethers_signers::{LocalWallet, Signer}; -use halo2_proofs::{dev::MockProver, halo2curves::bn256::Fr}; +use halo2_proofs::{ + dev::MockProver, + halo2curves::bn256::{Bn256, Fr}, + plonk::keygen_vk, +}; use log::error; #[cfg(not(feature = "scroll"))] use mock::MOCK_DIFFICULTY; #[cfg(feature = "scroll")] use mock::MOCK_DIFFICULTY_L2GETH as MOCK_DIFFICULTY; use mock::{eth, TestContext, MOCK_CHAIN_ID}; +use params::ScrollSuperCircuit; use rand::SeedableRng; use rand_chacha::ChaCha20Rng; use std::env::set_var; @@ -56,6 +63,27 @@ fn super_circuit_degree() { assert!(cs.degree() <= 9); } +// This circuit is used to prevent unexpected changes in circuit vk. +// This test can run successfully now standalone `RUST_LOG=info cargo test --release --features=scroll super_circuit_vk -- --ignored` +// but will fail in CI. I don't understand, may due to env var like COINBASE/DIFFICULT/KECCAK_ROWS? +// So have to ignore it now. +#[ignore = "enable this when we want to prevent unexpected changes in circuit"] +#[test] +fn super_circuit_vk() { + use halo2_proofs::poly::kzg::commitment::ParamsKZG; + let params = ParamsKZG::::unsafe_setup_with_s(20, Fr::from(1234u64)); + // chain_id is not related to vk, so we can use any value here. + let chain_id = 534351; + let circuit = ScrollSuperCircuit::new_from_block(&dummy_witness_block(chain_id)); + let vk = keygen_vk(¶ms, &circuit).unwrap(); + let protocol_hash = vk.transcript_repr(); + log::info!("transcript_repr {:?}", protocol_hash); + assert_eq!( + "0x1b3d158be8148c9e8ac9fce6eff2c576027c356ee1ff68ad7662d61556d5a7d7", + format!("{:?}", protocol_hash) + ); +} + #[cfg(feature = "scroll")] fn test_super_circuit< const MAX_TXS: usize, diff --git a/zkevm-circuits/src/witness.rs b/zkevm-circuits/src/witness.rs index dec46a2f9e..fcb32dcfa2 100644 --- a/zkevm-circuits/src/witness.rs +++ b/zkevm-circuits/src/witness.rs @@ -3,7 +3,7 @@ //! used to generate witnesses for circuits. mod block; -pub use block::{block_convert, Block, BlockContext, BlockContexts}; +pub use block::{block_convert, dummy_witness_block, Block, BlockContext, BlockContexts}; /// Keccak witness pub mod keccak; diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index 559084b7d4..a2e2150a93 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -7,18 +7,23 @@ use crate::evm_circuit::{detect_fixed_table_tags, EvmCircuit}; use crate::{ evm_circuit::util::rlc, + super_circuit::params::get_super_circuit_params, table::{BlockContextFieldTag, RwTableTag}, util::{Field, SubCircuit}, witness::keccak::keccak_inputs, }; use bus_mapping::{ circuit_input_builder::{ - self, BigModExp, CircuitsParams, CopyEvent, EcAddOp, EcMulOp, EcPairingOp, ExpEvent, - PrecompileEvents, SHA256, + self, BigModExp, CircuitInputBuilder, CircuitsParams, CopyEvent, EcAddOp, EcMulOp, + EcPairingOp, ExpEvent, PrecompileEvents, SHA256, }, Error, }; -use eth_types::{sign_types::SignData, Address, ToLittleEndian, Word, H256, U256}; +use eth_types::{ + sign_types::SignData, + state_db::{CodeDB, StateDB}, + Address, ToLittleEndian, Word, H256, U256, +}; use halo2_proofs::{circuit::Value, halo2curves::bn256::Fr}; use itertools::Itertools; @@ -611,3 +616,12 @@ pub fn block_convert( }; Ok(block) } + +/// Generate a empty witness block, which can be used for key-gen. +pub fn dummy_witness_block(chain_id: u64) -> Block { + let builder_block = circuit_input_builder::Blocks::init(chain_id, get_super_circuit_params()); + let mut builder: CircuitInputBuilder = + CircuitInputBuilder::new(StateDB::new(), CodeDB::new(), &builder_block); + builder.finalize_building().expect("should not fail"); + block_convert(&builder.block, &builder.code_db).expect("should not fail") +}