Skip to content

Commit

Permalink
chore: remove some TODOs from codebase (#376)
Browse files Browse the repository at this point in the history
* feat: update "QueryBack", to use "ColumnMid"

* fix: use "ArgumentMid" in "permutation" module

* feat: move truncation out of "EvaluationDomain::extended_to_coeff"

* chore: update test vectors
  • Loading branch information
guorong009 authored Oct 24, 2024
1 parent eff9c56 commit 433dfbb
Show file tree
Hide file tree
Showing 20 changed files with 155 additions and 119 deletions.
1 change: 0 additions & 1 deletion halo2_backend/src/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ pub(crate) fn eval_polynomial<F: Field>(poly: &[F], point: F) -> F {
///
/// This function will panic if the two vectors are not the same size.
pub(crate) fn compute_inner_product<F: Field>(a: &[F], b: &[F]) -> F {
// TODO: parallelize?
assert_eq!(a.len(), b.len());

let mut acc = F::ZERO;
Expand Down
7 changes: 2 additions & 5 deletions halo2_backend/src/plonk/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@ use halo2_middleware::expression::{Expression, Variable};
use halo2_middleware::poly::Rotation;
use halo2_middleware::{lookup, permutation::ArgumentMid, shuffle};

// TODO: Reuse ColumnMid inside this.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct QueryBack {
/// Query index
pub(crate) index: usize,
/// Column index
pub(crate) column_index: usize,
/// The type of the column.
pub(crate) column_type: Any,
/// Column
pub(crate) column: ColumnMid,
/// Rotation of this query
pub(crate) rotation: Rotation,
}
Expand Down
48 changes: 29 additions & 19 deletions halo2_backend/src/plonk/evaluation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,17 +701,17 @@ impl<C: CurveAffine> GraphEvaluator<C> {
ExpressionBack::Constant(scalar) => self.add_constant(scalar),
ExpressionBack::Var(VarBack::Query(query)) => {
let rot_idx = self.add_rotation(&query.rotation);
match query.column_type {
match query.column.column_type {
Any::Fixed => self.add_calculation(Calculation::Store(ValueSource::Fixed(
query.column_index,
query.column.index,
rot_idx,
))),
Any::Advice => self.add_calculation(Calculation::Store(ValueSource::Advice(
query.column_index,
query.column.index,
rot_idx,
))),
Any::Instance => self.add_calculation(Calculation::Store(
ValueSource::Instance(query.column_index, rot_idx),
ValueSource::Instance(query.column.index, rot_idx),
)),
}
}
Expand Down Expand Up @@ -863,10 +863,10 @@ pub(crate) fn evaluate<F: Field, B: LagrangeBasis>(
VarBack::Challenge(challenge) => challenges[challenge.index()],
VarBack::Query(query) => {
let rot_idx = get_rotation_idx(idx, query.rotation.0, rot_scale, isize);
match query.column_type {
Any::Fixed => fixed[query.column_index][rot_idx],
Any::Advice => advice[query.column_index][rot_idx],
Any::Instance => instance[query.column_index][rot_idx],
match query.column.column_type {
Any::Fixed => fixed[query.column.index][rot_idx],
Any::Advice => advice[query.column.index][rot_idx],
Any::Instance => instance[query.column.index][rot_idx],
}
}
},
Expand All @@ -883,7 +883,7 @@ pub(crate) fn evaluate<F: Field, B: LagrangeBasis>(
mod test {
use crate::plonk::circuit::{ExpressionBack, QueryBack, VarBack};
use crate::poly::LagrangeCoeff;
use halo2_middleware::circuit::{Any, ChallengeMid};
use halo2_middleware::circuit::{Any, ChallengeMid, ColumnMid};
use halo2_middleware::poly::Rotation;
use halo2curves::pasta::pallas::{Affine, Scalar};

Expand Down Expand Up @@ -954,8 +954,10 @@ mod test {
check_expr(
ExpressionBack::Var(Query(QueryBack {
index: 0,
column_index: col,
column_type: Any::Fixed,
column: ColumnMid {
index: col,
column_type: Any::Fixed,
},
rotation: Rotation(rot),
})),
expected,
Expand All @@ -965,8 +967,10 @@ mod test {
check_expr(
ExpressionBack::Var(Query(QueryBack {
index: 0,
column_index: col,
column_type: Any::Advice,
column: ColumnMid {
index: col,
column_type: Any::Advice,
},
rotation: Rotation(rot),
})),
expected,
Expand All @@ -976,8 +980,10 @@ mod test {
check_expr(
ExpressionBack::Var(Query(QueryBack {
index: 0,
column_index: col,
column_type: Any::Instance,
column: ColumnMid {
index: col,
column_type: Any::Instance,
},
rotation: Rotation(rot),
})),
expected,
Expand Down Expand Up @@ -1007,17 +1013,21 @@ mod test {
let two = || {
Box::new(ExpressionBack::<Scalar>::Var(Query(QueryBack {
index: 0,
column_index: 0,
column_type: Any::Fixed,
column: ColumnMid {
index: 0,
column_type: Any::Fixed,
},
rotation: Rotation(0),
})))
};

let three = || {
Box::new(ExpressionBack::<Scalar>::Var(Query(QueryBack {
index: 0,
column_index: 0,
column_type: Any::Fixed,
column: ColumnMid {
index: 0,
column_type: Any::Fixed,
},
rotation: Rotation(1),
})))
};
Expand Down
6 changes: 4 additions & 2 deletions halo2_backend/src/plonk/keygen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,10 @@ impl QueriesMap {
let index = self.add(column, query.rotation);
ExpressionBack::Var(VarBack::Query(QueryBack {
index,
column_index: query.column_index,
column_type: query.column_type,
column: ColumnMid {
index: query.column_index,
column_type: query.column_type,
},
rotation: query.rotation,
}))
}
Expand Down
14 changes: 7 additions & 7 deletions halo2_backend/src/plonk/lookup/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ impl<C: CurveAffine> Evaluated<C> {
&|scalar| scalar,
&|var| match var {
VarBack::Challenge(challenge) => challenges[challenge.index],
VarBack::Query(QueryBack {
index, column_type, ..
}) => match column_type {
Any::Fixed => fixed_evals[index],
Any::Advice => advice_evals[index],
Any::Instance => instance_evals[index],
},
VarBack::Query(QueryBack { index, column, .. }) => {
match column.column_type {
Any::Fixed => fixed_evals[index],
Any::Advice => advice_evals[index],
Any::Instance => instance_evals[index],
}
}
},
&|a| -a,
&|a, b| a + b,
Expand Down
5 changes: 2 additions & 3 deletions halo2_backend/src/plonk/permutation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use crate::{
helpers::{polynomial_slice_byte_length, read_polynomial_vec, write_polynomial_slice},
poly::{Coeff, ExtendedLagrangeCoeff, LagrangeCoeff, Polynomial},
};
// TODO: Remove the renaming
pub use halo2_middleware::permutation::ArgumentMid as Argument;
pub use halo2_middleware::permutation::ArgumentMid;

use std::io;

Expand All @@ -34,7 +33,7 @@ impl<C: CurveAffine> VerifyingKey<C> {

pub(crate) fn read<R: io::Read>(
reader: &mut R,
argument: &Argument,
argument: &ArgumentMid,
format: SerdeFormat,
) -> io::Result<Self>
where
Expand Down
12 changes: 6 additions & 6 deletions halo2_backend/src/plonk/permutation/keygen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use group::Curve;
use halo2_middleware::ff::{Field, PrimeField};
use halo2_middleware::zal::impls::H2cEngine;

use super::{Argument, ProvingKey, VerifyingKey};
use super::{ProvingKey, VerifyingKey};
use crate::{
arithmetic::{parallelize, CurveAffine},
plonk::Error,
Expand Down Expand Up @@ -40,7 +40,7 @@ impl Assembly {
Ok(assembly)
}

pub(crate) fn new(n: usize, p: &Argument) -> Self {
pub(crate) fn new(n: usize, p: &ArgumentMid) -> Self {
// Initialize the copy vector to keep track of copy constraints in all
// the permutation arguments.
let mut columns = vec![];
Expand Down Expand Up @@ -121,7 +121,7 @@ impl Assembly {
self,
params: &P,
domain: &EvaluationDomain<C::Scalar>,
p: &Argument,
p: &ArgumentMid,
) -> VerifyingKey<C> {
build_vk(params, domain, p, |i, j| self.mapping[i][j])
}
Expand All @@ -130,7 +130,7 @@ impl Assembly {
self,
params: &P,
domain: &EvaluationDomain<C::Scalar>,
p: &Argument,
p: &ArgumentMid,
) -> ProvingKey<C> {
build_pk(params, domain, p, |i, j| self.mapping[i][j])
}
Expand All @@ -139,7 +139,7 @@ impl Assembly {
pub(crate) fn build_pk<C: CurveAffine, P: Params<C>>(
params: &P,
domain: &EvaluationDomain<C::Scalar>,
p: &Argument,
p: &ArgumentMid,
mapping: impl Fn(usize, usize) -> (usize, usize) + Sync,
) -> ProvingKey<C> {
// Compute [omega^0, omega^1, ..., omega^{params.n - 1}]
Expand Down Expand Up @@ -215,7 +215,7 @@ pub(crate) fn build_pk<C: CurveAffine, P: Params<C>>(
pub(crate) fn build_vk<C: CurveAffine, P: Params<C>>(
params: &P,
domain: &EvaluationDomain<C::Scalar>,
p: &Argument,
p: &ArgumentMid,
mapping: impl Fn(usize, usize) -> (usize, usize) + Sync,
) -> VerifyingKey<C> {
// Compute [omega^0, omega^1, ..., omega^{params.n - 1}]
Expand Down
5 changes: 2 additions & 3 deletions halo2_backend/src/plonk/permutation/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ use group::{
ff::{BatchInvert, Field},
Curve,
};
use halo2_middleware::zal::traits::MsmAccel;
use halo2_middleware::{ff::PrimeField, zal::impls::PlonkEngine};
use halo2_middleware::{permutation::ArgumentMid, zal::traits::MsmAccel};
use rand_core::RngCore;
use std::iter::{self, ExactSizeIterator};

use super::Argument;
use crate::{
arithmetic::{eval_polynomial, parallelize, CurveAffine},
plonk::{self, permutation::ProvingKey, ChallengeBeta, ChallengeGamma, ChallengeX, Error},
Expand Down Expand Up @@ -62,7 +61,7 @@ pub(in crate::plonk) fn permutation_commit<
M: MsmAccel<C>,
>(
engine: &PlonkEngine<C, M>,
arg: &Argument,
arg: &ArgumentMid,
params: &P,
pk: &plonk::ProvingKey<C>,
pkey: &ProvingKey<C>,
Expand Down
11 changes: 7 additions & 4 deletions halo2_backend/src/plonk/permutation/verifier.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use halo2_middleware::ff::{Field, PrimeField};
use halo2_middleware::{
ff::{Field, PrimeField},
permutation::ArgumentMid,
};
use std::iter;

use super::{Argument, VerifyingKey};
use super::VerifyingKey;
use crate::{
arithmetic::CurveAffine,
plonk::{self, ChallengeBeta, ChallengeGamma, ChallengeX, Error},
Expand Down Expand Up @@ -35,7 +38,7 @@ pub(crate) fn permutation_read_product_commitments<
E: EncodedChallenge<C>,
T: TranscriptRead<C, E>,
>(
arg: &Argument,
arg: &ArgumentMid,
vk: &plonk::VerifyingKey<C>,
transcript: &mut T,
) -> Result<Committed<C>, Error> {
Expand Down Expand Up @@ -102,7 +105,7 @@ impl<C: CurveAffine> Evaluated<C> {
pub(in crate::plonk) fn expressions<'a>(
&'a self,
vk: &'a plonk::VerifyingKey<C>,
p: &'a Argument,
p: &'a ArgumentMid,
common: &'a CommonEvaluated<C>,
advice_evals: &'a [C::Scalar],
fixed_evals: &'a [C::Scalar],
Expand Down
14 changes: 7 additions & 7 deletions halo2_backend/src/plonk/shuffle/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ impl<C: CurveAffine> Evaluated<C> {
&|scalar| scalar,
&|var| match var {
VarBack::Challenge(challenge) => challenges[challenge.index],
VarBack::Query(QueryBack {
index, column_type, ..
}) => match column_type {
Any::Fixed => fixed_evals[index],
Any::Advice => advice_evals[index],
Any::Instance => instance_evals[index],
},
VarBack::Query(QueryBack { index, column, .. }) => {
match column.column_type {
Any::Fixed => fixed_evals[index],
Any::Advice => advice_evals[index],
Any::Instance => instance_evals[index],
}
}
},
&|a| -a,
&|a, b| a + b,
Expand Down
7 changes: 6 additions & 1 deletion halo2_backend/src/plonk/vanishing/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,12 @@ impl<C: CurveAffine> Committed<C> {
let h_poly = domain.divide_by_vanishing_poly(h_poly);

// Obtain final h(X) polynomial
let h_poly = domain.extended_to_coeff(h_poly);
let mut h_poly = domain.extended_to_coeff(h_poly);

// Truncate it to match the size of the quotient polynomial; the
// evaluation domain might be slightly larger than necessary because
// it always lies on a power-of-two boundary.
h_poly.truncate(((1u64 << domain.k()) as usize) * domain.get_quotient_poly_degree());

// Split h(X) up into pieces
let h_pieces = h_poly
Expand Down
2 changes: 1 addition & 1 deletion halo2_backend/src/plonk/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ where
gate.poly.evaluate(
&|scalar| scalar,
&|var| match var {
VarBack::Query(query) => match query.column_type {
VarBack::Query(query) => match query.column.column_type {
Any::Fixed => fixed_evals[query.index],
Any::Advice => advice_evals[query.index],
Any::Instance => instance_evals[query.index],
Expand Down
7 changes: 0 additions & 7 deletions halo2_backend/src/poly/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ impl<F: WithSmallOrderMulGroup<3>> EvaluationDomain<F> {
///
/// This function will panic if the provided vector is not the correct
/// length.
// TODO/FIXME: caller should be responsible for truncating
pub fn extended_to_coeff(&self, mut a: Polynomial<F, ExtendedLagrangeCoeff>) -> Vec<F> {
assert_eq!(a.values.len(), self.extended_len());

Expand All @@ -344,12 +343,6 @@ impl<F: WithSmallOrderMulGroup<3>> EvaluationDomain<F> {
// transformation we performed earlier.
self.distribute_powers_zeta(&mut a.values, false);

// Truncate it to match the size of the quotient polynomial; the
// evaluation domain might be slightly larger than necessary because
// it always lies on a power-of-two boundary.
a.values
.truncate((self.n * self.quotient_poly_degree) as usize);

a.values
}

Expand Down
4 changes: 2 additions & 2 deletions halo2_proofs/tests/compress_selectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,13 +494,13 @@ fn test_key_compression() -> Result<(), halo2_proofs::plonk::Error> {
// vk & pk keygen both WITH compression
test_result(
|| test_mycircuit(true, true).expect("should pass"),
"acae50508de5ead584170dd83b139daf40e1026b6debbb78eb05d515173fc2dd",
"44130c6388df3d99263be8da4a280b426dc05f1f315d35d3827347761534bf08",
);

// vk & pk keygen both WITHOUT compression
test_result(
|| test_mycircuit(false, false).expect("should pass"),
"f9c99bd341705ac6a13724a526dd28df0bac1c745e0cde40ab39cab3e1b95309",
"9f58d7a0088fa2c614e8d67bd238f61bc160300e72f5ffd5d52485ed5fb06752",
);

Ok(())
Expand Down
4 changes: 2 additions & 2 deletions halo2_proofs/tests/frontend_backend_split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ fn test_mycircuit_full_legacy() {

proof
},
"78aadfd46b5cc58b90d832ee47e4df57af3dfc28d1457c4ceeb5d0323a72f130",
"44a4bca99aec990b2f382d9c2e1dcc8d8e254d49c2e47cab7556918105346474",
);
}

Expand Down Expand Up @@ -626,6 +626,6 @@ fn test_mycircuit_full_split() {

proof
},
"78aadfd46b5cc58b90d832ee47e4df57af3dfc28d1457c4ceeb5d0323a72f130",
"44a4bca99aec990b2f382d9c2e1dcc8d8e254d49c2e47cab7556918105346474",
);
}
Loading

0 comments on commit 433dfbb

Please sign in to comment.