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

Commit 0ab41c9

Browse files
authored
[proof chunk] [WIP] [testing] fix various issue found in integration test (#1773)
### Content Reported issues found on multi-chunk testing - [x] refactor 1: simply chunking boundary judgement logic in bus-mapping to ready for finish other incompleted features + avoid tech dept in the future - [x] add uncompleted logic in bus-mapping: chronological and by-address rwtable not propagate pre-chunk last rw correctly. - [x] edge case: deal with dummy chunk for real chunk less than desired chunk in circuit params - [x] allow zero limb diff in state_circuit lexicoordering => we allow duplicated `rw_counter` in `padding`, and rely on permutation constraints on by-address/chronological rw_table to avoid malicious padding insert. - [x] super_circuit/root_circuit tests adopt multiple chunk ### Related Issue To close #1778
1 parent 64154f7 commit 0ab41c9

File tree

36 files changed

+1569
-797
lines changed

36 files changed

+1569
-797
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bus-mapping/src/circuit_input_builder.rs

Lines changed: 223 additions & 148 deletions
Large diffs are not rendered by default.

bus-mapping/src/circuit_input_builder/chunk.rs

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub struct ChunkContext {
2525
/// Index of current chunk, start from 0
2626
pub idx: usize,
2727
/// Used to track the inner chunk counter in every operation in the chunk.
28+
/// it will be reset for every new chunk.
2829
/// Contains the next available value.
2930
pub rwc: RWCounter,
3031
/// Number of chunks
@@ -33,16 +34,14 @@ pub struct ChunkContext {
3334
pub initial_rwc: usize,
3435
/// End global rw counter
3536
pub end_rwc: usize,
37+
/// tx range in block: [initial_tx_index, end_tx_index)
38+
pub initial_tx_index: usize,
3639
///
37-
pub initial_tx: usize,
40+
pub end_tx_index: usize,
41+
/// copy range in block: [initial_copy_index, end_copy_index)
42+
pub initial_copy_index: usize,
3843
///
39-
pub end_tx: usize,
40-
///
41-
pub initial_copy: usize,
42-
///
43-
pub end_copy: usize,
44-
/// Druing dry run, chuncking is desabled
45-
pub enable: bool,
44+
pub end_copy_index: usize,
4645
}
4746

4847
impl Default for ChunkContext {
@@ -60,11 +59,10 @@ impl ChunkContext {
6059
total_chunks,
6160
initial_rwc: 1, // rw counter start from 1
6261
end_rwc: 0, // end_rwc should be set in later phase
63-
initial_tx: 1,
64-
end_tx: 0,
65-
initial_copy: 0,
66-
end_copy: 0,
67-
enable: true,
62+
initial_tx_index: 0,
63+
end_tx_index: 0,
64+
initial_copy_index: 0,
65+
end_copy_index: 0,
6866
}
6967
}
7068

@@ -76,11 +74,10 @@ impl ChunkContext {
7674
total_chunks: 1,
7775
initial_rwc: 1, // rw counter start from 1
7876
end_rwc: 0, // end_rwc should be set in later phase
79-
initial_tx: 1,
80-
end_tx: 0,
81-
initial_copy: 0,
82-
end_copy: 0,
83-
enable: true,
77+
initial_tx_index: 0,
78+
end_tx_index: 0,
79+
initial_copy_index: 0,
80+
end_copy_index: 0,
8481
}
8582
}
8683

@@ -91,11 +88,11 @@ impl ChunkContext {
9188
self.idx += 1;
9289
self.rwc = RWCounter::new();
9390
self.initial_rwc = initial_rwc;
94-
self.initial_tx = initial_tx;
95-
self.initial_copy = initial_copy;
91+
self.initial_tx_index = initial_tx;
92+
self.initial_copy_index = initial_copy;
9693
self.end_rwc = 0;
97-
self.end_tx = 0;
98-
self.end_copy = 0;
94+
self.end_tx_index = 0;
95+
self.end_copy_index = 0;
9996
}
10097

10198
/// Is first chunk

bus-mapping/src/circuit_input_builder/input_state_ref.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ impl<'a> CircuitInputStateRef<'a> {
6767
exec_state: ExecState::InvalidTx,
6868
gas_left: self.tx.gas(),
6969
rwc: self.block_ctx.rwc,
70+
rwc_inner_chunk: self.chunk_ctx.rwc,
7071
..Default::default()
7172
}
7273
}
@@ -148,9 +149,13 @@ impl<'a> CircuitInputStateRef<'a> {
148149
/// Check whether rws will overflow circuit limit.
149150
pub fn check_rw_num_limit(&self) -> Result<(), Error> {
150151
if let Some(max_rws) = self.max_rws {
151-
let rwc = self.block_ctx.rwc.0;
152+
let rwc = self.chunk_ctx.rwc.0;
152153
if rwc > max_rws {
153-
log::error!("rwc > max_rws, rwc={}, max_rws={}", rwc, max_rws);
154+
log::error!(
155+
"chunk inner rwc > max_rws, rwc={}, max_rws={}",
156+
rwc,
157+
max_rws
158+
);
154159
return Err(Error::RwsNotEnough(max_rws, rwc));
155160
};
156161
}

circuit-benchmarks/src/copy_circuit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ mod tests {
156156
.handle_block(&block.eth_block, &block.geth_traces)
157157
.unwrap();
158158
let block = block_convert(&builder).unwrap();
159-
let chunk = chunk_convert(&builder, 0).unwrap();
159+
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
160160
assert_eq!(block.copy_events.len(), copy_event_num);
161161
(block, chunk)
162162
}

circuit-benchmarks/src/evm_circuit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ mod evm_circ_benches {
5454
.unwrap();
5555

5656
let block = block_convert(&builder).unwrap();
57-
let chunk = chunk_convert(&builder, 0).unwrap();
57+
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
5858

5959
let circuit = TestEvmCircuit::<Fr>::new(block, chunk);
6060
let mut rng = XorShiftRng::from_seed([

circuit-benchmarks/src/exp_circuit.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,8 @@ mod tests {
149149
.new_circuit_input_builder()
150150
.handle_block(&block.eth_block, &block.geth_traces)
151151
.unwrap();
152-
(
153-
block_convert(&builder).unwrap(),
154-
chunk_convert(&builder, 0).unwrap(),
155-
)
152+
let block = block_convert(&builder).unwrap();
153+
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
154+
(block, chunk)
156155
}
157156
}

circuit-benchmarks/src/super_circuit.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,9 @@ mod tests {
9191
max_evm_rows: 0,
9292
max_keccak_rows: 0,
9393
};
94-
let (_, circuit, instance, _) =
94+
let (_, mut circuits, mut instances, _) =
9595
SuperCircuit::build(block, circuits_params, Fr::from(0x100)).unwrap();
96+
let (circuit, instance) = (circuits.remove(0), instances.remove(0));
9697
let instance_refs: Vec<&[Fr]> = instance.iter().map(|v| &v[..]).collect();
9798

9899
// Bench setup generation

integration-tests/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ rand_chacha = "0.3"
2424
paste = "1.0"
2525
rand_xorshift = "0.3.0"
2626
rand_core = "0.6.4"
27+
itertools = "0.10"
2728
mock = { path = "../mock" }
2829

2930
[dev-dependencies]

integration-tests/src/integration_test_circuits.rs

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use halo2_proofs::{
2525
},
2626
},
2727
};
28+
use itertools::Itertools;
2829
use lazy_static::lazy_static;
2930
use mock::TestContext;
3031
use rand_chacha::rand_core::SeedableRng;
@@ -358,10 +359,26 @@ impl<C: SubCircuit<Fr> + Circuit<Fr>> IntegrationTest<C> {
358359
let fixed = mock_prover.fixed();
359360

360361
if let Some(prev_fixed) = self.fixed.clone() {
361-
assert!(
362-
fixed.eq(&prev_fixed),
363-
"circuit fixed columns are not constant for different witnesses"
364-
);
362+
fixed
363+
.iter()
364+
.enumerate()
365+
.zip_eq(prev_fixed.iter())
366+
.for_each(|((index, col1), col2)| {
367+
if !col1.eq(col2) {
368+
println!("on column index {} not equal", index);
369+
col1.iter().enumerate().zip_eq(col2.iter()).for_each(
370+
|((index, cellv1), cellv2)| {
371+
assert!(
372+
cellv1.eq(cellv2),
373+
"cellv1 {:?} != cellv2 {:?} on index {}",
374+
cellv1,
375+
cellv2,
376+
index
377+
);
378+
},
379+
);
380+
}
381+
});
365382
} else {
366383
self.fixed = Some(fixed.clone());
367384
}
@@ -383,6 +400,24 @@ impl<C: SubCircuit<Fr> + Circuit<Fr>> IntegrationTest<C> {
383400

384401
match self.root_fixed.clone() {
385402
Some(prev_fixed) => {
403+
fixed.iter().enumerate().zip_eq(prev_fixed.iter()).for_each(
404+
|((index, col1), col2)| {
405+
if !col1.eq(col2) {
406+
println!("on column index {} not equal", index);
407+
col1.iter().enumerate().zip_eq(col2.iter()).for_each(
408+
|((index, cellv1), cellv2)| {
409+
assert!(
410+
cellv1.eq(cellv2),
411+
"cellv1 {:?} != cellv2 {:?} on index {}",
412+
cellv1,
413+
cellv2,
414+
index
415+
);
416+
},
417+
);
418+
}
419+
},
420+
);
386421
assert!(
387422
fixed.eq(&prev_fixed),
388423
"root circuit fixed columns are not constant for different witnesses"
@@ -424,7 +459,7 @@ impl<C: SubCircuit<Fr> + Circuit<Fr>> IntegrationTest<C> {
424459
block_tag,
425460
);
426461
let mut block = block_convert(&builder).unwrap();
427-
let chunk = chunk_convert(&builder, 0).unwrap();
462+
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
428463
block.randomness = Fr::from(TEST_MOCK_RANDOMNESS);
429464
let circuit = C::new_from_block(&block, &chunk);
430465
let instance = circuit.instance();
@@ -441,7 +476,7 @@ impl<C: SubCircuit<Fr> + Circuit<Fr>> IntegrationTest<C> {
441476
);
442477

443478
// get chronological_rwtable and byaddr_rwtable columns index
444-
let mut cs = ConstraintSystem::<<Bn256 as Engine>::Scalar>::default();
479+
let mut cs = ConstraintSystem::<<Bn256 as Engine>::Fr>::default();
445480
let config = SuperCircuit::configure(&mut cs);
446481
let rwtable_columns = config.get_rwtable_columns();
447482

@@ -515,10 +550,9 @@ fn new_empty_block_chunk() -> (Block<Fr>, Chunk<Fr>) {
515550
.new_circuit_input_builder()
516551
.handle_block(&block.eth_block, &block.geth_traces)
517552
.unwrap();
518-
(
519-
block_convert(&builder).unwrap(),
520-
chunk_convert(&builder, 0).unwrap(),
521-
)
553+
let block = block_convert(&builder).unwrap();
554+
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
555+
(block, chunk)
522556
}
523557

524558
fn get_general_params(degree: u32) -> ParamsKZG<Bn256> {

testool/src/statetest/executor.rs

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,8 @@ use std::{collections::HashMap, str::FromStr};
1616
use thiserror::Error;
1717
use zkevm_circuits::{
1818
super_circuit::SuperCircuit,
19-
<<<<<<< HEAD
20-
test_util::CircuitTestBuilder,
21-
witness::{Block, Chunk},
22-
=======
2319
test_util::{CircuitTestBuilder, CircuitTestError},
24-
witness::Block,
25-
>>>>>>> main
20+
witness::{Block, Chunk},
2621
};
2722

2823
#[derive(PartialEq, Eq, Error, Debug)]
@@ -353,13 +348,9 @@ pub fn run_test(
353348

354349
let block: Block<Fr> =
355350
zkevm_circuits::evm_circuit::witness::block_convert(&builder).unwrap();
356-
let chunk: Chunk<Fr> =
357-
zkevm_circuits::evm_circuit::witness::chunk_convert(&builder, 0).unwrap();
358-
359-
<<<<<<< HEAD
360-
CircuitTestBuilder::<1, 1>::new_from_block(block, chunk).run();
361-
=======
362-
CircuitTestBuilder::<1, 1>::new_from_block(block)
351+
let chunks: Vec<Chunk<Fr>> =
352+
zkevm_circuits::evm_circuit::witness::chunk_convert(&block, &builder).unwrap();
353+
CircuitTestBuilder::<1, 1>::new_from_block(block, chunks)
363354
.run_with_result()
364355
.map_err(|err| match err {
365356
CircuitTestError::VerificationFailed { reasons, .. } => {
@@ -373,7 +364,6 @@ pub fn run_test(
373364
found: err.to_string(),
374365
},
375366
})?;
376-
>>>>>>> main
377367
} else {
378368
geth_data.sign(&wallets);
379369

@@ -389,10 +379,13 @@ pub fn run_test(
389379
max_evm_rows: 0,
390380
max_keccak_rows: 0,
391381
};
392-
let (k, circuit, instance, _builder) =
382+
let (k, mut circuits, mut instances, _builder) =
393383
SuperCircuit::<Fr>::build(geth_data, circuits_params, Fr::from(0x100)).unwrap();
394384
builder = _builder;
395385

386+
let circuit = circuits.remove(0);
387+
let instance = instances.remove(0);
388+
396389
let prover = MockProver::run(k, &circuit, instance).unwrap();
397390
prover
398391
.verify()

zkevm-circuits/src/copy_circuit.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@ impl<F: Field> SubCircuit<F> for CopyCircuit<F> {
849849
fn new_from_block(block: &witness::Block<F>, chunk: &Chunk<F>) -> Self {
850850
let chunked_copy_events = block
851851
.copy_events
852-
.get(chunk.chunk_context.initial_copy..chunk.chunk_context.end_copy)
852+
.get(chunk.chunk_context.initial_copy_index..chunk.chunk_context.end_copy_index)
853853
.unwrap_or_default();
854854
Self::new_with_external_data(
855855
chunked_copy_events.to_owned(),
@@ -859,8 +859,8 @@ impl<F: Field> SubCircuit<F> for CopyCircuit<F> {
859859
max_calldata: chunk.fixed_param.max_calldata,
860860
txs: block.txs.clone(),
861861
max_rws: chunk.fixed_param.max_rws,
862-
rws: chunk.rws.clone(),
863-
prev_chunk_last_rw: chunk.prev_chunk_last_rw,
862+
rws: chunk.chrono_rws.clone(),
863+
prev_chunk_last_rw: chunk.prev_chunk_last_chrono_rw,
864864
bytecodes: block.bytecodes.clone(),
865865
},
866866
)

0 commit comments

Comments
 (0)