Merge branch 'master' into o1vm/riscv32im/test-decoding-sub

ivc/src/prover.rs

@@ -31,8 +31,8 @@ use mina_poseidon::{sponge::ScalarChallenge, FqSponge};
 use o1_utils::ExtendedDensePolynomial;
 use poly_commitment::{
     commitment::{absorb_commitment, CommitmentCurve, PolyComm},
-    ipa::DensePolynomialOrEvaluations,
     kzg::{KZGProof, PairingSRS},
+    utils::DensePolynomialOrEvaluations,
     OpenProof, SRS,
 };
 use rand::{CryptoRng, RngCore};
@@ -437,11 +437,11 @@ where
     let u = u_chal.to_field(endo_r);
 
     let coefficients_form = DensePolynomialOrEvaluations::DensePolynomial;
-    let non_hiding = |d1_size| PolyComm {
-        chunks: vec![Fp::zero(); d1_size],
+    let non_hiding = |n_chunks| PolyComm {
+        chunks: vec![Fp::zero(); n_chunks],
     };
-    let hiding = |d1_size| PolyComm {
-        chunks: vec![Fp::one(); d1_size],
+    let hiding = |n_chunks| PolyComm {
+        chunks: vec![Fp::one(); n_chunks],
     };
 
     // Gathering all polynomials_to_open to use in the opening proof

kimchi/src/prover.rs

@@ -46,7 +46,7 @@ use poly_commitment::{
     commitment::{
         absorb_commitment, b_poly_coefficients, BlindedCommitment, CommitmentCurve, PolyComm,
     },
-    ipa::DensePolynomialOrEvaluations,
+    utils::DensePolynomialOrEvaluations,
     OpenProof, SRS as _,
 };
 use rand_core::{CryptoRng, RngCore};
@@ -1238,22 +1238,22 @@ where
         //~ 1. Create a list of all polynomials that will require evaluations
         //~    (and evaluation proofs) in the protocol.
         //~    First, include the previous challenges, in case we are in a recursive prover.
-        let non_hiding = |d1_size: usize| PolyComm {
-            chunks: vec![G::ScalarField::zero(); d1_size],
+        let non_hiding = |n_chunks: usize| PolyComm {
+            chunks: vec![G::ScalarField::zero(); n_chunks],
+        };
+
+        let fixed_hiding = |n_chunks: usize| PolyComm {
+            chunks: vec![G::ScalarField::one(); n_chunks],
         };
 
         let coefficients_form = DensePolynomialOrEvaluations::DensePolynomial;
         let evaluations_form = |e| DensePolynomialOrEvaluations::Evaluations(e, index.cs.domain.d1);
 
         let mut polynomials = polys
             .iter()
-            .map(|(p, d1_size)| (coefficients_form(p), non_hiding(*d1_size)))
+            .map(|(p, n_chunks)| (coefficients_form(p), non_hiding(*n_chunks)))
             .collect::<Vec<_>>();
 
-        let fixed_hiding = |d1_size: usize| PolyComm {
-            chunks: vec![G::ScalarField::one(); d1_size],
-        };
-
         //~ 1. Then, include:
         //~~ * the negated public polynomial
         //~~ * the ft polynomial

msm/src/prover.rs

@@ -30,7 +30,7 @@ use mina_poseidon::{sponge::ScalarChallenge, FqSponge};
 use o1_utils::ExtendedDensePolynomial;
 use poly_commitment::{
     commitment::{absorb_commitment, PolyComm},
-    ipa::DensePolynomialOrEvaluations,
+    utils::DensePolynomialOrEvaluations,
     OpenProof, SRS,
 };
 use rand::{CryptoRng, RngCore};
@@ -490,11 +490,11 @@ where
     let u = u_chal.to_field(endo_r);
 
     let coefficients_form = DensePolynomialOrEvaluations::DensePolynomial;
-    let non_hiding = |d1_size| PolyComm {
-        chunks: vec![G::ScalarField::zero(); d1_size],
+    let non_hiding = |n_chunks| PolyComm {
+        chunks: vec![G::ScalarField::zero(); n_chunks],
     };
-    let hiding = |d1_size| PolyComm {
-        chunks: vec![G::ScalarField::one(); d1_size],
+    let hiding = |n_chunks| PolyComm {
+        chunks: vec![G::ScalarField::one(); n_chunks],
     };
 
     // Gathering all polynomials to use in the opening proof

o1vm/src/interpreters/mips/column.rs

@@ -1,7 +1,4 @@
-use crate::interpreters::mips::{
-    witness::SCRATCH_SIZE,
-    Instruction::{self, IType, JType, RType},
-};
+use crate::interpreters::mips::Instruction::{self, IType, JType, RType};
 use kimchi_msm::{
     columns::{Column, ColumnIndexer},
     witness::Witness,
@@ -11,31 +8,40 @@ use strum::EnumCount;
 
 use super::{ITypeInstruction, JTypeInstruction, RTypeInstruction};
 
+pub(crate) const SCRATCH_SIZE_WITHOUT_KECCAK: usize = 45;
 /// The number of hashes performed so far in the block
-pub(crate) const MIPS_HASH_COUNTER_OFF: usize = 80;
+pub(crate) const MIPS_HASH_COUNTER_OFF: usize = SCRATCH_SIZE_WITHOUT_KECCAK;
 /// The number of bytes of the preimage that have been read so far in this hash
-pub(crate) const MIPS_BYTE_COUNTER_OFF: usize = 81;
+pub(crate) const MIPS_BYTE_COUNTER_OFF: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 1;
 /// A flag indicating whether the preimage has been read fully or not
-pub(crate) const MIPS_END_OF_PREIMAGE_OFF: usize = 82;
+pub(crate) const MIPS_END_OF_PREIMAGE_OFF: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 2;
 /// The number of preimage bytes processed in this step
-pub(crate) const MIPS_NUM_BYTES_READ_OFF: usize = 83;
+pub(crate) const MIPS_NUM_BYTES_READ_OFF: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 3;
 /// The at most 4-byte chunk of the preimage that has been read in this step.
 /// Contains a field element of at most 4 bytes.
-pub(crate) const MIPS_PREIMAGE_CHUNK_OFF: usize = 84;
+pub(crate) const MIPS_PREIMAGE_CHUNK_OFF: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 4;
 /// The at most 4-bytes of the preimage that are currently being processed
 /// Consists of 4 field elements of at most 1 byte each.
-pub(crate) const MIPS_PREIMAGE_BYTES_OFF: usize = 85;
+pub(crate) const MIPS_PREIMAGE_BYTES_OFF: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 5;
 /// The at most 4-bytes of the length that are currently being processed
-pub(crate) const MIPS_LENGTH_BYTES_OFF: usize = 89;
+pub(crate) const MIPS_LENGTH_BYTES_OFF: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 5 + 4;
 /// Flags indicating whether at least N bytes have been processed in this step
-pub(crate) const MIPS_HAS_N_BYTES_OFF: usize = 93;
+pub(crate) const MIPS_HAS_N_BYTES_OFF: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 5 + 4 + 4;
 /// The maximum size of a chunk (4 bytes)
 pub(crate) const MIPS_CHUNK_BYTES_LEN: usize = 4;
 /// The location of the preimage key as a field element of 248bits
-pub(crate) const MIPS_PREIMAGE_KEY: usize = 97;
+pub(crate) const MIPS_PREIMAGE_KEY: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 5 + 4 + 4 + 4;
+
+// MIPS + hash_counter + byte_counter + eof + num_bytes_read + chunk + bytes
+// + length + has_n_bytes + chunk_bytes + preimage
+pub const SCRATCH_SIZE: usize = SCRATCH_SIZE_WITHOUT_KECCAK + 5 + 4 + 4 + 4 + 1;
+
+/// Number of columns used by the MIPS interpreter to keep values to be
+/// inverted.
+pub const SCRATCH_SIZE_INVERSE: usize = 12;
 
 /// 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 =
@@ -50,6 +56,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 +75,12 @@ impl From<ColumnAlias> 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,
         }
     }
 }
@@ -132,16 +145,36 @@ impl<T: Clone> IndexMut<ColumnAlias> for MIPSWitness<T> {
 
 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::InstructionCounter => Column::Relation(SCRATCH_SIZE),
+            Self::ScratchStateInverse(ss) => {
+                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)
             }
         }

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<Fp> {
     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<E<Fp>>,
@@ -37,6 +38,7 @@ impl<Fp: Field> Default for Env<Fp> {
     fn default() -> Self {
         Self {
             scratch_state_idx: 0,
+            scratch_state_idx_inverse: 0,
             constraints: Vec::new(),
             lookups: Vec::new(),
             selector: None,
@@ -52,16 +54,22 @@ impl<Fp: Field> InterpreterEnv for Env<Fp> {
 
     // Use one of the available columns. It won't create a new column every time
     // this function is called. The number of columns is defined upfront by
-    // crate::mips::witness::SCRATCH_SIZE.
+    // crate::interpreters::mips::column::SCRATCH_SIZE.
     fn alloc_scratch(&mut self) -> Self::Position {
         // All columns are implemented using a simple index, and a name is given
-        // to the index. See crate::SCRATCH_SIZE for the maximum number of
+        // to the index. See crate::interpreters::mips::column::SCRATCH_SIZE for the maximum number of
         // columns the circuit can use.
         let scratch_idx = self.scratch_state_idx;
         self.scratch_state_idx += 1;
         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<Fp>;
 
     fn variable(&self, column: Self::Position) -> Self::Variable {
@@ -219,8 +227,8 @@ impl<Fp: Field> InterpreterEnv for Env<Fp> {
             unsafe { self.test_zero(x, pos) }
         };
         let x_inv_or_zero = {
-            let pos = self.alloc_scratch();
-            unsafe { self.inverse_or_zero(x, pos) }
+            let pos = self.alloc_scratch_inverse();
+            self.variable(pos)
         };
         // If x = 0, then res = 1 and x_inv_or_zero = 0
         // If x <> 0, then res = 0 and x_inv_or_zero = x^(-1)
@@ -229,14 +237,6 @@ impl<Fp: Field> InterpreterEnv for Env<Fp> {
         res
     }
 
-    unsafe fn inverse_or_zero(
-        &mut self,
-        _x: &Self::Variable,
-        position: Self::Position,
-    ) -> Self::Variable {
-        self.variable(position)
-    }
-
     fn equal(&mut self, x: &Self::Variable, y: &Self::Variable) -> Self::Variable {
         self.is_zero(&(x.clone() - y.clone()))
     }
@@ -631,6 +631,7 @@ impl<Fp: Field> InterpreterEnv for Env<Fp> {
 
     fn reset(&mut self) {
         self.scratch_state_idx = 0;
+        self.scratch_state_idx_inverse = 0;
         self.constraints.clear();
         self.lookups.clear();
         self.selector = None;

o1vm/src/interpreters/mips/interpreter.rs

@@ -166,12 +166,14 @@ pub trait InterpreterEnv {
     /// The variables are "freed" after each step/instruction.
     /// The variable allocation can be seen as an allocation on a stack that is
     /// popped after each step execution.
-    /// At the moment, [crate::interpreters::mips::witness::SCRATCH_SIZE - 46]
+    /// At the moment, [crate::interpreters::mips::column::SCRATCH_SIZE - 46]
     /// elements can be allocated. If more temporary variables are required for
     /// an instruction, increase the value
-    /// [crate::interpreters::mips::witness::SCRATCH_SIZE]
+    /// [crate::interpreters::mips::column::SCRATCH_SIZE]
     fn alloc_scratch(&mut self) -> Self::Position;
 
+    fn alloc_scratch_inverse(&mut self) -> Self::Position;
+
     type Variable: Clone
         + std::ops::Add<Self::Variable, Output = Self::Variable>
         + std::ops::Sub<Self::Variable, Output = Self::Variable>
@@ -683,21 +685,6 @@ pub trait InterpreterEnv {
     /// `x`.
     unsafe fn test_zero(&mut self, x: &Self::Variable, position: Self::Position) -> Self::Variable;
 
-    /// Returns `x^(-1)`, or `0` if `x` is `0`, storing the result in `position`.
-    ///
-    /// # Safety
-    ///
-    /// There are no constraints on the returned value; callers must assert the relationship with
-    /// `x`.
-    ///
-    /// The value returned may be a placeholder; callers should be careful not to depend directly
-    /// on the value stored in the variable.
-    unsafe fn inverse_or_zero(
-        &mut self,
-        x: &Self::Variable,
-        position: Self::Position,
-    ) -> Self::Variable;
-
     fn is_zero(&mut self, x: &Self::Variable) -> Self::Variable;
 
     /// Returns 1 if `x` is equal to `y`, or 0 otherwise, storing the result in `position`.

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);

o1vm/src/interpreters/mips/tests_helpers.rs

@@ -3,7 +3,7 @@ use crate::{
     interpreters::mips::{
         interpreter::{debugging::InstructionParts, InterpreterEnv},
         registers::Registers,
-        witness::{Env as WEnv, SyscallEnv, SCRATCH_SIZE},
+        witness::{Env as WEnv, SyscallEnv},
     },
     preimage_oracle::PreImageOracleT,
 };
@@ -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::column::{SCRATCH_SIZE, 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