fix: Delete field Share::evals, extract data from eval_proofs instead

vid/src/advz.rs

@@ -35,6 +35,7 @@ use digest::crypto_common::Output;
 use itertools::Itertools;
 use jf_merkle_tree::{
     hasher::{HasherDigest, HasherMerkleTree, HasherNode},
+    prelude::{MerkleNode, MerkleProof},
     MerkleCommitment, MerkleTreeScheme,
 };
 #[cfg(feature = "gpu-vid")]
@@ -289,17 +290,43 @@ where
 {
     index: u32,
 
-    #[serde(with = "canonical")]
-    evals: Vec<KzgEval<E>>,
-
     #[serde(with = "canonical")]
     // aggregate_proofs.len() equals multiplicity
     // TODO further aggregate into a single KZG proof.
     aggregate_proofs: Vec<KzgProof<E>>,
 
+    // eval_proofs.len() equals multiplicity
+    // TODO put all evals into a single merkle proof
+    // https://github.com/EspressoSystems/jellyfish/issues/671
     eval_proofs: Vec<KzgEvalsMerkleTreeProof<E, H>>,
 }
 
+impl<E, H> Share<E, H>
+where
+    E: Pairing,
+    H: HasherDigest,
+{
+    /// Return a [`Vec`] of payload data for this share.
+    ///
+    /// These data are extracted from [`MerkleProof`] structs. The returned
+    /// [`Vec`] has length `multiplicity * num_polys`s
+    ///
+    /// TODO store these data in a new field `Share::evals` after fixing
+    /// https://github.com/EspressoSystems/jellyfish/issues/658
+    fn evals(&self) -> VidResult<Vec<KzgEval<E>>> {
+        self.eval_proofs
+            .iter()
+            .map(|eval_proof| {
+                eval_proof
+                    .elem()
+                    .cloned()
+                    .ok_or_else(|| VidError::Internal(anyhow::anyhow!("empty merkle proof")))
+            })
+            .flatten_ok()
+            .collect()
+    }
+}
+
 /// The [`VidScheme::Common`] type for [`Advz`].
 #[derive(CanonicalSerialize, CanonicalDeserialize, Derivative, Deserialize, Serialize)]
 #[derivative(
@@ -435,10 +462,11 @@ where
                 common.multiplicity, multiplicity
             )));
         }
-        if share.evals.len() / multiplicity as usize != common.poly_commits.len() {
+        let evals = share.evals()?;
+        if evals.len() / multiplicity as usize != common.poly_commits.len() {
             return Err(VidError::Argument(format!(
                 "number of share evals / multiplicity {}/{} differs from number of common polynomial commitments {}",
-                share.evals.len(), multiplicity,
+                evals.len(), multiplicity,
                 common.poly_commits.len()
             )));
         }
@@ -497,14 +525,13 @@ where
         let verification_iter = parallelizable_slice_iter(&multiplicities).map(|i| {
             let range = i * polys_len..(i + 1) * polys_len;
             let aggregate_eval = polynomial_eval(
-                share
-                    .evals
+                evals
                     .get(range.clone())
                     .ok_or_else(|| {
                         VidError::Internal(anyhow::anyhow!(
                             "share evals range {:?} out of bounds for length {}",
                             range,
-                            share.evals.len()
+                            evals.len()
                         ))
                     })?
                     .iter()
@@ -553,23 +580,27 @@ where
             )));
         }
 
+        let shares_evals = shares
+            .iter()
+            .map(|share| share.evals())
+            .collect::<VidResult<Vec<_>>>()?;
+
         // check args: all shares must have equal evals len
-        let num_evals = shares
+        let num_evals = shares_evals
             .first()
             .ok_or_else(|| VidError::Argument("shares is empty".into()))?
-            .evals
             .len();
-        if let Some((index, share)) = shares
+        if let Some((index, share_evals)) = shares_evals
             .iter()
             .enumerate()
-            .find(|(_, s)| s.evals.len() != num_evals)
+            .find(|(_, evals)| evals.len() != num_evals)
         {
             return Err(VidError::Argument(format!(
                 "shares do not have equal evals lengths: share {} len {}, share {} len {}",
                 0,
                 num_evals,
                 index,
-                share.evals.len()
+                share_evals.len()
             )));
         }
         if num_evals != common.multiplicity as usize * common.poly_commits.len() {
@@ -590,12 +621,12 @@ where
         let mut elems = Vec::with_capacity(elems_capacity);
         let mut evals = Vec::with_capacity(num_evals);
         for p in 0..num_polys {
-            for share in shares {
+            for (share, share_evals) in shares.iter().zip(shares_evals.iter()) {
                 // extract all evaluations for polynomial p from the share
                 for m in 0..common.multiplicity as usize {
                     evals.push((
                         (share.index * common.multiplicity) as usize + m,
-                        share.evals[(m * num_polys) + p],
+                        share_evals[(m * num_polys) + p],
                     ))
                 }
             }
@@ -677,7 +708,6 @@ where
             multiplicity * self.recovery_threshold
         ));
         let all_storage_node_evals = {
-            let mut all_storage_node_evals = vec![Vec::with_capacity(polys.len()); code_word_size];
             let all_poly_evals = parallelizable_slice_iter(&polys)
                 .map(|poly| {
                     UnivariateKzgPCS::<E>::multi_open_rou_evals(
@@ -689,6 +719,10 @@ where
                 })
                 .collect::<Result<Vec<_>, VidError>>()?;
 
+            // distribute evals from each poly among the storage nodes
+            //
+            // perf warning: runtime is quadratic in payload_size
+            let mut all_storage_node_evals = vec![Vec::with_capacity(polys.len()); code_word_size];
             for poly_evals in all_poly_evals {
                 for (storage_node_evals, poly_eval) in all_storage_node_evals
                     .iter_mut()
@@ -776,7 +810,6 @@ where
                     let (evals, proofs, eval_proofs): (Vec<_>, _, _) = chunk.multiunzip();
                     Share {
                         index: index as u32,
-                        evals: evals.into_iter().flatten().collect::<Vec<_>>(),
                         aggregate_proofs: proofs,
                         eval_proofs,
                     }

vid/src/advz/test.rs

@@ -60,7 +60,7 @@ fn sad_path_verify_share_corrupt_share() {
         // missing share eval
         {
             let share_missing_eval = Share {
-                evals: share.evals[1..].to_vec(),
+                eval_proofs: share.eval_proofs[1..].to_vec(),
                 ..share.clone()
             };
             assert_arg_err(
@@ -72,7 +72,9 @@ fn sad_path_verify_share_corrupt_share() {
         // corrupted share eval
         {
             let mut share_bad_eval = share.clone();
-            share_bad_eval.evals[0].double_in_place();
+            Share::<Bn254, Sha256>::extract_leaf_mut(&mut share_bad_eval.eval_proofs[0]).unwrap()
+                [0]
+            .double_in_place();
             advz.verify_share(&share_bad_eval, &common, &commit)
                 .unwrap()
                 .expect_err("bad share value should fail verification");
@@ -168,7 +170,7 @@ fn sad_path_verify_share_corrupt_share_and_commit() {
     let (mut shares, mut common, commit) = (disperse.shares, disperse.common, disperse.commit);
 
     common.poly_commits.pop();
-    shares[0].evals.pop();
+    shares[0].eval_proofs.pop();
 
     // equal nonzero lengths for common, share
     assert_arg_err(
@@ -177,7 +179,7 @@ fn sad_path_verify_share_corrupt_share_and_commit() {
     );
 
     common.poly_commits.clear();
-    shares[0].evals.clear();
+    shares[0].eval_proofs.clear();
 
     // zero length for common, share
     assert_arg_err(
@@ -196,23 +198,18 @@ fn sad_path_recover_payload_corrupt_shares() {
         // unequal share eval lengths
         let mut shares_missing_evals = shares.clone();
         for i in 0..shares_missing_evals.len() - 1 {
-            shares_missing_evals[i].evals.pop();
+            shares_missing_evals[i].eval_proofs.pop();
             assert_arg_err(
                 advz.recover_payload(&shares_missing_evals, &common),
                 format!("{} shares missing 1 eval should be arg error", i + 1).as_str(),
             );
         }
 
         // 1 eval missing from all shares
-        shares_missing_evals.last_mut().unwrap().evals.pop();
+        shares_missing_evals.last_mut().unwrap().eval_proofs.pop();
         assert_arg_err(
             advz.recover_payload(&shares_missing_evals, &common),
-            format!(
-                "shares contain {} but expected {}",
-                shares_missing_evals[0].evals.len(),
-                &common.poly_commits.len()
-            )
-            .as_str(),
+            format!("all shares missing 1 eval should be arg error").as_str(),
         );
     }
 
@@ -278,7 +275,11 @@ fn sad_path_verify_share_with_multiplicity() {
         // corrupt the last evaluation of the share
         {
             let mut share_bad_eval = share.clone();
-            share_bad_eval.evals[common.multiplicity as usize - 1].double_in_place();
+            Share::<Bn254, Sha256>::extract_leaf_mut(
+                &mut share_bad_eval.eval_proofs[common.multiplicity as usize - 1],
+            )
+            .unwrap()[common.poly_commits.len() - 1]
+                .double_in_place();
             advz.verify_share(&share_bad_eval, &common, &commit)
                 .unwrap()
                 .expect_err("bad share value should fail verification");
@@ -450,6 +451,31 @@ fn max_multiplicity() {
     assert!(found_small_payload, "missing test for small payload");
 }
 
+impl<E, H> Share<E, H>
+where
+    E: Pairing,
+    H: HasherDigest,
+{
+    /// Like [`MerkleProof::elem`] except the returned reference is mutable.
+    fn extract_leaf_mut(
+        proof: &mut KzgEvalsMerkleTreeProof<E, H>,
+    ) -> VidResult<&mut Vec<KzgEval<E>>> {
+        // `eval_proof.proof` is a`Vec<MerkleNode>` with length >= 1
+        // whose first item always has variant `Leaf`. See
+        // `MerkleProof::verify_membership_proof`.
+        let merkle_node = proof
+            .proof
+            .get_mut(0)
+            .ok_or_else(|| VidError::Internal(anyhow::anyhow!("empty merkle proof")))?;
+        let MerkleNode::Leaf { elem, .. } = merkle_node else {
+            return Err(VidError::Internal(anyhow::anyhow!(
+                "expect MerkleNode::Leaf variant"
+            )));
+        };
+        Ok(elem)
+    }
+}
+
 struct AdvzParams {
     recovery_threshold: u32,
     num_storage_nodes: u32,