Skip to content

Commit

Permalink
Get rid of unwrap when possible
Browse files Browse the repository at this point in the history
  • Loading branch information
winderica committed Oct 28, 2024
1 parent 74a08d5 commit 5c6e3c5
Show file tree
Hide file tree
Showing 20 changed files with 198 additions and 152 deletions.
2 changes: 1 addition & 1 deletion examples/noir_full_flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ fn main() {
cur_path.to_str().unwrap()
);

let circuit = load_noir_circuit(circuit_path);
let circuit = load_noir_circuit(circuit_path).unwrap();
let f_circuit = NoirFCircuit {
circuit,
state_len: 1,
Expand Down
4 changes: 2 additions & 2 deletions folding-schemes/src/arith/r1cs/circuits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub mod tests {

let cs = cs.into_inner().unwrap();

let r1cs = extract_r1cs::<Fr>(&cs);
let r1cs = extract_r1cs::<Fr>(&cs).unwrap();
let (w, x) = extract_w_x::<Fr>(&cs);
r1cs.check_relation(&w, &x).unwrap();
let mut z = [vec![Fr::one()], x, w].concat();
Expand Down Expand Up @@ -274,7 +274,7 @@ pub mod tests {
circuit.generate_constraints(cs.clone()).unwrap();
cs.finalize();
let cs = cs.into_inner().unwrap();
let r1cs = extract_r1cs::<Fq>(&cs);
let r1cs = extract_r1cs::<Fq>(&cs).unwrap();
let (w, x) = extract_w_x::<Fq>(&cs);
let z = [vec![Fq::rand(rng)], x, w].concat();

Expand Down
14 changes: 10 additions & 4 deletions folding-schemes/src/arith/r1cs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,14 @@ impl<F: PrimeField> From<CCS<F>> for R1CS<F> {

/// extracts arkworks ConstraintSystem matrices into crate::utils::vec::SparseMatrix format as R1CS
/// struct.
pub fn extract_r1cs<F: PrimeField>(cs: &ConstraintSystem<F>) -> R1CS<F> {
let m = cs.to_matrices().unwrap();
pub fn extract_r1cs<F: PrimeField>(cs: &ConstraintSystem<F>) -> Result<R1CS<F>, Error> {
let m = cs.to_matrices().ok_or_else(|| {
Error::ConversionError(
"ConstraintSystem".into(),
"ConstraintMatrices".into(),
"The matrices have not been generated yet".into(),
)
})?;

let n_rows = cs.num_constraints;
let n_cols = cs.num_instance_variables + cs.num_witness_variables; // cs.num_instance_variables already counts the 1
Expand All @@ -145,12 +151,12 @@ pub fn extract_r1cs<F: PrimeField>(cs: &ConstraintSystem<F>) -> R1CS<F> {
coeffs: m.c,
};

R1CS::<F> {
Ok(R1CS::<F> {
l: cs.num_instance_variables - 1, // -1 to subtract the first '1'
A,
B,
C,
}
})
}

/// extracts the witness and the public inputs from arkworks ConstraintSystem.
Expand Down
10 changes: 7 additions & 3 deletions folding-schemes/src/folding/circuits/cyclefold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,11 @@ where
let U_vec = self.to_native_sponge_field_elements()?;
sponge.absorb(&pp_hash)?;
sponge.absorb(&U_vec)?;
Ok((sponge.squeeze_field_elements(1)?.pop().unwrap(), U_vec))
Ok((
// `unwrap` is safe because the sponge is guaranteed to return a single element
sponge.squeeze_field_elements(1)?.pop().unwrap(),
U_vec,
))
}
}

Expand Down Expand Up @@ -307,13 +311,13 @@ where
Ok(CycleFoldCommittedInstanceVar {
cmE: cmT.scalar_mul_le(r_bits.iter())? + ci1.cmE,
cmW: ci1.cmW + ci2.cmW.scalar_mul_le(r_bits.iter())?,
u: ci1.u.add_no_align(&r_nonnat).modulo::<CF1<C>>()?,
u: ci1.u.add_no_align(&r_nonnat)?.modulo::<CF1<C>>()?,
x: ci1
.x
.iter()
.zip(ci2.x)
.map(|(a, b)| {
a.add_no_align(&r_nonnat.mul_no_align(&b)?)
a.add_no_align(&r_nonnat.mul_no_align(&b)?)?
.modulo::<CF1<C>>()
})
.collect::<Result<Vec<_>, _>>()?,
Expand Down
8 changes: 7 additions & 1 deletion folding-schemes/src/folding/circuits/decider/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,13 @@ impl EvalGadget {
let mut v = v.to_vec();
v.resize(v.len().next_power_of_two(), FpVar::zero());
let n = v.len() as u64;
let gen = F::get_root_of_unity(n).unwrap();
let gen = F::get_root_of_unity(n).ok_or(SynthesisError::PolynomialDegreeTooLarge)?;
// `unwrap` below is safe because `Radix2DomainVar::new` only fails if
// `offset.enforce_not_equal(&FpVar::zero())` returns an error.
// But in our case, `offset` is `FpVar::one()`, i.e., both operands of
// `enforce_not_equal` are constants.
// Consequently, `FpVar`'s implementation of `enforce_not_equal` will
// always return `Ok(())`.
let domain = Radix2DomainVar::new(gen, log2(v.len()) as u64, FpVar::one()).unwrap();

let evaluations_var = EvaluationsVar::from_vec_and_domain(v, domain, true);
Expand Down
12 changes: 12 additions & 0 deletions folding-schemes/src/folding/circuits/nonnative/affine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@ impl<C: CurveGroup> R1CSVar<C::ScalarField> for NonNativeAffineVar<C> {
let y = <C::BaseField as Field>::BasePrimeField::from_le_bytes_mod_order(
&self.y.value()?.to_bytes_le(),
);
// Below is a workaround to convert the `x` and `y` coordinates to a
// point. This is because the `CurveGroup` trait does not provide a
// method to construct a point from `BaseField` elements.
let mut bytes = vec![];
// `unwrap` below is safe because serialization of a `PrimeField` value
// only fails if the serialization flag has more than 8 bits, but here
// we call `serialize_uncompressed` which uses an empty flag.
x.serialize_uncompressed(&mut bytes).unwrap();
// `unwrap` below is also safe, because the bit size of `SWFlags` is 2.
y.serialize_with_flags(
&mut bytes,
if x.is_zero() && y.is_zero() {
Expand All @@ -81,6 +88,9 @@ impl<C: CurveGroup> R1CSVar<C::ScalarField> for NonNativeAffineVar<C> {
},
)
.unwrap();
// `unwrap` below is safe because `bytes` is constructed from the `x`
// and `y` coordinates of a valid point, and these coordinates are
// serialized in the same way as the `CurveGroup` implementation.
Ok(C::deserialize_uncompressed_unchecked(&bytes[..]).unwrap())
}
}
Expand Down Expand Up @@ -170,6 +180,8 @@ impl<C: CurveGroup> Inputize<C::ScalarField, NonNativeAffineVar<C>> for C {

impl<C: CurveGroup> NonNativeAffineVar<C> {
pub fn zero() -> Self {
// `unwrap` below is safe because we are allocating a constant value,
// which is guaranteed to succeed.
Self::new_constant(ConstraintSystemRef::None, C::zero()).unwrap()
}
}
Expand Down
56 changes: 34 additions & 22 deletions folding-schemes/src/folding/circuits/nonnative/uint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ pub struct LimbVar<F: PrimeField> {
impl<F: PrimeField, B: AsRef<[Boolean<F>]>> From<B> for LimbVar<F> {
fn from(bits: B) -> Self {
Self {
// `Boolean::le_bits_to_fp_var` will return an error if the internal
// invocation of `Boolean::enforce_in_field_le` fails.
// However, this method is only called when the length of `bits` is
// greater than `F::MODULUS_BIT_SIZE`, which should not happen in
// our case where `bits` is guaranteed to be short.
v: Boolean::le_bits_to_fp_var(bits.as_ref()).unwrap(),
ub: (BigUint::one() << bits.as_ref().len()) - BigUint::one(),
}
Expand Down Expand Up @@ -220,7 +225,7 @@ impl<F: PrimeField> AllocVar<BoundedBigUint, F> for NonNativeUintVar<F> {
.collect::<Vec<_>>()
.chunks(Self::bits_per_limb())
{
let limb = F::from_bigint(F::BigInt::from_bits_le(chunk)).unwrap();
let limb = F::from(F::BigInt::from_bits_le(chunk));
let limb = FpVar::new_variable(cs.clone(), || Ok(limb), mode)?;
Self::enforce_bit_length(&limb, chunk.len())?;
limbs.push(LimbVar {
Expand All @@ -242,12 +247,15 @@ impl<F: PrimeField, G: Field> AllocVar<G, F> for NonNativeUintVar<F> {
let cs = cs.into().cs();
let v = f()?;
assert_eq!(G::extension_degree(), 1);
// `unwrap` is safe because `G` is a field with extension degree 1, and
// thus `G::to_base_prime_field_elements` should return an iterator with
// exactly one element.
let v = v.borrow().to_base_prime_field_elements().next().unwrap();

let mut limbs = vec![];

for chunk in v.into_bigint().to_bits_le().chunks(Self::bits_per_limb()) {
let limb = F::from_bigint(F::BigInt::from_bits_le(chunk)).unwrap();
let limb = F::from(F::BigInt::from_bits_le(chunk));
let limb = FpVar::new_variable(cs.clone(), || Ok(limb), mode)?;
Self::enforce_bit_length(&limb, chunk.len())?;
limbs.push(LimbVar {
Expand All @@ -263,13 +271,16 @@ impl<F: PrimeField, G: Field> AllocVar<G, F> for NonNativeUintVar<F> {
impl<F: PrimeField, T: Field> Inputize<F, NonNativeUintVar<F>> for T {
fn inputize(&self) -> Vec<F> {
assert_eq!(T::extension_degree(), 1);
// `unwrap` is safe because `T` is a field with extension degree 1, and
// thus `T::to_base_prime_field_elements` should return an iterator with
// exactly one element.
self.to_base_prime_field_elements()
.next()
.unwrap()
.into_bigint()
.to_bits_le()
.chunks(NonNativeUintVar::<F>::bits_per_limb())
.map(|chunk| F::from_bigint(F::BigInt::from_bits_le(chunk)).unwrap())
.map(|chunk| F::from(F::BigInt::from_bits_le(chunk)))
.collect()
}
}
Expand Down Expand Up @@ -451,7 +462,7 @@ impl<F: PrimeField> NonNativeUintVar<F> {
// (i.e., all of them are "non-negative"), implying that all
// limbs should be zero to make the sum zero.
LimbVar::add_many(&remaining_limbs[1..])
.unwrap()
.ok_or(SynthesisError::Unsatisfiable)?
.v
.enforce_equal(&FpVar::zero())?;
remaining_limbs[0].v.clone()
Expand Down Expand Up @@ -622,15 +633,15 @@ impl<F: PrimeField> NonNativeUintVar<F> {
}

/// Compute `self + other`, without aligning the limbs.
pub fn add_no_align(&self, other: &Self) -> Self {
pub fn add_no_align(&self, other: &Self) -> Result<Self, SynthesisError> {
let mut z = vec![LimbVar::zero(); max(self.0.len(), other.0.len())];
for (i, v) in self.0.iter().enumerate() {
z[i] = z[i].add(v).unwrap();
z[i] = z[i].add(v).ok_or(SynthesisError::Unsatisfiable)?;
}
for (i, v) in other.0.iter().enumerate() {
z[i] = z[i].add(v).unwrap();
z[i] = z[i].add(v).ok_or(SynthesisError::Unsatisfiable)?;
}
Self(z)
Ok(Self(z))
}

/// Compute `self * other`, without aligning the limbs.
Expand All @@ -651,7 +662,7 @@ impl<F: PrimeField> NonNativeUintVar<F> {
)
})
.collect::<Option<Vec<_>>>()
.unwrap();
.ok_or(SynthesisError::Unsatisfiable)?;
return Ok(Self(z));
}
let cs = self.cs().or(other.cs());
Expand Down Expand Up @@ -754,7 +765,7 @@ impl<F: PrimeField> NonNativeUintVar<F> {
let m = Self::new_constant(cs.clone(), BoundedBigUint(m, M::MODULUS_BIT_SIZE as usize))?;
// Enforce `self = q * m + r`
q.mul_no_align(&m)?
.add_no_align(&r)
.add_no_align(&r)?
.enforce_equal_unaligned(self)?;
// Enforce `r < m` (and `r >= 0` already holds)
r.enforce_lt(&m)?;
Expand Down Expand Up @@ -790,8 +801,8 @@ impl<F: PrimeField> NonNativeUintVar<F> {

let zero = Self::new_constant(cs.clone(), BoundedBigUint(BigUint::zero(), bits))?;
let m = Self::new_constant(cs.clone(), BoundedBigUint(m, M::MODULUS_BIT_SIZE as usize))?;
let l = self.add_no_align(&is_ge.select(&zero, &q)?.mul_no_align(&m)?);
let r = other.add_no_align(&is_ge.select(&q, &zero)?.mul_no_align(&m)?);
let l = self.add_no_align(&is_ge.select(&zero, &q)?.mul_no_align(&m)?)?;
let r = other.add_no_align(&is_ge.select(&q, &zero)?.mul_no_align(&m)?)?;
// If `self >= other`, enforce `self = other + q * m`
// Otherwise, enforce `self + q * m = other`
// Soundness holds because if `self` and `other` are not congruent, then
Expand Down Expand Up @@ -841,6 +852,9 @@ pub(super) fn nonnative_field_to_field_elements<TargetField: Field, BaseField: P
f: &TargetField,
) -> Vec<BaseField> {
assert_eq!(TargetField::extension_degree(), 1);
// `unwrap` is safe because `TargetField` is a field with extension degree
// 1, and thus `TargetField::to_base_prime_field_elements` should return an
// iterator with exactly one element.
let bits = f
.to_base_prime_field_elements()
.next()
Expand Down Expand Up @@ -871,11 +885,10 @@ pub(super) fn nonnative_field_to_field_elements<TargetField: Field, BaseField: P

impl<F: PrimeField> VectorGadget<NonNativeUintVar<F>> for [NonNativeUintVar<F>] {
fn add(&self, other: &Self) -> Result<Vec<NonNativeUintVar<F>>, SynthesisError> {
Ok(self
.iter()
self.iter()
.zip(other.iter())
.map(|(x, y)| x.add_no_align(y))
.collect())
.collect()
}

fn hadamard(&self, other: &Self) -> Result<Vec<NonNativeUintVar<F>>, SynthesisError> {
Expand All @@ -898,8 +911,7 @@ impl<CF: PrimeField> MatrixGadget<NonNativeUintVar<CF>> for SparseMatrixVar<NonN
&self,
v: &[NonNativeUintVar<CF>],
) -> Result<Vec<NonNativeUintVar<CF>>, SynthesisError> {
Ok(self
.coeffs
self.coeffs
.iter()
.map(|row| {
let len = row
Expand All @@ -911,7 +923,7 @@ impl<CF: PrimeField> MatrixGadget<NonNativeUintVar<CF>> for SparseMatrixVar<NonN
// that results in more flattened `LinearCombination`s.
// Consequently, `ConstraintSystem::inline_all_lcs` costs less
// time, thus making trusted setup and proof generation faster.
let limbs = (0..len)
(0..len)
.map(|i| {
LimbVar::add_many(
&row.iter()
Expand All @@ -924,10 +936,10 @@ impl<CF: PrimeField> MatrixGadget<NonNativeUintVar<CF>> for SparseMatrixVar<NonN
)
})
.collect::<Option<Vec<_>>>()
.unwrap();
NonNativeUintVar(limbs)
.ok_or(SynthesisError::Unsatisfiable)
.map(NonNativeUintVar)
})
.collect())
.collect::<Result<Vec<_>, _>>()
}
}

Expand Down Expand Up @@ -1046,7 +1058,7 @@ mod tests {
let mut r_var =
NonNativeUintVar::new_constant(cs.clone(), BoundedBigUint(BigUint::zero(), 0))?;
for (a, b) in a_var.into_iter().zip(b_var.into_iter()) {
r_var = r_var.add_no_align(&a.mul_no_align(&b)?);
r_var = r_var.add_no_align(&a.mul_no_align(&b)?)?;
}
r_var.enforce_congruent::<Fq>(&c_var)?;

Expand Down
14 changes: 6 additions & 8 deletions folding-schemes/src/folding/hypernova/circuits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,11 +580,9 @@ where
M: vec![],
};
let mut augmented_f_circuit = Self::default(poseidon_config, F, initial_ccs)?;
if ccs.is_some() {
augmented_f_circuit.ccs = ccs.unwrap();
} else {
augmented_f_circuit.ccs = augmented_f_circuit.upper_bound_ccs()?;
}
augmented_f_circuit.ccs = ccs
.ok_or(())
.or_else(|_| augmented_f_circuit.upper_bound_ccs())?;
Ok(augmented_f_circuit)
}

Expand All @@ -593,7 +591,7 @@ where
/// For a stable FCircuit circuit, the CCS parameters can be computed in advance and can be
/// feed in as parameter for the AugmentedFCircuit::empty method to avoid computing them there.
pub fn upper_bound_ccs(&self) -> Result<CCS<C1::ScalarField>, Error> {
let r1cs = get_r1cs_from_cs::<CF1<C1>>(self.clone()).unwrap();
let r1cs = get_r1cs_from_cs::<CF1<C1>>(self.clone())?;
let mut ccs = CCS::from(r1cs);

let z_0 = vec![C1::ScalarField::zero(); self.F.state_len()];
Expand Down Expand Up @@ -682,7 +680,7 @@ where
self.clone().generate_constraints(cs.clone())?;
cs.finalize();
let cs = cs.into_inner().ok_or(Error::NoInnerConstraintSystem)?;
let r1cs = extract_r1cs::<C1::ScalarField>(&cs);
let r1cs = extract_r1cs::<C1::ScalarField>(&cs)?;
let ccs = CCS::from(r1cs);

Ok((cs, ccs))
Expand Down Expand Up @@ -1223,7 +1221,7 @@ mod tests {
.into_inner()
.ok_or(Error::NoInnerConstraintSystem)
.unwrap();
let cf_r1cs = extract_r1cs::<Fq>(&cs2);
let cf_r1cs = extract_r1cs::<Fq>(&cs2).unwrap();
println!("CF m x n: {} x {}", cf_r1cs.A.n_rows, cf_r1cs.A.n_cols);

let (pedersen_params, _) =
Expand Down
8 changes: 4 additions & 4 deletions folding-schemes/src/folding/hypernova/decider_eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,11 @@ where
prep_param: Self::PreprocessorParam,
fs: FS,
) -> Result<(Self::ProverParam, Self::VerifierParam), Error> {
let circuit = DeciderEthCircuit::<C1, C2, GC2>::try_from(HyperNova::from(fs)).unwrap();
let circuit = DeciderEthCircuit::<C1, C2, GC2>::try_from(HyperNova::from(fs))?;

// get the Groth16 specific setup for the circuit
let (g16_pk, g16_vk) = S::circuit_specific_setup(circuit, &mut rng).unwrap();
let (g16_pk, g16_vk) = S::circuit_specific_setup(circuit, &mut rng)
.map_err(|e| Error::SNARKSetupFail(e.to_string()))?;

// get the FoldingScheme prover & verifier params from HyperNova
#[allow(clippy::type_complexity)]
Expand Down Expand Up @@ -133,8 +134,7 @@ where
) -> Result<Self::Proof, Error> {
let (snark_pk, cs_pk): (S::ProvingKey, CS1::ProverParams) = pp;

let circuit =
DeciderEthCircuit::<C1, C2, GC2>::try_from(HyperNova::from(folding_scheme)).unwrap();
let circuit = DeciderEthCircuit::<C1, C2, GC2>::try_from(HyperNova::from(folding_scheme))?;

let rho = circuit.randomness;

Expand Down
Loading

0 comments on commit 5c6e3c5

Please sign in to comment.