Skip to content
This repository was archived by the owner on Jul 5, 2024. It is now read-only.

Commit 2723775

Browse files
authored
fix nondeterministic constraint generation (#1706)
### Description Change HashMap to BTreeMap in memory.rs and in cell_placement_strategy. ### Issue Link #1700 ### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Contents - fix in memory.rs - fix in cell_placement_strategy ### Rationale Generates different circuit keys making proof verification fail for different witnesses. ### How Has This Been Tested? Manual testing.
1 parent 26cd21b commit 2723775

File tree

3 files changed

+51
-26
lines changed

3 files changed

+51
-26
lines changed

zkevm-circuits/src/circuit_tools/memory.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use halo2_proofs::{
77
poly::Rotation,
88
};
99
use std::{
10-
collections::HashMap,
10+
collections::BTreeMap,
1111
marker::PhantomData,
1212
ops::{Index, IndexMut},
1313
};
@@ -20,7 +20,7 @@ use super::{
2020

2121
#[derive(Clone, Debug, Default)]
2222
pub(crate) struct Memory<F: Field, C: CellType, MB: MemoryBank<F, C>> {
23-
banks: HashMap<C, MB>,
23+
banks: BTreeMap<C, MB>,
2424
_phantom: PhantomData<F>,
2525
tag_counter: usize,
2626
}
@@ -42,7 +42,7 @@ impl<F: Field, C: CellType, MB: MemoryBank<F, C>> IndexMut<C> for Memory<F, C, M
4242
impl<F: Field, C: CellType, MB: MemoryBank<F, C>> Memory<F, C, MB> {
4343
pub(crate) fn new() -> Self {
4444
Self {
45-
banks: HashMap::new(),
45+
banks: BTreeMap::new(),
4646
_phantom: PhantomData,
4747
tag_counter: 0,
4848
}

zkevm-circuits/src/mpt_circuit.rs

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -766,10 +766,38 @@ pub fn load_proof_from_file(path: &str) -> Vec<Node> {
766766
mod tests {
767767
use super::*;
768768
use halo2_proofs::{dev::MockProver, halo2curves::bn256::Fr};
769-
use std::{fs, ops::Deref};
769+
use itertools::Itertools;
770+
use std::{fs, ops::Deref, path::PathBuf};
770771

771772
#[test]
772773
fn test_mpt() {
774+
let degree = 15;
775+
get_witnesses()
776+
.enumerate()
777+
.for_each(|(idx, (path, num_rows, circuit))| {
778+
println!("{} {:?}", idx, path);
779+
let prover = MockProver::<Fr>::run(degree, &circuit, vec![]).unwrap();
780+
assert_eq!(prover.verify_at_rows(0..num_rows, 0..num_rows,), Ok(()));
781+
// assert_eq!(prover.verify_par(), Ok(()));
782+
// prover.assert_satisfied();
783+
});
784+
}
785+
786+
#[test]
787+
fn variadic_size_check() {
788+
let mut circuits = get_witnesses();
789+
let first = circuits.next().unwrap().2;
790+
let second = circuits.next().unwrap().2;
791+
792+
let degree = 15;
793+
let prover_1 = MockProver::<Fr>::run(degree, &first, vec![]).unwrap();
794+
let prover_2 = MockProver::<Fr>::run(degree, &second, vec![]).unwrap();
795+
796+
assert_eq!(prover_1.fixed(), prover_2.fixed());
797+
assert_eq!(prover_1.permutation(), prover_2.permutation());
798+
}
799+
800+
fn get_witnesses() -> impl Iterator<Item = (PathBuf, usize, MPTCircuit<Fr>)> {
773801
let path = "src/mpt_circuit/tests";
774802
let files = fs::read_dir(path).unwrap();
775803
files
@@ -781,8 +809,8 @@ mod tests {
781809
false
782810
}
783811
})
784-
.enumerate()
785-
.for_each(|(idx, f)| {
812+
.sorted_by(|a, b| a.file_name().cmp(&b.file_name()))
813+
.map(|f| {
786814
let path = f.path();
787815
let mut parts = path.to_str().unwrap().split('-');
788816
parts.next();
@@ -796,24 +824,21 @@ mod tests {
796824
keccak_data.push(k.deref().clone());
797825
}
798826
}
799-
800827
let disable_preimage_check = nodes[0].start.clone().unwrap().disable_preimage_check;
801828
let degree = 15;
802829
let max_nodes = 520;
803-
let circuit = MPTCircuit::<Fr> {
804-
nodes,
805-
keccak_data,
806-
degree,
807-
max_nodes,
808-
disable_preimage_check,
809-
_marker: PhantomData,
810-
};
811-
812-
println!("{} {:?}", idx, path);
813-
let prover = MockProver::<Fr>::run(degree as u32, &circuit, vec![]).unwrap();
814-
assert_eq!(prover.verify_at_rows(0..num_rows, 0..num_rows,), Ok(()));
815-
// assert_eq!(prover.verify(), Ok(()));
816-
// prover.assert_satisfied();
817-
});
830+
(
831+
path,
832+
num_rows,
833+
MPTCircuit::<Fr> {
834+
nodes,
835+
keccak_data,
836+
degree,
837+
max_nodes,
838+
disable_preimage_check,
839+
_marker: PhantomData,
840+
},
841+
)
842+
})
818843
}
819844
}

zkevm-circuits/src/util/cell_placement_strategy.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{BTreeMap, HashMap};
1+
use std::collections::BTreeMap;
22

33
use eth_types::Field;
44
use halo2_proofs::plonk::{Advice, Column, ConstraintSystem};
@@ -8,7 +8,7 @@ use super::cell_manager::{
88
};
99

1010
#[derive(Clone, Debug, Default)]
11-
pub(crate) struct CMFixedWidthStrategyDistribution(HashMap<CellType, Vec<Column<Advice>>>);
11+
pub(crate) struct CMFixedWidthStrategyDistribution(BTreeMap<CellType, Vec<Column<Advice>>>);
1212

1313
impl CMFixedWidthStrategyDistribution {
1414
pub(crate) fn add(&mut self, cell_type: CellType, advice: Column<Advice>) {
@@ -34,7 +34,7 @@ pub(crate) struct CMFixedWidthStrategy {
3434
advices: CMFixedWidthStrategyDistribution,
3535
height_offset: usize,
3636

37-
next: HashMap<CellType, (usize, usize)>,
37+
next: BTreeMap<CellType, (usize, usize)>,
3838

3939
perm_substitution: bool,
4040
max_height: usize,
@@ -52,7 +52,7 @@ impl CMFixedWidthStrategy {
5252
CMFixedWidthStrategy {
5353
advices,
5454
height_offset,
55-
next: HashMap::default(),
55+
next: BTreeMap::default(),
5656
perm_substitution: false,
5757
max_height: usize::max_value(),
5858
}

0 commit comments

Comments
 (0)