From f356fa358f3f40675819e11fff051003f796cffa Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sat, 27 Jan 2024 12:31:07 -0700 Subject: [PATCH] zcash_protocol: Modify `Zatoshis` to directly wrap a u64 --- components/zcash_protocol/src/value.rs | 98 ++++++++++++++------------ zcash_client_backend/src/zip321.rs | 54 ++++++-------- 2 files changed, 72 insertions(+), 80 deletions(-) diff --git a/components/zcash_protocol/src/value.rs b/components/zcash_protocol/src/value.rs index 7767220c00..34070a8a8e 100644 --- a/components/zcash_protocol/src/value.rs +++ b/components/zcash_protocol/src/value.rs @@ -5,8 +5,9 @@ use std::ops::{Add, AddAssign, Mul, Neg, Sub, SubAssign}; use memuse::DynamicUsage; -pub const COIN: i64 = 1_0000_0000; -pub const MAX_MONEY: i64 = 21_000_000 * COIN; +pub const COIN: u64 = 1_0000_0000; +pub const MAX_MONEY: u64 = 21_000_000 * COIN; +pub const MAX_BALANCE: i64 = MAX_MONEY as i64; /// A type-safe representation of a Zcash value delta, in zatoshis. /// @@ -32,25 +33,25 @@ impl ZatBalance { /// Creates a constant ZatBalance from an i64. /// - /// Panics: if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`. + /// Panics: if the amount is outside the range `{-MAX_BALANCE..MAX_BALANCE}`. pub const fn const_from_i64(amount: i64) -> Self { - assert!(-MAX_MONEY <= amount && amount <= MAX_MONEY); // contains is not const + assert!(-MAX_BALANCE <= amount && amount <= MAX_BALANCE); // contains is not const ZatBalance(amount) } /// Creates a constant ZatBalance from a u64. /// - /// Panics: if the amount is outside the range `{0..MAX_MONEY}`. - const fn const_from_u64(amount: u64) -> Self { - assert!(amount <= (MAX_MONEY as u64)); // contains is not const + /// Panics: if the amount is outside the range `{0..MAX_BALANCE}`. + pub const fn const_from_u64(amount: u64) -> Self { + assert!(amount <= MAX_MONEY); // contains is not const ZatBalance(amount as i64) } /// Creates an ZatBalance from an i64. /// - /// Returns an error if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`. + /// Returns an error if the amount is outside the range `{-MAX_BALANCE..MAX_BALANCE}`. pub fn from_i64(amount: i64) -> Result { - if (-MAX_MONEY..=MAX_MONEY).contains(&amount) { + if (-MAX_BALANCE..=MAX_BALANCE).contains(&amount) { Ok(ZatBalance(amount)) } else { Err(()) @@ -59,9 +60,9 @@ impl ZatBalance { /// Creates a non-negative ZatBalance from an i64. /// - /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. + /// Returns an error if the amount is outside the range `{0..MAX_BALANCE}`. pub fn from_nonnegative_i64(amount: i64) -> Result { - if (0..=MAX_MONEY).contains(&amount) { + if (0..=MAX_BALANCE).contains(&amount) { Ok(ZatBalance(amount)) } else { Err(()) @@ -72,7 +73,7 @@ impl ZatBalance { /// /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. pub fn from_u64(amount: u64) -> Result { - if amount <= MAX_MONEY as u64 { + if amount <= MAX_MONEY { Ok(ZatBalance(amount as i64)) } else { Err(()) @@ -81,7 +82,7 @@ impl ZatBalance { /// Reads an ZatBalance from a signed 64-bit little-endian integer. /// - /// Returns an error if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`. + /// Returns an error if the amount is outside the range `{-MAX_BALANCE..MAX_BALANCE}`. pub fn from_i64_le_bytes(bytes: [u8; 8]) -> Result { let amount = i64::from_le_bytes(bytes); ZatBalance::from_i64(amount) @@ -89,7 +90,7 @@ impl ZatBalance { /// Reads a non-negative ZatBalance from a signed 64-bit little-endian integer. /// - /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. + /// Returns an error if the amount is outside the range `{0..MAX_BALANCE}`. pub fn from_nonnegative_i64_le_bytes(bytes: [u8; 8]) -> Result { let amount = i64::from_le_bytes(bytes); ZatBalance::from_nonnegative_i64(amount) @@ -97,7 +98,7 @@ impl ZatBalance { /// Reads an ZatBalance from an unsigned 64-bit little-endian integer. /// - /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. + /// Returns an error if the amount is outside the range `{0..MAX_BALANCE}`. pub fn from_u64_le_bytes(bytes: [u8; 8]) -> Result { let amount = u64::from_le_bytes(bytes); ZatBalance::from_u64(amount) @@ -237,36 +238,41 @@ impl Mul for ZatBalance { /// A Zatoshis can only be constructed from an integer that is within the valid monetary /// range of `{0..MAX_MONEY}` (where `MAX_MONEY` = 21,000,000 × 10⁸ zatoshis). #[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Eq, Ord)] -pub struct Zatoshis(ZatBalance); +pub struct Zatoshis(u64); impl Zatoshis { /// Returns the identity `Zatoshis` - pub const ZERO: Self = Zatoshis(ZatBalance(0)); + pub const ZERO: Self = Zatoshis(0); /// Returns this Zatoshis as a u64. pub fn into_u64(self) -> u64 { - self.0.try_into().unwrap() + self.0 } /// Creates a Zatoshis from a u64. /// /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. pub fn from_u64(amount: u64) -> Result { - ZatBalance::from_u64(amount).map(Zatoshis) + if (0..=MAX_MONEY).contains(&amount) { + Ok(Zatoshis(amount)) + } else { + Err(()) + } } /// Creates a constant Zatoshis from a u64. /// - /// Panics: if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`. + /// Panics: if the amount is outside the range `{0..MAX_MONEY}`. pub const fn const_from_u64(amount: u64) -> Self { - Zatoshis(ZatBalance::const_from_u64(amount)) + assert!(amount <= MAX_MONEY); // contains is not const + Zatoshis(amount) } /// Creates a Zatoshis from an i64. /// /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. pub fn from_nonnegative_i64(amount: i64) -> Result { - ZatBalance::from_nonnegative_i64(amount).map(Zatoshis) + u64::try_from(amount).map(Zatoshis).map_err(|_| ()) } /// Reads an Zatoshis from an unsigned 64-bit little-endian integer. @@ -289,7 +295,7 @@ impl Zatoshis { /// Returns this Zatoshis encoded as a signed two's complement 64-bit /// little-endian value. pub fn to_i64_le_bytes(self) -> [u8; 8] { - self.0.to_i64_le_bytes() + (self.0 as i64).to_le_bytes() } /// Returns whether or not this `Zatoshis` is the zero value. @@ -305,13 +311,13 @@ impl Zatoshis { impl From for ZatBalance { fn from(n: Zatoshis) -> Self { - n.0 + ZatBalance(n.0 as i64) } } impl From<&Zatoshis> for ZatBalance { fn from(n: &Zatoshis) -> Self { - n.0 + ZatBalance(n.0 as i64) } } @@ -333,11 +339,7 @@ impl TryFrom for Zatoshis { type Error = (); fn try_from(value: ZatBalance) -> Result { - if value.is_negative() { - Err(()) - } else { - Ok(Zatoshis(value)) - } + Zatoshis::from_nonnegative_i64(value.0) } } @@ -345,7 +347,7 @@ impl Add for Zatoshis { type Output = Option; fn add(self, rhs: Zatoshis) -> Option { - (self.0 + rhs.0).map(Zatoshis) + Self::from_u64(self.0.checked_add(rhs.0)?).ok() } } @@ -361,7 +363,7 @@ impl Sub for Zatoshis { type Output = Option; fn sub(self, rhs: Zatoshis) -> Option { - (self.0 - rhs.0).and_then(|amt| Zatoshis::try_from(amt).ok()) + Zatoshis::from_u64(self.0.checked_sub(rhs.0)?).ok() } } @@ -377,7 +379,7 @@ impl Mul for Zatoshis { type Output = Option; fn mul(self, rhs: usize) -> Option { - (self.0 * rhs).and_then(|v| Zatoshis::try_from(v).ok()) + Zatoshis::from_u64(self.0.checked_mul(u64::try_from(rhs).ok()?)?).ok() } } @@ -430,30 +432,32 @@ impl From for BalanceError { pub mod testing { use proptest::prelude::prop_compose; - use super::{ZatBalance, Zatoshis, MAX_MONEY}; + use super::{ZatBalance, Zatoshis, MAX_BALANCE, MAX_MONEY}; prop_compose! { - pub fn arb_zat_balance()(amt in -MAX_MONEY..MAX_MONEY) -> ZatBalance { + pub fn arb_zat_balance()(amt in -MAX_BALANCE..MAX_BALANCE) -> ZatBalance { ZatBalance::from_i64(amt).unwrap() } } prop_compose! { - pub fn arb_positive_zat_balance()(amt in 1i64..MAX_MONEY) -> ZatBalance { + pub fn arb_positive_zat_balance()(amt in 1i64..MAX_BALANCE) -> ZatBalance { ZatBalance::from_i64(amt).unwrap() } } prop_compose! { - pub fn arb_zatoshis()(amt in 0i64..MAX_MONEY) -> Zatoshis { - Zatoshis::from_u64(amt as u64).unwrap() + pub fn arb_zatoshis()(amt in 0u64..MAX_MONEY) -> Zatoshis { + Zatoshis::from_u64(amt).unwrap() } } } #[cfg(test)] mod tests { - use super::{ZatBalance, MAX_MONEY}; + use crate::value::MAX_BALANCE; + + use super::ZatBalance; #[test] fn amount_in_range() { @@ -476,15 +480,15 @@ mod tests { let max_money = b"\x00\x40\x07\x5a\xf0\x75\x07\x00"; assert_eq!( ZatBalance::from_u64_le_bytes(*max_money).unwrap(), - ZatBalance(MAX_MONEY) + ZatBalance(MAX_BALANCE) ); assert_eq!( ZatBalance::from_nonnegative_i64_le_bytes(*max_money).unwrap(), - ZatBalance(MAX_MONEY) + ZatBalance(MAX_BALANCE) ); assert_eq!( ZatBalance::from_i64_le_bytes(*max_money).unwrap(), - ZatBalance(MAX_MONEY) + ZatBalance(MAX_BALANCE) ); let max_money_p1 = b"\x01\x40\x07\x5a\xf0\x75\x07\x00"; @@ -497,7 +501,7 @@ mod tests { assert!(ZatBalance::from_nonnegative_i64_le_bytes(*neg_max_money).is_err()); assert_eq!( ZatBalance::from_i64_le_bytes(*neg_max_money).unwrap(), - ZatBalance(-MAX_MONEY) + ZatBalance(-MAX_BALANCE) ); let neg_max_money_m1 = b"\xff\xbf\xf8\xa5\x0f\x8a\xf8\xff"; @@ -508,27 +512,27 @@ mod tests { #[test] fn add_overflow() { - let v = ZatBalance(MAX_MONEY); + let v = ZatBalance(MAX_BALANCE); assert_eq!(v + ZatBalance(1), None) } #[test] #[should_panic] fn add_assign_panics_on_overflow() { - let mut a = ZatBalance(MAX_MONEY); + let mut a = ZatBalance(MAX_BALANCE); a += ZatBalance(1); } #[test] fn sub_underflow() { - let v = ZatBalance(-MAX_MONEY); + let v = ZatBalance(-MAX_BALANCE); assert_eq!(v - ZatBalance(1), None) } #[test] #[should_panic] fn sub_assign_panics_on_underflow() { - let mut a = ZatBalance(-MAX_MONEY); + let mut a = ZatBalance(-MAX_BALANCE); a -= ZatBalance(1); } } diff --git a/zcash_client_backend/src/zip321.rs b/zcash_client_backend/src/zip321.rs index a062021c61..42ad986877 100644 --- a/zcash_client_backend/src/zip321.rs +++ b/zcash_client_backend/src/zip321.rs @@ -361,9 +361,8 @@ mod render { use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS}; use zcash_primitives::{ - consensus, + consensus, transaction::components::amount::NonNegativeAmount, transaction::components::amount::COIN, - transaction::components::{amount::NonNegativeAmount, Amount}, }; use super::{memo_to_base64, Address, MemoBytes}; @@ -416,10 +415,10 @@ mod render { /// Converts an [`Amount`] value to a correctly formatted decimal ZEC /// value for inclusion in a ZIP 321 URI. - pub fn amount_str(amount: Amount) -> Option { + pub fn amount_str(amount: NonNegativeAmount) -> Option { if amount.is_positive() { - let coins = i64::from(amount) / COIN; - let zats = i64::from(amount) % COIN; + let coins = u64::from(amount) / COIN; + let zats = u64::from(amount) % COIN; Some(if zats == 0 { format!("{}", coins) } else { @@ -435,7 +434,7 @@ mod render { /// Constructs an "amount" key/value pair containing the encoded ZEC amount /// at the specified parameter index. pub fn amount_param(amount: NonNegativeAmount, idx: Option) -> Option { - amount_str(amount.into()).map(|s| format!("amount{}={}", param_index(idx), s)) + amount_str(amount).map(|s| format!("amount{}={}", param_index(idx), s)) } /// Constructs a "memo" key/value pair containing the base64URI-encoded memo @@ -468,9 +467,8 @@ mod parse { }; use percent_encoding::percent_decode; use zcash_primitives::{ - consensus, + consensus, transaction::components::amount::NonNegativeAmount, transaction::components::amount::COIN, - transaction::components::{amount::NonNegativeAmount, Amount}, }; use crate::address::Address; @@ -644,7 +642,7 @@ mod parse { } /// Parses a value in decimal ZEC. - pub fn parse_amount(input: &str) -> IResult<&str, Amount> { + pub fn parse_amount(input: &str) -> IResult<&str, NonNegativeAmount> { map_res( tuple(( digit1, @@ -654,29 +652,24 @@ mod parse { )), )), |(whole_s, decimal_s): (&str, Option<&str>)| { - let coins: i64 = whole_s + let coins: u64 = whole_s .to_string() - .parse::() + .parse::() .map_err(|e| e.to_string())?; - let zats: i64 = match decimal_s { + let zats: u64 = match decimal_s { Some(d) => format!("{:0<8}", d) - .parse::() + .parse::() .map_err(|e| e.to_string())?, None => 0, }; - if coins >= 21000000 && (coins > 21000000 || zats > 0) { - return Err(format!( - "{} coins exceeds the maximum possible Zcash value.", - coins - )); - } - - let amt = coins * COIN + zats; - - Amount::from_nonnegative_i64(amt) - .map_err(|_| format!("Not a valid zat amount: {}", amt)) + coins + .checked_mul(COIN) + .and_then(|coin_zats| coin_zats.checked_add(zats)) + .ok_or(()) + .and_then(NonNegativeAmount::from_u64) + .map_err(|_| format!("Not a valid zat amount: {}.{}", coins, zats)) }, )(input) } @@ -696,11 +689,7 @@ mod parse { "amount" => parse_amount(value) .map_err(|e| e.to_string()) - .and_then(|(_, a)| { - NonNegativeAmount::try_from(a) - .map_err(|_| "Payment amount must be nonnegative.".to_owned()) - }) - .map(Param::Amount), + .map(|(_, amt)| Param::Amount(amt)), "label" => percent_decode(value.as_bytes()) .decode_utf8() @@ -838,7 +827,7 @@ mod tests { use zcash_primitives::{ consensus::{Parameters, TEST_NETWORK}, memo::Memo, - transaction::components::{amount::NonNegativeAmount, Amount}, + transaction::components::amount::NonNegativeAmount, }; #[cfg(feature = "local-consensus")] @@ -880,7 +869,7 @@ mod tests { let amounts = vec![1u64, 1000u64, 100000u64, 100000000u64, 100000000000u64]; for amt_u64 in amounts { - let amt = Amount::from_u64(amt_u64).unwrap(); + let amt = NonNegativeAmount::from_u64(amt_u64).unwrap(); let amt_str = amount_str(amt).unwrap(); assert_eq!(amt, parse_amount(&amt_str).unwrap().1); } @@ -1113,8 +1102,7 @@ mod tests { } #[test] - fn prop_zip321_roundtrip_amount(nn_amt in arb_nonnegative_amount()) { - let amt = Amount::from(nn_amt); + fn prop_zip321_roundtrip_amount(amt in arb_nonnegative_amount()) { let amt_str = amount_str(amt).unwrap(); assert_eq!(amt, parse_amount(&amt_str).unwrap().1); }