Repository: rust-bitcoin/rust-bitcoin
Pull request #2006 (2023-08-18 01:21:32)

Add transaction::Version data type
BIP-68 activated a fair while ago (circa 2019) and since then only transaction versions 1 and 2 have been considered standard.

Currently in our `Transaction` struct we use an `i32`, this means users can construct a non-standard transaction if they do not first look up what the value should be. We can help folk out here by abstracting over the version number.

Add transaction::Version data type that simply provides two consts `ONE` and `TWO`.

Add a `Default` impl on `Version` that returns `Version::TWO`.

In tests that used version 0, instead use `Version::default` because the test obviously does not care.

apoelstra: 5ae780682964700dde2cd03c802a12cdf2a9b4d2 looks good except that we should add a constructor for creating arbitrary version numbers. We can comment that there is no reason (on mainnet at least) to use this, but it should be possible.

Alternately we could make the inner `i32` public.
tcharding: Good reviewing lads, thank you. Changes in force push
- Make the inner `i32` public and document that we do not enforce any invariants on the version number.
- Add `non_standard` constructor
- Add `is_standard` (eg, in case folk want to check after decoding)

Patch:
diff --git a/bitcoin/examples/ecdsa-psbt.rs b/bitcoin/examples/ecdsa-psbt.rs
index 3dbdc99f..63308b23 100644
--- a/bitcoin/examples/ecdsa-psbt.rs
+++ b/bitcoin/examples/ecdsa-psbt.rs
@@ -39,8 +39,8 @@
 use bitcoin::psbt::{self, Input, Psbt, PsbtSighashType};
 use bitcoin::secp256k1::{Secp256k1, Signing, Verification};
 use bitcoin::{
-    Address, Amount, Network, OutPoint, PublicKey, ScriptBuf, Sequence, Transaction, TxIn, TxOut,
-    Witness,
+    transaction, Address, Amount, Network, OutPoint, PublicKey, ScriptBuf, Sequence, Transaction,
+    TxIn, TxOut, Witness,
 };
 
 type Result<T> = std::result::Result<T, Error>;
@@ -177,7 +177,7 @@ fn create_psbt<C: Verification>(&self, secp: &Secp256k1<C>) -> Result<Psbt> {
         let change_amount = Amount::from_str(CHANGE_AMOUNT_BTC)?;
 
         let tx = Transaction {
-            version: 2,
+            version: transaction::Version::TWO,
             lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn {
                 previous_output: OutPoint { txid: INPUT_UTXO_TXID.parse()?, vout: INPUT_UTXO_VOUT },
diff --git a/bitcoin/examples/taproot-psbt.rs b/bitcoin/examples/taproot-psbt.rs
index 4d8e00e8..e8ddb5ba 100644
--- a/bitcoin/examples/taproot-psbt.rs
+++ b/bitcoin/examples/taproot-psbt.rs
@@ -88,8 +88,8 @@
 use bitcoin::sighash::{self, SighashCache, TapSighash, TapSighashType};
 use bitcoin::taproot::{self, LeafVersion, TapLeafHash, TaprootBuilder, TaprootSpendInfo};
 use bitcoin::{
-    absolute, script, Address, Amount, Network, OutPoint, ScriptBuf, Transaction, TxIn, TxOut,
-    Witness,
+    absolute, script, transaction, Address, Amount, Network, OutPoint, ScriptBuf, Transaction,
+    TxIn, TxOut, Witness,
 };
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
@@ -229,7 +229,7 @@ fn generate_bip86_key_spend_tx(
 
     // CREATOR + UPDATER
     let tx1 = Transaction {
-        version: 2,
+        version: transaction::Version::TWO,
         lock_time: absolute::LockTime::ZERO,
         input: vec![TxIn {
             previous_output: OutPoint { txid: input_utxo.txid.parse()?, vout: input_utxo.vout },
@@ -414,7 +414,7 @@ fn create_inheritance_funding_tx(
 
         // CREATOR + UPDATER
         let next_tx = Transaction {
-            version: 2,
+            version: transaction::Version::TWO,
             lock_time,
             input: vec![TxIn {
                 previous_output: OutPoint { txid: tx.txid(), vout: 0 },
@@ -560,7 +560,7 @@ fn refresh_tx(
             .expect("failed to verify transaction");
 
             let next_tx = Transaction {
-                version: 2,
+                version: transaction::Version::TWO,
                 lock_time,
                 input: vec![TxIn {
                     previous_output: OutPoint { txid: tx.txid(), vout: 0 },
diff --git a/bitcoin/src/bip152.rs b/bitcoin/src/bip152.rs
index 2a55e8f6..a74a83d4 100644
--- a/bitcoin/src/bip152.rs
+++ b/bitcoin/src/bip152.rs
@@ -371,6 +371,7 @@ mod test {
 
     use super::*;
     use crate::blockdata::locktime::absolute;
+    use crate::blockdata::transaction;
     use crate::consensus::encode::{deserialize, serialize};
     use crate::hash_types::TxMerkleNode;
     use crate::{
@@ -380,7 +381,7 @@ mod test {
 
     fn dummy_tx(nonce: &[u8]) -> Transaction {
         Transaction {
-            version: 1,
+            version: transaction::Version::ONE,
             lock_time: absolute::LockTime::from_consensus(2),
             input: vec![TxIn {
                 previous_output: OutPoint::new(Txid::hash(nonce), 0),
diff --git a/bitcoin/src/blockdata/constants.rs b/bitcoin/src/blockdata/constants.rs
index c97a5692..9535e8db 100644
--- a/bitcoin/src/blockdata/constants.rs
+++ b/bitcoin/src/blockdata/constants.rs
@@ -17,7 +17,7 @@
 use crate::blockdata::locktime::absolute;
 use crate::blockdata::opcodes::all::*;
 use crate::blockdata::script;
-use crate::blockdata::transaction::{OutPoint, Sequence, Transaction, TxIn, TxOut};
+use crate::blockdata::transaction::{self, OutPoint, Sequence, Transaction, TxIn, TxOut};
 use crate::blockdata::witness::Witness;
 use crate::internal_macros::impl_bytes_newtype;
 use crate::network::Network;
@@ -64,7 +64,7 @@
 fn bitcoin_genesis_tx() -> Transaction {
     // Base
     let mut ret = Transaction {
-        version: 1,
+        version: transaction::Version::ONE,
         lock_time: absolute::LockTime::ZERO,
         input: vec![],
         output: vec![],
@@ -196,6 +196,7 @@ mod test {
 
     use super::*;
     use crate::blockdata::locktime::absolute;
+    use crate::blockdata::transaction;
     use crate::consensus::encode::serialize;
     use crate::internal_macros::hex;
     use crate::network::Network;
@@ -204,7 +205,7 @@ mod test {
     fn bitcoin_genesis_first_transaction() {
         let gen = bitcoin_genesis_tx();
 
-        assert_eq!(gen.version, 1);
+        assert_eq!(gen.version, transaction::Version::ONE);
         assert_eq!(gen.input.len(), 1);
         assert_eq!(gen.input[0].previous_output.txid, Hash::all_zeros());
         assert_eq!(gen.input[0].previous_output.vout, 0xFFFFFFFF);
diff --git a/bitcoin/src/blockdata/fee_rate.rs b/bitcoin/src/blockdata/fee_rate.rs
index 4bb0404e..c42916cf 100644
--- a/bitcoin/src/blockdata/fee_rate.rs
+++ b/bitcoin/src/blockdata/fee_rate.rs
@@ -96,9 +96,9 @@ pub fn checked_mul_by_weight(self, rhs: Weight) -> Option<Amount> {
     /// # Examples
     ///
     /// ```no_run
-    /// # use bitcoin::{absolute, FeeRate, Transaction};
+    /// # use bitcoin::{absolute, transaction, FeeRate, Transaction};
     /// # // Dummy transaction.
-    /// # let tx = Transaction { version: 1, lock_time: absolute::LockTime::ZERO, input: vec![], output: vec![] };
+    /// # let tx = Transaction { version: transaction::Version::ONE, lock_time: absolute::LockTime::ZERO, input: vec![], output: vec![] };
     ///
     /// let rate = FeeRate::from_sat_per_vb(1).expect("1 sat/vbyte is valid");
     /// let fee = rate.fee_wu(tx.weight());
diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs
index 8114929f..15f3952b 100644
--- a/bitcoin/src/blockdata/transaction.rs
+++ b/bitcoin/src/blockdata/transaction.rs
@@ -585,7 +585,7 @@ pub fn minimal_non_dust(script_pubkey: ScriptBuf) -> Self {
 #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub struct Transaction {
     /// The protocol version, is currently expected to be 1 or 2 (BIP 68).
-    pub version: i32,
+    pub version: Version,
     /// Block height or timestamp. Transaction cannot be included in a block until this height/time.
     ///
     /// ### Relevant BIPs
@@ -835,6 +835,50 @@ pub fn script_pubkey_lens(&self) -> impl Iterator<Item = usize> + '_ {
     }
 }
 
+/// The transaction version.
+///
+/// Currently, as specified by [BIP-68], only version 1 and 2 are considered standard.
+///
+/// Standardness of the inner `i32` is not an invariant because you are free to create transactions
+/// of any version, transactions with non-standard version numbers will not be relayed by the
+/// Bitcoin network.
+///
+/// [BIP-68]: https://github.com/bitcoin/bips/blob/master/bip-0068.mediawiki
+#[derive(Copy, PartialEq, Eq, Clone, Debug, PartialOrd, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
+pub struct Version(pub i32);
+
+impl Version {
+    /// The original Bitcoin transaction version (pre-BIP-68).
+    pub const ONE: Self = Self(1);
+
+    /// The second Bitcoin transaction version (post-BIP-68).
+    pub const TWO: Self = Self(2);
+
+    /// Creates a non-standard transaction version.
+    pub fn non_standard(version: i32) -> Version { Self(version) }
+
+    /// Returns true if this transaction version number is considered standard.
+    pub fn is_standard(&self) -> bool { *self == Version::ONE || *self == Version::TWO }
+}
+
+impl Default for Version {
+    fn default() -> Version { Version::TWO }
+}
+
+impl Encodable for Version {
+    fn consensus_encode<W: io::Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
+        self.0.consensus_encode(w)
+    }
+}
+
+impl Decodable for Version {
+    fn consensus_decode<R: io::Read + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
+        Decodable::consensus_decode(r).map(Version)
+    }
+}
+
 impl_consensus_encoding!(TxOut, value, script_pubkey);
 
 impl Encodable for OutPoint {
@@ -921,7 +965,7 @@ impl Decodable for Transaction {
     fn consensus_decode_from_finite_reader<R: io::Read + ?Sized>(
         r: &mut R,
     ) -> Result<Self, encode::Error> {
-        let version = i32::consensus_decode_from_finite_reader(r)?;
+        let version = Version::consensus_decode_from_finite_reader(r)?;
         let input = Vec::<TxIn>::consensus_decode_from_finite_reader(r)?;
         // segwit
         if input.is_empty() {
@@ -1374,7 +1418,7 @@ fn nonsegwit_transaction() {
         let realtx = tx.unwrap();
         // All these tests aren't really needed because if they fail, the hash check at the end
         // will also fail. But these will show you where the failure is so I'll leave them in.
-        assert_eq!(realtx.version, 1);
+        assert_eq!(realtx.version, Version::ONE);
         assert_eq!(realtx.input.len(), 1);
         // In particular this one is easy to get backward -- in bitcoin hashes are encoded
         // as little-endian 256-bit numbers rather than as data strings.
@@ -1415,7 +1459,7 @@ fn segwit_transaction() {
         let realtx = tx.unwrap();
         // All these tests aren't really needed because if they fail, the hash check at the end
         // will also fail. But these will show you where the failure is so I'll leave them in.
-        assert_eq!(realtx.version, 2);
+        assert_eq!(realtx.version, Version::TWO);
         assert_eq!(realtx.input.len(), 1);
         // In particular this one is easy to get backward -- in bitcoin hashes are encoded
         // as little-endian 256-bit numbers rather than as data strings.
@@ -1487,13 +1531,13 @@ fn transaction_version() {
         let tx: Result<Transaction, _> = deserialize(&tx_bytes);
         assert!(tx.is_ok());
         let realtx = tx.unwrap();
-        assert_eq!(realtx.version, 2147483647);
+        assert_eq!(realtx.version, Version::non_standard(2147483647));
 
         let tx2_bytes = hex!("000000800100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0100f2052a01000000434104678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5fac00000000");
         let tx2: Result<Transaction, _> = deserialize(&tx2_bytes);
         assert!(tx2.is_ok());
         let realtx2 = tx2.unwrap();
-        assert_eq!(realtx2.version, -2147483648);
+        assert_eq!(realtx2.version, Version::non_standard(-2147483648));
     }
 
     #[test]
@@ -1781,7 +1825,7 @@ fn txin_txout_weight() {
             ];
 
         let empty_transaction_weight = Transaction {
-            version: 0,
+            version: Version::default(),
             lock_time: absolute::LockTime::ZERO,
             input: vec![],
             output: vec![],
diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs
index 5d304063..cabf7de9 100644
--- a/bitcoin/src/crypto/sighash.rs
+++ b/bitcoin/src/crypto/sighash.rs
@@ -1136,10 +1136,10 @@ impl<R: BorrowMut<Transaction>> SighashCache<R> {
     ///
     /// This allows in-line signing such as
     /// ```
-    /// use bitcoin::{absolute, Amount, Transaction, Script};
+    /// use bitcoin::{absolute, transaction, Amount, Transaction, Script};
     /// use bitcoin::sighash::{EcdsaSighashType, SighashCache};
     ///
-    /// let mut tx_to_sign = Transaction { version: 2, lock_time: absolute::LockTime::ZERO, input: Vec::new(), output: Vec::new() };
+    /// let mut tx_to_sign = Transaction { version: transaction::Version::TWO, 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);
@@ -1262,6 +1262,7 @@ mod tests {
 
     use super::*;
     use crate::blockdata::locktime::absolute;
+    use crate::blockdata::transaction;
     use crate::consensus::deserialize;
     use crate::crypto::sighash::{LegacySighash, TapSighash};
     use crate::internal_macros::hex;
@@ -1275,7 +1276,7 @@ fn sighash_single_bug() {
 
         // We need a tx with more inputs than outputs.
         let tx = Transaction {
-            version: 1,
+            version: transaction::Version::ONE,
             lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn::default(), TxIn::default()],
             output: vec![TxOut::NULL],
@@ -1462,7 +1463,7 @@ fn test_sighashes_with_annex_and_script() {
     #[rustfmt::skip] // Allow long function call `taproot_signature_hash`.
     fn test_sighash_errors() {
         let dumb_tx = Transaction {
-            version: 0,
+            version: transaction::Version::default(),
             lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn::default()],
             output: vec![],
diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs
index 1b43d5e1..6a675e95 100644
--- a/bitcoin/src/psbt/mod.rs
+++ b/bitcoin/src/psbt/mod.rs
@@ -887,7 +887,7 @@ mod tests {
     use crate::bip32::{ChildNumber, KeySource, Xpriv, Xpub};
     use crate::blockdata::locktime::absolute;
     use crate::blockdata::script::ScriptBuf;
-    use crate::blockdata::transaction::{OutPoint, Sequence, Transaction, TxIn, TxOut};
+    use crate::blockdata::transaction::{self, OutPoint, Sequence, Transaction, TxIn, TxOut};
     use crate::blockdata::witness::Witness;
     use crate::internal_macros::hex;
     use crate::network::Network::Bitcoin;
@@ -899,7 +899,7 @@ mod tests {
     fn trivial_psbt() {
         let psbt = Psbt {
             unsigned_tx: Transaction {
-                version: 2,
+                version: transaction::Version::TWO,
                 lock_time: absolute::LockTime::ZERO,
                 input: vec![],
                 output: vec![],
@@ -972,7 +972,7 @@ fn serialize_then_deserialize_output() {
     fn serialize_then_deserialize_global() {
         let expected = Psbt {
             unsigned_tx: Transaction {
-                version: 2,
+                version: transaction::Version::TWO,
                 lock_time: absolute::LockTime::from_consensus(1257139),
                 input: vec![TxIn {
                     previous_output: OutPoint {
@@ -1043,7 +1043,7 @@ fn test_serde_psbt() {
 
         // create some values to use in the PSBT
         let tx = Transaction {
-            version: 1,
+            version: transaction::Version::ONE,
             lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn {
                 previous_output: OutPoint {
@@ -1240,7 +1240,7 @@ fn invalid_vector_5_base64() {
         fn valid_vector_1() {
             let unserialized = Psbt {
                 unsigned_tx: Transaction {
-                    version: 2,
+                    version: transaction::Version::TWO,
                     lock_time: absolute::LockTime::from_consensus(1257139),
                     input: vec![
                         TxIn {
@@ -1272,7 +1272,7 @@ fn valid_vector_1() {
                 inputs: vec![
                     Input {
                         non_witness_utxo: Some(Transaction {
-                            version: 1,
+                            version: transaction::Version::ONE,
                             lock_time: absolute::LockTime::ZERO,
                             input: vec![
                                 TxIn {
@@ -1572,7 +1572,7 @@ fn serialize_and_deserialize_preimage_psbt() {
         // same vector as valid_vector_1 from BIPs with added
         let mut unserialized = Psbt {
             unsigned_tx: Transaction {
-                version: 2,
+                version: transaction::Version::TWO,
                 lock_time: absolute::LockTime::from_consensus(1257139),
                 input: vec![
                     TxIn {
@@ -1604,7 +1604,7 @@ fn serialize_and_deserialize_preimage_psbt() {
             inputs: vec![
                 Input {
                     non_witness_utxo: Some(Transaction {
-                        version: 1,
+                        version: transaction::Version::ONE,
                         lock_time: absolute::LockTime::ZERO,
                         input: vec![
                             TxIn {
@@ -1741,7 +1741,7 @@ fn test_fee() {
 
         let mut t = Psbt {
             unsigned_tx: Transaction {
-                version: 2,
+                version: transaction::Version::TWO,
                 lock_time: absolute::LockTime::from_consensus(1257139),
                 input: vec![
                     TxIn {
@@ -1772,7 +1772,7 @@ fn test_fee() {
             inputs: vec![
                 Input {
                     non_witness_utxo: Some(Transaction {
-                        version: 1,
+                        version: transaction::Version::ONE,
                         lock_time: absolute::LockTime::ZERO,
                         input: vec![
                             TxIn {
@@ -1850,7 +1850,7 @@ fn sign_psbt() {
         use crate::{WPubkeyHash, WitnessProgram};
 
         let unsigned_tx = Transaction {
-            version: 2,
+            version: transaction::Version::TWO,
             lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn::default(), TxIn::default()],
             output: vec![TxOut::NULL],


Test patch:
diff --git a/bitcoin/tests/psbt.rs b/bitcoin/tests/psbt.rs
index 492db965..b2849ea9 100644
--- a/bitcoin/tests/psbt.rs
+++ b/bitcoin/tests/psbt.rs
@@ -7,7 +7,7 @@
 
 use bitcoin::bip32::{Fingerprint, IntoDerivationPath, KeySource, Xpriv, Xpub};
 use bitcoin::blockdata::opcodes::OP_0;
-use bitcoin::blockdata::script;
+use bitcoin::blockdata::{script, transaction};
 use bitcoin::consensus::encode::{deserialize, serialize_hex};
 use bitcoin::hex::FromHex;
 use bitcoin::psbt::{Psbt, PsbtSighashType};
@@ -163,7 +163,7 @@ struct TvInput {
     }
 
     Transaction {
-        version: 2,
+        version: transaction::Version::TWO,
         lock_time: absolute::LockTime::ZERO,
         input: vec![
             TxIn {
diff --git a/bitcoin/tests/serde.rs b/bitcoin/tests/serde.rs
index 4f224c67..c20011da 100644
--- a/bitcoin/tests/serde.rs
+++ b/bitcoin/tests/serde.rs
@@ -38,8 +38,8 @@
 use bitcoin::sighash::{EcdsaSighashType, TapSighashType};
 use bitcoin::taproot::{self, ControlBlock, LeafVersion, TapTree, TaprootBuilder};
 use bitcoin::{
-    ecdsa, Address, Amount, Block, Network, OutPoint, PrivateKey, PublicKey, ScriptBuf, Sequence,
-    Target, Transaction, TxIn, TxOut, Txid, Work,
+    ecdsa, transaction, Address, Amount, Block, Network, OutPoint, PrivateKey, PublicKey,
+    ScriptBuf, Sequence, Target, Transaction, TxIn, TxOut, Txid, Work,
 };
 
 /// Implicitly does regression test for `BlockHeader` also.
@@ -221,7 +221,7 @@ fn serde_regression_public_key() {
 #[test]
 fn serde_regression_psbt() {
     let tx = Transaction {
-        version: 1,
+        version: transaction::Version::ONE,
         lock_time: absolute::LockTime::ZERO,
         input: vec![TxIn {
             previous_output: OutPoint {