From 7f2d2fe67ee2e48ec3a4756d3e75d8743f2fd62d Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 08:56:11 +0100 Subject: [PATCH 01/15] o1vm/mips: introduce scratch_state_inverse These fields will be used to keep track of the elements that should be inverted while generating the proof. --- o1vm/src/interpreters/mips/tests_helpers.rs | 4 ++++ o1vm/src/interpreters/mips/witness.rs | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/o1vm/src/interpreters/mips/tests_helpers.rs b/o1vm/src/interpreters/mips/tests_helpers.rs index 0ad73ab3cc..f07b260e21 100644 --- a/o1vm/src/interpreters/mips/tests_helpers.rs +++ b/o1vm/src/interpreters/mips/tests_helpers.rs @@ -13,6 +13,8 @@ use std::{fs, path::PathBuf}; // FIXME: we should parametrize the tests with different fields. use ark_bn254::Fr as Fp; +use super::witness::SCRATCH_SIZE_INVERSE; + const PAGE_INDEX_EXECUTABLE_MEMORY: u32 = 1; pub(crate) struct OnDiskPreImageOracle; @@ -87,7 +89,9 @@ where registers: Registers::default(), registers_write_index: Registers::default(), scratch_state_idx: 0, + scratch_state_idx_inverse: 0, scratch_state: [Fp::from(0); SCRATCH_SIZE], + scratch_state_inverse: [Fp::from(0); SCRATCH_SIZE_INVERSE], selector: crate::interpreters::mips::column::N_MIPS_SEL_COLS, halt: false, // Keccak related diff --git a/o1vm/src/interpreters/mips/witness.rs b/o1vm/src/interpreters/mips/witness.rs index ca0b98e1ef..c3936b38ce 100644 --- a/o1vm/src/interpreters/mips/witness.rs +++ b/o1vm/src/interpreters/mips/witness.rs @@ -50,10 +50,17 @@ pub const NUM_INSTRUCTION_LOOKUP_TERMS: usize = 5; pub const NUM_LOOKUP_TERMS: usize = NUM_GLOBAL_LOOKUP_TERMS + NUM_DECODING_LOOKUP_TERMS + NUM_INSTRUCTION_LOOKUP_TERMS; // TODO: Delete and use a vector instead +// FIXME: since the introduction of the scratch size inverse, the value below +// can be decreased. It implies to change the offsets defined in [column]. At +// the moment, it incurs an overhead we could avoid as some columns are zeroes. // MIPS + hash_counter + byte_counter + eof + num_bytes_read + chunk + bytes // + length + has_n_bytes + chunk_bytes + preimage pub const SCRATCH_SIZE: usize = 98; +/// Number of columns used by the MIPS interpreter to keep values to be +/// inverted. +pub const SCRATCH_SIZE_INVERSE: usize = 12; + #[derive(Clone, Default)] pub struct SyscallEnv { pub last_hint: Option>, @@ -81,7 +88,9 @@ pub struct Env { pub registers: Registers, pub registers_write_index: Registers, pub scratch_state_idx: usize, + pub scratch_state_idx_inverse: usize, pub scratch_state: [Fp; SCRATCH_SIZE], + pub scratch_state_inverse: [Fp; SCRATCH_SIZE_INVERSE], pub halt: bool, pub syscall_env: SyscallEnv, pub selector: usize, @@ -878,7 +887,9 @@ impl Env { registers: initial_registers.clone(), registers_write_index: Registers::default(), scratch_state_idx: 0, + scratch_state_idx_inverse: 0, scratch_state: fresh_scratch_state(), + scratch_state_inverse: fresh_scratch_state(), halt: state.exited, syscall_env, selector, From ff73fa22bded511d015a8c1d48585b63cad0dc8e Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 09:05:27 +0100 Subject: [PATCH 02/15] o1vm/MIPS: intro a new column to keep track of values to be inverted --- o1vm/src/interpreters/mips/column.rs | 19 +++++++++++++++---- o1vm/src/interpreters/mips/constraints.rs | 8 ++++++++ o1vm/src/interpreters/mips/interpreter.rs | 2 ++ o1vm/src/interpreters/mips/witness.rs | 7 +++++++ 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/o1vm/src/interpreters/mips/column.rs b/o1vm/src/interpreters/mips/column.rs index 1b2acfc830..d6e018b0b0 100644 --- a/o1vm/src/interpreters/mips/column.rs +++ b/o1vm/src/interpreters/mips/column.rs @@ -9,7 +9,7 @@ use kimchi_msm::{ use std::ops::{Index, IndexMut}; use strum::EnumCount; -use super::{ITypeInstruction, JTypeInstruction, RTypeInstruction}; +use super::{witness::SCRATCH_SIZE_INVERSE, ITypeInstruction, JTypeInstruction, RTypeInstruction}; /// The number of hashes performed so far in the block pub(crate) const MIPS_HASH_COUNTER_OFF: usize = 80; @@ -50,6 +50,9 @@ pub const N_MIPS_COLS: usize = N_MIPS_REL_COLS + N_MIPS_SEL_COLS; pub enum ColumnAlias { // Can be seen as the abstract indexed variable X_{i} ScratchState(usize), + // A column whose value needs to be inverted in the final witness. + // We're keeping a separate column to perform a batch inversion at the end. + ScratchStateInverse(usize), InstructionCounter, Selector(usize), } @@ -66,8 +69,12 @@ impl From for usize { assert!(i < SCRATCH_SIZE); i } - ColumnAlias::InstructionCounter => SCRATCH_SIZE, - ColumnAlias::Selector(s) => SCRATCH_SIZE + 1 + s, + ColumnAlias::ScratchStateInverse(i) => { + assert!(i < SCRATCH_SIZE_INVERSE); + SCRATCH_SIZE + i + } + ColumnAlias::InstructionCounter => SCRATCH_SIZE + SCRATCH_SIZE_INVERSE, + ColumnAlias::Selector(s) => SCRATCH_SIZE + SCRATCH_SIZE_INVERSE + 1 + s, } } } @@ -138,7 +145,11 @@ impl ColumnIndexer for ColumnAlias { assert!(ss < SCRATCH_SIZE); Column::Relation(ss) } - Self::InstructionCounter => Column::Relation(SCRATCH_SIZE), + Self::ScratchStateInverse(ss) => { + assert!(ss < SCRATCH_SIZE_INVERSE); + Column::Relation(SCRATCH_SIZE + ss) + } + Self::InstructionCounter => Column::Relation(SCRATCH_SIZE + SCRATCH_SIZE_INVERSE), // TODO: what happens with error? It does not have a corresponding alias Self::Selector(s) => { assert!(s < N_MIPS_SEL_COLS); diff --git a/o1vm/src/interpreters/mips/constraints.rs b/o1vm/src/interpreters/mips/constraints.rs index 94369c890b..fa047f199a 100644 --- a/o1vm/src/interpreters/mips/constraints.rs +++ b/o1vm/src/interpreters/mips/constraints.rs @@ -25,6 +25,7 @@ use super::column::N_MIPS_SEL_COLS; /// The environment keeping the constraints between the different polynomials pub struct Env { scratch_state_idx: usize, + scratch_state_idx_inverse: usize, /// A list of constraints, which are multi-variate polynomials over a field, /// represented using the expression framework of `kimchi`. constraints: Vec>, @@ -37,6 +38,7 @@ impl Default for Env { fn default() -> Self { Self { scratch_state_idx: 0, + scratch_state_idx_inverse: 0, constraints: Vec::new(), lookups: Vec::new(), selector: None, @@ -62,6 +64,12 @@ impl InterpreterEnv for Env { MIPSColumn::ScratchState(scratch_idx) } + fn alloc_scratch_inverse(&mut self) -> Self::Position { + let scratch_idx = self.scratch_state_idx_inverse; + self.scratch_state_idx_inverse += 1; + MIPSColumn::ScratchStateInverse(scratch_idx) + } + type Variable = E; fn variable(&self, column: Self::Position) -> Self::Variable { diff --git a/o1vm/src/interpreters/mips/interpreter.rs b/o1vm/src/interpreters/mips/interpreter.rs index 8bf4f61d2e..d10b28eca2 100644 --- a/o1vm/src/interpreters/mips/interpreter.rs +++ b/o1vm/src/interpreters/mips/interpreter.rs @@ -172,6 +172,8 @@ pub trait InterpreterEnv { /// [crate::interpreters::mips::witness::SCRATCH_SIZE] fn alloc_scratch(&mut self) -> Self::Position; + fn alloc_scratch_inverse(&mut self) -> Self::Position; + type Variable: Clone + std::ops::Add + std::ops::Sub diff --git a/o1vm/src/interpreters/mips/witness.rs b/o1vm/src/interpreters/mips/witness.rs index c3936b38ce..1ea7e74a6e 100644 --- a/o1vm/src/interpreters/mips/witness.rs +++ b/o1vm/src/interpreters/mips/witness.rs @@ -115,6 +115,12 @@ impl InterpreterEnv for Env Self::Position { + let scratch_idx = self.scratch_state_idx_inverse; + self.scratch_state_idx_inverse += 1; + Column::ScratchStateInverse(scratch_idx) + } + type Variable = u64; fn variable(&self, _column: Self::Position) -> Self::Variable { @@ -915,6 +921,7 @@ impl Env { pub fn write_field_column(&mut self, column: Column, value: Fp) { match column { Column::ScratchState(idx) => self.scratch_state[idx] = value, + Column::ScratchStateInverse(idx) => self.scratch_state_inverse[idx] = value, Column::InstructionCounter => panic!("Cannot overwrite the column {:?}", column), Column::Selector(s) => self.selector = s, } From 52d5cd9f0248d4e16538840475240cca5365fa42 Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 09:11:08 +0100 Subject: [PATCH 03/15] o1vm/mips/witness: introduce and call reset_scratch_state_inverse --- o1vm/src/interpreters/mips/tests.rs | 1 + o1vm/src/interpreters/mips/witness.rs | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/o1vm/src/interpreters/mips/tests.rs b/o1vm/src/interpreters/mips/tests.rs index 3056fae7cd..1b212077d1 100644 --- a/o1vm/src/interpreters/mips/tests.rs +++ b/o1vm/src/interpreters/mips/tests.rs @@ -114,6 +114,7 @@ mod rtype { // that condition would generate an infinite loop instead) while dummy_env.registers.preimage_offset < total_length { dummy_env.reset_scratch_state(); + dummy_env.reset_scratch_state_inverse(); // Set maximum number of bytes to read in this call dummy_env.registers[6] = rng.gen_range(1..=4); diff --git a/o1vm/src/interpreters/mips/witness.rs b/o1vm/src/interpreters/mips/witness.rs index 1ea7e74a6e..6e2b950300 100644 --- a/o1vm/src/interpreters/mips/witness.rs +++ b/o1vm/src/interpreters/mips/witness.rs @@ -914,6 +914,11 @@ impl Env { self.selector = N_MIPS_SEL_COLS; } + pub fn reset_scratch_state_inverse(&mut self) { + self.scratch_state_idx_inverse = 0; + self.scratch_state_inverse = fresh_scratch_state(); + } + pub fn write_column(&mut self, column: Column, value: u64) { self.write_field_column(column, value.into()) } @@ -1156,6 +1161,7 @@ impl Env { start: &Start, ) -> Instruction { self.reset_scratch_state(); + self.reset_scratch_state_inverse(); let (opcode, _instruction) = self.decode_instruction(); self.pp_info(&config.info_at, metadata, start); From f108bf27bb257535971cf7e9446ae52117e10261 Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 09:11:39 +0100 Subject: [PATCH 04/15] o1vm/mips/constraints: reset scratch state idx inverse --- o1vm/src/interpreters/mips/constraints.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/o1vm/src/interpreters/mips/constraints.rs b/o1vm/src/interpreters/mips/constraints.rs index fa047f199a..7e97317bbd 100644 --- a/o1vm/src/interpreters/mips/constraints.rs +++ b/o1vm/src/interpreters/mips/constraints.rs @@ -631,6 +631,7 @@ impl InterpreterEnv for Env { fn reset(&mut self) { self.scratch_state_idx = 0; + self.scratch_state_idx_inverse = 0; self.constraints.clear(); self.lookups.clear(); self.selector = None; From 8f37429fa304a226dac4863b6dd8e0e1ff7a4b18 Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 10:43:18 +0100 Subject: [PATCH 05/15] o1vm/mips: introduce scratch_inverse in the proof --- o1vm/src/pickles/proof.rs | 7 ++++++- o1vm/src/pickles/prover.rs | 33 +++++++++++++++++++++++++++++++++ o1vm/src/pickles/tests.rs | 1 + o1vm/src/pickles/verifier.rs | 11 +++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/o1vm/src/pickles/proof.rs b/o1vm/src/pickles/proof.rs index e3548cdbfe..d98d04113c 100644 --- a/o1vm/src/pickles/proof.rs +++ b/o1vm/src/pickles/proof.rs @@ -1,10 +1,14 @@ use kimchi::{curve::KimchiCurve, proof::PointEvaluations}; use poly_commitment::{ipa::OpeningProof, PolyComm}; -use crate::interpreters::mips::{column::N_MIPS_SEL_COLS, witness::SCRATCH_SIZE}; +use crate::interpreters::mips::{ + column::N_MIPS_SEL_COLS, + witness::{SCRATCH_SIZE, SCRATCH_SIZE_INVERSE}, +}; pub struct WitnessColumns { pub scratch: [G; SCRATCH_SIZE], + pub scratch_inverse: [G; SCRATCH_SIZE_INVERSE], pub instruction_counter: G, pub error: G, pub selector: S, @@ -19,6 +23,7 @@ impl ProofInputs { ProofInputs { evaluations: WitnessColumns { scratch: std::array::from_fn(|_| Vec::with_capacity(domain_size)), + scratch_inverse: std::array::from_fn(|_| Vec::with_capacity(domain_size)), instruction_counter: Vec::with_capacity(domain_size), error: Vec::with_capacity(domain_size), selector: Vec::with_capacity(domain_size), diff --git a/o1vm/src/pickles/prover.rs b/o1vm/src/pickles/prover.rs index 2f40787546..dc0ac2dd08 100644 --- a/o1vm/src/pickles/prover.rs +++ b/o1vm/src/pickles/prover.rs @@ -84,6 +84,7 @@ where > = { let WitnessColumns { scratch, + scratch_inverse, instruction_counter, error, selector, @@ -110,9 +111,17 @@ where }; // Doing in parallel let scratch = scratch.into_par_iter().map(eval_col).collect::>(); + let scratch_inverse = scratch_inverse + .into_par_iter() + .map(|mut evals| { + ark_ff::batch_inversion(&mut evals); + eval_col(evals) + }) + .collect::>(); let selector = selector.into_par_iter().map(eval_col).collect::>(); WitnessColumns { scratch: scratch.try_into().unwrap(), + scratch_inverse: scratch_inverse.try_into().unwrap(), instruction_counter: eval_col(instruction_counter), error: eval_col(error.clone()), selector: selector.try_into().unwrap(), @@ -123,6 +132,7 @@ where let commitments: WitnessColumns, [PolyComm; N_MIPS_SEL_COLS]> = { let WitnessColumns { scratch, + scratch_inverse, instruction_counter, error, selector, @@ -139,9 +149,11 @@ where }; // Doing in parallel let scratch = scratch.par_iter().map(comm).collect::>(); + let scratch_inverse = scratch_inverse.par_iter().map(comm).collect::>(); let selector = selector.par_iter().map(comm).collect::>(); WitnessColumns { scratch: scratch.try_into().unwrap(), + scratch_inverse: scratch_inverse.try_into().unwrap(), instruction_counter: comm(instruction_counter), error: comm(error), selector: selector.try_into().unwrap(), @@ -156,6 +168,7 @@ where let evaluations_d8 = { let WitnessColumns { scratch, + scratch_inverse, instruction_counter, error, selector, @@ -164,9 +177,14 @@ where |poly: &DensePolynomial| poly.evaluate_over_domain_by_ref(domain.d8); // Doing in parallel let scratch = scratch.into_par_iter().map(eval_d8).collect::>(); + let scratch_inverse = scratch_inverse + .into_par_iter() + .map(eval_d8) + .collect::>(); let selector = selector.into_par_iter().map(eval_d8).collect::>(); WitnessColumns { scratch: scratch.try_into().unwrap(), + scratch_inverse: scratch_inverse.try_into().unwrap(), instruction_counter: eval_d8(instruction_counter), error: eval_d8(error), selector: selector.try_into().unwrap(), @@ -178,6 +196,9 @@ where for comm in commitments.scratch.iter() { absorb_commitment(&mut fq_sponge, comm) } + for comm in commitments.scratch_inverse.iter() { + absorb_commitment(&mut fq_sponge, comm) + } absorb_commitment(&mut fq_sponge, &commitments.instruction_counter); absorb_commitment(&mut fq_sponge, &commitments.error); for comm in commitments.selector.iter() { @@ -291,15 +312,18 @@ where let evals = |point| { let WitnessColumns { scratch, + scratch_inverse, instruction_counter, error, selector, } = &polys; let eval = |poly: &DensePolynomial| poly.evaluate(point); let scratch = scratch.par_iter().map(eval).collect::>(); + let scratch_inverse = scratch_inverse.par_iter().map(eval).collect::>(); let selector = selector.par_iter().map(eval).collect::>(); WitnessColumns { scratch: scratch.try_into().unwrap(), + scratch_inverse: scratch_inverse.try_into().unwrap(), instruction_counter: eval(instruction_counter), error: eval(error), selector: selector.try_into().unwrap(), @@ -342,6 +366,14 @@ where fr_sponge.absorb(zeta_eval); fr_sponge.absorb(zeta_omega_eval); } + for (zeta_eval, zeta_omega_eval) in zeta_evaluations + .scratch_inverse + .iter() + .zip(zeta_omega_evaluations.scratch_inverse.iter()) + { + fr_sponge.absorb(zeta_eval); + fr_sponge.absorb(zeta_omega_eval); + } fr_sponge.absorb(&zeta_evaluations.instruction_counter); fr_sponge.absorb(&zeta_omega_evaluations.instruction_counter); fr_sponge.absorb(&zeta_evaluations.error); @@ -367,6 +399,7 @@ where //////////////////////////////////////////////////////////////////////////// let mut polynomials: Vec<_> = polys.scratch.into_iter().collect(); + polynomials.extend(polys.scratch_inverse); polynomials.push(polys.instruction_counter); polynomials.push(polys.error); polynomials.extend(polys.selector); diff --git a/o1vm/src/pickles/tests.rs b/o1vm/src/pickles/tests.rs index f19c610400..474a42c44a 100644 --- a/o1vm/src/pickles/tests.rs +++ b/o1vm/src/pickles/tests.rs @@ -63,6 +63,7 @@ fn test_small_circuit() { let proof_input = ProofInputs:: { evaluations: WitnessColumns { scratch: std::array::from_fn(|_| zero_to_n_minus_one(8)), + scratch_inverse: std::array::from_fn(|_| zero_to_n_minus_one(8)), instruction_counter: zero_to_n_minus_one(8) .into_iter() .map(|x| x + Fq::one()) diff --git a/o1vm/src/pickles/verifier.rs b/o1vm/src/pickles/verifier.rs index 9057a89802..fbbaa912eb 100644 --- a/o1vm/src/pickles/verifier.rs +++ b/o1vm/src/pickles/verifier.rs @@ -96,6 +96,9 @@ where for comm in commitments.scratch.iter() { absorb_commitment(&mut fq_sponge, comm) } + for comm in commitments.scratch_inverse.iter() { + absorb_commitment(&mut fq_sponge, comm) + } absorb_commitment(&mut fq_sponge, &commitments.instruction_counter); absorb_commitment(&mut fq_sponge, &commitments.error); for comm in commitments.selector.iter() { @@ -137,6 +140,14 @@ where fr_sponge.absorb(zeta_eval); fr_sponge.absorb(zeta_omega_eval); } + for (zeta_eval, zeta_omega_eval) in zeta_evaluations + .scratch_inverse + .iter() + .zip(zeta_omega_evaluations.scratch_inverse.iter()) + { + fr_sponge.absorb(zeta_eval); + fr_sponge.absorb(zeta_omega_eval); + } fr_sponge.absorb(&zeta_evaluations.instruction_counter); fr_sponge.absorb(&zeta_omega_evaluations.instruction_counter); fr_sponge.absorb(&zeta_evaluations.error); From acc02530028d717ce96f7be6eb987497774a3dbd Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 11:01:11 +0100 Subject: [PATCH 06/15] o1vm/mips: use alloc_scratch_inverse for the inverse --- o1vm/src/interpreters/mips/witness.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/o1vm/src/interpreters/mips/witness.rs b/o1vm/src/interpreters/mips/witness.rs index 6e2b950300..04bbf6c021 100644 --- a/o1vm/src/interpreters/mips/witness.rs +++ b/o1vm/src/interpreters/mips/witness.rs @@ -333,13 +333,13 @@ impl InterpreterEnv for Env InterpreterEnv for Env Date: Thu, 21 Nov 2024 13:59:43 +0100 Subject: [PATCH 07/15] o1vm/column: increase the total number of columns incl. inverses --- o1vm/src/interpreters/mips/column.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/o1vm/src/interpreters/mips/column.rs b/o1vm/src/interpreters/mips/column.rs index d6e018b0b0..b42b23cafd 100644 --- a/o1vm/src/interpreters/mips/column.rs +++ b/o1vm/src/interpreters/mips/column.rs @@ -35,7 +35,7 @@ pub(crate) const MIPS_CHUNK_BYTES_LEN: usize = 4; pub(crate) const MIPS_PREIMAGE_KEY: usize = 97; /// The number of columns used for relation witness in the MIPS circuit -pub const N_MIPS_REL_COLS: usize = SCRATCH_SIZE + 2; +pub const N_MIPS_REL_COLS: usize = SCRATCH_SIZE + SCRATCH_SIZE_INVERSE + 2; /// The number of witness columns used to store the instruction selectors. pub const N_MIPS_SEL_COLS: usize = From dcf661cc6f752004bf596a011821ea1884d91f0a Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 14:00:32 +0100 Subject: [PATCH 08/15] o1vm/column: add error message when assert is false --- o1vm/src/interpreters/mips/column.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/o1vm/src/interpreters/mips/column.rs b/o1vm/src/interpreters/mips/column.rs index b42b23cafd..ea4f55e9d4 100644 --- a/o1vm/src/interpreters/mips/column.rs +++ b/o1vm/src/interpreters/mips/column.rs @@ -139,20 +139,36 @@ impl IndexMut for MIPSWitness { impl ColumnIndexer for ColumnAlias { const N_COL: usize = N_MIPS_COLS; + fn to_column(self) -> Column { match self { Self::ScratchState(ss) => { - assert!(ss < SCRATCH_SIZE); + assert!( + ss < SCRATCH_SIZE, + "The maximum index is {}, got {}", + SCRATCH_SIZE, + ss + ); Column::Relation(ss) } Self::ScratchStateInverse(ss) => { - assert!(ss < SCRATCH_SIZE_INVERSE); + assert!( + ss < SCRATCH_SIZE_INVERSE, + "The maximum index is {}, got {}", + SCRATCH_SIZE_INVERSE, + ss + ); Column::Relation(SCRATCH_SIZE + ss) } Self::InstructionCounter => Column::Relation(SCRATCH_SIZE + SCRATCH_SIZE_INVERSE), // TODO: what happens with error? It does not have a corresponding alias Self::Selector(s) => { - assert!(s < N_MIPS_SEL_COLS); + assert!( + s < N_MIPS_SEL_COLS, + "The maximum index is {}, got {}", + N_MIPS_SEL_COLS, + s + ); Column::DynamicSelector(s) } } From ac0380be78cd3f6b45b1ec763bcd72153e497044 Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 14:01:16 +0100 Subject: [PATCH 09/15] o1vm/mips: simplify by removing unused variable --- o1vm/src/interpreters/mips/witness.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/o1vm/src/interpreters/mips/witness.rs b/o1vm/src/interpreters/mips/witness.rs index 04bbf6c021..ee170bbc0f 100644 --- a/o1vm/src/interpreters/mips/witness.rs +++ b/o1vm/src/interpreters/mips/witness.rs @@ -354,15 +354,12 @@ impl InterpreterEnv for Env Date: Thu, 21 Nov 2024 14:02:21 +0100 Subject: [PATCH 10/15] o1vm/pickles: improve error message when panicking when picking col --- o1vm/src/pickles/column_env.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/o1vm/src/pickles/column_env.rs b/o1vm/src/pickles/column_env.rs index c66f68160f..d333533a39 100644 --- a/o1vm/src/pickles/column_env.rs +++ b/o1vm/src/pickles/column_env.rs @@ -3,7 +3,10 @@ use ark_poly::{Evaluations, Radix2EvaluationDomain}; use kimchi_msm::columns::Column; use crate::{ - interpreters::mips::{column::N_MIPS_SEL_COLS, witness::SCRATCH_SIZE}, + interpreters::mips::{ + column::N_MIPS_SEL_COLS, + witness::{SCRATCH_SIZE, SCRATCH_SIZE_INVERSE}, + }, pickles::proof::WitnessColumns, }; use kimchi::circuits::{ @@ -60,19 +63,24 @@ impl WitnessColumns { let res = &self.error; Some(res) } else { - panic!("We should not have that many relation columns"); + panic!("We should not have that many relation columns. We have {} columns and index {} was given", SCRATCH_SIZE + SCRATCH_SIZE_INVERSE + 2, i); } } Column::DynamicSelector(i) => { assert!( i < N_MIPS_SEL_COLS, - "We do not have that many dynamic selector columns" + "We do not have that many dynamic selector columns. We have {} columns and index {} was given", + N_MIPS_SEL_COLS, + i ); let res = &self.selector[i]; Some(res) } _ => { - panic!("We should not have any other type of columns") + panic!( + "We should not have any other type of columns. The column {:?} was given", + col + ); } } } From c65b14a6a02c188d237f77dfdc1646f69ae77f24 Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 14:03:58 +0100 Subject: [PATCH 11/15] o1vm/pickles: incl scratch_size_inverse selection in get_all_columns --- o1vm/src/pickles/column_env.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/o1vm/src/pickles/column_env.rs b/o1vm/src/pickles/column_env.rs index d333533a39..8e8b027b08 100644 --- a/o1vm/src/pickles/column_env.rs +++ b/o1vm/src/pickles/column_env.rs @@ -39,8 +39,9 @@ pub struct ColumnEnvironment<'a, F: FftField> { } pub fn get_all_columns() -> Vec { - let mut cols = Vec::::with_capacity(SCRATCH_SIZE + 2 + N_MIPS_SEL_COLS); - for i in 0..SCRATCH_SIZE + 2 { + let mut cols = + Vec::::with_capacity(SCRATCH_SIZE + SCRATCH_SIZE_INVERSE + 2 + N_MIPS_SEL_COLS); + for i in 0..SCRATCH_SIZE + SCRATCH_SIZE_INVERSE + 2 { cols.push(Column::Relation(i)); } for i in 0..N_MIPS_SEL_COLS { @@ -56,10 +57,13 @@ impl WitnessColumns { if i < SCRATCH_SIZE { let res = &self.scratch[i]; Some(res) - } else if i == SCRATCH_SIZE { + } else if i < SCRATCH_SIZE + SCRATCH_SIZE_INVERSE { + let res = &self.scratch_inverse[i - SCRATCH_SIZE]; + Some(res) + } else if i == SCRATCH_SIZE + SCRATCH_SIZE_INVERSE { let res = &self.instruction_counter; Some(res) - } else if i == SCRATCH_SIZE + 1 { + } else if i == SCRATCH_SIZE + SCRATCH_SIZE_INVERSE + 1 { let res = &self.error; Some(res) } else { From 3071eabb9d181498a708a5a96359136f985b3ac0 Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 14:34:20 +0100 Subject: [PATCH 12/15] o1vm/pickles: regtest reg. arkworks batch inversion Checking as a regression tests that the batch inversion works with zeroes --- o1vm/src/pickles/tests.rs | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/o1vm/src/pickles/tests.rs b/o1vm/src/pickles/tests.rs index 474a42c44a..0c772d1d91 100644 --- a/o1vm/src/pickles/tests.rs +++ b/o1vm/src/pickles/tests.rs @@ -11,7 +11,7 @@ use crate::{ }, pickles::{verifier::verify, MAXIMUM_DEGREE_CONSTRAINTS, TOTAL_NUMBER_OF_CONSTRAINTS}, }; -use ark_ff::{One, Zero}; +use ark_ff::{Field, One, UniformRand, Zero}; use kimchi::circuits::{domains::EvaluationDomains, expr::Expr, gate::CurrOrNext}; use kimchi_msm::{columns::Column, expr::E}; use log::debug; @@ -101,3 +101,31 @@ fn test_small_circuit() { ); assert!(verif, "Verification fails"); } + +#[test] +fn test_arkworks_batch_inversion_with_only_zeroes() { + let input = vec![Fq::zero(); 8]; + let exp_output = vec![Fq::zero(); 8]; + let mut output = input.clone(); + ark_ff::batch_inversion::(&mut output); + assert_eq!(output, exp_output); +} + +#[test] +fn test_arkworks_batch_inversion_with_zeroes_and_ones() { + let input: Vec = vec![Fq::zero(), Fq::one(), Fq::zero()]; + let exp_output: Vec = vec![Fq::zero(), Fq::one(), Fq::zero()]; + let mut output: Vec = input.clone(); + ark_ff::batch_inversion::(&mut output); + assert_eq!(output, exp_output); +} + +#[test] +fn test_arkworks_batch_inversion_with_zeroes_and_random() { + let mut rng = o1_utils::tests::make_test_rng(None); + let input: Vec = vec![Fq::zero(), Fq::rand(&mut rng), Fq::one()]; + let exp_output: Vec = vec![Fq::zero(), input[1].inverse().unwrap(), Fq::one()]; + let mut output: Vec = input.clone(); + ark_ff::batch_inversion::(&mut output); + assert_eq!(output, exp_output); +} From f4b52a7578725c27d7534f990d15d08260ccf48d Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 14:49:54 +0100 Subject: [PATCH 13/15] o1vm/pickles: add scratch_state_inverse in the proof inputs --- o1vm/src/pickles/main.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/o1vm/src/pickles/main.rs b/o1vm/src/pickles/main.rs index f60ae5948f..5083d7d58d 100644 --- a/o1vm/src/pickles/main.rs +++ b/o1vm/src/pickles/main.rs @@ -105,6 +105,13 @@ pub fn main() -> ExitCode { { scratch_chunk.push(*scratch); } + for (scratch, scratch_chunk) in mips_wit_env + .scratch_state_inverse + .iter() + .zip(curr_proof_inputs.evaluations.scratch_inverse.iter_mut()) + { + scratch_chunk.push(*scratch); + } curr_proof_inputs .evaluations .instruction_counter From be87dce544b90e512d328c8895a98ee0265686f8 Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Thu, 21 Nov 2024 14:52:24 +0100 Subject: [PATCH 14/15] o1vm/mips: allocate correctly the inverse column --- o1vm/src/interpreters/mips/constraints.rs | 2 +- o1vm/src/interpreters/mips/witness.rs | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/o1vm/src/interpreters/mips/constraints.rs b/o1vm/src/interpreters/mips/constraints.rs index 7e97317bbd..cb46508b9b 100644 --- a/o1vm/src/interpreters/mips/constraints.rs +++ b/o1vm/src/interpreters/mips/constraints.rs @@ -227,7 +227,7 @@ impl InterpreterEnv for Env { unsafe { self.test_zero(x, pos) } }; let x_inv_or_zero = { - let pos = self.alloc_scratch(); + let pos = self.alloc_scratch_inverse(); self.variable(pos) }; // If x = 0, then res = 1 and x_inv_or_zero = 0 diff --git a/o1vm/src/interpreters/mips/witness.rs b/o1vm/src/interpreters/mips/witness.rs index ee170bbc0f..38eba183bb 100644 --- a/o1vm/src/interpreters/mips/witness.rs +++ b/o1vm/src/interpreters/mips/witness.rs @@ -329,15 +329,15 @@ impl InterpreterEnv for Env Self::Variable { // write the result - let pos = self.alloc_scratch(); - let res = if *x == 0 { 1 } else { 0 }; - self.write_column(pos, res); + let res = { + let pos = self.alloc_scratch(); + unsafe { self.test_zero(x, pos) } + }; // write the non deterministic advice inv_or_zero + let pos = self.alloc_scratch_inverse(); if *x == 0 { - let pos = self.alloc_scratch(); self.write_field_column(pos, Fp::zero()); } else { - let pos = self.alloc_scratch_inverse(); self.write_field_column(pos, Fp::from(*x)); }; // return the result @@ -354,11 +354,10 @@ impl InterpreterEnv for Env Date: Thu, 21 Nov 2024 17:24:11 +0100 Subject: [PATCH 15/15] o1vm/mips: fix tests related to the new inverse scratch state --- o1vm/src/interpreters/mips/column.rs | 4 +++- o1vm/src/pickles/tests.rs | 9 ++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/o1vm/src/interpreters/mips/column.rs b/o1vm/src/interpreters/mips/column.rs index ea4f55e9d4..88cd6cc68a 100644 --- a/o1vm/src/interpreters/mips/column.rs +++ b/o1vm/src/interpreters/mips/column.rs @@ -9,7 +9,9 @@ use kimchi_msm::{ use std::ops::{Index, IndexMut}; use strum::EnumCount; -use super::{witness::SCRATCH_SIZE_INVERSE, ITypeInstruction, JTypeInstruction, RTypeInstruction}; +pub use super::{ + witness::SCRATCH_SIZE_INVERSE, ITypeInstruction, JTypeInstruction, RTypeInstruction, +}; /// The number of hashes performed so far in the block pub(crate) const MIPS_HASH_COUNTER_OFF: usize = 80; diff --git a/o1vm/src/pickles/tests.rs b/o1vm/src/pickles/tests.rs index 0c772d1d91..e5c80f8f4b 100644 --- a/o1vm/src/pickles/tests.rs +++ b/o1vm/src/pickles/tests.rs @@ -7,7 +7,10 @@ use super::{ }; use crate::{ interpreters::mips::{ - constraints as mips_constraints, interpreter, interpreter::InterpreterEnv, Instruction, + column::SCRATCH_SIZE_INVERSE, + constraints as mips_constraints, + interpreter::{self, InterpreterEnv}, + Instruction, }, pickles::{verifier::verify, MAXIMUM_DEGREE_CONSTRAINTS, TOTAL_NUMBER_OF_CONSTRAINTS}, }; @@ -63,7 +66,7 @@ fn test_small_circuit() { let proof_input = ProofInputs:: { evaluations: WitnessColumns { scratch: std::array::from_fn(|_| zero_to_n_minus_one(8)), - scratch_inverse: std::array::from_fn(|_| zero_to_n_minus_one(8)), + scratch_inverse: std::array::from_fn(|_| (0..8).map(|_| Fq::zero()).collect()), instruction_counter: zero_to_n_minus_one(8) .into_iter() .map(|x| x + Fq::one()) @@ -75,7 +78,7 @@ fn test_small_circuit() { }, }; let mut expr = Expr::zero(); - for i in 0..SCRATCH_SIZE + 2 { + for i in 0..SCRATCH_SIZE + SCRATCH_SIZE_INVERSE + 2 { expr += Expr::cell(Column::Relation(i), CurrOrNext::Curr); } let mut rng = make_test_rng(None);