Enforce MAX_MONEY invariant in amount types

Recently we changed MAX to equal MAX_MONEY for the two amount types. In
doing so we left the module in quite a mess.

Enforcing the `MAX_MONEY` invariant is quite involved because it means
multiple things:

- Constructing amounts is now fallible
- Converting from unsigned to signed is now infallible
- Taking the absolute value is now infallible
- Integer overflow is illuminated in various places

Details:

- Update `from_sat` to check the invariant
- Fix all docs including examples
- Use the unchecked constructor in test code
- Comment any other use of the unchecked constructor
- Comment any place we do unchecked integer ops
- Deprecate `unchecked_abs`
- Fail serde using the horrible string error
- Try not to use the unchecked constructor in rustdocs, no need to
  encourage unsuspecting users to use it.
- Fix the txout regression test to use MAX_MONEY
- Use unchecked constructor in `TxOut::NULL`
- Remove `iter::Sum` impls
- Favour `MAX_MONEY` in code over `MAX` just to be explicit and save
  unsuspecting devs from missing this change.
- Use `?` in rustdoc examples (required by Rust API guidlines)
- Add `# Returns` section to checked ops functions, this differs from
  other places in the crate because the amount types differ also so I
  believe the non-uniformity is warranted.
- Remove `TryFrom<Amount> for SignedAmount` because the conversion is
  now infallible. Add a `From` impl.
- Fix the arbitrary impls
- Maintain correct formatting
- Add units test that verify overflow assumptions (these are testing
  Rust really but I was paranoid about introducing bugs)
- Remove private `check_max` function as its no longer needed

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);

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::{Amount, FeeRate};
+use crate::{amount, Amount, FeeRate};
 
 #[rustfmt::skip]            // Keep public re-exports separate.
 #[doc(inline)]
@@ -273,7 +273,7 @@ crate::internal_macros::define_extension_trait! {
         ///
         /// [`minimal_non_dust_custom`]: Script::minimal_non_dust_custom
         fn minimal_non_dust(&self) -> Amount {
-            self.minimal_non_dust_internal(DUST_RELAY_TX_FEE.into())
+            self.minimal_non_dust_internal(DUST_RELAY_TX_FEE.into()).expect("DUST_RELAY_FEE==3000 computes valid amount")
         }
 
         /// Returns the minimum value an output with this script should have in order to be
@@ -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) -> Amount {
+        fn minimal_non_dust_custom(&self, dust_relay_fee: FeeRate) -> Result<Amount, amount::OutOfRangeError> {
             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) -> Amount {
+        fn minimal_non_dust_internal(&self, dust_relay_fee: u64) -> Result<Amount, amount::OutOfRangeError> {
             // 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

bitcoin/src/blockdata/script/tests.rs

@@ -687,7 +687,7 @@ fn default_dust_value() {
     assert!(script_p2wpkh.is_p2wpkh());
     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)),
+        script_p2wpkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)).unwrap(),
         Amount::from_sat_unchecked(588)
     );
 
@@ -701,7 +701,7 @@ fn default_dust_value() {
     assert!(script_p2pkh.is_p2pkh());
     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)),
+        script_p2pkh.minimal_non_dust_custom(FeeRate::from_sat_per_vb_unchecked(6)).unwrap(),
         Amount::from_sat_unchecked(1092)
     );
 }

bitcoin/src/blockdata/transaction.rs

@@ -26,7 +26,7 @@ use crate::script::{Script, ScriptBuf, ScriptExt as _, ScriptExtPriv as _};
 #[cfg(doc)]
 use crate::sighash::{EcdsaSighashType, TapSighashType};
 use crate::witness::Witness;
-use crate::{Amount, FeeRate, SignedAmount};
+use crate::{amount, Amount, FeeRate, SignedAmount};
 
 #[rustfmt::skip]            // Keep public re-exports separate.
 #[doc(inline)]
@@ -198,8 +198,8 @@ crate::internal_macros::define_extension_trait! {
         /// To use the default Bitcoin Core value, use [`minimal_non_dust`].
         ///
         /// [`minimal_non_dust`]: TxOut::minimal_non_dust
-        fn minimal_non_dust_custom(script_pubkey: ScriptBuf, dust_relay_fee: FeeRate) -> Self {
-            TxOut { value: script_pubkey.minimal_non_dust_custom(dust_relay_fee), script_pubkey }
+        fn minimal_non_dust_custom(script_pubkey: ScriptBuf, dust_relay_fee: FeeRate) -> Result<TxOut, amount::OutOfRangeError> {
+            Ok(TxOut { value: script_pubkey.minimal_non_dust_custom(dust_relay_fee)?, script_pubkey })
         }
     }
 }

bitcoin/src/lib.rs

@@ -204,7 +204,7 @@ pub mod amount {
     //! This module mainly introduces the [`Amount`] and [`SignedAmount`] types.
     //! We refer to the documentation on the types for more information.
 
-    use crate::consensus::{encode, Decodable, Encodable};
+    use crate::consensus::{self, encode, Decodable, Encodable};
     use crate::io::{BufRead, Write};
 
     #[rustfmt::skip]            // Keep public re-exports separate.
@@ -221,7 +221,9 @@ pub mod amount {
     impl Decodable for Amount {
         #[inline]
         fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
-            Ok(Amount::from_sat(Decodable::consensus_decode(r)?))
+            Amount::from_sat(Decodable::consensus_decode(r)?).map_err(|_| {
+                consensus::parse_failed_error("amount is greater than Amount::MAX_MONEY")
+            })
         }
     }
 

bitcoin/src/psbt/error.rs

@@ -10,6 +10,8 @@ use crate::consensus::encode;
 use crate::prelude::Box;
 use crate::psbt::raw;
 use crate::transaction::Transaction;
+#[cfg(doc)]
+use crate::Amount;
 
 /// Enum for marking psbt hash error.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
@@ -78,7 +80,7 @@ pub enum Error {
     ConsensusParse(encode::ParseError),
     /// Negative fee
     NegativeFee,
-    /// Integer overflow in fee calculation
+    /// Fee greater than [`Amount::MAX_MONEY`].
     FeeOverflow,
     /// Parsing error indicating invalid public keys
     InvalidPublicKey(crate::crypto::key::FromSliceError),
@@ -153,7 +155,7 @@ impl fmt::Display for Error {
             ConsensusParse(ref e) =>
                 write_err!(f, "error parsing bitcoin consensus encoded object"; e),
             NegativeFee => f.write_str("PSBT has a negative fee which is not allowed"),
-            FeeOverflow => f.write_str("integer overflow in fee calculation"),
+            FeeOverflow => f.write_str("fee greater than Amount::MAX_MONEY"),
             InvalidPublicKey(ref e) => write_err!(f, "invalid public key"; e),
             InvalidSecp256k1PublicKey(ref e) => write_err!(f, "invalid secp256k1 public key"; e),
             InvalidXOnlyPublicKey => f.write_str("invalid xonly public key"),

bitcoin/src/psbt/mod.rs

@@ -1264,7 +1264,7 @@ mod tests {
                     witness: Witness::default(),
                 }],
                 output: vec![TxOut {
-                    value: Amount::from_sat(output),
+                    value: Amount::from_sat(output).expect("output amount too big"),
                     script_pubkey: ScriptBuf::from_hex(
                         "a9143545e6e33b832c47050f24d3eeb93c9c03948bc787",
                     )
@@ -1278,7 +1278,7 @@ mod tests {
 
             inputs: vec![Input {
                 witness_utxo: Some(TxOut {
-                    value: Amount::from_sat(input),
+                    value: Amount::from_sat(input).expect("input amount too big"),
                     script_pubkey: ScriptBuf::from_hex(
                         "a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587",
                     )

bitcoin/tests/psbt-sign-taproot.rs

@@ -205,7 +205,8 @@ fn create_psbt_for_taproot_key_path_spend(
 ) -> Psbt {
     let send_value = 6400;
     let out_puts = vec![TxOut {
-        value: Amount::from_sat(send_value),
+        // Use _unchecked because 6400 sats is within valid range.
+        value: Amount::from_sat_unchecked(send_value),
         script_pubkey: to_address.script_pubkey(),
     }];
     let prev_tx_id = "06980ca116f74c7845a897461dd0e1d15b114130176de5004957da516b4dee3a";
@@ -243,7 +244,8 @@ fn create_psbt_for_taproot_key_path_spend(
     let mut input = Input {
         witness_utxo: {
             let script_pubkey = from_address.script_pubkey();
-            Some(TxOut { value: Amount::from_sat(utxo_value), script_pubkey })
+            // Use _unchecked because 6588 sats is within valid range.
+            Some(TxOut { value: Amount::from_sat_unchecked(utxo_value), script_pubkey })
         },
         tap_key_origins: origins,
         ..Default::default()
@@ -283,7 +285,8 @@ fn create_psbt_for_taproot_script_path_spend(
     let mfp = "73c5da0a";
 
     let out_puts = vec![TxOut {
-        value: Amount::from_sat(send_value),
+        // Use _unchecked because 6000 sats is within valid range.
+        value: Amount::from_sat_unchecked(send_value),
         script_pubkey: to_address.script_pubkey(),
     }];
     let prev_tx_id = "9d7c6770fca57285babab60c51834cfcfd10ad302119cae842d7216b4ac9a376";
@@ -322,7 +325,10 @@ fn create_psbt_for_taproot_script_path_spend(
     let mut input = Input {
         witness_utxo: {
             let script_pubkey = from_address.script_pubkey();
-            Some(TxOut { value: Amount::from_sat(utxo_value), script_pubkey })
+            Some(TxOut {
+                value: Amount::from_sat(utxo_value).expect("invalid utxo_value amount"),
+                script_pubkey,
+            })
         },
         tap_key_origins: origins,
         tap_scripts,

bitcoin/tests/serde.rs

@@ -109,10 +109,9 @@ fn serde_regression_txin() {
 
 #[test]
 fn serde_regression_txout() {
-    let txout = TxOut {
-        value: Amount::from_sat(0xDEADBEEFCAFEBABE),
-        script_pubkey: ScriptBuf::from(vec![0u8, 1u8, 2u8]),
-    };
+    let txout =
+        TxOut { value: Amount::MAX_MONEY, script_pubkey: ScriptBuf::from(vec![0u8, 1u8, 2u8]) };
+
     let got = serialize(&txout).unwrap();
     let want = include_bytes!("data/serde/txout_bincode") as &[_];
     assert_eq!(got, want)

primitives/src/transaction.rs

@@ -353,8 +353,11 @@ pub struct TxOut {
 #[cfg(feature = "alloc")]
 impl TxOut {
     /// This is used as a "null txout" in consensus signing code.
-    pub const NULL: Self =
-        TxOut { value: Amount::from_sat(0xffffffffffffffff), script_pubkey: ScriptBuf::new() };
+    pub const NULL: Self = TxOut {
+        // Breaks the `Amount` invariant but since this is explicitly an invalid utxo that is ok.
+        value: Amount::from_sat_unchecked(0xffffffffffffffff),
+        script_pubkey: ScriptBuf::new(),
+    };
 }
 
 /// A reference to a transaction output.

units/src/amount/mod.rs

@@ -57,13 +57,20 @@ pub use self::{
 ///
 /// ```
 /// # use bitcoin_units::Amount;
-///
-/// assert_eq!("1 BTC".parse::<Amount>().unwrap(), Amount::from_sat(100_000_000));
-/// assert_eq!("1 cBTC".parse::<Amount>().unwrap(), Amount::from_sat(1_000_000));
-/// assert_eq!("1 mBTC".parse::<Amount>().unwrap(), Amount::from_sat(100_000));
-/// assert_eq!("1 uBTC".parse::<Amount>().unwrap(), Amount::from_sat(100));
-/// assert_eq!("1 bit".parse::<Amount>().unwrap(), Amount::from_sat(100));
-/// assert_eq!("1 sat".parse::<Amount>().unwrap(), Amount::from_sat(1));
+/// let equal = [
+///     ("1 BTC", 100_000_000),
+///     ("1 cBTC", 1_000_000),
+///     ("1 mBTC", 100_000),
+///     ("1 uBTC", 100),
+///     ("1 bit", 100),
+///     ("1 sat", 1),
+/// ];
+/// for (string, sats) in equal {
+///     assert_eq!(
+///         string.parse::<Amount>().expect("valid bitcoin amount string"),
+///         Amount::from_sat(sats).expect("valid amount in satoshis"),
+///     )
+/// }
 /// ```
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
 #[non_exhaustive]
@@ -571,8 +578,13 @@ enum DisplayStyle {
 
 /// Calculates the sum over the iterator using checked arithmetic.
 pub trait CheckedSum<R>: sealed::Sealed<R> {
-    /// Calculates the sum over the iterator using checked arithmetic. If an over or underflow would
-    /// happen it returns [`None`].
+    /// Calculates the sum over the iterator using checked arithmetic.
+    ///
+    /// # Returns
+    ///
+    /// Returns [`None`] if the result is above [`MAX_MONEY`].
+    ///
+    /// [`MAX_MONEY`]: crate::Amount::MAX_MONEY
     fn checked_sum(self) -> Option<R>;
 }
 

units/src/amount/serde.rs

@@ -92,7 +92,8 @@ impl SerdeAmount for Amount {
         u64::serialize(&self.to_sat(), s)
     }
     fn des_sat<'d, D: Deserializer<'d>>(d: D, _: private::Token) -> Result<Self, D::Error> {
-        Ok(Amount::from_sat(u64::deserialize(d)?))
+        use serde::de::Error;
+        Amount::from_sat(u64::deserialize(d)?).map_err(D::Error::custom)
     }
     #[cfg(feature = "alloc")]
     fn ser_btc<S: Serializer>(self, s: S, _: private::Token) -> Result<S::Ok, S::Error> {
@@ -137,7 +138,8 @@ impl SerdeAmount for SignedAmount {
         i64::serialize(&self.to_sat(), s)
     }
     fn des_sat<'d, D: Deserializer<'d>>(d: D, _: private::Token) -> Result<Self, D::Error> {
-        Ok(SignedAmount::from_sat(i64::deserialize(d)?))
+        use serde::de::Error;
+        SignedAmount::from_sat(i64::deserialize(d)?).map_err(D::Error::custom)
     }
     #[cfg(feature = "alloc")]
     fn ser_btc<S: Serializer>(self, s: S, _: private::Token) -> Result<S::Ok, S::Error> {