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

Commit

Permalink
Fix ExpCircuit parameter bug + Improve tests
Browse files Browse the repository at this point in the history
- max_rows and max_steps were being confused
- `gen_data` in test now uses dynamic parameters for all tests
  except variadic size test.
  • Loading branch information
davidnevadoc committed Jun 21, 2023
1 parent 6a82fce commit 159d576
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
10 changes: 6 additions & 4 deletions zkevm-circuits/src/exp_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,11 +291,12 @@ impl<F: Field> ExpCircuitConfig<F> {
&self,
layouter: &mut impl Layouter<F>,
exp_events: &[ExpEvent],
max_exp_steps: usize,
max_exp_rows: usize,
) -> Result<(), Error> {
let max_exp_rows = max_exp_steps * OFFSET_INCREMENT;
let min_n_rows = Self::min_num_rows(exp_events);
dbg!(max_exp_rows, min_n_rows);
debug_assert!(
Self::min_num_rows(exp_events) <= max_exp_rows,
min_n_rows <= max_exp_rows,
"insufficient rows to populate the exponentiation trace"
);

Expand Down Expand Up @@ -484,7 +485,8 @@ pub struct ExpCircuit<F> {

impl<F: Field> ExpCircuit<F> {
/// Return a new ExpCircuit
pub fn new(exp_events: Vec<ExpEvent>, max_exp_rows: usize) -> Self {
pub fn new(exp_events: Vec<ExpEvent>, max_exp_steps: usize) -> Self {
let max_exp_rows = max_exp_steps * OFFSET_INCREMENT + UNUSABLE_EXP_ROWS;
Self {
exp_events,
max_exp_rows,
Expand Down
28 changes: 18 additions & 10 deletions zkevm-circuits/src/exp_circuit/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,29 +51,37 @@ fn gen_code_multiple(args: Vec<(Word, Word)>) -> Bytecode {
code
}

fn gen_data(code: Bytecode) -> CircuitInputBuilder<FixedCParams> {
fn gen_data(code: Bytecode, default_params: bool) -> CircuitInputBuilder<FixedCParams> {
let test_ctx = TestContext::<2, 1>::simple_ctx_with_bytecode(code).unwrap();
let block: GethData = test_ctx.into();
// Needs default parameters for variadic size test
let mut builder =
BlockData::new_from_geth_data_with_params(block.clone(), FixedCParams::default())
.new_circuit_input_builder();
builder
.handle_block(&block.eth_block, &block.geth_traces)
.unwrap();
let builder = if default_params {
let mut builder =
BlockData::new_from_geth_data_with_params(block.clone(), FixedCParams::default())
.new_circuit_input_builder();
builder
.handle_block(&block.eth_block, &block.geth_traces)
.unwrap();
builder
} else {
let builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder();
builder
.handle_block(&block.eth_block, &block.geth_traces)
.unwrap()
};
builder
}

fn test_ok(base: Word, exponent: Word, k: Option<u32>) {
let code = gen_code_single(base, exponent);
let builder = gen_data(code);
let builder = gen_data(code, false);
let block = block_convert::<Fr>(&builder).unwrap();
test_exp_circuit(k.unwrap_or(18), block);
}

fn test_ok_multiple(args: Vec<(Word, Word)>) {
let code = gen_code_multiple(args);
let builder = gen_data(code);
let builder = gen_data(code, false);
let block = block_convert::<Fr>(&builder).unwrap();
test_exp_circuit(20, block);
}
Expand Down Expand Up @@ -142,7 +150,7 @@ fn variadic_size_check() {
EXP
STOP
};
let builder = gen_data(code);
let builder = gen_data(code, true);
let block = block_convert::<Fr>(&builder).unwrap();
let circuit = ExpCircuit::<Fr>::new(
block.exp_events.clone(),
Expand Down

0 comments on commit 159d576

Please sign in to comment.