Make transaction::Version field private

jrakibi · jrakibi · commit cb270eae8e22 · 2025-02-24T06:54:21.000+07:00
This commit addresses rust-bitcoin#4041 by making the transaction::Version field private Changes: - Make the `Version` field private with `pub(crate)` - Rename `non_standard` to `maybe_non_standard` for clarity since it accepts both standard and non-standard versions - Add `#[inline]` attributes to small, frequently used methods: - `as_u32` - `maybe_non_standard` Users now must use either: - Constants (`Version::ONE/TWO/THREE`) for standard versions - `maybe_non_standard` method for any version (standard or non-standard)
diff --git a/bitcoin/src/blockdata/transaction.rs b/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]
diff --git a/primitives/src/transaction.rs b/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 {

Original file line number	Diff line number	Diff line change
`@@ -627,26 +627,15 @@ impl std::error::Error for IndexOutOfBoundsError {`
`627`	`627`	`fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }`
`628`	`628`	`}`
`629`	`629`
`630`		`-crate::internal_macros::define_extension_trait! {`
`631`		- /// Extension functionality for the [`Version`] type.
`632`		`- pub trait VersionExt impl for Version {`
`633`		`- /// Constructs a new non-standard transaction version.`
`634`		`- fn non_standard(version: u32) -> Version { Self(version) }`
`635`		`-`
`636`		`- /// Returns true if this transaction version number is considered standard.`
`637`		`- fn is_standard(&self) -> bool { self == Version::ONE \|\| self == Version::TWO \|\| *self == Version::THREE }`
`638`		`- }`
`639`		`-}`
`640`		`-`
`641`	`630`	`impl Encodable for Version {`
`642`	`631`	`fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {`
`643`		`- self.0.consensus_encode(w)`
	`632`	`+ self.to_u32().consensus_encode(w)`
`644`	`633`	`}`
`645`	`634`	`}`
`646`	`635`
`647`	`636`	`impl Decodable for Version {`
`648`	`637`	`fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {`
`649`		`- Decodable::consensus_decode(r).map(Version)`
	`638`	`+ Decodable::consensus_decode(r).map(Version::maybe_non_standard)`
`650`	`639`	`}`
`651`	`640`	`}`
`652`	`641`
`@@ -1373,7 +1362,7 @@ mod tests {`
`1373`	`1362`	`let tx: Result<Transaction, _> = deserialize(&tx_bytes);`
`1374`	`1363`	`assert!(tx.is_ok());`
`1375`	`1364`	`let realtx = tx.unwrap();`
`1376`		`- assert_eq!(realtx.version, Version::non_standard(u32::MAX));`
	`1365`	`+ assert_eq!(realtx.version, Version::maybe_non_standard(u32::MAX));`
`1377`	`1366`	`}`
`1378`	`1367`
`1379`	`1368`	`#[test]`