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

Commit

Permalink
[proof chunk] [WIP] [testing] fix various issue found in integration …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
hero78119 authored Mar 1, 2024
1 parent 64154f7 commit 0ab41c9
Show file tree
Hide file tree
Showing 36 changed files with 1,569 additions and 797 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

371 changes: 223 additions & 148 deletions bus-mapping/src/circuit_input_builder.rs

Large diffs are not rendered by default.

41 changes: 19 additions & 22 deletions bus-mapping/src/circuit_input_builder/chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub struct ChunkContext {
/// Index of current chunk, start from 0
pub idx: usize,
/// Used to track the inner chunk counter in every operation in the chunk.
/// it will be reset for every new chunk.
/// Contains the next available value.
pub rwc: RWCounter,
/// Number of chunks
Expand All @@ -33,16 +34,14 @@ pub struct ChunkContext {
pub initial_rwc: usize,
/// End global rw counter
pub end_rwc: usize,
/// tx range in block: [initial_tx_index, end_tx_index)
pub initial_tx_index: usize,
///
pub initial_tx: usize,
pub end_tx_index: usize,
/// copy range in block: [initial_copy_index, end_copy_index)
pub initial_copy_index: usize,
///
pub end_tx: usize,
///
pub initial_copy: usize,
///
pub end_copy: usize,
/// Druing dry run, chuncking is desabled
pub enable: bool,
pub end_copy_index: usize,
}

impl Default for ChunkContext {
Expand All @@ -60,11 +59,10 @@ impl ChunkContext {
total_chunks,
initial_rwc: 1, // rw counter start from 1
end_rwc: 0, // end_rwc should be set in later phase
initial_tx: 1,
end_tx: 0,
initial_copy: 0,
end_copy: 0,
enable: true,
initial_tx_index: 0,
end_tx_index: 0,
initial_copy_index: 0,
end_copy_index: 0,
}
}

Expand All @@ -76,11 +74,10 @@ impl ChunkContext {
total_chunks: 1,
initial_rwc: 1, // rw counter start from 1
end_rwc: 0, // end_rwc should be set in later phase
initial_tx: 1,
end_tx: 0,
initial_copy: 0,
end_copy: 0,
enable: true,
initial_tx_index: 0,
end_tx_index: 0,
initial_copy_index: 0,
end_copy_index: 0,
}
}

Expand All @@ -91,11 +88,11 @@ impl ChunkContext {
self.idx += 1;
self.rwc = RWCounter::new();
self.initial_rwc = initial_rwc;
self.initial_tx = initial_tx;
self.initial_copy = initial_copy;
self.initial_tx_index = initial_tx;
self.initial_copy_index = initial_copy;
self.end_rwc = 0;
self.end_tx = 0;
self.end_copy = 0;
self.end_tx_index = 0;
self.end_copy_index = 0;
}

/// Is first chunk
Expand Down
9 changes: 7 additions & 2 deletions bus-mapping/src/circuit_input_builder/input_state_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impl<'a> CircuitInputStateRef<'a> {
exec_state: ExecState::InvalidTx,
gas_left: self.tx.gas(),
rwc: self.block_ctx.rwc,
rwc_inner_chunk: self.chunk_ctx.rwc,
..Default::default()
}
}
Expand Down Expand Up @@ -148,9 +149,13 @@ impl<'a> CircuitInputStateRef<'a> {
/// Check whether rws will overflow circuit limit.
pub fn check_rw_num_limit(&self) -> Result<(), Error> {
if let Some(max_rws) = self.max_rws {
let rwc = self.block_ctx.rwc.0;
let rwc = self.chunk_ctx.rwc.0;
if rwc > max_rws {
log::error!("rwc > max_rws, rwc={}, max_rws={}", rwc, max_rws);
log::error!(
"chunk inner rwc > max_rws, rwc={}, max_rws={}",
rwc,
max_rws
);
return Err(Error::RwsNotEnough(max_rws, rwc));
};
}
Expand Down
2 changes: 1 addition & 1 deletion circuit-benchmarks/src/copy_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ mod tests {
.handle_block(&block.eth_block, &block.geth_traces)
.unwrap();
let block = block_convert(&builder).unwrap();
let chunk = chunk_convert(&builder, 0).unwrap();
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
assert_eq!(block.copy_events.len(), copy_event_num);
(block, chunk)
}
Expand Down
2 changes: 1 addition & 1 deletion circuit-benchmarks/src/evm_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ mod evm_circ_benches {
.unwrap();

let block = block_convert(&builder).unwrap();
let chunk = chunk_convert(&builder, 0).unwrap();
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);

let circuit = TestEvmCircuit::<Fr>::new(block, chunk);
let mut rng = XorShiftRng::from_seed([
Expand Down
7 changes: 3 additions & 4 deletions circuit-benchmarks/src/exp_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,8 @@ mod tests {
.new_circuit_input_builder()
.handle_block(&block.eth_block, &block.geth_traces)
.unwrap();
(
block_convert(&builder).unwrap(),
chunk_convert(&builder, 0).unwrap(),
)
let block = block_convert(&builder).unwrap();
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
(block, chunk)
}
}
3 changes: 2 additions & 1 deletion circuit-benchmarks/src/super_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ mod tests {
max_evm_rows: 0,
max_keccak_rows: 0,
};
let (_, circuit, instance, _) =
let (_, mut circuits, mut instances, _) =
SuperCircuit::build(block, circuits_params, Fr::from(0x100)).unwrap();
let (circuit, instance) = (circuits.remove(0), instances.remove(0));
let instance_refs: Vec<&[Fr]> = instance.iter().map(|v| &v[..]).collect();

// Bench setup generation
Expand Down
1 change: 1 addition & 0 deletions integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ rand_chacha = "0.3"
paste = "1.0"
rand_xorshift = "0.3.0"
rand_core = "0.6.4"
itertools = "0.10"
mock = { path = "../mock" }

[dev-dependencies]
Expand Down
54 changes: 44 additions & 10 deletions integration-tests/src/integration_test_circuits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use halo2_proofs::{
},
},
};
use itertools::Itertools;
use lazy_static::lazy_static;
use mock::TestContext;
use rand_chacha::rand_core::SeedableRng;
Expand Down Expand Up @@ -358,10 +359,26 @@ impl<C: SubCircuit<Fr> + Circuit<Fr>> IntegrationTest<C> {
let fixed = mock_prover.fixed();

if let Some(prev_fixed) = self.fixed.clone() {
assert!(
fixed.eq(&prev_fixed),
"circuit fixed columns are not constant for different witnesses"
);
fixed
.iter()
.enumerate()
.zip_eq(prev_fixed.iter())
.for_each(|((index, col1), col2)| {
if !col1.eq(col2) {
println!("on column index {} not equal", index);
col1.iter().enumerate().zip_eq(col2.iter()).for_each(
|((index, cellv1), cellv2)| {
assert!(
cellv1.eq(cellv2),
"cellv1 {:?} != cellv2 {:?} on index {}",
cellv1,
cellv2,
index
);
},
);
}
});
} else {
self.fixed = Some(fixed.clone());
}
Expand All @@ -383,6 +400,24 @@ impl<C: SubCircuit<Fr> + Circuit<Fr>> IntegrationTest<C> {

match self.root_fixed.clone() {
Some(prev_fixed) => {
fixed.iter().enumerate().zip_eq(prev_fixed.iter()).for_each(
|((index, col1), col2)| {
if !col1.eq(col2) {
println!("on column index {} not equal", index);
col1.iter().enumerate().zip_eq(col2.iter()).for_each(
|((index, cellv1), cellv2)| {
assert!(
cellv1.eq(cellv2),
"cellv1 {:?} != cellv2 {:?} on index {}",
cellv1,
cellv2,
index
);
},
);
}
},
);
assert!(
fixed.eq(&prev_fixed),
"root circuit fixed columns are not constant for different witnesses"
Expand Down Expand Up @@ -424,7 +459,7 @@ impl<C: SubCircuit<Fr> + Circuit<Fr>> IntegrationTest<C> {
block_tag,
);
let mut block = block_convert(&builder).unwrap();
let chunk = chunk_convert(&builder, 0).unwrap();
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
block.randomness = Fr::from(TEST_MOCK_RANDOMNESS);
let circuit = C::new_from_block(&block, &chunk);
let instance = circuit.instance();
Expand All @@ -441,7 +476,7 @@ impl<C: SubCircuit<Fr> + Circuit<Fr>> IntegrationTest<C> {
);

// get chronological_rwtable and byaddr_rwtable columns index
let mut cs = ConstraintSystem::<<Bn256 as Engine>::Scalar>::default();
let mut cs = ConstraintSystem::<<Bn256 as Engine>::Fr>::default();
let config = SuperCircuit::configure(&mut cs);
let rwtable_columns = config.get_rwtable_columns();

Expand Down Expand Up @@ -515,10 +550,9 @@ fn new_empty_block_chunk() -> (Block<Fr>, Chunk<Fr>) {
.new_circuit_input_builder()
.handle_block(&block.eth_block, &block.geth_traces)
.unwrap();
(
block_convert(&builder).unwrap(),
chunk_convert(&builder, 0).unwrap(),
)
let block = block_convert(&builder).unwrap();
let chunk = chunk_convert(&block, &builder).unwrap().remove(0);
(block, chunk)
}

fn get_general_params(degree: u32) -> ParamsKZG<Bn256> {
Expand Down
23 changes: 8 additions & 15 deletions testool/src/statetest/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,8 @@ use std::{collections::HashMap, str::FromStr};
use thiserror::Error;
use zkevm_circuits::{
super_circuit::SuperCircuit,
<<<<<<< HEAD
test_util::CircuitTestBuilder,
witness::{Block, Chunk},
=======
test_util::{CircuitTestBuilder, CircuitTestError},
witness::Block,
>>>>>>> main
witness::{Block, Chunk},
};

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

let block: Block<Fr> =
zkevm_circuits::evm_circuit::witness::block_convert(&builder).unwrap();
let chunk: Chunk<Fr> =
zkevm_circuits::evm_circuit::witness::chunk_convert(&builder, 0).unwrap();

<<<<<<< HEAD
CircuitTestBuilder::<1, 1>::new_from_block(block, chunk).run();
=======
CircuitTestBuilder::<1, 1>::new_from_block(block)
let chunks: Vec<Chunk<Fr>> =
zkevm_circuits::evm_circuit::witness::chunk_convert(&block, &builder).unwrap();
CircuitTestBuilder::<1, 1>::new_from_block(block, chunks)
.run_with_result()
.map_err(|err| match err {
CircuitTestError::VerificationFailed { reasons, .. } => {
Expand All @@ -373,7 +364,6 @@ pub fn run_test(
found: err.to_string(),
},
})?;
>>>>>>> main
} else {
geth_data.sign(&wallets);

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

let circuit = circuits.remove(0);
let instance = instances.remove(0);

let prover = MockProver::run(k, &circuit, instance).unwrap();
prover
.verify()
Expand Down
6 changes: 3 additions & 3 deletions zkevm-circuits/src/copy_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ impl<F: Field> SubCircuit<F> for CopyCircuit<F> {
fn new_from_block(block: &witness::Block<F>, chunk: &Chunk<F>) -> Self {
let chunked_copy_events = block
.copy_events
.get(chunk.chunk_context.initial_copy..chunk.chunk_context.end_copy)
.get(chunk.chunk_context.initial_copy_index..chunk.chunk_context.end_copy_index)
.unwrap_or_default();
Self::new_with_external_data(
chunked_copy_events.to_owned(),
Expand All @@ -859,8 +859,8 @@ impl<F: Field> SubCircuit<F> for CopyCircuit<F> {
max_calldata: chunk.fixed_param.max_calldata,
txs: block.txs.clone(),
max_rws: chunk.fixed_param.max_rws,
rws: chunk.rws.clone(),
prev_chunk_last_rw: chunk.prev_chunk_last_rw,
rws: chunk.chrono_rws.clone(),
prev_chunk_last_rw: chunk.prev_chunk_last_chrono_rw,
bytecodes: block.bytecodes.clone(),
},
)
Expand Down
Loading

0 comments on commit 0ab41c9

Please sign in to comment.