diff --git a/masp_primitives/src/transaction/builder.rs b/masp_primitives/src/transaction/builder.rs index c8353730..1745ef39 100644 --- a/masp_primitives/src/transaction/builder.rs +++ b/masp_primitives/src/transaction/builder.rs @@ -1,6 +1,5 @@ //! Structs for building transactions. -use std::convert::TryInto; use std::error; use std::fmt; use std::sync::mpsc::Sender; @@ -19,7 +18,7 @@ use crate::{ sapling::{prover::TxProver, Diversifier, Node, Note, PaymentAddress}, transaction::{ components::{ - amount::{BalanceError, FromNt, I128Sum, I64Sum, ValueSum, MAX_MONEY}, + amount::{BalanceError, FromNt, I128Sum, U64Sum, ValueSum, MAX_MONEY}, sapling::{ self, builder::{SaplingBuilder, SaplingMetadata}, @@ -46,7 +45,7 @@ pub enum Error { InsufficientFunds(I128Sum), /// The transaction has inputs in excess of outputs and fees; the user must /// add a change output. - ChangeRequired(I64Sum), + ChangeRequired(U64Sum), /// An error occurred in computing the fees for a transaction. Fee(FeeError), /// An overflow or underflow occurred when computing value balances @@ -251,7 +250,7 @@ impl Builder { 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 @@ -273,9 +272,9 @@ impl Builder { &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); } @@ -326,7 +325,7 @@ impl Builder { fn build_internal( self, prover: &impl TxProver, - fee: I64Sum, + fee: U64Sum, ) -> Result<(Transaction, SaplingMetadata), Error> { let consensus_branch_id = BranchId::for_height(&self.params, self.target_height); @@ -480,9 +479,8 @@ 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, @@ -490,7 +488,7 @@ mod tests { use super::{Builder, Error}; - #[test] + /*#[test] fn fails_on_overflow_output() { let extsk = ExtendedSpendingKey::master(&[]); let dfvk = extsk.to_diversifiable_full_viewing_key(); @@ -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 { @@ -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; diff --git a/masp_primitives/src/transaction/components.rs b/masp_primitives/src/transaction/components.rs index 2cb438fe..7e76b55b 100644 --- a/masp_primitives/src/transaction/components.rs +++ b/masp_primitives/src/transaction/components.rs @@ -4,7 +4,10 @@ pub mod amount; pub mod sapling; pub mod transparent; pub use self::{ - amount::{I128Sum, I64Sum, ValueSum}, + amount::{ + FromNt, I128Sum, I16Sum, I32Sum, I64Sum, I8Sum, TryFromNt, U128Sum, U16Sum, U32Sum, U64Sum, + U8Sum, ValueSum, + }, sapling::{ConvertDescription, OutputDescription, SpendDescription}, transparent::{TxIn, TxOut}, }; diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index 4c09cc8c..56f2c287 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -11,10 +11,9 @@ 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(); +pub static ref DEFAULT_FEE: U64Sum = ValueSum::from_pair(zec(), 1000).unwrap(); } /// A type-safe representation of some quantity of Zcash. /// @@ -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 { if amount == Magnitude::default() { Ok(Self::zero()) @@ -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 { if amount == Magnitude::default() { Ok(Self::zero()) @@ -653,29 +648,29 @@ pub fn default_fee() -> ValueSum { 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() } } @@ -683,7 +678,7 @@ pub mod testing { #[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() { @@ -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"; @@ -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() ); } @@ -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(); } } diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index ad0bf9b4..ceda98a8 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -146,7 +146,7 @@ impl SaplingOutputInfo { memo: MemoBytes, ) -> Result { 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); } diff --git a/masp_primitives/src/transaction/components/transparent.rs b/masp_primitives/src/transaction/components/transparent.rs index d2553fc5..85391e8c 100644 --- a/masp_primitives/src/transaction/components/transparent.rs +++ b/masp_primitives/src/transaction/components/transparent.rs @@ -65,26 +65,14 @@ impl Bundle { 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::>() .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::>() .map_err(|_| BalanceError::Overflow)?; @@ -96,7 +84,7 @@ impl Bundle { #[derive(Debug, Clone, PartialEq, Eq)] pub struct TxIn { pub asset_type: AssetType, - pub value: i64, + pub value: u64, pub address: TransparentAddress, pub transparent_sig: A::TransparentSig, } @@ -112,9 +100,9 @@ impl TxIn { 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", @@ -144,7 +132,7 @@ impl TxIn { #[derive(Clone, Debug, Hash, PartialOrd, PartialEq, Ord, Eq)] pub struct TxOut { pub asset_type: AssetType, - pub value: i64, + pub value: u64, pub address: TransparentAddress, } @@ -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", diff --git a/masp_primitives/src/transaction/components/transparent/builder.rs b/masp_primitives/src/transaction/components/transparent/builder.rs index b5aa9fe9..f97eb491 100644 --- a/masp_primitives/src/transaction/components/transparent/builder.rs +++ b/masp_primitives/src/transaction/components/transparent/builder.rs @@ -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(()) @@ -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); } @@ -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::>() .map_err(|_| BalanceError::Overflow)?; @@ -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::>() .map_err(|_| BalanceError::Overflow)?; @@ -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() diff --git a/masp_primitives/src/transaction/components/transparent/fees.rs b/masp_primitives/src/transaction/components/transparent/fees.rs index cff0a544..99522494 100644 --- a/masp_primitives/src/transaction/components/transparent/fees.rs +++ b/masp_primitives/src/transaction/components/transparent/fees.rs @@ -16,7 +16,7 @@ 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. @@ -24,7 +24,7 @@ pub trait OutputView { } impl OutputView for TxOut { - fn value(&self) -> i64 { + fn value(&self) -> u64 { self.value } diff --git a/masp_primitives/src/transaction/fees.rs b/masp_primitives/src/transaction/fees.rs index eed0f838..24d50aad 100644 --- a/masp_primitives/src/transaction/fees.rs +++ b/masp_primitives/src/transaction/fees.rs @@ -2,7 +2,7 @@ use crate::{ consensus::{self, BlockHeight}, - transaction::components::{amount::I64Sum, transparent::fees as transparent}, + transaction::components::{amount::U64Sum, transparent::fees as transparent}, }; pub mod fixed; @@ -24,5 +24,5 @@ pub trait FeeRule { transparent_outputs: &[impl transparent::OutputView], sapling_input_count: usize, sapling_output_count: usize, - ) -> Result; + ) -> Result; } diff --git a/masp_primitives/src/transaction/fees/fixed.rs b/masp_primitives/src/transaction/fees/fixed.rs index 92d434da..8ea28c72 100644 --- a/masp_primitives/src/transaction/fees/fixed.rs +++ b/masp_primitives/src/transaction/fees/fixed.rs @@ -1,7 +1,7 @@ use crate::{ consensus::{self, BlockHeight}, transaction::components::{ - amount::{I64Sum, DEFAULT_FEE}, + amount::{U64Sum, DEFAULT_FEE}, transparent::fees as transparent, }, }; @@ -10,12 +10,12 @@ use crate::{ /// the transaction being constructed. #[derive(Clone, Debug)] pub struct FeeRule { - fixed_fee: I64Sum, + fixed_fee: U64Sum, } impl FeeRule { /// Creates a new nonstandard fixed fee rule with the specified fixed fee. - pub fn non_standard(fixed_fee: I64Sum) -> Self { + pub fn non_standard(fixed_fee: U64Sum) -> Self { Self { fixed_fee } } @@ -27,7 +27,7 @@ impl FeeRule { } /// Returns the fixed fee amount which which this rule was configured. - pub fn fixed_fee(&self) -> I64Sum { + pub fn fixed_fee(&self) -> U64Sum { self.fixed_fee.clone() } } @@ -42,7 +42,7 @@ impl super::FeeRule for FeeRule { _transparent_outputs: &[impl transparent::OutputView], _sapling_input_count: usize, _sapling_output_count: usize, - ) -> Result { + ) -> Result { Ok(self.fixed_fee.clone()) } } diff --git a/masp_primitives/src/transaction/sighash.rs b/masp_primitives/src/transaction/sighash.rs index e6c9d3c4..c4d4f5d7 100644 --- a/masp_primitives/src/transaction/sighash.rs +++ b/masp_primitives/src/transaction/sighash.rs @@ -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 diff --git a/masp_proofs/benches/convert.rs b/masp_proofs/benches/convert.rs index a295cf00..60fbbc74 100644 --- a/masp_proofs/benches/convert.rs +++ b/masp_proofs/benches/convert.rs @@ -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()