Skip to content

Commit

Permalink
Merge rust-bitcoin#3811: Introduce an unchecked constructor for the `…
Browse files Browse the repository at this point in the history 
…Amount` type

2042d91 api: Run just check-api (Tobin C. Harding)
04bae4b Use _unchecked in unit tests (Tobin C. Harding)
8e6784d Introduce Amount::from_sat_unchecked (Tobin C. Harding)
f32af7d Use ua_sat throughout test function (Tobin C. Harding)
75f0afd Add local variable to use as constructor (Tobin C. Harding)
940a244 Replace Amount::from_sat(0) with ZERO (Tobin C. Harding)

Pull request description:

  This is a small-ish changeset in order to mirror what was started in rust-bitcoin#3769 and reduce the diff in rust-bitcoin#3794

  - Patch 1: Trivial refactor to use `Amount::ZERO`.
  - Patch 2: Add the constructor and refactor a single unit test to use it.
  - Patch 3: Use the two `_unchecked` constructors throughout `units/src/amount/tests.rs`.
  - Patch 4: Add the API text file changes.

ACKs for top commit:
  jamillambert:
    ACK 2042d91
  apoelstra:
    ACK 2042d91; successfully ran local tests

Tree-SHA512: 8f9c3816509bb3fc0707cd4a47426130d001358e82273457b24170522644e10fe36b596e16500ce1778738f5546dfad592f1f3c4de552ec0afcb146442176cf5
  • Loading branch information
@apoelstra
apoelstra committed Dec 30, 2024
2 parents 148b8b1 + 2042d91 commit 13c066e
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 68 deletions.
1 change: 1 addition & 0 deletions api/units/all-features.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ pub const fn bitcoin_units::Amount::checked_rem(self, rhs: u64) -> core::option:
pub const fn bitcoin_units::Amount::checked_sub(self, rhs: bitcoin_units::Amount) -> core::option::Option<bitcoin_units::Amount>
pub const fn bitcoin_units::Amount::from_int_btc_const(whole_bitcoin: u32) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::from_sat(satoshi: u64) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::from_sat_unchecked(satoshi: u64) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::to_sat(self) -> u64
pub const fn bitcoin_units::SignedAmount::checked_abs(self) -> core::option::Option<bitcoin_units::SignedAmount>
pub const fn bitcoin_units::SignedAmount::checked_add(self, rhs: bitcoin_units::SignedAmount) -> core::option::Option<bitcoin_units::SignedAmount>
Expand Down
1 change: 1 addition & 0 deletions api/units/alloc-only.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,7 @@ pub const fn bitcoin_units::Amount::checked_rem(self, rhs: u64) -> core::option:
pub const fn bitcoin_units::Amount::checked_sub(self, rhs: bitcoin_units::Amount) -> core::option::Option<bitcoin_units::Amount>
pub const fn bitcoin_units::Amount::from_int_btc_const(whole_bitcoin: u32) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::from_sat(satoshi: u64) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::from_sat_unchecked(satoshi: u64) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::to_sat(self) -> u64
pub const fn bitcoin_units::SignedAmount::checked_abs(self) -> core::option::Option<bitcoin_units::SignedAmount>
pub const fn bitcoin_units::SignedAmount::checked_add(self, rhs: bitcoin_units::SignedAmount) -> core::option::Option<bitcoin_units::SignedAmount>
Expand Down
1 change: 1 addition & 0 deletions api/units/no-features.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ pub const fn bitcoin_units::Amount::checked_rem(self, rhs: u64) -> core::option:
pub const fn bitcoin_units::Amount::checked_sub(self, rhs: bitcoin_units::Amount) -> core::option::Option<bitcoin_units::Amount>
pub const fn bitcoin_units::Amount::from_int_btc_const(whole_bitcoin: u32) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::from_sat(satoshi: u64) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::from_sat_unchecked(satoshi: u64) -> bitcoin_units::Amount
pub const fn bitcoin_units::Amount::to_sat(self) -> u64
pub const fn bitcoin_units::SignedAmount::checked_abs(self) -> core::option::Option<bitcoin_units::SignedAmount>
pub const fn bitcoin_units::SignedAmount::checked_add(self, rhs: bitcoin_units::SignedAmount) -> core::option::Option<bitcoin_units::SignedAmount>
Expand Down
4 changes: 1 addition & 3 deletions units/src/amount/signed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ impl SignedAmount {
/// 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_unchecked(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) }

/// Converts from a value expressing a decimal number of bitcoin to a [`SignedAmount`].
///
Expand Down
134 changes: 69 additions & 65 deletions units/src/amount/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn from_str_zero() {
let s = format!("{} {}", v, denom);
match s.parse::<Amount>() {
Err(e) => panic!("failed to crate amount from {}: {:?}", s, e),
Ok(amount) => assert_eq!(amount, Amount::from_sat(0)),
Ok(amount) => assert_eq!(amount, Amount::ZERO),
}
}

Expand All @@ -41,7 +41,7 @@ fn from_str_zero() {
}
match s.parse::<SignedAmount>() {
Err(e) => panic!("failed to crate amount from {}: {:?}", s, e),
Ok(amount) => assert_eq!(amount, SignedAmount::from_sat(0)),
Ok(amount) => assert_eq!(amount, SignedAmount::ZERO),
}
}
}
Expand All @@ -68,16 +68,16 @@ fn from_str_zero_without_denomination() {
#[test]
fn from_int_btc() {
let amt = Amount::from_int_btc_const(2);
assert_eq!(Amount::from_sat(200_000_000), amt);
assert_eq!(Amount::from_sat_unchecked(200_000_000), amt);
}

#[test]
fn test_amount_try_from_signed_amount() {
let sa_positive = SignedAmount::from_sat(123);
let sa_positive = SignedAmount::from_sat_unchecked(123);
let ua_positive = Amount::try_from(sa_positive).unwrap();
assert_eq!(ua_positive, Amount::from_sat(123));
assert_eq!(ua_positive, Amount::from_sat_unchecked(123));

let sa_negative = SignedAmount::from_sat(-123);
let sa_negative = SignedAmount::from_sat_unchecked(-123);
let result = Amount::try_from(sa_negative);
assert_eq!(result, Err(OutOfRangeError { is_signed: false, is_greater_than_max: false }));
}
Expand Down Expand Up @@ -105,9 +105,9 @@ fn mul_div() {
#[test]
fn test_overflows() {
// panic on overflow
let result = panic::catch_unwind(|| Amount::MAX + Amount::from_sat(1));
let result = panic::catch_unwind(|| Amount::MAX + Amount::from_sat_unchecked(1));
assert!(result.is_err());
let result = panic::catch_unwind(|| Amount::from_sat(8_446_744_073_709_551_615) * 3);
let result = panic::catch_unwind(|| Amount::from_sat_unchecked(8_446_744_073_709_551_615) * 3);
assert!(result.is_err());
}

Expand All @@ -129,12 +129,12 @@ fn checked_arithmetic() {
#[test]
fn amount_checked_div_by_weight_ceil() {
let weight = Weight::from_kwu(1).unwrap();
let fee_rate = Amount::from_sat(1).checked_div_by_weight_ceil(weight).unwrap();
let fee_rate = Amount::from_sat_unchecked(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(329).checked_div_by_weight_ceil(weight).unwrap();
let fee_rate = Amount::from_sat_unchecked(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));
Expand All @@ -147,12 +147,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(1).checked_div_by_weight_floor(weight).unwrap();
let fee_rate = Amount::from_sat_unchecked(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(329).checked_div_by_weight_floor(weight).unwrap();
let fee_rate = Amount::from_sat_unchecked(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));
Expand Down Expand Up @@ -239,22 +239,22 @@ fn parsing() {
#[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(1)));
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(1001)));
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("1", btc), Ok(Amount::from_sat(1_000_000_00)));
assert_eq!(sp("-.5", btc), Ok(SignedAmount::from_sat(-500_000_00)));
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)));
#[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(1_100_000_00)));
assert_eq!(p("100", sat), Ok(Amount::from_sat(100)));
assert_eq!(p("55", sat), Ok(Amount::from_sat(55)));
assert_eq!(p("2100000000000000", sat), Ok(Amount::from_sat(21_000_000__000_000_00)));
assert_eq!(p("2100000000000000.", sat), Ok(Amount::from_sat(21_000_000__000_000_00)));
assert_eq!(p("21000000", btc), Ok(Amount::from_sat(21_000_000__000_000_00)));
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)));

// exactly 50 chars.
assert_eq!(
Expand All @@ -277,13 +277,13 @@ 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(-42).to_string_in(D::Bitcoin), "-0.00000042");
assert_eq!(SignedAmount::from_sat_unchecked(-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(-42).to_string_with_denomination(D::Bitcoin),
SignedAmount::from_sat_unchecked(-42).to_string_with_denomination(D::Bitcoin),
"-0.00000042 BTC"
);
}
Expand Down Expand Up @@ -543,12 +543,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(50));
ok_scase("-.5 bits", SignedAmount::from_sat(-50));
ok_case("0.00253583 BTC", Amount::from_sat(253_583));
ok_scase("-5 satoshi", SignedAmount::from_sat(-5));
ok_case("0.10000000 BTC", Amount::from_sat(100_000_00));
ok_scase("-100 bits", SignedAmount::from_sat(-10_000));
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("21000000 BTC", Amount::MAX);
ok_scase("21000000 BTC", SignedAmount::MAX);
ok_scase("-21000000 BTC", SignedAmount::MIN);
Expand All @@ -560,44 +560,45 @@ fn from_str() {
fn to_from_string_in() {
use super::Denomination as D;
let ua_str = Amount::from_str_in;
let ua_sat = Amount::from_sat;
let ua_sat = Amount::from_sat_unchecked;
let sa_str = SignedAmount::from_str_in;
let sa_sat = SignedAmount::from_sat_unchecked;

assert_eq!("0.5", Amount::from_sat(50).to_string_in(D::Bit));
assert_eq!("-0.5", SignedAmount::from_sat(-50).to_string_in(D::Bit));
assert_eq!("0.00253583", Amount::from_sat(253_583).to_string_in(D::Bitcoin));
assert_eq!("-5", SignedAmount::from_sat(-5).to_string_in(D::Satoshi));
assert_eq!("0.1", Amount::from_sat(100_000_00).to_string_in(D::Bitcoin));
assert_eq!("-0.1", SignedAmount::from_sat(-100_000_00).to_string_in(D::Bitcoin));
assert_eq!("0.5", ua_sat(50).to_string_in(D::Bit));
assert_eq!("-0.5", sa_sat(-50).to_string_in(D::Bit));
assert_eq!("0.00253583", ua_sat(253_583).to_string_in(D::Bitcoin));
assert_eq!("-5", sa_sat(-5).to_string_in(D::Satoshi));
assert_eq!("0.1", ua_sat(100_000_00).to_string_in(D::Bitcoin));
assert_eq!("-0.1", sa_sat(-100_000_00).to_string_in(D::Bitcoin));

assert_eq!("0.253583", Amount::from_sat(253_583).to_string_in(D::CentiBitcoin));
assert_eq!("-0.253583", SignedAmount::from_sat(-253_583).to_string_in(D::CentiBitcoin));
assert_eq!("10", Amount::from_sat(100_000_00).to_string_in(D::CentiBitcoin));
assert_eq!("-10", SignedAmount::from_sat(-100_000_00).to_string_in(D::CentiBitcoin));
assert_eq!("0.253583", ua_sat(253_583).to_string_in(D::CentiBitcoin));
assert_eq!("-0.253583", sa_sat(-253_583).to_string_in(D::CentiBitcoin));
assert_eq!("10", ua_sat(100_000_00).to_string_in(D::CentiBitcoin));
assert_eq!("-10", sa_sat(-100_000_00).to_string_in(D::CentiBitcoin));

assert_eq!("2.53583", Amount::from_sat(253_583).to_string_in(D::MilliBitcoin));
assert_eq!("-2.53583", SignedAmount::from_sat(-253_583).to_string_in(D::MilliBitcoin));
assert_eq!("100", Amount::from_sat(100_000_00).to_string_in(D::MilliBitcoin));
assert_eq!("-100", SignedAmount::from_sat(-100_000_00).to_string_in(D::MilliBitcoin));
assert_eq!("2.53583", ua_sat(253_583).to_string_in(D::MilliBitcoin));
assert_eq!("-2.53583", sa_sat(-253_583).to_string_in(D::MilliBitcoin));
assert_eq!("100", ua_sat(100_000_00).to_string_in(D::MilliBitcoin));
assert_eq!("-100", sa_sat(-100_000_00).to_string_in(D::MilliBitcoin));

assert_eq!("2535.83", Amount::from_sat(253_583).to_string_in(D::MicroBitcoin));
assert_eq!("-2535.83", SignedAmount::from_sat(-253_583).to_string_in(D::MicroBitcoin));
assert_eq!("100000", Amount::from_sat(100_000_00).to_string_in(D::MicroBitcoin));
assert_eq!("-100000", SignedAmount::from_sat(-100_000_00).to_string_in(D::MicroBitcoin));
assert_eq!("2535.83", ua_sat(253_583).to_string_in(D::MicroBitcoin));
assert_eq!("-2535.83", sa_sat(-253_583).to_string_in(D::MicroBitcoin));
assert_eq!("100000", ua_sat(100_000_00).to_string_in(D::MicroBitcoin));
assert_eq!("-100000", sa_sat(-100_000_00).to_string_in(D::MicroBitcoin));

assert_eq!("0.5", Amount::from_sat(50).to_string_in(D::Bit));
assert_eq!("100", Amount::from_sat(10_000).to_string_in(D::Bit));
assert_eq!("-0.5", SignedAmount::from_sat(-50).to_string_in(D::Bit));
assert_eq!("-100", SignedAmount::from_sat(-10_000).to_string_in(D::Bit));
assert_eq!("0.5", ua_sat(50).to_string_in(D::Bit));
assert_eq!("100", ua_sat(10_000).to_string_in(D::Bit));
assert_eq!("-0.5", sa_sat(-50).to_string_in(D::Bit));
assert_eq!("-100", sa_sat(-10_000).to_string_in(D::Bit));

assert_eq!("5", Amount::from_sat(5).to_string_in(D::Satoshi));
assert_eq!("-5", SignedAmount::from_sat(-5).to_string_in(D::Satoshi));
assert_eq!("5", ua_sat(5).to_string_in(D::Satoshi));
assert_eq!("-5", sa_sat(-5).to_string_in(D::Satoshi));

assert_eq!("0.50", format!("{:.2}", Amount::from_sat(50).display_in(D::Bit)));
assert_eq!("-0.50", format!("{:.2}", SignedAmount::from_sat(-50).display_in(D::Bit)));
assert_eq!("0.50", format!("{:.2}", ua_sat(50).display_in(D::Bit)));
assert_eq!("-0.50", format!("{:.2}", sa_sat(-50).display_in(D::Bit)));

assert_eq!("0.10000000", format!("{:.8}", Amount::from_sat(100_000_00).display_in(D::Bitcoin)));
assert_eq!("-100.00", format!("{:.2}", SignedAmount::from_sat(-10_000).display_in(D::Bit)));
assert_eq!("0.10000000", format!("{:.8}", ua_sat(100_000_00).display_in(D::Bitcoin)));
assert_eq!("-100.00", format!("{:.2}", sa_sat(-10_000).display_in(D::Bit)));

assert_eq!(ua_str(&ua_sat(500).to_string_in(D::Bitcoin), D::Bitcoin), Ok(ua_sat(500)));
assert_eq!(ua_str(&ua_sat(1).to_string_in(D::CentiBitcoin), D::CentiBitcoin), Ok(ua_sat(1)));
Expand Down Expand Up @@ -637,7 +638,7 @@ fn to_string_with_denomination_from_str_roundtrip() {

use super::Denomination as D;

let amt = Amount::from_sat(42);
let amt = Amount::from_sat_unchecked(42);
let denom = Amount::to_string_with_denomination;
assert_eq!(denom(amt, D::Bitcoin).parse::<Amount>(), Ok(amt));
assert_eq!(denom(amt, D::CentiBitcoin).parse::<Amount>(), Ok(amt));
Expand Down Expand Up @@ -668,7 +669,10 @@ fn serde_as_sat() {
}

serde_test::assert_tokens(
&T { amt: Amount::from_sat(123_456_789), samt: SignedAmount::from_sat(-123_456_789) },
&T {
amt: Amount::from_sat_unchecked(123_456_789),
samt: SignedAmount::from_sat_unchecked(-123_456_789),
},
&[
serde_test::Token::Struct { name: "T", len: 2 },
serde_test::Token::Str("amt"),
Expand Down Expand Up @@ -873,8 +877,8 @@ fn serde_as_str_opt() {

#[test]
fn sum_amounts() {
assert_eq!(Amount::from_sat(0), [].iter().sum::<Amount>());
assert_eq!(SignedAmount::from_sat(0), [].iter().sum::<SignedAmount>());
assert_eq!(Amount::ZERO, [].iter().sum::<Amount>());
assert_eq!(SignedAmount::ZERO, [].iter().sum::<SignedAmount>());

let amounts = [Amount::from_sat(42), Amount::from_sat(1337), Amount::from_sat(21)];
let sum = amounts.into_iter().sum::<Amount>();
Expand All @@ -888,8 +892,8 @@ fn sum_amounts() {

#[test]
fn checked_sum_amounts() {
assert_eq!(Some(Amount::from_sat(0)), [].into_iter().checked_sum());
assert_eq!(Some(SignedAmount::from_sat(0)), [].into_iter().checked_sum());
assert_eq!(Some(Amount::ZERO), [].into_iter().checked_sum());
assert_eq!(Some(SignedAmount::ZERO), [].into_iter().checked_sum());

let amounts = [Amount::from_sat(42), Amount::from_sat(1337), Amount::from_sat(21)];
let sum = amounts.into_iter().checked_sum();
Expand Down
5 changes: 5 additions & 0 deletions units/src/amount/unsigned.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ impl Amount {
/// ```
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`].
///
/// # Errors
Expand Down

0 comments on commit 13c066e

Please sign in to comment.