Remove unsafe creation of dalek_ff_group::EdwardsPoint in BP+

This commit is contained in:
Luke Parker
2024-04-21 23:45:07 -04:00
parent 703c6a2358
commit 98b08eaa38
4 changed files with 21 additions and 23 deletions

View File

@@ -87,9 +87,8 @@ impl Bulletproofs {
Ok(if !plus {
Bulletproofs::Original(OriginalStruct::prove(rng, outputs))
} else {
use dalek_ff_group::EdwardsPoint as DfgPoint;
Bulletproofs::Plus(
AggregateRangeStatement::new(outputs.iter().map(|com| DfgPoint(com.calculate())).collect())
AggregateRangeStatement::new(outputs.iter().map(Commitment::calculate).collect())
.unwrap()
.prove(rng, &Zeroizing::new(AggregateRangeWitness::new(outputs.to_vec()).unwrap()))
.unwrap(),
@@ -104,13 +103,7 @@ impl Bulletproofs {
Bulletproofs::Original(bp) => bp.verify(rng, commitments),
Bulletproofs::Plus(bp) => {
let mut verifier = BatchVerifier::new(1);
// If this commitment is torsioned (which is allowed), this won't be a well-formed
// dfg::EdwardsPoint (expected to be of prime-order)
// The actual BP+ impl will perform a torsion clear though, making this safe
// TODO: Have AggregateRangeStatement take in dalek EdwardsPoint for clarity on this
let Some(statement) = AggregateRangeStatement::new(
commitments.iter().map(|c| dalek_ff_group::EdwardsPoint(*c)).collect(),
) else {
let Some(statement) = AggregateRangeStatement::new(commitments.to_vec()) else {
return false;
};
if !statement.verify(rng, &mut verifier, (), bp.clone()) {
@@ -135,9 +128,7 @@ impl Bulletproofs {
match self {
Bulletproofs::Original(bp) => bp.batch_verify(rng, verifier, id, commitments),
Bulletproofs::Plus(bp) => {
let Some(statement) = AggregateRangeStatement::new(
commitments.iter().map(|c| dalek_ff_group::EdwardsPoint(*c)).collect(),
) else {
let Some(statement) = AggregateRangeStatement::new(commitments.to_vec()) else {
return false;
};
statement.verify(rng, verifier, id, bp.clone())

View File

@@ -9,6 +9,7 @@ use group::{
ff::{Field, PrimeField},
Group, GroupEncoding,
};
use curve25519_dalek::EdwardsPoint as DalekPoint;
use dalek_ff_group::{Scalar, EdwardsPoint};
use crate::{
@@ -28,7 +29,7 @@ use crate::{
#[derive(Clone, Debug)]
pub(crate) struct AggregateRangeStatement {
generators: Generators,
V: Vec<EdwardsPoint>,
V: Vec<DalekPoint>,
}
impl Zeroize for AggregateRangeStatement {
@@ -57,7 +58,7 @@ pub struct AggregateRangeProof {
}
impl AggregateRangeStatement {
pub(crate) fn new(V: Vec<EdwardsPoint>) -> Option<Self> {
pub(crate) fn new(V: Vec<DalekPoint>) -> Option<Self> {
if V.is_empty() || (V.len() > MAX_M) {
return None;
}
@@ -98,11 +99,14 @@ impl AggregateRangeStatement {
}
let mn = V.len() * N;
// 2, 4, 6, 8... powers of z, of length equivalent to the amount of commitments
let mut z_pow = Vec::with_capacity(V.len());
// z**2
z_pow.push(z * z);
let mut d = ScalarVector::new(mn);
for j in 1 ..= V.len() {
z_pow.push(z.pow(Scalar::from(2 * u64::try_from(j).unwrap()))); // TODO: Optimize this
z_pow.push(*z_pow.last().unwrap() * z_pow[0]);
d = d + &(Self::d_j(j, V.len()) * (z_pow[j - 1]));
}
@@ -157,7 +161,7 @@ impl AggregateRangeStatement {
return None;
}
for (commitment, witness) in self.V.iter().zip(witness.0.iter()) {
if witness.calculate() != **commitment {
if witness.calculate() != *commitment {
return None;
}
}
@@ -170,9 +174,9 @@ impl AggregateRangeStatement {
// Commitments aren't transmitted INV_EIGHT though, so this multiplies by INV_EIGHT to enable
// clearing its cofactor without mutating the value
// For some reason, these values are transcripted * INV_EIGHT, not as transmitted
let mut V = V.into_iter().map(|V| EdwardsPoint(V.0 * crate::INV_EIGHT())).collect::<Vec<_>>();
let V = V.into_iter().map(|V| V * crate::INV_EIGHT()).collect::<Vec<_>>();
let mut transcript = initial_transcript(V.iter());
V.iter_mut().for_each(|V| *V = V.mul_by_cofactor());
let mut V = V.into_iter().map(|V| EdwardsPoint(V.mul_by_cofactor())).collect::<Vec<_>>();
// Pad V
while V.len() < padded_pow_of_2(V.len()) {
@@ -239,9 +243,11 @@ impl AggregateRangeStatement {
) -> bool {
let Self { generators, V } = self;
let mut V = V.into_iter().map(|V| EdwardsPoint(V.0 * crate::INV_EIGHT())).collect::<Vec<_>>();
let V = V.into_iter().map(|V| V * crate::INV_EIGHT()).collect::<Vec<_>>();
let mut transcript = initial_transcript(V.iter());
V.iter_mut().for_each(|V| *V = V.mul_by_cofactor());
// With the torsion clear, wrap it into a EdwardsPoint from dalek-ff-group
// (which is prime-order)
let V = V.into_iter().map(|V| EdwardsPoint(V.mul_by_cofactor())).collect::<Vec<_>>();
let generators = generators.reduce(V.len() * N);

View File

@@ -1,6 +1,7 @@
use std_shims::{sync::OnceLock, vec::Vec};
use dalek_ff_group::{Scalar, EdwardsPoint};
use curve25519_dalek::EdwardsPoint;
use dalek_ff_group::Scalar;
use monero_generators::{hash_to_point as raw_hash_to_point};
use crate::{hash, hash_to_scalar as dalek_hash};

View File

@@ -2,7 +2,7 @@ use rand_core::{RngCore, OsRng};
use multiexp::BatchVerifier;
use group::ff::Field;
use dalek_ff_group::{Scalar, EdwardsPoint};
use dalek_ff_group::Scalar;
use crate::{
Commitment,
@@ -19,7 +19,7 @@ fn test_aggregate_range_proof() {
for _ in 0 .. m {
commitments.push(Commitment::new(*Scalar::random(&mut OsRng), OsRng.next_u64()));
}
let commitment_points = commitments.iter().map(|com| EdwardsPoint(com.calculate())).collect();
let commitment_points = commitments.iter().map(Commitment::calculate).collect();
let statement = AggregateRangeStatement::new(commitment_points).unwrap();
let witness = AggregateRangeWitness::new(commitments).unwrap();