From 2ec1c2a0448c92bf12eb56d8c6bcf2d96b8bcf53 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 18 Mar 2025 09:37:10 +1100 Subject: [PATCH 1/8] units: Make from_int_btc_const take a 16 bit integer The `from_int_btc_const` constructors are specifically designed for easily creating amount types in const context but currently they return an error which is annoying to handle in const context. If we make the `whole_bitcoin` parameter a 16 bit integer this gives us a nicer const constructor with the downside that it can only create values upto a maximum of - unsigned: 65_536 - signed: 32_767 That is plenty high enough for most use cases. Then use the new `from_int_btc_const` in associated consts. Note that because `from_sat` checks max (and min) values we must define max and min from sats directly. --- units/src/amount/signed.rs | 24 +++++++++--------------- units/src/amount/tests.rs | 2 +- units/src/amount/unsigned.rs | 24 +++++++++--------------- 3 files changed, 19 insertions(+), 31 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 3f7927b47..2119f488d 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -77,9 +77,9 @@ impl SignedAmount { /// Exactly one satoshi. pub const ONE_SAT: Self = SignedAmount::from_sat_unchecked(1); /// Exactly one bitcoin. - pub const ONE_BTC: Self = SignedAmount::from_sat_unchecked(100_000_000); + pub const ONE_BTC: Self = SignedAmount::from_int_btc_const(1); /// Exactly fifty bitcoin. - pub const FIFTY_BTC: Self = SignedAmount::from_sat_unchecked(50 * 100_000_000); + pub const FIFTY_BTC: Self = SignedAmount::from_int_btc_const(50); /// The maximum value allowed as an amount. Useful for sanity checking. pub const MAX_MONEY: Self = SignedAmount::from_sat_unchecked(21_000_000 * 100_000_000); /// The minimum value of an amount. @@ -134,27 +134,21 @@ impl SignedAmount { } /// Converts from a value expressing a whole number of bitcoin to a [`SignedAmount`]. - /// - /// # Errors - /// - /// If `whole_bitcoin` is greater than `21_000_000`. #[allow(clippy::missing_panics_doc)] - pub fn from_int_btc>(whole_bitcoin: T) -> Result { + 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. - /// - /// # Errors - /// - /// If `whole_bitcoin` is greater than `21_000_000`. #[allow(clippy::missing_panics_doc)] - pub const fn from_int_btc_const(whole_bitcoin: i32) -> Result { + pub const fn from_int_btc_const(whole_bitcoin: i16) -> 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!("cannot overflow in i64"), + let sats = btc * 100_000_000; + + match SignedAmount::from_sat(sats) { + Ok(amount) => amount, + Err(_) => panic!("unreachable - 65536 BTC is within range"), } } diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index fedeb78cd..5fc495588 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -102,7 +102,7 @@ fn from_str_zero_without_denomination() { #[test] fn from_int_btc() { - let amt = Amount::from_int_btc_const(2).unwrap(); + let amt = Amount::from_int_btc_const(2); assert_eq!(sat(200_000_000), amt); } diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 1b328f964..ab1abb9c3 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -75,9 +75,9 @@ impl Amount { /// Exactly one satoshi. pub const ONE_SAT: Self = Amount::from_sat_unchecked(1); /// Exactly one bitcoin. - pub const ONE_BTC: Self = Amount::from_sat_unchecked(100_000_000); + pub const ONE_BTC: Self = Amount::from_int_btc_const(1); /// Exactly fifty bitcoin. - pub const FIFTY_BTC: Self = Amount::from_sat_unchecked(50 * 100_000_000); + pub const FIFTY_BTC: Self = Amount::from_int_btc_const(50); /// The maximum value allowed as an amount. Useful for sanity checking. pub const MAX_MONEY: Self = Amount::from_sat_unchecked(21_000_000 * 100_000_000); /// The minimum value of an amount. @@ -132,27 +132,21 @@ impl Amount { } /// Converts from a value expressing a whole number of bitcoin to an [`Amount`]. - /// - /// # Errors - /// - /// If `whole_bitcoin` is greater than `21_000_000`. #[allow(clippy::missing_panics_doc)] - pub fn from_int_btc>(whole_bitcoin: T) -> Result { + 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. - /// - /// # Errors - /// - /// If `whole_bitcoin` is greater than `21_000_000`. #[allow(clippy::missing_panics_doc)] - pub const fn from_int_btc_const(whole_bitcoin: u32) -> Result { + pub const fn from_int_btc_const(whole_bitcoin: u16) -> Amount { 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!("cannot overflow a u64"), + let sats = btc * 100_000_000; + + match Amount::from_sat(sats) { + Ok(amount) => amount, + Err(_) => panic!("unreachable - 65536 BTC is within range"), } } From 82d9d1aeea6c29a239a069045c20d2f65f97ac7e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Mar 2025 13:24:54 +0000 Subject: [PATCH 2/8] amount: rename `from_int_btc_const` unctions to hungarian ones We have `from_int_btc_const` on both `Amount` and `SignedAmount` because the "real" `from_int_btc` is generic over the integer type it accepts, which means that it cannot be a constfn. But we do want a constfn. However, just because `from_int_btc_const` exists for the sake of constfn doesn't mean that that's what it *is*. So rename these methods to reflect what they *are*. --- units/src/amount/signed.rs | 8 ++++---- units/src/amount/tests.rs | 4 +++- units/src/amount/unsigned.rs | 8 ++++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 2119f488d..463982895 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -77,9 +77,9 @@ impl SignedAmount { /// Exactly one satoshi. pub const ONE_SAT: Self = SignedAmount::from_sat_unchecked(1); /// Exactly one bitcoin. - pub const ONE_BTC: Self = SignedAmount::from_int_btc_const(1); + pub const ONE_BTC: Self = SignedAmount::from_btc_i16(1); /// Exactly fifty bitcoin. - pub const FIFTY_BTC: Self = SignedAmount::from_int_btc_const(50); + pub const FIFTY_BTC: Self = SignedAmount::from_btc_i16(50); /// The maximum value allowed as an amount. Useful for sanity checking. pub const MAX_MONEY: Self = SignedAmount::from_sat_unchecked(21_000_000 * 100_000_000); /// The minimum value of an amount. @@ -136,13 +136,13 @@ impl SignedAmount { /// Converts from a value expressing a whole number of bitcoin to a [`SignedAmount`]. #[allow(clippy::missing_panics_doc)] pub fn from_int_btc>(whole_bitcoin: T) -> SignedAmount { - SignedAmount::from_int_btc_const(whole_bitcoin.into()) + SignedAmount::from_btc_i16(whole_bitcoin.into()) } /// Converts from a value expressing a whole number of bitcoin to a [`SignedAmount`] /// in const context. #[allow(clippy::missing_panics_doc)] - pub const fn from_int_btc_const(whole_bitcoin: i16) -> SignedAmount { + pub const fn from_btc_i16(whole_bitcoin: i16) -> SignedAmount { let btc = whole_bitcoin as i64; // Can't call `into` in const context. let sats = btc * 100_000_000; diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 5fc495588..60676f605 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -102,8 +102,10 @@ fn from_str_zero_without_denomination() { #[test] fn from_int_btc() { - let amt = Amount::from_int_btc_const(2); + let amt = Amount::from_btc_u16(2); assert_eq!(sat(200_000_000), amt); + let amt = SignedAmount::from_btc_i16(-2); + assert_eq!(ssat(-200_000_000), amt); } #[test] diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index ab1abb9c3..9f6215d08 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -75,9 +75,9 @@ impl Amount { /// Exactly one satoshi. pub const ONE_SAT: Self = Amount::from_sat_unchecked(1); /// Exactly one bitcoin. - pub const ONE_BTC: Self = Amount::from_int_btc_const(1); + pub const ONE_BTC: Self = Amount::from_btc_u16(1); /// Exactly fifty bitcoin. - pub const FIFTY_BTC: Self = Amount::from_int_btc_const(50); + pub const FIFTY_BTC: Self = Amount::from_btc_u16(50); /// The maximum value allowed as an amount. Useful for sanity checking. pub const MAX_MONEY: Self = Amount::from_sat_unchecked(21_000_000 * 100_000_000); /// The minimum value of an amount. @@ -134,13 +134,13 @@ impl Amount { /// Converts from a value expressing a whole number of bitcoin to an [`Amount`]. #[allow(clippy::missing_panics_doc)] pub fn from_int_btc>(whole_bitcoin: T) -> Amount { - Amount::from_int_btc_const(whole_bitcoin.into()) + Amount::from_btc_u16(whole_bitcoin.into()) } /// Converts from a value expressing a whole number of bitcoin to an [`Amount`] /// in const context. #[allow(clippy::missing_panics_doc)] - pub const fn from_int_btc_const(whole_bitcoin: u16) -> Amount { + pub const fn from_btc_u16(whole_bitcoin: u16) -> Amount { let btc = whole_bitcoin as u64; // Can't call `into` in const context. let sats = btc * 100_000_000; From beaa2db7e523ca1ac2bb6c5eaf22f8c42db67f5c Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Mar 2025 13:36:15 +0000 Subject: [PATCH 3/8] amount: add from_sat_i32 and from_sat_u32 methods for small constants We have a ton of calls to `from_sat_unchecked` for small constants which were clearly in range, e.g. in fee.rs. Add a new constfn for these cases. Don't bother making a generic Into/Into variant because there isn't an obvious name for it. There are 7 instances where we're using this method with values that are out of range, which we leave as from_sat_unchecked for now. --- bitcoin/examples/ecdsa-psbt-simple.rs | 8 ++++---- bitcoin/examples/sighash.rs | 4 ++-- bitcoin/examples/sign-tx-segwit-v0.rs | 6 +++--- bitcoin/examples/sign-tx-taproot.rs | 6 +++--- bitcoin/examples/taproot-psbt-simple.rs | 8 ++++---- bitcoin/examples/taproot-psbt.rs | 2 +- bitcoin/src/blockdata/script/tests.rs | 10 +++++----- bitcoin/src/crypto/sighash.rs | 6 +++--- bitcoin/src/psbt/mod.rs | 26 ++++++++++++------------- units/src/amount/signed.rs | 12 ++++++++++-- units/src/amount/unsigned.rs | 12 ++++++++++-- units/src/fee.rs | 16 +++++++-------- 12 files changed, 66 insertions(+), 50 deletions(-) diff --git a/bitcoin/examples/ecdsa-psbt-simple.rs b/bitcoin/examples/ecdsa-psbt-simple.rs index 024f71c56..ac5d84b43 100644 --- a/bitcoin/examples/ecdsa-psbt-simple.rs +++ b/bitcoin/examples/ecdsa-psbt-simple.rs @@ -47,12 +47,12 @@ const BIP84_DERIVATION_PATH: &str = "m/84'/0'/0'"; const MASTER_FINGERPRINT: &str = "9680603f"; // The dummy UTXO amounts we are spending. -const DUMMY_UTXO_AMOUNT_INPUT_1: Amount = Amount::from_sat_unchecked(20_000_000); -const DUMMY_UTXO_AMOUNT_INPUT_2: Amount = Amount::from_sat_unchecked(10_000_000); +const DUMMY_UTXO_AMOUNT_INPUT_1: Amount = Amount::from_sat_u32(20_000_000); +const DUMMY_UTXO_AMOUNT_INPUT_2: Amount = Amount::from_sat_u32(10_000_000); // The amounts we are sending to someone, and receiving back as change. -const SPEND_AMOUNT: Amount = Amount::from_sat_unchecked(25_000_000); -const CHANGE_AMOUNT: Amount = Amount::from_sat_unchecked(4_990_000); // 10_000 sat fee. +const SPEND_AMOUNT: Amount = Amount::from_sat_u32(25_000_000); +const CHANGE_AMOUNT: Amount = Amount::from_sat_u32(4_990_000); // 10_000 sat fee. // Derive the external address xpriv. fn get_external_address_xpriv( diff --git a/bitcoin/examples/sighash.rs b/bitcoin/examples/sighash.rs index 0878ea439..1055a52f7 100644 --- a/bitcoin/examples/sighash.rs +++ b/bitcoin/examples/sighash.rs @@ -148,7 +148,7 @@ fn sighash_p2wpkh() { let inp_idx = 0; //output value from the referenced vout:0 from the referenced tx: //bitcoin-cli getrawtransaction 752d675b9cc0bd14e0bd23969effee0005ad6d7e550dcc832f0216c7ffd4e15c 3 - let ref_out_value = Amount::from_sat_unchecked(200000000); + let ref_out_value = Amount::from_sat_u32(200000000); println!("\nsighash_p2wpkh:"); compute_sighash_p2wpkh(&raw_tx, inp_idx, ref_out_value); @@ -178,7 +178,7 @@ fn sighash_p2wsh_multisig_2x2() { //For the witness transaction sighash computation, we need its referenced output's value from the original transaction: //bitcoin-cli getrawtransaction 2845399a8cd7a52733f9f9d0e0b8b6c5d1c88aea4cee09f8d8fa762912b49e1b 3 //we need vout 0 value in sats: - let ref_out_value = Amount::from_sat_unchecked(968240); + let ref_out_value = Amount::from_sat_u32(968240); println!("\nsighash_p2wsh_multisig_2x2:"); compute_sighash_p2wsh(&raw_tx, 0, ref_out_value); diff --git a/bitcoin/examples/sign-tx-segwit-v0.rs b/bitcoin/examples/sign-tx-segwit-v0.rs index a30c02d93..b163a24b0 100644 --- a/bitcoin/examples/sign-tx-segwit-v0.rs +++ b/bitcoin/examples/sign-tx-segwit-v0.rs @@ -13,9 +13,9 @@ use bitcoin::{ Txid, Witness, }; -const DUMMY_UTXO_AMOUNT: Amount = Amount::from_sat_unchecked(20_000_000); -const SPEND_AMOUNT: Amount = Amount::from_sat_unchecked(5_000_000); -const CHANGE_AMOUNT: Amount = Amount::from_sat_unchecked(14_999_000); // 1000 sat fee. +const DUMMY_UTXO_AMOUNT: Amount = Amount::from_sat_u32(20_000_000); +const SPEND_AMOUNT: Amount = Amount::from_sat_u32(5_000_000); +const CHANGE_AMOUNT: Amount = Amount::from_sat_u32(14_999_000); // 1000 sat fee. fn main() { let secp = Secp256k1::new(); diff --git a/bitcoin/examples/sign-tx-taproot.rs b/bitcoin/examples/sign-tx-taproot.rs index df7532cbd..06a3fab16 100644 --- a/bitcoin/examples/sign-tx-taproot.rs +++ b/bitcoin/examples/sign-tx-taproot.rs @@ -13,9 +13,9 @@ use bitcoin::{ Txid, Witness, }; -const DUMMY_UTXO_AMOUNT: Amount = Amount::from_sat_unchecked(20_000_000); -const SPEND_AMOUNT: Amount = Amount::from_sat_unchecked(5_000_000); -const CHANGE_AMOUNT: Amount = Amount::from_sat_unchecked(14_999_000); // 1000 sat fee. +const DUMMY_UTXO_AMOUNT: Amount = Amount::from_sat_u32(20_000_000); +const SPEND_AMOUNT: Amount = Amount::from_sat_u32(5_000_000); +const CHANGE_AMOUNT: Amount = Amount::from_sat_u32(14_999_000); // 1000 sat fee. fn main() { let secp = Secp256k1::new(); diff --git a/bitcoin/examples/taproot-psbt-simple.rs b/bitcoin/examples/taproot-psbt-simple.rs index 8671032c1..8181573a2 100644 --- a/bitcoin/examples/taproot-psbt-simple.rs +++ b/bitcoin/examples/taproot-psbt-simple.rs @@ -45,12 +45,12 @@ const BIP86_DERIVATION_PATH: &str = "m/86'/0'/0'"; const MASTER_FINGERPRINT: &str = "9680603f"; // The dummy UTXO amounts we are spending. -const DUMMY_UTXO_AMOUNT_INPUT_1: Amount = Amount::from_sat_unchecked(20_000_000); -const DUMMY_UTXO_AMOUNT_INPUT_2: Amount = Amount::from_sat_unchecked(10_000_000); +const DUMMY_UTXO_AMOUNT_INPUT_1: Amount = Amount::from_sat_u32(20_000_000); +const DUMMY_UTXO_AMOUNT_INPUT_2: Amount = Amount::from_sat_u32(10_000_000); // The amounts we are sending to someone, and receiving back as change. -const SPEND_AMOUNT: Amount = Amount::from_sat_unchecked(25_000_000); -const CHANGE_AMOUNT: Amount = Amount::from_sat_unchecked(4_990_000); // 10_000 sat fee. +const SPEND_AMOUNT: Amount = Amount::from_sat_u32(25_000_000); +const CHANGE_AMOUNT: Amount = Amount::from_sat_u32(4_990_000); // 10_000 sat fee. // Derive the external address xpriv. fn get_external_address_xpriv( diff --git a/bitcoin/examples/taproot-psbt.rs b/bitcoin/examples/taproot-psbt.rs index c5bb6d34e..92af85156 100644 --- a/bitcoin/examples/taproot-psbt.rs +++ b/bitcoin/examples/taproot-psbt.rs @@ -40,7 +40,7 @@ const UTXO_SCRIPT_PUBKEY: &str = "5120be27fa8b1f5278faf82cab8da23e8761f8f9bd5d5ebebbb37e0e12a70d92dd16"; const UTXO_PUBKEY: &str = "a6ac32163539c16b6b5dbbca01b725b8e8acaa5f821ba42c80e7940062140d19"; const UTXO_MASTER_FINGERPRINT: &str = "e61b318f"; -const ABSOLUTE_FEES: Amount = Amount::from_sat_unchecked(1_000); +const ABSOLUTE_FEES: Amount = Amount::from_sat_u32(1_000); // UTXO_1 will be used for spending example 1 const UTXO_1: P2trUtxo = P2trUtxo { diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs index 3f4d56aea..27dcd32ea 100644 --- a/bitcoin/src/blockdata/script/tests.rs +++ b/bitcoin/src/blockdata/script/tests.rs @@ -675,7 +675,7 @@ fn bitcoinconsensus() { let spent_bytes = hex!("0020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d"); let spent = Script::from_bytes(&spent_bytes); let spending = hex!("010000000001011f97548fbbe7a0db7588a66e18d803d0089315aa7d4cc28360b6ec50ef36718a0100000000ffffffff02df1776000000000017a9146c002a686959067f4866b8fb493ad7970290ab728757d29f0000000000220020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d04004730440220565d170eed95ff95027a69b313758450ba84a01224e1f7f130dda46e94d13f8602207bdd20e307f062594022f12ed5017bbf4a055a06aea91c10110a0e3bb23117fc014730440220647d2dc5b15f60bc37dc42618a370b2a1490293f9e5c8464f53ec4fe1dfe067302203598773895b4b16d37485cbe21b337f4e4b650739880098c592553add7dd4355016952210375e00eb72e29da82b89367947f29ef34afb75e8654f6ea368e0acdfd92976b7c2103a1b26313f430c4b15bb1fdce663207659d8cac749a0e53d70eff01874496feff2103c96d495bfdd5ba4145e3e046fee45e84a8a48ad05bd8dbb395c011a32cf9f88053ae00000000"); - spent.verify(0, Amount::from_sat_unchecked(18393430), &spending).unwrap(); + spent.verify(0, Amount::from_sat_u32(18393430), &spending).unwrap(); } #[test] @@ -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(), Some(Amount::from_sat_unchecked(294))); + assert_eq!(script_p2wpkh.minimal_non_dust(), Some(Amount::from_sat_u32(294))); assert_eq!( script_p2wpkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)), - Some(Amount::from_sat_unchecked(588)) + Some(Amount::from_sat_u32(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(), Some(Amount::from_sat_unchecked(546))); + assert_eq!(script_p2pkh.minimal_non_dust(), Some(Amount::from_sat_u32(546))); assert_eq!( script_p2pkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)), - Some(Amount::from_sat_unchecked(1092)) + Some(Amount::from_sat_u32(1092)) ); } diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index 0fbe05a7d..8ef0047d4 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -2089,7 +2089,7 @@ mod tests { ).unwrap(); let spk = ScriptBuf::from_hex("00141d0f172a0ecb48aee1be1f2687d2963ae33f71a1").unwrap(); - let value = Amount::from_sat_unchecked(600_000_000); + let value = Amount::from_sat_u32(600_000_000); let mut cache = SighashCache::new(&tx); assert_eq!( @@ -2130,7 +2130,7 @@ mod tests { let redeem_script = ScriptBuf::from_hex("001479091972186c449eb1ded22b78e40d009bdf0089").unwrap(); - let value = Amount::from_sat_unchecked(1_000_000_000); + let value = Amount::from_sat_u32(1_000_000_000); let mut cache = SighashCache::new(&tx); assert_eq!( @@ -2180,7 +2180,7 @@ mod tests { ) .unwrap(); - let value = Amount::from_sat_unchecked(987_654_321); + let value = Amount::from_sat_u32(987_654_321); (tx, witness_script, value) } diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index ffba04ea9..6de738779 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -1429,14 +1429,14 @@ mod tests { }], output: vec![ TxOut { - value: Amount::from_sat_unchecked(99_999_699), + value: Amount::from_sat_u32(99_999_699), script_pubkey: ScriptBuf::from_hex( "76a914d0c59903c5bac2868760e90fd521a4665aa7652088ac", ) .unwrap(), }, TxOut { - value: Amount::from_sat_unchecked(100_000_000), + value: Amount::from_sat_u32(100_000_000), script_pubkey: ScriptBuf::from_hex( "a9143545e6e33b832c47050f24d3eeb93c9c03948bc787", ) @@ -1678,11 +1678,11 @@ mod tests { ], output: vec![ TxOut { - value: Amount::from_sat_unchecked(99_999_699), + value: Amount::from_sat_u32(99_999_699), script_pubkey: ScriptBuf::from_hex("76a914d0c59903c5bac2868760e90fd521a4665aa7652088ac").unwrap(), }, TxOut { - value: Amount::from_sat_unchecked(100_000_000), + value: Amount::from_sat_u32(100_000_000), script_pubkey: ScriptBuf::from_hex("a9143545e6e33b832c47050f24d3eeb93c9c03948bc787").unwrap(), }, ], @@ -1725,7 +1725,7 @@ mod tests { ], output: vec![ TxOut { - value: Amount::from_sat_unchecked(200_000_000), + value: Amount::from_sat_u32(200_000_000), script_pubkey: ScriptBuf::from_hex("76a91485cff1097fd9e008bb34af709c62197b38978a4888ac").unwrap(), }, TxOut { @@ -2011,11 +2011,11 @@ mod tests { ], output: vec![ TxOut { - value: Amount::from_sat_unchecked(99_999_699), + value: Amount::from_sat_u32(99_999_699), script_pubkey: ScriptBuf::from_hex("76a914d0c59903c5bac2868760e90fd521a4665aa7652088ac").unwrap(), }, TxOut { - value: Amount::from_sat_unchecked(100_000_000), + value: Amount::from_sat_u32(100_000_000), script_pubkey: ScriptBuf::from_hex("a9143545e6e33b832c47050f24d3eeb93c9c03948bc787").unwrap(), }, ], @@ -2058,7 +2058,7 @@ mod tests { ], output: vec![ TxOut { - value: Amount::from_sat_unchecked(200_000_000), + value: Amount::from_sat_u32(200_000_000), script_pubkey: ScriptBuf::from_hex("76a91485cff1097fd9e008bb34af709c62197b38978a4888ac").unwrap(), }, TxOut { @@ -2171,9 +2171,9 @@ mod tests { #[test] fn fee() { - let output_0_val = Amount::from_sat_unchecked(99_999_699); - let output_1_val = Amount::from_sat_unchecked(100_000_000); - let prev_output_val = Amount::from_sat_unchecked(200_000_000); + let output_0_val = Amount::from_sat_u32(99_999_699); + let output_1_val = Amount::from_sat_u32(100_000_000); + let prev_output_val = Amount::from_sat_u32(200_000_000); let t = Psbt { unsigned_tx: Transaction { @@ -2296,7 +2296,7 @@ mod tests { // First input we can spend. See comment above on key_map for why we use defaults here. let txout_wpkh = TxOut { - value: Amount::from_sat_unchecked(10), + value: Amount::from_sat_u32(10), script_pubkey: ScriptBuf::new_p2wpkh(pk.wpubkey_hash().unwrap()), }; psbt.inputs[0].witness_utxo = Some(txout_wpkh); @@ -2308,7 +2308,7 @@ mod tests { // Second input is unspendable by us e.g., from another wallet that supports future upgrades. let unknown_prog = WitnessProgram::new(WitnessVersion::V4, &[0xaa; 34]).unwrap(); let txout_unknown_future = TxOut { - value: Amount::from_sat_unchecked(10), + value: Amount::from_sat_u32(10), script_pubkey: ScriptBuf::new_witness_program(&unknown_prog), }; psbt.inputs[1].witness_utxo = Some(txout_unknown_future); diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 463982895..cf633d4a4 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -57,6 +57,14 @@ mod encapsulate { /// See [`Self::MIN`] and [`Self::MAX`]. pub const fn from_sat_unchecked(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) } + /// 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) -> SignedAmount { + SignedAmount(satoshi as i64) // cannot use i64::from in a constfn + } + /// Gets the number of satoshis in this [`SignedAmount`]. /// /// # Examples @@ -73,9 +81,9 @@ pub use encapsulate::SignedAmount; impl SignedAmount { /// The zero amount. - pub const ZERO: Self = SignedAmount::from_sat_unchecked(0); + pub const ZERO: Self = SignedAmount::from_sat_i32(0); /// Exactly one satoshi. - pub const ONE_SAT: Self = SignedAmount::from_sat_unchecked(1); + pub const ONE_SAT: Self = SignedAmount::from_sat_i32(1); /// Exactly one bitcoin. pub const ONE_BTC: Self = SignedAmount::from_btc_i16(1); /// Exactly fifty bitcoin. diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 9f6215d08..b4191fe89 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -55,6 +55,14 @@ mod encapsulate { /// Caller to guarantee that `satoshi` is within valid range. See [`Self::MAX`]. pub const fn from_sat_unchecked(satoshi: u64) -> Amount { Self(satoshi) } + /// 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) -> Amount { + Amount(satoshi as u64) // cannot use u64::from in a constfn + } + /// Gets the number of satoshis in this [`Amount`]. /// /// # Examples @@ -71,9 +79,9 @@ pub use encapsulate::Amount; impl Amount { /// The zero amount. - pub const ZERO: Self = Amount::from_sat_unchecked(0); + pub const ZERO: Self = Amount::from_sat_u32(0); /// Exactly one satoshi. - pub const ONE_SAT: Self = Amount::from_sat_unchecked(1); + pub const ONE_SAT: Self = Amount::from_sat_u32(1); /// Exactly one bitcoin. pub const ONE_BTC: Self = Amount::from_btc_u16(1); /// Exactly fifty bitcoin. diff --git a/units/src/fee.rs b/units/src/fee.rs index 73c53b817..21d223c9c 100644 --- a/units/src/fee.rs +++ b/units/src/fee.rs @@ -216,7 +216,7 @@ mod tests { #[test] fn fee_rate_div_by_weight() { - let fee_rate = Amount::from_sat_unchecked(329) / Weight::from_wu(381); + let fee_rate = Amount::from_sat_u32(329) / Weight::from_wu(381); assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(863)); } @@ -227,7 +227,7 @@ mod tests { let fee_rate = FeeRate::from_sat_per_vb(2).unwrap(); let weight = Weight::from_vb(3).unwrap(); - assert_eq!(fee_rate.to_fee(weight).unwrap(), Amount::from_sat_unchecked(6)); + assert_eq!(fee_rate.to_fee(weight).unwrap(), Amount::from_sat_u32(6)); } #[test] @@ -237,7 +237,7 @@ mod tests { .unwrap() .checked_mul_by_weight(weight) .expect("expected Amount"); - assert_eq!(Amount::from_sat_unchecked(100), fee); + assert_eq!(Amount::from_sat_u32(100), fee); let fee = FeeRate::from_sat_per_kwu(10).checked_mul_by_weight(Weight::MAX); assert!(fee.is_none()); @@ -245,14 +245,14 @@ mod tests { let weight = Weight::from_vb(3).unwrap(); let fee_rate = FeeRate::from_sat_per_vb(3).unwrap(); let fee = fee_rate.checked_mul_by_weight(weight).unwrap(); - assert_eq!(Amount::from_sat_unchecked(9), fee); + assert_eq!(Amount::from_sat_u32(9), fee); let weight = Weight::from_wu(381); let fee_rate = FeeRate::from_sat_per_kwu(864); let fee = weight.checked_mul_by_fee_rate(fee_rate).unwrap(); // 381 * 0.864 yields 329.18. // The result is then rounded up to 330. - assert_eq!(fee, Amount::from_sat_unchecked(330)); + assert_eq!(fee, Amount::from_sat_u32(330)); } #[test] @@ -260,7 +260,7 @@ mod tests { fn multiply() { let two = FeeRate::from_sat_per_vb(2).unwrap(); let three = Weight::from_vb(3).unwrap(); - let six = Amount::from_sat_unchecked(6); + let six = Amount::from_sat_u32(6); assert_eq!(two * three, six.into()); @@ -274,7 +274,7 @@ mod tests { #[allow(clippy::op_ref)] fn amount_div_by_fee_rate() { // Test exact division - let amount = Amount::from_sat_unchecked(1000); + let amount = Amount::from_sat_u32(1000); let fee_rate = FeeRate::from_sat_per_kwu(2); let weight = amount / fee_rate; assert_eq!(weight, Weight::from_wu(500_000)); @@ -288,7 +288,7 @@ mod tests { assert_eq!(weight_ref3, Weight::from_wu(500_000)); // Test truncation behavior - let amount = Amount::from_sat_unchecked(1000); + let amount = Amount::from_sat_u32(1000); let fee_rate = FeeRate::from_sat_per_kwu(3); let weight = amount / fee_rate; // 1000 * 1000 = 1,000,000 msats From 05c8b043ff7b4389ef0dc899091033347a44c420 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Mar 2025 14:47:44 +0000 Subject: [PATCH 4/8] tests: replace Amount::from_sat_unchecked with from_sat.unwrap There are only 7 instances of this so just call .unwrap() on each one. --- bitcoin/src/psbt/mod.rs | 10 +++++----- bitcoin/tests/serde.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index 6de738779..1f582cfa1 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -1502,7 +1502,7 @@ mod tests { )]), }], output: vec![TxOut { - value: Amount::from_sat_unchecked(190_303_501_938), + value: Amount::from_sat(190_303_501_938).unwrap(), script_pubkey: ScriptBuf::from_hex( "a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587", ) @@ -1553,7 +1553,7 @@ mod tests { Input { non_witness_utxo: Some(tx), witness_utxo: Some(TxOut { - value: Amount::from_sat_unchecked(190_303_501_938), + value: Amount::from_sat(190_303_501_938).unwrap(), script_pubkey: ScriptBuf::from_hex("a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587").unwrap(), }), sighash_type: Some("SIGHASH_SINGLE|SIGHASH_ANYONECANPAY".parse::().unwrap()), @@ -1729,7 +1729,7 @@ mod tests { script_pubkey: ScriptBuf::from_hex("76a91485cff1097fd9e008bb34af709c62197b38978a4888ac").unwrap(), }, TxOut { - value: Amount::from_sat_unchecked(190_303_501_938), + value: Amount::from_sat(190_303_501_938).unwrap(), script_pubkey: ScriptBuf::from_hex("a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587").unwrap(), }, ], @@ -2062,7 +2062,7 @@ mod tests { script_pubkey: ScriptBuf::from_hex("76a91485cff1097fd9e008bb34af709c62197b38978a4888ac").unwrap(), }, TxOut { - value: Amount::from_sat_unchecked(190_303_501_938), + value: Amount::from_sat(190_303_501_938).unwrap(), script_pubkey: ScriptBuf::from_hex("a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587").unwrap(), }, ], @@ -2234,7 +2234,7 @@ mod tests { script_pubkey: ScriptBuf::new() }, TxOut { - value: Amount::from_sat_unchecked(190_303_501_938), + value: Amount::from_sat(190_303_501_938).unwrap(), script_pubkey: ScriptBuf::new() }, ], diff --git a/bitcoin/tests/serde.rs b/bitcoin/tests/serde.rs index 66ffc63b0..25c89306c 100644 --- a/bitcoin/tests/serde.rs +++ b/bitcoin/tests/serde.rs @@ -235,7 +235,7 @@ fn serde_regression_psbt() { .unwrap()]), }], output: vec![TxOut { - value: Amount::from_sat_unchecked(190_303_501_938), + value: Amount::from_sat(190_303_501_938).unwrap(), script_pubkey: ScriptBuf::from_hex("a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587") .unwrap(), }], @@ -282,7 +282,7 @@ fn serde_regression_psbt() { inputs: vec![Input { non_witness_utxo: Some(tx), witness_utxo: Some(TxOut { - value: Amount::from_sat_unchecked(190_303_501_938), + value: Amount::from_sat(190_303_501_938).unwrap(), script_pubkey: ScriptBuf::from_hex("a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587").unwrap(), }), sighash_type: Some(PsbtSighashType::from("SIGHASH_SINGLE|SIGHASH_ANYONECANPAY".parse::().unwrap())), From 3370c14d745423cf2fb5ba01e08eee55d9bf31d4 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Mar 2025 13:38:36 +0000 Subject: [PATCH 5/8] amount: stop using from_sat_unchecked in tests There is no need for this. It's a 2-line diff to change it. --- units/src/amount/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 60676f605..127e7566c 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -648,9 +648,9 @@ 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_unchecked; + let ua_sat = |n| Amount::from_sat(n).unwrap(); let sa_str = SignedAmount::from_str_in; - let sa_sat = SignedAmount::from_sat_unchecked; + let sa_sat = |n| SignedAmount::from_sat(n).unwrap(); assert_eq!("0.5", ua_sat(50).to_string_in(D::Bit)); assert_eq!("-0.5", sa_sat(-50).to_string_in(D::Bit)); From 004d07318465c73da9f302e13cbd50cfbe6fda35 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Mar 2025 14:26:20 +0000 Subject: [PATCH 6/8] amount: return i64 from parse_signed_to_satoshi This private function is used for string-parsing an amount. It returns a sign boolean and a u64, but its surrounding logic can be simplified if it just returns a i64 (which is OK now since the range of `Amount` fits into the range of i64). Along the way we eliminate some instances of from_sat_unchecked. Annoyingly we still need the bool to distinguish -0 from +0; when parsing Amount we reject -0 (and we have tests for this). This causes a couple error constructors to no longer be used outside of unit tests. May be worth looking at whether we need them, or whether we should be using them in parse_signed_to_satoshi. --- units/src/amount/error.rs | 2 ++ units/src/amount/mod.rs | 21 ++++++++++++++------- units/src/amount/signed.rs | 16 ++++------------ units/src/amount/unsigned.rs | 11 +++-------- 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/units/src/amount/error.rs b/units/src/amount/error.rs index 62e03a306..992d7e543 100644 --- a/units/src/amount/error.rs +++ b/units/src/amount/error.rs @@ -182,8 +182,10 @@ impl OutOfRangeError { /// Returns true if the input value was smaller than the minimum allowed value. pub fn is_below_min(self) -> bool { !self.is_greater_than_max } + #[cfg(test)] pub(crate) fn too_big(is_signed: bool) -> Self { Self { is_signed, is_greater_than_max: true } } + #[cfg(test)] pub(crate) fn too_small() -> Self { Self { // implied - negative() is used for the other diff --git a/units/src/amount/mod.rs b/units/src/amount/mod.rs index 98e2137a8..8928cea9d 100644 --- a/units/src/amount/mod.rs +++ b/units/src/amount/mod.rs @@ -202,10 +202,12 @@ const INPUT_STRING_LEN_LIMIT: usize = 50; /// Parses a decimal string in the given denomination into a satoshi value and a /// [`bool`] indicator for a negative amount. +/// +/// The `bool` is only needed to distinguish -0 from 0. fn parse_signed_to_satoshi( mut s: &str, denom: Denomination, -) -> Result<(bool, u64), InnerParseError> { +) -> Result<(bool, SignedAmount), InnerParseError> { if s.is_empty() { return Err(InnerParseError::MissingDigits(MissingDigitsError { kind: MissingDigitsKind::Empty, @@ -237,7 +239,7 @@ fn parse_signed_to_satoshi( let last_n = precision_diff.unsigned_abs().into(); if let Some(position) = is_too_precise(s, last_n) { match s.parse::() { - Ok(0) => return Ok((is_negative, 0)), + Ok(0) => return Ok((is_negative, SignedAmount::ZERO)), _ => return Err(InnerParseError::TooPrecise(TooPreciseError { position: position + usize::from(is_negative), @@ -252,14 +254,14 @@ fn parse_signed_to_satoshi( }; let mut decimals = None; - let mut value: u64 = 0; // as satoshis + let mut value: i64 = 0; // as satoshis for (i, c) in s.char_indices() { match c { '0'..='9' => { // Do `value = 10 * value + digit`, catching overflows. - match 10_u64.checked_mul(value) { + match 10_i64.checked_mul(value) { None => return Err(InnerParseError::Overflow { is_negative }), - Some(val) => match val.checked_add(u64::from(c as u8 - b'0')) { + Some(val) => match val.checked_add(i64::from(c as u8 - b'0')) { None => return Err(InnerParseError::Overflow { is_negative }), Some(val) => value = val, }, @@ -295,13 +297,18 @@ fn parse_signed_to_satoshi( // Decimally shift left by `max_decimals - decimals`. let scale_factor = max_decimals - decimals.unwrap_or(0); for _ in 0..scale_factor { - value = match 10_u64.checked_mul(value) { + value = match 10_i64.checked_mul(value) { Some(v) => v, None => return Err(InnerParseError::Overflow { is_negative }), }; } - Ok((is_negative, value)) + let mut ret = + SignedAmount::from_sat(value).map_err(|_| InnerParseError::Overflow { is_negative })?; + if is_negative { + ret = -ret; + } + Ok((is_negative, ret)) } #[derive(Debug)] diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index cf633d4a4..262106673 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -10,7 +10,7 @@ use core::{default, fmt}; #[cfg(feature = "arbitrary")] use arbitrary::{Arbitrary, Unstructured}; -use super::error::{ParseAmountErrorInner, ParseErrorInner}; +use super::error::ParseErrorInner; use super::{ parse_signed_to_satoshi, split_amount_and_denomination, Amount, Denomination, Display, DisplayStyle, OutOfRangeError, ParseAmountError, ParseError, @@ -169,17 +169,9 @@ impl SignedAmount { /// /// If the amount is too big (positive or negative) or too precise. pub fn from_str_in(s: &str, denom: Denomination) -> Result { - match parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(true))? { - // (negative, amount) - (false, sat) if sat > SignedAmount::MAX.to_sat() as u64 => Err(ParseAmountError( - ParseAmountErrorInner::OutOfRange(OutOfRangeError::too_big(true)), - )), - (false, sat) => Ok(SignedAmount::from_sat_unchecked(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::from_sat_unchecked(-(sat as i64))), // Cast ok, value in this arm does not wrap. - } + parse_signed_to_satoshi(s, denom) + .map(|(_, amount)| amount) + .map_err(|error| error.convert(true)) } /// Parses amounts with denomination suffix as produced by [`Self::to_string_with_denomination`] diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index b4191fe89..25533675f 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -167,19 +167,14 @@ impl Amount { /// /// If the amount is too precise, negative, or greater than 21,000,000. pub fn from_str_in(s: &str, denom: Denomination) -> Result { - let (negative, sats) = + let (is_neg, amount) = parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(false))?; - if negative { + if is_neg { return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange( OutOfRangeError::negative(), ))); } - if sats > Self::MAX.to_sat() { - return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange( - OutOfRangeError::too_big(false), - ))); - } - Ok(Self::from_sat_unchecked(sats)) + Self::try_from(amount).map_err(|e| ParseAmountError(ParseAmountErrorInner::OutOfRange(e))) } /// Parses amounts with denomination suffix as produced by [`Self::to_string_with_denomination`] From d0d7a156047b1567bc065abba1454fd52a3d32d6 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Mar 2025 14:28:18 +0000 Subject: [PATCH 7/8] amount: move MIN/MAX constants and constructors inside the privacy boundary It's conceptually a bit tortured to have an `Amount` type defined in a private module, with an _unchecked method allowing you to set values out of range, which needs to be used outside of the module to *define* the range and the constructors that check it. Move the constants and constructors inside the privacy module, where they can be written directly. This is easier to understand and eliminates a couple _unchecked calls. --- units/src/amount/signed.rs | 63 +++++++++++++++++++----------------- units/src/amount/unsigned.rs | 59 +++++++++++++++++---------------- 2 files changed, 64 insertions(+), 58 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 262106673..069eceee0 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -17,6 +17,8 @@ use super::{ }; mod encapsulate { + use super::OutOfRangeError; + /// A signed amount. /// /// The [`SignedAmount`] type can be used to express Bitcoin amounts that support arithmetic and @@ -50,6 +52,11 @@ mod encapsulate { pub struct SignedAmount(i64); impl SignedAmount { + /// The maximum value of an amount. + pub const MAX: Self = Self(21_000_000 * 100_000_000); + /// 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. /// /// Caller to guarantee that `satoshi` is within valid range. @@ -74,6 +81,31 @@ mod encapsulate { /// assert_eq!(SignedAmount::ONE_BTC.to_sat(), 100_000_000); /// ``` pub const fn to_sat(self) -> i64 { self.0 } + + /// Constructs a new [`SignedAmount`] from the given number of satoshis. + /// + /// # Errors + /// + /// If `satoshi` is outside of valid range (see [`Self::MAX_MONEY`]). + /// + /// # Examples + /// + /// ``` + /// # use bitcoin_units::{amount, SignedAmount}; + /// # let sat = -100_000; + /// let amount = SignedAmount::from_sat(sat)?; + /// assert_eq!(amount.to_sat(), sat); + /// # Ok::<_, amount::OutOfRangeError>(()) + /// ``` + pub const fn from_sat(satoshi: i64) -> Result { + if satoshi < Self::MIN.to_sat() { + Err(OutOfRangeError { is_signed: true, is_greater_than_max: false }) + } else if satoshi > Self::MAX_MONEY.to_sat() { + Err(OutOfRangeError { is_signed: true, is_greater_than_max: true }) + } else { + Ok(Self(satoshi)) + } + } } } #[doc(inline)] @@ -89,36 +121,7 @@ impl SignedAmount { /// Exactly fifty bitcoin. pub const FIFTY_BTC: Self = SignedAmount::from_btc_i16(50); /// The maximum value allowed as an amount. Useful for sanity checking. - 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::from_sat_unchecked(-21_000_000 * 100_000_000); - /// The maximum value of an amount. - pub const MAX: Self = SignedAmount::MAX_MONEY; - - /// Constructs a new [`SignedAmount`] from the given number of satoshis. - /// - /// # Errors - /// - /// If `satoshi` is outside of valid range (see [`Self::MAX_MONEY`]). - /// - /// # Examples - /// - /// ``` - /// # use bitcoin_units::{amount, SignedAmount}; - /// # let sat = -100_000; - /// let amount = SignedAmount::from_sat(sat)?; - /// assert_eq!(amount.to_sat(), sat); - /// # Ok::<_, amount::OutOfRangeError>(()) - /// ``` - pub const fn from_sat(satoshi: i64) -> Result { - if satoshi < Self::MIN.to_sat() { - Err(OutOfRangeError { is_signed: true, is_greater_than_max: false }) - } else if satoshi > Self::MAX_MONEY.to_sat() { - Err(OutOfRangeError { is_signed: true, is_greater_than_max: true }) - } else { - Ok(SignedAmount::from_sat_unchecked(satoshi)) - } - } + pub const MAX_MONEY: Self = Self::MAX; /// Converts from a value expressing a decimal number of bitcoin to a [`SignedAmount`]. /// diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 25533675f..d82c3f881 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -17,6 +17,8 @@ use super::{ }; mod encapsulate { + use super::OutOfRangeError; + /// An amount. /// /// The [`Amount`] type can be used to express Bitcoin amounts that support arithmetic and @@ -50,6 +52,11 @@ mod encapsulate { pub struct Amount(u64); impl Amount { + /// The maximum value of an amount. + pub const MAX: Self = Self(21_000_000 * 100_000_000); + /// 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. /// /// Caller to guarantee that `satoshi` is within valid range. See [`Self::MAX`]. @@ -72,6 +79,29 @@ mod encapsulate { /// assert_eq!(Amount::ONE_BTC.to_sat(), 100_000_000); /// ``` pub const fn to_sat(self) -> u64 { self.0 } + + /// Constructs a new [`Amount`] from the given number of satoshis. + /// + /// # Errors + /// + /// If `satoshi` is outside of valid range (greater than [`Self::MAX_MONEY`]). + /// + /// # Examples + /// + /// ``` + /// # use bitcoin_units::{amount, Amount}; + /// # let sat = 100_000; + /// let amount = Amount::from_sat(sat)?; + /// assert_eq!(amount.to_sat(), sat); + /// # Ok::<_, amount::OutOfRangeError>(()) + /// ``` + pub const fn from_sat(satoshi: u64) -> Result { + if satoshi > Self::MAX_MONEY.to_sat() { + Err(OutOfRangeError { is_signed: false, is_greater_than_max: true }) + } else { + Ok(Self(satoshi)) + } + } } } #[doc(inline)] @@ -87,37 +117,10 @@ impl Amount { /// Exactly fifty bitcoin. pub const FIFTY_BTC: Self = Amount::from_btc_u16(50); /// The maximum value allowed as an amount. Useful for sanity checking. - 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. - pub const MAX: Self = Amount::MAX_MONEY; + pub const MAX_MONEY: Self = Amount::MAX; /// 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`] from the given number of satoshis. - /// - /// # Errors - /// - /// If `satoshi` is outside of valid range (greater than [`Self::MAX_MONEY`]). - /// - /// # Examples - /// - /// ``` - /// # use bitcoin_units::{amount, Amount}; - /// # let sat = 100_000; - /// let amount = Amount::from_sat(sat)?; - /// assert_eq!(amount.to_sat(), sat); - /// # Ok::<_, amount::OutOfRangeError>(()) - /// ``` - pub const fn from_sat(satoshi: u64) -> Result { - if satoshi > Self::MAX_MONEY.to_sat() { - Err(OutOfRangeError { is_signed: false, is_greater_than_max: true }) - } else { - Ok(Self::from_sat_unchecked(satoshi)) - } - } - /// Converts from a value expressing a decimal number of bitcoin to an [`Amount`]. /// /// # Errors From 2958521117e6ec3db98ec42c490b3dfea60f7ab3 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 18 Mar 2025 14:34:44 +0000 Subject: [PATCH 8/8] amount: remove from_sat_unchecked Turns out we don't even need this. It is only used in one place now and we can just stick an .expect there. --- units/src/amount/signed.rs | 7 ------- units/src/amount/unsigned.rs | 9 +++------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 069eceee0..cc16fb8ee 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -57,13 +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. - /// - /// Caller to guarantee that `satoshi` is within valid range. - /// - /// See [`Self::MIN`] and [`Self::MAX`]. - pub const fn from_sat_unchecked(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) } - /// 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 diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index d82c3f881..1ff3637b6 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -57,11 +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. - /// - /// Caller to guarantee that `satoshi` is within valid range. See [`Self::MAX`]. - pub const fn from_sat_unchecked(satoshi: u64) -> Amount { Self(satoshi) } - /// 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 @@ -398,8 +393,10 @@ impl Amount { /// Converts to a signed amount. #[rustfmt::skip] // Moves code comments to the wrong line. + #[allow(clippy::missing_panics_doc)] pub fn to_signed(self) -> SignedAmount { - SignedAmount::from_sat_unchecked(self.to_sat() as i64) // Cast ok, signed amount and amount share positive range. + SignedAmount::from_sat(self.to_sat() as i64) // Cast ok, signed amount and amount share positive range. + .expect("range of Amount is within range of SignedAmount") } }