From 6c614d9320e542e19196753bddc5b124e0274176 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 19 Mar 2025 13:58:12 +1100 Subject: [PATCH 1/2] units: Fix panic message Recently I wrote a panic message that included the maximum value of an integer however I used the max of a 16 bit value for both signed and unsigned - this is incorrect. Use the correct values for `u16::MAX` and `i16::MAX`. --- units/src/amount/signed.rs | 2 +- units/src/amount/unsigned.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 7070adf77..8b152c2df 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -152,7 +152,7 @@ impl SignedAmount { match Self::from_sat(sats) { Ok(amount) => amount, - Err(_) => panic!("unreachable - 65536 BTC is within range"), + Err(_) => panic!("unreachable - 32,767 BTC is within range"), } } diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 20bb6f82b..9504af2a0 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -152,7 +152,7 @@ impl Amount { match Self::from_sat(sats) { Ok(amount) => amount, - Err(_) => panic!("unreachable - 65536 BTC is within range"), + Err(_) => panic!("unreachable - 65,535 BTC is within range"), } } From ca6c607953c03aa2dc168f58329681d9e69eee04 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 19 Mar 2025 14:00:59 +1100 Subject: [PATCH 2/2] Adhere to sanity rules for amount types From my reading of the new sanity rules (#4090) we should only have a single constructor that accesses the inner field of the amount types. Furthermore we have one const constructor inside the privacy boundry and a couple outside. Move the const constructors outside of the privacy boundry. Internal change only. Please note The function being inside privacy boundary allows it to not have the "runtime" check (most likely optimized-away after inlining). But if we wanted to get rid of that check we should have _unchecked method instead. But we don't want that (yet), since the check here will have zero performance impact in optimized builds and it's not worth the cost of dealing with unchecked constructors to optimize debug builds. --- units/src/amount/signed.rs | 20 ++++++++++++-------- units/src/amount/unsigned.rs | 20 ++++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 8b152c2df..c03fc5a45 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -57,14 +57,6 @@ mod encapsulate { /// The minimum value of an amount. pub const MIN: Self = Self(-21_000_000 * 100_000_000); - /// Constructs a new [`SignedAmount`] with satoshi precision and the given number of satoshis. - /// - /// Accepts an `i32` which is guaranteed to be in range for the type, but which can only - /// represent roughly -21.47 to 21.47 BTC. - pub const fn from_sat_i32(satoshi: i32) -> Self { - Self(satoshi as i64) // cannot use i64::from in a constfn - } - /// Gets the number of satoshis in this [`SignedAmount`]. /// /// # Examples @@ -116,6 +108,18 @@ impl SignedAmount { /// The maximum value allowed as an amount. Useful for sanity checking. pub const MAX_MONEY: Self = Self::MAX; + /// Constructs a new [`SignedAmount`] with satoshi precision and the given number of satoshis. + /// + /// Accepts an `i32` which is guaranteed to be in range for the type, but which can only + /// represent roughly -21.47 to 21.47 BTC. + pub const fn from_sat_i32(satoshi: i32) -> Self { + let sats = satoshi as i64; // cannot use i64::from in a constfn + match Self::from_sat(sats) { + Ok(amount) => amount, + Err(_) => panic!("unreachable - 32,767 BTC is within range"), + } + } + /// Converts from a value expressing a decimal number of bitcoin to a [`SignedAmount`]. /// /// # Errors diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 9504af2a0..1109f7d0a 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -57,14 +57,6 @@ mod encapsulate { /// The minimum value of an amount. pub const MIN: Self = Self(0); - /// Constructs a new [`Amount`] with satoshi precision and the given number of satoshis. - /// - /// Accepts an `u32` which is guaranteed to be in range for the type, but which can only - /// represent roughly 0 to 42.95 BTC. - pub const fn from_sat_u32(satoshi: u32) -> Self { - Self(satoshi as u64) // cannot use u64::from in a constfn - } - /// Gets the number of satoshis in this [`Amount`]. /// /// # Examples @@ -116,6 +108,18 @@ 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. + /// + /// Accepts an `u32` which is guaranteed to be in range for the type, but which can only + /// represent roughly 0 to 42.95 BTC. + pub const fn from_sat_u32(satoshi: u32) -> Self { + let sats = satoshi as u64; // cannot use i64::from in a constfn + match Self::from_sat(sats) { + Ok(amount) => amount, + Err(_) => panic!("unreachable - 65,536 BTC is within range"), + } + } + /// Converts from a value expressing a decimal number of bitcoin to an [`Amount`]. /// /// # Errors