From d12b9ba623877ae18fe79ed76467fa28904077a8 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Fri, 23 Feb 2024 16:30:43 +0800 Subject: [PATCH] [wip] [with debug log] allow zero limb diff in state_circuit lexicoordering --- .../src/evm_circuit/execution/end_chunk.rs | 11 +- zkevm-circuits/src/state_circuit.rs | 2 +- .../state_circuit/lexicographic_ordering.rs | 34 ++-- zkevm-circuits/src/state_circuit/test.rs | 33 +++- zkevm-circuits/src/witness/block.rs | 14 +- zkevm-circuits/src/witness/chunk.rs | 167 ++++++++---------- zkevm-circuits/src/witness/rw.rs | 84 +++++---- 7 files changed, 183 insertions(+), 162 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs index 91acd9ed503..e75b3352543 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs @@ -117,7 +117,7 @@ mod test { let builder = BlockData::new_from_geth_data_with_params( block.clone(), FixedCParams { - total_chunks: 6, + total_chunks: 4, max_rws: 64, max_txs: 2, ..Default::default() @@ -135,6 +135,13 @@ mod test { idx, chunk.by_address_rw_fingerprints, chunk.chrono_rw_fingerprints, ); }); + // assert last fingerprint acc are equal + if let Some(last_chunk) = chunks.last() { + assert_eq!( + last_chunk.by_address_rw_fingerprints.mul_acc, + last_chunk.chrono_rw_fingerprints.mul_acc + ) + } } #[test] @@ -202,7 +209,7 @@ mod test { .params({ FixedCParams { total_chunks: 6, - max_rws: 64, + max_rws: 90, max_txs: 2, ..Default::default() } diff --git a/zkevm-circuits/src/state_circuit.rs b/zkevm-circuits/src/state_circuit.rs index 31657cdfe72..7f302a6c619 100644 --- a/zkevm-circuits/src/state_circuit.rs +++ b/zkevm-circuits/src/state_circuit.rs @@ -484,7 +484,7 @@ impl StateCircuit { permu_alpha: chunk.permu_alpha, permu_gamma: chunk.permu_gamma, rw_fingerprints: chunk.by_address_rw_fingerprints.clone(), - prev_chunk_last_rw: chunk.prev_chunk_last_chrono_rw, + prev_chunk_last_rw: chunk.prev_chunk_last_by_address_rw, _marker: PhantomData::default(), } } diff --git a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs index 062a85cb008..e0a7002ee06 100644 --- a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs +++ b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs @@ -99,7 +99,7 @@ pub struct Config { pub(crate) selector: Column, pub first_different_limb: BinaryNumberConfig, limb_difference: Column, - limb_difference_inverse: Column, + // limb_difference_inverse: Column, } impl Config { @@ -112,26 +112,26 @@ impl Config { let selector = meta.fixed_column(); let first_different_limb = BinaryNumberChip::configure(meta, selector, None); let limb_difference = meta.advice_column(); - let limb_difference_inverse = meta.advice_column(); + // let limb_difference_inverse = meta.advice_column(); let config = Config { selector, first_different_limb, limb_difference, - limb_difference_inverse, + // limb_difference_inverse, }; lookup.range_check_u16(meta, "limb_difference fits into u16", |meta| { meta.query_advice(limb_difference, Rotation::cur()) }); - meta.create_gate("limb_difference is not zero", |meta| { - let selector = meta.query_fixed(selector, Rotation::cur()); - let limb_difference = meta.query_advice(limb_difference, Rotation::cur()); - let limb_difference_inverse = - meta.query_advice(limb_difference_inverse, Rotation::cur()); - vec![selector * (1.expr() - limb_difference * limb_difference_inverse)] - }); + // meta.create_gate("limb_difference is not zero", |meta| { + // let selector = meta.query_fixed(selector, Rotation::cur()); + // let limb_difference = meta.query_advice(limb_difference, Rotation::cur()); + // let limb_difference_inverse = + // meta.query_advice(limb_difference_inverse, Rotation::cur()); + // vec![selector * (1.expr() - limb_difference * limb_difference_inverse)] + // }); meta.create_gate( "limb differences before first_different_limb are all 0", @@ -221,12 +221,12 @@ impl Config { offset, || Value::known(limb_difference), )?; - region.assign_advice( - || "limb_difference_inverse", - self.limb_difference_inverse, - offset, - || Value::known(limb_difference.invert().unwrap()), - )?; + // region.assign_advice( + // || "limb_difference_inverse", + // self.limb_difference_inverse, + // offset, + // || Value::known(limb_difference.invert().unwrap()), + // )?; Ok(index) } @@ -235,7 +235,7 @@ impl Config { pub fn annotate_columns_in_region(&self, region: &mut Region, prefix: &str) { [ (self.limb_difference, "LO_limb_difference"), - (self.limb_difference_inverse, "LO_limb_difference_inverse"), + // (self.limb_difference_inverse, "LO_limb_difference_inverse"), ] .iter() .for_each(|(col, ann)| region.name_column(|| format!("{}_{}", prefix, ann), *col)); diff --git a/zkevm-circuits/src/state_circuit/test.rs b/zkevm-circuits/src/state_circuit/test.rs index a00b45c7986..3cf36b1ed3b 100644 --- a/zkevm-circuits/src/state_circuit/test.rs +++ b/zkevm-circuits/src/state_circuit/test.rs @@ -34,6 +34,26 @@ fn state_circuit_unusable_rows() { ) } +fn new_chunk_from_rw_map(rws: &RwMap, padding_start_rw: Option) -> Chunk { + let (alpha, gamma) = get_permutation_randomness(); + let mut chunk = Chunk { + by_address_rws: rws.clone(), + ..Default::default() + }; + + let rw_fingerprints = get_permutation_fingerprint_of_rwmap( + &chunk.by_address_rws, + chunk.fixed_param.max_rws, + alpha, + gamma, + F::from(1), + false, + padding_start_rw, + ); + chunk.by_address_rw_fingerprints = rw_fingerprints; + chunk +} + fn test_state_circuit_ok( memory_ops: Vec>, stack_ops: Vec>, @@ -45,7 +65,7 @@ fn test_state_circuit_ok( storage: storage_ops, ..Default::default() }); - let chunk = Chunk::new_from_rw_map(&rw_map, None, None); + let chunk = new_chunk_from_rw_map(&rw_map, None); let circuit = StateCircuit::::new(&chunk); let instance = circuit.instance(); @@ -69,7 +89,7 @@ fn verifying_key_independent_of_rw_length() { let no_rows = StateCircuit::::new(&chunk); - chunk = Chunk::new_from_rw_map( + chunk = new_chunk_from_rw_map( &RwMap::from(&OperationContainer { memory: vec![Operation::new( RWCounter::from(1), @@ -80,7 +100,6 @@ fn verifying_key_independent_of_rw_length() { ..Default::default() }), None, - None, ); let one_row = StateCircuit::::new(&chunk); @@ -948,11 +967,7 @@ fn variadic_size_check() { }, ]; // let rw_map: RwMap = rows.clone().into(); - let circuit = StateCircuit::new(&Chunk::new_from_rw_map( - &RwMap::from(rows.clone()), - None, - None, - )); + let circuit = StateCircuit::new(&new_chunk_from_rw_map(&RwMap::from(rows.clone()), None)); let power_of_randomness = circuit.instance(); let prover1 = MockProver::::run(17, &circuit, power_of_randomness).unwrap(); @@ -973,7 +988,7 @@ fn variadic_size_check() { }, ]); - let circuit = StateCircuit::new(&Chunk::new_from_rw_map(&rows.into(), None, None)); + let circuit = StateCircuit::new(&new_chunk_from_rw_map(&rows.into(), None)); let power_of_randomness = circuit.instance(); let prover2 = MockProver::::run(17, &circuit, power_of_randomness).unwrap(); diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index d62a0700ecd..96c180353fe 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -295,13 +295,21 @@ pub fn block_convert( .chunks .iter() .fold(BTreeMap::new(), |mut map, chunk| { - assert!(chunk.ctx.rwc.0.saturating_sub(1) <= builder.circuits_params.max_rws); - // [chunk.ctx.rwc.0, builder.circuits_params.max_rws + 1) - (chunk.ctx.rwc.0..builder.circuits_params.max_rws + 1).for_each(|padding_rw_counter| { + assert!( + chunk.ctx.rwc.0.saturating_sub(1) <= builder.circuits_params.max_rws, + "max_rws size {} must larger than chunk rws size {}", + builder.circuits_params.max_rws, + chunk.ctx.rwc.0.saturating_sub(1), + ); + // [chunk.ctx.rwc.0, builder.circuits_params.max_rws) + (chunk.ctx.rwc.0..builder.circuits_params.max_rws).for_each(|padding_rw_counter| { *map.entry(padding_rw_counter).or_insert(0) += 1; }); map }); + rw_padding_meta.iter().for_each(|(padding_rwc, count)| { + println!("padding_rwc {}, count {}", padding_rwc, count); + }); let mut block = Block { // randomness: F::from(0x100), // Special value to reveal elements after RLC diff --git a/zkevm-circuits/src/witness/chunk.rs b/zkevm-circuits/src/witness/chunk.rs index 3a0681b23de..628f4596947 100755 --- a/zkevm-circuits/src/witness/chunk.rs +++ b/zkevm-circuits/src/witness/chunk.rs @@ -1,4 +1,4 @@ -use std::{cmp::min, iter}; +use std::iter; /// use super::{ @@ -8,6 +8,7 @@ use super::{ use crate::util::unwrap_value; use bus_mapping::{ circuit_input_builder::{self, Call, ChunkContext, FixedCParams}, + operation::Target, Error, }; use eth_types::Field; @@ -75,40 +76,6 @@ impl Default for Chunk { } } -impl Chunk { - #[cfg(test)] - pub(crate) fn new_from_rw_map( - rws: &RwMap, - padding_start_rw: Option, - chrono_padding_start_rw: Option, - ) -> Self { - let (alpha, gamma) = get_permutation_randomness(); - let mut chunk = Chunk::default(); - let rw_fingerprints = get_permutation_fingerprint_of_rwmap( - rws, - chunk.fixed_param.max_rws, - alpha, - gamma, - F::from(1), - false, - padding_start_rw, - ); - let chrono_rw_fingerprints = get_permutation_fingerprint_of_rwmap( - rws, - chunk.fixed_param.max_rws, - alpha, - gamma, - F::from(1), - true, - chrono_padding_start_rw, - ); - chunk.chrono_rws = rws.clone(); - chunk.by_address_rw_fingerprints = rw_fingerprints; - chunk.chrono_rw_fingerprints = chrono_rw_fingerprints; - chunk - } -} - /// Convert the idx-th chunk struct in bus-mapping to a witness chunk used in circuits pub fn chunk_convert( block: &Block, @@ -127,52 +94,20 @@ pub fn chunk_convert( .enumerate() { let chunk = chunk.unwrap(); // current chunk always there - let (prev_chunk_last_chrono_rw, prev_chunk_last_by_address_rw) = prev_chunk - .map(|prev_chunk| { - let prev_chunk_last_chrono_rw = { - assert!(builder.circuits_params.max_rws > 0); - let chunk_inner_rwc = prev_chunk.ctx.rwc.0; - if chunk_inner_rwc.saturating_sub(1) == builder.circuits_params.max_rws { - // if prev chunk rws are full, then get the last rwc - RwMap::get_rw(&builder.block.container, prev_chunk.ctx.end_rwc - 1) - .expect("Rw does not exist") - } else { - // last is the padding row - Rw::Padding { - rw_counter: builder.circuits_params.max_rws, - } - } - }; - // get prev_chunk last by_address sorted rw - let prev_chunk_by_address_last_rw = { - let by_address_last_rwc_index = - (prev_chunk.ctx.idx + 1) * builder.circuits_params.max_rws; - let prev_chunk_by_address_last_rwc = by_address_rws - .get(by_address_last_rwc_index - 1) - .cloned() - .or_else(|| { - let target_padding_count = - (by_address_last_rwc_index + 1) - by_address_rws.len(); - let mut accu_count = 0; - padding_meta - .iter() - .find(|(_, count)| { - accu_count += **count; - target_padding_count <= accu_count.try_into().unwrap() - }) - .map(|(padding_rwc, _)| Rw::Padding { - rw_counter: *padding_rwc, - }) - }); - - prev_chunk_by_address_last_rwc - }; - (prev_chunk_last_chrono_rw, prev_chunk_by_address_last_rw) - }) - .map(|(prev_chunk_last_rwc, prev_chunk_by_address_last_rwc)| { - (Some(prev_chunk_last_rwc), prev_chunk_by_address_last_rwc) - }) - .unwrap_or_else(|| (None, None)); + let prev_chunk_last_chrono_rw = prev_chunk.map(|prev_chunk| { + assert!(builder.circuits_params.max_rws > 0); + let chunk_inner_rwc = prev_chunk.ctx.rwc.0; + if chunk_inner_rwc.saturating_sub(1) == builder.circuits_params.max_rws { + // if prev chunk rws are full, then get the last rwc + RwMap::get_rw(&builder.block.container, prev_chunk.ctx.end_rwc - 1) + .expect("Rw does not exist") + } else { + // last is the padding row + Rw::Padding { + rw_counter: builder.circuits_params.max_rws - 1, + } + } + }); println!( "| {:?} ... {:?} | @chunk_convert", @@ -180,26 +115,50 @@ pub fn chunk_convert( ); // Get the rws in the i-th chunk - let chrono_rws = RwMap::from(&builder.block.container) - .take_rw_counter_range(chunk.ctx.initial_rwc, chunk.ctx.end_rwc); - - chrono_rws.check_value(); + let chrono_rws = { + let mut chrono_rws = RwMap::from(&builder.block.container); + // remove paading here since it will be attached later + if let Some(padding_vec) = chrono_rws.0.get_mut(&Target::Padding) { + padding_vec.clear() + } + chrono_rws.take_rw_counter_range(chunk.ctx.initial_rwc, chunk.ctx.end_rwc) + }; - let by_address_rws = RwMap::from({ + let (prev_chunk_last_by_address_rw, by_address_rws) = { // by_address_rws - let start = min( - chunk.ctx.idx * builder.circuits_params.max_rws, - by_address_rws.len(), - ); - let end = min( - (chunk.ctx.idx + 1) * builder.circuits_params.max_rws, - by_address_rws.len(), - ); - by_address_rws[start..end].to_vec() - }) - .take_rw_counter_range(chunk.ctx.initial_rwc, chunk.ctx.end_rwc); + let start = chunk.ctx.idx * builder.circuits_params.max_rws; + let size = builder.circuits_params.max_rws; + // by_address_rws[start..end].to_vec() + + let skipped = by_address_rws + .iter() + // remove paading here since it will be attached later + .filter(|rw| rw.tag() != Target::Padding) + .cloned() // TODO avoid clone here + .chain(padding_meta.iter().flat_map(|(k, v)| { + vec![ + Rw::Padding { rw_counter: *k }; + >::try_into(*v).unwrap() + ] + })); + // there is no previous chunk + if start == 0 { + (None, RwMap::from(skipped.take(size).collect::>())) + } else { + // here have `chunk.ctx.idx - 1` because each chunk first row are probagated from + // prev chunk. giving idx>0 th chunk, there will be (idx-1) placeholders cant' count + // in real order + let mut skipped = skipped.skip(start - 1 - (chunk.ctx.idx - 1)); + let prev_chunk_last_by_address_rw = skipped.next(); + ( + prev_chunk_last_by_address_rw, + RwMap::from(skipped.take(size).collect::>()), + ) + } + }; // Comupute cur fingerprints from last fingerprints and current Rw rows + println!("by address gogo"); let by_address_rw_fingerprints = get_permutation_fingerprint_of_rwmap( &by_address_rws, chunk.fixed_param.max_rws, @@ -214,6 +173,7 @@ pub fn chunk_convert( prev_chunk_last_by_address_rw, ); + println!("by chrono gogo"); let chrono_rw_fingerprints = get_permutation_fingerprint_of_rwmap( &chrono_rws, chunk.fixed_param.max_rws, @@ -330,6 +290,19 @@ pub fn get_permutation_fingerprint_of_rwrowvec( padding_start_rwrow: Option>>, ) -> RwFingerprints { let (rows, _) = RwRow::padding(rwrowvec, max_row, padding_start_rwrow); + rows.iter().for_each(|rw| { + println!( + "rw tag {:?}, rw_counter {:?} ", + { usize::from_le_bytes(unwrap_value(rw.tag).to_repr()[..8].try_into().unwrap()) }, + { + usize::from_le_bytes( + unwrap_value(rw.rw_counter).to_repr()[..8] + .try_into() + .unwrap(), + ) + }, + ) + }); let x = rows.to2dvec(); let fingerprints = get_permutation_fingerprints( &x, diff --git a/zkevm-circuits/src/witness/rw.rs b/zkevm-circuits/src/witness/rw.rs index 907472b900b..06120f95a39 100644 --- a/zkevm-circuits/src/witness/rw.rs +++ b/zkevm-circuits/src/witness/rw.rs @@ -1,5 +1,8 @@ //! The Read-Write table related structs -use std::{collections::HashMap, iter}; +use std::{ + collections::{HashMap, HashSet}, + iter, +}; use bus_mapping::{ exec_trace::OperationRef, @@ -116,8 +119,9 @@ impl RwMap { } /// Calculates the number of Rw::Padding rows needed. /// `target_len` is allowed to be 0 as an "auto" mode, + /// return padding size also allow to be 0, means no padding pub(crate) fn padding_len(rows_len: usize, target_len: usize) -> usize { - if target_len > rows_len { + if target_len >= rows_len { target_len - rows_len } else { if target_len != 0 { @@ -135,10 +139,17 @@ impl RwMap { target_len: usize, padding_start_rw: Option, ) -> (Vec, usize) { + let mut padding_exist = HashSet::new(); // Remove Start/Padding rows as we will add them from scratch. let rows_trimmed: Vec = rows .iter() - .filter(|rw| !matches!(rw, Rw::Start { .. } | Rw::Padding { .. })) + .filter(|rw| { + if let Rw::Padding { rw_counter } = rw { + padding_exist.insert(*rw_counter); + } + + !matches!(rw, Rw::Start { .. }) + }) .cloned() .collect(); let padding_length = { @@ -146,25 +157,23 @@ impl RwMap { length.saturating_sub(1) }; - // option 1: need to provide padding starting rw_counter at function parameters - // option 2: just padding after local max rw_counter + 1 - // We adapt option 2 for now - // the side effect is it introduce malleable proof when append `Rw::Padding` rw_counter, - // because `Rw::Padding` is not global unique - let start_padding_rw_counter = rows_trimmed - .iter() - .map(|rw| rw.rw_counter()) - .max() - .unwrap_or(1) - + 1; + // padding rw_counter starting from + // +1 for to including padding_start row + let start_padding_rw_counter = rows_trimmed.len() + 1; - let padding = (start_padding_rw_counter..start_padding_rw_counter + padding_length) - .map(|rw_counter| Rw::Padding { rw_counter }); + let padding = (start_padding_rw_counter..).flat_map(|rw_counter| { + if padding_exist.contains(&rw_counter) { + None + } else { + Some(Rw::Padding { rw_counter }) + } + }); ( iter::empty() .chain([padding_start_rw.unwrap_or(Rw::Start { rw_counter: 1 })]) .chain(rows_trimmed.into_iter()) .chain(padding.into_iter()) + .take(target_len) .collect(), padding_length, ) @@ -191,9 +200,9 @@ impl RwMap { } /// take only rw_counter within range - pub fn take_rw_counter_range(mut self, start: usize, end: usize) -> Self { + pub fn take_rw_counter_range(mut self, start_rwc: usize, end_rwc: usize) -> Self { for rw in self.0.values_mut() { - rw.retain(|r| r.rw_counter() >= start && r.rw_counter() < end) + rw.retain(|r| r.rw_counter() >= start_rwc && r.rw_counter() < end_rwc) } self } @@ -428,13 +437,22 @@ impl RwRow> { target_len: usize, padding_start_rwrow: Option>>, ) -> (Vec>>, usize) { + let mut padding_exist = HashSet::new(); // Remove Start/Padding rows as we will add them from scratch. let rows_trimmed = rows .iter() .filter(|rw| { let tag = unwrap_value(rw.tag); - !(tag == F::from(Target::Start as u64) || tag == F::from(Target::Padding as u64)) - && tag != F::ZERO // 0 is invalid tag + + if tag == F::from(Target::Padding as u64) { + let rw_counter = u64::from_le_bytes( + unwrap_value(rw.rw_counter).to_repr()[..U64_BYTES] + .try_into() + .unwrap(), + ); + padding_exist.insert(rw_counter); + } + tag != F::from(Target::Start as u64) && tag != F::ZERO // 0 is invalid tag }) .cloned() .collect::>>>(); @@ -443,12 +461,7 @@ impl RwRow> { length.saturating_sub(1) // first row always got padding }; let start_padding_rw_counter = { - let start_padding_rw_counter = rows_trimmed - .iter() - .map(|rw| unwrap_value(rw.rw_counter)) - .max() - .unwrap_or(F::from(1u64)) - + F::ONE; + let start_padding_rw_counter = F::from(rows_trimmed.len() as u64) + F::ONE; // Assume root of unity < 2**64 assert!( start_padding_rw_counter.to_repr()[U64_BYTES..] @@ -465,13 +478,17 @@ impl RwRow> { ) } as usize; - let padding = (start_padding_rw_counter..start_padding_rw_counter + padding_length).map( - |rw_counter| RwRow { - rw_counter: Value::known(F::from(rw_counter as u64)), - tag: Value::known(F::from(Target::Padding as u64)), - ..Default::default() - }, - ); + let padding = (start_padding_rw_counter..).flat_map(|rw_counter| { + if padding_exist.contains(&rw_counter.try_into().unwrap()) { + None + } else { + Some(RwRow { + rw_counter: Value::known(F::from(rw_counter as u64)), + tag: Value::known(F::from(Target::Padding as u64)), + ..Default::default() + }) + } + }); ( iter::once(padding_start_rwrow.unwrap_or(RwRow { rw_counter: Value::known(F::ONE), @@ -480,6 +497,7 @@ impl RwRow> { })) .chain(rows_trimmed.into_iter()) .chain(padding.into_iter()) + .take(target_len) .collect(), padding_length, )