Skip to content

Commit

Permalink
fix: Delete field Share::evals, extract data from eval_proofs instead (…
Browse files Browse the repository at this point in the history 
…#674)

* add comments

* new method Share::evals() replicates struct field Share::evals

* delete field Shares::evals

* comment out tests that are broken by deleting Share::evals

* Share::evals() use extract_leaf

* new test-only method Share::extract_leaf_mut, use it to restore tests

* oops: use evals instead of share.evals()?

* use MerkleProof::elem instead of doing it myself

* move Share::extract_leaf_mut to test.rs, delete Share::extract_leaf

* revise comment
  • Loading branch information
@ggutoski
ggutoski authored Sep 4, 2024
1 parent 7e2eeef commit ae16b4c
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 30 deletions.
67 changes: 50 additions & 17 deletions vid/src/advz.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()
)));
}
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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() {
Expand All @@ -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],
))
}
}
Expand Down Expand Up @@ -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(
Expand All @@ -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()
Expand Down Expand Up @@ -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,
}
Expand Down
52 changes: 39 additions & 13 deletions vid/src/advz/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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");
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(),
);
}

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit ae16b4c

Please sign in to comment.