Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: refactor the verify api #359

Merged
merged 10 commits into from
Sep 27, 2024
38 changes: 31 additions & 7 deletions halo2_backend/src/plonk/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,29 +33,28 @@ pub use batch::BatchVerifier;
pub fn verify_proof_single<'params, Scheme, V, E, T, Strategy>(
ed255 marked this conversation as resolved.
Show resolved Hide resolved
params: &'params Scheme::ParamsVerifier,
vk: &VerifyingKey<Scheme::Curve>,
strategy: Strategy,
instance: Vec<Vec<Scheme::Scalar>>,
transcript: &mut T,
) -> Result<Strategy::Output, Error>
) -> bool
where
Scheme::Scalar: WithSmallOrderMulGroup<3> + FromUniformBytes<64>,
Scheme: CommitmentScheme,
V: Verifier<'params, Scheme>,
E: EncodedChallenge<Scheme::Curve>,
T: TranscriptRead<Scheme::Curve, E>,
Strategy: VerificationStrategy<'params, Scheme, V>,
Strategy: VerificationStrategy<'params, Scheme, V, Output = Strategy>,
{
verify_proof(params, vk, strategy, &[instance], transcript)
verify_proof::<Scheme, V, E, T, Strategy>(params, vk, &[instance], transcript)
}

/// Returns a boolean indicating whether or not the proof is valid
pub fn verify_proof<
/// Process the proof, checks that the proof is valid and returns the `Strategy` output.
pub fn verify_proof_with_strategy<
'params,
Scheme: CommitmentScheme,
V: Verifier<'params, Scheme>,
E: EncodedChallenge<Scheme::Curve>,
T: TranscriptRead<Scheme::Curve, E>,
Strategy: VerificationStrategy<'params, Scheme, V>,
Strategy: VerificationStrategy<'params, Scheme, V, Output = Strategy>,
>(
params: &'params Scheme::ParamsVerifier,
vk: &VerifyingKey<Scheme::Curve>,
Expand Down Expand Up @@ -519,3 +518,28 @@ where
.map_err(|_| Error::Opening)
})
}

/// Returns a boolean indicating whether or not the proof is valid
pub fn verify_proof<
'params,
Scheme: CommitmentScheme,
V: Verifier<'params, Scheme>,
E: EncodedChallenge<Scheme::Curve>,
T: TranscriptRead<Scheme::Curve, E>,
Strategy: VerificationStrategy<'params, Scheme, V, Output = Strategy>,
>(
params: &'params Scheme::ParamsVerifier,
vk: &VerifyingKey<Scheme::Curve>,
instances: &[Vec<Vec<Scheme::Scalar>>],
transcript: &mut T,
) -> bool
where
Scheme::Scalar: WithSmallOrderMulGroup<3> + FromUniformBytes<64>,
{
let strategy = Strategy::new(params);
let strategy = match verify_proof_with_strategy(params, vk, strategy, instances, transcript) {
Ok(strategy) => strategy,
Err(_) => return false,
};
strategy.finalize()
}
18 changes: 11 additions & 7 deletions halo2_backend/src/plonk/verifier/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use halo2_middleware::zal::impls::H2cEngine;
use halo2curves::CurveAffine;
use rand_core::OsRng;

use super::{verify_proof, VerificationStrategy};
use super::{verify_proof_with_strategy, VerificationStrategy};
use crate::{
multicore::{
IndexedParallelIterator, IntoParallelIterator, ParallelIterator, TryFoldAndReduce,
Expand Down Expand Up @@ -34,7 +34,7 @@ struct BatchStrategy<'params, C: CurveAffine> {
impl<'params, C: CurveAffine> VerificationStrategy<'params, IPACommitmentScheme<C>, VerifierIPA<C>>
for BatchStrategy<'params, C>
{
type Output = MSMIPA<'params, C>;
type Output = Self;

fn new(params: &'params ParamsVerifierIPA<C>) -> Self {
BatchStrategy {
Expand All @@ -47,7 +47,9 @@ impl<'params, C: CurveAffine> VerificationStrategy<'params, IPACommitmentScheme<
f: impl FnOnce(MSMIPA<'params, C>) -> Result<GuardIPA<'params, C>, Error>,
) -> Result<Self::Output, Error> {
let guard = f(self.msm)?;
Ok(guard.use_challenges())
Ok(Self {
msm: guard.use_challenges(),
})
}

fn finalize(self) -> bool {
Expand Down Expand Up @@ -110,10 +112,12 @@ where
.map(|(i, item)| {
let strategy = BatchStrategy::new(params);
let mut transcript = Blake2bRead::init(&item.proof[..]);
verify_proof(params, vk, strategy, &item.instances, &mut transcript).map_err(|e| {
tracing::debug!("Batch item {} failed verification: {}", i, e);
e
})
verify_proof_with_strategy(params, vk, strategy, &item.instances, &mut transcript)
.map_err(|e| {
tracing::debug!("Batch item {} failed verification: {}", i, e);
e
})
.map(|st| st.msm)
})
.try_fold_and_reduce(
|| ParamsVerifier::<'_, C>::empty_msm(params),
Expand Down
15 changes: 6 additions & 9 deletions halo2_backend/src/poly/ipa/strategy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub struct SingleStrategy<'params, C: CurveAffine> {
impl<'params, C: CurveAffine> VerificationStrategy<'params, IPACommitmentScheme<C>, VerifierIPA<C>>
for SingleStrategy<'params, C>
{
type Output = ();
type Output = Self;

fn new(params: &'params ParamsIPA<C>) -> Self {
SingleStrategy {
Expand All @@ -132,13 +132,9 @@ impl<'params, C: CurveAffine> VerificationStrategy<'params, IPACommitmentScheme<
f: impl FnOnce(MSMIPA<'params, C>) -> Result<GuardIPA<'params, C>, Error>,
) -> Result<Self::Output, Error> {
let guard = f(self.msm)?;
let msm = guard.use_challenges();
// ZAL: Verification is (supposedly) cheap, hence we don't use an accelerator engine
if msm.check(&H2cEngine::new()) {
Ok(())
} else {
Err(Error::ConstraintSystemFailure)
}
Ok(Self {
msm: guard.use_challenges(),
})
}

/// Finalizes the batch and checks its validity.
Expand All @@ -147,7 +143,8 @@ impl<'params, C: CurveAffine> VerificationStrategy<'params, IPACommitmentScheme<
/// specific failing proofs, it must re-process the proofs separately.
#[must_use]
fn finalize(self) -> bool {
unreachable!()
// TODO: Verification is cheap, ZkAccel on verifier is not a priority.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we file an issue for the TODO?

Copy link
Author

@guorong009 guorong009 Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we should.
We can discuss if it is worthy or not, to apply ZkAccel on verification.

self.msm.check(&H2cEngine::new())
}
}

Expand Down
17 changes: 8 additions & 9 deletions halo2_backend/src/poly/kzg/strategy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ where
E::G1: CurveExt<AffineExt = E::G1Affine>,
E::G2Affine: SerdeCurveAffine,
{
type Output = ();
Copy link
Member

@ed255 ed255 Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that all the implementations of VerificationStrategy have Output = Self.
That seems redundant, we could remove this associated type. Then the verification process either returns bool, or the VerificationStrategy object, no need to do VerificationStrategy::Output which is already VerificationStragey all the time.

Also, I think we should discourage the usage of verification functions that return the VerificationStrategy because returning a bool is much clearer. Since this PR is scheduled to be merged after the 0.4 release, and we determined that in the 0.5 release we can remove the halo2_proofs legacy layer, maybe we can just remove the verification methods that return VerificationStrategy? If not, at least could we mark them as deprecated via cfg?

Or is there any strong reason to keep the API that takes and returns a VerificationStrategy object?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the proposal from @ed255
And if we break API is indeed a good time to do it!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ed255
I agree that Output = Self is redundant.
For the quick patch, I remove the Output type from VerificationStrategy trait. e5153c4

Or is there any strong reason to keep the API that takes and returns a VerificationStrategy object?

This is related to process API.
I think this API could be useful for batch verification, like following one.

pub fn finalize(self, params: &ParamsVerifierIPA<C>, vk: &VerifyingKey<C>) -> bool {
fn accumulate_msm<'params, C: CurveAffine>(
mut acc: MSMIPA<'params, C>,
msm: MSMIPA<'params, C>,
) -> MSMIPA<'params, C> {
// Scale the MSM by a random factor to ensure that if the existing MSM has
// `is_zero() == false` then this argument won't be able to interfere with it
// to make it true, with high probability.
acc.scale(C::Scalar::random(OsRng));
acc.add_msm(&msm);
acc
}
let final_msm = self
.items
.into_par_iter()
.enumerate()
.map(|(i, item)| {
let strategy = BatchStrategy::new(params);
let mut transcript = Blake2bRead::init(&item.proof[..]);
verify_proof_with_strategy(params, vk, strategy, &item.instances, &mut transcript)
.map_err(|e| {
tracing::debug!("Batch item {} failed verification: {}", i, e);
e
})
.map(|st| st.msm)
})
.try_fold_and_reduce(
|| ParamsVerifier::<'_, C>::empty_msm(params),
|acc, res| res.map(|proof_msm| accumulate_msm(acc, proof_msm)),
);
match final_msm {
// ZAL: Verification is (supposedly) cheap, hence we don't use an accelerator engine
Ok(msm) => msm.check(&H2cEngine::new()),
Err(_) => false,
}

The BatchVerifier delays the MSM check until it accumulates all the MSM from every batch items.

What do you think about this? @ed255 @CPerezz

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ed255 @CPerezz any input about last comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current state of this PR looks good to me (removal of Output from trait).

type Output = Self;

fn new(params: &'params ParamsVerifierKZG<E>) -> Self {
Self::new(params)
Expand All @@ -168,16 +168,15 @@ where
// Guard is updated with new msm contributions
let guard = f(self.msm)?;
let msm = guard.msm_accumulator;
// Verification is (supposedly) cheap, hence we don't use an accelerator engine
let default_engine = H2cEngine::new();
if msm.check(&default_engine, &self.params) {
Ok(())
} else {
Err(Error::ConstraintSystemFailure)
}
Ok(Self {
msm,
params: self.params,
})
}

fn finalize(self) -> bool {
unreachable!();
// Verification is (supposedly) cheap, hence we don't use an accelerator engine
let default_engine = H2cEngine::new();
self.msm.check(&default_engine, &self.params)
}
}
19 changes: 10 additions & 9 deletions halo2_proofs/benches/plonk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,10 @@ use halo2curves::pasta::{EqAffine, Fp};
use rand_core::OsRng;

use halo2_proofs::{
poly::{
ipa::{
commitment::{IPACommitmentScheme, ParamsIPA},
multiopen::ProverIPA,
strategy::SingleStrategy,
},
VerificationStrategy,
poly::ipa::{
commitment::{IPACommitmentScheme, ParamsIPA},
multiopen::ProverIPA,
strategy::SingleStrategy,
},
transcript::{TranscriptReadBuffer, TranscriptWriterBuffer},
};
Expand Down Expand Up @@ -300,9 +297,13 @@ fn criterion_benchmark(c: &mut Criterion) {
}

fn verifier(params: &ParamsIPA<EqAffine>, vk: &VerifyingKey<EqAffine>, proof: &[u8]) {
let strategy = SingleStrategy::new(params);
let mut transcript = Blake2bRead::<_, _, Challenge255<_>>::init(proof);
assert!(verify_proof(params, vk, strategy, &[vec![vec![]]], &mut transcript).is_ok());
assert!(verify_proof::<_, _, _, _, SingleStrategy<_>>(
params,
vk,
&[vec![vec![]]],
&mut transcript
));
}

let k_range = 8..=16;
Expand Down
12 changes: 6 additions & 6 deletions halo2_proofs/tests/compress_selectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,16 +375,16 @@ fn test_mycircuit(
// Verify
let mut verifier_transcript =
Blake2bRead::<_, G1Affine, Challenge255<_>>::init(proof.as_slice());
let strategy = SingleStrategy::new(&verifier_params);

verify_proof::<KZGCommitmentScheme<Bn256>, VerifierSHPLONK<Bn256>, _, _, _>(
if !verify_proof::<KZGCommitmentScheme<Bn256>, VerifierSHPLONK<Bn256>, _, _, SingleStrategy<_>>(
&verifier_params,
&vk,
strategy,
instances.as_slice(),
&mut verifier_transcript,
)
.map_err(halo2_proofs::plonk::Error::Backend)?;
) {
return Err(halo2_proofs::plonk::Error::Backend(
halo2_backend::plonk::Error::Opening,
));
};

Ok(proof)
}
Expand Down
45 changes: 26 additions & 19 deletions halo2_proofs/tests/frontend_backend_split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,16 +525,22 @@ fn test_mycircuit_full_legacy() {
let mut verifier_transcript =
Blake2bRead::<_, G1Affine, Challenge255<_>>::init(proof.as_slice());
let verifier_params = params.verifier_params();
let strategy = SingleStrategy::new(&verifier_params);

verify_proof::<KZGCommitmentScheme<Bn256>, VerifierSHPLONK<Bn256>, _, _, _>(
&verifier_params,
&vk,
strategy,
instances.as_slice(),
&mut verifier_transcript,
)
.expect("verify succeeds");
assert!(
verify_proof::<
KZGCommitmentScheme<Bn256>,
VerifierSHPLONK<Bn256>,
_,
_,
SingleStrategy<_>,
>(
&verifier_params,
&vk,
instances.as_slice(),
&mut verifier_transcript,
),
"failed to verify proof"
);
println!("Verify: {:?}", start.elapsed());

proof
Expand Down Expand Up @@ -605,16 +611,17 @@ fn test_mycircuit_full_split() {
let mut verifier_transcript =
Blake2bRead::<_, G1Affine, Challenge255<_>>::init(proof.as_slice());
let verifier_params = params.verifier_params();
let strategy = SingleStrategy::new(&verifier_params);

verify_proof_single::<KZGCommitmentScheme<Bn256>, VerifierSHPLONK<Bn256>, _, _, _>(
&verifier_params,
&vk,
strategy,
instances,
&mut verifier_transcript,
)
.expect("verify succeeds");

assert!(
verify_proof_single::<
KZGCommitmentScheme<Bn256>,
VerifierSHPLONK<Bn256>,
_,
_,
SingleStrategy<_>,
>(&verifier_params, &vk, instances, &mut verifier_transcript,),
"failed to verify proof"
);
println!("Verify: {:?}", start.elapsed());

proof
Expand Down
11 changes: 6 additions & 5 deletions halo2_proofs/tests/plonk_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,11 +560,12 @@ fn plonk_api() {
let mut transcript = T::init(proof);
let instance = [vec![vec![instance_val]], vec![vec![instance_val]]];

let strategy = Strategy::new(params_verifier);
let strategy =
verify_plonk_proof(params_verifier, vk, strategy, &instance, &mut transcript).unwrap();

assert!(strategy.finalize());
assert!(verify_plonk_proof::<_, _, _, _, Strategy>(
params_verifier,
vk,
&instance,
&mut transcript
));
}

fn test_plonk_api_gwc() {
Expand Down
30 changes: 15 additions & 15 deletions halo2_proofs/tests/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,22 +183,22 @@ fn test_serialization() {
let proof = transcript.finalize();

let verifier_params = params.verifier_params();
let strategy = SingleStrategy::new(&verifier_params);
let mut transcript = Blake2bRead::<_, _, Challenge255<_>>::init(&proof[..]);
assert!(verify_proof::<
KZGCommitmentScheme<Bn256>,
VerifierGWC<Bn256>,
Challenge255<G1Affine>,
Blake2bRead<&[u8], G1Affine, Challenge255<G1Affine>>,
SingleStrategy<Bn256>,
>(
&verifier_params,
pk.get_vk(),
strategy,
instances.as_slice(),
&mut transcript
)
.is_ok());
assert!(
verify_proof::<
KZGCommitmentScheme<Bn256>,
VerifierGWC<Bn256>,
Challenge255<G1Affine>,
Blake2bRead<&[u8], G1Affine, Challenge255<G1Affine>>,
SingleStrategy<Bn256>,
>(
&verifier_params,
pk.get_vk(),
instances.as_slice(),
&mut transcript
),
"failed to verify proof"
);

proof
},
Expand Down
7 changes: 1 addition & 6 deletions halo2_proofs/tests/shuffle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use halo2_proofs::{
multiopen::{ProverIPA, VerifierIPA},
strategy::AccumulatorStrategy,
},
VerificationStrategy,
},
transcript::{
Blake2bRead, Blake2bWrite, Challenge255, TranscriptReadBuffer, TranscriptWriterBuffer,
Expand Down Expand Up @@ -301,18 +300,14 @@ where
};

let accepted = {
let strategy = AccumulatorStrategy::new(&params);
let mut transcript = Blake2bRead::<_, _, Challenge255<_>>::init(&proof[..]);

verify_proof::<IPACommitmentScheme<C>, VerifierIPA<C>, _, _, _>(
verify_proof::<IPACommitmentScheme<C>, VerifierIPA<C>, _, _, AccumulatorStrategy<C>>(
&params,
pk.get_vk(),
strategy,
&[vec![]],
&mut transcript,
)
.map(|strategy| strategy.finalize())
.unwrap_or_default()
};

assert_eq!(accepted, expected);
Expand Down
Loading
Loading