From 8e16a4825277934491d81cb1356940bdcc0534a4 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 20 Jan 2025 08:37:36 +1100 Subject: [PATCH 1/9] Run the formatter Manually run `just fmt` - no other changes. --- bitcoin/src/blockdata/constants.rs | 5 +---- units/src/amount/tests.rs | 30 ++++++------------------------ units/src/fee.rs | 4 +--- 3 files changed, 8 insertions(+), 31 deletions(-) diff --git a/bitcoin/src/blockdata/constants.rs b/bitcoin/src/blockdata/constants.rs index e1f15030c..ad8e96fdf 100644 --- a/bitcoin/src/blockdata/constants.rs +++ b/bitcoin/src/blockdata/constants.rs @@ -112,10 +112,7 @@ fn bitcoin_genesis_tx(params: &Params) -> Transaction { witness: Witness::default(), }); - ret.output.push(TxOut { - value: Amount::FIFTY_BTC, - script_pubkey: out_script, - }); + ret.output.push(TxOut { value: Amount::FIFTY_BTC, script_pubkey: out_script }); // end ret diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 94b9a5462..a37971563 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -22,10 +22,7 @@ fn sanity_check() { let ssat = SignedAmount::from_sat; assert_eq!(ssat(-100).abs(), ssat(100)); - assert_eq!( - ssat(i64::MIN + 1).checked_abs().unwrap(), - ssat(i64::MAX) - ); + assert_eq!(ssat(i64::MIN + 1).checked_abs().unwrap(), ssat(i64::MAX)); assert_eq!(ssat(-100).signum(), -1); assert_eq!(ssat(0).signum(), 0); assert_eq!(ssat(100).signum(), 1); @@ -36,14 +33,8 @@ fn sanity_check() { #[cfg(feature = "alloc")] { - assert_eq!( - Amount::from_float_in(0_f64, Denomination::Bitcoin).unwrap(), - sat(0) - ); - assert_eq!( - Amount::from_float_in(2_f64, Denomination::Bitcoin).unwrap(), - sat(200_000_000) - ); + assert_eq!(Amount::from_float_in(0_f64, Denomination::Bitcoin).unwrap(), sat(0)); + assert_eq!(Amount::from_float_in(2_f64, Denomination::Bitcoin).unwrap(), sat(200_000_000)); assert!(Amount::from_float_in(-100_f64, Denomination::Bitcoin).is_err()); } } @@ -185,14 +176,8 @@ fn unchecked_arithmetic() { let sat = Amount::from_sat; let ssat = SignedAmount::from_sat; - assert_eq!( - ssat(10).unchecked_add(ssat(20)), - ssat(30) - ); - assert_eq!( - ssat(50).unchecked_sub(ssat(10)), - ssat(40) - ); + assert_eq!(ssat(10).unchecked_add(ssat(20)), ssat(30)); + assert_eq!(ssat(50).unchecked_sub(ssat(10)), ssat(40)); assert_eq!(sat(5).unchecked_add(sat(7)), sat(12)); assert_eq!(sat(10).unchecked_sub(sat(7)), sat(3)); } @@ -201,10 +186,7 @@ fn unchecked_arithmetic() { fn positive_sub() { let ssat = SignedAmount::from_sat; - assert_eq!( - ssat(10).positive_sub(ssat(7)).unwrap(), - ssat(3) - ); + assert_eq!(ssat(10).positive_sub(ssat(7)).unwrap(), ssat(3)); assert!(ssat(-10).positive_sub(ssat(7)).is_none()); assert!(ssat(10).positive_sub(ssat(-7)).is_none()); assert!(ssat(10).positive_sub(ssat(11)).is_none()); diff --git a/units/src/fee.rs b/units/src/fee.rs index a1475f9bd..1f0a535b4 100644 --- a/units/src/fee.rs +++ b/units/src/fee.rs @@ -188,9 +188,7 @@ impl ops::Div for Amount { /// This operation will panic if `fee_rate` is zero or the division results in overflow. /// /// Note: This uses floor division. For ceiling division use [`Amount::checked_div_by_fee_rate_ceil`]. - fn div(self, rhs: FeeRate) -> Self::Output { - self.checked_div_by_fee_rate_floor(rhs).unwrap() - } + fn div(self, rhs: FeeRate) -> Self::Output { self.checked_div_by_fee_rate_floor(rhs).unwrap() } } #[cfg(test)] From 154a4420fce5466273b492977d179b8f77b48a97 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 17 Jan 2025 10:24:45 +1100 Subject: [PATCH 2/9] Stop using FQP on Amount type Using `crate::Amount` adds no additional information - stop doing that. Internal change only. --- bitcoin/src/blockdata/script/borrowed.rs | 12 ++++++------ bitcoin/src/blockdata/script/tests.rs | 12 ++++++------ bitcoin/src/consensus_validation.rs | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs index b3a65ff96..14c32e314 100644 --- a/bitcoin/src/blockdata/script/borrowed.rs +++ b/bitcoin/src/blockdata/script/borrowed.rs @@ -15,7 +15,7 @@ use crate::opcodes::{self, Opcode}; use crate::policy::{DUST_RELAY_TX_FEE, MAX_OP_RETURN_RELAY}; use crate::prelude::{sink, DisplayHex, String, ToString}; use crate::taproot::{LeafVersion, TapLeafHash, TapLeafHashExt as _}; -use crate::FeeRate; +use crate::{Amount, FeeRate}; #[rustfmt::skip] // Keep public re-exports separate. #[doc(inline)] @@ -261,7 +261,7 @@ crate::internal_macros::define_extension_trait! { /// Returns the minimum value an output with this script should have in order to be /// broadcastable on today’s Bitcoin network. #[deprecated(since = "0.32.0", note = "use `minimal_non_dust` etc. instead")] - fn dust_value(&self) -> crate::Amount { self.minimal_non_dust() } + fn dust_value(&self) -> Amount { self.minimal_non_dust() } /// Returns the minimum value an output with this script should have in order to be /// broadcastable on today's Bitcoin network. @@ -272,7 +272,7 @@ crate::internal_macros::define_extension_trait! { /// To use a custom value, use [`minimal_non_dust_custom`]. /// /// [`minimal_non_dust_custom`]: Script::minimal_non_dust_custom - fn minimal_non_dust(&self) -> crate::Amount { + fn minimal_non_dust(&self) -> Amount { self.minimal_non_dust_internal(DUST_RELAY_TX_FEE.into()) } @@ -287,7 +287,7 @@ crate::internal_macros::define_extension_trait! { /// To use the default Bitcoin Core value, use [`minimal_non_dust`]. /// /// [`minimal_non_dust`]: Script::minimal_non_dust - fn minimal_non_dust_custom(&self, dust_relay_fee: FeeRate) -> crate::Amount { + fn minimal_non_dust_custom(&self, dust_relay_fee: FeeRate) -> Amount { self.minimal_non_dust_internal(dust_relay_fee.to_sat_per_kwu() * 4) } @@ -394,7 +394,7 @@ mod sealed { crate::internal_macros::define_extension_trait! { pub(crate) trait ScriptExtPriv impl for Script { - fn minimal_non_dust_internal(&self, dust_relay_fee: u64) -> crate::Amount { + fn minimal_non_dust_internal(&self, dust_relay_fee: u64) -> Amount { // This must never be lower than Bitcoin Core's GetDustThreshold() (as of v0.21) as it may // otherwise allow users to create transactions which likely can never be broadcast/confirmed. let sats = dust_relay_fee @@ -414,7 +414,7 @@ crate::internal_macros::define_extension_trait! { // Note: We ensure the division happens at the end, since Core performs the division at the end. // This will make sure none of the implicit floor operations mess with the value. - crate::Amount::from_sat(sats) + Amount::from_sat(sats) } fn count_sigops_internal(&self, accurate: bool) -> usize { diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs index b14ec86f4..4085085d2 100644 --- a/bitcoin/src/blockdata/script/tests.rs +++ b/bitcoin/src/blockdata/script/tests.rs @@ -9,7 +9,7 @@ use crate::address::script_pubkey::{ }; use crate::consensus::encode::{deserialize, serialize}; use crate::crypto::key::{PublicKey, XOnlyPublicKey}; -use crate::FeeRate; +use crate::{Amount, FeeRate}; #[test] #[rustfmt::skip] @@ -676,7 +676,7 @@ fn bitcoinconsensus() { let spent_bytes = hex!("0020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d"); let spent = Script::from_bytes(&spent_bytes); let spending = hex!("010000000001011f97548fbbe7a0db7588a66e18d803d0089315aa7d4cc28360b6ec50ef36718a0100000000ffffffff02df1776000000000017a9146c002a686959067f4866b8fb493ad7970290ab728757d29f0000000000220020701a8d401c84fb13e6baf169d59684e17abd9fa216c8cc5b9fc63d622ff8c58d04004730440220565d170eed95ff95027a69b313758450ba84a01224e1f7f130dda46e94d13f8602207bdd20e307f062594022f12ed5017bbf4a055a06aea91c10110a0e3bb23117fc014730440220647d2dc5b15f60bc37dc42618a370b2a1490293f9e5c8464f53ec4fe1dfe067302203598773895b4b16d37485cbe21b337f4e4b650739880098c592553add7dd4355016952210375e00eb72e29da82b89367947f29ef34afb75e8654f6ea368e0acdfd92976b7c2103a1b26313f430c4b15bb1fdce663207659d8cac749a0e53d70eff01874496feff2103c96d495bfdd5ba4145e3e046fee45e84a8a48ad05bd8dbb395c011a32cf9f88053ae00000000"); - spent.verify(0, crate::Amount::from_sat_unchecked(18393430), &spending).unwrap(); + spent.verify(0, Amount::from_sat_unchecked(18393430), &spending).unwrap(); } #[test] @@ -685,10 +685,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(), crate::Amount::from_sat_unchecked(294)); + assert_eq!(script_p2wpkh.minimal_non_dust(), Amount::from_sat_unchecked(294)); assert_eq!( script_p2wpkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)), - crate::Amount::from_sat_unchecked(588) + Amount::from_sat_unchecked(588) ); let script_p2pkh = Builder::new() @@ -699,10 +699,10 @@ fn default_dust_value() { .push_opcode(OP_CHECKSIG) .into_script(); assert!(script_p2pkh.is_p2pkh()); - assert_eq!(script_p2pkh.minimal_non_dust(), crate::Amount::from_sat_unchecked(546)); + assert_eq!(script_p2pkh.minimal_non_dust(), Amount::from_sat_unchecked(546)); assert_eq!( script_p2pkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)), - crate::Amount::from_sat_unchecked(1092) + Amount::from_sat_unchecked(1092) ); } diff --git a/bitcoin/src/consensus_validation.rs b/bitcoin/src/consensus_validation.rs index b461374d1..e80994f90 100644 --- a/bitcoin/src/consensus_validation.rs +++ b/bitcoin/src/consensus_validation.rs @@ -134,7 +134,7 @@ define_extension_trait! { fn verify( &self, index: usize, - amount: crate::Amount, + amount: Amount, spending_tx: &[u8], ) -> Result<(), BitcoinconsensusError> { verify_script(self, index, amount, spending_tx) @@ -153,7 +153,7 @@ define_extension_trait! { fn verify_with_flags( &self, index: usize, - amount: crate::Amount, + amount: Amount, spending_tx: &[u8], flags: impl Into, ) -> Result<(), BitcoinconsensusError> { From dbec9807f9adb6a3c0356cd4bc1ad25ec0d9857a Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 17 Jan 2025 10:30:02 +1100 Subject: [PATCH 3/9] Shorten identifiers by removing _in_sats Using `_in_sats` is not meaningful because an `Amount` is an amount, period. Remove `_in_sats` and `_IN_SATS` from identifiers. Internal change only. --- bitcoin/examples/taproot-psbt.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/bitcoin/examples/taproot-psbt.rs b/bitcoin/examples/taproot-psbt.rs index d75b4e14f..1d712ed1b 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_IN_SATS: Amount = Amount::from_sat_unchecked(1_000); +const ABSOLUTE_FEES: Amount = Amount::from_sat_unchecked(1_000); // UTXO_1 will be used for spending example 1 const UTXO_1: P2trUtxo = P2trUtxo { @@ -49,7 +49,7 @@ const UTXO_1: P2trUtxo = P2trUtxo { script_pubkey: UTXO_SCRIPT_PUBKEY, pubkey: UTXO_PUBKEY, master_fingerprint: UTXO_MASTER_FINGERPRINT, - amount_in_sats: Amount::FIFTY_BTC, + amount: Amount::FIFTY_BTC, derivation_path: BIP86_DERIVATION_PATH, }; @@ -60,7 +60,7 @@ const UTXO_2: P2trUtxo = P2trUtxo { script_pubkey: UTXO_SCRIPT_PUBKEY, pubkey: UTXO_PUBKEY, master_fingerprint: UTXO_MASTER_FINGERPRINT, - amount_in_sats: Amount::FIFTY_BTC, + amount: Amount::FIFTY_BTC, derivation_path: BIP86_DERIVATION_PATH, }; @@ -71,7 +71,7 @@ const UTXO_3: P2trUtxo = P2trUtxo { script_pubkey: UTXO_SCRIPT_PUBKEY, pubkey: UTXO_PUBKEY, master_fingerprint: UTXO_MASTER_FINGERPRINT, - amount_in_sats: Amount::FIFTY_BTC, + amount: Amount::FIFTY_BTC, derivation_path: BIP86_DERIVATION_PATH, }; @@ -106,11 +106,11 @@ fn main() -> Result<(), Box> { let change_address = "bcrt1pz449kexzydh2kaypatup5ultru3ej284t6eguhnkn6wkhswt0l7q3a7j76" .parse::>()? .require_network(Network::Regtest)?; - let amount_to_send_in_sats = Amount::ONE_BTC; + let amount_to_send = Amount::ONE_BTC; let change_amount = UTXO_1 - .amount_in_sats - .checked_sub(amount_to_send_in_sats) - .and_then(|x| x.checked_sub(ABSOLUTE_FEES_IN_SATS)) + .amount + .checked_sub(amount_to_send) + .and_then(|x| x.checked_sub(ABSOLUTE_FEES)) .ok_or("fees more than input amount!")?; let tx_hex_string = encode::serialize_hex(&generate_bip86_key_spend_tx( @@ -120,7 +120,7 @@ fn main() -> Result<(), Box> { // Set these fields with valid data for the UTXO from step 5 above UTXO_1, vec![ - TxOut { value: amount_to_send_in_sats, script_pubkey: to_address.script_pubkey() }, + TxOut { value: amount_to_send, script_pubkey: to_address.script_pubkey() }, TxOut { value: change_amount, script_pubkey: change_address.script_pubkey() }, ], )?); @@ -215,7 +215,7 @@ struct P2trUtxo<'a> { script_pubkey: &'a str, pubkey: &'a str, master_fingerprint: &'a str, - amount_in_sats: Amount, + amount: Amount, derivation_path: &'a str, } @@ -226,7 +226,7 @@ fn generate_bip86_key_spend_tx( input_utxo: P2trUtxo, outputs: Vec, ) -> Result> { - let from_amount = input_utxo.amount_in_sats; + let from_amount = input_utxo.amount; let input_pubkey = input_utxo.pubkey.parse::()?; // CREATOR + UPDATER @@ -274,7 +274,7 @@ fn generate_bip86_key_spend_tx( let mut input_txouts = Vec::::new(); for input in [&input_utxo].iter() { input_txouts.push(TxOut { - value: input.amount_in_sats, + value: input.amount, script_pubkey: ScriptBuf::from_hex(input.script_pubkey)?, }); } @@ -412,7 +412,7 @@ impl BenefactorWallet { taproot_spend_info.internal_key(), taproot_spend_info.merkle_root(), ); - let value = input_utxo.amount_in_sats - ABSOLUTE_FEES_IN_SATS; + let value = input_utxo.amount - ABSOLUTE_FEES; // Spend a normal BIP86-like output as an input in our inheritance funding transaction let tx = generate_bip86_key_spend_tx( @@ -476,7 +476,7 @@ impl BenefactorWallet { let mut psbt = self.next_psbt.clone().expect("should have next_psbt"); let input = &mut psbt.inputs[0]; let input_value = input.witness_utxo.as_ref().unwrap().value; - let output_value = input_value - ABSOLUTE_FEES_IN_SATS; + let output_value = input_value - ABSOLUTE_FEES; // We use some other derivation path in this example for our inheritance protocol. The important thing is to ensure // that we use an unhardened path so we can make use of xpubs. @@ -649,7 +649,7 @@ impl BeneficiaryWallet { psbt.unsigned_tx.lock_time = lock_time; psbt.unsigned_tx.output = vec![TxOut { script_pubkey: to_address.script_pubkey(), - value: input_value - ABSOLUTE_FEES_IN_SATS, + value: input_value - ABSOLUTE_FEES, }]; psbt.outputs = vec![Output::default()]; let unsigned_tx = psbt.unsigned_tx.clone(); From f3e853e07a207bef1e63e4a5752c4af1bfc722b6 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 23 Jan 2025 14:43:11 +1100 Subject: [PATCH 4/9] units: Do trivial refactor of amount::tests We are trying to make changes to `units::amount` but the diff is too big - do the remove-whitespace change on its own. Whitespace only. --- units/src/amount/tests.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index a37971563..82a966912 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -576,9 +576,7 @@ fn unsigned_signed_conversion() { let max_sats: u64 = Amount::MAX.to_sat(); assert_eq!(ua(max_sats).to_signed(), sa(max_sats as i64)); - assert_eq!(sa(max_sats as i64).to_unsigned(), Ok(ua(max_sats))); - assert_eq!(sa(max_sats as i64).to_unsigned().unwrap().to_signed(), sa(max_sats as i64)); } From c6f056672b20aa8970a544e5e7750d8e1c3a0fef Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 24 Jan 2025 08:56:58 +1100 Subject: [PATCH 5/9] Change local var sa to ssat Match other identifiers in the `amount` test module. Only done for the signed amount to make review brain dead easy. --- units/src/amount/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 82a966912..034a80ee5 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -571,13 +571,13 @@ check_format_non_negative_show_denom! { #[test] fn unsigned_signed_conversion() { - let sa = SignedAmount::from_sat; + let ssat = SignedAmount::from_sat; let ua = Amount::from_sat; let max_sats: u64 = Amount::MAX.to_sat(); - assert_eq!(ua(max_sats).to_signed(), sa(max_sats as i64)); - assert_eq!(sa(max_sats as i64).to_unsigned(), Ok(ua(max_sats))); - assert_eq!(sa(max_sats as i64).to_unsigned().unwrap().to_signed(), sa(max_sats as i64)); + assert_eq!(ua(max_sats).to_signed(), ssat(max_sats as i64)); + assert_eq!(ssat(max_sats as i64).to_unsigned(), Ok(ua(max_sats))); + assert_eq!(ssat(max_sats as i64).to_unsigned().unwrap().to_signed(), ssat(max_sats as i64)); } #[test] From 8fdec67f7dbf0d6a1766247168db8b53b8fb672c Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 24 Jan 2025 08:58:54 +1100 Subject: [PATCH 6/9] Change local var ua to sat As we did for the signed amount; match other identifiers in the `amount` test module. --- units/src/amount/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 034a80ee5..27d5607fe 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -572,11 +572,11 @@ check_format_non_negative_show_denom! { #[test] fn unsigned_signed_conversion() { let ssat = SignedAmount::from_sat; - let ua = Amount::from_sat; + let sat = Amount::from_sat; let max_sats: u64 = Amount::MAX.to_sat(); - assert_eq!(ua(max_sats).to_signed(), ssat(max_sats as i64)); - assert_eq!(ssat(max_sats as i64).to_unsigned(), Ok(ua(max_sats))); + assert_eq!(sat(max_sats).to_signed(), ssat(max_sats as i64)); + assert_eq!(ssat(max_sats as i64).to_unsigned(), Ok(sat(max_sats))); assert_eq!(ssat(max_sats as i64).to_unsigned().unwrap().to_signed(), ssat(max_sats as i64)); } From 00b71a670fcedeca255e69fe5e3d08e5a9d1b72e Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 24 Jan 2025 09:05:00 +1100 Subject: [PATCH 7/9] Use from_sat_unchecked for hardcoded ints We have an `_unchecked` amount constructor that makes no assumptions about the argument. We would like to start enforcing MAX_MONEY but the diff to introduce this is massive. In an effort to make it smaller we can do all the hardcoded ints first. We did this already but a bunch more snuck in or were missed. In any amount constructor that passes in a hardcoded const as a decimal integer (i.e., not hex) use the `_unchecked` version. Done in preparation for enforcing MAX_MONEY. --- bitcoin/examples/sighash.rs | 4 ++-- units/src/amount/tests.rs | 6 +++--- units/src/fee.rs | 18 +++++++++--------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/bitcoin/examples/sighash.rs b/bitcoin/examples/sighash.rs index 15439b46c..4f1f4d441 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(200000000); + let ref_out_value = Amount::from_sat_unchecked(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(968240); + let ref_out_value = Amount::from_sat_unchecked(968240); println!("\nsighash_p2wsh_multisig_2x2:"); compute_sighash_p2wsh(&raw_tx, 0, ref_out_value); diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 27d5607fe..33eeb5f8d 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -142,7 +142,7 @@ fn mul_div() { #[test] fn neg() { - let amount = -SignedAmount::from_sat(2); + let amount = -SignedAmount::from_sat_unchecked(2); assert_eq!(amount.to_sat(), -2); } @@ -231,7 +231,7 @@ fn amount_checked_div_by_weight_floor() { #[cfg(feature = "alloc")] #[test] fn amount_checked_div_by_fee_rate() { - let amount = Amount::from_sat(1000); + let amount = Amount::from_sat_unchecked(1000); let fee_rate = FeeRate::from_sat_per_kwu(2); // Test floor division @@ -244,7 +244,7 @@ fn amount_checked_div_by_fee_rate() { assert_eq!(weight, Weight::from_wu(500_000)); // Same result for exact division // Test truncation behavior - let amount = Amount::from_sat(1000); + let amount = Amount::from_sat_unchecked(1000); let fee_rate = FeeRate::from_sat_per_kwu(3); let floor_weight = amount.checked_div_by_fee_rate_floor(fee_rate).unwrap(); let ceil_weight = amount.checked_div_by_fee_rate_ceil(fee_rate).unwrap(); diff --git a/units/src/fee.rs b/units/src/fee.rs index 1f0a535b4..833399279 100644 --- a/units/src/fee.rs +++ b/units/src/fee.rs @@ -197,7 +197,7 @@ mod tests { #[test] fn fee_rate_div_by_weight() { - let fee_rate = Amount::from_sat(329) / Weight::from_wu(381); + let fee_rate = Amount::from_sat_unchecked(329) / Weight::from_wu(381); assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(863)); } @@ -208,7 +208,7 @@ mod tests { let fee_rate = FeeRate::from_sat_per_vb(2).unwrap(); let weight = Weight::from_vb(3).unwrap(); - assert_eq!(fee_rate.fee_wu(weight).unwrap(), Amount::from_sat(6)); + assert_eq!(fee_rate.fee_wu(weight).unwrap(), Amount::from_sat_unchecked(6)); } #[test] @@ -217,7 +217,7 @@ mod tests { assert!(fee_overflow.is_none()); let fee_rate = FeeRate::from_sat_per_vb(2).unwrap(); - assert_eq!(fee_rate.fee_vb(3).unwrap(), Amount::from_sat(6)); + assert_eq!(fee_rate.fee_vb(3).unwrap(), Amount::from_sat_unchecked(6)); } #[test] @@ -227,7 +227,7 @@ mod tests { .unwrap() .checked_mul_by_weight(weight) .expect("expected Amount"); - assert_eq!(Amount::from_sat(100), fee); + assert_eq!(Amount::from_sat_unchecked(100), fee); let fee = FeeRate::from_sat_per_kwu(10).checked_mul_by_weight(Weight::MAX); assert!(fee.is_none()); @@ -235,14 +235,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(9), fee); + assert_eq!(Amount::from_sat_unchecked(9), fee); let weight = Weight::from_wu(381); let fee_rate = FeeRate::from_sat_per_kwu(864); let fee = fee_rate.checked_mul_by_weight(weight).unwrap(); // 381 * 0.864 yields 329.18. // The result is then rounded up to 330. - assert_eq!(fee, Amount::from_sat(330)); + assert_eq!(fee, Amount::from_sat_unchecked(330)); } #[test] @@ -250,7 +250,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(6); + let six = Amount::from_sat_unchecked(6); assert_eq!(two * three, six); } @@ -258,13 +258,13 @@ mod tests { #[test] fn amount_div_by_fee_rate() { // Test exact division - let amount = Amount::from_sat(1000); + let amount = Amount::from_sat_unchecked(1000); let fee_rate = FeeRate::from_sat_per_kwu(2); let weight = amount / fee_rate; assert_eq!(weight, Weight::from_wu(500_000)); // Test truncation behavior - let amount = Amount::from_sat(1000); + let amount = Amount::from_sat_unchecked(1000); let fee_rate = FeeRate::from_sat_per_kwu(3); let weight = amount / fee_rate; // 1000 * 1000 = 1,000,000 msats From 34e3049ae0ebe15f24be771f0ece3c72ee46ce5c Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 24 Jan 2025 11:36:32 +1100 Subject: [PATCH 8/9] Use sats instead of satoshi No one says that, just use `sats`. Internal change only. --- units/src/amount/unsigned.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index d3c494921..0cc4a4356 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -152,19 +152,19 @@ impl Amount { /// /// If the amount is too big, too precise or negative. pub fn from_str_in(s: &str, denom: Denomination) -> Result { - let (negative, satoshi) = + let (negative, sats) = parse_signed_to_satoshi(s, denom).map_err(|error| error.convert(false))?; if negative { return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange( OutOfRangeError::negative(), ))); } - if satoshi > Self::MAX.0 { + if sats > Self::MAX.0 { return Err(ParseAmountError(ParseAmountErrorInner::OutOfRange( OutOfRangeError::too_big(false), ))); } - Ok(Amount::from_sat(satoshi)) + Ok(Amount::from_sat(sats)) } /// Parses amounts with denomination suffix as produced by [`Self::to_string_with_denomination`] From 13a3f490b80e4c8f8e1753111a914315eefd73e6 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 24 Jan 2025 11:54:24 +1100 Subject: [PATCH 9/9] Use Self instead of amount type I claim that if the two amount modules are coded as similarly as possible it will be easier to ensure that we have the API's uniform and bug free. To make auditing the modules easier and less error prone use `Self` instead of the explicit type. This makes it easier to see differences in the modules and to ensure the differences are correct and required. Internal change, no logic changes whatsoever. --- units/src/amount/signed.rs | 24 ++++++++++++------------ units/src/amount/unsigned.rs | 24 ++++++++++++------------ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs index 82a07fa05..aece125bd 100644 --- a/units/src/amount/signed.rs +++ b/units/src/amount/signed.rs @@ -109,7 +109,7 @@ impl SignedAmount { /// ``` #[cfg(feature = "alloc")] pub fn from_btc(btc: f64) -> Result { - SignedAmount::from_float_in(btc, Denomination::Bitcoin) + Self::from_float_in(btc, Denomination::Bitcoin) } /// Converts from a value expressing a whole number of bitcoin to a [`SignedAmount`]. @@ -120,7 +120,7 @@ impl SignedAmount { /// per bitcoin overflows an `i64` type. pub fn from_int_btc>(whole_bitcoin: T) -> Result { match whole_bitcoin.into().checked_mul(100_000_000) { - Some(amount) => Ok(SignedAmount::from_sat(amount)), + Some(amount) => Ok(Self::from_sat(amount)), None => Err(OutOfRangeError { is_signed: true, is_greater_than_max: true }), } } @@ -180,7 +180,7 @@ impl SignedAmount { /// ``` pub fn from_str_with_denomination(s: &str) -> Result { let (amt, denom) = split_amount_and_denomination(s)?; - SignedAmount::from_str_in(amt, denom).map_err(Into::into) + Self::from_str_in(amt, denom).map_err(Into::into) } /// Expresses this [`SignedAmount`] as a floating-point value in the given [`Denomination`]. @@ -228,7 +228,7 @@ impl SignedAmount { ) -> Result { // This is inefficient, but the safest way to deal with this. The parsing logic is safe. // Any performance-critical application should not be dealing with floats. - SignedAmount::from_str_in(&value.to_string(), denom) + Self::from_str_in(&value.to_string(), denom) } /// Constructs a new object that implements [`fmt::Display`] in the given [`Denomination`]. @@ -548,14 +548,14 @@ impl FromStr for SignedAmount { /// /// If the returned value would be zero or negative zero, then no denomination is required. fn from_str(s: &str) -> Result { - let result = SignedAmount::from_str_with_denomination(s); + let result = Self::from_str_with_denomination(s); match result { Err(ParseError(ParseErrorInner::MissingDenomination(_))) => { - let d = SignedAmount::from_str_in(s, Denomination::Satoshi); + let d = Self::from_str_in(s, Denomination::Satoshi); - if d == Ok(SignedAmount::ZERO) { - Ok(SignedAmount::ZERO) + if d == Ok(Self::ZERO) { + Ok(Self::ZERO) } else { result } @@ -568,14 +568,14 @@ impl FromStr for SignedAmount { impl From for SignedAmount { fn from(value: Amount) -> Self { let v = value.to_sat() as i64; // Cast ok, signed amount and amount share positive range. - SignedAmount::from_sat_unchecked(v) + Self::from_sat_unchecked(v) } } impl core::iter::Sum for SignedAmount { fn sum>(iter: I) -> Self { let sats: i64 = iter.map(|amt| amt.0).sum(); - SignedAmount::from_sat(sats) + Self::from_sat(sats) } } @@ -585,7 +585,7 @@ impl<'a> core::iter::Sum<&'a SignedAmount> for SignedAmount { I: Iterator, { let sats: i64 = iter.map(|amt| amt.0).sum(); - SignedAmount::from_sat(sats) + Self::from_sat(sats) } } @@ -593,6 +593,6 @@ impl<'a> core::iter::Sum<&'a SignedAmount> for SignedAmount { impl<'a> Arbitrary<'a> for SignedAmount { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { let s = i64::arbitrary(u)?; - Ok(SignedAmount(s)) + Ok(Self(s)) } } diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 0cc4a4356..e80f5c6b7 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -112,7 +112,7 @@ impl Amount { /// ``` #[cfg(feature = "alloc")] pub fn from_btc(btc: f64) -> Result { - Amount::from_float_in(btc, Denomination::Bitcoin) + Self::from_float_in(btc, Denomination::Bitcoin) } /// Converts from a value expressing a whole number of bitcoin to an [`Amount`]. @@ -123,7 +123,7 @@ impl Amount { /// per bitcoin overflows a `u64` type. pub fn from_int_btc>(whole_bitcoin: T) -> Result { match whole_bitcoin.into().checked_mul(100_000_000) { - Some(amount) => Ok(Amount::from_sat(amount)), + Some(amount) => Ok(Self::from_sat(amount)), None => Err(OutOfRangeError { is_signed: false, is_greater_than_max: true }), } } @@ -164,7 +164,7 @@ impl Amount { OutOfRangeError::too_big(false), ))); } - Ok(Amount::from_sat(sats)) + Ok(Self::from_sat(sats)) } /// Parses amounts with denomination suffix as produced by [`Self::to_string_with_denomination`] @@ -186,7 +186,7 @@ impl Amount { /// ``` pub fn from_str_with_denomination(s: &str) -> Result { let (amt, denom) = split_amount_and_denomination(s)?; - Amount::from_str_in(amt, denom).map_err(Into::into) + Self::from_str_in(amt, denom).map_err(Into::into) } /// Expresses this [`Amount`] as a floating-point value in the given [`Denomination`]. @@ -234,7 +234,7 @@ impl Amount { } // This is inefficient, but the safest way to deal with this. The parsing logic is safe. // Any performance-critical application should not be dealing with floats. - Amount::from_str_in(&value.to_string(), denom) + Self::from_str_in(&value.to_string(), denom) } /// Constructs a new object that implements [`fmt::Display`] in the given [`Denomination`]. @@ -483,14 +483,14 @@ impl FromStr for Amount { /// /// If the returned value would be zero or negative zero, then no denomination is required. fn from_str(s: &str) -> Result { - let result = Amount::from_str_with_denomination(s); + let result = Self::from_str_with_denomination(s); match result { Err(ParseError(ParseErrorInner::MissingDenomination(_))) => { - let d = Amount::from_str_in(s, Denomination::Satoshi); + let d = Self::from_str_in(s, Denomination::Satoshi); - if d == Ok(Amount::ZERO) { - Ok(Amount::ZERO) + if d == Ok(Self::ZERO) { + Ok(Self::ZERO) } else { result } @@ -509,7 +509,7 @@ impl TryFrom for Amount { impl core::iter::Sum for Amount { fn sum>(iter: I) -> Self { let sats: u64 = iter.map(|amt| amt.0).sum(); - Amount::from_sat(sats) + Self::from_sat(sats) } } @@ -519,7 +519,7 @@ impl<'a> core::iter::Sum<&'a Amount> for Amount { I: Iterator, { let sats: u64 = iter.map(|amt| amt.0).sum(); - Amount::from_sat(sats) + Self::from_sat(sats) } } @@ -527,6 +527,6 @@ impl<'a> core::iter::Sum<&'a Amount> for Amount { impl<'a> Arbitrary<'a> for Amount { fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { let a = u64::arbitrary(u)?; - Ok(Amount(a)) + Ok(Self(a)) } }