From 938461cc65c09a5237cc9b7245c4f503ec3977fe Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 31 Jan 2025 09:01:56 +1100 Subject: [PATCH 01/10] psbt: Use Amount::ZERO in unit test We have a const for this, use it. Internal change only. --- bitcoin/src/psbt/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index f10cbbb32..b2787d73e 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -2283,7 +2283,7 @@ mod tests { version: transaction::Version::TWO, lock_time: absolute::LockTime::ZERO, input: vec![TxIn::EMPTY_COINBASE, TxIn::EMPTY_COINBASE], - output: vec![TxOut { value: Amount::from_sat(0), script_pubkey: ScriptBuf::new() }], + output: vec![TxOut { value: Amount::ZERO, script_pubkey: ScriptBuf::new() }], }; let mut psbt = Psbt::from_unsigned_tx(unsigned_tx).unwrap(); From 8ecdc7c27512ea0ae91c955441c32d7074d5cd98 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 28 Feb 2025 10:09:41 +1100 Subject: [PATCH 02/10] Use den_ prefix for local Denomination variable Throughout the `amount::tests` module we use `sat` and `ssat` as aliases to amount constructors but in on test we use them as `Denomination` variables. To assist clarity and so we can introduce uniform usage of the constructor aliases change the variable names to use the `den_` prefix. Internal change only, no logic changes. --- units/src/amount/tests.rs | 57 ++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index d263623eb..748cd3876 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -339,53 +339,62 @@ fn floating_point() { #[allow(clippy::inconsistent_digit_grouping)] // Group to show 100,000,000 sats per bitcoin. fn parsing() { use super::ParseAmountError as E; - let btc = Denomination::Bitcoin; - let sat = Denomination::Satoshi; + let den_btc = Denomination::Bitcoin; + let den_sat = Denomination::Satoshi; let p = Amount::from_str_in; let sp = SignedAmount::from_str_in; - assert_eq!(p("x", btc), Err(E::from(InvalidCharacterError { invalid_char: 'x', position: 0 }))); assert_eq!( - p("-", btc), + p("x", den_btc), + Err(E::from(InvalidCharacterError { invalid_char: 'x', position: 0 })) + ); + assert_eq!( + p("-", den_btc), Err(E::from(MissingDigitsError { kind: MissingDigitsKind::OnlyMinusSign })) ); assert_eq!( - sp("-", btc), + sp("-", den_btc), Err(E::from(MissingDigitsError { kind: MissingDigitsKind::OnlyMinusSign })) ); assert_eq!( - p("-1.0x", btc), + p("-1.0x", den_btc), Err(E::from(InvalidCharacterError { invalid_char: 'x', position: 4 })) ); assert_eq!( - p("0.0 ", btc), + p("0.0 ", den_btc), Err(E::from(InvalidCharacterError { invalid_char: ' ', position: 3 })) ); assert_eq!( - p("0.000.000", btc), + p("0.000.000", den_btc), Err(E::from(InvalidCharacterError { invalid_char: '.', position: 5 })) ); #[cfg(feature = "alloc")] let more_than_max = format!("{}", Amount::MAX.to_sat() + 1); #[cfg(feature = "alloc")] - assert_eq!(p(&more_than_max, btc), Err(OutOfRangeError::too_big(false).into())); - assert_eq!(p("0.000000042", btc), Err(TooPreciseError { position: 10 }.into())); - assert_eq!(p("1.0000000", sat), Ok(Amount::from_sat_unchecked(1))); - assert_eq!(p("1.1", sat), Err(TooPreciseError { position: 2 }.into())); - assert_eq!(p("1000.1", sat), Err(TooPreciseError { position: 5 }.into())); - assert_eq!(p("1001.0000000", sat), Ok(Amount::from_sat_unchecked(1001))); - assert_eq!(p("1000.0000001", sat), Err(TooPreciseError { position: 11 }.into())); + assert_eq!(p(&more_than_max, den_btc), Err(OutOfRangeError::too_big(false).into())); + assert_eq!(p("0.000000042", den_btc), Err(TooPreciseError { position: 10 }.into())); + assert_eq!(p("1.0000000", den_sat), Ok(Amount::from_sat_unchecked(1))); + assert_eq!(p("1.1", den_sat), Err(TooPreciseError { position: 2 }.into())); + assert_eq!(p("1000.1", den_sat), Err(TooPreciseError { position: 5 }.into())); + assert_eq!(p("1001.0000000", den_sat), Ok(Amount::from_sat_unchecked(1001))); + assert_eq!(p("1000.0000001", den_sat), Err(TooPreciseError { position: 11 }.into())); - assert_eq!(p("1", btc), Ok(Amount::from_sat_unchecked(1_000_000_00))); - assert_eq!(sp("-.5", btc), Ok(SignedAmount::from_sat_unchecked(-500_000_00))); + assert_eq!(p("1", den_btc), Ok(Amount::from_sat_unchecked(1_000_000_00))); + assert_eq!(sp("-.5", den_btc), Ok(SignedAmount::from_sat_unchecked(-500_000_00))); #[cfg(feature = "alloc")] - assert_eq!(sp(&SignedAmount::MIN.to_sat().to_string(), sat), Ok(SignedAmount::MIN)); - assert_eq!(p("1.1", btc), Ok(Amount::from_sat_unchecked(1_100_000_00))); - assert_eq!(p("100", sat), Ok(Amount::from_sat_unchecked(100))); - assert_eq!(p("55", sat), Ok(Amount::from_sat_unchecked(55))); - assert_eq!(p("2100000000000000", sat), Ok(Amount::from_sat_unchecked(21_000_000__000_000_00))); - assert_eq!(p("2100000000000000.", sat), Ok(Amount::from_sat_unchecked(21_000_000__000_000_00))); - assert_eq!(p("21000000", btc), Ok(Amount::from_sat_unchecked(21_000_000__000_000_00))); + assert_eq!(sp(&SignedAmount::MIN.to_sat().to_string(), den_sat), Ok(SignedAmount::MIN)); + assert_eq!(p("1.1", den_btc), Ok(Amount::from_sat_unchecked(1_100_000_00))); + assert_eq!(p("100", den_sat), Ok(Amount::from_sat_unchecked(100))); + assert_eq!(p("55", den_sat), Ok(Amount::from_sat_unchecked(55))); + assert_eq!( + p("2100000000000000", den_sat), + Ok(Amount::from_sat_unchecked(21_000_000__000_000_00)) + ); + assert_eq!( + p("2100000000000000.", den_sat), + Ok(Amount::from_sat_unchecked(21_000_000__000_000_00)) + ); + assert_eq!(p("21000000", den_btc), Ok(Amount::from_sat_unchecked(21_000_000__000_000_00))); // exactly 50 chars. assert_eq!( From ef0af8d62e28dad70cbe2f847769d4f839b3077e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 8 Mar 2025 15:15:13 +0000 Subject: [PATCH 03/10] Use sat/ssat constructors throughout tests There is an as yet unresolved discussion about the unchecked amount constructor. In an effort to focus the amount of changes required later and also to make the `tests` module uniform use the `sat` and `ssat` constructor functions everywhere. Internal change only, no logic changes. --- units/src/amount/tests.rs | 152 +++++++++++--------------------------- 1 file changed, 44 insertions(+), 108 deletions(-) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 748cd3876..af41eaf62 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -16,11 +16,11 @@ 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) } + #[test] fn sanity_check() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - 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); @@ -102,25 +102,22 @@ fn from_str_zero_without_denomination() { #[test] fn from_int_btc() { let amt = Amount::from_int_btc_const(2); - assert_eq!(Amount::from_sat_unchecked(200_000_000), amt); + assert_eq!(sat(200_000_000), amt); } #[test] fn amount_try_from_signed_amount() { - let sa_positive = SignedAmount::from_sat_unchecked(123); + let sa_positive = ssat(123); let ua_positive = Amount::try_from(sa_positive).unwrap(); - assert_eq!(ua_positive, Amount::from_sat_unchecked(123)); + assert_eq!(ua_positive, sat(123)); - let sa_negative = SignedAmount::from_sat_unchecked(-123); + let sa_negative = ssat(-123); let result = Amount::try_from(sa_negative); assert_eq!(result, Err(OutOfRangeError { is_signed: false, is_greater_than_max: false })); } #[test] fn mul_div() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - let op_result_sat = |sat| NumOpResult::Valid(Amount::from_sat(sat)); let op_result_ssat = |sat| NumOpResult::Valid(SignedAmount::from_sat(sat)); @@ -141,17 +138,14 @@ fn neg() { #[cfg(feature = "std")] #[test] fn overflows() { - let result = Amount::MAX + Amount::from_sat_unchecked(1); + let result = Amount::MAX + sat(1); assert!(result.is_error()); - let result = Amount::from_sat_unchecked(8_446_744_073_709_551_615) * 3; + let result = sat(8_446_744_073_709_551_615) * 3; assert!(result.is_error()); } #[test] fn add() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - assert!(sat(0) + sat(0) == sat(0).into()); assert!(sat(127) + sat(179) == sat(306).into()); @@ -164,9 +158,6 @@ fn add() { #[test] fn sub() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - assert!(sat(0) - sat(0) == sat(0).into()); assert!(sat(179) - sat(127) == sat(52).into()); assert!((sat(127) - sat(179)).is_error()); @@ -180,9 +171,6 @@ fn sub() { #[test] fn checked_arithmetic() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - assert_eq!(SignedAmount::MAX.checked_add(ssat(1)), None); assert_eq!(SignedAmount::MIN.checked_sub(ssat(1)), None); assert_eq!(Amount::MAX.checked_add(sat(1)), None); @@ -195,9 +183,6 @@ fn checked_arithmetic() { #[test] #[allow(deprecated_in_future)] fn unchecked_arithmetic() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - assert_eq!(ssat(10).unchecked_add(ssat(20)), ssat(30)); assert_eq!(ssat(50).unchecked_sub(ssat(10)), ssat(40)); assert_eq!(sat(5).unchecked_add(sat(7)), sat(12)); @@ -206,8 +191,6 @@ fn unchecked_arithmetic() { #[test] fn positive_sub() { - let ssat = SignedAmount::from_sat; - assert_eq!(ssat(10).positive_sub(ssat(7)).unwrap(), ssat(3)); assert!(ssat(-10).positive_sub(ssat(7)).is_none()); assert!(ssat(10).positive_sub(ssat(-7)).is_none()); @@ -218,12 +201,12 @@ fn positive_sub() { #[test] fn amount_checked_div_by_weight_ceil() { let weight = Weight::from_kwu(1).unwrap(); - let fee_rate = Amount::from_sat_unchecked(1).checked_div_by_weight_ceil(weight).unwrap(); + let fee_rate = sat(1).checked_div_by_weight_ceil(weight).unwrap(); // 1 sats / 1,000 wu = 1 sats/kwu assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(1)); let weight = Weight::from_wu(381); - let fee_rate = Amount::from_sat_unchecked(329).checked_div_by_weight_ceil(weight).unwrap(); + let fee_rate = sat(329).checked_div_by_weight_ceil(weight).unwrap(); // 329 sats / 381 wu = 863.5 sats/kwu // round up to 864 assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(864)); @@ -236,12 +219,12 @@ fn amount_checked_div_by_weight_ceil() { #[test] fn amount_checked_div_by_weight_floor() { let weight = Weight::from_kwu(1).unwrap(); - let fee_rate = Amount::from_sat_unchecked(1).checked_div_by_weight_floor(weight).unwrap(); + let fee_rate = sat(1).checked_div_by_weight_floor(weight).unwrap(); // 1 sats / 1,000 wu = 1 sats/kwu assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(1)); let weight = Weight::from_wu(381); - let fee_rate = Amount::from_sat_unchecked(329).checked_div_by_weight_floor(weight).unwrap(); + let fee_rate = sat(329).checked_div_by_weight_floor(weight).unwrap(); // 329 sats / 381 wu = 863.5 sats/kwu // round down to 863 assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(863)); @@ -253,7 +236,7 @@ fn amount_checked_div_by_weight_floor() { #[cfg(feature = "alloc")] #[test] fn amount_checked_div_by_fee_rate() { - let amount = Amount::from_sat_unchecked(1000); + let amount = sat(1000); let fee_rate = FeeRate::from_sat_per_kwu(2); // Test floor division @@ -266,7 +249,7 @@ fn amount_checked_div_by_fee_rate() { assert_eq!(weight, Weight::from_wu(500_000)); // Same result for exact division // Test truncation behavior - let amount = Amount::from_sat_unchecked(1000); + let amount = sat(1000); let fee_rate = FeeRate::from_sat_per_kwu(3); let floor_weight = amount.checked_div_by_fee_rate_floor(fee_rate).unwrap(); let ceil_weight = amount.checked_div_by_fee_rate_ceil(fee_rate).unwrap(); @@ -287,7 +270,7 @@ fn amount_checked_div_by_fee_rate() { // Test overflow case let tiny_fee_rate = FeeRate::from_sat_per_kwu(1); - let large_amount = Amount::from_sat(u64::MAX); + 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()); } @@ -298,8 +281,6 @@ fn floating_point() { use super::Denomination as D; let f = Amount::from_float_in; let sf = SignedAmount::from_float_in; - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; assert_eq!(f(11.22, D::Bitcoin), Ok(sat(1_122_000_000))); assert_eq!(sf(-11.22, D::MilliBitcoin), Ok(ssat(-1_122_000))); @@ -339,6 +320,7 @@ fn floating_point() { #[allow(clippy::inconsistent_digit_grouping)] // Group to show 100,000,000 sats per bitcoin. fn parsing() { use super::ParseAmountError as E; + let den_btc = Denomination::Bitcoin; let den_sat = Denomination::Satoshi; let p = Amount::from_str_in; @@ -373,28 +355,28 @@ fn parsing() { #[cfg(feature = "alloc")] assert_eq!(p(&more_than_max, den_btc), Err(OutOfRangeError::too_big(false).into())); assert_eq!(p("0.000000042", den_btc), Err(TooPreciseError { position: 10 }.into())); - assert_eq!(p("1.0000000", den_sat), Ok(Amount::from_sat_unchecked(1))); + assert_eq!(p("1.0000000", den_sat), Ok(sat(1))); assert_eq!(p("1.1", den_sat), Err(TooPreciseError { position: 2 }.into())); assert_eq!(p("1000.1", den_sat), Err(TooPreciseError { position: 5 }.into())); - assert_eq!(p("1001.0000000", den_sat), Ok(Amount::from_sat_unchecked(1001))); + assert_eq!(p("1001.0000000", den_sat), Ok(sat(1001))); assert_eq!(p("1000.0000001", den_sat), Err(TooPreciseError { position: 11 }.into())); - assert_eq!(p("1", den_btc), Ok(Amount::from_sat_unchecked(1_000_000_00))); - assert_eq!(sp("-.5", den_btc), Ok(SignedAmount::from_sat_unchecked(-500_000_00))); + assert_eq!(p("1", den_btc), Ok(sat(1_000_000_00))); + assert_eq!(sp("-.5", den_btc), Ok(ssat(-500_000_00))); #[cfg(feature = "alloc")] assert_eq!(sp(&SignedAmount::MIN.to_sat().to_string(), den_sat), Ok(SignedAmount::MIN)); - assert_eq!(p("1.1", den_btc), Ok(Amount::from_sat_unchecked(1_100_000_00))); - assert_eq!(p("100", den_sat), Ok(Amount::from_sat_unchecked(100))); - assert_eq!(p("55", den_sat), Ok(Amount::from_sat_unchecked(55))); + assert_eq!(p("1.1", den_btc), Ok(sat(1_100_000_00))); + assert_eq!(p("100", den_sat), Ok(sat(100))); + assert_eq!(p("55", den_sat), Ok(sat(55))); assert_eq!( p("2100000000000000", den_sat), - Ok(Amount::from_sat_unchecked(21_000_000__000_000_00)) + Ok(sat(21_000_000__000_000_00)) ); assert_eq!( p("2100000000000000.", den_sat), - Ok(Amount::from_sat_unchecked(21_000_000__000_000_00)) + Ok(sat(21_000_000__000_000_00)) ); - assert_eq!(p("21000000", den_btc), Ok(Amount::from_sat_unchecked(21_000_000__000_000_00))); + assert_eq!(p("21000000", den_btc), Ok(sat(21_000_000__000_000_00))); // exactly 50 chars. assert_eq!( @@ -417,15 +399,12 @@ fn to_string() { assert_eq!(format!("{:.8}", Amount::ONE_BTC.display_in(D::Bitcoin)), "1.00000000"); assert_eq!(Amount::ONE_BTC.to_string_in(D::Satoshi), "100000000"); assert_eq!(Amount::ONE_SAT.to_string_in(D::Bitcoin), "0.00000001"); - assert_eq!(SignedAmount::from_sat_unchecked(-42).to_string_in(D::Bitcoin), "-0.00000042"); + assert_eq!(ssat(-42).to_string_in(D::Bitcoin), "-0.00000042"); assert_eq!(Amount::ONE_BTC.to_string_with_denomination(D::Bitcoin), "1 BTC"); assert_eq!(SignedAmount::ONE_BTC.to_string_with_denomination(D::Satoshi), "100000000 satoshi"); assert_eq!(Amount::ONE_SAT.to_string_with_denomination(D::Bitcoin), "0.00000001 BTC"); - assert_eq!( - SignedAmount::from_sat_unchecked(-42).to_string_with_denomination(D::Bitcoin), - "-0.00000042 BTC" - ); + assert_eq!(ssat(-42).to_string_with_denomination(D::Bitcoin), "-0.00000042 BTC"); } // May help identify a problem sooner @@ -602,8 +581,6 @@ check_format_non_negative_show_denom! { #[test] fn unsigned_signed_conversion() { - let ssat = SignedAmount::from_sat; - let sat = Amount::from_sat; let max_sats: u64 = Amount::MAX.to_sat(); assert_eq!(sat(max_sats).to_signed(), ssat(max_sats as i64)); @@ -681,12 +658,12 @@ fn from_str() { case("21000001 BTC", Err(OutOfRangeError::too_big(false))); case("18446744073709551616 sat", Err(OutOfRangeError::too_big(false))); - ok_case(".5 bits", Amount::from_sat_unchecked(50)); - ok_scase("-.5 bits", SignedAmount::from_sat_unchecked(-50)); - ok_case("0.00253583 BTC", Amount::from_sat_unchecked(253_583)); - ok_scase("-5 satoshi", SignedAmount::from_sat_unchecked(-5)); - ok_case("0.10000000 BTC", Amount::from_sat_unchecked(100_000_00)); - ok_scase("-100 bits", SignedAmount::from_sat_unchecked(-10_000)); + ok_case(".5 bits", sat(50)); + ok_scase("-.5 bits", ssat(-50)); + ok_case("0.00253583 BTC", sat(253_583)); + ok_scase("-5 satoshi", ssat(-5)); + ok_case("0.10000000 BTC", sat(100_000_00)); + ok_scase("-100 bits", ssat(-10_000)); ok_case("21000000 BTC", Amount::MAX); ok_scase("21000000 BTC", SignedAmount::MAX); ok_scase("-21000000 BTC", SignedAmount::MIN); @@ -776,7 +753,7 @@ fn to_string_with_denomination_from_str_roundtrip() { use super::Denomination as D; - let amt = Amount::from_sat_unchecked(42); + let amt = sat(42); let denom = Amount::to_string_with_denomination; assert_eq!(denom(amt, D::Bitcoin).parse::(), Ok(amt)); assert_eq!(denom(amt, D::CentiBitcoin).parse::(), Ok(amt)); @@ -807,10 +784,7 @@ fn serde_as_sat() { } serde_test::assert_tokens( - &T { - amt: Amount::from_sat_unchecked(123_456_789), - samt: SignedAmount::from_sat_unchecked(-123_456_789), - }, + &T { amt: sat(123_456_789), samt: ssat(-123_456_789) }, &[ serde_test::Token::Struct { name: "T", len: 2 }, serde_test::Token::Str("amt"), @@ -837,10 +811,7 @@ fn serde_as_btc() { pub samt: SignedAmount, } - let orig = T { - amt: Amount::from_sat_unchecked(20_000_000__000_000_01), - samt: SignedAmount::from_sat_unchecked(-20_000_000__000_000_01), - }; + let orig = T { amt: sat(20_000_000__000_000_01), samt: ssat(-20_000_000__000_000_01) }; let json = "{\"amt\": 20000000.00000001, \ \"samt\": -20000000.00000001}"; @@ -874,10 +845,7 @@ fn serde_as_str() { } serde_test::assert_tokens( - &T { - amt: Amount::from_sat_unchecked(123_456_789), - samt: SignedAmount::from_sat_unchecked(-123_456_789), - }, + &T { amt: sat(123_456_789), samt: ssat(-123_456_789) }, &[ serde_test::Token::Struct { name: "T", len: 2 }, serde_test::Token::String("amt"), @@ -904,10 +872,7 @@ fn serde_as_btc_opt() { pub samt: Option, } - let with = T { - amt: Some(Amount::from_sat_unchecked(2_500_000_00)), - samt: Some(SignedAmount::from_sat_unchecked(-2_500_000_00)), - }; + let with = T { amt: Some(sat(2_500_000_00)), samt: Some(ssat(-2_500_000_00)) }; let without = T { amt: None, samt: None }; // Test Roundtripping @@ -946,10 +911,7 @@ fn serde_as_sat_opt() { pub samt: Option, } - let with = T { - amt: Some(Amount::from_sat_unchecked(2_500_000_00)), - samt: Some(SignedAmount::from_sat_unchecked(-2_500_000_00)), - }; + let with = T { amt: Some(sat(2_500_000_00)), samt: Some(ssat(-2_500_000_00)) }; let without = T { amt: None, samt: None }; // Test Roundtripping @@ -988,10 +950,7 @@ fn serde_as_str_opt() { pub samt: Option, } - let with = T { - amt: Some(Amount::from_sat_unchecked(123_456_789)), - samt: Some(SignedAmount::from_sat_unchecked(-123_456_789)), - }; + let with = T { amt: Some(sat(123_456_789)), samt: Some(ssat(-123_456_789)) }; let without = T { amt: None, samt: None }; // Test Roundtripping @@ -1018,9 +977,6 @@ fn serde_as_str_opt() { #[test] fn sum_amounts() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - assert_eq!([].iter().sum::>(), Amount::ZERO.into()); assert_eq!([].iter().sum::>(), SignedAmount::ZERO.into()); @@ -1053,9 +1009,6 @@ fn sum_amounts() { #[test] fn checked_sum_amounts() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - assert_eq!([].into_iter().checked_sum(), Some(Amount::ZERO)); assert_eq!([].into_iter().checked_sum(), Some(SignedAmount::ZERO)); @@ -1121,8 +1074,6 @@ fn disallow_unknown_denomination() { #[test] #[cfg(feature = "alloc")] fn trailing_zeros_for_amount() { - let sat = Amount::from_sat; - assert_eq!(format!("{}", sat(1_000_000)), "0.01 BTC"); assert_eq!(format!("{}", Amount::ONE_SAT), "0.00000001 BTC"); assert_eq!(format!("{}", Amount::ONE_BTC), "1 BTC"); @@ -1203,8 +1154,6 @@ fn add_sub_combos() { #[test] fn unsigned_addition() { - let sat = Amount::from_sat; - assert_eq!(sat(0) + sat(0), NumOpResult::from(sat(0))); assert_eq!(sat(0) + sat(307), NumOpResult::from(sat(307))); assert_eq!(sat(307) + sat(0), NumOpResult::from(sat(307))); @@ -1214,8 +1163,6 @@ fn unsigned_addition() { #[test] fn signed_addition() { - let ssat = SignedAmount::from_sat; - assert_eq!(ssat(0) + ssat(0), NumOpResult::from(ssat(0))); assert_eq!(ssat(0) + ssat(307), NumOpResult::from(ssat(307))); assert_eq!(ssat(307) + ssat(0), NumOpResult::from(ssat(307))); @@ -1235,8 +1182,6 @@ fn signed_addition() { #[test] fn unsigned_subtraction() { - let sat = Amount::from_sat; - assert_eq!(sat(0) - sat(0), NumOpResult::from(sat(0))); assert_eq!(sat(307) - sat(0), NumOpResult::from(sat(307))); assert_eq!(sat(461) - sat(307), NumOpResult::from(sat(154))); @@ -1244,8 +1189,6 @@ fn unsigned_subtraction() { #[test] fn signed_subtraction() { - let ssat = SignedAmount::from_sat; - assert_eq!(ssat(0) - ssat(0), NumOpResult::from(ssat(0))); assert_eq!(ssat(0) - ssat(307), NumOpResult::from(ssat(-307))); assert_eq!(ssat(307) - ssat(0), NumOpResult::from(ssat(307))); @@ -1261,11 +1204,8 @@ fn signed_subtraction() { #[test] fn op_int_combos() { - let sat = Amount::from_sat; - let ssat = SignedAmount::from_sat; - - let res = |sat| NumOpResult::from(Amount::from_sat(sat)); - let sres = |ssat| NumOpResult::from(SignedAmount::from_sat(ssat)); + let res = |n_sat| NumOpResult::from(sat(n_sat)); + let sres = |n_ssat| NumOpResult::from(ssat(n_ssat)); assert_eq!(sat(23) * 31, res(713)); assert_eq!(ssat(23) * 31, sres(713)); @@ -1304,8 +1244,6 @@ fn op_int_combos() { #[test] fn unsigned_amount_div_by_amount() { - let sat = Amount::from_sat; - assert_eq!(sat(0) / sat(7), 0); assert_eq!(sat(1897) / sat(7), 271); } @@ -1318,8 +1256,6 @@ fn unsigned_amount_div_by_amount_zero() { #[test] fn signed_amount_div_by_amount() { - let ssat = SignedAmount::from_sat; - assert_eq!(ssat(0) / ssat(7), 0); assert_eq!(ssat(1897) / ssat(7), 271); From e6f7b26d80a8f2680ca4732013d89e029bcb2abc Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 28 Feb 2025 09:41:10 +1100 Subject: [PATCH 04/10] Use _unchecked in amount const types We are about to start enforcing the MAX_MONEY invariant. Doing so will change constructors to return an error type. In preparation use the `_unchecked` constructor for all the consts. Internal change only, no logic changes. --- units/src/amount/signed.rs | 12 ++++++------ units/src/amount/unsigned.rs | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 7551ac197..d4c2d0227 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -50,17 +50,17 @@ pub struct SignedAmount(i64); impl SignedAmount { /// The zero amount. - pub const ZERO: Self = SignedAmount(0); + pub const ZERO: Self = SignedAmount::from_sat_unchecked(0); /// Exactly one satoshi. - pub const ONE_SAT: Self = SignedAmount(1); + pub const ONE_SAT: Self = SignedAmount::from_sat_unchecked(1); /// Exactly one bitcoin. - pub const ONE_BTC: Self = SignedAmount(100_000_000); + pub const ONE_BTC: Self = SignedAmount::from_sat_unchecked(100_000_000); /// Exactly fifty bitcoin. - pub const FIFTY_BTC: Self = Self::from_sat_unchecked(50 * 100_000_000); + pub const FIFTY_BTC: Self = SignedAmount::from_sat_unchecked(50 * 100_000_000); /// The maximum value allowed as an amount. Useful for sanity checking. - pub const MAX_MONEY: Self = SignedAmount(21_000_000 * 100_000_000); + pub const MAX_MONEY: Self = SignedAmount::from_sat_unchecked(21_000_000 * 100_000_000); /// The minimum value of an amount. - pub const MIN: Self = SignedAmount(-21_000_000 * 100_000_000); + pub const MIN: Self = SignedAmount::from_sat_unchecked(-21_000_000 * 100_000_000); /// The maximum value of an amount. pub const MAX: Self = SignedAmount::MAX_MONEY; diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 89d7eb35f..71ee58de4 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -42,15 +42,15 @@ pub struct Amount(u64); impl Amount { /// The zero amount. - pub const ZERO: Self = Amount(0); + pub const ZERO: Self = Amount::from_sat_unchecked(0); /// Exactly one satoshi. - pub const ONE_SAT: Self = Amount(1); + pub const ONE_SAT: Self = Amount::from_sat_unchecked(1); /// Exactly one bitcoin. - pub const ONE_BTC: Self = Self::from_int_btc_const(1); + pub const ONE_BTC: Self = Amount::from_sat_unchecked(100_000_000); /// Exactly fifty bitcoin. - pub const FIFTY_BTC: Self = Self::from_sat_unchecked(50 * 100_000_000); + pub const FIFTY_BTC: Self = Amount::from_sat_unchecked(50 * 100_000_000); /// The maximum value allowed as an amount. Useful for sanity checking. - pub const MAX_MONEY: Self = Self::from_int_btc_const(21_000_000); + pub const MAX_MONEY: Self = Amount::from_sat_unchecked(21_000_000 * 100_000_000); /// The minimum value of an amount. pub const MIN: Self = Amount::ZERO; /// The maximum value of an amount. From 6d70c77cf90a373041c93ac6e3a746f85a51b900 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 28 Feb 2025 09:54:50 +1100 Subject: [PATCH 05/10] Enforce newtype sanity rules for amount types The unchecked-should-be-unsafe conversation is out of scope for this patch. We want to bite off small chunks so the constructors are left as they currently are - we are just doing the encapsulation here. This is in preparation for enforcing the MAX_MONEY invariant which is not currently enforced. As per the sanity rules policy outline in: https://github.com/rust-bitcoin/rust-bitcoin/discussions/4090 For both amount types create a private `encapsulate` module that consists of exactly the type and a single constructor and a single getter. --- units/src/amount/signed.rs | 156 +++++++++++++++++++---------------- units/src/amount/unsigned.rs | 126 ++++++++++++++++------------ 2 files changed, 156 insertions(+), 126 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index d4c2d0227..5e5b4c5a2 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -16,37 +16,60 @@ use super::{ DisplayStyle, OutOfRangeError, ParseAmountError, ParseError, }; -/// A signed amount. -/// -/// The [`SignedAmount`] type can be used to express Bitcoin amounts that support arithmetic and -/// conversion to various denominations. The [`SignedAmount`] type does not implement [`serde`] -/// traits but we do provide modules for serializing as satoshis or bitcoin. -/// -/// Warning! -/// -/// This type implements several arithmetic operations from [`core::ops`]. -/// To prevent errors due to an overflow when using these operations, -/// it is advised to instead use the checked arithmetic methods whose names -/// start with `checked_`. The operations from [`core::ops`] that [`SignedAmount`] -/// implements will panic when an overflow occurs. -/// -/// # Examples -/// -/// ``` -/// # #[cfg(feature = "serde")] { -/// use serde::{Serialize, Deserialize}; -/// use bitcoin_units::SignedAmount; -/// -/// #[derive(Serialize, Deserialize)] -/// struct Foo { -/// // If you are using `rust-bitcoin` then `bitcoin::amount::serde::as_sat` also works. -/// #[serde(with = "bitcoin_units::amount::serde::as_sat")] // Also `serde::as_btc`. -/// amount: SignedAmount, -/// } -/// # } -/// ``` -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct SignedAmount(i64); +mod encapsulate { + /// A signed amount. + /// + /// The [`SignedAmount`] type can be used to express Bitcoin amounts that support arithmetic and + /// conversion to various denominations. The [`SignedAmount`] type does not implement [`serde`] + /// traits but we do provide modules for serializing as satoshis or bitcoin. + /// + /// Warning! + /// + /// This type implements several arithmetic operations from [`core::ops`]. + /// To prevent errors due to an overflow when using these operations, + /// it is advised to instead use the checked arithmetic methods whose names + /// start with `checked_`. The operations from [`core::ops`] that [`SignedAmount`] + /// implements will panic when an overflow occurs. + /// + /// # Examples + /// + /// ``` + /// # #[cfg(feature = "serde")] { + /// use serde::{Serialize, Deserialize}; + /// use bitcoin_units::SignedAmount; + /// + /// #[derive(Serialize, Deserialize)] + /// struct Foo { + /// // If you are using `rust-bitcoin` then `bitcoin::amount::serde::as_sat` also works. + /// #[serde(with = "bitcoin_units::amount::serde::as_sat")] // Also `serde::as_btc`. + /// amount: SignedAmount, + /// } + /// # } + /// ``` + #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct SignedAmount(i64); + + impl SignedAmount { + /// Constructs a new [`SignedAmount`] with satoshi precision and the given number of satoshis. + /// + /// Caller to guarantee that `satoshi` is within valid range. + /// + /// See [`Self::MIN`] and [`Self::MAX_MONEY`]. + pub const fn from_sat_unchecked(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) } + + /// Gets the number of satoshis in this [`SignedAmount`]. + /// + /// # Examples + /// + /// ``` + /// # use bitcoin_units::SignedAmount; + /// assert_eq!(SignedAmount::ONE_BTC.to_sat(), 100_000_000); + /// ``` + pub const fn to_sat(self) -> i64 { self.0 } + } +} +#[doc(inline)] +pub use encapsulate::SignedAmount; impl SignedAmount { /// The zero amount. @@ -73,24 +96,9 @@ impl SignedAmount { /// let amount = SignedAmount::from_sat(-100_000); /// assert_eq!(amount.to_sat(), -100_000); /// ``` - pub const fn from_sat(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) } - - /// Gets the number of satoshis in this [`SignedAmount`]. - /// - /// # Examples - /// - /// ``` - /// # use bitcoin_units::SignedAmount; - /// assert_eq!(SignedAmount::ONE_BTC.to_sat(), 100_000_000); - /// ``` - pub const fn to_sat(self) -> i64 { self.0 } - - /// Constructs a new [`SignedAmount`] with satoshi precision and the given number of satoshis. - /// - /// Caller to guarantee that `satoshi` is within valid range. - /// - /// See [`Self::MIN`] and [`Self::MAX_MONEY`]. - pub const fn from_sat_unchecked(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) } + pub const fn from_sat(satoshi: i64) -> SignedAmount { + SignedAmount::from_sat_unchecked(satoshi) + } /// Converts from a value expressing a decimal number of bitcoin to a [`SignedAmount`]. /// @@ -153,11 +161,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(sat as i64)), // Cast ok, value in this arm does not wrap. + (false, sat) => Ok(SignedAmount::from_sat(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(-(sat as i64))), // Cast ok, value in this arm does not wrap. + (true, sat) => Ok(SignedAmount::from_sat(-(sat as i64))), // Cast ok, value in this arm does not wrap. } } @@ -300,11 +308,11 @@ impl SignedAmount { /// Gets the absolute value of this [`SignedAmount`]. #[must_use] - pub fn abs(self) -> SignedAmount { SignedAmount(self.0.abs()) } + pub fn abs(self) -> SignedAmount { SignedAmount::from_sat(self.to_sat().abs()) } /// Gets the absolute value of this [`SignedAmount`] returning [`Amount`]. #[must_use] - pub fn unsigned_abs(self) -> Amount { Amount::from_sat(self.0.unsigned_abs()) } + pub fn unsigned_abs(self) -> Amount { Amount::from_sat(self.to_sat().unsigned_abs()) } /// Returns a number representing sign of this [`SignedAmount`]. /// @@ -312,19 +320,19 @@ impl SignedAmount { /// - `1` if the amount is positive /// - `-1` if the amount is negative #[must_use] - pub fn signum(self) -> i64 { self.0.signum() } + pub fn signum(self) -> i64 { self.to_sat().signum() } /// Checks if this [`SignedAmount`] is positive. /// /// Returns `true` if this [`SignedAmount`] is positive and `false` if /// this [`SignedAmount`] is zero or negative. - pub fn is_positive(self) -> bool { self.0.is_positive() } + pub fn is_positive(self) -> bool { self.to_sat().is_positive() } /// Checks if this [`SignedAmount`] is negative. /// /// Returns `true` if this [`SignedAmount`] is negative and `false` if /// this [`SignedAmount`] is zero or positive. - pub fn is_negative(self) -> bool { self.0.is_negative() } + pub fn is_negative(self) -> bool { self.to_sat().is_negative() } /// Returns the absolute value of this [`SignedAmount`]. /// @@ -334,8 +342,8 @@ impl SignedAmount { #[must_use] pub const fn checked_abs(self) -> Option { // No `map()` in const context. - match self.0.checked_abs() { - Some(res) => Some(SignedAmount(res)), + match self.to_sat().checked_abs() { + Some(res) => Some(SignedAmount::from_sat(res)), None => None, } } @@ -346,8 +354,8 @@ impl SignedAmount { #[must_use] pub const fn checked_add(self, rhs: SignedAmount) -> Option { // No `map()` in const context. - match self.0.checked_add(rhs.0) { - Some(res) => SignedAmount(res).check_min_max(), + match self.to_sat().checked_add(rhs.to_sat()) { + Some(res) => SignedAmount::from_sat(res).check_min_max(), None => None, } } @@ -359,8 +367,8 @@ impl SignedAmount { #[must_use] pub const fn checked_sub(self, rhs: SignedAmount) -> Option { // No `map()` in const context. - match self.0.checked_sub(rhs.0) { - Some(res) => SignedAmount(res).check_min_max(), + match self.to_sat().checked_sub(rhs.to_sat()) { + Some(res) => SignedAmount::from_sat(res).check_min_max(), None => None, } } @@ -372,8 +380,8 @@ impl SignedAmount { #[must_use] pub const fn checked_mul(self, rhs: i64) -> Option { // No `map()` in const context. - match self.0.checked_mul(rhs) { - Some(res) => SignedAmount(res).check_min_max(), + match self.to_sat().checked_mul(rhs) { + Some(res) => SignedAmount::from_sat(res).check_min_max(), None => None, } } @@ -386,8 +394,8 @@ impl SignedAmount { #[must_use] pub const fn checked_div(self, rhs: i64) -> Option { // No `map()` in const context. - match self.0.checked_div(rhs) { - Some(res) => Some(SignedAmount(res)), + match self.to_sat().checked_div(rhs) { + Some(res) => Some(SignedAmount::from_sat(res)), None => None, } } @@ -398,8 +406,8 @@ impl SignedAmount { #[must_use] pub const fn checked_rem(self, rhs: i64) -> Option { // No `map()` in const context. - match self.0.checked_rem(rhs) { - Some(res) => Some(SignedAmount(res)), + match self.to_sat().checked_rem(rhs) { + Some(res) => Some(SignedAmount::from_sat(res)), None => None, } } @@ -413,7 +421,9 @@ impl SignedAmount { /// On overflow, panics in debug mode, wraps in release mode. #[must_use] #[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] - pub fn unchecked_add(self, rhs: SignedAmount) -> SignedAmount { Self(self.0 + rhs.0) } + pub fn unchecked_add(self, rhs: SignedAmount) -> SignedAmount { + Self::from_sat(self.to_sat() + rhs.to_sat()) + } /// Unchecked subtraction. /// @@ -424,7 +434,9 @@ impl SignedAmount { /// On overflow, panics in debug mode, wraps in release mode. #[must_use] #[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] - pub fn unchecked_sub(self, rhs: SignedAmount) -> SignedAmount { Self(self.0 - rhs.0) } + pub fn unchecked_sub(self, rhs: SignedAmount) -> SignedAmount { + Self::from_sat(self.to_sat() - rhs.to_sat()) + } /// Subtraction that doesn't allow negative [`SignedAmount`]s. /// @@ -453,7 +465,7 @@ impl SignedAmount { /// Checks the amount is within the allowed range. const fn check_min_max(self) -> Option { - if self.0 < Self::MIN.0 || self.0 > Self::MAX.0 { + if self.to_sat() < Self::MIN.to_sat() || self.to_sat() > Self::MAX.to_sat() { None } else { Some(self) @@ -514,6 +526,6 @@ impl From for SignedAmount { impl<'a> Arbitrary<'a> for SignedAmount { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { let s = i64::arbitrary(u)?; - Ok(Self(s)) + Ok(Self::from_sat(s)) } } diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 71ee58de4..ae9c3f5c2 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -16,29 +16,58 @@ use super::{ OutOfRangeError, ParseAmountError, ParseError, SignedAmount, }; -/// An amount. -/// -/// The [`Amount`] type can be used to express Bitcoin amounts that support arithmetic and -/// conversion to various denominations. The [`Amount`] type does not implement [`serde`] traits -/// but we do provide modules for serializing as satoshis or bitcoin. -/// -/// # Examples -/// -/// ``` -/// # #[cfg(feature = "serde")] { -/// use serde::{Serialize, Deserialize}; -/// use bitcoin_units::Amount; -/// -/// #[derive(Serialize, Deserialize)] -/// struct Foo { -/// // If you are using `rust-bitcoin` then `bitcoin::amount::serde::as_sat` also works. -/// #[serde(with = "bitcoin_units::amount::serde::as_sat")] // Also `serde::as_btc`. -/// amount: Amount, -/// } -/// # } -/// ``` -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct Amount(u64); +mod encapsulate { + /// An amount. + /// + /// The [`Amount`] type can be used to express Bitcoin amounts that support arithmetic and + /// conversion to various denominations. The [`Amount`] type does not implement [`serde`] traits + /// but we do provide modules for serializing as satoshis or bitcoin. + /// + /// Warning! + /// + /// This type implements several arithmetic operations from [`core::ops`]. + /// To prevent errors due to an overflow when using these operations, + /// it is advised to instead use the checked arithmetic methods whose names + /// start with `checked_`. The operations from [`core::ops`] that [`Amount`] + /// implements will panic when an overflow occurs. + /// + /// # Examples + /// + /// ``` + /// # #[cfg(feature = "serde")] { + /// use serde::{Serialize, Deserialize}; + /// use bitcoin_units::Amount; + /// + /// #[derive(Serialize, Deserialize)] + /// struct Foo { + /// // If you are using `rust-bitcoin` then `bitcoin::amount::serde::as_sat` also works. + /// #[serde(with = "bitcoin_units::amount::serde::as_sat")] // Also `serde::as_btc`. + /// amount: Amount, + /// } + /// # } + /// ``` + #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct Amount(u64); + + impl Amount { + /// Constructs a new [`Amount`] with satoshi precision and the given number of satoshis. + /// + /// Caller to guarantee that `satoshi` is within valid range. See [`Self::MAX_MONEY`]. + pub const fn from_sat_unchecked(satoshi: u64) -> Amount { Self(satoshi) } + + /// Gets the number of satoshis in this [`Amount`]. + /// + /// # Examples + /// + /// ``` + /// # use bitcoin_units::Amount; + /// assert_eq!(Amount::ONE_BTC.to_sat(), 100_000_000); + /// ``` + pub const fn to_sat(self) -> u64 { self.0 } + } +} +#[doc(inline)] +pub use encapsulate::Amount; impl Amount { /// The zero amount. @@ -67,22 +96,7 @@ impl Amount { /// let amount = Amount::from_sat(100_000); /// assert_eq!(amount.to_sat(), 100_000); /// ``` - pub const fn from_sat(satoshi: u64) -> Amount { Amount(satoshi) } - - /// Gets the number of satoshis in this [`Amount`]. - /// - /// # Examples - /// - /// ``` - /// # use bitcoin_units::Amount; - /// assert_eq!(Amount::ONE_BTC.to_sat(), 100_000_000); - /// ``` - pub const fn to_sat(self) -> u64 { self.0 } - - /// Constructs a new [`Amount`] with satoshi precision and the given number of satoshis. - /// - /// Caller to guarantee that `satoshi` is within valid range. See [`Self::MAX_MONEY`]. - pub const fn from_sat_unchecked(satoshi: u64) -> Amount { Self(satoshi) } + pub const fn from_sat(satoshi: u64) -> Amount { Amount::from_sat_unchecked(satoshi) } /// Converts from a value expressing a decimal number of bitcoin to an [`Amount`]. /// @@ -148,7 +162,7 @@ impl Amount { OutOfRangeError::negative(), ))); } - if sats > Self::MAX.0 { + if sats > Self::MAX.to_sat() { return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange( OutOfRangeError::too_big(false), ))); @@ -299,8 +313,8 @@ impl Amount { #[must_use] pub const fn checked_add(self, rhs: Amount) -> Option { // No `map()` in const context. - match self.0.checked_add(rhs.0) { - Some(res) => Amount(res).check_max(), + match self.to_sat().checked_add(rhs.to_sat()) { + Some(res) => Amount::from_sat(res).check_max(), None => None, } } @@ -311,8 +325,8 @@ impl Amount { #[must_use] pub const fn checked_sub(self, rhs: Amount) -> Option { // No `map()` in const context. - match self.0.checked_sub(rhs.0) { - Some(res) => Some(Amount(res)), + match self.to_sat().checked_sub(rhs.to_sat()) { + Some(res) => Some(Amount::from_sat(res)), None => None, } } @@ -323,8 +337,8 @@ impl Amount { #[must_use] pub const fn checked_mul(self, rhs: u64) -> Option { // No `map()` in const context. - match self.0.checked_mul(rhs) { - Some(res) => Amount(res).check_max(), + match self.to_sat().checked_mul(rhs) { + Some(res) => Amount::from_sat(res).check_max(), None => None, } } @@ -337,8 +351,8 @@ impl Amount { #[must_use] pub const fn checked_div(self, rhs: u64) -> Option { // No `map()` in const context. - match self.0.checked_div(rhs) { - Some(res) => Some(Amount(res)), + match self.to_sat().checked_div(rhs) { + Some(res) => Some(Amount::from_sat(res)), None => None, } } @@ -349,8 +363,8 @@ impl Amount { #[must_use] pub const fn checked_rem(self, rhs: u64) -> Option { // No `map()` in const context. - match self.0.checked_rem(rhs) { - Some(res) => Some(Amount(res)), + match self.to_sat().checked_rem(rhs) { + Some(res) => Some(Amount::from_sat(res)), None => None, } } @@ -364,7 +378,9 @@ impl Amount { /// On overflow, panics in debug mode, wraps in release mode. #[must_use] #[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] - pub fn unchecked_add(self, rhs: Amount) -> Amount { Self(self.0 + rhs.0) } + pub fn unchecked_add(self, rhs: Amount) -> Amount { + Self::from_sat(self.to_sat() + rhs.to_sat()) + } /// Unchecked subtraction. /// @@ -375,7 +391,9 @@ impl Amount { /// On overflow, panics in debug mode, wraps in release mode. #[must_use] #[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] - pub fn unchecked_sub(self, rhs: Amount) -> Amount { Self(self.0 - rhs.0) } + pub fn unchecked_sub(self, rhs: Amount) -> Amount { + Self::from_sat(self.to_sat() - rhs.to_sat()) + } /// Converts to a signed amount. #[rustfmt::skip] // Moves code comments to the wrong line. @@ -385,7 +403,7 @@ impl Amount { /// Checks if the amount is below the maximum value. Returns `None` if it is above. const fn check_max(self) -> Option { - if self.0 > Self::MAX.0 { + if self.to_sat() > Self::MAX.to_sat() { None } else { Some(self) @@ -445,6 +463,6 @@ impl TryFrom for Amount { impl<'a> Arbitrary<'a> for Amount { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { let a = u64::arbitrary(u)?; - Ok(Self(a)) + Ok(Self::from_sat(a)) } } From ac7168020274f2240aa6376b3aa12efc11c0aa57 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 28 Feb 2025 10:21:15 +1100 Subject: [PATCH 06/10] Pick one - MAX or MAX_MONEY Just use MAX everywhere in this codebase. After discussion in PR consensus was to just use MAX throughout the codebase. ref: https://github.com/rust-bitcoin/rust-bitcoin/pull/4164#discussion_r1979441438 --- bitcoin/tests/serde.rs | 2 +- units/src/amount/signed.rs | 2 +- units/src/amount/tests.rs | 31 ++++++++++++++++++++++--------- units/src/amount/unsigned.rs | 2 +- units/tests/str.rs | 4 ++-- 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/bitcoin/tests/serde.rs b/bitcoin/tests/serde.rs index 807f838a6..dff93b0c5 100644 --- a/bitcoin/tests/serde.rs +++ b/bitcoin/tests/serde.rs @@ -110,7 +110,7 @@ fn serde_regression_txin() { #[test] fn serde_regression_txout() { let txout = - TxOut { value: Amount::MAX_MONEY, script_pubkey: ScriptBuf::from(vec![0u8, 1u8, 2u8]) }; + TxOut { value: Amount::MAX, script_pubkey: ScriptBuf::from(vec![0u8, 1u8, 2u8]) }; let got = serialize(&txout).unwrap(); let want = include_bytes!("data/serde/txout_bincode") as &[_]; assert_eq!(got, want) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 5e5b4c5a2..bdabfefde 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -54,7 +54,7 @@ mod encapsulate { /// /// Caller to guarantee that `satoshi` is within valid range. /// - /// See [`Self::MIN`] and [`Self::MAX_MONEY`]. + /// See [`Self::MIN`] and [`Self::MAX`]. pub const fn from_sat_unchecked(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) } /// Gets the number of satoshis in this [`SignedAmount`]. diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index af41eaf62..a692ca650 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -725,15 +725,28 @@ fn to_from_string_in() { assert_eq!(ua_str(&ua_sat(21_000_000).to_string_in(D::Bit), D::Bit), Ok(ua_sat(21_000_000))); assert_eq!(ua_str(&ua_sat(0).to_string_in(D::Satoshi), D::Satoshi), Ok(ua_sat(0))); - assert!(ua_str(&ua_sat(Amount::MAX.to_sat()).to_string_in(D::Bitcoin), D::Bitcoin).is_ok()); - assert!(ua_str(&ua_sat(Amount::MAX.to_sat()).to_string_in(D::CentiBitcoin), D::CentiBitcoin) - .is_ok()); - assert!(ua_str(&ua_sat(Amount::MAX.to_sat()).to_string_in(D::MilliBitcoin), D::MilliBitcoin) - .is_ok()); - assert!(ua_str(&ua_sat(Amount::MAX.to_sat()).to_string_in(D::MicroBitcoin), D::MicroBitcoin) - .is_ok()); + assert!( + ua_str(&ua_sat(Amount::MAX.to_sat()).to_string_in(D::Bitcoin), D::Bitcoin).is_ok() + ); + assert!(ua_str( + &ua_sat(Amount::MAX.to_sat()).to_string_in(D::CentiBitcoin), + D::CentiBitcoin + ) + .is_ok()); + assert!(ua_str( + &ua_sat(Amount::MAX.to_sat()).to_string_in(D::MilliBitcoin), + D::MilliBitcoin + ) + .is_ok()); + assert!(ua_str( + &ua_sat(Amount::MAX.to_sat()).to_string_in(D::MicroBitcoin), + D::MicroBitcoin + ) + .is_ok()); assert!(ua_str(&ua_sat(Amount::MAX.to_sat()).to_string_in(D::Bit), D::Bit).is_ok()); - assert!(ua_str(&ua_sat(Amount::MAX.to_sat()).to_string_in(D::Satoshi), D::Satoshi).is_ok()); + assert!( + ua_str(&ua_sat(Amount::MAX.to_sat()).to_string_in(D::Satoshi), D::Satoshi).is_ok() + ); assert_eq!( sa_str(&SignedAmount::MAX.to_string_in(D::Satoshi), D::MicroBitcoin), @@ -1276,7 +1289,7 @@ fn check_const() { assert_eq!(Amount::ONE_BTC.to_sat(), 100_000_000); assert_eq!(SignedAmount::FIFTY_BTC.to_sat(), SignedAmount::ONE_BTC.to_sat() * 50); assert_eq!(Amount::FIFTY_BTC.to_sat(), Amount::ONE_BTC.to_sat() * 50); - assert_eq!(Amount::MAX_MONEY.to_sat() as i64, SignedAmount::MAX_MONEY.to_sat()); + assert_eq!(Amount::MAX.to_sat() as i64, SignedAmount::MAX.to_sat()); } // Sanity check than stdlib supports the set of reference combinations for the ops we want. diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index ae9c3f5c2..27435fba0 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -52,7 +52,7 @@ mod encapsulate { impl Amount { /// Constructs a new [`Amount`] with satoshi precision and the given number of satoshis. /// - /// Caller to guarantee that `satoshi` is within valid range. See [`Self::MAX_MONEY`]. + /// Caller to guarantee that `satoshi` is within valid range. See [`Self::MAX`]. pub const fn from_sat_unchecked(satoshi: u64) -> Amount { Self(satoshi) } /// Gets the number of satoshis in this [`Amount`]. diff --git a/units/tests/str.rs b/units/tests/str.rs index 89068bf9b..0b365af20 100644 --- a/units/tests/str.rs +++ b/units/tests/str.rs @@ -22,10 +22,10 @@ macro_rules! check { check! { amount_unsigned_one_sat, Amount, Amount::ONE_SAT, "0.00000001 BTC"; - amount_unsigned_max_money, Amount, Amount::MAX_MONEY, "21000000 BTC"; + amount_unsigned_max_money, Amount, Amount::MAX, "21000000 BTC"; amount_signed_one_sat, SignedAmount, SignedAmount::ONE_SAT, "0.00000001 BTC"; - amount_signed_max_money, SignedAmount, SignedAmount::MAX_MONEY, "21000000 BTC"; + amount_signed_max_money, SignedAmount, SignedAmount::MAX, "21000000 BTC"; block_height_min, BlockHeight, BlockHeight::MIN, "0"; block_height_max, BlockHeight, BlockHeight::MAX, "4294967295"; From 13595fbe7d5ddcab0492445169297e009b6d6cc7 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 28 Feb 2025 10:45:34 +1100 Subject: [PATCH 07/10] Fix amount whole bitcoin constructors I royally botched the recent effort to make const amount constructors use a smaller type. I left in an unnecessary panic and forgot to do both of them. Note these function return values will change again very shortly when we start enforcing the MAX_MONEY invariant. However the 64 to 32 bit change is unrelated to that and is easier to review if done separately. Whole bitcoin can not in any sane environment be greater than 21,000,000 which fits in 32 bits so we can take a 32 bit integer in the whole bitcoin constructors without loss of utility. Doing so removes the potential panic. This is a breaking API change. We elect not to deprecate because we want to keep the same function names. --- units/src/amount/signed.rs | 26 ++++++++------------------ units/src/amount/unsigned.rs | 23 ++++++----------------- 2 files changed, 14 insertions(+), 35 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index bdabfefde..df9aa18f3 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -121,29 +121,19 @@ impl SignedAmount { } /// Converts from a value expressing a whole number of bitcoin to a [`SignedAmount`]. - /// - /// # Errors - /// - /// The function errors if the argument multiplied by the number of sats - /// per bitcoin overflows an `i64` type. - pub fn from_int_btc>(whole_bitcoin: T) -> Result { - match whole_bitcoin.into().checked_mul(100_000_000) { - Some(amount) => Ok(Self::from_sat(amount)), - None => Err(OutOfRangeError { is_signed: true, is_greater_than_max: true }), - } + #[allow(clippy::missing_panics_doc)] + pub fn from_int_btc>(whole_bitcoin: T) -> SignedAmount { + SignedAmount::from_int_btc_const(whole_bitcoin.into()) } /// Converts from a value expressing a whole number of bitcoin to a [`SignedAmount`] /// in const context. - /// - /// # Panics - /// - /// The function panics if the argument multiplied by the number of sats - /// per bitcoin overflows an `i64` type. - pub const fn from_int_btc_const(whole_bitcoin: i64) -> SignedAmount { - match whole_bitcoin.checked_mul(100_000_000) { + #[allow(clippy::missing_panics_doc)] + pub const fn from_int_btc_const(whole_bitcoin: i32) -> SignedAmount { + 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), - None => panic!("checked_mul overflowed"), + None => panic!("cannot overflow in i64"), } } diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 27435fba0..167ad85f3 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -119,30 +119,19 @@ impl Amount { } /// Converts from a value expressing a whole number of bitcoin to an [`Amount`]. - /// - /// # Errors - /// - /// The function errors if the argument multiplied by the number of sats - /// per bitcoin overflows a `u64` type. - pub fn from_int_btc>(whole_bitcoin: T) -> Result { - match whole_bitcoin.into().checked_mul(100_000_000) { - Some(amount) => Ok(Self::from_sat(amount)), - None => Err(OutOfRangeError { is_signed: false, is_greater_than_max: true }), - } + #[allow(clippy::missing_panics_doc)] + pub fn from_int_btc>(whole_bitcoin: T) -> Amount { + Amount::from_int_btc_const(whole_bitcoin.into()) } /// Converts from a value expressing a whole number of bitcoin to an [`Amount`] /// in const context. - /// - /// # Panics - /// - /// The function panics if the argument multiplied by the number of sats - /// per bitcoin overflows a `u64` type. + #[allow(clippy::missing_panics_doc)] pub const fn from_int_btc_const(whole_bitcoin: u32) -> Amount { - let btc = whole_bitcoin as u64; // Can't call u64::from in const context. + 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), - None => panic!("checked_mul overflowed"), + None => panic!("cannot overflow a u64"), } } From f9eb3079537ebc6d4a43121f7de839ba5b5018dc Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 28 Feb 2025 12:04:18 +1100 Subject: [PATCH 08/10] Remove panic in dust value functions Calculating the minimum non-dust fee currently panics if either the script is really big or the dust fee rate is really big. Harden the API by returning an `Option` instead of panicing. --- bitcoin/src/blockdata/script/borrowed.rs | 13 ++++++------- bitcoin/src/blockdata/script/tests.rs | 8 ++++---- bitcoin/src/blockdata/transaction.rs | 8 ++++---- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs index 624feb3b5..4ae5c9ce1 100644 --- a/bitcoin/src/blockdata/script/borrowed.rs +++ b/bitcoin/src/blockdata/script/borrowed.rs @@ -261,7 +261,7 @@ crate::internal_macros::define_extension_trait! { /// Returns the minimum value an output with this script should have in order to be /// broadcastable on today’s Bitcoin network. #[deprecated(since = "0.32.0", note = "use `minimal_non_dust` etc. instead")] - fn dust_value(&self) -> Amount { self.minimal_non_dust() } + fn dust_value(&self) -> Option { self.minimal_non_dust() } /// Returns the minimum value an output with this script should have in order to be /// broadcastable on today's Bitcoin network. @@ -272,7 +272,7 @@ crate::internal_macros::define_extension_trait! { /// To use a custom value, use [`minimal_non_dust_custom`]. /// /// [`minimal_non_dust_custom`]: Script::minimal_non_dust_custom - fn minimal_non_dust(&self) -> Amount { + fn minimal_non_dust(&self) -> Option { self.minimal_non_dust_internal(DUST_RELAY_TX_FEE.into()) } @@ -287,7 +287,7 @@ crate::internal_macros::define_extension_trait! { /// To use the default Bitcoin Core value, use [`minimal_non_dust`]. /// /// [`minimal_non_dust`]: Script::minimal_non_dust - fn minimal_non_dust_custom(&self, dust_relay_fee: FeeRate) -> Amount { + fn minimal_non_dust_custom(&self, dust_relay_fee: FeeRate) -> Option { self.minimal_non_dust_internal(dust_relay_fee.to_sat_per_kwu() * 4) } @@ -394,7 +394,7 @@ mod sealed { crate::internal_macros::define_extension_trait! { pub(crate) trait ScriptExtPriv impl for Script { - fn minimal_non_dust_internal(&self, dust_relay_fee: u64) -> Amount { + fn minimal_non_dust_internal(&self, dust_relay_fee: u64) -> Option { // This must never be lower than Bitcoin Core's GetDustThreshold() (as of v0.21) as it may // otherwise allow users to create transactions which likely can never be broadcast/confirmed. let sats = dust_relay_fee @@ -408,13 +408,12 @@ crate::internal_macros::define_extension_trait! { 32 + 4 + 1 + 107 + 4 + // The spend cost copied from Core 8 + // The serialized size of the TxOut's amount field self.consensus_encode(&mut sink()).expect("sinks don't error").to_u64() // The serialized size of this script_pubkey - }) - .expect("dust_relay_fee or script length should not be absurdly large") + })? / 1000; // divide by 1000 like in Core to get value as it cancels out DEFAULT_MIN_RELAY_TX_FEE // 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. - Amount::from_sat(sats) + Some(Amount::from_sat(sats)) } fn count_sigops_internal(&self, accurate: bool) -> usize { diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs index 9b47ef652..4443645b1 100644 --- a/bitcoin/src/blockdata/script/tests.rs +++ b/bitcoin/src/blockdata/script/tests.rs @@ -684,10 +684,10 @@ fn default_dust_value() { // well-known scriptPubKey types. let script_p2wpkh = Builder::new().push_int_unchecked(0).push_slice([42; 20]).into_script(); assert!(script_p2wpkh.is_p2wpkh()); - assert_eq!(script_p2wpkh.minimal_non_dust(), Amount::from_sat_unchecked(294)); + assert_eq!(script_p2wpkh.minimal_non_dust(), Some(Amount::from_sat_unchecked(294))); assert_eq!( script_p2wpkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)), - Amount::from_sat_unchecked(588) + Some(Amount::from_sat_unchecked(588)) ); let script_p2pkh = Builder::new() @@ -698,10 +698,10 @@ fn default_dust_value() { .push_opcode(OP_CHECKSIG) .into_script(); assert!(script_p2pkh.is_p2pkh()); - assert_eq!(script_p2pkh.minimal_non_dust(), Amount::from_sat_unchecked(546)); + assert_eq!(script_p2pkh.minimal_non_dust(), Some(Amount::from_sat_unchecked(546))); assert_eq!( script_p2pkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)), - Amount::from_sat_unchecked(1092) + Some(Amount::from_sat_unchecked(1092)) ); } diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 4510c465d..0c60d3c65 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -183,8 +183,8 @@ crate::internal_macros::define_extension_trait! { /// To use a custom value, use [`minimal_non_dust_custom`]. /// /// [`minimal_non_dust_custom`]: TxOut::minimal_non_dust_custom - fn minimal_non_dust(script_pubkey: ScriptBuf) -> Self { - TxOut { value: script_pubkey.minimal_non_dust(), script_pubkey } + fn minimal_non_dust(script_pubkey: ScriptBuf) -> Option { + Some(TxOut { value: script_pubkey.minimal_non_dust()?, script_pubkey }) } /// Constructs a new `TxOut` with given script and the smallest possible `value` that is **not** dust @@ -198,8 +198,8 @@ crate::internal_macros::define_extension_trait! { /// To use the default Bitcoin Core value, use [`minimal_non_dust`]. /// /// [`minimal_non_dust`]: TxOut::minimal_non_dust - fn minimal_non_dust_custom(script_pubkey: ScriptBuf, dust_relay_fee: FeeRate) -> Self { - TxOut { value: script_pubkey.minimal_non_dust_custom(dust_relay_fee), script_pubkey } + fn minimal_non_dust_custom(script_pubkey: ScriptBuf, dust_relay_fee: FeeRate) -> Option { + Some(TxOut { value: script_pubkey.minimal_non_dust_custom(dust_relay_fee)?, script_pubkey }) } } } From 76a2d70b2866c9a9e2e24c1d71a9630a57559737 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 28 Feb 2025 12:17:57 +1100 Subject: [PATCH 09/10] Make mul weight by fee return NumOpResult Now that we have the `NumOpResult` type that is used to show a math calculation returned a valid amount we can use it when multiplying weight and fee rates thus removing panics. --- units/src/amount/mod.rs | 1 + units/src/amount/result.rs | 5 ++--- units/src/fee.rs | 13 ++++++------- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/units/src/amount/mod.rs b/units/src/amount/mod.rs index 7fb8bc535..940b9a5a8 100644 --- a/units/src/amount/mod.rs +++ b/units/src/amount/mod.rs @@ -39,6 +39,7 @@ pub use self::{ signed::SignedAmount, unsigned::Amount, }; +pub(crate) use self::result::OptionExt; /// A set of denominations in which amounts can be expressed. /// diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs index ae1c6e12d..4b78d0080 100644 --- a/units/src/amount/result.rs +++ b/units/src/amount/result.rs @@ -1,7 +1,6 @@ // SPDX-License-Identifier: CC0-1.0 -//! Provides a monodic numeric result type that is used to return the result of -//! doing mathematical operations (`core::ops`) on amount types. +//! Provides a monodic type used when mathematical operations (`core::ops`) return an amount type. use core::{fmt, ops}; @@ -378,7 +377,7 @@ impl<'a> core::iter::Sum<&'a NumOpResult> for NumOpResult { +pub(crate) trait OptionExt { fn valid_or_error(self) -> NumOpResult; } diff --git a/units/src/fee.rs b/units/src/fee.rs index 1bb3b9b66..39dff63b0 100644 --- a/units/src/fee.rs +++ b/units/src/fee.rs @@ -13,6 +13,7 @@ use core::ops; +use crate::amount::{NumOpResult, OptionExt}; use crate::{Amount, FeeRate, Weight}; impl Amount { @@ -160,17 +161,15 @@ impl FeeRate { /// Computes the ceiling so that the fee computation is conservative. impl ops::Mul for Weight { - type Output = Amount; + type Output = NumOpResult; - fn mul(self, rhs: FeeRate) -> Self::Output { - Amount::from_sat((rhs.to_sat_per_kwu() * self.to_wu() + 999) / 1000) - } + fn mul(self, rhs: FeeRate) -> Self::Output { rhs.checked_mul_by_weight(self).valid_or_error() } } impl ops::Mul for FeeRate { - type Output = Amount; + type Output = NumOpResult; - fn mul(self, rhs: Weight) -> Self::Output { rhs * self } + fn mul(self, rhs: Weight) -> Self::Output { self.checked_mul_by_weight(rhs).valid_or_error() } } impl ops::Div for Amount { @@ -265,7 +264,7 @@ mod tests { let three = Weight::from_vb(3).unwrap(); let six = Amount::from_sat_unchecked(6); - assert_eq!(two * three, six); + assert_eq!(two * three, six.into()); } #[test] From 5d851f1c3e98d7d426e5897b2d734b77a299ccfb Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 28 Feb 2025 13:02:38 +1100 Subject: [PATCH 10/10] Remove deprecated amount methods When we enforce the MAX_MONEY invariant these functions would require the function signature changing - might as well just delete them. --- units/src/amount/signed.rs | 26 -------------------------- units/src/amount/tests.rs | 9 --------- units/src/amount/unsigned.rs | 26 -------------------------- 3 files changed, 61 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index df9aa18f3..a04795de6 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -402,32 +402,6 @@ impl SignedAmount { } } - /// Unchecked addition. - /// - /// Computes `self + rhs`. - /// - /// # Panics - /// - /// On overflow, panics in debug mode, wraps in release mode. - #[must_use] - #[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] - pub fn unchecked_add(self, rhs: SignedAmount) -> SignedAmount { - Self::from_sat(self.to_sat() + rhs.to_sat()) - } - - /// Unchecked subtraction. - /// - /// Computes `self - rhs`. - /// - /// # Panics - /// - /// On overflow, panics in debug mode, wraps in release mode. - #[must_use] - #[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] - pub fn unchecked_sub(self, rhs: SignedAmount) -> SignedAmount { - Self::from_sat(self.to_sat() - rhs.to_sat()) - } - /// Subtraction that doesn't allow negative [`SignedAmount`]s. /// /// Returns [`None`] if either `self`, `rhs` or the result is strictly negative. diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index a692ca650..2d3b5e1c6 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -180,15 +180,6 @@ fn checked_arithmetic() { assert_eq!(ssat(-6).checked_div(2), Some(ssat(-3))); } -#[test] -#[allow(deprecated_in_future)] -fn unchecked_arithmetic() { - assert_eq!(ssat(10).unchecked_add(ssat(20)), ssat(30)); - assert_eq!(ssat(50).unchecked_sub(ssat(10)), ssat(40)); - assert_eq!(sat(5).unchecked_add(sat(7)), sat(12)); - assert_eq!(sat(10).unchecked_sub(sat(7)), sat(3)); -} - #[test] fn positive_sub() { assert_eq!(ssat(10).positive_sub(ssat(7)).unwrap(), ssat(3)); diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 167ad85f3..c3488dfaf 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -358,32 +358,6 @@ impl Amount { } } - /// Unchecked addition. - /// - /// Computes `self + rhs`. - /// - /// # Panics - /// - /// On overflow, panics in debug mode, wraps in release mode. - #[must_use] - #[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] - pub fn unchecked_add(self, rhs: Amount) -> Amount { - Self::from_sat(self.to_sat() + rhs.to_sat()) - } - - /// Unchecked subtraction. - /// - /// Computes `self - rhs`. - /// - /// # Panics - /// - /// On overflow, panics in debug mode, wraps in release mode. - #[must_use] - #[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] - pub fn unchecked_sub(self, rhs: Amount) -> Amount { - Self::from_sat(self.to_sat() - rhs.to_sat()) - } - /// Converts to a signed amount. #[rustfmt::skip] // Moves code comments to the wrong line. pub fn to_signed(self) -> SignedAmount {