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 00cd3e0. * try align with develop * enable vk check * Update test.rs --------- Co-authored-by: Rohit Narurkar <[email protected]>
scroll-tech · Jul 9, 2024 · ede39de · ede39de
1 parent 84b9bb5
commit ede39de
Show file tree

Hide file tree

Showing 7 changed files with 112 additions and 59 deletions.
diff --git 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<Block> {
     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
@@ -285,7 +285,7 @@ impl<F: Field> EvmCircuit<F> {
     }
 }
 
-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<F: Field> SubCircuit<F> for EvmCircuit<F> {

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
@@ -41,6 +41,8 @@ pub(crate) struct ErrorOOGMemoryCopyGadget<F> {
     external_address: Word<F>,
 
     addr_expansion_gadget: MemoryAddrExpandGadget<F>,
+    // mcopy expansion
+    memory_expansion_mcopy: MemoryExpansionGadget<F, 2, N_BYTES_MEMORY_WORD_SIZE>,
     // other kind(CALLDATACOPY, CODECOPY, EXTCODECOPY, RETURNDATACOPY) expansion
     memory_expansion_normal: MemoryExpansionGadget<F, 1, N_BYTES_MEMORY_WORD_SIZE>,
     memory_copier_gas: MemoryCopierGasGadget<F, { GasCost::COPY }>,
@@ -91,12 +93,16 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGMemoryCopyGadget<F> {
             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<F: Field> ExecutionGadget<F> for ErrorOOGMemoryCopyGadget<F> {
 
         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<F: Field> ExecutionGadget<F> for ErrorOOGMemoryCopyGadget<F> {
             tx_id,
             external_address,
             addr_expansion_gadget,
+            memory_expansion_mcopy,
             memory_expansion_normal,
             memory_copier_gas,
             insufficient_gas,
@@ -227,13 +234,12 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGMemoryCopyGadget<F> {
         )?;
 
         // 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<F> {
     src_memory_addr: MemoryExpandedAddressGadget<F>,
     /// Destination offset and size to copy
     dst_memory_addr: MemoryExpandedAddressGadget<F>,
-    // mcopy expansion
-    memory_expansion_mcopy: MemoryExpansionGadget<F, 2, N_BYTES_MEMORY_WORD_SIZE>,
 }
 
 // construct src_memory_addr, dst_memory_addr and memory_expansion_mcopy.
 impl<F: Field> MemoryAddrExpandGadget<F> {
-    fn construct(cb: &mut EVMConstraintBuilder<F>, is_mcopy: Expression<F>) -> Self {
+    fn construct(cb: &mut EVMConstraintBuilder<F>) -> 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<F>,
+        is_mcopy: Expression<F>,
+    ) -> MemoryExpansionGadget<F, 2, N_BYTES_MEMORY_WORD_SIZE> {
+        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,16 +718,21 @@ mod tests {
     #[derive(Clone)]
     struct ErrOOGMemoryCopyGadgetTestContainer<F> {
         gadget: MemoryAddrExpandGadget<F>,
+        memory_expansion_mcopy: MemoryExpansionGadget<F, 2, N_BYTES_MEMORY_WORD_SIZE>,
         is_mcopy: Cell<F>,
     }
 
     impl<F: Field> MathGadgetContainer<F> for ErrOOGMemoryCopyGadgetTestContainer<F> {
         fn configure_gadget_container(cb: &mut EVMConstraintBuilder<F>) -> Self {
             let is_mcopy = cb.query_cell();
             cb.require_boolean("is_mcopy is bool", is_mcopy.expr());
-            let gadget = MemoryAddrExpandGadget::<F>::construct(cb, is_mcopy.expr());
-
-            ErrOOGMemoryCopyGadgetTestContainer { gadget, is_mcopy }
+            let gadget = MemoryAddrExpandGadget::<F>::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
@@ -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
@@ -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::<Bn256>::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(&params, &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
@@ -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
@@ -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")
+}