Remove potentially-failing unchecked arithmetic operations for ones which error

In response to 9.13.3.

Requires a bump to Rust 1.82 to take advantage of `Option::is_none_or`.
This commit is contained in:
Luke Parker
2025-08-08 22:30:34 -04:00
parent cbab9486c6
commit 188fcc3cb4
13 changed files with 65 additions and 18 deletions

View File

@@ -166,7 +166,14 @@ impl Decoys {
/// `offsets` are the positions of each ring member within the Monero blockchain, offset from the
/// prior member's position (with the initial ring member offset from 0).
pub fn new(offsets: Vec<u64>, signer_index: u8, ring: Vec<[EdwardsPoint; 2]>) -> Option<Self> {
if (offsets.len() != ring.len()) || (usize::from(signer_index) >= ring.len()) {
if (offsets.len() > usize::from(u8::MAX)) ||
(offsets.len() != ring.len()) ||
(usize::from(signer_index) >= ring.len())
{
None?;
}
// Check these offsets form representable positions
if offsets.iter().copied().try_fold(0, u64::checked_add).is_none() {
None?;
}
Some(Decoys { offsets, signer_index, ring })

View File

@@ -23,6 +23,11 @@ pub(crate) struct InternalBatchVerifier {
impl InternalBatchVerifier {
#[must_use]
fn verify(self, G: EdwardsPoint, H: EdwardsPoint, generators: &Generators) -> bool {
/*
Technically, this following line can overflow, and joining these `Vec`s _may_ panic if
they're individually acceptable lengths yet their sum isn't. This is so negligible, due to
the amount of memory required, it's dismissed.
*/
let capacity = 2 + self.g_bold.len() + self.h_bold.len() + self.other.len();
let mut scalars = Vec::with_capacity(capacity);
let mut points = Vec::with_capacity(capacity);

View File

@@ -86,13 +86,16 @@ impl Bulletproof {
/// Bulletproofs(+) are logarithmically sized yet linearly timed. Evaluating by their size alone
/// accordingly doesn't properly represent the burden of the proof. Monero 'claws back' some of
/// the weight lost by using a proof smaller than it is fast to compensate for this.
///
/// If the amount of outputs specified exceeds the maximum amount of outputs, the result for the
/// maximum amount of outputs will be returned.
// https://github.com/monero-project/monero/blob/94e67bf96bbc010241f29ada6abc89f49a81759c/
// src/cryptonote_basic/cryptonote_format_utils.cpp#L106-L124
pub fn calculate_bp_clawback(plus: bool, n_outputs: usize) -> (usize, usize) {
#[allow(non_snake_case)]
let mut LR_len = 0;
let mut n_padded_outputs = 1;
while n_padded_outputs < n_outputs {
while n_padded_outputs < n_outputs.min(MAX_COMMITMENTS) {
LR_len += 1;
n_padded_outputs = 1 << LR_len;
}

View File

@@ -1146,7 +1146,13 @@ impl<R: Rpc> DecoyRpc for R {
)))?;
}
let expected_len = if zero_zero_case { 2 } else { (to - start_height) + 1 };
let expected_len = if zero_zero_case {
2
} else {
(to - start_height).checked_add(1).ok_or_else(|| {
RpcError::InternalError("expected length of distribution exceeded usize".to_string())
})?
};
// Yet this is actually a height
if expected_len != distribution.len() {
Err(RpcError::InvalidNode(format!(
@@ -1161,6 +1167,20 @@ impl<R: Rpc> DecoyRpc for R {
if zero_zero_case {
distribution.pop();
}
// Check the distribution monotonically increases
{
let mut monotonic = 0;
for d in &distribution {
if *d < monotonic {
Err(RpcError::InvalidNode(
"received output distribution didn't increase monotonically".to_string(),
))?;
}
monotonic = *d;
}
}
Ok(distribution)
}
}
@@ -1271,8 +1291,8 @@ impl<R: Rpc> DecoyRpc for R {
// https://github.com/monero-project/monero/blob
// /cc73fe71162d564ffda8e549b79a350bca53c454/src/cryptonote_core
// /blockchain.cpp#L3836
((out.height + DEFAULT_LOCK_WINDOW) <= height) &&
(Timelock::Block(height - 1 + ACCEPTED_TIMELOCK_DELTA) >=
out.height.checked_add(DEFAULT_LOCK_WINDOW).is_some_and(|locked| locked <= height) &&
(Timelock::Block(height.wrapping_add(ACCEPTED_TIMELOCK_DELTA - 1)) >=
txs[i].prefix().additional_timelock)
} else {
out.unlocked

View File

@@ -343,7 +343,13 @@ impl RctPrunable {
Ok(match rct_type {
RctType::AggregateMlsagBorromean => RctPrunable::AggregateMlsagBorromean {
borromean: read_raw_vec(BorromeanRange::read, outputs, r)?,
mlsag: Mlsag::read(ring_length, inputs + 1, r)?,
mlsag: Mlsag::read(
ring_length,
inputs.checked_add(1).ok_or_else(|| {
io::Error::other("reading a MLSAG for more inputs than representable")
})?,
r,
)?,
},
RctType::MlsagBorromean => RctPrunable::MlsagBorromean {
borromean: read_raw_vec(BorromeanRange::read, outputs, r)?,

View File

@@ -6,7 +6,7 @@ license = "MIT"
repository = "https://github.com/serai-dex/serai/tree/develop/networks/monero/wallet"
authors = ["Luke Parker <lukeparker5132@gmail.com>"]
edition = "2021"
rust-version = "1.80"
rust-version = "1.82"
[package.metadata.docs.rs]
all-features = true

View File

@@ -94,11 +94,10 @@ pub(crate) fn encode_check(mut data: Vec<u8>) -> String {
// Decode an arbitrary-length stream of data, with a checksum
pub(crate) fn decode_check(data: &str) -> Option<Vec<u8>> {
if data.len() < CHECKSUM_LEN {
let mut res = decode(data)?;
if res.len() < CHECKSUM_LEN {
None?;
}
let mut res = decode(data)?;
let checksum_pos = res.len() - CHECKSUM_LEN;
if keccak256(&res[.. checksum_pos])[.. CHECKSUM_LEN] != res[checksum_pos ..] {
None?;

View File

@@ -18,7 +18,7 @@ use crate::{
};
const RECENT_WINDOW: u64 = 15;
const BLOCKS_PER_YEAR: usize = 365 * 24 * 60 * 60 / BLOCK_TIME;
const BLOCKS_PER_YEAR: usize = (365 * 24 * 60 * 60) / BLOCK_TIME;
#[allow(clippy::cast_precision_loss)]
const TIP_APPLICATION: f64 = (DEFAULT_LOCK_WINDOW * BLOCK_TIME) as f64;

View File

@@ -232,7 +232,13 @@ impl InternalScanner {
res.push(WalletOutput {
absolute_id: AbsoluteId { transaction: tx_hash, index_in_transaction: o },
relative_id: RelativeId { index_on_blockchain: output_index_for_first_ringct_output + o },
relative_id: RelativeId {
index_on_blockchain: output_index_for_first_ringct_output.checked_add(o).ok_or(
ScanError::InvalidScannableBlock(
"transaction's output's index isn't representable as a u64",
),
)?,
},
data: OutputData { key: output_key, key_offset, commitment },
metadata: Metadata {
additional_timelock: tx.prefix().additional_timelock,

View File

@@ -305,12 +305,13 @@ impl SignableTransaction {
.payments
.iter()
.filter_map(|payment| match payment {
InternalPayment::Payment(_, amount) => Some(amount),
InternalPayment::Payment(_, amount) => Some(*amount),
InternalPayment::Change(_) => None,
})
.sum::<u64>();
.try_fold(0, u64::checked_add);
let payments_amount = payments_amount.ok_or(SendError::TooManyOutputs)?;
let (weight, necessary_fee) = self.weight_and_necessary_fee();
if in_amount < (payments_amount + necessary_fee) {
if payments_amount.checked_add(necessary_fee).is_none_or(|total_out| in_amount < total_out) {
Err(SendError::NotEnoughFunds {
inputs: in_amount,
outputs: payments_amount,