From 8bb9ce3e472993f5820d9d66337343e5f956b937 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 3 Mar 2025 05:20:01 +1100 Subject: [PATCH 1/6] Add tests for amount op int We aim to support three ops on amount types that use an integer for the right hand size. Prove that implement them. --- units/src/amount/tests.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index d3463660f..e021448bc 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -1289,6 +1289,15 @@ fn num_op_result_ops_integer() { } } check_op! { + // Operations on an amount type and an integer. + let _ = sat * 3_u64; // Explicit type for the benefit of the reader. + let _ = sat / 3; + let _ = sat % 3; + + let _ = ssat * 3_i64; // Explicit type for the benefit of the reader. + let _ = ssat / 3; + let _ = ssat % 3; + // Operations on a `NumOpResult` and integer. let _ = res * 3_u64; // Explicit type for the benefit of the reader. let _ = res / 3; From 47923957b19c04ad8db131785adffe44bc542ebe Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 3 Mar 2025 07:02:30 +1100 Subject: [PATCH 2/6] Improve add/sub tests for amount types Add a few macros to test `Add` and `Sub` impls for both amount types, all combos of type and res (eg `Amount` and `NumOpResult`), and all combos of references. --- units/src/amount/tests.rs | 96 ++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 51 deletions(-) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index e021448bc..f4b101023 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -1127,64 +1127,58 @@ fn trailing_zeros_for_amount() { } #[test] -#[allow(clippy::op_ref)] -fn unsigned_addition() { - let sat = Amount::from_sat; +fn add_sub_combos() { + // Checks lhs op rhs for all reference combos. + macro_rules! check_ref { + ($($lhs:ident $op:tt $rhs:ident = $ans:ident);* $(;)?) => { + $( + assert_eq!($lhs $op $rhs, $ans); + assert_eq!(&$lhs $op $rhs, $ans); + assert_eq!($lhs $op &$rhs, $ans); + assert_eq!(&$lhs $op &$rhs, $ans); + )* + } + } - let one = sat(1); - let two = sat(2); - let three = sat(3); + // Checks lhs op rhs for all amount and `NumOpResult` combos. + macro_rules! check_res { + ($($amount:ident, $op:tt, $lhs:literal, $rhs:literal, $ans:literal);* $(;)?) => { + $( + let amt = |sat| $amount::from_sat(sat); - assert!((one + two) == three.into()); - assert!((one + two) == three.into()); - assert!((&one + two) == three.into()); - assert!((one + &two) == three.into()); - assert!((&one + &two) == three.into()); -} + let sat_lhs = amt($lhs); + let sat_rhs = amt($rhs); -#[test] -#[allow(clippy::op_ref)] -fn unsigned_subtract() { - let sat = Amount::from_sat; + let res_lhs = NumOpResult::from(sat_lhs); + let res_rhs = NumOpResult::from(sat_rhs); - let one = sat(1); - let two = sat(2); - let three = sat(3); + let ans = NumOpResult::from(amt($ans)); - assert!(three - two == one.into()); - assert!(&three - two == one.into()); - assert!(three - &two == one.into()); - assert!(&three - &two == one.into()); -} + check_ref! { + sat_lhs $op sat_rhs = ans; + sat_lhs $op res_rhs = ans; + res_lhs $op sat_rhs = ans; + res_lhs $op res_rhs = ans; + } + )* + } + } -#[test] -#[allow(clippy::op_ref)] -fn signed_addition() { - let ssat = SignedAmount::from_sat; + // Checks lhs op rhs for both amount types. + macro_rules! check_op { + ($($lhs:literal $op:tt $rhs:literal = $ans:literal);* $(;)?) => { + $( + check_res!(Amount, $op, $lhs, $rhs, $ans); + check_res!(SignedAmount, $op, $lhs, $rhs, $ans); + )* + } + } - let one = ssat(1); - let two = ssat(2); - let three = ssat(3); - - assert!(one + two == three.into()); - assert!(&one + two == three.into()); - assert!(one + &two == three.into()); - assert!(&one + &two == three.into()); -} - -#[test] -#[allow(clippy::op_ref)] -fn signed_subtract() { - let ssat = SignedAmount::from_sat; - - let one = ssat(1); - let two = ssat(2); - let three = ssat(3); - - assert!(three - two == one.into()); - assert!(&three - two == one.into()); - assert!(three - &two == one.into()); - assert!(&three - &two == one.into()); + // We do not currently support division involving `NumOpResult` and an amount type. + check_op! { + 307 + 461 = 768; + 461 - 307 = 154; + } } #[test] From 851080d3b19eecea147224f8a36f9788d3e7d328 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 3 Mar 2025 07:26:16 +1100 Subject: [PATCH 3/6] Add more add/sub tests Add units tests for various values including negative values. --- units/src/amount/tests.rs | 55 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index f4b101023..fc2dd2f59 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -1181,6 +1181,61 @@ 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))); + assert_eq!(sat(307) + sat(461), NumOpResult::from(sat(768))); + assert_eq!(sat(0) + Amount::MAX_MONEY, NumOpResult::from(Amount::MAX_MONEY)); +} + +#[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))); + assert_eq!(ssat(307) + ssat(461), NumOpResult::from(ssat(768))); + assert_eq!(ssat(0) + SignedAmount::MAX_MONEY, NumOpResult::from(SignedAmount::MAX_MONEY)); + + assert_eq!(ssat(0) + ssat(-307), NumOpResult::from(ssat(-307))); + assert_eq!(ssat(-307) + ssat(0), NumOpResult::from(ssat(-307))); + assert_eq!(ssat(-307) + ssat(461), NumOpResult::from(ssat(154))); + assert_eq!(ssat(307) + ssat(-461), NumOpResult::from(ssat(-154))); + assert_eq!(ssat(-307) + ssat(-461), NumOpResult::from(ssat(-768))); + assert_eq!(SignedAmount::MAX_MONEY + -SignedAmount::MAX_MONEY, NumOpResult::from(SignedAmount::ZERO)); +} + +#[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))); +} + +#[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))); + assert_eq!(ssat(307) - ssat(461), NumOpResult::from(ssat(-154))); + assert_eq!(ssat(0) - SignedAmount::MAX_MONEY, NumOpResult::from(-SignedAmount::MAX_MONEY)); + + assert_eq!(ssat(0) - ssat(-307), NumOpResult::from(ssat(307))); + assert_eq!(ssat(-307) - ssat(0), NumOpResult::from(ssat(-307))); + assert_eq!(ssat(-307) - ssat(461), NumOpResult::from(ssat(-768))); + assert_eq!(ssat(307) - ssat(-461), NumOpResult::from(ssat(768))); + assert_eq!(ssat(-307) - ssat(-461), NumOpResult::from(ssat(154))); +} + #[test] fn check_const() { assert_eq!(SignedAmount::ONE_BTC.to_sat(), 100_000_000); From 501c9ab89e6cee63b4ba8e493393bfc1c544a225 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 3 Mar 2025 07:47:33 +1100 Subject: [PATCH 4/6] Test amount ops that involve an integer From the amount types `Div`, `Mul`, and `Rem` can have an integer on one side. Test them - includes commented out test for one missing combo. --- units/src/amount/tests.rs | 46 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index fc2dd2f59..3ab69af1d 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -1207,7 +1207,10 @@ fn signed_addition() { assert_eq!(ssat(-307) + ssat(461), NumOpResult::from(ssat(154))); assert_eq!(ssat(307) + ssat(-461), NumOpResult::from(ssat(-154))); assert_eq!(ssat(-307) + ssat(-461), NumOpResult::from(ssat(-768))); - assert_eq!(SignedAmount::MAX_MONEY + -SignedAmount::MAX_MONEY, NumOpResult::from(SignedAmount::ZERO)); + assert_eq!( + SignedAmount::MAX_MONEY + -SignedAmount::MAX_MONEY, + NumOpResult::from(SignedAmount::ZERO) + ); } #[test] @@ -1236,6 +1239,47 @@ fn signed_subtraction() { assert_eq!(ssat(-307) - ssat(-461), NumOpResult::from(ssat(154))); } +#[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)); + + assert_eq!(sat(23) * 31, res(713)); + assert_eq!(ssat(23) * 31, sres(713)); + assert_eq!(res(23) * 31, res(713)); + assert_eq!(sres(23) * 31, sres(713)); + + // assert_eq!(31 * sat(23), res(713)); + // assert_eq!(31 * ssat(23), sres(713)); + + // No remainder. + assert_eq!(sat(1897) / 7, res(271)); + assert_eq!(ssat(1897) / 7, sres(271)); + assert_eq!(res(1897) / 7, res(271)); + assert_eq!(sres(1897) / 7, sres(271)); + + // Truncation works as expected. + assert_eq!(sat(1901) / 7, res(271)); + assert_eq!(ssat(1901) / 7, sres(271)); + assert_eq!(res(1901) / 7, res(271)); + assert_eq!(sres(1901) / 7, sres(271)); + + // No remainder. + assert_eq!(sat(1897) % 7, res(0)); + assert_eq!(ssat(1897) % 7, sres(0)); + assert_eq!(res(1897) % 7, res(0)); + assert_eq!(sres(1897) % 7, sres(0)); + + // Remainder works as expected. + assert_eq!(sat(1901) % 7, res(4)); + assert_eq!(ssat(1901) % 7, sres(4)); + assert_eq!(res(1901) % 7, res(4)); + assert_eq!(sres(1901) % 7, sres(4)); +} + #[test] fn check_const() { assert_eq!(SignedAmount::ONE_BTC.to_sat(), 100_000_000); From b57bfb9bc53107d7a2f6f9f7cb2f4c6c5d733534 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 3 Mar 2025 07:51:54 +1100 Subject: [PATCH 5/6] Add missing Mul impls for amount types Add and test missing `Mul` impls for both amount types. --- units/src/amount/result.rs | 20 ++++++++++++++++++++ units/src/amount/tests.rs | 6 ++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs index 724230833..5f1d7742b 100644 --- a/units/src/amount/result.rs +++ b/units/src/amount/result.rs @@ -161,6 +161,16 @@ crate::internal_macros::impl_op_for_references! { fn mul(self, rhs: u64) -> Self::Output { self.and_then(|lhs| lhs * rhs) } } + impl ops::Mul for u64 { + type Output = NumOpResult; + + fn mul(self, rhs: Amount) -> Self::Output { rhs.checked_mul(self).valid_or_error() } + } + impl ops::Mul> for u64 { + type Output = NumOpResult; + + fn mul(self, rhs: NumOpResult) -> Self::Output { rhs.and_then(|rhs| self * rhs) } + } impl ops::Div for Amount { type Output = NumOpResult; @@ -221,6 +231,16 @@ crate::internal_macros::impl_op_for_references! { fn mul(self, rhs: i64) -> Self::Output { self.and_then(|lhs| lhs * rhs) } } + impl ops::Mul for i64 { + type Output = NumOpResult; + + fn mul(self, rhs: SignedAmount) -> Self::Output { rhs.checked_mul(self).valid_or_error() } + } + impl ops::Mul> for i64 { + type Output = NumOpResult; + + fn mul(self, rhs: NumOpResult) -> Self::Output { rhs.and_then(|rhs| self * rhs) } + } impl ops::Div for SignedAmount { type Output = NumOpResult; diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 3ab69af1d..4a87ba420 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -1252,8 +1252,10 @@ fn op_int_combos() { assert_eq!(res(23) * 31, res(713)); assert_eq!(sres(23) * 31, sres(713)); - // assert_eq!(31 * sat(23), res(713)); - // assert_eq!(31 * ssat(23), sres(713)); + assert_eq!(31 * sat(23), res(713)); + assert_eq!(31 * ssat(23), sres(713)); + assert_eq!(31 * res(23), res(713)); + assert_eq!(31 * sres(23), sres(713)); // No remainder. assert_eq!(sat(1897) / 7, res(271)); From 0a9f14f7b036c5232449d058fb6d425c8376d87a Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 3 Mar 2025 08:00:14 +1100 Subject: [PATCH 6/6] Implement Div by amount for amount types It is semantically valid to divide an amount by another amount. The result of the operation is an integer. Note that we cannot implement `Div` by `NumOpResult` because there is no way to show the div by invalid case. Implement `Div` by amount for both amount types. --- units/src/amount/result.rs | 10 ++++++++++ units/src/amount/tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs index 5f1d7742b..be106d6b1 100644 --- a/units/src/amount/result.rs +++ b/units/src/amount/result.rs @@ -182,6 +182,11 @@ crate::internal_macros::impl_op_for_references! { fn div(self, rhs: u64) -> Self::Output { self.and_then(|lhs| lhs / rhs) } } + impl ops::Div for Amount { + type Output = u64; + + fn div(self, rhs: Amount) -> Self::Output { self.to_sat() / rhs.to_sat() } + } impl ops::Rem for Amount { type Output = NumOpResult; @@ -252,6 +257,11 @@ crate::internal_macros::impl_op_for_references! { fn div(self, rhs: i64) -> Self::Output { self.and_then(|lhs| lhs / rhs) } } + impl ops::Div for SignedAmount { + type Output = i64; + + fn div(self, rhs: SignedAmount) -> Self::Output { self.to_sat() / rhs.to_sat() } + } impl ops::Rem for SignedAmount { type Output = NumOpResult; diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 4a87ba420..b3e859341 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -1282,6 +1282,38 @@ fn op_int_combos() { assert_eq!(sres(1901) % 7, sres(4)); } +#[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); +} + +#[test] +#[should_panic(expected = "attempt to divide by zero")] +fn unsigned_amount_div_by_amount_zero() { + let _ = Amount::from_sat(1897) / 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); + assert_eq!(ssat(1897) / ssat(-7), -271); + assert_eq!(ssat(-1897) / ssat(7), -271); + assert_eq!(ssat(-1897) / ssat(-7), 271); +} + +#[test] +#[should_panic(expected = "attempt to divide by zero")] +fn signed_amount_div_by_amount_zero() { + let _ = SignedAmount::from_sat(1897) / SignedAmount::ZERO; +} + #[test] fn check_const() { assert_eq!(SignedAmount::ONE_BTC.to_sat(), 100_000_000);