Merge rust-bitcoin/rust-bitcoin#4099: Make transaction::Version field private

cb270eae8e Make transaction::Version field private (jrakibi)
6c69b66b0d Use Version constant (jrakibi)

Pull request description:

  This commit addresses #4041 by making the `transaction::Version` field private.

  This forces people to either use the associated constants (`Version::ONE/TWO/THREE`) or the `non_standard`/`from_consensus` methods for any other transaction version.

  This aligns with our approach to `block::Version`

ACKs for top commit:
  tcharding:
    ACK cb270eae8e
  Kixunil:
    ACK cb270eae8e

Tree-SHA512: dcf5b50dfeda04e56fec350acd735dcb7099989b552afce4261d559a8a846c0eb3705369ad635ef9bbbfb2373d203a2c3641178925de6685426aa91245db9a8c

bitcoin/src/blockdata/transaction.rs

@@ -627,26 +627,15 @@ impl std::error::Error for IndexOutOfBoundsError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
 }
 
-crate::internal_macros::define_extension_trait! {
-    /// Extension functionality for the [`Version`] type.
-    pub trait VersionExt impl for Version {
-        /// Constructs a new non-standard transaction version.
-        fn non_standard(version: u32) -> Version { Self(version) }
-
-        /// Returns true if this transaction version number is considered standard.
-        fn is_standard(&self) -> bool { *self == Version::ONE || *self == Version::TWO || *self == Version::THREE }
-    }
-}
-
 impl Encodable for Version {
     fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
-        self.0.consensus_encode(w)
+        self.to_u32().consensus_encode(w)
     }
 }
 
 impl Decodable for Version {
     fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
-        Decodable::consensus_decode(r).map(Version)
+        Decodable::consensus_decode(r).map(Version::maybe_non_standard)
     }
 }
 
@@ -1373,7 +1362,7 @@ mod tests {
         let tx: Result<Transaction, _> = deserialize(&tx_bytes);
         assert!(tx.is_ok());
         let realtx = tx.unwrap();
-        assert_eq!(realtx.version, Version::non_standard(u32::MAX));
+        assert_eq!(realtx.version, Version::maybe_non_standard(u32::MAX));
     }
 
     #[test]

bitcoin/tests/psbt-sign-taproot.rs

@@ -211,7 +211,7 @@ fn create_psbt_for_taproot_key_path_spend(
     let prev_tx_id = "06980ca116f74c7845a897461dd0e1d15b114130176de5004957da516b4dee3a";
 
     let transaction = Transaction {
-        version: Version(2),
+        version: Version::TWO,
         lock_time: absolute::LockTime::ZERO,
         input: vec![TxIn {
             previous_output: OutPoint { txid: prev_tx_id.parse().unwrap(), vout: 0 },
@@ -288,7 +288,7 @@ fn create_psbt_for_taproot_script_path_spend(
     }];
     let prev_tx_id = "9d7c6770fca57285babab60c51834cfcfd10ad302119cae842d7216b4ac9a376";
     let transaction = Transaction {
-        version: Version(2),
+        version: Version::TWO,
         lock_time: absolute::LockTime::ZERO,
         input: vec![TxIn {
             previous_output: OutPoint { txid: prev_tx_id.parse().unwrap(), vout: 0 },

primitives/src/transaction.rs

@@ -518,7 +518,7 @@ impl Wtxid {
 /// [BIP-431]: https://github.com/bitcoin/bips/blob/master/bip-0431.mediawiki
 #[derive(Copy, PartialEq, Eq, Clone, Debug, PartialOrd, Ord, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct Version(pub u32);
+pub struct Version(u32);
 
 impl Version {
     /// The original Bitcoin transaction version (pre-BIP-68).
@@ -529,12 +529,35 @@ impl Version {
 
     /// The third Bitcoin transaction version (post-BIP-431).
     pub const THREE: Self = Self(3);
+
+    /// Constructs a potentially non-standard transaction version.
+    ///
+    /// This can accept both standard and non-standard versions.
+    #[inline]
+    pub fn maybe_non_standard(version: u32) -> Version { Self(version) }
+
+    /// Returns the inner `u32` value of this `Version`.
+    #[inline]
+    pub const fn to_u32(self) -> u32 { self.0 }
+
+    /// Returns true if this transaction version number is considered standard.
+    ///
+    /// The behavior of this method matches whatever Bitcoin Core considers standard at the time
+    /// of the release and may change in future versions to accommodate new standard versions.
+    /// As of Bitcoin Core 28.0 ([release notes](https://bitcoincore.org/en/releases/28.0/)),
+    /// versions 1, 2, and 3 are considered standard.
+    #[inline]
+    pub fn is_standard(&self) -> bool { *self == Version::ONE || *self == Version::TWO || *self == Version::THREE }
 }
 
 impl fmt::Display for Version {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.0, f) }
 }
 
+impl From<Version> for u32 {
+    fn from(version: Version) -> Self { version.0 }
+}
+
 #[cfg(feature = "arbitrary")]
 #[cfg(feature = "alloc")]
 impl<'a> Arbitrary<'a> for Transaction {