Merge rust-bitcoin/rust-bitcoin#1330: Remove `PackedLockTime` in favor of `absolute::LockTime`

1b15a13e5a run cargo clippy and fmt (Andrew Poelstra)
f2a5596899 examples: clean up taproot PSBT example locktime handling (Andrew Poelstra)
821842e1a1 drop Ord on absolute::LockTime; add Ord to Transaction (Andrew Poelstra)
5b7d801ee6 remove PackedLockTime type (Andrew Poelstra)
4dee116b8a delete PackedLockTime by aliasing it to LockTime (Andrew Poelstra)
fa81568fb6 locktime: add `FromHexStr` impl for `LockTime` (Andrew Poelstra)
74ff4946e4 locktime: unify serde impls (Andrew Poelstra)

Pull request description:

  This is potentially a controversial PR, but hear me out. My argument is that right now `absolute::LockTime` and `PackedLockTime` are basically identical types; they can be converted between each other with `From` and they have exactly the same semantics except that `PackedLockTime` has `Ord` on it. This makes it very confusing to tell which one should be used, for example in PSBT2 where there are now extra locktime-related fields.

  The motivation for having `PackedLockTime` independent of `LockTime` are:
  * `PackedLockTime` is theoretically more efficient because you don't need to unpack the field, don't need to store a enum discriminate, etc. I don't buy this. If you are trying to save individual bytes in your transaction-parsing there are lots of places you'd look before messing around with locktimes, so we shouldn't privilege this specific thing.
  * `PackedLockTIme` has an `Ord` impl, and removing that will have a cascading effect on transactions, blocks, etc., preventing them from being held in `BTreeMaps` etc. **My proposal**, implemented here, is to just manually impl `Ord` on `Transaction` and don't impl it on `LockTime`.

  I recall some argument that we need to be able to sort miniscripts, and miniscripts might have locktimes in them, but I think this is wrong -- miniscripts always have explicitly either a `Height` or a `Time`, and there is no problem ordering these.

  Closes #1455

ACKs for top commit:
  sanket1729:
    code review ACK 1b15a13e5a
  tcharding:
    ACK 1b15a13e5a

Tree-SHA512: d9776f14560c3822980e8fff8978bf8ca753035152f957a84af25d063c66e4e9df3d5cf98af38d6cb59cc52ba407282f722925b1ca670ae623ac91add3149d2f

bitcoin/examples/ecdsa-psbt.rs

@@ -185,7 +185,7 @@ impl WatchOnly {
 
         let tx = Transaction {
             version: 2,
-            lock_time: absolute::PackedLockTime::ZERO,
+            lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn {
                 previous_output: OutPoint {
                     txid: Txid::from_hex(INPUT_UTXO_TXID)?,

bitcoin/examples/taproot-psbt.rs

@@ -144,7 +144,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             ExtendedPrivKey::from_str(BENEFACTOR_XPRIV_STR)?,
             beneficiary.master_xpub(),
         )?;
-        let (tx, psbt) = benefactor.create_inheritance_funding_tx(1000, UTXO_2)?;
+        let (tx, psbt) = benefactor.create_inheritance_funding_tx(
+            absolute::LockTime::from_height(1000).unwrap(),
+            UTXO_2,
+        )?;
         let tx_hex = encode::serialize_hex(&tx);
 
         println!("Inheritance funding tx hex:\n\n{}", tx_hex);
@@ -154,7 +157,11 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         // And mine a block to confirm the transaction:
         // bt generatetoaddress 1 $(bt-benefactor getnewaddress '' 'bech32m')
 
-        let spending_tx = beneficiary.spend_inheritance(psbt, 1000, to_address)?;
+        let spending_tx = beneficiary.spend_inheritance(
+            psbt,
+            absolute::LockTime::from_height(1000).unwrap(),
+            to_address,
+        )?;
         let spending_tx_hex = encode::serialize_hex(&spending_tx);
         println!("\nInheritance spending tx hex:\n\n{}", spending_tx_hex);
         // If you try to broadcast now, the transaction will be rejected as it is timelocked.
@@ -176,7 +183,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
             ExtendedPrivKey::from_str(BENEFACTOR_XPRIV_STR)?,
             beneficiary.master_xpub(),
         )?;
-        let (tx, _) = benefactor.create_inheritance_funding_tx(2000, UTXO_3)?;
+        let (tx, _) = benefactor.create_inheritance_funding_tx(
+            absolute::LockTime::from_height(2000).unwrap(),
+            UTXO_3,
+        )?;
         let tx_hex = encode::serialize_hex(&tx);
 
         println!("Inheritance funding tx hex:\n\n{}", tx_hex);
@@ -225,7 +235,7 @@ fn generate_bip86_key_spend_tx(
     // CREATOR + UPDATER
     let tx1 = Transaction {
         version: 2,
-        lock_time: absolute::PackedLockTime::ZERO,
+        lock_time: absolute::LockTime::ZERO,
         input: vec![TxIn {
             previous_output: OutPoint {
                 txid: Txid::from_hex(input_utxo.txid)?,
@@ -357,9 +367,9 @@ impl BenefactorWallet {
         })
     }
 
-    fn time_lock_script(locktime: u32, beneficiary_key: XOnlyPublicKey) -> Script {
+    fn time_lock_script(locktime: absolute::LockTime, beneficiary_key: XOnlyPublicKey) -> Script {
         script::Builder::new()
-            .push_int(locktime as i64)
+            .push_int(locktime.to_consensus_u32() as i64)
             .push_opcode(OP_CLTV)
             .push_opcode(OP_DROP)
             .push_x_only_key(&beneficiary_key)
@@ -369,7 +379,7 @@ impl BenefactorWallet {
 
     fn create_inheritance_funding_tx(
         &mut self,
-        lock_time: u32,
+        lock_time: absolute::LockTime,
         input_utxo: P2trUtxo,
     ) -> Result<(Transaction, Psbt), Box<dyn std::error::Error>> {
         if let ChildNumber::Normal { index } = self.next {
@@ -412,7 +422,7 @@ impl BenefactorWallet {
         // CREATOR + UPDATER
         let next_tx = Transaction {
             version: 2,
-            lock_time: absolute::PackedLockTime(lock_time),
+            lock_time,
             input: vec![TxIn {
                 previous_output: OutPoint { txid: tx.txid(), vout: 0 },
                 script_sig: Script::new(),
@@ -482,7 +492,10 @@ impl BenefactorWallet {
                 self.beneficiary_xpub.derive_pub(&self.secp, &new_derivation_path)?.to_x_only_pub();
 
             // Build up the leaf script and combine with internal key into a taproot commitment
-            let lock_time = psbt.unsigned_tx.lock_time.to_u32() + lock_time_delta;
+            let lock_time = absolute::LockTime::from_height(
+                psbt.unsigned_tx.lock_time.to_consensus_u32() + lock_time_delta,
+            )
+            .unwrap();
             let script = Self::time_lock_script(lock_time, beneficiary_key);
             let leaf_hash = TapLeafHash::from_script(&script, LeafVersion::TapScript);
 
@@ -501,7 +514,7 @@ impl BenefactorWallet {
             psbt.unsigned_tx.output =
                 vec![TxOut { script_pubkey: output_script_pubkey.clone(), value: output_value }];
             psbt.outputs = vec![Output::default()];
-            psbt.unsigned_tx.lock_time = absolute::PackedLockTime::ZERO;
+            psbt.unsigned_tx.lock_time = absolute::LockTime::ZERO;
 
             let hash_ty = input
                 .sighash_type
@@ -557,7 +570,7 @@ impl BenefactorWallet {
 
             let next_tx = Transaction {
                 version: 2,
-                lock_time: absolute::PackedLockTime(lock_time),
+                lock_time,
                 input: vec![TxIn {
                     previous_output: OutPoint { txid: tx.txid(), vout: 0 },
                     script_sig: Script::new(),
@@ -626,13 +639,13 @@ impl BeneficiaryWallet {
     fn spend_inheritance(
         &self,
         mut psbt: Psbt,
-        lock_time: u32,
+        lock_time: absolute::LockTime,
         to_address: Address,
     ) -> Result<Transaction, Box<dyn std::error::Error>> {
         let input_value = psbt.inputs[0].witness_utxo.as_ref().unwrap().value;
         let input_script_pubkey =
             psbt.inputs[0].witness_utxo.as_ref().unwrap().script_pubkey.clone();
-        psbt.unsigned_tx.lock_time = absolute::PackedLockTime(lock_time);
+        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,

bitcoin/src/amount.rs

@@ -1470,8 +1470,14 @@ mod verification {
         let n2 = kani::any::<i64>();
         kani::assume(n1.checked_add(n2).is_some()); // assume we don't overflow in the actual test
         kani::assume(n1.checked_sub(n2).is_some()); // assume we don't overflow in the actual test
-        assert_eq!(SignedAmount::from_sat(n1) + SignedAmount::from_sat(n2), SignedAmount::from_sat(n1 + n2));
+        assert_eq!(
-        assert_eq!(SignedAmount::from_sat(n1) - SignedAmount::from_sat(n2), SignedAmount::from_sat(n1 - n2));
+            SignedAmount::from_sat(n1) + SignedAmount::from_sat(n2),
+            SignedAmount::from_sat(n1 + n2)
+        );
+        assert_eq!(
+            SignedAmount::from_sat(n1) - SignedAmount::from_sat(n2),
+            SignedAmount::from_sat(n1 - n2)
+        );
 
         let mut amt = SignedAmount::from_sat(n1);
         amt += SignedAmount::from_sat(n2);
@@ -1506,7 +1512,11 @@ mod verification {
 
         assert_eq!(
             SignedAmount::from_sat(n1).positive_sub(SignedAmount::from_sat(n2)),
-            if n1 >= 0 && n2 >= 0 && n1 >= n2 { Some(SignedAmount::from_sat(n1 - n2)) } else { None },
+            if n1 >= 0 && n2 >= 0 && n1 >= n2 {
+                Some(SignedAmount::from_sat(n1 - n2))
+            } else {
+                None
+            },
         );
     }
 }

bitcoin/src/bip152.rs

@@ -384,7 +384,7 @@ mod test {
     fn dummy_tx(nonce: &[u8]) -> Transaction {
         Transaction {
             version: 1,
-            lock_time: absolute::LockTime::from_consensus(2).into(),
+            lock_time: absolute::LockTime::from_consensus(2),
             input: vec![TxIn {
                 previous_output: OutPoint::new(Txid::hash(nonce), 0),
                 script_sig: Script::new(),

bitcoin/src/blockdata/constants.rs

@@ -67,7 +67,7 @@ fn bitcoin_genesis_tx() -> Transaction {
     // Base
     let mut ret = Transaction {
         version: 1,
-        lock_time: absolute::PackedLockTime::ZERO,
+        lock_time: absolute::LockTime::ZERO,
         input: vec![],
         output: vec![],
     };
@@ -213,7 +213,7 @@ mod test {
         assert_eq!(serialize(&gen.output[0].script_pubkey),
                    Vec::from_hex("434104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac").unwrap());
         assert_eq!(gen.output[0].value, 50 * COIN_VALUE);
-        assert_eq!(gen.lock_time, absolute::PackedLockTime::ZERO);
+        assert_eq!(gen.lock_time, absolute::LockTime::ZERO);
 
         assert_eq!(gen.wtxid().to_hex(), "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b");
     }

bitcoin/src/blockdata/locktime/absolute.rs

@@ -37,123 +37,6 @@ use crate::absolute;
 /// [Bitcoin Core]: https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/script/script.h#L39
 pub const LOCK_TIME_THRESHOLD: u32 = 500_000_000;
 
-/// Packed lock time wraps a [`LockTime`] consensus value i.e., the raw `u32` used by the network.
-///
-/// This struct may be preferred in performance-critical applications because it's slightly smaller
-/// than [`LockTime`] and has a bit more performant (de)serialization. In particular, this may be
-/// relevant when the value is not processed, just passed around. Note however that the difference
-/// is super-small, so unless you do something extreme you shouldn't worry about it.
-///
-/// This type implements a naive ordering based on the `u32`, this is _not_ a semantically correct
-/// ordering for a lock time, hence [`LockTime`] does not implement `Ord`. This type is useful if
-/// you want to use a lock time as part of a struct and wish to derive `Ord`. For all other uses,
-/// consider using [`LockTime`] directly.
-///
-/// # Examples
-/// ```
-/// # use bitcoin::Amount;
-/// # use bitcoin::absolute::{PackedLockTime, LockTime};
-/// #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
-/// struct S {
-///     lock_time: PackedLockTime,
-///     amount: Amount,
-/// }
-///
-/// let _ = S {
-///     lock_time: LockTime::from_consensus(741521).into(),
-///     amount: Amount::from_sat(10_000_000),
-/// };
-/// ```
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
-pub struct PackedLockTime(pub u32);
-
-impl PackedLockTime {
-    /// If [`crate::Transaction::lock_time`] is set to zero it is ignored, in other words a
-    /// transaction with nLocktime==0 is able to be included immediately in any block.
-    pub const ZERO: PackedLockTime = PackedLockTime(0);
-
-    /// Returns the inner `u32`.
-    #[inline]
-    pub fn to_u32(self) -> u32 {
-        self.0
-    }
-}
-
-impl fmt::Display for PackedLockTime {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        fmt::Display::fmt(&self.0, f)
-    }
-}
-
-impl FromHexStr for PackedLockTime {
-    type Error = Error;
-
-    fn from_hex_str_no_prefix<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, Self::Error> {
-        let packed_lock_time = crate::parse::hex_u32(s)?;
-        Ok(Self(packed_lock_time))
-    }
-}
-
-impl Encodable for PackedLockTime {
-    #[inline]
-    fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
-        self.0.consensus_encode(w)
-    }
-}
-
-impl Decodable for PackedLockTime {
-    #[inline]
-    fn consensus_decode<R: Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
-        u32::consensus_decode(r).map(PackedLockTime)
-    }
-}
-
-impl From<LockTime> for PackedLockTime {
-    fn from(n: LockTime) -> Self {
-        PackedLockTime(n.to_consensus_u32())
-    }
-}
-
-impl From<PackedLockTime> for LockTime {
-    fn from(n: PackedLockTime) -> Self {
-        LockTime::from_consensus(n.0)
-    }
-}
-
-impl From<&LockTime> for PackedLockTime {
-    fn from(n: &LockTime) -> Self {
-        PackedLockTime(n.to_consensus_u32())
-    }
-}
-
-impl From<&PackedLockTime> for LockTime {
-    fn from(n: &PackedLockTime) -> Self {
-        LockTime::from_consensus(n.0)
-    }
-}
-
-impl From<PackedLockTime> for u32 {
-    fn from(p: PackedLockTime) -> Self {
-        p.0
-    }
-}
-
-impl_parse_str_through_int!(PackedLockTime);
-
-impl fmt::LowerHex for PackedLockTime {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{:x}", self.0)
-    }
-}
-
-impl fmt::UpperHex for PackedLockTime {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{:X}", self.0)
-    }
-}
-
 /// An absolute lock time value, representing either a block height or a UNIX timestamp (seconds
 /// since epoch).
 ///
@@ -179,8 +62,6 @@ impl fmt::UpperHex for PackedLockTime {
 /// ```
 #[allow(clippy::derive_ord_xor_partial_ord)]
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub enum LockTime {
     /// A block height lock time value.
     ///
@@ -425,6 +306,15 @@ impl fmt::Display for LockTime {
     }
 }
 
+impl FromHexStr for LockTime {
+    type Error = Error;
+
+    fn from_hex_str_no_prefix<S: AsRef<str> + Into<String>>(s: S) -> Result<Self, Self::Error> {
+        let packed_lock_time = crate::parse::hex_u32(s)?;
+        Ok(Self::from_consensus(packed_lock_time))
+    }
+}
+
 impl Encodable for LockTime {
     #[inline]
     fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
@@ -440,6 +330,44 @@ impl Decodable for LockTime {
     }
 }
 
+#[cfg(feature = "serde")]
+impl serde::Serialize for LockTime {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.serialize_u32(self.to_consensus_u32())
+    }
+}
+
+#[cfg(feature = "serde")]
+impl<'de> serde::Deserialize<'de> for LockTime {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        struct Visitor;
+        impl<'de> serde::de::Visitor<'de> for Visitor {
+            type Value = u32;
+            fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("a u32") }
+            // We cannot just implement visit_u32 because JSON (among other things) always
+            // calls visit_u64, even when called from Deserializer::deserialize_u32. The
+            // other visit_u*s have default implementations that forward to visit_u64.
+            fn visit_u64<E: serde::de::Error>(self, v: u64) -> Result<u32, E> {
+                use core::convert::TryInto;
+                v.try_into().map_err(|_| E::invalid_value(serde::de::Unexpected::Unsigned(v), &"a 32-bit number"))
+            }
+            // Also do the signed version, just for good measure.
+            fn visit_i64<E: serde::de::Error>(self, v: i64) -> Result<u32, E> {
+                use core::convert::TryInto;
+                v.try_into().map_err(|_| E::invalid_value(serde::de::Unexpected::Signed(v), &"a 32-bit number"))
+            }
+        }
+        deserializer.deserialize_u32(Visitor).map(LockTime::from_consensus)
+    }
+}
+
+
 /// 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))]
@@ -802,21 +730,21 @@ mod tests {
 
     #[test]
     fn packed_lock_time_from_str_hex_happy_path() {
-        let actual = PackedLockTime::from_hex_str("0xBA70D").unwrap();
+        let actual = LockTime::from_hex_str("0xBA70D").unwrap();
-        let expected = PackedLockTime(0xBA70D);
+        let expected = LockTime::from_consensus(0xBA70D);
         assert_eq!(actual, expected);
     }
 
     #[test]
     fn packed_lock_time_from_str_hex_no_prefix_happy_path() {
-        let lock_time = PackedLockTime::from_hex_str_no_prefix("BA70D").unwrap();
+        let lock_time = LockTime::from_hex_str_no_prefix("BA70D").unwrap();
-        assert_eq!(lock_time, PackedLockTime(0xBA70D));
+        assert_eq!(lock_time, LockTime::from_consensus(0xBA70D));
     }
 
     #[test]
     fn packed_lock_time_from_str_hex_invalid_hex_should_ergr() {
         let hex = "0xzb93";
-        let result = PackedLockTime::from_hex_str(hex);
+        let result = LockTime::from_hex_str(hex);
         assert!(result.is_err());
     }
 

bitcoin/src/blockdata/transaction.rs

@@ -16,7 +16,7 @@ use crate::prelude::*;
 
 use crate::io;
 use crate::string::FromHexStr;
-use core::{fmt, str, default::Default};
+use core::{cmp, fmt, str, default::Default};
 use core::convert::TryFrom;
 
 use bitcoin_internals::write_err;
@@ -580,7 +580,7 @@ impl<E> EncodeSigningDataResult<E> {
 ///
 /// We therefore deviate from the spec by always using the Segwit witness encoding
 /// for 0-input transactions, which results in unambiguously parseable transactions.
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
+#[derive(Clone, PartialEq, Eq, Debug, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub struct Transaction {
@@ -592,13 +592,27 @@ pub struct Transaction {
     ///
     /// * [BIP-65 OP_CHECKLOCKTIMEVERIFY](https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki)
     /// * [BIP-113 Median time-past as endpoint for lock-time calculations](https://github.com/bitcoin/bips/blob/master/bip-0113.mediawiki)
-    pub lock_time: absolute::PackedLockTime,
+    pub lock_time: absolute::LockTime,
     /// List of transaction inputs.
     pub input: Vec<TxIn>,
     /// List of transaction outputs.
     pub output: Vec<TxOut>,
 }
 
+impl cmp::PartialOrd for Transaction {
+    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+impl cmp::Ord for Transaction {
+    fn cmp(&self, other: &Self) -> cmp::Ordering {
+        self.version.cmp(&other.version)
+            .then(self.lock_time.to_consensus_u32().cmp(&other.lock_time.to_consensus_u32()))
+            .then(self.input.cmp(&other.input))
+            .then(self.output.cmp(&other.output))
+    }
+}
+
 impl Transaction {
     /// Computes a "normalized TXID" which does not include any signatures.
     ///
@@ -896,7 +910,7 @@ impl Transaction {
         if !self.is_lock_time_enabled() {
             return true;
         }
-        absolute::LockTime::from(self.lock_time).is_satisfied_by(height, time)
+        self.lock_time.is_satisfied_by(height, time)
     }
 
     /// Returns `true` if this transactions nLockTime is enabled ([BIP-65]).
@@ -1159,7 +1173,7 @@ mod tests {
                    "ce9ea9f6f5e422c6a9dbcddb3b9a14d1c78fab9ab520cb281aa2a74a09575da1".to_string());
         assert_eq!(realtx.input[0].previous_output.vout, 1);
         assert_eq!(realtx.output.len(), 1);
-        assert_eq!(realtx.lock_time, absolute::PackedLockTime::ZERO);
+        assert_eq!(realtx.lock_time, absolute::LockTime::ZERO);
 
         assert_eq!(format!("{:x}", realtx.txid()),
                    "a6eab3c14ab5272a58a5ba91505ba1a4b6d7a3a9fcbd187b6cd99a7b6d548cb7".to_string());
@@ -1193,7 +1207,7 @@ mod tests {
                    "7cac3cf9a112cf04901a51d605058615d56ffe6d04b45270e89d1720ea955859".to_string());
         assert_eq!(realtx.input[0].previous_output.vout, 1);
         assert_eq!(realtx.output.len(), 1);
-        assert_eq!(realtx.lock_time, absolute::PackedLockTime::ZERO);
+        assert_eq!(realtx.lock_time, absolute::LockTime::ZERO);
 
         assert_eq!(format!("{:x}", realtx.txid()),
                    "f5864806e3565c34d1b41e716f72609d00b55ea5eac5b924c9719a842ef42206".to_string());

bitcoin/src/lib.rs

@@ -111,20 +111,18 @@ pub mod string;
 pub mod taproot;
 pub mod util;
 
-#[cfg(feature = "std")]
-use std::io;
-
-#[cfg(not(feature = "std"))]
-use core2::io;
-
 // May depend on crate features and we don't want to bother with it
 #[allow(unused)]
 #[cfg(feature = "std")]
 use std::error::Error as StdError;
+#[cfg(feature = "std")]
+use std::io;
 
 #[allow(unused)]
 #[cfg(not(feature = "std"))]
 use core2::error::Error as StdError;
+#[cfg(not(feature = "std"))]
+use core2::io;
 
 pub use crate::address::{Address, AddressType};
 pub use crate::amount::{Amount, Denomination, SignedAmount};

bitcoin/src/network/constants.rs

@@ -133,11 +133,11 @@ impl Network {
         use Network::*;
 
         let network = match core_arg {
-           "main" => Bitcoin,
+            "main" => Bitcoin,
-           "test" => Testnet,
+            "test" => Testnet,
-           "signet" => Signet, 
+            "signet" => Signet,
-           "regtest" => Regtest,
+            "regtest" => Regtest,
-           _ => return Err(ParseNetworkError(core_arg.to_owned())),
+            _ => return Err(ParseNetworkError(core_arg.to_owned())),
         };
         Ok(network)
     }

bitcoin/src/psbt/mod.rs

@@ -907,7 +907,7 @@ mod tests {
         let psbt = PartiallySignedTransaction {
             unsigned_tx: Transaction {
                 version: 2,
-                lock_time: absolute::PackedLockTime::ZERO,
+                lock_time: absolute::LockTime::ZERO,
                 input: vec![],
                 output: vec![],
             },
@@ -977,7 +977,7 @@ mod tests {
         let expected = PartiallySignedTransaction {
             unsigned_tx: Transaction {
                 version: 2,
-                lock_time: absolute::PackedLockTime(1257139),
+                lock_time: absolute::LockTime::from_consensus(1257139),
                 input: vec![TxIn {
                     previous_output: OutPoint {
                         txid: Txid::from_hex(
@@ -1050,7 +1050,7 @@ mod tests {
         // create some values to use in the PSBT
         let tx = Transaction {
             version: 1,
-            lock_time: absolute::PackedLockTime::ZERO,
+            lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn {
                 previous_output: OutPoint {
                     txid: Txid::from_hex("e567952fb6cc33857f392efa3a46c995a28f69cca4bb1b37e0204dab1ec7a389").unwrap(),
@@ -1237,7 +1237,7 @@ mod tests {
             let unserialized = PartiallySignedTransaction {
                 unsigned_tx: Transaction {
                     version: 2,
-                    lock_time: absolute::PackedLockTime(1257139),
+                    lock_time: absolute::LockTime::from_consensus(1257139),
                     input: vec![TxIn {
                         previous_output: OutPoint {
                             txid: Txid::from_hex(
@@ -1268,7 +1268,7 @@ mod tests {
                 inputs: vec![Input {
                     non_witness_utxo: Some(Transaction {
                         version: 1,
-                        lock_time: absolute::PackedLockTime::ZERO,
+                        lock_time: absolute::LockTime::ZERO,
                         input: vec![TxIn {
                             previous_output: OutPoint {
                                 txid: Txid::from_hex(
@@ -1549,7 +1549,7 @@ mod tests {
         let mut unserialized = PartiallySignedTransaction {
             unsigned_tx: Transaction {
                 version: 2,
-                lock_time: absolute::PackedLockTime(1257139),
+                lock_time: absolute::LockTime::from_consensus(1257139),
                 input: vec![TxIn {
                     previous_output: OutPoint {
                         txid: Txid::from_hex(
@@ -1580,7 +1580,7 @@ mod tests {
             inputs: vec![Input {
                 non_witness_utxo: Some(Transaction {
                     version: 1,
-                    lock_time: absolute::PackedLockTime::ZERO,
+                    lock_time: absolute::LockTime::ZERO,
                     input: vec![TxIn {
                         previous_output: OutPoint {
                             txid: Txid::from_hex(
@@ -1719,7 +1719,7 @@ mod tests {
         let mut t = PartiallySignedTransaction {
             unsigned_tx: Transaction {
                 version: 2,
-                lock_time: absolute::PackedLockTime(1257139),
+                lock_time: absolute::LockTime::from_consensus(1257139),
                 input: vec![TxIn {
                     previous_output: OutPoint {
                         txid: Txid::from_hex(
@@ -1749,7 +1749,7 @@ mod tests {
             inputs: vec![Input {
                 non_witness_utxo: Some(Transaction {
                     version: 1,
-                    lock_time: absolute::PackedLockTime::ZERO,
+                    lock_time: absolute::LockTime::ZERO,
                     input: vec![TxIn {
                         previous_output: OutPoint {
                             txid: Txid::from_hex(
@@ -1818,7 +1818,7 @@ mod tests {
 
         let unsigned_tx = Transaction {
             version: 2,
-            lock_time: absolute::PackedLockTime::ZERO,
+            lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn::default(), TxIn::default()],
             output: vec![TxOut::default()],
         };

bitcoin/src/sighash.rs

@@ -1001,7 +1001,7 @@ impl<R: DerefMut<Target = Transaction>> SighashCache<R> {
     /// use bitcoin::{absolute, Transaction, Script};
     /// use bitcoin::sighash::{EcdsaSighashType, SighashCache};
     ///
-    /// let mut tx_to_sign = Transaction { version: 2, lock_time: absolute::PackedLockTime::ZERO, input: Vec::new(), output: Vec::new() };
+    /// let mut tx_to_sign = Transaction { version: 2, lock_time: absolute::LockTime::ZERO, input: Vec::new(), output: Vec::new() };
     /// let input_count = tx_to_sign.input.len();
     ///
     /// let mut sig_hasher = SighashCache::new(&mut tx_to_sign);
@@ -1075,7 +1075,7 @@ mod tests {
         // We need a tx with more inputs than outputs.
         let tx = Transaction {
             version: 1,
-            lock_time: absolute::PackedLockTime::ZERO,
+            lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn::default(), TxIn::default()],
             output: vec![TxOut::default()],
         };
@@ -1264,7 +1264,7 @@ mod tests {
     fn test_sighash_errors() {
         let dumb_tx = Transaction {
             version: 0,
-            lock_time: absolute::PackedLockTime::ZERO,
+            lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn::default()],
             output: vec![],
         };
@@ -1379,18 +1379,26 @@ mod tests {
     #[cfg(feature = "serde")]
     #[test]
     fn bip_341_sighash_tests() {
-        fn sighash_deser_numeric<'de, D>(deserializer: D) -> Result<SchnorrSighashType, D::Error> where D: actual_serde::Deserializer<'de> {
+        fn sighash_deser_numeric<'de, D>(deserializer: D) -> Result<SchnorrSighashType, D::Error>
+        where
+            D: actual_serde::Deserializer<'de>,
+        {
             use actual_serde::de::{Deserialize, Error, Unexpected};
 
             let raw = u8::deserialize(deserializer)?;
-            SchnorrSighashType::from_consensus_u8(raw)
+            SchnorrSighashType::from_consensus_u8(raw).map_err(|_| {
-                .map_err(|_| D::Error::invalid_value(Unexpected::Unsigned(raw.into()), &"number in range 0-3 or 0x81-0x83"))
+                D::Error::invalid_value(
+                    Unexpected::Unsigned(raw.into()),
+                    &"number in range 0-3 or 0x81-0x83",
+                )
+            })
         }
-            
+
-        use crate::hashes::hex::ToHex;
-        use crate::taproot::{TapTweakHash, TapBranchHash};
         use secp256k1::{self, SecretKey, XOnlyPublicKey};
+
         use crate::consensus::serde as con_serde;
+        use crate::hashes::hex::ToHex;
+        use crate::taproot::{TapBranchHash, TapTweakHash};
 
         #[derive(serde::Deserialize)]
         #[serde(crate = "actual_serde")]
@@ -1480,14 +1488,18 @@ mod tests {
         }
 
         let json_str = include_str!("../tests/data/bip341_tests.json");
-        let mut data = serde_json::from_str::<TestData>(json_str).expect("JSON was not well-formatted");
+        let mut data =
+            serde_json::from_str::<TestData>(json_str).expect("JSON was not well-formatted");
 
         assert_eq!(data.version, 1u64);
         let secp = &secp256k1::Secp256k1::new();
         let key_path = data.key_path_spending.remove(0);
 
         let raw_unsigned_tx = key_path.given.raw_unsigned_tx;
-        let utxos = key_path.given.utxos_spent.into_iter()
+        let utxos = key_path
+            .given
+            .utxos_spent
+            .into_iter()
             .map(|txo| TxOut { value: txo.value, script_pubkey: txo.script_pubkey })
             .collect::<Vec<_>>();
 

bitcoin/tests/psbt.rs

@@ -162,7 +162,7 @@ fn create_transaction() -> Transaction {
 
     Transaction {
         version: 2,
-        lock_time: absolute::PackedLockTime::ZERO,
+        lock_time: absolute::LockTime::ZERO,
         input: vec![
             TxIn {
                 previous_output: OutPoint {

bitcoin/tests/serde.rs

@@ -223,7 +223,7 @@ fn serde_regression_public_key() {
 fn serde_regression_psbt() {
     let tx = Transaction {
         version: 1,
-        lock_time: absolute::PackedLockTime::ZERO,
+        lock_time: absolute::LockTime::ZERO,
         input: vec![TxIn {
             previous_output: OutPoint {
                 txid: Txid::from_hex(

internals/src/hex/buf_encoder.rs

@@ -215,9 +215,7 @@ impl<T: AsOutBytes> BufEncoder<T> {
     ///
     /// Note that this returns the number of bytes before encoding, not number of hex digits.
     #[inline]
-    pub fn space_remaining(&self) -> usize {
+    pub fn space_remaining(&self) -> usize { (self.buf.as_out_bytes().len() - self.pos) / 2 }
-        (self.buf.as_out_bytes().len() - self.pos) / 2
-    }
 }
 
 #[cfg(test)]