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.
This commit is contained in:
Tobin C. Harding 2025-02-28 09:54:50 +11:00
parent e6f7b26d80
commit 6d70c77cf9
No known key found for this signature in database
GPG Key ID: 40BF9E4C269D6607
2 changed files with 156 additions and 126 deletions

View File

@ -16,6 +16,7 @@ use super::{
DisplayStyle, OutOfRangeError, ParseAmountError, ParseError, DisplayStyle, OutOfRangeError, ParseAmountError, ParseError,
}; };
mod encapsulate {
/// A signed amount. /// A signed amount.
/// ///
/// The [`SignedAmount`] type can be used to express Bitcoin amounts that support arithmetic and /// The [`SignedAmount`] type can be used to express Bitcoin amounts that support arithmetic and
@ -48,6 +49,28 @@ use super::{
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct SignedAmount(i64); 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 { impl SignedAmount {
/// The zero amount. /// The zero amount.
pub const ZERO: Self = SignedAmount::from_sat_unchecked(0); pub const ZERO: Self = SignedAmount::from_sat_unchecked(0);
@ -73,24 +96,9 @@ impl SignedAmount {
/// let amount = SignedAmount::from_sat(-100_000); /// let amount = SignedAmount::from_sat(-100_000);
/// assert_eq!(amount.to_sat(), -100_000); /// assert_eq!(amount.to_sat(), -100_000);
/// ``` /// ```
pub const fn from_sat(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) } pub const fn from_sat(satoshi: i64) -> SignedAmount {
SignedAmount::from_sat_unchecked(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) }
/// Converts from a value expressing a decimal number of bitcoin to a [`SignedAmount`]. /// 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( (false, sat) if sat > SignedAmount::MAX.to_sat() as u64 => Err(ParseAmountError(
ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_big(true)), 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( (true, sat) if sat > SignedAmount::MIN.to_sat().unsigned_abs() => Err(
ParseAmountError(ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_small())), 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`]. /// Gets the absolute value of this [`SignedAmount`].
#[must_use] #[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`]. /// Gets the absolute value of this [`SignedAmount`] returning [`Amount`].
#[must_use] #[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`]. /// 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 positive
/// - `-1` if the amount is negative /// - `-1` if the amount is negative
#[must_use] #[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. /// Checks if this [`SignedAmount`] is positive.
/// ///
/// Returns `true` if this [`SignedAmount`] is positive and `false` if /// Returns `true` if this [`SignedAmount`] is positive and `false` if
/// this [`SignedAmount`] is zero or negative. /// 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. /// Checks if this [`SignedAmount`] is negative.
/// ///
/// Returns `true` if this [`SignedAmount`] is negative and `false` if /// Returns `true` if this [`SignedAmount`] is negative and `false` if
/// this [`SignedAmount`] is zero or positive. /// 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`]. /// Returns the absolute value of this [`SignedAmount`].
/// ///
@ -334,8 +342,8 @@ impl SignedAmount {
#[must_use] #[must_use]
pub const fn checked_abs(self) -> Option<SignedAmount> { pub const fn checked_abs(self) -> Option<SignedAmount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_abs() { match self.to_sat().checked_abs() {
Some(res) => Some(SignedAmount(res)), Some(res) => Some(SignedAmount::from_sat(res)),
None => None, None => None,
} }
} }
@ -346,8 +354,8 @@ impl SignedAmount {
#[must_use] #[must_use]
pub const fn checked_add(self, rhs: SignedAmount) -> Option<SignedAmount> { pub const fn checked_add(self, rhs: SignedAmount) -> Option<SignedAmount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_add(rhs.0) { match self.to_sat().checked_add(rhs.to_sat()) {
Some(res) => SignedAmount(res).check_min_max(), Some(res) => SignedAmount::from_sat(res).check_min_max(),
None => None, None => None,
} }
} }
@ -359,8 +367,8 @@ impl SignedAmount {
#[must_use] #[must_use]
pub const fn checked_sub(self, rhs: SignedAmount) -> Option<SignedAmount> { pub const fn checked_sub(self, rhs: SignedAmount) -> Option<SignedAmount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_sub(rhs.0) { match self.to_sat().checked_sub(rhs.to_sat()) {
Some(res) => SignedAmount(res).check_min_max(), Some(res) => SignedAmount::from_sat(res).check_min_max(),
None => None, None => None,
} }
} }
@ -372,8 +380,8 @@ impl SignedAmount {
#[must_use] #[must_use]
pub const fn checked_mul(self, rhs: i64) -> Option<SignedAmount> { pub const fn checked_mul(self, rhs: i64) -> Option<SignedAmount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_mul(rhs) { match self.to_sat().checked_mul(rhs) {
Some(res) => SignedAmount(res).check_min_max(), Some(res) => SignedAmount::from_sat(res).check_min_max(),
None => None, None => None,
} }
} }
@ -386,8 +394,8 @@ impl SignedAmount {
#[must_use] #[must_use]
pub const fn checked_div(self, rhs: i64) -> Option<SignedAmount> { pub const fn checked_div(self, rhs: i64) -> Option<SignedAmount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_div(rhs) { match self.to_sat().checked_div(rhs) {
Some(res) => Some(SignedAmount(res)), Some(res) => Some(SignedAmount::from_sat(res)),
None => None, None => None,
} }
} }
@ -398,8 +406,8 @@ impl SignedAmount {
#[must_use] #[must_use]
pub const fn checked_rem(self, rhs: i64) -> Option<SignedAmount> { pub const fn checked_rem(self, rhs: i64) -> Option<SignedAmount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_rem(rhs) { match self.to_sat().checked_rem(rhs) {
Some(res) => Some(SignedAmount(res)), Some(res) => Some(SignedAmount::from_sat(res)),
None => None, None => None,
} }
} }
@ -413,7 +421,9 @@ impl SignedAmount {
/// On overflow, panics in debug mode, wraps in release mode. /// On overflow, panics in debug mode, wraps in release mode.
#[must_use] #[must_use]
#[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] #[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. /// Unchecked subtraction.
/// ///
@ -424,7 +434,9 @@ impl SignedAmount {
/// On overflow, panics in debug mode, wraps in release mode. /// On overflow, panics in debug mode, wraps in release mode.
#[must_use] #[must_use]
#[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] #[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. /// Subtraction that doesn't allow negative [`SignedAmount`]s.
/// ///
@ -453,7 +465,7 @@ impl SignedAmount {
/// Checks the amount is within the allowed range. /// Checks the amount is within the allowed range.
const fn check_min_max(self) -> Option<SignedAmount> { const fn check_min_max(self) -> Option<SignedAmount> {
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 None
} else { } else {
Some(self) Some(self)
@ -514,6 +526,6 @@ impl From<Amount> for SignedAmount {
impl<'a> Arbitrary<'a> for SignedAmount { impl<'a> Arbitrary<'a> for SignedAmount {
fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
let s = i64::arbitrary(u)?; let s = i64::arbitrary(u)?;
Ok(Self(s)) Ok(Self::from_sat(s))
} }
} }

View File

@ -16,12 +16,21 @@ use super::{
OutOfRangeError, ParseAmountError, ParseError, SignedAmount, OutOfRangeError, ParseAmountError, ParseError, SignedAmount,
}; };
mod encapsulate {
/// An amount. /// An amount.
/// ///
/// The [`Amount`] type can be used to express Bitcoin amounts that support arithmetic and /// 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 /// conversion to various denominations. The [`Amount`] type does not implement [`serde`] traits
/// but we do provide modules for serializing as satoshis or bitcoin. /// 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 /// # Examples
/// ///
/// ``` /// ```
@ -40,6 +49,26 @@ use super::{
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Amount(u64); 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 { impl Amount {
/// The zero amount. /// The zero amount.
pub const ZERO: Self = Amount::from_sat_unchecked(0); pub const ZERO: Self = Amount::from_sat_unchecked(0);
@ -67,22 +96,7 @@ impl Amount {
/// let amount = Amount::from_sat(100_000); /// let amount = Amount::from_sat(100_000);
/// assert_eq!(amount.to_sat(), 100_000); /// assert_eq!(amount.to_sat(), 100_000);
/// ``` /// ```
pub const fn from_sat(satoshi: u64) -> Amount { Amount(satoshi) } pub const fn from_sat(satoshi: u64) -> Amount { Amount::from_sat_unchecked(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) }
/// Converts from a value expressing a decimal number of bitcoin to an [`Amount`]. /// Converts from a value expressing a decimal number of bitcoin to an [`Amount`].
/// ///
@ -148,7 +162,7 @@ impl Amount {
OutOfRangeError::negative(), OutOfRangeError::negative(),
))); )));
} }
if sats > Self::MAX.0 { if sats > Self::MAX.to_sat() {
return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange( return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange(
OutOfRangeError::too_big(false), OutOfRangeError::too_big(false),
))); )));
@ -299,8 +313,8 @@ impl Amount {
#[must_use] #[must_use]
pub const fn checked_add(self, rhs: Amount) -> Option<Amount> { pub const fn checked_add(self, rhs: Amount) -> Option<Amount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_add(rhs.0) { match self.to_sat().checked_add(rhs.to_sat()) {
Some(res) => Amount(res).check_max(), Some(res) => Amount::from_sat(res).check_max(),
None => None, None => None,
} }
} }
@ -311,8 +325,8 @@ impl Amount {
#[must_use] #[must_use]
pub const fn checked_sub(self, rhs: Amount) -> Option<Amount> { pub const fn checked_sub(self, rhs: Amount) -> Option<Amount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_sub(rhs.0) { match self.to_sat().checked_sub(rhs.to_sat()) {
Some(res) => Some(Amount(res)), Some(res) => Some(Amount::from_sat(res)),
None => None, None => None,
} }
} }
@ -323,8 +337,8 @@ impl Amount {
#[must_use] #[must_use]
pub const fn checked_mul(self, rhs: u64) -> Option<Amount> { pub const fn checked_mul(self, rhs: u64) -> Option<Amount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_mul(rhs) { match self.to_sat().checked_mul(rhs) {
Some(res) => Amount(res).check_max(), Some(res) => Amount::from_sat(res).check_max(),
None => None, None => None,
} }
} }
@ -337,8 +351,8 @@ impl Amount {
#[must_use] #[must_use]
pub const fn checked_div(self, rhs: u64) -> Option<Amount> { pub const fn checked_div(self, rhs: u64) -> Option<Amount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_div(rhs) { match self.to_sat().checked_div(rhs) {
Some(res) => Some(Amount(res)), Some(res) => Some(Amount::from_sat(res)),
None => None, None => None,
} }
} }
@ -349,8 +363,8 @@ impl Amount {
#[must_use] #[must_use]
pub const fn checked_rem(self, rhs: u64) -> Option<Amount> { pub const fn checked_rem(self, rhs: u64) -> Option<Amount> {
// No `map()` in const context. // No `map()` in const context.
match self.0.checked_rem(rhs) { match self.to_sat().checked_rem(rhs) {
Some(res) => Some(Amount(res)), Some(res) => Some(Amount::from_sat(res)),
None => None, None => None,
} }
} }
@ -364,7 +378,9 @@ impl Amount {
/// On overflow, panics in debug mode, wraps in release mode. /// On overflow, panics in debug mode, wraps in release mode.
#[must_use] #[must_use]
#[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] #[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. /// Unchecked subtraction.
/// ///
@ -375,7 +391,9 @@ impl Amount {
/// On overflow, panics in debug mode, wraps in release mode. /// On overflow, panics in debug mode, wraps in release mode.
#[must_use] #[must_use]
#[deprecated(since = "TBD", note = "consider converting to u64 using `to_sat`")] #[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. /// Converts to a signed amount.
#[rustfmt::skip] // Moves code comments to the wrong line. #[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. /// Checks if the amount is below the maximum value. Returns `None` if it is above.
const fn check_max(self) -> Option<Amount> { const fn check_max(self) -> Option<Amount> {
if self.0 > Self::MAX.0 { if self.to_sat() > Self::MAX.to_sat() {
None None
} else { } else {
Some(self) Some(self)
@ -445,6 +463,6 @@ impl TryFrom<SignedAmount> for Amount {
impl<'a> Arbitrary<'a> for Amount { impl<'a> Arbitrary<'a> for Amount {
fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
let a = u64::arbitrary(u)?; let a = u64::arbitrary(u)?;
Ok(Self(a)) Ok(Self::from_sat(a))
} }
} }