Merge pull request #3274 from o1-labs/al/update-commitment

Saffron: add update commitment logic

Author: anne-laure-s

saffron/src/blob.rs

@@ -45,7 +45,7 @@ impl FieldBlob {
         domain: &Radix2EvaluationDomain<ScalarField>,
         diff: &Diff<ScalarField>,
     ) {
-        assert!(diff.addresses.len() == diff.new_values.len());
+        assert_eq!(diff.addresses.len(), diff.diff_values.len());
 
         let lagrange_basis = srs
             .get_lagrange_basis(*domain)
@@ -60,28 +60,13 @@ impl FieldBlob {
             .map(|idx| basis[*idx as usize])
             .collect();
 
-        // Old values at `addresses`
-        let old_values_at_addr: Vec<_> = diff
-            .addresses
-            .iter()
-            .map(|idx| self.data[diff.region as usize * SRS_SIZE + *idx as usize])
-            .collect();
-
-        for (idx, value) in diff.addresses.iter().zip(diff.new_values.iter()) {
-            self.data[SRS_SIZE * diff.region as usize + *idx as usize] = *value;
+        for (idx, value) in diff.addresses.iter().zip(diff.diff_values.iter()) {
+            self.data[SRS_SIZE * diff.region as usize + *idx as usize] += *value;
         }
 
         // Lagrange commitment to the (new values-old values) at `addresses`
-        let delta_data_commitment_at_addr = ProjectiveCurve::msm(
-            address_basis.as_slice(),
-            old_values_at_addr
-                .iter()
-                .zip(diff.new_values.iter())
-                .map(|(old, new)| new - old)
-                .collect::<Vec<_>>()
-                .as_slice(),
-        )
-        .unwrap();
+        let delta_data_commitment_at_addr =
+            ProjectiveCurve::msm(address_basis.as_slice(), diff.diff_values.as_slice()).unwrap();
 
         let new_commitment =
             (self.commitments[diff.region as usize] + delta_data_commitment_at_addr).into();

saffron/src/commitment.rs

@@ -1,12 +1,13 @@
+use crate::{diff::Diff, utils};
 use ark_ec::{AffineRepr, CurveGroup, VariableBaseMSM};
 use kimchi::curve::KimchiCurve;
 use mina_poseidon::FqSponge;
 use poly_commitment::{ipa::SRS, SRS as _};
 use rayon::prelude::*;
 use tracing::instrument;
 
-use crate::utils;
-
+/// Compute the commitment to `data` ; if the length of `data` is greater than
+/// `SRS_SIZE`, the data is splitted in chunks of at most `SRS_SIZE` length.
 #[instrument(skip_all, level = "debug")]
 pub fn commit_to_field_elems<G: KimchiCurve>(srs: &SRS<G>, data: &[G::ScalarField]) -> Vec<G>
 where
@@ -48,3 +49,46 @@ pub fn combine_commitments<G: AffineRepr, EFqSponge: FqSponge<G::BaseField, G, G
     let combined_data_commitment = utils::aggregate_commitments(alpha, commitments);
     (combined_data_commitment, alpha)
 }
+
+/// A commitment that represent a whole data
+/// TODO: for now, we consider 1 commitment = 1 contract = 1 data
+/// This type may be redundant with other types in Proof-systems
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Commitment<G: KimchiCurve> {
+    pub cm: G,
+}
+
+impl<G: KimchiCurve> From<G> for Commitment<G> {
+    fn from(cm: G) -> Self {
+        Self { cm }
+    }
+}
+
+impl<G: KimchiCurve> Commitment<G> {
+    /// Commit a `data` of length smaller than `SRS_SIZE`
+    /// If greater data is provided, anything above `SRS_SIZE` is ignored
+    pub fn from_data(srs: &SRS<G>, data: &[G::ScalarField]) -> Commitment<G> {
+        Commitment {
+            cm: commit_to_field_elems::<G>(srs, data)[0],
+        }
+    }
+
+    /// TODO: This only handle the single commitment version for now
+    /// This function update the given commitment based on the given diff. The
+    /// returned commitment correspond to the data for the given commitment updated
+    /// according to the diff.
+    /// This function is tested in storage.rs
+    pub fn update(&self, srs: &SRS<G>, diff: Diff<G::ScalarField>) -> Commitment<G> {
+        // TODO: precompute this, or cache it & compute it in a lazy way ; it feels like it’s already cached but I’m not sure
+        let basis: Vec<G> = srs
+            .get_lagrange_basis_from_domain_size(crate::SRS_SIZE)
+            .iter()
+            .map(|x| x.chunks[0])
+            .collect();
+        let basis: Vec<G> = diff.addresses.iter().map(|&i| basis[i as usize]).collect();
+        let cm_diff = G::Group::msm(&basis, &diff.diff_values).unwrap();
+        Commitment {
+            cm: self.cm.add(cm_diff).into(),
+        }
+    }
+}

saffron/src/diff.rs

@@ -13,8 +13,8 @@ pub struct Diff<F: PrimeField> {
     pub region: u64,
     /// A list of unique addresses, each ∈ [0, SRS_SIZE]
     pub addresses: Vec<u64>,
-    /// A list of new values, each corresponding to address in `addresses`
-    pub new_values: Vec<F>,
+    /// A list of `new_value - old_values`, each corresponding to address in `addresses`
+    pub diff_values: Vec<F>,
 }
 
 #[derive(Debug, Error, Clone, PartialEq)]
@@ -44,19 +44,19 @@ impl<F: PrimeField> Diff<F> {
             .enumerate()
             .filter_map(|(region, (o, n))| {
                 let mut addresses: Vec<u64> = vec![];
-                let mut new_values: Vec<F> = vec![];
+                let mut diff_values: Vec<F> = vec![];
                 for (index, (o_elem, n_elem)) in o.iter().zip(n.iter()).enumerate() {
                     if o_elem != n_elem {
                         addresses.push(index as u64);
-                        new_values.push(*n_elem);
+                        diff_values.push(*n_elem - *o_elem);
                     }
                 }
 
                 if !addresses.is_empty() {
                     Some(Diff {
                         region: region as u64,
                         addresses,
-                        new_values,
+                        diff_values,
                     })
                 } else {
                     // do not record a diff with empty changes
@@ -82,8 +82,8 @@ impl<F: PrimeField> Diff<F> {
     /// Updates the data with the provided diff, replacing old values at
     /// specified addresses by corresponding new ones
     pub fn apply_inplace(data: &mut [Vec<F>], diff: &Diff<F>) {
-        for (addr, new_value) in diff.addresses.iter().zip(diff.new_values.iter()) {
-            data[diff.region as usize][*addr as usize] = *new_value;
+        for (addr, diff_value) in diff.addresses.iter().zip(diff.diff_values.iter()) {
+            data[diff.region as usize][*addr as usize] += *diff_value;
         }
     }
 

saffron/src/storage.rs

@@ -60,8 +60,8 @@ impl<F: PrimeField> Data<F> {
 
     /// Commit a `data` of length smaller than `SRS_SIZE`
     /// If greater data is provided, anything above `SRS_SIZE` is ignored
-    pub fn to_commitment<G: KimchiCurve<ScalarField = F>>(&self, srs: &SRS<G>) -> G {
-        commit_to_field_elems::<G>(srs, &self.data)[0]
+    pub fn to_commitment<G: KimchiCurve<ScalarField = F>>(&self, srs: &SRS<G>) -> Commitment<G> {
+        Commitment::from_data(srs, &self.data)
     }
 
     /// Modifies inplace the provided data with `diff`
@@ -111,23 +111,45 @@ pub fn read<F: PrimeField>(path: &str) -> std::io::Result<Data<F>> {
 /// new scalar value expected for the new data.
 /// Note that this only update the file, not the commitment
 pub fn update<F: PrimeField>(path: &str, diff: &Diff<F>) -> std::io::Result<()> {
-    let mut file = OpenOptions::new().write(true).open(path)?;
+    // Open the file in read mode to get the old value & write mode to write the new value
+    let mut file = OpenOptions::new().read(true).write(true).open(path)?;
     let region_offset = diff.region * (SRS_SIZE as u64);
     let scalar_size = encoding::encoding_size_full::<F>() as u64;
-    for (index, new_value) in diff.addresses.iter().zip(diff.new_values.iter()) {
+    for (index, diff_value) in diff.addresses.iter().zip(diff.diff_values.iter()) {
         let corresponding_bytes_index = (region_offset + index) * scalar_size;
         file.seek(SeekFrom::Start(corresponding_bytes_index))?;
-        let new_value_bytes = encoding::decode_full(*new_value);
+        let new_value: F = {
+            // The old value is taken directly from the file
+            let old_value: F = {
+                // Save the current cursor position to be able to reset the
+                // cursor after the read later
+                let pos = file.stream_position()?;
+                let mut old_value_bytes = vec![0u8; encoding::encoding_size_full::<F>()];
+                file.read_exact(&mut old_value_bytes)?;
+                // Go back to the previous position in the file, so the read value
+                // will be overwritten by the new one
+                file.seek(SeekFrom::Start(pos))?;
+                encoding::encode(&old_value_bytes)
+            };
+            old_value + diff_value
+        };
+        let new_value_bytes = encoding::decode_full(new_value);
         file.write_all(&new_value_bytes)?;
     }
     Ok(())
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::{diff::Diff, encoding, storage, storage::Data, Curve, ScalarField, SRS_SIZE};
+    use crate::{
+        diff::Diff,
+        encoding, storage,
+        storage::{Commitment, Data},
+        Curve, ScalarField, SRS_SIZE,
+    };
     use ark_ff::{One, UniformRand, Zero};
     use mina_curves::pasta::Fp;
+    use poly_commitment::ipa::SRS;
     use rand::Rng;
     use std::fs;
     use tempfile::NamedTempFile;
@@ -139,7 +161,7 @@ mod tests {
     fn test_data_consistency() {
         let mut rng = o1_utils::tests::make_test_rng(None);
 
-        let srs = poly_commitment::precomputed_srs::get_srs_test();
+        let srs: SRS<Curve> = poly_commitment::precomputed_srs::get_srs_test();
 
         // Path of the file that will contain the test data
         let file = NamedTempFile::new().unwrap();
@@ -148,7 +170,10 @@ mod tests {
         let data_bytes: Vec<u8> = (0..(SRS_SIZE * (encoding::encoding_size_full::<ScalarField>())))
             .map(|_| rng.gen())
             .collect();
-        let data = Data::of_bytes(&data_bytes);
+        let mut data = Data::of_bytes(&data_bytes);
+        // Setting the first value of data to zero will make the updated bytes
+        // with the well chosen diff
+        data.data[0] = Fp::zero();
         let data_comm = data.to_commitment(&srs);
 
         let read_consistency = {
@@ -157,10 +182,10 @@ mod tests {
             let read_data_comm = read_data.to_commitment(&srs);
 
             // True if read data are the same as initial data
-            Curve::eq(&data_comm, &read_data_comm)
+            Commitment::eq(&data_comm, &read_data_comm)
         };
 
-        let (data_updated, update_consistency) = {
+        let (data_updated, update_consistency, diff_comm_consistency) = {
             let diff = {
                 // The number of updates is proportional to the data length,
                 // but we make sure to have at least one update if the data is
@@ -170,16 +195,16 @@ mod tests {
                 let addresses: Vec<u64> = (0..nb_updates)
                     .map(|_| (rng.gen_range(0..data.len() as u64)))
                     .collect();
-                let mut new_values: Vec<ScalarField> =
+                let mut diff_values: Vec<ScalarField> =
                     addresses.iter().map(|_| Fp::rand(&mut rng)).collect();
                 // The first value is replaced by a scalar that would
                 // overflow 31 bytes, so the update is not consistent and the
                 // test fails if this case is not handled
-                new_values[0] = Fp::zero() - Fp::one();
+                diff_values[0] = Fp::zero() - Fp::one();
                 Diff {
                     region,
                     addresses,
-                    new_values,
+                    diff_values,
                 }
             };
 
@@ -191,10 +216,15 @@ mod tests {
             let updated_read_data = storage::read(path).unwrap();
             let updated_read_data_comm = updated_read_data.to_commitment(&srs);
 
+            let updated_diff_data_comm = data_comm.update(&srs, diff);
+
             (
-                Curve::ne(&updated_data_comm, &data_comm),
+                // True if the data have changed because of the update
+                Commitment::ne(&updated_data_comm, &data_comm),
                 // True if read data from updated file are the same as updated data
-                Curve::eq(&updated_data_comm, &updated_read_data_comm),
+                Commitment::eq(&updated_data_comm, &updated_read_data_comm),
+                // True if the commitments are the same as the commitment obtained by direct diff application
+                Commitment::eq(&updated_diff_data_comm, &updated_data_comm),
             )
         };
 
@@ -203,5 +233,6 @@ mod tests {
         assert!(read_consistency);
         assert!(data_updated);
         assert!(update_consistency);
+        assert!(diff_comm_consistency);
     }
 }