Merge rust-bitcoin/rust-bitcoin#798: Audit conversion methods

5fbb211 Use fn name to_ instead of as_ (Tobin Harding)
8ffa323 Use fn name to_ instead of into_ (Tobin Harding)
6874ce9 Remove as_inner (Tobin C. Harding)

Pull request description:

  Rust has naming conventions surrounding conversion functions

  We have a handful of methods that are not following convention. This PR is done as three patches, separated by incorrect function name (`into_` or `as_`) and by whether or not the original method needs deprecating. Can be squashed if folks prefer.

  From the docs: https://rust-lang.github.io/api-guidelines/naming.html

  <h2><a class="header" href="https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv" id="ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv">Ad-hoc conversions follow <code>as_</code>, <code>to_</code>, <code>into_</code> conventions (C-CONV)</a></h2>
  <p>Conversions should be provided as methods, with names prefixed as follows:</p>

  Prefix | Cost | Ownership
  -- | -- | --
  as_ | Free | borrowed -> borrowed
  to_ | Expensive | borrowed -> borrowed
  | | | borrowed -> owned (non-Copy types)
  | | | owned -> owned (Copy types)
  into_ | Variable | owned -> owned (non-Copy types)

  EDIT: I did actually audit all uses of `to_` when I first did this, I did this by grepping for `fn to_` and checking the output against the table.

ACKs for top commit:
  apoelstra:
    ACK 5fbb211
  Kixunil:
    ACK 5fbb211

Tree-SHA512: f750b2d1a10bc1d4bb030d8528a582701cc3d615aa8a8ab391324dae639544bb3629a19b372784e1e274a8ddcc613c621c7aae21a3ea54fde356a6aa5e611ac0

Author: apoelstra

src/blockdata/opcodes.rs

@@ -736,7 +736,14 @@ impl All {
 
     /// Encode as a byte
     #[inline]
+    #[deprecated(since = "0.29.0", note = "use to_u8 instead")]
     pub fn into_u8(self) -> u8 {
+        self.to_u8()
+    }
+
+    /// Encodes [`All`] as a byte.
+    #[inline]
+    pub fn to_u8(self) -> u8 {
         self.code
     }
 }
@@ -859,9 +866,17 @@ ordinary_opcode! {
 impl Ordinary {
     /// Encode as a byte
     #[inline]
+    #[deprecated(since = "0.29.0", note = "use to_u8 instead")]
     pub fn into_u8(self) -> u8 {
+        self.to_u8()
+    }
+
+    /// Encodes [`All`] as a byte.
+    #[inline]
+    pub fn to_u8(self) -> u8 {
         self as u8
     }
+
 }
 
 #[cfg(test)]
@@ -872,7 +887,7 @@ mod tests {
 
     macro_rules! roundtrip {
         ($unique:expr, $op:ident) => {
-            assert_eq!(all::$op, All::from(all::$op.into_u8()));
+            assert_eq!(all::$op, All::from(all::$op.to_u8()));
 
             let s1 = format!("{}", all::$op);
             let s2 = format!("{:?}", all::$op);

src/blockdata/script.rs

@@ -485,33 +485,33 @@ impl Script {
     #[inline]
     pub fn is_p2sh(&self) -> bool {
         self.0.len() == 23
-            && self.0[0] == opcodes::all::OP_HASH160.into_u8()
-            && self.0[1] == opcodes::all::OP_PUSHBYTES_20.into_u8()
-            && self.0[22] == opcodes::all::OP_EQUAL.into_u8()
+            && self.0[0] == opcodes::all::OP_HASH160.to_u8()
+            && self.0[1] == opcodes::all::OP_PUSHBYTES_20.to_u8()
+            && self.0[22] == opcodes::all::OP_EQUAL.to_u8()
     }
 
     /// Checks whether a script pubkey is a P2PKH output.
     #[inline]
     pub fn is_p2pkh(&self) -> bool {
         self.0.len() == 25
-            && self.0[0] == opcodes::all::OP_DUP.into_u8()
-            && self.0[1] == opcodes::all::OP_HASH160.into_u8()
-            && self.0[2] == opcodes::all::OP_PUSHBYTES_20.into_u8()
-            && self.0[23] == opcodes::all::OP_EQUALVERIFY.into_u8()
-            && self.0[24] == opcodes::all::OP_CHECKSIG.into_u8()
+            && self.0[0] == opcodes::all::OP_DUP.to_u8()
+            && self.0[1] == opcodes::all::OP_HASH160.to_u8()
+            && self.0[2] == opcodes::all::OP_PUSHBYTES_20.to_u8()
+            && self.0[23] == opcodes::all::OP_EQUALVERIFY.to_u8()
+            && self.0[24] == opcodes::all::OP_CHECKSIG.to_u8()
     }
 
     /// Checks whether a script pubkey is a P2PK output.
     #[inline]
     pub fn is_p2pk(&self) -> bool {
         match self.len() {
             67 => {
-                self.0[0] == opcodes::all::OP_PUSHBYTES_65.into_u8()
-                    && self.0[66] == opcodes::all::OP_CHECKSIG.into_u8()
+                self.0[0] == opcodes::all::OP_PUSHBYTES_65.to_u8()
+                    && self.0[66] == opcodes::all::OP_CHECKSIG.to_u8()
             }
             35 => {
-                self.0[0] == opcodes::all::OP_PUSHBYTES_33.into_u8()
-                    && self.0[34] == opcodes::all::OP_CHECKSIG.into_u8()
+                self.0[0] == opcodes::all::OP_PUSHBYTES_33.to_u8()
+                    && self.0[34] == opcodes::all::OP_CHECKSIG.to_u8()
             }
             _ => false
         }
@@ -531,8 +531,8 @@ impl Script {
         let ver_opcode = opcodes::All::from(self.0[0]); // Version 0 or PUSHNUM_1-PUSHNUM_16
         let push_opbyte = self.0[1]; // Second byte push opcode 2-40 bytes
         WitnessVersion::from_opcode(ver_opcode).is_ok()
-            && push_opbyte >= opcodes::all::OP_PUSHBYTES_2.into_u8()
-            && push_opbyte <= opcodes::all::OP_PUSHBYTES_40.into_u8()
+            && push_opbyte >= opcodes::all::OP_PUSHBYTES_2.to_u8()
+            && push_opbyte <= opcodes::all::OP_PUSHBYTES_40.to_u8()
             // Check that the rest of the script has the correct size
             && script_len - 2 == push_opbyte as usize
     }
@@ -542,29 +542,29 @@ impl Script {
     pub fn is_v0_p2wsh(&self) -> bool {
         self.0.len() == 34
             && self.witness_version() == Some(WitnessVersion::V0)
-            && self.0[1] == opcodes::all::OP_PUSHBYTES_32.into_u8()
+            && self.0[1] == opcodes::all::OP_PUSHBYTES_32.to_u8()
     }
 
     /// Checks whether a script pubkey is a P2WPKH output.
     #[inline]
     pub fn is_v0_p2wpkh(&self) -> bool {
         self.0.len() == 22
             && self.witness_version() == Some(WitnessVersion::V0)
-            && self.0[1] == opcodes::all::OP_PUSHBYTES_20.into_u8()
+            && self.0[1] == opcodes::all::OP_PUSHBYTES_20.to_u8()
     }
 
     /// Checks whether a script pubkey is a P2TR output.
     #[inline]
     pub fn is_v1_p2tr(&self) -> bool {
         self.0.len() == 34
             && self.witness_version() == Some(WitnessVersion::V1)
-            && self.0[1] == opcodes::all::OP_PUSHBYTES_32.into_u8()
+            && self.0[1] == opcodes::all::OP_PUSHBYTES_32.to_u8()
     }
 
     /// Check if this is an OP_RETURN output.
     pub fn is_op_return (&self) -> bool {
         match self.0.first() {
-            Some(b) => *b == opcodes::all::OP_RETURN.into_u8(),
+            Some(b) => *b == opcodes::all::OP_RETURN.to_u8(),
             None => false
         }
     }
@@ -645,7 +645,7 @@ impl Script {
     #[cfg(feature="bitcoinconsensus")]
     #[cfg_attr(docsrs, doc(cfg(feature = "bitcoinconsensus")))]
     pub fn verify_with_flags<F: Into<u32>>(&self, index: usize, amount: crate::Amount, spending: &[u8], flags: F) -> Result<(), Error> {
-        Ok(bitcoinconsensus::verify_with_flags (&self.0[..], amount.as_sat(), spending, index, flags.into())?)
+        Ok(bitcoinconsensus::verify_with_flags (&self.0[..], amount.to_sat(), spending, index, flags.into())?)
     }
 
     /// Writes the assembly decoding of the script bytes to the formatter.
@@ -879,7 +879,7 @@ impl Builder {
         // We can special-case -1, 1-16
         if data == -1 || (1..=16).contains(&data) {
             let opcode = opcodes::All::from(
-                (data - 1 + opcodes::OP_TRUE.into_u8() as i64) as u8
+                (data - 1 + opcodes::OP_TRUE.to_u8() as i64) as u8
             );
             self.push_opcode(opcode)
         }
@@ -903,16 +903,16 @@ impl Builder {
         match data.len() as u64 {
             n if n < opcodes::Ordinary::OP_PUSHDATA1 as u64 => { self.0.push(n as u8); },
             n if n < 0x100 => {
-                self.0.push(opcodes::Ordinary::OP_PUSHDATA1.into_u8());
+                self.0.push(opcodes::Ordinary::OP_PUSHDATA1.to_u8());
                 self.0.push(n as u8);
             },
             n if n < 0x10000 => {
-                self.0.push(opcodes::Ordinary::OP_PUSHDATA2.into_u8());
+                self.0.push(opcodes::Ordinary::OP_PUSHDATA2.to_u8());
                 self.0.push((n % 0x100) as u8);
                 self.0.push((n / 0x100) as u8);
             },
             n if n < 0x100000000 => {
-                self.0.push(opcodes::Ordinary::OP_PUSHDATA4.into_u8());
+                self.0.push(opcodes::Ordinary::OP_PUSHDATA4.to_u8());
                 self.0.push((n % 0x100) as u8);
                 self.0.push(((n / 0x100) % 0x100) as u8);
                 self.0.push(((n / 0x10000) % 0x100) as u8);
@@ -942,7 +942,7 @@ impl Builder {
 
     /// Adds a single opcode to the script.
     pub fn push_opcode(mut self, data: opcodes::All) -> Builder {
-        self.0.push(data.into_u8());
+        self.0.push(data.to_u8());
         self.1 = Some(data);
         self
     }

src/network/address.rs

@@ -267,7 +267,7 @@ impl Encodable for AddrV2Message {
     fn consensus_encode<W: io::Write>(&self, mut e: W) -> Result<usize, io::Error> {
         let mut len = 0;
         len += self.time.consensus_encode(&mut e)?;
-        len += VarInt(self.services.as_u64()).consensus_encode(&mut e)?;
+        len += VarInt(self.services.to_u64()).consensus_encode(&mut e)?;
         len += self.addr.consensus_encode(&mut e)?;
 
         // consensus_encode always encodes in LE, and we want to encode in BE.

src/network/constants.rs

@@ -178,7 +178,13 @@ impl ServiceFlags {
     }
 
     /// Get the integer representation of this [ServiceFlags].
+    #[deprecated(since = "0.29.0", note = "use to_u64 instead")]
     pub fn as_u64(self) -> u64 {
+        self.to_u64()
+    }
+
+    /// Gets the integer representation of this [`ServiceFlags`].
+    pub fn to_u64(self) -> u64 {
         self.0
     }
 }

src/util/address.rs

@@ -296,10 +296,10 @@ impl WitnessVersion {
     /// If the opcode does not correspond to any witness version, errors with
     /// [`Error::MalformedWitnessVersion`].
     pub fn from_opcode(opcode: opcodes::All) -> Result<Self, Error> {
-        match opcode.into_u8() {
+        match opcode.to_u8() {
             0 => Ok(WitnessVersion::V0),
-            version if version >= opcodes::all::OP_PUSHNUM_1.into_u8() && version <= opcodes::all::OP_PUSHNUM_16.into_u8() =>
-                WitnessVersion::from_num(version - opcodes::all::OP_PUSHNUM_1.into_u8() + 1),
+            version if version >= opcodes::all::OP_PUSHNUM_1.to_u8() && version <= opcodes::all::OP_PUSHNUM_16.to_u8() =>
+                WitnessVersion::from_num(version - opcodes::all::OP_PUSHNUM_1.to_u8() + 1),
             _ => Err(Error::MalformedWitnessVersion)
         }
     }
@@ -326,7 +326,17 @@ impl WitnessVersion {
     /// NB: this is not the same as an integer representation of the opcode signifying witness
     /// version in bitcoin script. Thus, there is no function to directly convert witness version
     /// into a byte since the conversion requires context (bitcoin script or just a version number).
+    #[deprecated(since = "0.29.0", note = "use to_num instead")]
     pub fn into_num(self) -> u8 {
+        self.to_num()
+    }
+
+    /// Returns integer version number representation for a given [`WitnessVersion`] value.
+    ///
+    /// NB: this is not the same as an integer representation of the opcode signifying witness
+    /// version in bitcoin script. Thus, there is no function to directly convert witness version
+    /// into a byte since the conversion requires context (bitcoin script or just a version number).
+    pub fn to_num(self) -> u8 {
         self as u8
     }
 
@@ -342,7 +352,7 @@ impl WitnessVersion {
 impl From<WitnessVersion> for ::bech32::u5 {
     /// Converts [`WitnessVersion`] instance into corresponding Bech32(m) u5-value ([`bech32::u5`]).
     fn from(version: WitnessVersion) -> Self {
-        ::bech32::u5::try_from_u8(version.into_num()).expect("WitnessVersion must be 0..=16")
+        ::bech32::u5::try_from_u8(version.to_num()).expect("WitnessVersion must be 0..=16")
     }
 }
 
@@ -351,7 +361,7 @@ impl From<WitnessVersion> for opcodes::All {
     fn from(version: WitnessVersion) -> opcodes::All {
         match version {
             WitnessVersion::V0 => opcodes::all::OP_PUSHBYTES_0,
-            no => opcodes::All::from(opcodes::all::OP_PUSHNUM_1.into_u8() + no.into_num() - 1)
+            no => opcodes::All::from(opcodes::all::OP_PUSHNUM_1.to_u8() + no.to_num() - 1)
         }
     }
 }
@@ -473,7 +483,7 @@ impl Payload {
     pub fn p2tr_tweaked(output_key: TweakedPublicKey) -> Payload {
         Payload::WitnessProgram {
             version: WitnessVersion::V1,
-            program: output_key.as_inner().serialize().to_vec(),
+            program: output_key.to_inner().serialize().to_vec(),
         }
     }