From 467485c3f065522f278e93e6b8fee94d963ceee2 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 22 Jul 2021 14:33:02 +0200 Subject: [PATCH 01/10] Remove accumulator wires in GMiMCGate --- src/gadgets/hash.rs | 9 -------- src/gates/gmimc.rs | 54 ++++---------------------------------------- src/merkle_proofs.rs | 15 ++---------- 3 files changed, 7 insertions(+), 71 deletions(-) diff --git a/src/gadgets/hash.rs b/src/gadgets/hash.rs index fae6d433..136bfef8 100644 --- a/src/gadgets/hash.rs +++ b/src/gadgets/hash.rs @@ -22,15 +22,6 @@ impl, const D: usize> CircuitBuilder { }); self.route(zero, swap_wire); - // The old accumulator wire doesn't matter, since we won't read the new accumulator wire. - // We do have to set it to something though, so we'll arbitrary pick 0. - let old_acc_wire = GMiMCGate::::WIRE_INDEX_ACCUMULATOR_OLD; - let old_acc_wire = Target::Wire(Wire { - gate, - input: old_acc_wire, - }); - self.route(zero, old_acc_wire); - // Route input wires. for i in 0..12 { let in_wire = GMiMCGate::::wire_input(i); diff --git a/src/gates/gmimc.rs b/src/gates/gmimc.rs index 0404884b..651879a5 100644 --- a/src/gates/gmimc.rs +++ b/src/gates/gmimc.rs @@ -48,22 +48,18 @@ impl, const D: usize, const R: usize> GMiMCGate { W + i } - /// Used to incrementally compute the index of the leaf based on a series of swap bits. - pub const WIRE_INDEX_ACCUMULATOR_OLD: usize = 2 * W; - pub const WIRE_INDEX_ACCUMULATOR_NEW: usize = 2 * W + 1; - /// If this is set to 1, the first four inputs will be swapped with the next four inputs. This /// is useful for ordering hashes in Merkle proofs. Otherwise, this should be set to 0. - pub const WIRE_SWAP: usize = 2 * W + 2; + pub const WIRE_SWAP: usize = 2 * W; /// A wire which stores the input to the `i`th cubing. fn wire_cubing_input(i: usize) -> usize { - 2 * W + 3 + i + 2 * W + 1 + i } /// End of wire indices, exclusive. fn end() -> usize { - 2 * W + 3 + R + 2 * W + 1 + R } } @@ -79,11 +75,6 @@ impl, const D: usize, const R: usize> Gate for GMiMCGate< let swap = vars.local_wires[Self::WIRE_SWAP]; constraints.push(swap * (swap - F::Extension::ONE)); - let old_index_acc = vars.local_wires[Self::WIRE_INDEX_ACCUMULATOR_OLD]; - let new_index_acc = vars.local_wires[Self::WIRE_INDEX_ACCUMULATOR_NEW]; - let computed_new_index_acc = F::Extension::TWO * old_index_acc + swap; - constraints.push(computed_new_index_acc - new_index_acc); - let mut state = Vec::with_capacity(12); for i in 0..4 { let a = vars.local_wires[i]; @@ -131,13 +122,6 @@ impl, const D: usize, const R: usize> Gate for GMiMCGate< let swap = vars.local_wires[Self::WIRE_SWAP]; constraints.push(builder.mul_sub_extension(swap, swap, swap)); - let old_index_acc = vars.local_wires[Self::WIRE_INDEX_ACCUMULATOR_OLD]; - let new_index_acc = vars.local_wires[Self::WIRE_INDEX_ACCUMULATOR_NEW]; - // computed_new_index_acc = 2 * old_index_acc + swap - let two = builder.two_extension(); - let computed_new_index_acc = builder.mul_add_extension(two, old_index_acc, swap); - constraints.push(builder.sub_extension(computed_new_index_acc, new_index_acc)); - let mut state = Vec::with_capacity(12); for i in 0..4 { let a = vars.local_wires[i]; @@ -221,12 +205,11 @@ impl, const D: usize, const R: usize> SimpleGenerator for GMiMCGenerator { fn dependencies(&self) -> Vec { - let mut dep_input_indices = Vec::with_capacity(W + 2); + let mut dep_input_indices = Vec::with_capacity(W + 1); for i in 0..W { dep_input_indices.push(GMiMCGate::::wire_input(i)); } dep_input_indices.push(GMiMCGate::::WIRE_SWAP); - dep_input_indices.push(GMiMCGate::::WIRE_INDEX_ACCUMULATOR_OLD); dep_input_indices .into_iter() @@ -240,7 +223,7 @@ impl, const D: usize, const R: usize> SimpleGenerator } fn run_once(&self, witness: &PartialWitness) -> GeneratedValues { - let mut result = GeneratedValues::with_capacity(R + W + 1); + let mut result = GeneratedValues::with_capacity(R + W); let mut state = (0..W) .map(|i| { @@ -262,20 +245,6 @@ impl, const D: usize, const R: usize> SimpleGenerator } } - // Update the index accumulator. - let old_index_acc_value = witness.get_wire(Wire { - gate: self.gate_index, - input: GMiMCGate::::WIRE_INDEX_ACCUMULATOR_OLD, - }); - let new_index_acc_value = F::TWO * old_index_acc_value + swap_value; - result.set_wire( - Wire { - gate: self.gate_index, - input: GMiMCGate::::WIRE_INDEX_ACCUMULATOR_NEW, - }, - new_index_acc_value, - ); - // Value that is implicitly added to each element. // See https://affine.group/2020/02/starkware-challenge let mut addition_buffer = F::ZERO; @@ -349,13 +318,6 @@ mod tests { let permutation_inputs = (0..W).map(F::from_canonical_usize).collect::>(); let mut witness = PartialWitness::new(); - witness.set_wire( - Wire { - gate: 0, - input: Gate::WIRE_INDEX_ACCUMULATOR_OLD, - }, - F::from_canonical_usize(7), - ); witness.set_wire( Wire { gate: 0, @@ -386,12 +348,6 @@ mod tests { }); assert_eq!(out, expected_outputs[i]); } - - let acc_new = witness.get_wire(Wire { - gate: 0, - input: Gate::WIRE_INDEX_ACCUMULATOR_NEW, - }); - assert_eq!(acc_new, F::from_canonical_usize(7 * 2)); } #[test] diff --git a/src/merkle_proofs.rs b/src/merkle_proofs.rs index 03d5099d..66558ba7 100644 --- a/src/merkle_proofs.rs +++ b/src/merkle_proofs.rs @@ -68,6 +68,7 @@ impl, const D: usize> CircuitBuilder { proof: &MerkleProofTarget, ) { let zero = self.zero(); + let two = self.two(); let height = proof.siblings.len(); let purported_index_bits = self.split_le_virtual(leaf_index, height); @@ -85,19 +86,7 @@ impl, const D: usize> CircuitBuilder { }); self.generate_copy(bit, swap_wire); - let old_acc_wire = GMiMCGate::::WIRE_INDEX_ACCUMULATOR_OLD; - let old_acc_wire = Target::Wire(Wire { - gate, - input: old_acc_wire, - }); - self.route(acc_leaf_index, old_acc_wire); - - let new_acc_wire = GMiMCGate::::WIRE_INDEX_ACCUMULATOR_NEW; - let new_acc_wire = Target::Wire(Wire { - gate, - input: new_acc_wire, - }); - acc_leaf_index = new_acc_wire; + acc_leaf_index = self.mul_add(two, acc_leaf_index, bit); let input_wires = (0..12) .map(|i| { From 15a64017dc98cfec4ed4d7e080079e64a220123b Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 22 Jul 2021 14:42:42 +0200 Subject: [PATCH 02/10] We need only 126 wires now --- src/circuit_data.rs | 2 +- src/gates/gmimc.rs | 2 +- src/recursive_verifier.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/circuit_data.rs b/src/circuit_data.rs index 912f05cc..787c446d 100644 --- a/src/circuit_data.rs +++ b/src/circuit_data.rs @@ -56,7 +56,7 @@ impl CircuitConfig { pub(crate) fn large_config() -> Self { Self { - num_wires: 134, + num_wires: 126, num_routed_wires: 34, security_bits: 128, rate_bits: 3, diff --git a/src/gates/gmimc.rs b/src/gates/gmimc.rs index 651879a5..a3c26608 100644 --- a/src/gates/gmimc.rs +++ b/src/gates/gmimc.rs @@ -310,7 +310,7 @@ mod tests { let gate = Gate::with_constants(constants.clone()); let config = CircuitConfig { - num_wires: 134, + num_wires: 126, num_routed_wires: 200, ..Default::default() }; diff --git a/src/recursive_verifier.rs b/src/recursive_verifier.rs index 66b63e7f..0e4080ea 100644 --- a/src/recursive_verifier.rs +++ b/src/recursive_verifier.rs @@ -363,7 +363,7 @@ mod tests { type F = CrandallField; const D: usize = 4; let config = CircuitConfig { - num_wires: 134, + num_wires: 126, num_routed_wires: 33, security_bits: 128, rate_bits: 3, From 22fcc3bc0640595c5452eb40d87842aaf60476f0 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 22 Jul 2021 15:10:55 +0200 Subject: [PATCH 03/10] Pass bits around --- src/fri/recursive_verifier.rs | 23 +++++++++++++---------- src/gadgets/range_check.rs | 8 ++++++++ src/merkle_proofs.rs | 18 +++++------------- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/src/fri/recursive_verifier.rs b/src/fri/recursive_verifier.rs index 588ad423..4c3a3704 100644 --- a/src/fri/recursive_verifier.rs +++ b/src/fri/recursive_verifier.rs @@ -20,7 +20,7 @@ impl, const D: usize> CircuitBuilder { fn compute_evaluation( &mut self, x: Target, - old_x_index: Target, + old_x_index_bits: &[Target], arity_bits: usize, last_evals: &[ExtensionTarget], beta: ExtensionTarget, @@ -33,7 +33,7 @@ impl, const D: usize> CircuitBuilder { // The evaluation vector needs to be reordered first. let mut evals = last_evals.to_vec(); reverse_index_bits_in_place(&mut evals); - let mut old_x_index_bits = self.split_le(old_x_index, arity_bits); + let mut old_x_index_bits = old_x_index_bits.to_vec(); old_x_index_bits.reverse(); // Want `g^(arity - rev_old_x_index)` as in the out-of-circuit version. // Compute it as `g^(arity-1-rev_old_x_index) * g`, where the first term is gotten using two's complement. @@ -148,7 +148,7 @@ impl, const D: usize> CircuitBuilder { fn fri_verify_initial_proof( &mut self, - x_index: Target, + x_index_bits: &[Target], proof: &FriInitialTreeProofTarget, initial_merkle_roots: &[HashTarget], ) { @@ -161,7 +161,7 @@ impl, const D: usize> CircuitBuilder { context!( self, &format!("verify {}'th initial Merkle proof", i), - self.verify_merkle_proof(evals.clone(), x_index, root, merkle_proof) + self.verify_merkle_proof(evals.clone(), x_index_bits, root, merkle_proof) ); } } @@ -255,18 +255,19 @@ impl, const D: usize> CircuitBuilder { // TODO: Do we need to range check `x_index` to a target smaller than `p`? let mut x_index = challenger.get_challenge(self); x_index = self.split_low_high(x_index, n_log, 64).0; + let mut x_index_bits = self.low_bits(x_index, n_log, 64); let mut x_index_num_bits = n_log; let mut domain_size = n; context!( self, "check FRI initial proof", self.fri_verify_initial_proof( - x_index, + &x_index_bits, &round_proof.initial_trees_proof, initial_merkle_roots, ) ); - let mut old_x_index = self.zero(); + let mut old_x_index_bits = Vec::new(); // `subgroup_x` is `subgroup[x_index]`, i.e., the actual field element in the domain. let mut subgroup_x = context!(self, "compute x from its index", { @@ -302,7 +303,7 @@ impl, const D: usize> CircuitBuilder { "infer evaluation using interpolation", self.compute_evaluation( subgroup_x, - old_x_index, + &old_x_index_bits, config.reduction_arity_bits[i - 1], last_evals, betas[i - 1], @@ -313,13 +314,15 @@ impl, const D: usize> CircuitBuilder { // Insert P(y) into the evaluation vector, since it wasn't included by the prover. let (low_x_index, high_x_index) = self.split_low_high(x_index, arity_bits, x_index_num_bits); + let high_x_index_bits = x_index_bits.split_off(arity_bits); + old_x_index_bits = x_index_bits; evals = self.insert(low_x_index, e_x, evals); context!( self, "verify FRI round Merkle proof.", self.verify_merkle_proof( flatten_target(&evals), - high_x_index, + &high_x_index_bits, proof.commit_phase_merkle_roots[i], &round_proof.steps[i].merkle_proof, ) @@ -331,9 +334,9 @@ impl, const D: usize> CircuitBuilder { subgroup_x = self.exp_power_of_2(subgroup_x, config.reduction_arity_bits[i - 1]); } domain_size = next_domain_size; - old_x_index = low_x_index; x_index = high_x_index; x_index_num_bits -= arity_bits; + x_index_bits = high_x_index_bits; } let last_evals = evaluations.last().unwrap(); @@ -343,7 +346,7 @@ impl, const D: usize> CircuitBuilder { "infer final evaluation using interpolation", self.compute_evaluation( subgroup_x, - old_x_index, + &old_x_index_bits, final_arity_bits, last_evals, *betas.last().unwrap(), diff --git a/src/gadgets/range_check.rs b/src/gadgets/range_check.rs index c0848af8..261e5c7d 100644 --- a/src/gadgets/range_check.rs +++ b/src/gadgets/range_check.rs @@ -14,6 +14,14 @@ impl, const D: usize> CircuitBuilder { self.route(x, sum); } + /// Returns the first `n_log` little-endian bits of `x`. + /// Note: `x` is assumed to be range-checked for having `num_bits` bits. + pub fn low_bits(&mut self, x: Target, n_log: usize, num_bits: usize) -> Vec { + let mut res = self.split_le(x, num_bits); + res.truncate(n_log); + res + } + /// Returns `(a,b)` such that `x = a + 2^n_log * b` with `a < 2^n_log`. /// `x` is assumed to be range-checked for having `num_bits` bits. pub fn split_low_high(&mut self, x: Target, n_log: usize, num_bits: usize) -> (Target, Target) { diff --git a/src/merkle_proofs.rs b/src/merkle_proofs.rs index 66558ba7..e971b3c5 100644 --- a/src/merkle_proofs.rs +++ b/src/merkle_proofs.rs @@ -59,23 +59,20 @@ pub(crate) fn verify_merkle_proof( impl, const D: usize> CircuitBuilder { /// Verifies that the given leaf data is present at the given index in the Merkle tree with the - /// given root. + /// given root. The index is given by it's little-endian bits. pub(crate) fn verify_merkle_proof( &mut self, leaf_data: Vec, - leaf_index: Target, + leaf_index_bits: &[Target], merkle_root: HashTarget, proof: &MerkleProofTarget, ) { let zero = self.zero(); - let two = self.two(); let height = proof.siblings.len(); - let purported_index_bits = self.split_le_virtual(leaf_index, height); let mut state: HashTarget = self.hash_or_noop(leaf_data); - let mut acc_leaf_index = zero; - for (bit, &sibling) in purported_index_bits.into_iter().zip(&proof.siblings) { + for (&bit, &sibling) in leaf_index_bits.iter().zip(&proof.siblings) { let gate = self .add_gate_no_constants(GMiMCGate::::with_automatic_constants()); @@ -86,8 +83,6 @@ impl, const D: usize> CircuitBuilder { }); self.generate_copy(bit, swap_wire); - acc_leaf_index = self.mul_add(two, acc_leaf_index, bit); - let input_wires = (0..12) .map(|i| { Target::Wire(Wire { @@ -115,10 +110,6 @@ impl, const D: usize> CircuitBuilder { ) } - // TODO: this is far from optimal. - let leaf_index_rev = self.reverse_limbs::<2>(leaf_index, height); - self.assert_equal(acc_leaf_index, leaf_index_rev); - self.named_assert_hashes_equal(state, merkle_root, "check Merkle root".into()) } @@ -180,13 +171,14 @@ mod tests { pw.set_hash_target(root_t, tree.root); let i_c = builder.constant(F::from_canonical_usize(i)); + let i_bits = builder.split_le(i_c, log_n); let data = builder.add_virtual_targets(tree.leaves[i].len()); for j in 0..data.len() { pw.set_target(data[j], tree.leaves[i][j]); } - builder.verify_merkle_proof(data, i_c, root_t, &proof_t); + builder.verify_merkle_proof(data, &i_bits, root_t, &proof_t); let data = builder.build(); let proof = data.prove(pw)?; From c729a3c2353a3c2e8d8dd0c39c24f7a9f3b4b5e8 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 22 Jul 2021 16:07:07 +0200 Subject: [PATCH 04/10] Remove all non-bits indices in the FRI verifier --- src/fri/recursive_verifier.rs | 11 ++--- src/gadgets/split_base.rs | 89 +++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 8 deletions(-) diff --git a/src/fri/recursive_verifier.rs b/src/fri/recursive_verifier.rs index 4c3a3704..12246504 100644 --- a/src/fri/recursive_verifier.rs +++ b/src/fri/recursive_verifier.rs @@ -253,10 +253,8 @@ impl, const D: usize> CircuitBuilder { let config = &common_data.config.fri_config; let n_log = log2_strict(n); // TODO: Do we need to range check `x_index` to a target smaller than `p`? - let mut x_index = challenger.get_challenge(self); - x_index = self.split_low_high(x_index, n_log, 64).0; + let x_index = challenger.get_challenge(self); let mut x_index_bits = self.low_bits(x_index, n_log, 64); - let mut x_index_num_bits = n_log; let mut domain_size = n; context!( self, @@ -274,7 +272,7 @@ impl, const D: usize> CircuitBuilder { let g = self.constant(F::MULTIPLICATIVE_GROUP_GENERATOR); let phi = self.constant(F::primitive_root_of_unity(n_log)); - let reversed_x = self.reverse_limbs::<2>(x_index, n_log); + let reversed_x = self.base_sum(x_index_bits.iter().rev()); let phi = self.exp(phi, reversed_x, n_log); self.mul(g, phi) }); @@ -312,10 +310,9 @@ impl, const D: usize> CircuitBuilder { }; let mut evals = round_proof.steps[i].evals.clone(); // Insert P(y) into the evaluation vector, since it wasn't included by the prover. - let (low_x_index, high_x_index) = - self.split_low_high(x_index, arity_bits, x_index_num_bits); let high_x_index_bits = x_index_bits.split_off(arity_bits); old_x_index_bits = x_index_bits; + let low_x_index = self.base_sum(old_x_index_bits.iter()); evals = self.insert(low_x_index, e_x, evals); context!( self, @@ -334,8 +331,6 @@ impl, const D: usize> CircuitBuilder { subgroup_x = self.exp_power_of_2(subgroup_x, config.reduction_arity_bits[i - 1]); } domain_size = next_domain_size; - x_index = high_x_index; - x_index_num_bits -= arity_bits; x_index_bits = high_x_index_bits; } diff --git a/src/gadgets/split_base.rs b/src/gadgets/split_base.rs index 1223170e..da26c1db 100644 --- a/src/gadgets/split_base.rs +++ b/src/gadgets/split_base.rs @@ -1,7 +1,12 @@ +use std::borrow::Borrow; + use crate::circuit_builder::CircuitBuilder; use crate::field::extension_field::Extendable; +use crate::field::field::Field; use crate::gates::base_sum::BaseSumGate; +use crate::generator::{GeneratedValues, SimpleGenerator}; use crate::target::Target; +use crate::witness::PartialWitness; impl, const D: usize> CircuitBuilder { /// Split the given element into a list of targets, where each one represents a @@ -33,11 +38,63 @@ impl, const D: usize> CircuitBuilder { Target::wire(gate, BaseSumGate::::WIRE_REVERSED_SUM) } + + pub(crate) fn base_sum( + &mut self, + limbs: impl ExactSizeIterator> + Clone, + ) -> Target { + let num_limbs = limbs.len(); + debug_assert!( + BaseSumGate::<2>::START_LIMBS + num_limbs <= self.config.num_routed_wires, + "Not enough routed wires." + ); + let gate_index = self.add_gate(BaseSumGate::<2>::new(num_limbs), vec![]); + for (limb, wire) in limbs + .clone() + .zip(BaseSumGate::<2>::START_LIMBS..BaseSumGate::<2>::START_LIMBS + num_limbs) + { + self.route(*limb.borrow(), Target::wire(gate_index, wire)); + } + + self.add_generator(BaseSumGenerator::<2> { + gate_index, + limbs: limbs.map(|l| *l.borrow()).collect(), + }); + + Target::wire(gate_index, BaseSumGate::<2>::WIRE_SUM) + } +} + +#[derive(Debug)] +struct BaseSumGenerator { + gate_index: usize, + limbs: Vec, +} + +impl SimpleGenerator for BaseSumGenerator { + fn dependencies(&self) -> Vec { + self.limbs.clone() + } + + fn run_once(&self, witness: &PartialWitness) -> GeneratedValues { + let sum = self + .limbs + .iter() + .map(|&t| witness.get_target(t)) + .rev() + .fold(F::ZERO, |acc, limb| acc * F::from_canonical_usize(B) + limb); + + GeneratedValues::singleton_target( + Target::wire(self.gate_index, BaseSumGate::::WIRE_SUM), + sum, + ) + } } #[cfg(test)] mod tests { use anyhow::Result; + use rand::{thread_rng, Rng}; use super::*; use crate::circuit_data::CircuitConfig; @@ -73,4 +130,36 @@ mod tests { verify(proof, &data.verifier_only, &data.common) } + + #[test] + fn test_base_sum() -> Result<()> { + type F = CrandallField; + let config = CircuitConfig::large_config(); + let mut builder = CircuitBuilder::::new(config); + + let n = thread_rng().gen_range(0, 1 << 10); + let x = builder.constant(F::from_canonical_usize(n)); + + let zero = builder.zero(); + let one = builder.one(); + + let y = builder.base_sum( + (0..10) + .scan(n, |acc, _| { + let tmp = *acc % 2; + *acc /= 2; + Some(if tmp == 1 { one } else { zero }) + }) + .collect::>() + .iter(), + ); + + builder.assert_equal(x, y); + + let data = builder.build(); + + let proof = data.prove(PartialWitness::new())?; + + verify(proof, &data.verifier_only, &data.common) + } } From ca3a2fcfc86865a399d04d8caa331123a6b369ce Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 22 Jul 2021 16:09:54 +0200 Subject: [PATCH 05/10] Clippy --- src/gates/gmimc.rs | 6 ------ src/merkle_proofs.rs | 1 - 2 files changed, 7 deletions(-) diff --git a/src/gates/gmimc.rs b/src/gates/gmimc.rs index a3c26608..04c9d54a 100644 --- a/src/gates/gmimc.rs +++ b/src/gates/gmimc.rs @@ -309,12 +309,6 @@ mod tests { type Gate = GMiMCGate; let gate = Gate::with_constants(constants.clone()); - let config = CircuitConfig { - num_wires: 126, - num_routed_wires: 200, - ..Default::default() - }; - let permutation_inputs = (0..W).map(F::from_canonical_usize).collect::>(); let mut witness = PartialWitness::new(); diff --git a/src/merkle_proofs.rs b/src/merkle_proofs.rs index e971b3c5..233f4af5 100644 --- a/src/merkle_proofs.rs +++ b/src/merkle_proofs.rs @@ -68,7 +68,6 @@ impl, const D: usize> CircuitBuilder { proof: &MerkleProofTarget, ) { let zero = self.zero(); - let height = proof.siblings.len(); let mut state: HashTarget = self.hash_or_noop(leaf_data); From 1d92191227f8bd6913b6ecaf9a7cca623e3bf43b Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 22 Jul 2021 16:18:13 +0200 Subject: [PATCH 06/10] Make `exp_complement_bits` take an iterator to avoid cloning. --- src/fri/recursive_verifier.rs | 6 +----- src/gadgets/arithmetic.rs | 12 +++++++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/fri/recursive_verifier.rs b/src/fri/recursive_verifier.rs index 12246504..11cdd7d4 100644 --- a/src/fri/recursive_verifier.rs +++ b/src/fri/recursive_verifier.rs @@ -33,13 +33,9 @@ impl, const D: usize> CircuitBuilder { // The evaluation vector needs to be reordered first. let mut evals = last_evals.to_vec(); reverse_index_bits_in_place(&mut evals); - let mut old_x_index_bits = old_x_index_bits.to_vec(); - old_x_index_bits.reverse(); // Want `g^(arity - rev_old_x_index)` as in the out-of-circuit version. // Compute it as `g^(arity-1-rev_old_x_index) * g`, where the first term is gotten using two's complement. - // TODO: Once the exponentiation gate lands, we won't need the bits and will be able to compute - // `g^(arity-rev_old_x_index)` directly. - let start = self.exp_from_complement_bits(gt, &old_x_index_bits); + let start = self.exp_from_complement_bits(gt, old_x_index_bits.iter().rev()); let coset_start = self.mul_many(&[start, gt, x]); // The answer is gotten by interpolating {(x*g^i, P(x*g^i))} and evaluating at beta. diff --git a/src/gadgets/arithmetic.rs b/src/gadgets/arithmetic.rs index 6dcf1b3d..0598f3e7 100644 --- a/src/gadgets/arithmetic.rs +++ b/src/gadgets/arithmetic.rs @@ -1,3 +1,5 @@ +use std::borrow::Borrow; + use crate::circuit_builder::CircuitBuilder; use crate::field::extension_field::Extendable; use crate::target::Target; @@ -187,15 +189,19 @@ impl, const D: usize> CircuitBuilder { // TODO: Optimize this, maybe with a new gate. // TODO: Test /// Exponentiate `base` to the power of `2^bit_length-1-exponent`, given by its little-endian bits. - pub fn exp_from_complement_bits(&mut self, base: Target, exponent_bits: &[Target]) -> Target { + pub fn exp_from_complement_bits( + &mut self, + base: Target, + exponent_bits: impl ExactSizeIterator> + Clone, + ) -> Target { let mut current = base; let one_ext = self.one_extension(); let mut product = self.one(); - for &bit in exponent_bits { + for bit in exponent_bits { let current_ext = self.convert_to_ext(current); // TODO: Add base field select. - let multiplicand = self.select(bit, one_ext, current_ext); + let multiplicand = self.select(*bit.borrow(), one_ext, current_ext); product = self.mul(product, multiplicand.0[0]); current = self.mul(current, current); } From 05419569428a8865f168a998e0b196e17bf87365 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 22 Jul 2021 16:22:23 +0200 Subject: [PATCH 07/10] Remove useless clone --- src/gadgets/arithmetic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gadgets/arithmetic.rs b/src/gadgets/arithmetic.rs index 0598f3e7..f27a185f 100644 --- a/src/gadgets/arithmetic.rs +++ b/src/gadgets/arithmetic.rs @@ -192,7 +192,7 @@ impl, const D: usize> CircuitBuilder { pub fn exp_from_complement_bits( &mut self, base: Target, - exponent_bits: impl ExactSizeIterator> + Clone, + exponent_bits: impl ExactSizeIterator>, ) -> Target { let mut current = base; let one_ext = self.one_extension(); From e87aa2c90b3765f7649b3149a4bbc39dc701b64a Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Thu, 22 Jul 2021 16:25:47 +0200 Subject: [PATCH 08/10] Renaming --- src/fri/recursive_verifier.rs | 4 ++-- src/gadgets/split_base.rs | 20 +++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/fri/recursive_verifier.rs b/src/fri/recursive_verifier.rs index 11cdd7d4..fec8c065 100644 --- a/src/fri/recursive_verifier.rs +++ b/src/fri/recursive_verifier.rs @@ -268,7 +268,7 @@ impl, const D: usize> CircuitBuilder { let g = self.constant(F::MULTIPLICATIVE_GROUP_GENERATOR); let phi = self.constant(F::primitive_root_of_unity(n_log)); - let reversed_x = self.base_sum(x_index_bits.iter().rev()); + let reversed_x = self.le_sum(x_index_bits.iter().rev()); let phi = self.exp(phi, reversed_x, n_log); self.mul(g, phi) }); @@ -308,7 +308,7 @@ impl, const D: usize> CircuitBuilder { // Insert P(y) into the evaluation vector, since it wasn't included by the prover. let high_x_index_bits = x_index_bits.split_off(arity_bits); old_x_index_bits = x_index_bits; - let low_x_index = self.base_sum(old_x_index_bits.iter()); + let low_x_index = self.le_sum(old_x_index_bits.iter()); evals = self.insert(low_x_index, e_x, evals); context!( self, diff --git a/src/gadgets/split_base.rs b/src/gadgets/split_base.rs index da26c1db..46d7c5b1 100644 --- a/src/gadgets/split_base.rs +++ b/src/gadgets/split_base.rs @@ -39,26 +39,28 @@ impl, const D: usize> CircuitBuilder { Target::wire(gate, BaseSumGate::::WIRE_REVERSED_SUM) } - pub(crate) fn base_sum( + /// Takes an iterator of bits `(b_i)` and returns `sum b_i * 2^i`, i.e., + /// the number with little-endian bit representation given by `bits`. + pub(crate) fn le_sum( &mut self, - limbs: impl ExactSizeIterator> + Clone, + bits: impl ExactSizeIterator> + Clone, ) -> Target { - let num_limbs = limbs.len(); + let num_bits = bits.len(); debug_assert!( - BaseSumGate::<2>::START_LIMBS + num_limbs <= self.config.num_routed_wires, + BaseSumGate::<2>::START_LIMBS + num_bits <= self.config.num_routed_wires, "Not enough routed wires." ); - let gate_index = self.add_gate(BaseSumGate::<2>::new(num_limbs), vec![]); - for (limb, wire) in limbs + let gate_index = self.add_gate(BaseSumGate::<2>::new(num_bits), vec![]); + for (limb, wire) in bits .clone() - .zip(BaseSumGate::<2>::START_LIMBS..BaseSumGate::<2>::START_LIMBS + num_limbs) + .zip(BaseSumGate::<2>::START_LIMBS..BaseSumGate::<2>::START_LIMBS + num_bits) { self.route(*limb.borrow(), Target::wire(gate_index, wire)); } self.add_generator(BaseSumGenerator::<2> { gate_index, - limbs: limbs.map(|l| *l.borrow()).collect(), + limbs: bits.map(|l| *l.borrow()).collect(), }); Target::wire(gate_index, BaseSumGate::<2>::WIRE_SUM) @@ -143,7 +145,7 @@ mod tests { let zero = builder.zero(); let one = builder.one(); - let y = builder.base_sum( + let y = builder.le_sum( (0..10) .scan(n, |acc, _| { let tmp = *acc % 2; From 47b9936487597595b3e501355b6cdc0593afdb9a Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Fri, 23 Jul 2021 08:15:13 +0200 Subject: [PATCH 09/10] PR feedback --- src/gadgets/arithmetic.rs | 2 +- src/gadgets/range_check.rs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/gadgets/arithmetic.rs b/src/gadgets/arithmetic.rs index f27a185f..b858671d 100644 --- a/src/gadgets/arithmetic.rs +++ b/src/gadgets/arithmetic.rs @@ -192,7 +192,7 @@ impl, const D: usize> CircuitBuilder { pub fn exp_from_complement_bits( &mut self, base: Target, - exponent_bits: impl ExactSizeIterator>, + exponent_bits: impl Iterator>, ) -> Target { let mut current = base; let one_ext = self.one_extension(); diff --git a/src/gadgets/range_check.rs b/src/gadgets/range_check.rs index 261e5c7d..e3408cbf 100644 --- a/src/gadgets/range_check.rs +++ b/src/gadgets/range_check.rs @@ -14,11 +14,10 @@ impl, const D: usize> CircuitBuilder { self.route(x, sum); } - /// Returns the first `n_log` little-endian bits of `x`. - /// Note: `x` is assumed to be range-checked for having `num_bits` bits. - pub fn low_bits(&mut self, x: Target, n_log: usize, num_bits: usize) -> Vec { + /// Returns the first `num_low_bits` little-endian bits of `x`. + pub fn low_bits(&mut self, x: Target, num_low_bits: usize, num_bits: usize) -> Vec { let mut res = self.split_le(x, num_bits); - res.truncate(n_log); + res.truncate(num_low_bits); res } From a70e97befcb80d441a512ddfa9711d46065b69e8 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Fri, 23 Jul 2021 08:21:55 +0200 Subject: [PATCH 10/10] Fix merge issues --- src/gadgets/arithmetic.rs | 2 +- src/gates/gmimc.rs | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/gadgets/arithmetic.rs b/src/gadgets/arithmetic.rs index 1e5e6001..7afa9319 100644 --- a/src/gadgets/arithmetic.rs +++ b/src/gadgets/arithmetic.rs @@ -196,7 +196,7 @@ impl, const D: usize> CircuitBuilder { let one = self.one(); let mut product = one; - for &bit in exponent_bits { + for bit in exponent_bits { let multiplicand = self.select(*bit.borrow(), one, current); product = self.mul(product, multiplicand); current = self.mul(current, current); diff --git a/src/gates/gmimc.rs b/src/gates/gmimc.rs index b4fa6345..a7651a2a 100644 --- a/src/gates/gmimc.rs +++ b/src/gates/gmimc.rs @@ -119,11 +119,6 @@ impl, const D: usize, const R: usize> Gate for GMiMCGate< let swap = vars.local_wires[Self::WIRE_SWAP]; constraints.push(swap * (swap - F::ONE)); - let old_index_acc = vars.local_wires[Self::WIRE_INDEX_ACCUMULATOR_OLD]; - let new_index_acc = vars.local_wires[Self::WIRE_INDEX_ACCUMULATOR_NEW]; - let computed_new_index_acc = F::TWO * old_index_acc + swap; - constraints.push(computed_new_index_acc - new_index_acc); - let mut state = Vec::with_capacity(12); for i in 0..4 { let a = vars.local_wires[i]; @@ -240,7 +235,7 @@ impl, const D: usize, const R: usize> Gate for GMiMCGate< } fn num_constraints(&self) -> usize { - R + W + 2 + R + W + 1 } }