diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs index 4ae5c9ce1..c048ba367 100644 --- a/bitcoin/src/blockdata/script/borrowed.rs +++ b/bitcoin/src/blockdata/script/borrowed.rs @@ -413,7 +413,7 @@ crate::internal_macros::define_extension_trait! { // Note: We ensure the division happens at the end, since Core performs the division at the end. // This will make sure none of the implicit floor operations mess with the value. - Some(Amount::from_sat(sats)) + Amount::from_sat(sats).ok() } fn count_sigops_internal(&self, accurate: bool) -> usize { diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 308248313..29fe22483 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -198,7 +198,7 @@ pub mod amount { //! This module mainly introduces the [`Amount`] and [`SignedAmount`] types. //! We refer to the documentation on the types for more information. - use crate::consensus::{encode, Decodable, Encodable}; + use crate::consensus::{self, encode, Decodable, Encodable}; use crate::io::{BufRead, Write}; #[rustfmt::skip] // Keep public re-exports separate. @@ -215,7 +215,9 @@ pub mod amount { impl Decodable for Amount { #[inline] fn consensus_decode(r: &mut R) -> Result { - Ok(Amount::from_sat(Decodable::consensus_decode(r)?)) + Amount::from_sat(Decodable::consensus_decode(r)?).map_err(|_| { + consensus::parse_failed_error("amount is greater than Amount::MAX_MONEY") + }) } } diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index b2787d73e..bfe528258 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -1268,7 +1268,7 @@ mod tests { witness: Witness::default(), }], output: vec![TxOut { - value: Amount::from_sat(output), + value: Amount::from_sat(output).unwrap(), script_pubkey: ScriptBuf::from_hex( "a9143545e6e33b832c47050f24d3eeb93c9c03948bc787", ) @@ -1282,7 +1282,7 @@ mod tests { inputs: vec![Input { witness_utxo: Some(TxOut { - value: Amount::from_sat(input), + value: Amount::from_sat(input).unwrap(), script_pubkey: ScriptBuf::from_hex( "a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587", ) diff --git a/bitcoin/tests/psbt-sign-taproot.rs b/bitcoin/tests/psbt-sign-taproot.rs index 89611e799..0f8cf5933 100644 --- a/bitcoin/tests/psbt-sign-taproot.rs +++ b/bitcoin/tests/psbt-sign-taproot.rs @@ -205,7 +205,7 @@ fn create_psbt_for_taproot_key_path_spend( ) -> Psbt { let send_value = 6400; let out_puts = vec![TxOut { - value: Amount::from_sat(send_value), + value: Amount::from_sat(send_value).unwrap(), script_pubkey: to_address.script_pubkey(), }]; let prev_tx_id = "06980ca116f74c7845a897461dd0e1d15b114130176de5004957da516b4dee3a"; @@ -243,7 +243,7 @@ fn create_psbt_for_taproot_key_path_spend( let mut input = Input { witness_utxo: { let script_pubkey = from_address.script_pubkey(); - Some(TxOut { value: Amount::from_sat(utxo_value), script_pubkey }) + Some(TxOut { value: Amount::from_sat(utxo_value).unwrap(), script_pubkey }) }, tap_key_origins: origins, ..Default::default() @@ -283,7 +283,7 @@ fn create_psbt_for_taproot_script_path_spend( let mfp = "73c5da0a"; let out_puts = vec![TxOut { - value: Amount::from_sat(send_value), + value: Amount::from_sat(send_value).unwrap(), script_pubkey: to_address.script_pubkey(), }]; let prev_tx_id = "9d7c6770fca57285babab60c51834cfcfd10ad302119cae842d7216b4ac9a376"; @@ -322,7 +322,7 @@ fn create_psbt_for_taproot_script_path_spend( let mut input = Input { witness_utxo: { let script_pubkey = from_address.script_pubkey(); - Some(TxOut { value: Amount::from_sat(utxo_value), script_pubkey }) + Some(TxOut { value: Amount::from_sat(utxo_value).unwrap(), script_pubkey }) }, tap_key_origins: origins, tap_scripts, diff --git a/primitives/src/transaction.rs b/primitives/src/transaction.rs index 6dfef80a8..bb2f81522 100644 --- a/primitives/src/transaction.rs +++ b/primitives/src/transaction.rs @@ -670,7 +670,7 @@ mod tests { witness: Witness::new(), }; - let txout = TxOut { value: Amount::from_sat(123_456_789), script_pubkey: ScriptBuf::new() }; + let txout = TxOut { value: Amount::from_sat(123_456_789).unwrap(), script_pubkey: ScriptBuf::new() }; let tx_orig = Transaction { version: Version::ONE, @@ -682,7 +682,7 @@ mod tests { // Test changing the transaction let mut tx = tx_orig.clone(); tx.inputs_mut()[0].previous_output.txid = Txid::from_byte_array([0xFF; 32]); - tx.outputs_mut()[0].value = Amount::from_sat(987_654_321); + tx.outputs_mut()[0].value = Amount::from_sat(987_654_321).unwrap(); assert_eq!(tx.inputs()[0].previous_output.txid.to_byte_array(), [0xFF; 32]); assert_eq!(tx.outputs()[0].value.to_sat(), 987_654_321); diff --git a/units/src/amount/mod.rs b/units/src/amount/mod.rs index 940b9a5a8..9333b87e6 100644 --- a/units/src/amount/mod.rs +++ b/units/src/amount/mod.rs @@ -59,7 +59,7 @@ pub(crate) use self::result::OptionExt; /// # Examples /// /// ``` -/// # use bitcoin_units::Amount; +/// # use bitcoin_units::{amount, Amount}; /// /// let equal = [ /// ("1 BTC", 100_000_000), @@ -72,9 +72,10 @@ pub(crate) use self::result::OptionExt; /// for (string, sats) in equal { /// assert_eq!( /// string.parse::().expect("valid bitcoin amount string"), -/// Amount::from_sat(sats), +/// Amount::from_sat(sats)?, /// ) /// } +/// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] #[non_exhaustive] diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs index 4b78d0080..ab06f207f 100644 --- a/units/src/amount/result.rs +++ b/units/src/amount/result.rs @@ -328,7 +328,9 @@ crate::internal_macros::impl_op_for_references! { impl ops::Neg for SignedAmount { type Output = Self; - fn neg(self) -> Self::Output { Self::from_sat(self.to_sat().neg()) } + fn neg(self) -> Self::Output { + Self::from_sat(self.to_sat().neg()).expect("all +ve and -ve values are valid") + } } impl core::iter::Sum> for NumOpResult { diff --git a/units/src/amount/serde.rs b/units/src/amount/serde.rs index 4754d6cb2..1db2e9eea 100644 --- a/units/src/amount/serde.rs +++ b/units/src/amount/serde.rs @@ -92,7 +92,8 @@ impl SerdeAmount for Amount { u64::serialize(&self.to_sat(), s) } fn des_sat<'d, D: Deserializer<'d>>(d: D, _: private::Token) -> Result { - Ok(Amount::from_sat(u64::deserialize(d)?)) + use serde::de::Error; + Amount::from_sat(u64::deserialize(d)?).map_err(D::Error::custom) } #[cfg(feature = "alloc")] fn ser_btc(self, s: S, _: private::Token) -> Result { @@ -137,7 +138,8 @@ impl SerdeAmount for SignedAmount { i64::serialize(&self.to_sat(), s) } fn des_sat<'d, D: Deserializer<'d>>(d: D, _: private::Token) -> Result { - Ok(SignedAmount::from_sat(i64::deserialize(d)?)) + use serde::de::Error; + SignedAmount::from_sat(i64::deserialize(d)?).map_err(D::Error::custom) } #[cfg(feature = "alloc")] fn ser_btc(self, s: S, _: private::Token) -> Result { diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index a04795de6..3f7927b47 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -87,17 +87,29 @@ impl SignedAmount { /// The maximum value of an amount. pub const MAX: Self = SignedAmount::MAX_MONEY; - /// Constructs a new [`SignedAmount`] with satoshi precision and the given number of satoshis. + /// Constructs a new [`SignedAmount`] from the given number of satoshis. + /// + /// # Errors + /// + /// If `satoshi` is outside of valid range (see [`Self::MAX_MONEY`]). /// /// # Examples /// /// ``` - /// # use bitcoin_units::SignedAmount; - /// let amount = SignedAmount::from_sat(-100_000); - /// assert_eq!(amount.to_sat(), -100_000); + /// # use bitcoin_units::{amount, SignedAmount}; + /// # let sat = -100_000; + /// let amount = SignedAmount::from_sat(sat)?; + /// assert_eq!(amount.to_sat(), sat); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` - pub const fn from_sat(satoshi: i64) -> SignedAmount { - SignedAmount::from_sat_unchecked(satoshi) + pub const fn from_sat(satoshi: i64) -> Result { + if satoshi < Self::MIN.to_sat() { + Err(OutOfRangeError { is_signed: true, is_greater_than_max: false }) + } else if satoshi > Self::MAX_MONEY.to_sat() { + Err(OutOfRangeError { is_signed: true, is_greater_than_max: true }) + } else { + Ok(SignedAmount::from_sat_unchecked(satoshi)) + } } /// Converts from a value expressing a decimal number of bitcoin to a [`SignedAmount`]. @@ -111,9 +123,10 @@ impl SignedAmount { /// # Examples /// /// ``` - /// # use bitcoin_units::SignedAmount; - /// let amount = SignedAmount::from_btc(-0.01).expect("we know 0.01 is valid"); + /// # use bitcoin_units::{amount, SignedAmount}; + /// let amount = SignedAmount::from_btc(-0.01)?; /// assert_eq!(amount.to_sat(), -1_000_000); + /// # Ok::<_, amount::ParseAmountError>(()) /// ``` #[cfg(feature = "alloc")] pub fn from_btc(btc: f64) -> Result { @@ -121,15 +134,23 @@ impl SignedAmount { } /// Converts from a value expressing a whole number of bitcoin to a [`SignedAmount`]. + /// + /// # Errors + /// + /// If `whole_bitcoin` is greater than `21_000_000`. #[allow(clippy::missing_panics_doc)] - pub fn from_int_btc>(whole_bitcoin: T) -> SignedAmount { + pub fn from_int_btc>(whole_bitcoin: T) -> Result { SignedAmount::from_int_btc_const(whole_bitcoin.into()) } /// Converts from a value expressing a whole number of bitcoin to a [`SignedAmount`] /// in const context. + /// + /// # Errors + /// + /// If `whole_bitcoin` is greater than `21_000_000`. #[allow(clippy::missing_panics_doc)] - pub const fn from_int_btc_const(whole_bitcoin: i32) -> SignedAmount { + pub const fn from_int_btc_const(whole_bitcoin: i32) -> Result { let btc = whole_bitcoin as i64; // Can't call `into` in const context. match btc.checked_mul(100_000_000) { Some(amount) => SignedAmount::from_sat(amount), @@ -151,11 +172,11 @@ impl SignedAmount { (false, sat) if sat > SignedAmount::MAX.to_sat() as u64 => Err(ParseAmountError( ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_big(true)), )), - (false, sat) => Ok(SignedAmount::from_sat(sat as i64)), // Cast ok, value in this arm does not wrap. + (false, sat) => Ok(SignedAmount::from_sat_unchecked(sat as i64)), // Cast ok, value in this arm does not wrap. (true, sat) if sat > SignedAmount::MIN.to_sat().unsigned_abs() => Err( ParseAmountError(ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_small())), ), - (true, sat) => Ok(SignedAmount::from_sat(-(sat as i64))), // Cast ok, value in this arm does not wrap. + (true, sat) => Ok(SignedAmount::from_sat_unchecked(-(sat as i64))), // Cast ok, value in this arm does not wrap. } } @@ -173,7 +194,7 @@ impl SignedAmount { /// ``` /// # use bitcoin_units::{amount, SignedAmount}; /// let amount = SignedAmount::from_str_with_denomination("0.1 BTC")?; - /// assert_eq!(amount, SignedAmount::from_sat(10_000_000)); + /// assert_eq!(amount, SignedAmount::from_sat(10_000_000)?); /// # Ok::<_, amount::ParseError>(()) /// ``` pub fn from_str_with_denomination(s: &str) -> Result { @@ -188,9 +209,10 @@ impl SignedAmount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{SignedAmount, Denomination}; - /// let amount = SignedAmount::from_sat(100_000); - /// assert_eq!(amount.to_float_in(Denomination::Bitcoin), 0.001) + /// # use bitcoin_units::amount::{self, SignedAmount, Denomination}; + /// let amount = SignedAmount::from_sat(100_000)?; + /// assert_eq!(amount.to_float_in(Denomination::Bitcoin), 0.001); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[cfg(feature = "alloc")] #[allow(clippy::missing_panics_doc)] @@ -205,9 +227,10 @@ impl SignedAmount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{SignedAmount, Denomination}; - /// let amount = SignedAmount::from_sat(100_000); - /// assert_eq!(amount.to_btc(), amount.to_float_in(Denomination::Bitcoin)) + /// # use bitcoin_units::amount::{self, SignedAmount, Denomination}; + /// let amount = SignedAmount::from_sat(100_000)?; + /// assert_eq!(amount.to_btc(), amount.to_float_in(Denomination::Bitcoin)); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[cfg(feature = "alloc")] pub fn to_btc(self) -> f64 { self.to_float_in(Denomination::Bitcoin) } @@ -236,13 +259,13 @@ impl SignedAmount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{SignedAmount, Denomination}; + /// # use bitcoin_units::amount::{self, SignedAmount, Denomination}; /// # use std::fmt::Write; - /// let amount = SignedAmount::from_sat(10_000_000); + /// let amount = SignedAmount::from_sat(10_000_000)?; /// let mut output = String::new(); - /// write!(&mut output, "{}", amount.display_in(Denomination::Bitcoin))?; + /// let _ = write!(&mut output, "{}", amount.display_in(Denomination::Bitcoin)); /// assert_eq!(output, "0.1"); - /// # Ok::<(), std::fmt::Error>(()) + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[must_use] pub fn display_in(self, denomination: Denomination) -> Display { @@ -274,9 +297,10 @@ impl SignedAmount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{SignedAmount, Denomination}; - /// let amount = SignedAmount::from_sat(10_000_000); - /// assert_eq!(amount.to_string_in(Denomination::Bitcoin), "0.1") + /// # use bitcoin_units::amount::{self, SignedAmount, Denomination}; + /// let amount = SignedAmount::from_sat(10_000_000)?; + /// assert_eq!(amount.to_string_in(Denomination::Bitcoin), "0.1"); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[cfg(feature = "alloc")] pub fn to_string_in(self, denom: Denomination) -> String { self.display_in(denom).to_string() } @@ -287,9 +311,10 @@ impl SignedAmount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{SignedAmount, Denomination}; - /// let amount = SignedAmount::from_sat(10_000_000); - /// assert_eq!(amount.to_string_with_denomination(Denomination::Bitcoin), "0.1 BTC") + /// # use bitcoin_units::amount::{self, SignedAmount, Denomination}; + /// let amount = SignedAmount::from_sat(10_000_000)?; + /// assert_eq!(amount.to_string_with_denomination(Denomination::Bitcoin), "0.1 BTC"); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[cfg(feature = "alloc")] pub fn to_string_with_denomination(self, denom: Denomination) -> String { @@ -297,12 +322,23 @@ impl SignedAmount { } /// Gets the absolute value of this [`SignedAmount`]. + /// + /// This function never overflows or panics, unlike `i64::abs()`. #[must_use] - pub fn abs(self) -> SignedAmount { SignedAmount::from_sat(self.to_sat().abs()) } + pub const fn abs(self) -> SignedAmount { + // `i64::abs()` can never overflow because SignedAmount::MIN == -MAX_MONEY. + match Self::from_sat(self.to_sat().abs()) { + Ok(amount) => amount, + Err(_) => panic!("a positive signed amount is always valid"), + } + } /// Gets the absolute value of this [`SignedAmount`] returning [`Amount`]. #[must_use] - pub fn unsigned_abs(self) -> Amount { Amount::from_sat(self.to_sat().unsigned_abs()) } + #[allow(clippy::missing_panics_doc)] + pub fn unsigned_abs(self) -> Amount { + self.abs().to_unsigned().expect("a positive signed amount is always valid") + } /// Returns a number representing sign of this [`SignedAmount`]. /// @@ -330,13 +366,9 @@ impl SignedAmount { /// /// Returns [`None`] if overflow occurred. (`self == i64::MIN`) #[must_use] - pub const fn checked_abs(self) -> Option { - // No `map()` in const context. - match self.to_sat().checked_abs() { - Some(res) => Some(SignedAmount::from_sat(res)), - None => None, - } - } + #[deprecated(since = "TBD", note = "Never returns none, use `abs()` instead")] + #[allow(clippy::unnecessary_wraps)] // To match stdlib function definition. + pub const fn checked_abs(self) -> Option { Some(self.abs()) } /// Checked addition. /// @@ -345,7 +377,10 @@ impl SignedAmount { pub const fn checked_add(self, rhs: SignedAmount) -> Option { // No `map()` in const context. match self.to_sat().checked_add(rhs.to_sat()) { - Some(res) => SignedAmount::from_sat(res).check_min_max(), + Some(res) => match SignedAmount::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, + }, None => None, } } @@ -358,7 +393,10 @@ impl SignedAmount { pub const fn checked_sub(self, rhs: SignedAmount) -> Option { // No `map()` in const context. match self.to_sat().checked_sub(rhs.to_sat()) { - Some(res) => SignedAmount::from_sat(res).check_min_max(), + Some(res) => match Self::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, + }, None => None, } } @@ -371,7 +409,10 @@ impl SignedAmount { pub const fn checked_mul(self, rhs: i64) -> Option { // No `map()` in const context. match self.to_sat().checked_mul(rhs) { - Some(res) => SignedAmount::from_sat(res).check_min_max(), + Some(res) => match Self::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, + }, None => None, } } @@ -385,7 +426,10 @@ impl SignedAmount { pub const fn checked_div(self, rhs: i64) -> Option { // No `map()` in const context. match self.to_sat().checked_div(rhs) { - Some(res) => Some(SignedAmount::from_sat(res)), + Some(res) => match Self::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, // Unreachable because of checked_div above. + }, None => None, } } @@ -397,7 +441,10 @@ impl SignedAmount { pub const fn checked_rem(self, rhs: i64) -> Option { // No `map()` in const context. match self.to_sat().checked_rem(rhs) { - Some(res) => Some(SignedAmount::from_sat(res)), + Some(res) => match Self::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, // Unreachable because of checked_rem above. + }, None => None, } } @@ -419,20 +466,14 @@ impl SignedAmount { /// # Errors /// /// If the amount is negative. + #[allow(clippy::missing_panics_doc)] pub fn to_unsigned(self) -> Result { if self.is_negative() { Err(OutOfRangeError::negative()) } else { - Ok(Amount::from_sat(self.to_sat() as u64)) // Cast ok, checked not negative above. - } - } - - /// Checks the amount is within the allowed range. - const fn check_min_max(self) -> Option { - if self.to_sat() < Self::MIN.to_sat() || self.to_sat() > Self::MAX.to_sat() { - None - } else { - Some(self) + // Cast ok, checked not negative above. + Ok(Amount::from_sat(self.to_sat() as u64) + .expect("a positive signed amount is always valid")) } } } @@ -482,14 +523,14 @@ impl FromStr for SignedAmount { impl From for SignedAmount { fn from(value: Amount) -> Self { let v = value.to_sat() as i64; // Cast ok, signed amount and amount share positive range. - Self::from_sat_unchecked(v) + Self::from_sat(v).expect("all amounts are valid signed amounts") } } #[cfg(feature = "arbitrary")] impl<'a> Arbitrary<'a> for SignedAmount { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { - let s = i64::arbitrary(u)?; - Ok(Self::from_sat(s)) + let sats = u.int_in_range(Self::MIN.to_sat()..=Self::MAX.to_sat())?; + Ok(Self::from_sat(sats).expect("range is valid")) } } diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 2d3b5e1c6..b9d5518eb 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -16,18 +16,19 @@ use super::*; #[cfg(feature = "alloc")] use crate::{FeeRate, Weight}; -fn sat(sat: u64) -> Amount { Amount::from_sat(sat) } -fn ssat(ssat: i64) -> SignedAmount { SignedAmount::from_sat(ssat) } +#[track_caller] +fn sat(sat: u64) -> Amount { Amount::from_sat(sat).unwrap() } + +#[track_caller] +fn ssat(ssat: i64) -> SignedAmount { SignedAmount::from_sat(ssat).unwrap() } #[test] fn sanity_check() { assert_eq!(ssat(-100).abs(), ssat(100)); - assert_eq!(ssat(i64::MIN + 1).checked_abs().unwrap(), ssat(i64::MAX)); assert_eq!(ssat(-100).signum(), -1); assert_eq!(ssat(0).signum(), 0); assert_eq!(ssat(100).signum(), 1); assert_eq!(SignedAmount::from(sat(100)), ssat(100)); - assert!(ssat(i64::MIN).checked_abs().is_none()); assert!(!ssat(-100).is_positive()); assert!(ssat(100).is_positive()); @@ -101,7 +102,7 @@ fn from_str_zero_without_denomination() { #[test] fn from_int_btc() { - let amt = Amount::from_int_btc_const(2); + let amt = Amount::from_int_btc_const(2).unwrap(); assert_eq!(sat(200_000_000), amt); } @@ -118,8 +119,8 @@ fn amount_try_from_signed_amount() { #[test] fn mul_div() { - let op_result_sat = |sat| NumOpResult::Valid(Amount::from_sat(sat)); - let op_result_ssat = |sat| NumOpResult::Valid(SignedAmount::from_sat(sat)); + let op_result_sat = |a| NumOpResult::Valid(sat(a)); + let op_result_ssat = |a| NumOpResult::Valid(ssat(a)); assert_eq!(sat(14) * 3, op_result_sat(42)); assert_eq!(sat(14) / 2, op_result_sat(7)); @@ -131,19 +132,10 @@ fn mul_div() { #[test] fn neg() { - let amount = -SignedAmount::from_sat_unchecked(2); + let amount = -ssat(2); assert_eq!(amount.to_sat(), -2); } -#[cfg(feature = "std")] -#[test] -fn overflows() { - let result = Amount::MAX + sat(1); - assert!(result.is_error()); - let result = sat(8_446_744_073_709_551_615) * 3; - assert!(result.is_error()); -} - #[test] fn add() { assert!(sat(0) + sat(0) == sat(0).into()); @@ -258,12 +250,6 @@ fn amount_checked_div_by_fee_rate() { let weight = max_amount.checked_div_by_fee_rate_floor(small_fee_rate).unwrap(); // 21_000_000_0000_0000 sats / (1 sat/kwu) = 2_100_000_000_000_000_000 wu assert_eq!(weight, Weight::from_wu(2_100_000_000_000_000_000)); - - // Test overflow case - let tiny_fee_rate = FeeRate::from_sat_per_kwu(1); - let large_amount = sat(u64::MAX); - assert!(large_amount.checked_div_by_fee_rate_floor(tiny_fee_rate).is_none()); - assert!(large_amount.checked_div_by_fee_rate_ceil(tiny_fee_rate).is_none()); } #[cfg(feature = "alloc")] @@ -417,8 +403,8 @@ macro_rules! check_format_non_negative { #[test] #[cfg(feature = "alloc")] fn $test_name() { - assert_eq!(format!($format_string, Amount::from_sat($val).display_in(Denomination::$denom)), $expected); - assert_eq!(format!($format_string, SignedAmount::from_sat($val as i64).display_in(Denomination::$denom)), $expected); + assert_eq!(format!($format_string, sat($val).display_in(Denomination::$denom)), $expected); + assert_eq!(format!($format_string, ssat($val as i64).display_in(Denomination::$denom)), $expected); } )* } @@ -430,8 +416,8 @@ macro_rules! check_format_non_negative_show_denom { #[test] #[cfg(feature = "alloc")] fn $test_name() { - assert_eq!(format!($format_string, Amount::from_sat($val).display_in(Denomination::$denom).show_denomination()), concat!($expected, $denom_suffix)); - assert_eq!(format!($format_string, SignedAmount::from_sat($val as i64).display_in(Denomination::$denom).show_denomination()), concat!($expected, $denom_suffix)); + assert_eq!(format!($format_string, sat($val).display_in(Denomination::$denom).show_denomination()), concat!($expected, $denom_suffix)); + assert_eq!(format!($format_string, ssat($val as i64).display_in(Denomination::$denom).show_denomination()), concat!($expected, $denom_suffix)); } )* } @@ -1020,7 +1006,7 @@ fn checked_sum_amounts() { let sum = amounts.into_iter().checked_sum(); assert_eq!(sum, Some(sat(1400))); - let amounts = [sat(u64::MAX), sat(1337), sat(21)]; + let amounts = [Amount::MAX_MONEY, sat(1337), sat(21)]; let sum = amounts.into_iter().checked_sum(); assert_eq!(sum, None); @@ -1119,7 +1105,7 @@ fn add_sub_combos() { macro_rules! check_res { ($($amount:ident, $op:tt, $lhs:literal, $rhs:literal, $ans:literal);* $(;)?) => { $( - let amt = |sat| $amount::from_sat(sat); + let amt = |sat| $amount::from_sat(sat).unwrap(); let sat_lhs = amt($lhs); let sat_rhs = amt($rhs); @@ -1255,7 +1241,7 @@ fn unsigned_amount_div_by_amount() { #[test] #[should_panic(expected = "attempt to divide by zero")] fn unsigned_amount_div_by_amount_zero() { - let _ = Amount::from_sat(1897) / Amount::ZERO; + let _ = sat(1897) / Amount::ZERO; } #[test] @@ -1271,7 +1257,7 @@ fn signed_amount_div_by_amount() { #[test] #[should_panic(expected = "attempt to divide by zero")] fn signed_amount_div_by_amount_zero() { - let _ = SignedAmount::from_sat(1897) / SignedAmount::ZERO; + let _ = ssat(1897) / SignedAmount::ZERO; } #[test] @@ -1307,8 +1293,8 @@ fn sanity_all_ops() { #[test] #[allow(clippy::op_ref)] // We are explicitly testing the references work with ops. fn num_op_result_ops() { - let sat = Amount::from_sat(1); - let ssat = SignedAmount::from_sat(1); + let sat = Amount::from_sat(1).unwrap(); + let ssat = SignedAmount::from_sat(1).unwrap(); // Explicit type as sanity check. let res: NumOpResult = sat + sat; @@ -1358,8 +1344,8 @@ fn num_op_result_ops() { #[test] #[allow(clippy::op_ref)] // We are explicitly testing the references work with ops. fn num_op_result_ops_integer() { - let sat = Amount::from_sat(1); - let ssat = SignedAmount::from_sat(1); + let sat = Amount::from_sat(1).unwrap(); + let ssat = SignedAmount::from_sat(1).unwrap(); // Explicit type as sanity check. let res: NumOpResult = sat + sat; @@ -1401,8 +1387,8 @@ fn num_op_result_ops_integer() { fn amount_op_result_neg() { // TODO: Implement Neg all round. - // let sat = Amount::from_sat(1); - let ssat = SignedAmount::from_sat(1); + // let sat = Amount::from_sat(1).unwrap(); + let ssat = SignedAmount::from_sat(1).unwrap(); // let _ = -sat; let _ = -ssat; @@ -1413,7 +1399,7 @@ fn amount_op_result_neg() { // Verify we have implemented all `Sum` for the `NumOpResult` type. #[test] fn amount_op_result_sum() { - let res = Amount::from_sat(1) + Amount::from_sat(1); + let res = Amount::from_sat(1).unwrap() + Amount::from_sat(1).unwrap(); let amounts = [res, res]; let amount_refs = [&res, &res]; diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index c3488dfaf..1b328f964 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -87,31 +87,44 @@ impl Amount { /// The number of bytes that an amount contributes to the size of a transaction. pub const SIZE: usize = 8; // Serialized length of a u64. - /// Constructs a new [`Amount`] with satoshi precision and the given number of satoshis. + /// Constructs a new [`Amount`] from the given number of satoshis. + /// + /// # Errors + /// + /// If `satoshi` is outside of valid range (greater than [`Self::MAX_MONEY`]). /// /// # Examples /// /// ``` - /// # use bitcoin_units::Amount; - /// let amount = Amount::from_sat(100_000); - /// assert_eq!(amount.to_sat(), 100_000); + /// # use bitcoin_units::{amount, Amount}; + /// # let sat = 100_000; + /// let amount = Amount::from_sat(sat)?; + /// assert_eq!(amount.to_sat(), sat); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` - pub const fn from_sat(satoshi: u64) -> Amount { Amount::from_sat_unchecked(satoshi) } + pub const fn from_sat(satoshi: u64) -> Result { + if satoshi > Self::MAX_MONEY.to_sat() { + Err(OutOfRangeError { is_signed: false, is_greater_than_max: true }) + } else { + Ok(Self::from_sat_unchecked(satoshi)) + } + } /// Converts from a value expressing a decimal number of bitcoin to an [`Amount`]. /// /// # Errors /// - /// If the amount is too big, too precise or negative. + /// If the amount is too precise, negative, or greater than 21,000,000. /// /// Please be aware of the risk of using floating-point numbers. /// /// # Examples /// /// ``` - /// # use bitcoin_units::Amount; - /// let amount = Amount::from_btc(0.01).expect("we know 0.01 is valid"); + /// # use bitcoin_units::{amount, Amount}; + /// let amount = Amount::from_btc(0.01)?; /// assert_eq!(amount.to_sat(), 1_000_000); + /// # Ok::<_, amount::ParseAmountError>(()) /// ``` #[cfg(feature = "alloc")] pub fn from_btc(btc: f64) -> Result { @@ -119,15 +132,23 @@ impl Amount { } /// Converts from a value expressing a whole number of bitcoin to an [`Amount`]. + /// + /// # Errors + /// + /// If `whole_bitcoin` is greater than `21_000_000`. #[allow(clippy::missing_panics_doc)] - pub fn from_int_btc>(whole_bitcoin: T) -> Amount { + pub fn from_int_btc>(whole_bitcoin: T) -> Result { Amount::from_int_btc_const(whole_bitcoin.into()) } /// Converts from a value expressing a whole number of bitcoin to an [`Amount`] /// in const context. + /// + /// # Errors + /// + /// If `whole_bitcoin` is greater than `21_000_000`. #[allow(clippy::missing_panics_doc)] - pub const fn from_int_btc_const(whole_bitcoin: u32) -> Amount { + pub const fn from_int_btc_const(whole_bitcoin: u32) -> Result { let btc = whole_bitcoin as u64; // Can't call `into` in const context. match btc.checked_mul(100_000_000) { Some(amount) => Amount::from_sat(amount), @@ -142,7 +163,7 @@ impl Amount { /// /// # Errors /// - /// If the amount is too big, too precise or negative. + /// If the amount is too precise, negative, or greater than 21,000,000. pub fn from_str_in(s: &str, denom: Denomination) -> Result { let (negative, sats) = parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(false))?; @@ -156,7 +177,7 @@ impl Amount { OutOfRangeError::too_big(false), ))); } - Ok(Self::from_sat(sats)) + Ok(Self::from_sat_unchecked(sats)) } /// Parses amounts with denomination suffix as produced by [`Self::to_string_with_denomination`] @@ -173,7 +194,7 @@ impl Amount { /// ``` /// # use bitcoin_units::{amount, Amount}; /// let amount = Amount::from_str_with_denomination("0.1 BTC")?; - /// assert_eq!(amount, Amount::from_sat(10_000_000)); + /// assert_eq!(amount, Amount::from_sat(10_000_000)?); /// # Ok::<_, amount::ParseError>(()) /// ``` pub fn from_str_with_denomination(s: &str) -> Result { @@ -188,9 +209,10 @@ impl Amount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{Amount, Denomination}; - /// let amount = Amount::from_sat(100_000); - /// assert_eq!(amount.to_float_in(Denomination::Bitcoin), 0.001) + /// # use bitcoin_units::amount::{self, Amount, Denomination}; + /// let amount = Amount::from_sat(100_000)?; + /// assert_eq!(amount.to_float_in(Denomination::Bitcoin), 0.001); + /// # Ok::<_, amount::ParseError>(()) /// ``` #[cfg(feature = "alloc")] #[allow(clippy::missing_panics_doc)] @@ -205,9 +227,10 @@ impl Amount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{Amount, Denomination}; - /// let amount = Amount::from_sat(100_000); - /// assert_eq!(amount.to_btc(), amount.to_float_in(Denomination::Bitcoin)) + /// # use bitcoin_units::amount::{self, Amount, Denomination}; + /// let amount = Amount::from_sat(100_000)?; + /// assert_eq!(amount.to_btc(), amount.to_float_in(Denomination::Bitcoin)); + /// # Ok::<_, amount::ParseError>(()) /// ``` #[cfg(feature = "alloc")] pub fn to_btc(self) -> f64 { self.to_float_in(Denomination::Bitcoin) } @@ -236,13 +259,13 @@ impl Amount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{Amount, Denomination}; + /// # use bitcoin_units::amount::{self, Amount, Denomination}; /// # use std::fmt::Write; - /// let amount = Amount::from_sat(10_000_000); + /// let amount = Amount::from_sat(10_000_000)?; /// let mut output = String::new(); - /// write!(&mut output, "{}", amount.display_in(Denomination::Bitcoin))?; + /// let _ = write!(&mut output, "{}", amount.display_in(Denomination::Bitcoin)); /// assert_eq!(output, "0.1"); - /// # Ok::<(), std::fmt::Error>(()) + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[must_use] pub fn display_in(self, denomination: Denomination) -> Display { @@ -274,9 +297,10 @@ impl Amount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{Amount, Denomination}; - /// let amount = Amount::from_sat(10_000_000); - /// assert_eq!(amount.to_string_in(Denomination::Bitcoin), "0.1") + /// # use bitcoin_units::amount::{self, Amount, Denomination}; + /// let amount = Amount::from_sat(10_000_000)?; + /// assert_eq!(amount.to_string_in(Denomination::Bitcoin), "0.1"); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[cfg(feature = "alloc")] pub fn to_string_in(self, denom: Denomination) -> String { self.display_in(denom).to_string() } @@ -287,9 +311,10 @@ impl Amount { /// # Examples /// /// ``` - /// # use bitcoin_units::amount::{Amount, Denomination}; - /// let amount = Amount::from_sat(10_000_000); - /// assert_eq!(amount.to_string_with_denomination(Denomination::Bitcoin), "0.1 BTC") + /// # use bitcoin_units::amount::{self, Amount, Denomination}; + /// let amount = Amount::from_sat(10_000_000)?; + /// assert_eq!(amount.to_string_with_denomination(Denomination::Bitcoin), "0.1 BTC"); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[cfg(feature = "alloc")] pub fn to_string_with_denomination(self, denom: Denomination) -> String { @@ -302,9 +327,10 @@ impl Amount { #[must_use] pub const fn checked_add(self, rhs: Amount) -> Option { // No `map()` in const context. - match self.to_sat().checked_add(rhs.to_sat()) { - Some(res) => Amount::from_sat(res).check_max(), - None => None, + // Unchecked add ok, adding two values less than `MAX_MONEY` cannot overflow an `i64`. + match Self::from_sat(self.to_sat() + rhs.to_sat()) { + Ok(amount) => Some(amount), + Err(_) => None, } } @@ -315,7 +341,10 @@ impl Amount { pub const fn checked_sub(self, rhs: Amount) -> Option { // No `map()` in const context. match self.to_sat().checked_sub(rhs.to_sat()) { - Some(res) => Some(Amount::from_sat(res)), + Some(res) => match Self::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, // Unreachable because of checked_sub above. + }, None => None, } } @@ -327,7 +356,10 @@ impl Amount { pub const fn checked_mul(self, rhs: u64) -> Option { // No `map()` in const context. match self.to_sat().checked_mul(rhs) { - Some(res) => Amount::from_sat(res).check_max(), + Some(res) => match Self::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, + }, None => None, } } @@ -341,7 +373,10 @@ impl Amount { pub const fn checked_div(self, rhs: u64) -> Option { // No `map()` in const context. match self.to_sat().checked_div(rhs) { - Some(res) => Some(Amount::from_sat(res)), + Some(res) => match Self::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, // Unreachable because of checked_div above. + }, None => None, } } @@ -353,7 +388,10 @@ impl Amount { pub const fn checked_rem(self, rhs: u64) -> Option { // No `map()` in const context. match self.to_sat().checked_rem(rhs) { - Some(res) => Some(Amount::from_sat(res)), + Some(res) => match Self::from_sat(res) { + Ok(amount) => Some(amount), + Err(_) => None, // Unreachable because of checked_div above. + }, None => None, } } @@ -363,15 +401,6 @@ impl Amount { pub fn to_signed(self) -> SignedAmount { SignedAmount::from_sat_unchecked(self.to_sat() as i64) // Cast ok, signed amount and amount share positive range. } - - /// Checks if the amount is below the maximum value. Returns `None` if it is above. - const fn check_max(self) -> Option { - if self.to_sat() > Self::MAX.to_sat() { - None - } else { - Some(self) - } - } } impl default::Default for Amount { @@ -425,7 +454,7 @@ impl TryFrom for Amount { #[cfg(feature = "arbitrary")] impl<'a> Arbitrary<'a> for Amount { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { - let a = u64::arbitrary(u)?; - Ok(Self::from_sat(a)) + let sats = u.int_in_range(Self::MIN.to_sat()..=Self::MAX.to_sat())?; + Ok(Self::from_sat(sats).expect("range is valid")) } } diff --git a/units/src/amount/verification.rs b/units/src/amount/verification.rs index ecf35d217..f8c988608 100644 --- a/units/src/amount/verification.rs +++ b/units/src/amount/verification.rs @@ -23,17 +23,21 @@ use super::*; fn u_amount_homomorphic() { let n1 = kani::any::(); let n2 = kani::any::(); - // Assume we don't overflow in the actual tests. - kani::assume(n1.checked_add(n2).is_some()); // Adding u64s doesn't overflow. - let a1 = Amount::from_sat(n1); // TODO: If from_sat enforces invariant assume this `is_ok()`. - let a2 = Amount::from_sat(n2); - kani::assume(a1.checked_add(a2).is_some()); // Adding amounts doesn't overflow. - assert_eq!(Amount::from_sat(n1) + Amount::from_sat(n2), Amount::from_sat(n1 + n2).into()); + // Assume the values are within range. + kani::assume(Amount::from_sat(n1).is_ok()); + kani::assume(Amount::from_sat(n2).is_ok()); + + let sat = |sat| Amount::from_sat(sat).unwrap(); + + // Assume sum is within range. + kani::assume(sat(n1).checked_add(sat(n2)).is_some()); + + assert_eq!(sat(n1) + sat(n2), sat(n1 + n2).into()); let max = cmp::max(n1, n2); let min = cmp::min(n1, n2); - assert_eq!(Amount::from_sat(max) - Amount::from_sat(min), Amount::from_sat(max - min).into()); + assert_eq!(sat(max) - sat(min), sat(max - min).into()); } #[kani::unwind(4)] @@ -41,20 +45,16 @@ fn u_amount_homomorphic() { fn s_amount_homomorphic() { let n1 = kani::any::(); let n2 = kani::any::(); - // Assume we don't overflow in the actual tests. - kani::assume(n1.checked_add(n2).is_some()); // Adding i64s doesn't overflow. - kani::assume(n1.checked_sub(n2).is_some()); // Subbing i64s doesn't overflow. - let a1 = SignedAmount::from_sat(n1); // TODO: If from_sat enforces invariant assume this `is_ok()`. - let a2 = SignedAmount::from_sat(n2); - kani::assume(a1.checked_add(a2).is_some()); // Adding amounts doesn't overflow. - kani::assume(a1.checked_sub(a2).is_some()); // Subbing amounts doesn't overflow. - assert_eq!( - SignedAmount::from_sat(n1) + SignedAmount::from_sat(n2), - SignedAmount::from_sat(n1 + n2).into() - ); - assert_eq!( - SignedAmount::from_sat(n1) - SignedAmount::from_sat(n2), - SignedAmount::from_sat(n1 - n2).into() - ); + // Assume the values are within range. + kani::assume(SignedAmount::from_sat(n1).is_ok()); + kani::assume(SignedAmount::from_sat(n2).is_ok()); + + let ssat = |ssat| SignedAmount::from_sat(ssat).unwrap(); + + kani::assume(ssat(n1).checked_add(ssat(n2)).is_some()); // Adding amounts doesn't overflow. + kani::assume(ssat(n1).checked_sub(ssat(n2)).is_some()); // Subbing amounts doesn't overflow. + + assert_eq!(ssat(n1) + ssat(n2), ssat(n1 + n2).into()); + assert_eq!(ssat(n1) - ssat(n2), ssat(n1 - n2).into()); } diff --git a/units/src/fee.rs b/units/src/fee.rs index 39dff63b0..569d6d662 100644 --- a/units/src/fee.rs +++ b/units/src/fee.rs @@ -28,11 +28,12 @@ impl Amount { /// # Examples /// /// ``` - /// # use bitcoin_units::{Amount, FeeRate, Weight}; - /// let amount = Amount::from_sat(10); + /// # use bitcoin_units::{amount, Amount, FeeRate, Weight}; + /// let amount = Amount::from_sat(10)?; /// let weight = Weight::from_wu(300); /// let fee_rate = amount.checked_div_by_weight_ceil(weight).expect("Division by weight failed"); /// assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(34)); + /// # Ok::<_, amount::OutOfRangeError>(()) /// ``` #[must_use] pub const fn checked_div_by_weight_ceil(self, weight: Weight) -> Option { @@ -151,7 +152,10 @@ impl FeeRate { // No `?` operator in const context. match self.to_sat_per_kwu().checked_mul(weight.to_wu()) { Some(mul_res) => match mul_res.checked_add(999) { - Some(add_res) => Some(Amount::from_sat(add_res / 1000)), + Some(add_res) => match Amount::from_sat(add_res / 1000) { + Ok(fee) => Some(fee), + Err(_) => None, + }, None => None, }, None => None,