Skip to content

Commit

Permalink
chore: arithmetic lint (#118)
Browse files Browse the repository at this point in the history
The [Quarkslab
audit](https://github.com/tari-project/bulletproofs-plus/blob/da71f7872f02a0e9d3000c316bb083181daa9942/docs/quarkslab-audit/README.md)
of this library recommended the use of an arithmetic side-effect lint.
This was not done at the time due to difficulty getting the lint to work
with Ristretto types, which do not pose risk.

This PR adds the lint and makes necessary changes to account for it.

Closes #117.
  • Loading branch information
AaronFeickert authored Mar 5, 2024
1 parent 8759a59 commit ded62ca
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 5 deletions.
3 changes: 3 additions & 0 deletions benches/range_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ static EXTRACT_MASKS: [VerifyAction; 1] = [VerifyAction::VerifyOnly];
// static EXTENSION_DEGREE: [ExtensionDegree; 3] = [ExtensionDegree::Zero, ExtensionDegree::Two, ExtensionDegree::Four];
// static EXTRACT_MASKS: [VerifyAction; 2] = [VerifyAction::VerifyOnly, VerifyAction::RecoverOnly];

#[allow(clippy::arithmetic_side_effects)]
fn create_aggregated_rangeproof_helper(bit_length: usize, extension_degree: ExtensionDegree, c: &mut Criterion) {
let mut group = c.benchmark_group("range_proof_creation");
group.sampling_mode(SamplingMode::Flat);
Expand Down Expand Up @@ -111,6 +112,7 @@ fn create_aggregated_rangeproof_n_64(c: &mut Criterion) {
}
}

#[allow(clippy::arithmetic_side_effects)]
fn verify_aggregated_rangeproof_helper(bit_length: usize, extension_degree: ExtensionDegree, c: &mut Criterion) {
let mut group = c.benchmark_group("range_proof_verification");
group.sampling_mode(SamplingMode::Flat);
Expand Down Expand Up @@ -193,6 +195,7 @@ fn verify_aggregated_rangeproof_n_64(c: &mut Criterion) {
}
}

#[allow(clippy::arithmetic_side_effects)]
fn verify_batched_rangeproofs_helper(bit_length: usize, extension_degree: ExtensionDegree, c: &mut Criterion) {
let mut group = c.benchmark_group("batched_range_proof_verification");
group.sampling_mode(SamplingMode::Flat);
Expand Down
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
arithmetic-side-effects-allowed = [ "tari_curve25519_dalek::Scalar" ]
5 changes: 4 additions & 1 deletion lints.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,8 @@ deny = [
'clippy::cast_possible_truncation',
'clippy::cast_possible_wrap',
'clippy::cast_precision_loss',
'clippy::cast_sign_loss'
'clippy::cast_sign_loss',

# Mathematical mistakes
'clippy::arithmetic_side_effects',
]
23 changes: 19 additions & 4 deletions src/range_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ where
}
for j in 1..aggregation_factor {
for i in 0..bit_length {
#[allow(clippy::arithmetic_side_effects)]
d.push(d.get((j - 1) * bit_length + i).ok_or(ProofError::SizeOverflow)? * z_square);
}
}
Expand All @@ -373,7 +374,11 @@ where
for opening in &witness.openings {
z_even_powers *= z_square;
for (r, alpha1_val) in opening.r.iter().zip(alpha.iter_mut()) {
*alpha1_val += z_even_powers * r * y_powers.get(full_length + 1).ok_or(ProofError::SizeOverflow)?;
*alpha1_val += z_even_powers *
r *
y_powers
.get(full_length.checked_add(1).ok_or(ProofError::SizeOverflow)?)
.ok_or(ProofError::SizeOverflow)?;
}
}

Expand Down Expand Up @@ -441,15 +446,19 @@ where
)
};

round += 1;
round = round.checked_add(1).ok_or(ProofError::SizeOverflow)?;

let c_l = Zeroizing::new(
izip!(a_lo, y_powers.iter().skip(1), b_hi)
.fold(Scalar::ZERO, |acc, (a, y_power, b)| acc + a * y_power * b),
);
let c_r = Zeroizing::new(
izip!(a_hi, y_powers.iter().skip(n + 1), b_lo)
.fold(Scalar::ZERO, |acc, (a, y_power, b)| acc + a * y_power * b),
izip!(
a_hi,
y_powers.iter().skip(n.checked_add(1).ok_or(ProofError::SizeOverflow)?),
b_lo
)
.fold(Scalar::ZERO, |acc, (a, y_power, b)| acc + a * y_power * b),
);

// Compute L and R by multi-scalar multiplication
Expand Down Expand Up @@ -544,12 +553,15 @@ where
)
};

#[allow(clippy::arithmetic_side_effects)]
let mut a1 =
&gi_base[0] * *r + &hi_base[0] * *s + h_base * (*r * y_powers[1] * a_ri[0] + *s * y_powers[1] * a_li[0]);
let mut b = h_base * (*r * y_powers[1] * *s);
#[allow(clippy::arithmetic_side_effects)]
for (g_base, &d) in g_base.iter().zip(d.iter()) {
a1 += g_base * d;
}
#[allow(clippy::arithmetic_side_effects)]
for (g_base, &eta) in g_base.iter().zip(eta.iter()) {
b += g_base * eta;
}
Expand Down Expand Up @@ -889,6 +901,7 @@ where
for _ in 1..bit_length {
d.push(two * d.last().ok_or(ProofError::SizeOverflow)?);
}
#[allow(clippy::arithmetic_side_effects)]
for j in 1..aggregation_factor {
for i in 0..bit_length {
d.push(d.get((j - 1) * bit_length + i).ok_or(ProofError::SizeOverflow)? * z_square);
Expand Down Expand Up @@ -945,6 +958,7 @@ where
let log_i = usize::try_from(i.checked_ilog2().ok_or(ProofError::SizeOverflow)?)
.map_err(|_| ProofError::SizeOverflow)?;
let j = 1 << log_i;
#[allow(clippy::arithmetic_side_effects)]
s.push(
s.get(i - j).ok_or(ProofError::SizeOverflow)? *
challenges_sq.get(rounds - log_i - 1).ok_or(ProofError::SizeOverflow)?,
Expand Down Expand Up @@ -1077,6 +1091,7 @@ where
/// Then we serialize the rest of the proof elements as canonical byte encodings
pub fn to_bytes(&self) -> Vec<u8> {
// The total proof size: extension degree encoding, fixed elements, vectors
#[allow(clippy::arithmetic_side_effects)]
let mut buf = Vec::with_capacity(
ENCODED_EXTENSION_SIZE +
(self.li.len() + self.ri.len() + FIXED_PROOF_ELEMENTS + self.d1.len()) * SERIALIZED_ELEMENT_SIZE,
Expand Down
1 change: 1 addition & 0 deletions src/ristretto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ fn get_g_base(extension_degree: ExtensionDegree) -> (Vec<RistrettoPoint>, Vec<Co
}

/// A static array of pre-generated points
#[allow(clippy::arithmetic_side_effects)]
fn ristretto_masking_basepoints() -> &'static [RistrettoPoint; ExtensionDegree::COUNT] {
static INSTANCE: OnceCell<[RistrettoPoint; ExtensionDegree::COUNT]> = OnceCell::new();
INSTANCE.get_or_init(|| {
Expand Down
1 change: 1 addition & 0 deletions src/transcripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ where

// Serialize the witness if provided
let bytes = if let Some(witness) = witness {
#[allow(clippy::arithmetic_side_effects)]
let size: usize = witness
.openings
.iter()
Expand Down
1 change: 1 addition & 0 deletions tests/ristretto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ enum ProofOfMinimumValueStrategy {
LargerThanValue,
}

#[allow(clippy::arithmetic_side_effects)]
fn prove_and_verify(
bit_lengths: &[usize],
proof_batch: &[usize],
Expand Down

0 comments on commit ded62ca

Please sign in to comment.