test(mpt circuit): transfer eth to non existing address (#949)

* test mpt for non zero value to non existing addr

* Update callop.rs

* Generate mock mpt updates in test

* Add poseidon table to config for test mpt circuit

* assert HASH_SCHEME_DONE when building mock mpt proofs

* Simplify loading mpt poseidon hashes

* clippy

* remove debug prover verify

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
Co-authored-by: Mason Liang <mason@scroll.io>
Co-authored-by: z2trillion <z2trillion@users.noreply.github.com>

Author: 4 people

zkevm-circuits/src/evm_circuit/execution/callop.rs

@@ -1158,12 +1158,18 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::test_util::CircuitTestBuilder;
-    use bus_mapping::circuit_input_builder::CircuitsParams;
+    use crate::{
+        mpt_circuit::MptCircuit, test_util::CircuitTestBuilder, util::SubCircuit,
+        witness::block_convert,
+    };
+    use bus_mapping::{circuit_input_builder::CircuitsParams, mock::BlockData};
     use eth_types::{
-        address, bytecode, evm_types::OpcodeId, geth_types::Account, word, Address, ToWord, Word,
+        address, bytecode,
+        evm_types::OpcodeId,
+        geth_types::{Account, GethData},
+        word, Address, ToWord, Word,
     };
-
+    use halo2_proofs::{dev::MockProver, halo2curves::bn256::Fr};
     use itertools::Itertools;
     use mock::{
         test_ctx::helpers::{account_0_code_account_1_no_code, tx_from_1_to_0},
@@ -1561,6 +1567,43 @@ mod test {
             .run();
     }
 
+    // maybe consider to move to mpt_circuit module
+    #[cfg(feature = "scroll")]
+    #[test]
+    fn call_non_exist_with_value_mpt_circuit() {
+        let callee_code = bytecode! {
+            .op_call(0xc350, 0xff, 0x13, 0x0, 0x0, 0x0, 0x0)
+        };
+
+        let ctx = TestContext::<2, 1>::new(
+            None,
+            account_0_code_account_1_no_code(callee_code),
+            tx_from_1_to_0,
+            |block, _tx| block.number(0xcafeu64),
+        )
+        .unwrap();
+
+        let block: GethData = ctx.into();
+        let mut builder = BlockData::new_from_geth_data_with_params(
+            block.clone(),
+            CircuitsParams {
+                max_rws: 1024,
+                max_copy_rows: 1024,
+                max_mpt_rows: 3500,
+                ..Default::default()
+            },
+        )
+        .new_circuit_input_builder();
+        builder
+            .handle_block(&block.eth_block, &block.geth_traces)
+            .unwrap();
+        let mut block = block_convert::<Fr>(&builder.block, &builder.code_db).unwrap();
+        block.mpt_updates.mock_fill_state_roots();
+        let mpt_circuit = MptCircuit::new_from_block(&block);
+        let prover = MockProver::<Fr>::run(12, &mpt_circuit, vec![]).unwrap();
+        assert!(prover.verify().is_ok());
+    }
+
     // minimal testool case: returndatasize_bug_d0_g0_v0
     #[test]
     fn test_oog_call_with_inner_sstore() {

zkevm-circuits/src/mpt_circuit.rs

@@ -1,17 +1,23 @@
 #![allow(missing_docs)]
 //! wrapping of mpt-circuit
+// #[cfg(test)]
+// use crate::mpt_circuit::mpt;
 use crate::{
     table::{LookupTable, MptTable, PoseidonTable},
     util::{Challenges, SubCircuit, SubCircuitConfig},
     witness,
 };
 use eth_types::Field;
+#[cfg(test)]
+use halo2_proofs::{circuit::SimpleFloorPlanner, plonk::Circuit};
 use halo2_proofs::{
-    circuit::{Layouter, SimpleFloorPlanner, Value},
+    circuit::{Layouter, Value},
     halo2curves::bn256::Fr,
-    plonk::{Advice, Circuit, Column, ConstraintSystem, Error, Fixed},
+    plonk::{Advice, Column, ConstraintSystem, Error, Fixed},
 };
 use itertools::Itertools;
+#[cfg(test)]
+use mpt_zktrie::mpt_circuits::gadgets::mpt_update::hash_traces;
 use mpt_zktrie::mpt_circuits::{gadgets::poseidon::PoseidonLookup, mpt, types::Proof};
 
 impl PoseidonLookup for PoseidonTable {
@@ -135,9 +141,9 @@ impl SubCircuit<Fr> for MptCircuit<Fr> {
     }
 }
 
-#[cfg(any(feature = "test", test))]
+#[cfg(test)]
 impl Circuit<Fr> for MptCircuit<Fr> {
-    type Config = (MptCircuitConfig<Fr>, Challenges);
+    type Config = (MptCircuitConfig<Fr>, PoseidonTable, Challenges);
     type FloorPlanner = SimpleFloorPlanner;
 
     fn without_witnesses(&self) -> Self {
@@ -149,7 +155,7 @@ impl Circuit<Fr> for MptCircuit<Fr> {
 
     fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {
         let challenges = Challenges::construct(meta);
-        let poseidon_table = PoseidonTable::dev_construct(meta);
+        let poseidon_table = PoseidonTable::construct(meta);
         let mpt_table = MptTable::construct(meta);
 
         let config = {
@@ -163,15 +169,22 @@ impl Circuit<Fr> for MptCircuit<Fr> {
             )
         };
 
-        (config, challenges)
+        (config, poseidon_table, challenges)
     }
 
     fn synthesize(
         &self,
-        (config, challenges): Self::Config,
+        (mpt_config, poseidon_table, challenges): Self::Config,
         mut layouter: impl Layouter<Fr>,
     ) -> Result<(), Error> {
+        let poseidon_table_rows: Vec<_> = hash_traces(&self.proofs)
+            .iter()
+            .map(|([left, right], domain, hash)| {
+                [*hash, *left, *right, Fr::zero(), *domain, Fr::one()].map(Value::known)
+            })
+            .collect();
+        poseidon_table.load(&mut layouter, &poseidon_table_rows)?;
         let challenges = challenges.values(&layouter);
-        self.synthesize_sub(&config, &challenges, &mut layouter)
+        self.synthesize_sub(&mpt_config, &challenges, &mut layouter)
     }
 }

zkevm-circuits/src/table.rs

@@ -891,59 +891,25 @@ impl PoseidonTable {
         }
     }
 
-    /// Construct a new PoseidonTable for dev
-    pub(crate) fn dev_construct<F: FieldExt>(meta: &mut ConstraintSystem<F>) -> Self {
-        Self::construct(meta)
-    }
-
-    pub(crate) fn assign<F: Field>(
-        &self,
-        region: &mut Region<'_, F>,
-        offset: usize,
-        row: &[Value<F>],
-    ) -> Result<(), Error> {
-        region.assign_fixed(
-            || "assign poseidon table row value",
-            self.q_enable,
-            offset,
-            || Value::known(F::one()),
-        )?;
-        let poseidon_table_columns = <PoseidonTable as LookupTable<F>>::advice_columns(self);
-        for (column, value) in poseidon_table_columns.iter().zip_eq(row) {
-            region.assign_advice(
-                || "assign poseidon table row value",
-                *column,
-                offset,
-                || *value,
-            )?;
-        }
-        Ok(())
-    }
-
-    // Is this method used anyhwhere?
-    pub(crate) fn load<'d, F: Field>(
+    #[cfg(test)]
+    /// Load mpt hashes (without the poseidon circuit) for testing purposes.
+    pub fn load<F: Field>(
         &self,
         layouter: &mut impl Layouter<F>,
-        hashes: impl Iterator<Item = &'d [Value<F>]> + Clone,
+        hashes: &[[Value<F>; 6]],
     ) -> Result<(), Error> {
         layouter.assign_region(
             || "poseidon table",
-            |mut region| self.load_with_region(&mut region, hashes.clone()),
+            |mut region| {
+                self.assign(&mut region, 0, [Value::known(F::zero()); 6])?;
+                for (offset, row) in hashes.iter().enumerate() {
+                    self.assign(&mut region, offset + 1, *row)?;
+                }
+                Ok(())
+            },
         )
     }
 
-    pub(crate) fn load_with_region<'d, F: Field>(
-        &self,
-        region: &mut Region<'_, F>,
-        hashes: impl Iterator<Item = &'d [Value<F>]>,
-    ) -> Result<(), Error> {
-        self.assign(region, 0, [Value::known(F::zero()); 6].as_slice())?;
-        for (offset, row) in hashes.enumerate() {
-            self.assign(region, offset + 1, row)?;
-        }
-        Ok(())
-    }
-
     /// Provide this function for the case that we want to consume a poseidon
     /// table but without running the full poseidon circuit
     pub fn dev_load<'a, F: Field>(
@@ -1064,6 +1030,30 @@ impl PoseidonTable {
             },
         )
     }
+
+    fn assign<F: Field>(
+        &self,
+        region: &mut Region<'_, F>,
+        offset: usize,
+        row: [Value<F>; 6],
+    ) -> Result<(), Error> {
+        region.assign_fixed(
+            || "assign poseidon table row value",
+            self.q_enable,
+            offset,
+            || Value::known(F::one()),
+        )?;
+        let poseidon_table_columns = <PoseidonTable as LookupTable<F>>::advice_columns(self);
+        for (column, value) in poseidon_table_columns.iter().zip_eq(row) {
+            region.assign_advice(
+                || "assign poseidon table row value",
+                *column,
+                offset,
+                || value,
+            )?;
+        }
+        Ok(())
+    }
 }
 
 /// Tag to identify the field in a Bytecode Table row

zkevm-circuits/src/witness/mpt.rs

@@ -5,11 +5,14 @@ use crate::{
 use eth_types::{Address, Field, ToLittleEndian, ToScalar, Word, U256};
 use halo2_proofs::circuit::Value;
 use itertools::Itertools;
+#[cfg(test)]
+use mpt_zktrie::state::builder::HASH_SCHEME_DONE;
 use mpt_zktrie::{
     mpt_circuits::{serde::SMTTrace, MPTProofType},
     state,
     state::witness::WitnessGenerator,
 };
+
 use serde::{Deserialize, Serialize};
 use std::collections::BTreeMap;
 
@@ -78,9 +81,10 @@ impl MptUpdates {
         })
     }
 
-    pub(crate) fn mock_fill_state_roots(&mut self) {
-        // initialize a mock witness generator that is consistent with the old values of
-        // self.updates
+    #[cfg(test)]
+    /// initialize a mock witness generator that is consistent with the old values of self.updates
+    pub fn mock_fill_state_roots(&mut self) {
+        assert!(*HASH_SCHEME_DONE);
         let mut wit_gen = WitnessGenerator::from(&ZktrieState::default());
         for (key, update) in &mut self.updates {
             let key = key.set_non_exists(Word::zero(), update.old_value);