Repository: rust-bitcoin/rust-bitcoin
Pull request #2230 (2023-11-27 15:18:57)

Add effective value calculation
Draft PR for adding effective value calculation to TxOut.  Adding this method was discussed here: https://github.com/rust-bitcoin/rust-bitcoin/pull/2217

yancyribbens: @Kixunil what do you think?  Will add some tests around this if it seems ok.
yancyribbens: force push to add test for `effective_value`
Kixunil: Thinking about this it's weird that it's a method on `TxOut` since `script_pubkey` is not used. It kinda should be on `Amount` but that seems odd too. Perhaps have a free-standing function in the `transaction` module that takes just `Amount` (and the remaining parameters`)?

If there's a method on `TxOut` it should really do something with the script - e.g. scan it and determine that it's P2PKH and use known constant. Return `None` if it's unknown script. But then the question is what to do about `P2TR` - it's not possible to know if it will be a script spend or key spend.

I think there should be something like `SpendInfo` type that carries information about how an output can be spent and have a method on it. But this is an advanced idea and would need a lot of design work etc. Also I guess something like this is already covered by `miniscript`. So while I'm intrigued by it I prefer to postpone it. We have ton of other more urgent work now.
yancyribbens: > So while I'm intrigued by it I prefer to postpone it. We have ton of other more urgent work now.

Ok, well in the meantime re-opening this PR then which I closed in favor of this current work https://github.com/rust-bitcoin/rust-bitcoin/pull/2217 would fix the broken crate I'm working on.  Any opposition to that?
Kixunil: I mean I'm intrigued by the advanced stuff. A free-standing function would be still fine (and still useful for uncommon cases like custom scripts).
yancyribbens: > In the transaction module.

Alright, I think that makes sense to me too.  Done.
yancyribbens: This is probably ready to take out of draft state now.
yancyribbens: Rebased and dropped a clippy commit that had already been merged with master.
yancyribbens: I added `:` to `effective_value can be calculated as: value - (fee_rate * weight).`.  Also waffling back and forth on if this is describing a `TxOut` or the `TxIn` to a new transaction.
yancyribbens: Composing an input set for a transaction requires the input amount to be the net amount of spending the corresponding output (TxOut), so I believe the terminology TxOut here is correct.
yancyribbens: Sorry for all the noise.  Should read "value of a output" not input which is what it was before.
yancyribbens: changed to `an output` instead of `a output` (grammar).
yancyribbens: Rebased with master
yancyribbens: Maybe I ought to reword the doc comment since it's sort of confusing how the commit uses the `TxIn::BASE_WEIGHT`.

What about something like:

The `effective_value` is used to calculate how much a particular output contributes to a target spend amount.  Therefore, the `effective_value` returns the net amount, that is, the output amount minus the overhead to spend the output.  The overhead to spend the output is the fee.  In order to calculate the fee, the size of the input which consumes the output is calculated as the input `base_weight` plus the size of the output once the spending conditions are satisfied (the satisfaction_weight) multiplied by a `fee_rate`.  Lastly, subtract the fee from the output value to find the effective_value.
yancyribbens: > If we incorporate the output version of this API, the effective cost of adding a change output.

@sanket1729 I'm not sure I follow.  Can you give an example of how using the effective cost of a change output is used?
sanket1729: @yancyribbens, I have seen some coin selection code where the cost of a change output is subtracted from the change output. It is also useful when building complex transactions with multiple recipients.

In any case, we don't need to add this as there is no requirement for this. 
yancyribbens: > I have seen some coin selection code where the cost of a change output is subtracted from the change output. It is also useful when building complex transactions with multiple recipients.

Ah ok, I see.  I'm starting to 2nd guess if this is actually worth adding to Rust Bitcoin since it's so specific to coin selection.  Maybe it's better to just keep this within rust-bitcoin-coin-selection instead of rust-bitcoin.

Patch:
diff --git a/.github/workflows/kani.yml b/.github/workflows/kani.yml
index dff1e54b..520ad176 100644
--- a/.github/workflows/kani.yml
+++ b/.github/workflows/kani.yml
@@ -11,4 +11,4 @@ jobs:
         uses: actions/checkout@v4
 
       - name: 'Run Kani on your code.'
-        uses: model-checking/kani-github-action@v1.1
+        uses: model-checking/kani-github-action@v1.0
diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock
index fb048b8c..1cae13ca 100644
--- a/Cargo-minimal.lock
+++ b/Cargo-minimal.lock
@@ -45,7 +45,6 @@ dependencies = [
  "hex-conservative",
  "hex_lit",
  "mutagen",
- "ordered",
  "secp256k1",
  "serde",
  "serde_json",
@@ -236,12 +235,6 @@ dependencies = [
  "proc-macro2",
 ]
 
-[[package]]
-name = "ordered"
-version = "0.2.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7f0642533dea0bb58bd5cae31bafc1872429f0f12ac8c61fe2b4ba44f80b959b"
-
 [[package]]
 name = "ppv-lite86"
 version = "0.2.8"
diff --git a/Cargo-recent.lock b/Cargo-recent.lock
index ceda6598..90a84ae2 100644
--- a/Cargo-recent.lock
+++ b/Cargo-recent.lock
@@ -44,7 +44,6 @@ dependencies = [
  "hex-conservative",
  "hex_lit",
  "mutagen",
- "ordered",
  "secp256k1",
  "serde",
  "serde_json",
@@ -235,12 +234,6 @@ dependencies = [
  "proc-macro2",
 ]
 
-[[package]]
-name = "ordered"
-version = "0.2.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7f0642533dea0bb58bd5cae31bafc1872429f0f12ac8c61fe2b4ba44f80b959b"
-
 [[package]]
 name = "ppv-lite86"
 version = "0.2.17"
diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml
index 8c899897..f06cc572 100644
--- a/bitcoin/Cargo.toml
+++ b/bitcoin/Cargo.toml
@@ -38,7 +38,6 @@ secp256k1 = { version = "0.28.0", default-features = false, features = ["hashes"
 units = { package = "bitcoin-units", version = "0.1.0", default-features = false, features = ["alloc"] }
 
 base64 = { version = "0.21.3", optional = true }
-ordered = { version = "0.2.0", optional = true }
 # Only use this feature for no-std builds, otherwise use bitcoinconsensus-std.
 bitcoinconsensus = { version = "0.20.2-0.5.0", default-features = false, optional = true }
 
diff --git a/bitcoin/examples/sighash.rs b/bitcoin/examples/sighash.rs
index 04b0fb3f..bdddaa7e 100644
--- a/bitcoin/examples/sighash.rs
+++ b/bitcoin/examples/sighash.rs
@@ -44,7 +44,7 @@ fn compute_sighash_p2wpkh(raw_tx: &[u8], inp_idx: usize, value: u64) {
 
     let mut cache = sighash::SighashCache::new(&tx);
     let sighash = cache
-        .p2wpkh_signature_hash(inp_idx, &spk, Amount::from_sat(value), sig.sighash_type)
+        .p2wpkh_signature_hash(inp_idx, &spk, Amount::from_sat(value), sig.hash_ty)
         .expect("failed to compute sighash");
     println!("Segwit p2wpkh sighash: {:x}", sighash);
     let msg = secp256k1::Message::from_digest(sighash.to_byte_array());
@@ -93,9 +93,9 @@ fn compute_sighash_legacy(raw_tx: &[u8], inp_idx: usize, script_pubkey_bytes_opt
         let sig = ecdsa::Signature::from_slice(instr.unwrap().push_bytes().unwrap().as_bytes())
             .expect("failed to parse sig");
         let sighash = cache
-            .legacy_signature_hash(inp_idx, script_code, sig.sighash_type.to_u32())
+            .legacy_signature_hash(inp_idx, script_code, sig.hash_ty.to_u32())
             .expect("failed to compute sighash");
-        println!("Legacy sighash: {:x} (sighash flag {})", sighash, sig.sighash_type);
+        println!("Legacy sighash: {:x} (sighash flag {})", sighash, sig.hash_ty);
     }
 }
 
@@ -126,14 +126,9 @@ fn compute_sighash_p2wsh(raw_tx: &[u8], inp_idx: usize, value: u64) {
         assert!((70..=72).contains(&sig_len), "signature length {} out of bounds", sig_len);
         //here we assume that all sighash_flags are the same. Can they be different?
         let sighash = cache
-            .p2wsh_signature_hash(
-                inp_idx,
-                witness_script,
-                Amount::from_sat(value),
-                sig.sighash_type,
-            )
+            .p2wsh_signature_hash(inp_idx, witness_script, Amount::from_sat(value), sig.hash_ty)
             .expect("failed to compute sighash");
-        println!("Segwit p2wsh sighash: {:x} ({})", sighash, sig.sighash_type);
+        println!("Segwit p2wsh sighash: {:x} ({})", sighash, sig.hash_ty);
     }
 }
 
diff --git a/bitcoin/examples/sign-tx-segwit-v0.rs b/bitcoin/examples/sign-tx-segwit-v0.rs
index 6a8879d3..ef7f969c 100644
--- a/bitcoin/examples/sign-tx-segwit-v0.rs
+++ b/bitcoin/examples/sign-tx-segwit-v0.rs
@@ -66,10 +66,10 @@ fn main() {
 
     // Sign the sighash using the secp256k1 library (exported by rust-bitcoin).
     let msg = Message::from(sighash);
-    let signature = secp.sign_ecdsa(&msg, &sk);
+    let sig = secp.sign_ecdsa(&msg, &sk);
 
     // Update the witness stack.
-    let signature = bitcoin::ecdsa::Signature { signature, sighash_type };
+    let signature = bitcoin::ecdsa::Signature { sig, hash_ty: EcdsaSighashType::All };
     let pk = sk.public_key(&secp);
     *sighasher.witness_mut(input_index).unwrap() = Witness::p2wpkh(&signature, &pk);
 
diff --git a/bitcoin/examples/sign-tx-taproot.rs b/bitcoin/examples/sign-tx-taproot.rs
index f8fe3f02..c9025df2 100644
--- a/bitcoin/examples/sign-tx-taproot.rs
+++ b/bitcoin/examples/sign-tx-taproot.rs
@@ -72,10 +72,10 @@ fn main() {
     // Sign the sighash using the secp256k1 library (exported by rust-bitcoin).
     let tweaked: TweakedKeypair = keypair.tap_tweak(&secp, None);
     let msg = Message::from_digest(sighash.to_byte_array());
-    let signature = secp.sign_schnorr(&msg, &tweaked.to_inner());
+    let sig = secp.sign_schnorr(&msg, &tweaked.to_inner());
 
     // Update the witness stack.
-    let signature = bitcoin::taproot::Signature { signature, sighash_type };
+    let signature = bitcoin::taproot::Signature { sig, hash_ty: sighash_type };
     sighasher.witness_mut(input_index).unwrap().push(&signature.to_vec());
 
     // Get the signed transaction.
diff --git a/bitcoin/examples/taproot-psbt.rs b/bitcoin/examples/taproot-psbt.rs
index 66f6b796..1b4d4a81 100644
--- a/bitcoin/examples/taproot-psbt.rs
+++ b/bitcoin/examples/taproot-psbt.rs
@@ -282,14 +282,14 @@ fn generate_bip86_key_spend_tx(
     let unsigned_tx = psbt.unsigned_tx.clone();
     psbt.inputs.iter_mut().enumerate().try_for_each::<_, Result<(), Box<dyn std::error::Error>>>(
         |(vout, input)| {
-            let sighash_type = input
+            let hash_ty = input
                 .sighash_type
                 .and_then(|psbt_sighash_type| psbt_sighash_type.taproot_hash_ty().ok())
                 .unwrap_or(TapSighashType::All);
             let hash = SighashCache::new(&unsigned_tx).taproot_key_spend_signature_hash(
                 vout,
                 &sighash::Prevouts::All(input_txouts.as_slice()),
-                sighash_type,
+                hash_ty,
             )?;
 
             let (_, (_, derivation_path)) = input
@@ -304,7 +304,7 @@ fn generate_bip86_key_spend_tx(
                 None,
                 input,
                 hash,
-                sighash_type,
+                hash_ty,
                 secp,
             );
 
@@ -512,7 +512,7 @@ fn refresh_tx(
             psbt.outputs = vec![Output::default()];
             psbt.unsigned_tx.lock_time = absolute::LockTime::ZERO;
 
-            let sighash_type = input
+            let hash_ty = input
                 .sighash_type
                 .and_then(|psbt_sighash_type| psbt_sighash_type.taproot_hash_ty().ok())
                 .unwrap_or(TapSighashType::All);
@@ -522,7 +522,7 @@ fn refresh_tx(
                     value: input_value,
                     script_pubkey: prevout_script_pubkey,
                 }]),
-                sighash_type,
+                hash_ty,
             )?;
 
             {
@@ -538,7 +538,7 @@ fn refresh_tx(
                     None,
                     input,
                     hash,
-                    sighash_type,
+                    hash_ty,
                     &self.secp,
                 );
             }
@@ -654,7 +654,7 @@ fn spend_inheritance(
             let secret_key =
                 self.master_xpriv.derive_priv(&self.secp, &derivation_path)?.to_priv().inner;
             for lh in leaf_hashes {
-                let sighash_type = TapSighashType::All;
+                let hash_ty = TapSighashType::All;
                 let hash = SighashCache::new(&unsigned_tx).taproot_script_spend_signature_hash(
                     0,
                     &sighash::Prevouts::All(&[TxOut {
@@ -662,7 +662,7 @@ fn spend_inheritance(
                         script_pubkey: input_script_pubkey.clone(),
                     }]),
                     *lh,
-                    sighash_type,
+                    hash_ty,
                 )?;
                 sign_psbt_taproot(
                     &secret_key,
@@ -670,7 +670,7 @@ fn spend_inheritance(
                     Some(*lh),
                     &mut psbt.inputs[0],
                     hash,
-                    sighash_type,
+                    hash_ty,
                     &self.secp,
                 );
             }
@@ -730,7 +730,7 @@ fn sign_psbt_taproot(
     leaf_hash: Option<TapLeafHash>,
     psbt_input: &mut psbt::Input,
     hash: TapSighash,
-    sighash_type: TapSighashType,
+    hash_ty: TapSighashType,
     secp: &Secp256k1<secp256k1::All>,
 ) {
     let keypair = secp256k1::Keypair::from_seckey_slice(secp, secret_key.as_ref()).unwrap();
@@ -740,9 +740,9 @@ fn sign_psbt_taproot(
     };
 
     let msg = secp256k1::Message::from_digest(hash.to_byte_array());
-    let signature = secp.sign_schnorr(&msg, &keypair);
+    let sig = secp.sign_schnorr(&msg, &keypair);
 
-    let final_signature = taproot::Signature { signature, sighash_type };
+    let final_signature = taproot::Signature { sig, hash_ty };
 
     if let Some(lh) = leaf_hash {
         psbt_input.tap_script_sigs.insert((pubkey, lh), final_signature);
diff --git a/bitcoin/src/address/mod.rs b/bitcoin/src/address/mod.rs
index d1d8175e..9934f5ab 100644
--- a/bitcoin/src/address/mod.rs
+++ b/bitcoin/src/address/mod.rs
@@ -132,8 +132,9 @@ impl NetworkValidation for NetworkUnchecked {
 
 /// The inner representation of an address, without the network validation tag.
 ///
-/// This struct represents the inner representation of an address without the network validation
-/// tag, which is used to ensure that addresses are used only on the appropriate network.
+/// An `Address` is composed of a payload and a network. This struct represents the inner
+/// representation of an address without the network validation tag, which is used to ensure that
+/// addresses are used only on the appropriate network.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 enum AddressInner {
     P2pkh { hash: PubkeyHash, network: NetworkKind },
@@ -318,8 +319,11 @@ fn from(n: Network) -> Self { Self::from_network(n) }
 /// * [BIP341 - Taproot: SegWit version 1 spending rules](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki)
 /// * [BIP350 - Bech32m format for v1+ witness addresses](https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki)
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
-// The `#[repr(transparent)]` attribute is used to guarantee the layout of the `Address` struct. It
-// is an implementation detail and users should not rely on it in their code.
+///
+/// The `#[repr(transparent)]` attribute is used to guarantee that the layout of the
+/// `Address` struct is the same as the layout of the `AddressInner` struct. This attribute is
+/// an implementation detail and users should not rely on it in their code.
+///
 #[repr(transparent)]
 pub struct Address<V = NetworkChecked>(AddressInner, PhantomData<V>)
 where
diff --git a/bitcoin/src/bip158.rs b/bitcoin/src/bip158.rs
index 7ada3c3f..57b2f4d3 100644
--- a/bitcoin/src/bip158.rs
+++ b/bitcoin/src/bip158.rs
@@ -668,14 +668,14 @@ fn test_filter() {
         let bytes = out;
 
         {
-            let query = [hex!("abcdef"), hex!("eeeeee")];
+            let query = vec![hex!("abcdef"), hex!("eeeeee")];
             let reader = GcsFilterReader::new(0, 0, M, P);
             assert!(reader
                 .match_any(&mut bytes.as_slice(), &mut query.iter().map(|v| v.as_slice()))
                 .unwrap());
         }
         {
-            let query = [hex!("abcdef"), hex!("123456")];
+            let query = vec![hex!("abcdef"), hex!("123456")];
             let reader = GcsFilterReader::new(0, 0, M, P);
             assert!(!reader
                 .match_any(&mut bytes.as_slice(), &mut query.iter().map(|v| v.as_slice()))
diff --git a/bitcoin/src/blockdata/locktime/absolute.rs b/bitcoin/src/blockdata/locktime/absolute.rs
index 6138aa4f..4099c2b9 100644
--- a/bitcoin/src/blockdata/locktime/absolute.rs
+++ b/bitcoin/src/blockdata/locktime/absolute.rs
@@ -389,20 +389,6 @@ fn visit_i64<E: serde::de::Error>(self, v: i64) -> Result<u32, E> {
     }
 }
 
-#[cfg(feature = "ordered")]
-impl ordered::ArbitraryOrd for LockTime {
-    fn arbitrary_cmp(&self, other: &Self) -> Ordering {
-        use LockTime::*;
-
-        match (self, other) {
-            (Blocks(_), Seconds(_)) => Ordering::Less,
-            (Seconds(_), Blocks(_)) => Ordering::Greater,
-            (Blocks(this), Blocks(that)) => this.cmp(that),
-            (Seconds(this), Seconds(that)) => this.cmp(that),
-        }
-    }
-}
-
 /// An absolute block height, guaranteed to always contain a valid height value.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs
index b7724b69..a052e428 100644
--- a/bitcoin/src/blockdata/script/borrowed.rs
+++ b/bitcoin/src/blockdata/script/borrowed.rs
@@ -357,10 +357,7 @@ pub fn is_op_return(&self) -> bool {
     ///
     /// This method has potentially confusing semantics and an unclear purpose, so it's going to be
     /// removed. Use `is_op_return` if you want `OP_RETURN` semantics.
-    #[deprecated(
-        since = "TBD",
-        note = "The method is not very useful, you might want `is_op_return`"
-    )]
+    #[deprecated(since = "TBD", note = "The method is not very useful, you might want `is_op_return`")]
     #[inline]
     pub fn is_provably_unspendable(&self) -> bool {
         use crate::blockdata::opcodes::Class::{IllegalOp, ReturnOp};
diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs
index caf4274d..414cab22 100644
--- a/bitcoin/src/blockdata/transaction.rs
+++ b/bitcoin/src/blockdata/transaction.rs
@@ -19,11 +19,11 @@
 use io::{Read, Write};
 
 use super::Weight;
-use crate::blockdata::fee_rate::FeeRate;
 use crate::blockdata::locktime::absolute::{self, Height, Time};
 use crate::blockdata::locktime::relative;
 use crate::blockdata::script::{Script, ScriptBuf};
 use crate::blockdata::witness::Witness;
+use crate::blockdata::FeeRate;
 use crate::consensus::{encode, Decodable, Encodable};
 use crate::internal_macros::{impl_consensus_encoding, impl_hashencode};
 use crate::parse::impl_parse_str_from_int_infallible;
@@ -31,7 +31,7 @@
 #[cfg(doc)]
 use crate::sighash::{EcdsaSighashType, TapSighashType};
 use crate::string::FromHexStr;
-use crate::{Amount, VarInt};
+use crate::{Amount, SignedAmount, VarInt};
 
 #[rustfmt::skip]                // Keep public re-exports separate.
 #[cfg(feature = "bitcoinconsensus")]
@@ -226,6 +226,12 @@ pub struct TxIn {
 }
 
 impl TxIn {
+    /// Returns the input base weight.
+    ///
+    /// Base weight excludes the witness and script.
+    const BASE_WEIGHT: Weight =
+        Weight::from_vb_unwrap(OutPoint::SIZE as u64 + Sequence::SIZE as u64);
+
     /// Returns true if this input enables the [`absolute::LockTime`] (aka `nLockTime`) of its
     /// [`Transaction`].
     ///
@@ -1168,6 +1174,29 @@ impl From<&Transaction> for Wtxid {
     fn from(tx: &Transaction) -> Wtxid { tx.wtxid() }
 }
 
+/// Computes the value of an output accounting for the cost of spending it.
+///
+/// The effective value is the value of an output value minus the amount to spend it.  That is, the
+/// effective_value can be calculated as: value - (fee_rate * weight).
+///
+/// Note: the effective value of a [`Transaction`] may increase less than the effective value of
+/// a [`TxOut`] when adding another [`TxOut`] to the transaction.  This happens when the new
+/// [`TxOut`] added causes the output length `VarInt` to increase its encoding length.
+///
+/// # Arguments
+///
+/// * `fee_rate` - the fee rate of the transaction being created.
+/// * `satisfaction_weight` - satisfied spending conditions weight.
+pub fn effective_value(
+    fee_rate: FeeRate,
+    satisfaction_weight: Weight,
+    value: Amount,
+) -> Option<SignedAmount> {
+    let weight = satisfaction_weight.checked_add(TxIn::BASE_WEIGHT)?;
+    let signed_input_fee = fee_rate.checked_mul_by_weight(weight)?.to_signed().ok()?;
+    value.to_signed().ok()?.checked_sub(signed_input_fee)
+}
+
 /// Predicts the weight of a to-be-constructed transaction.
 ///
 /// This function computes the weight of a transaction which is not fully known. All that is needed
@@ -1181,7 +1210,7 @@ fn from(tx: &Transaction) -> Wtxid { tx.wtxid() }
 ///   of the to-be-constructed transaction.
 ///
 /// Note that lengths of the scripts and witness elements must be non-serialized, IOW *without* the
-/// preceding compact size. The length of preceding compact size is computed and added inside the
+/// preceding compact size. The lenght of preceding compact size is computed and added inside the
 /// function for convenience.
 ///
 /// If you  have the transaction already constructed (except for signatures) with a dummy value for
@@ -1989,6 +2018,37 @@ fn sequence_from_str_hex_invalid_hex_should_err() {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn effective_value_happy_path() {
+        let value = Amount::from_str("1 cBTC").unwrap();
+        let fee_rate = FeeRate::from_sat_per_kwu(10);
+        let satisfaction_weight = Weight::from_wu(204);
+        let effective_value = effective_value(fee_rate, satisfaction_weight, value).unwrap();
+
+        // 10 sat/kwu * (204wu + BASE_WEIGHT) = 4 sats
+        let expected_fee = SignedAmount::from_str("4 sats").unwrap();
+        let expected_effective_value = value.to_signed().unwrap() - expected_fee;
+        assert_eq!(effective_value, expected_effective_value);
+    }
+
+    #[test]
+    fn effective_value_fee_rate_does_not_overflow() {
+        let eff_value = effective_value(FeeRate::MAX, Weight::ZERO, Amount::ZERO);
+        assert!(eff_value.is_none());
+    }
+
+    #[test]
+    fn effective_value_weight_does_not_overflow() {
+        let eff_value = effective_value(FeeRate::ZERO, Weight::MAX, Amount::ZERO);
+        assert!(eff_value.is_none());
+    }
+
+    #[test]
+    fn effective_value_value_does_not_overflow() {
+        let eff_value = effective_value(FeeRate::ZERO, Weight::ZERO, Amount::MAX);
+        assert!(eff_value.is_none());
+    }
+
     #[test]
     fn txin_txout_weight() {
         // [(is_segwit, tx_hex, expected_weight)]
diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs
index daf744a7..b7011638 100644
--- a/bitcoin/src/blockdata/witness.rs
+++ b/bitcoin/src/blockdata/witness.rs
@@ -637,9 +637,9 @@ fn test_push_ecdsa_sig() {
         // The very first signature in block 734,958
         let sig_bytes =
             hex!("304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b541");
-        let signature = secp256k1::ecdsa::Signature::from_der(&sig_bytes).unwrap();
+        let sig = secp256k1::ecdsa::Signature::from_der(&sig_bytes).unwrap();
         let mut witness = Witness::default();
-        let signature = crate::ecdsa::Signature { signature, sighash_type: EcdsaSighashType::All };
+        let signature = crate::ecdsa::Signature { sig, hash_ty: EcdsaSighashType::All };
         witness.push_ecdsa_signature(&signature);
         let expected_witness = vec![hex!(
             "304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b54101")
diff --git a/bitcoin/src/crypto/ecdsa.rs b/bitcoin/src/crypto/ecdsa.rs
index 2ce06002..14251fac 100644
--- a/bitcoin/src/crypto/ecdsa.rs
+++ b/bitcoin/src/crypto/ecdsa.rs
@@ -22,24 +22,24 @@
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub struct Signature {
-    /// The underlying ECDSA Signature.
-    pub signature: secp256k1::ecdsa::Signature,
-    /// The corresponding hash type.
-    pub sighash_type: EcdsaSighashType,
+    /// The underlying ECDSA Signature
+    pub sig: secp256k1::ecdsa::Signature,
+    /// The corresponding hash type
+    pub hash_ty: EcdsaSighashType,
 }
 
 impl Signature {
     /// Constructs an ECDSA Bitcoin signature for [`EcdsaSighashType::All`].
-    pub fn sighash_all(signature: secp256k1::ecdsa::Signature) -> Signature {
-        Signature { signature, sighash_type: EcdsaSighashType::All }
+    pub fn sighash_all(sig: secp256k1::ecdsa::Signature) -> Signature {
+        Signature { sig, hash_ty: EcdsaSighashType::All }
     }
 
     /// Deserializes from slice following the standardness rules for [`EcdsaSighashType`].
     pub fn from_slice(sl: &[u8]) -> Result<Self, Error> {
-        let (sighash_type, sig) = sl.split_last().ok_or(Error::EmptySignature)?;
-        let sighash_type = EcdsaSighashType::from_standard(*sighash_type as u32)?;
-        let signature = secp256k1::ecdsa::Signature::from_der(sig).map_err(Error::Secp256k1)?;
-        Ok(Signature { signature, sighash_type })
+        let (hash_ty, sig) = sl.split_last().ok_or(Error::EmptySignature)?;
+        let hash_ty = EcdsaSighashType::from_standard(*hash_ty as u32)?;
+        let sig = secp256k1::ecdsa::Signature::from_der(sig).map_err(Error::Secp256k1)?;
+        Ok(Signature { sig, hash_ty })
     }
 
     /// Serializes an ECDSA signature (inner secp256k1 signature in DER format).
@@ -47,9 +47,9 @@ pub fn from_slice(sl: &[u8]) -> Result<Self, Error> {
     /// This does **not** perform extra heap allocation.
     pub fn serialize(&self) -> SerializedSignature {
         let mut buf = [0u8; MAX_SIG_LEN];
-        let signature = self.signature.serialize_der();
+        let signature = self.sig.serialize_der();
         buf[..signature.len()].copy_from_slice(&signature);
-        buf[signature.len()] = self.sighash_type as u8;
+        buf[signature.len()] = self.hash_ty as u8;
         SerializedSignature { data: buf, len: signature.len() + 1 }
     }
 
@@ -59,19 +59,14 @@ pub fn serialize(&self) -> SerializedSignature {
     /// [`serialize`](Self::serialize) method instead.
     pub fn to_vec(self) -> Vec<u8> {
         // TODO: add support to serialize to a writer to SerializedSig
-        self.signature
-            .serialize_der()
-            .iter()
-            .copied()
-            .chain(iter::once(self.sighash_type as u8))
-            .collect()
+        self.sig.serialize_der().iter().copied().chain(iter::once(self.hash_ty as u8)).collect()
     }
 }
 
 impl fmt::Display for Signature {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        fmt::LowerHex::fmt(&self.signature.serialize_der().as_hex(), f)?;
-        fmt::LowerHex::fmt(&[self.sighash_type as u8].as_hex(), f)
+        fmt::LowerHex::fmt(&self.sig.serialize_der().as_hex(), f)?;
+        fmt::LowerHex::fmt(&[self.hash_ty as u8].as_hex(), f)
     }
 }
 
@@ -82,8 +77,8 @@ fn from_str(s: &str) -> Result<Self, Self::Err> {
         let bytes = Vec::from_hex(s)?;
         let (sighash_byte, signature) = bytes.split_last().ok_or(Error::EmptySignature)?;
         Ok(Signature {
-            signature: secp256k1::ecdsa::Signature::from_der(signature)?,
-            sighash_type: EcdsaSighashType::from_standard(*sighash_byte as u32)?,
+            sig: secp256k1::ecdsa::Signature::from_der(signature)?,
+            hash_ty: EcdsaSighashType::from_standard(*sighash_byte as u32)?,
         })
     }
 }
diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs
index f5ac68d7..9ee810cc 100644
--- a/bitcoin/src/crypto/key.rs
+++ b/bitcoin/src/crypto/key.rs
@@ -202,7 +202,7 @@ pub fn verify<C: secp256k1::Verification>(
         msg: &secp256k1::Message,
         sig: &ecdsa::Signature,
     ) -> Result<(), Error> {
-        Ok(secp.verify_ecdsa(msg, &sig.signature, &self.inner)?)
+        Ok(secp.verify_ecdsa(msg, &sig.sig, &self.inner)?)
     }
 }
 
@@ -321,7 +321,7 @@ pub fn verify<C: secp256k1::Verification>(
         msg: &secp256k1::Message,
         sig: &ecdsa::Signature,
     ) -> Result<(), Error> {
-        Ok(secp.verify_ecdsa(msg, &sig.signature, &self.0)?)
+        Ok(secp.verify_ecdsa(msg, &sig.sig, &self.0)?)
     }
 }
 
diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs
index 4ed1cf53..4b805281 100644
--- a/bitcoin/src/crypto/sighash.rs
+++ b/bitcoin/src/crypto/sighash.rs
@@ -586,10 +586,10 @@ pub(crate) fn split_anyonecanpay_flag(self) -> (TapSighashType, bool) {
     }
 
     /// Constructs a [`TapSighashType`] from a raw `u8`.
-    pub fn from_consensus_u8(sighash_type: u8) -> Result<Self, InvalidSighashTypeError> {
+    pub fn from_consensus_u8(hash_ty: u8) -> Result<Self, InvalidSighashTypeError> {
         use TapSighashType::*;
 
-        Ok(match sighash_type {
+        Ok(match hash_ty {
             0x00 => Default,
             0x01 => All,
             0x02 => None,
diff --git a/bitcoin/src/crypto/taproot.rs b/bitcoin/src/crypto/taproot.rs
index c0a1a2d7..ca771c8c 100644
--- a/bitcoin/src/crypto/taproot.rs
+++ b/bitcoin/src/crypto/taproot.rs
@@ -18,10 +18,10 @@
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub struct Signature {
-    /// The underlying schnorr signature.
-    pub signature: secp256k1::schnorr::Signature,
-    /// The corresponding hash type.
-    pub sighash_type: TapSighashType,
+    /// The underlying schnorr signature
+    pub sig: secp256k1::schnorr::Signature,
+    /// The corresponding hash type
+    pub hash_ty: TapSighashType,
 }
 
 impl Signature {
@@ -30,14 +30,14 @@ pub fn from_slice(sl: &[u8]) -> Result<Self, SigFromSliceError> {
         match sl.len() {
             64 => {
                 // default type
-                let signature = secp256k1::schnorr::Signature::from_slice(sl)?;
-                Ok(Signature { signature, sighash_type: TapSighashType::Default })
+                let sig = secp256k1::schnorr::Signature::from_slice(sl)?;
+                Ok(Signature { sig, hash_ty: TapSighashType::Default })
             }
             65 => {
-                let (sighash_type, signature) = sl.split_last().expect("Slice len checked == 65");
-                let sighash_type = TapSighashType::from_consensus_u8(*sighash_type)?;
-                let signature = secp256k1::schnorr::Signature::from_slice(signature)?;
-                Ok(Signature { signature, sighash_type })
+                let (hash_ty, sig) = sl.split_last().expect("Slice len checked == 65");
+                let hash_ty = TapSighashType::from_consensus_u8(*hash_ty)?;
+                let sig = secp256k1::schnorr::Signature::from_slice(sig)?;
+                Ok(Signature { sig, hash_ty })
             }
             len => Err(SigFromSliceError::InvalidSignatureSize(len)),
         }
@@ -48,11 +48,11 @@ pub fn from_slice(sl: &[u8]) -> Result<Self, SigFromSliceError> {
     /// Note: this allocates on the heap, prefer [`serialize`](Self::serialize) if vec is not needed.
     pub fn to_vec(self) -> Vec<u8> {
         // TODO: add support to serialize to a writer to SerializedSig
-        let mut ser_sig = self.signature.as_ref().to_vec();
-        if self.sighash_type == TapSighashType::Default {
+        let mut ser_sig = self.sig.as_ref().to_vec();
+        if self.hash_ty == TapSighashType::Default {
             // default sighash type, don't add extra sighash byte
         } else {
-            ser_sig.push(self.sighash_type as u8);
+            ser_sig.push(self.hash_ty as u8);
         }
         ser_sig
     }
@@ -63,13 +63,13 @@ pub fn to_vec(self) -> Vec<u8> {
     /// You can get a slice from it using deref coercions or turn it into an iterator.
     pub fn serialize(self) -> SerializedSignature {
         let mut buf = [0; serialized_signature::MAX_LEN];
-        let ser_sig = self.signature.serialize();
+        let ser_sig = self.sig.serialize();
         buf[..64].copy_from_slice(&ser_sig);
-        let len = if self.sighash_type == TapSighashType::Default {
+        let len = if self.hash_ty == TapSighashType::Default {
             // default sighash type, don't add extra sighash byte
             64
         } else {
-            buf[64] = self.sighash_type as u8;
+            buf[64] = self.hash_ty as u8;
             65
         };
         SerializedSignature::from_raw_parts(buf, len)
diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs
index 25265c8d..f92e8b93 100644
--- a/bitcoin/src/lib.rs
+++ b/bitcoin/src/lib.rs
@@ -40,7 +40,6 @@
 #![allow(clippy::needless_question_mark)] // https://github.com/rust-bitcoin/rust-bitcoin/pull/2134
 #![allow(clippy::uninhabited_references)] // falsely claims that 100% safe code is UB
 #![allow(clippy::manual_range_contains)] // more readable than clippy's format
-#![allow(clippy::unconditional_recursion)] // broken; see https://github.com/rust-lang/rust-clippy/issues/12133
 
 // Disable 16-bit support at least for now as we can't guarantee it yet.
 #[cfg(target_pointer_width = "16")]
@@ -71,10 +70,6 @@
 /// Re-export the `bitcoin-io` crate.
 pub extern crate io;
 
-/// Re-export the `ordered` crate.
-#[cfg(feature = "ordered")]
-pub extern crate ordered;
-
 /// Rust wrapper library for Pieter Wuille's libsecp256k1.  Implements ECDSA and BIP 340 signatures
 /// for the SECG elliptic curve group secp256k1 and related utilities.
 pub extern crate secp256k1;
diff --git a/bitcoin/src/pow.rs b/bitcoin/src/pow.rs
index 68afaab4..24772b68 100644
--- a/bitcoin/src/pow.rs
+++ b/bitcoin/src/pow.rs
@@ -1676,13 +1676,13 @@ fn u256_max_min_inverse_roundtrip() {
     }
 
     #[test]
-    fn u256_wrapping_add_wraps_at_boundary() {
+    fn u256_wrapping_add_wraps_at_boundry() {
         assert_eq!(U256::MAX.wrapping_add(U256::ONE), U256::ZERO);
         assert_eq!(U256::MAX.wrapping_add(U256::from(2_u8)), U256::ONE);
     }
 
     #[test]
-    fn u256_wrapping_sub_wraps_at_boundary() {
+    fn u256_wrapping_sub_wraps_at_boundry() {
         assert_eq!(U256::ZERO.wrapping_sub(U256::ONE), U256::MAX);
         assert_eq!(U256::ONE.wrapping_sub(U256::from(2_u8)), U256::MAX);
     }
diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs
index 13943728..b59d55f6 100644
--- a/bitcoin/src/psbt/mod.rs
+++ b/bitcoin/src/psbt/mod.rs
@@ -371,10 +371,8 @@ fn bip32_sign_ecdsa<C, K, T>(
                 Ok((msg, sighash_ty)) => (msg, sighash_ty),
             };
 
-            let sig = ecdsa::Signature {
-                signature: secp.sign_ecdsa(&msg, &sk.inner),
-                sighash_type: sighash_ty,
-            };
+            let sig =
+                ecdsa::Signature { sig: secp.sign_ecdsa(&msg, &sk.inner), hash_ty: sighash_ty };
 
             let pk = sk.public_key(secp);
 
diff --git a/hashes/README.md b/hashes/README.md
index 4ce91fa9..a6143d70 100644
--- a/hashes/README.md
+++ b/hashes/README.md
@@ -11,6 +11,18 @@ since these are needed to display hashes anway.
 
 This library should always compile with any combination of features on **Rust 1.56.1**.
 
+
+To build with the MSRV you will need to pin `serde` (if you have either the `serde` or the
+`schemars` feature enabled)
+
+```
+# serde 1.0.157 uses syn 2.0 which requires edition 2021
+cargo update -p serde --precise 1.0.156
+```
+
+before building. (And if your code is a library, your downstream users will need to run these
+commands, and so on.)
+
 ## Contributions
 
 Contributions are welcome, including additional hash function implementations.
diff --git a/io/README.md b/io/README.md
deleted file mode 100644
index e38e2d06..00000000
--- a/io/README.md
+++ /dev/null
@@ -1,10 +0,0 @@
-Rust-Bitcoin IO Library
-=======================
-
-The `std::io` module is not exposed in `no-std` Rust so building `no-std` applications which require
-reading and writing objects via standard traits is not generally possible. Thus, this library exists
-to export a minmal version of `std::io`'s traits which we use in `rust-bitcoin` so that we can
-support `no-std` applications.
-
-These traits are not one-for-one drop-ins, but are as close as possible while still implementing
-`std::io`'s traits without unnecessary complexity.
diff --git a/io/src/lib.rs b/io/src/lib.rs
index f820140e..8d234c19 100644
--- a/io/src/lib.rs
+++ b/io/src/lib.rs
@@ -1,11 +1,16 @@
 //! Rust-Bitcoin IO Library
 //!
-//! The `std::io` module is not exposed in `no-std` Rust so building `no-std` applications which
-//! require reading and writing objects via standard traits is not generally possible. Thus, this
-//! library exists to export a minmal version of `std::io`'s traits which we use in `rust-bitcoin`
-//! so that we can support `no-std` applications.
+//! Because the core `std::io` module is not yet exposed in `no-std` Rust, building `no-std`
+//! applications which require reading and writing objects via standard traits is not generally
+//! possible. While there is ongoing work to improve this situation, this module is not likely to
+//! be available for applications with broad rustc version support for some time.
 //!
-//! These traits are not one-for-one drop-ins, but are as close as possible while still implementing
+//! Thus, this library exists to export a minmal version of `std::io`'s traits which `no-std`
+//! applications may need. With the `std` feature, these traits are also implemented for the
+//! `std::io` traits, allowing standard objects to be used wherever the traits from this crate are
+//! required.
+//!
+//! This traits are not one-for-one drop-ins, but are as close as possible while still implementing
 //! `std::io`'s traits without unnecessary complexity.
 
 #![cfg_attr(not(feature = "std"), no_std)]


Test patch:
diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs
index d2a970a0..88c641d4 100644
--- a/bitcoin/src/blockdata/script/tests.rs
+++ b/bitcoin/src/blockdata/script/tests.rs
@@ -176,7 +176,7 @@ fn p2pk_public_key_compressed_key_returns_some() {
 
 #[test]
 fn script_x_only_key() {
-    // Notice the "20" which prepends the keystr. That 20 is hexadecimal for "32". The Builder automatically adds the 32 opcode
+    // Notice the "20" which prepends the keystr. That 20 is hexidecimal for "32". The Builder automatically adds the 32 opcode
     // to our script in order to give a heads up to the script compiler that it should add the next 32 bytes to the stack.
     // From: https://github.com/bitcoin-core/btcdeb/blob/e8c2750c4a4702768c52d15640ed03bf744d2601/doc/tapscript-example.md?plain=1#L43
     const KEYSTR: &str = "209997a497d964fc1a62885b05a51166a65a90df00492c8d7cf61d6accf54803be";
diff --git a/bitcoin/tests/serde.rs b/bitcoin/tests/serde.rs
index 6112ce6d..04686a7c 100644
--- a/bitcoin/tests/serde.rs
+++ b/bitcoin/tests/serde.rs
@@ -174,8 +174,8 @@ fn serde_regression_extended_pub_key() {
 fn serde_regression_ecdsa_sig() {
     let s = include_str!("data/serde/ecdsa_sig_hex");
     let sig = ecdsa::Signature {
-        signature: secp256k1::ecdsa::Signature::from_str(s.trim()).unwrap(),
-        sighash_type: EcdsaSighashType::All,
+        sig: secp256k1::ecdsa::Signature::from_str(s.trim()).unwrap(),
+        hash_ty: EcdsaSighashType::All,
     };
 
     let got = serialize(&sig).unwrap();
@@ -348,8 +348,8 @@ fn serde_regression_proprietary_key() {
 fn serde_regression_taproot_sig() {
     let s = include_str!("data/serde/taproot_sig_hex");
     let sig = taproot::Signature {
-        signature: secp256k1::schnorr::Signature::from_str(s.trim()).unwrap(),
-        sighash_type: TapSighashType::All,
+        sig: secp256k1::schnorr::Signature::from_str(s.trim()).unwrap(),
+        hash_ty: TapSighashType::All,
     };
 
     let got = serialize(&sig).unwrap();