Skip to content

Commit

Permalink
Allow TxOut and TxIn values to take the value u64::MAX.
Browse files Browse the repository at this point in the history
  • Loading branch information
murisi committed Jul 28, 2023
1 parent 438369a commit 25a729e
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 93 deletions.
33 changes: 8 additions & 25 deletions masp_primitives/src/transaction/builder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Structs for building transactions.

use std::convert::TryInto;
use std::error;
use std::fmt;
use std::sync::mpsc::Sender;
Expand Down Expand Up @@ -251,7 +250,7 @@ impl<P: consensus::Parameters, R: RngCore> Builder<P, R> {
value: u64,
memo: MemoBytes,
) -> Result<(), sapling::builder::Error> {
if value > MAX_MONEY.try_into().unwrap() {
if value > MAX_MONEY {
return Err(sapling::builder::Error::InvalidAmount);
}
self.sapling_builder
Expand All @@ -273,9 +272,9 @@ impl<P: consensus::Parameters, R: RngCore> Builder<P, R> {
&mut self,
to: &TransparentAddress,
asset_type: AssetType,
value: i64,
value: u64,
) -> Result<(), transparent::builder::Error> {
if value < 0 || value > MAX_MONEY {
if value > MAX_MONEY {
return Err(transparent::builder::Error::InvalidAmount);
}

Expand Down Expand Up @@ -480,17 +479,16 @@ mod tests {
merkle_tree::{CommitmentTree, IncrementalWitness},
sapling::Rseed,
transaction::{
components::amount::{FromNt, I128Sum, ValueSum, DEFAULT_FEE, MAX_MONEY},
sapling::builder::{self as build_s},
transparent::builder::{self as build_t},
components::amount::{FromNt, I128Sum, ValueSum, DEFAULT_FEE},
sapling::builder as build_s,
TransparentAddress,
},
zip32::ExtendedSpendingKey,
};

use super::{Builder, Error};

#[test]
/*#[test]
fn fails_on_overflow_output() {
let extsk = ExtendedSpendingKey::master(&[]);
let dfvk = extsk.to_diversifiable_full_viewing_key();
Expand All @@ -507,12 +505,12 @@ mod tests {
Some(ovk),
to,
zec(),
MAX_MONEY as u64 + 1,
MAX_MONEY + 1,
MemoBytes::empty()
),
Err(build_s::Error::InvalidAmount)
);
}
}*/

/// Generate ZEC asset type
fn zec() -> AssetType {
Expand Down Expand Up @@ -565,21 +563,6 @@ mod tests {
);
}

#[test]
fn fails_on_negative_transparent_output() {
let mut rng = OsRng;

let transparent_address = TransparentAddress(rng.gen::<[u8; 20]>());
let tx_height = TEST_NETWORK
.activation_height(NetworkUpgrade::MASP)
.unwrap();
let mut builder = Builder::new(TEST_NETWORK, tx_height);
assert_eq!(
builder.add_transparent_output(&transparent_address, zec(), -1,),
Err(build_t::Error::InvalidAmount)
);
}

#[test]
fn fails_on_negative_change() {
let mut rng = OsRng;
Expand Down
4 changes: 3 additions & 1 deletion masp_primitives/src/transaction/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ pub mod amount;
pub mod sapling;
pub mod transparent;
pub use self::{
amount::{I128Sum, I64Sum, ValueSum},
amount::{
I128Sum, I16Sum, I32Sum, I64Sum, I8Sum, U128Sum, U16Sum, U32Sum, U64Sum, U8Sum, ValueSum,
},
sapling::{ConvertDescription, OutputDescription, SpendDescription},
transparent::{TxIn, TxOut},
};
Expand Down
31 changes: 13 additions & 18 deletions masp_primitives/src/transaction/components/amount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ use std::iter::Sum;
use std::ops::{Add, AddAssign, Index, Mul, MulAssign, Neg, Sub, SubAssign};
use zcash_encoding::Vector;

pub const MAX_MONEY: i64 = i64::MAX;
pub const MIN_MONEY: i64 = i64::MIN;
pub const MAX_MONEY: u64 = u64::MAX;
lazy_static::lazy_static! {
pub static ref DEFAULT_FEE: I64Sum = ValueSum::from_pair(zec(), 1000).unwrap();
}
Expand Down Expand Up @@ -76,9 +75,7 @@ where
Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone,
Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd,
{
/// Creates a non-negative Amount from an i64.
///
/// Returns an error if the amount is outside the range `{0..MAX_MONEY}`.
/// Creates a non-negative Amount from a Magnitude.
pub fn from_nonnegative(atype: Unit, amount: Magnitude) -> Result<Self, ()> {
if amount == Magnitude::default() {
Ok(Self::zero())
Expand All @@ -97,9 +94,7 @@ where
Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone,
Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default,
{
/// Creates an Amount from a type convertible to i64.
///
/// Returns an error if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`.
/// Creates an Amount from a Magnitude.
pub fn from_pair(atype: Unit, amount: Magnitude) -> Result<Self, ()> {
if amount == Magnitude::default() {
Ok(Self::zero())
Expand Down Expand Up @@ -653,37 +648,37 @@ pub fn default_fee() -> ValueSum<AssetType, i64> {
pub mod testing {
use proptest::prelude::prop_compose;

use super::{I128Sum, I64Sum, ValueSum, MAX_MONEY};
use super::{I128Sum, I64Sum, U64Sum, ValueSum, MAX_MONEY};
use crate::asset_type::testing::arb_asset_type;

prop_compose! {
pub fn arb_i64_sum()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> I64Sum {
pub fn arb_i64_sum()(asset_type in arb_asset_type(), amt in i64::MIN..i64::MAX) -> I64Sum {
ValueSum::from_pair(asset_type, amt).unwrap()
}
}

prop_compose! {
pub fn arb_i128_sum()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> I128Sum {
pub fn arb_i128_sum()(asset_type in arb_asset_type(), amt in i128::MIN..i128::MAX) -> I128Sum {
ValueSum::from_pair(asset_type, amt as i128).unwrap()
}
}

prop_compose! {
pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0i64..MAX_MONEY) -> I64Sum {
pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0u64..MAX_MONEY) -> U64Sum {
ValueSum::from_pair(asset_type, amt).unwrap()
}
}

prop_compose! {
pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1i64..MAX_MONEY) -> I64Sum {
pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1u64..MAX_MONEY) -> U64Sum {
ValueSum::from_pair(asset_type, amt).unwrap()
}
}
}

#[cfg(test)]
mod tests {
use super::{zec, I64Sum, ValueSum, MAX_MONEY, MIN_MONEY};
use super::{zec, I64Sum, ValueSum, MAX_MONEY};

#[test]
fn amount_in_range() {
Expand All @@ -700,7 +695,7 @@ mod tests {

assert_eq!(
I64Sum::read(&mut max_money.as_ref()).unwrap(),
I64Sum::from_pair(zec(), MAX_MONEY).unwrap()
I64Sum::from_pair(zec(), i64::MAX).unwrap()
);

//let max_money_p1 = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x01\x40\x07\x5a\xf0\x75\x07\x00";
Expand All @@ -715,7 +710,7 @@ mod tests {
let neg_max_money = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x01\x00\x00\x00\x00\x00\x00\x80";
assert_eq!(
I64Sum::read(&mut neg_max_money.as_ref()).unwrap(),
I64Sum::from_pair(zec(), -MAX_MONEY).unwrap()
I64Sum::from_pair(zec(), -i64::MAX).unwrap()
);
}

Expand All @@ -736,14 +731,14 @@ mod tests {
#[test]
#[should_panic]
fn sub_panics_on_underflow() {
let v = ValueSum::from_pair(zec(), MIN_MONEY).unwrap();
let v = ValueSum::from_pair(zec(), 0u64).unwrap();
let _diff = v - ValueSum::from_pair(zec(), 1).unwrap();
}

#[test]
#[should_panic]
fn sub_assign_panics_on_underflow() {
let mut a = ValueSum::from_pair(zec(), MIN_MONEY).unwrap();
let mut a = ValueSum::from_pair(zec(), 0u64).unwrap();
a -= ValueSum::from_pair(zec(), 1).unwrap();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl SaplingOutputInfo {
memo: MemoBytes,
) -> Result<Self, Error> {
let g_d = to.g_d().ok_or(Error::InvalidAddress)?;
if value > MAX_MONEY.try_into().unwrap() {
if value > MAX_MONEY {
return Err(Error::InvalidAmount);
}

Expand Down
28 changes: 8 additions & 20 deletions masp_primitives/src/transaction/components/transparent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,26 +65,14 @@ impl<A: Authorization> Bundle<A> {
let input_sum = self
.vin
.iter()
.map(|p| {
if p.value >= 0 {
ValueSum::from_pair(p.asset_type, p.value as i128)
} else {
Err(())
}
})
.map(|p| ValueSum::from_pair(p.asset_type, p.value as i128))
.sum::<Result<I128Sum, ()>>()
.map_err(|_| BalanceError::Overflow)?;

let output_sum = self
.vout
.iter()
.map(|p| {
if p.value >= 0 {
ValueSum::from_pair(p.asset_type, p.value as i128)
} else {
Err(())
}
})
.map(|p| ValueSum::from_pair(p.asset_type, p.value as i128))
.sum::<Result<I128Sum, ()>>()
.map_err(|_| BalanceError::Overflow)?;

Expand All @@ -96,7 +84,7 @@ impl<A: Authorization> Bundle<A> {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TxIn<A: Authorization> {
pub asset_type: AssetType,
pub value: i64,
pub value: u64,
pub address: TransparentAddress,
pub transparent_sig: A::TransparentSig,
}
Expand All @@ -112,9 +100,9 @@ impl TxIn<Authorized> {
let value = {
let mut tmp = [0u8; 8];
reader.read_exact(&mut tmp)?;
i64::from_le_bytes(tmp)
u64::from_le_bytes(tmp)
};
if value < 0 || value > MAX_MONEY {
if value > MAX_MONEY {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"value out of range",
Expand Down Expand Up @@ -144,7 +132,7 @@ impl TxIn<Authorized> {
#[derive(Clone, Debug, Hash, PartialOrd, PartialEq, Ord, Eq)]
pub struct TxOut {
pub asset_type: AssetType,
pub value: i64,
pub value: u64,
pub address: TransparentAddress,
}

Expand All @@ -159,9 +147,9 @@ impl TxOut {
let value = {
let mut tmp = [0u8; 8];
reader.read_exact(&mut tmp)?;
i64::from_le_bytes(tmp)
u64::from_le_bytes(tmp)
};
if value < 0 || value > MAX_MONEY {
if value > MAX_MONEY {
return Err(io::Error::new(
io::ErrorKind::InvalidData,
"value out of range",
Expand Down
26 changes: 5 additions & 21 deletions masp_primitives/src/transaction/components/transparent/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,6 @@ impl TransparentBuilder {
/// Adds a coin (the output of a previous transaction) to be spent to the transaction.
#[cfg(feature = "transparent-inputs")]
pub fn add_input(&mut self, coin: TxOut) -> Result<(), Error> {
if coin.value.is_negative() {
return Err(Error::InvalidAmount);
}

self.inputs.push(TransparentInputInfo { coin });

Ok(())
Expand All @@ -118,9 +114,9 @@ impl TransparentBuilder {
&mut self,
to: &TransparentAddress,
asset_type: AssetType,
value: i64,
value: u64,
) -> Result<(), Error> {
if value < 0 || value > MAX_MONEY {
if value > MAX_MONEY {
return Err(Error::InvalidAmount);
}

Expand All @@ -138,13 +134,7 @@ impl TransparentBuilder {
let input_sum = self
.inputs
.iter()
.map(|input| {
if input.coin.value >= 0 {
ValueSum::from_pair(input.coin.asset_type, input.coin.value as i128)
} else {
Err(())
}
})
.map(|input| ValueSum::from_pair(input.coin.asset_type, input.coin.value as i128))
.sum::<Result<I128Sum, ()>>()
.map_err(|_| BalanceError::Overflow)?;

Expand All @@ -154,13 +144,7 @@ impl TransparentBuilder {
let output_sum = self
.vout
.iter()
.map(|vo| {
if vo.value >= 0 {
ValueSum::from_pair(vo.asset_type, vo.value as i128)
} else {
Err(())
}
})
.map(|vo| ValueSum::from_pair(vo.asset_type, vo.value as i128))
.sum::<Result<I128Sum, ()>>()
.map_err(|_| BalanceError::Overflow)?;

Expand Down Expand Up @@ -208,7 +192,7 @@ impl TransparentAuthorizingContext for Unauthorized {

#[cfg(feature = "transparent-inputs")]
impl TransparentAuthorizingContext for Unauthorized {
fn input_amounts(&self) -> Vec<(AssetType, i64)> {
fn input_amounts(&self) -> Vec<(AssetType, u64)> {
return self
.inputs
.iter()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ pub trait InputView {
/// fee and change computation.
pub trait OutputView {
/// Returns the value of the output being created.
fn value(&self) -> i64;
fn value(&self) -> u64;
/// Returns the asset type of the output being created.
fn asset_type(&self) -> AssetType;
/// Returns the script corresponding to the newly created output.
fn transparent_address(&self) -> &TransparentAddress;
}

impl OutputView for TxOut {
fn value(&self) -> i64 {
fn value(&self) -> u64 {
self.value
}

Expand Down
2 changes: 1 addition & 1 deletion masp_primitives/src/transaction/sighash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub trait TransparentAuthorizingContext: transparent::Authorization {
/// so that wallets can commit to the transparent input breakdown
/// without requiring the full data of the previous transactions
/// providing these inputs.
fn input_amounts(&self) -> Vec<(AssetType, i64)>;
fn input_amounts(&self) -> Vec<(AssetType, u64)>;
}

/// Computes the signature hash for an input to a transaction, given
Expand Down
8 changes: 4 additions & 4 deletions masp_proofs/benches/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ fn criterion_benchmark(c: &mut Criterion) {
.unwrap();

c.bench_function("convert", |b| {
let i = rng.next_u32();
let i = rng.next_u32() >> 1;
let spend_asset = AssetType::new(format!("asset {}", i).as_bytes()).unwrap();
let output_asset = AssetType::new(format!("asset {}", i + 1).as_bytes()).unwrap();
let mint_asset = AssetType::new(b"reward").unwrap();

let spend_value = -(i as i64 + 1);
let output_value = i as i64 + 1;
let mint_value = i as i64 + 1;
let spend_value = -(i as i32 + 1);
let output_value = i as i32 + 1;
let mint_value = i as i32 + 1;

let allowed_conversion: AllowedConversion = (ValueSum::from_pair(spend_asset, spend_value)
.unwrap()
Expand Down

0 comments on commit 25a729e

Please sign in to comment.