Merge rust-bitcoin/rust-bitcoin#1161: Fix public API clippy warnings

5cef2f1a8f Remove stutter for error variants (Tobin C. Harding)
08c4a2204a Allow wrong_self_convention for non-Copy type (Tobin C. Harding)
0786d92a5c Take self by value (Tobin C. Harding)

Pull request description:

  [Turns out](https://github.com/rust-lang/rust-clippy/issues/9248) by default clippy does not lint certain parts of the public API.

  Specifically, by setting `avoid-breaking-exported-api = false` we can get clippy to lint the public API for various things including
  `wrong_self_convention`.

  This means the saga of to/into/as continues ... we have used the wrong from for many of our `to_*` methods, they should consme `self`. Patch 1 fixes them. Patch 2 adds an `allow`. Patch 3 fixes a few error enum variant identifiers (removes `Error` suffix).

ACKs for top commit:
  Kixunil:
    ACK 5cef2f1a8f
  sanket1729:
    code review ACK 5cef2f1a8f
  apoelstra:
    ACK 5cef2f1a8f

Tree-SHA512: f1cdb94764a132a7c58f3e97a208f3f8d618e4f2620753d0394590b19d3dab0df7669e187f6e0a3d0aa358a7d8aee5495f78008f244b6eedb33715ff1df151b1

src/blockdata/locktime.rs

@@ -81,7 +81,7 @@ impl PackedLockTime {
 
     /// Returns the inner `u32`.
     #[inline]
-    pub fn to_u32(&self) -> u32 {
+    pub fn to_u32(self) -> u32 {
         self.0
     }
 }
@@ -338,8 +338,8 @@ impl LockTime {
     /// // let is_satisfied = n.partial_cmp(&lock_time).expect("invalid comparison").is_le();
     /// ```
     #[inline]
-    pub fn to_consensus_u32(&self) -> u32 {
+    pub fn to_consensus_u32(self) -> u32 {
-        match *self {
+        match self {
             LockTime::Blocks(ref h) => h.to_consensus_u32(),
             LockTime::Seconds(ref t) => t.to_consensus_u32(),
         }
@@ -446,7 +446,7 @@ impl Height {
     /// assert!(lock_time.is_block_height());
     /// assert_eq!(lock_time.to_consensus_u32(), n_lock_time);
     #[inline]
-    pub fn to_consensus_u32(&self) -> u32 {
+    pub fn to_consensus_u32(self) -> u32 {
         self.0
     }
 }
@@ -529,7 +529,7 @@ impl Time {
     /// assert_eq!(lock_time.to_consensus_u32(), n_lock_time);
     /// ```
     #[inline]
-    pub fn to_consensus_u32(&self) -> u32 {
+    pub fn to_consensus_u32(self) -> u32 {
         self.0
     }
 }

src/blockdata/script.rs

@@ -157,7 +157,7 @@ pub enum Error {
     /// Can not find the spent output
     UnknownSpentOutput(OutPoint),
     /// Can not serialize the spending transaction
-    SerializationError
+    Serialization
 }
 
 impl fmt::Display for Error {
@@ -169,7 +169,7 @@ impl fmt::Display for Error {
             #[cfg(feature = "bitcoinconsensus")]
             Error::BitcoinConsensus(ref _n) => "bitcoinconsensus verification failed",
             Error::UnknownSpentOutput(ref _point) => "unknown spent output Transaction::verify()",
-            Error::SerializationError => "can not serialize the spending transaction in Transaction::verify()",
+            Error::Serialization => "can not serialize the spending transaction in Transaction::verify()",
         };
         f.write_str(str)
     }
@@ -186,7 +186,7 @@ impl std::error::Error for Error {
             | EarlyEndOfScript
             | NumericOverflow
             | UnknownSpentOutput(_)
-            | SerializationError => None,
+            | Serialization => None,
             #[cfg(feature = "bitcoinconsensus")]
             BitcoinConsensus(_) => None,
         }

src/blockdata/transaction.rs

@@ -379,7 +379,7 @@ impl Sequence {
 
     /// Returns the inner 32bit integer value of Sequence.
     #[inline]
-    pub fn to_consensus_u32(&self) -> u32 {
+    pub fn to_consensus_u32(self) -> u32 {
         self.0
     }
 }
@@ -503,6 +503,7 @@ impl<E> EncodeSigningDataResult<E> {
     ///     // use a hash value of "1", instead of computing the actual hash due to SIGHASH_SINGLE bug
     /// }
     /// ```
+    #[allow(clippy::wrong_self_convention)] // E is not Copy so we consume self.
     pub fn is_sighash_single_bug(self) -> Result<bool, E> {
         match self {
             EncodeSigningDataResult::SighashSingleBug => Ok(true),

src/internal_macros.rs

@@ -78,7 +78,7 @@ macro_rules! impl_array_newtype {
 
             /// Returns the underlying bytes.
             #[inline]
-            pub fn to_bytes(&self) -> [$ty; $len] { self.0.clone() }
+            pub fn to_bytes(self) -> [$ty; $len] { self.0.clone() }
 
             /// Returns the underlying bytes.
             #[inline]

src/util/bip32.rs

@@ -545,7 +545,7 @@ impl ExtendedPrivKey {
     }
 
     /// Constructs ECDSA compressed private key matching internal secret key representation.
-    pub fn to_priv(&self) -> PrivateKey {
+    pub fn to_priv(self) -> PrivateKey {
         PrivateKey {
             compressed: true,
             network: self.network,
@@ -555,7 +555,7 @@ impl ExtendedPrivKey {
 
     /// Constructs BIP340 keypair for Schnorr signatures and Taproot use matching the internal
     /// secret key representation.
-    pub fn to_keypair<C: secp256k1::Signing>(&self, secp: &Secp256k1<C>) -> KeyPair {
+    pub fn to_keypair<C: secp256k1::Signing>(self, secp: &Secp256k1<C>) -> KeyPair {
         KeyPair::from_seckey_slice(secp, &self.private_key[..]).expect("BIP32 internal private key representation is broken")
     }
 
@@ -677,7 +677,7 @@ impl ExtendedPubKey {
     }
 
     /// Constructs ECDSA compressed public key matching internal public key representation.
-    pub fn to_pub(&self) -> PublicKey {
+    pub fn to_pub(self) -> PublicKey {
         PublicKey {
             compressed: true,
             inner: self.public_key
@@ -686,7 +686,7 @@ impl ExtendedPubKey {
 
     /// Constructs BIP340 x-only public key for BIP-340 signatures and Taproot use matching
     /// the internal public key representation.
-    pub fn to_x_only_pub(&self) -> XOnlyPublicKey {
+    pub fn to_x_only_pub(self) -> XOnlyPublicKey {
         XOnlyPublicKey::from(self.public_key)
     }
 

src/util/ecdsa.rs

@@ -46,7 +46,7 @@ impl EcdsaSig {
     }
 
     /// Serializes an ECDSA signature (inner secp256k1 signature in DER format).
-    pub fn to_vec(&self) -> Vec<u8> {
+    pub fn to_vec(self) -> Vec<u8> {
         // TODO: add support to serialize to a writer to SerializedSig
         self.sig.serialize_der()
             .iter().copied()

src/util/key.rs

@@ -167,7 +167,7 @@ impl PublicKey {
     }
 
     /// Serialize the public key to bytes
-    pub fn to_bytes(&self) -> Vec<u8> {
+    pub fn to_bytes(self) -> Vec<u8> {
         let mut buf = Vec::new();
         self.write_into(&mut buf).expect("vecs don't error");
         buf
@@ -219,11 +219,11 @@ impl PublicKey {
     ///     pk("04c4b0bbb339aa236bff38dbe6a451e111972a7909a126bc424013cba2ec33bc38e98ac269ffe028345c31ac8d0a365f29c8f7e7cfccac72f84e1acd02bc554f35"),
     /// ];
     ///
-    /// unsorted.sort_unstable_by_key(PublicKey::to_sort_key);
+    /// unsorted.sort_unstable_by_key(|k| PublicKey::to_sort_key(*k));
     ///
     /// assert_eq!(unsorted, sorted);
     /// ```
-    pub fn to_sort_key(&self) -> SortKey {
+    pub fn to_sort_key(self) -> SortKey {
         if self.compressed {
             let bytes = self.inner.serialize();
             let mut res = [0; 32];
@@ -337,7 +337,7 @@ impl PrivateKey {
     }
 
     /// Serialize the private key to bytes
-    pub fn to_bytes(&self) -> Vec<u8> {
+    pub fn to_bytes(self) -> Vec<u8> {
         self.inner[..].to_vec()
     }
 
@@ -364,7 +364,7 @@ impl PrivateKey {
     }
 
     /// Get WIF encoding of this private key.
-    pub fn to_wif(&self) -> String {
+    pub fn to_wif(self) -> String {
         let mut buf = String::new();
         buf.write_fmt(format_args!("{}", self)).unwrap();
         buf.shrink_to_fit();
@@ -833,7 +833,7 @@ mod tests {
             },
         ];
         for mut vector in vectors {
-            vector.input.sort_by_cached_key(PublicKey::to_sort_key);
+            vector.input.sort_by_cached_key(|k| PublicKey::to_sort_key(*k));
             assert_eq!(vector.input, vector.expect);
         }
     }

src/util/misc.rs

@@ -179,7 +179,7 @@ mod message_signing {
         /// Convert to base64 encoding.
         #[cfg(feature = "base64")]
         #[cfg_attr(docsrs, doc(cfg(feature = "base64")))]
-        pub fn to_base64(&self) -> String {
+        pub fn to_base64(self) -> String {
             base64::encode(&self.serialize()[..])
         }
     }

src/util/psbt/error.rs

@@ -58,7 +58,7 @@ pub enum Error {
     /// Unable to parse as a standard sighash type.
     NonStandardSighashType(u32),
     /// Parsing errors from bitcoin_hashes
-    HashParseError(hashes::Error),
+    HashParse(hashes::Error),
     /// The pre-image must hash to the correponding psbt hash
     InvalidPreimageHashPair {
         /// Hash-type
@@ -93,7 +93,7 @@ impl fmt::Display for Error {
             Error::NoMorePairs => f.write_str("no more key-value pairs for this psbt map"),
             Error::UnexpectedUnsignedTx { expected: ref e, actual: ref a } => write!(f, "different unsigned transaction: expected {}, actual {}", e.txid(), a.txid()),
             Error::NonStandardSighashType(ref sht) => write!(f, "non-standard sighash type: {}", sht),
-            Error::HashParseError(ref e) => write_err!(f, "hash parse error"; e),
+            Error::HashParse(ref e) => write_err!(f, "hash parse error"; e),
             Error::InvalidPreimageHashPair{ref preimage, ref hash, ref hash_type} => {
                 // directly using debug forms of psbthash enums
                 write!(f, "Preimage {:?} does not match {:?} hash {:?}", preimage, hash_type, hash )
@@ -111,7 +111,7 @@ impl std::error::Error for Error {
         use self::Error::*;
 
         match self {
-            HashParseError(e) => Some(e),
+            HashParse(e) => Some(e),
             | InvalidMagic
             | MissingUtxo
             | InvalidSeparator
@@ -135,7 +135,7 @@ impl std::error::Error for Error {
 #[doc(hidden)]
 impl From<hashes::Error> for Error {
     fn from(e: hashes::Error) -> Error {
-        Error::HashParseError(e)
+        Error::HashParse(e)
     }
 }
 

src/util/schnorr.rs

@@ -241,7 +241,7 @@ impl SchnorrSig {
     }
 
     /// Serialize SchnorrSig
-    pub fn to_vec(&self) -> Vec<u8> {
+    pub fn to_vec(self) -> Vec<u8> {
         // TODO: add support to serialize to a writer to SerializedSig
         let mut ser_sig = self.sig.as_ref().to_vec();
         if self.hash_ty == SchnorrSighashType::Default {

src/util/taproot.rs

@@ -87,7 +87,7 @@ impl TapTweakHash {
     }
 
     /// Converts a `TapTweakHash` into a `Scalar` ready for use with key tweaking API.
-    pub fn to_scalar(&self) -> Scalar {
+    pub fn to_scalar(self) -> Scalar {
         // This is statistically extremely unlikely to panic.
         Scalar::from_be_bytes(self.into_inner()).expect("hash value greater than curve order")
     }

src/util/uint.rs

@@ -106,7 +106,7 @@ macro_rules! construct_uint {
             }
 
             /// Convert a big integer into a byte array using big-endian encoding
-            pub fn to_be_bytes(&self) -> [u8; $n_words * 8] {
+            pub fn to_be_bytes(self) -> [u8; $n_words * 8] {
                 use super::endian::u64_to_array_be;
                 let mut res = [0; $n_words * 8];
                 for i in 0..$n_words {