Skip to content

Commit

Permalink
refactor: simplify commitment trait (#55)
Browse files Browse the repository at this point in the history
# Rationale for this change

The `Commitment` trait has a `fold_commitments` which is only used
internally to the `CommitmentEvaluationProof`, and as a result, is
somewhat superfluous, making adding new commitment schemes more complex.

# What changes are included in this PR?

* `Commitment::fold_commitments` is removed.
* `CommitmentEvaluationProof::verify_batched_proof` is no longer a
provided method. Instead, `CommitmentEvaluationProof::verify_proof` is a
provided method, and implementers must implement `verify_batched_proof`
instead.

# Are these changes tested?

By existing tests.
  • Loading branch information
JayWhite2357 authored Jul 24, 2024
1 parent bc404ae commit ac8aeef
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,18 @@ pub trait CommitmentEvaluationProof {
generators_offset: u64,
table_length: usize,
setup: &Self::VerifierPublicSetup<'_>,
) -> Result<(), Self::Error>;
) -> Result<(), Self::Error> {
self.verify_batched_proof(
transcript,
core::slice::from_ref(a_commit),
&[Self::Scalar::ONE],
product,
b_point,
generators_offset,
table_length,
setup,
)
}
/// Verify a batch proof. This can be more efficient than verifying individual proofs for some schemes.
#[allow(clippy::too_many_arguments)]
fn verify_batched_proof(
Expand All @@ -65,17 +76,7 @@ pub trait CommitmentEvaluationProof {
generators_offset: u64,
table_length: usize,
setup: &Self::VerifierPublicSetup<'_>,
) -> Result<(), Self::Error> {
self.verify_proof(
transcript,
&Self::Commitment::fold_commitments(commit_batch, batching_factors),
product,
b_point,
generators_offset,
table_length,
setup,
)
}
) -> Result<(), Self::Error>;
}

#[cfg(feature = "blitzar")]
Expand Down Expand Up @@ -107,10 +108,12 @@ impl CommitmentEvaluationProof for InnerProductProof {
generators_offset,
)
}
fn verify_proof(

fn verify_batched_proof(
&self,
transcript: &mut Transcript,
a_commit: &Self::Commitment,
commit_batch: &[Self::Commitment],
batching_factors: &[Self::Scalar],
product: &Self::Scalar,
b_point: &[Self::Scalar],
generators_offset: u64,
Expand All @@ -127,7 +130,14 @@ impl CommitmentEvaluationProof for InnerProductProof {
}
self.verify(
transcript,
a_commit,
&commit_batch
.iter()
.zip(batching_factors.iter())
.map(|(c, m)| *m * c)
.fold(Default::default(), |mut a, c| {
a += c;
a
}),
&product.into(),
&slice_ops::slice_cast(b),
generators_offset,
Expand Down
14 changes: 0 additions & 14 deletions crates/proof-of-sql/src/base/commitment/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,6 @@ pub trait Commitment:
offset: usize,
setup: &Self::PublicSetup<'_>,
);

/// Compute a linear combination of the given commitments: `sum commitment[i] * multiplier[i]`.
fn fold_commitments(commitments: &[Self], multipliers: &[Self::Scalar]) -> Self;
}

impl Commitment for RistrettoPoint {
Expand Down Expand Up @@ -112,17 +109,6 @@ impl Commitment for RistrettoPoint {
) {
unimplemented!()
}

fn fold_commitments(commitments: &[Self], multipliers: &[Self::Scalar]) -> Self {
commitments
.iter()
.zip(multipliers.iter())
.map(|(c, m)| *m * c)
.fold(Default::default(), |mut a, c| {
a += c;
a
})
}
}

mod commitment_evaluation_proof;
Expand Down
12 changes: 1 addition & 11 deletions crates/proof-of-sql/src/proof_primitive/dory/dory_commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ use crate::base::{
impl_serde_for_ark_serde_checked,
scalar::{scalar_conversion_to_int, MontScalar, Scalar, ScalarConversionError},
};
use ark_ec::{pairing::PairingOutput, VariableBaseMSM};
use ark_ec::pairing::PairingOutput;
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
use bytemuck::TransparentWrapper;
use core::ops::Mul;
use derive_more::{AddAssign, Neg, Sub, SubAssign};
use num_traits::One;
Expand Down Expand Up @@ -60,10 +59,8 @@ impl Scalar for DoryScalar {
SubAssign,
CanonicalSerialize,
CanonicalDeserialize,
TransparentWrapper,
)]
/// The Dory commitment type.
#[repr(transparent)]
pub struct DoryCommitment(pub(super) GT);

/// The default for GT is the the additive identity, but should be the multiplicative identity.
Expand Down Expand Up @@ -101,13 +98,6 @@ impl Commitment for DoryCommitment {
let c = super::compute_dory_commitments(committable_columns, offset, setup);
commitments.copy_from_slice(&c);
}

fn fold_commitments(commitments: &[Self], multipliers: &[Self::Scalar]) -> Self {
Self(VariableBaseMSM::msm_unchecked(
TransparentWrapper::peel_slice(commitments),
TransparentWrapper::peel_slice(multipliers),
))
}
}

#[cfg(test)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use super::{
};
use crate::base::commitment::CommitmentEvaluationProof;
use merlin::Transcript;
use num_traits::One;
use thiserror::Error;

/// The `CommitmentEvaluationProof` for the Dory PCS.
Expand Down Expand Up @@ -63,28 +62,11 @@ impl CommitmentEvaluationProof for DoryEvaluationProof {
messages
}

#[tracing::instrument(name = "DoryEvaluationProof::verify_proof", level = "debug", skip_all)]
fn verify_proof(
&self,
transcript: &mut Transcript,
a_commit: &Self::Commitment,
product: &Self::Scalar,
b_point: &[Self::Scalar],
generators_offset: u64,
_table_length: usize,
setup: &Self::VerifierPublicSetup<'_>,
) -> Result<(), Self::Error> {
self.verify_batched_proof(
transcript,
&[*a_commit],
&[DoryScalar::one()],
product,
b_point,
generators_offset,
_table_length,
setup,
)
}
#[tracing::instrument(
name = "DoryEvaluationProof::verify_batched_proof",
level = "debug",
skip_all
)]
fn verify_batched_proof(
&self,
transcript: &mut Transcript,
Expand Down

0 comments on commit ac8aeef

Please sign in to comment.