Return error from wpubkey_hash

Calling `wpubkey_hash` on a key that is uncompressed is flat out an
error, really it is a programmer error at build time because a segwit
key should never be compressed, however, for historical reasons we do
not enforce this in the type system. As a step towards clarity make it
an error to call `wpubkey_hash` on a an uncompressed pubkey. This adds
documentation and potentially might assist debugging for newer devs.

bitcoin/src/address/error.rs

@@ -6,6 +6,7 @@ use internals::write_err;
 
 use crate::address::{Address, NetworkUnchecked};
 use crate::blockdata::script::{witness_program, witness_version};
+use crate::crypto::key;
 use crate::prelude::String;
 use crate::{base58, Network};
 
@@ -18,7 +19,7 @@ pub enum Error {
     /// A witness program error.
     WitnessProgram(witness_program::Error),
     /// An uncompressed pubkey was used where it is not allowed.
-    UncompressedPubkey,
+    UncompressedPubkey(key::UncompressedPubkeyError),
     /// Address size more than 520 bytes is not allowed.
     ExcessiveScriptSize,
     /// Script is not a p2pkh, p2sh or witness program.
@@ -41,8 +42,7 @@ impl fmt::Display for Error {
         match *self {
             WitnessVersion(ref e) => write_err!(f, "witness version construction error"; e),
             WitnessProgram(ref e) => write_err!(f, "witness program error"; e),
-            UncompressedPubkey =>
+            UncompressedPubkey(ref e) => write_err!(f, "uncompressed pubkey"; e),
-                write!(f, "an uncompressed pubkey was used where it is not allowed"),
             ExcessiveScriptSize => write!(f, "script size exceed 520 bytes"),
             UnrecognizedScript => write!(f, "script is not a p2pkh, p2sh or witness program"),
             NetworkValidation { required, found, ref address } => {
@@ -66,14 +66,16 @@ impl std::error::Error for Error {
         match self {
             WitnessVersion(e) => Some(e),
             WitnessProgram(e) => Some(e),
-            UncompressedPubkey
+            UncompressedPubkey(e) => Some(e),
-            | ExcessiveScriptSize
+            ExcessiveScriptSize | UnrecognizedScript | NetworkValidation { .. } => None,
-            | UnrecognizedScript
-            | NetworkValidation { .. } => None,
         }
     }
 }
 
+impl From<key::UncompressedPubkeyError> for Error {
+    fn from(e: key::UncompressedPubkeyError) -> Self { Self::UncompressedPubkey(e) }
+}
+
 impl From<witness_version::TryFromError> for Error {
     fn from(e: witness_version::TryFromError) -> Error { Error::WitnessVersion(e) }
 }

bitcoin/src/address/mod.rs

@@ -424,10 +424,7 @@ impl Address {
     /// # Errors
     /// Will only return an error if an uncompressed public key is provided.
     pub fn p2wpkh(pk: &PublicKey, network: Network) -> Result<Address, Error> {
-        let prog = WitnessProgram::new(
+        let prog = WitnessProgram::new(WitnessVersion::V0, pk.wpubkey_hash()?)?;
-            WitnessVersion::V0,
-            pk.wpubkey_hash().ok_or(Error::UncompressedPubkey)?,
-        )?;
         let payload = Payload::WitnessProgram(prog);
         Ok(Address(AddressInner { network, payload }, PhantomData))
     }
@@ -439,9 +436,7 @@ impl Address {
     /// # Errors
     /// Will only return an Error if an uncompressed public key is provided.
     pub fn p2shwpkh(pk: &PublicKey, network: Network) -> Result<Address, Error> {
-        let builder = script::Builder::new()
+        let builder = script::Builder::new().push_int(0).push_slice(pk.wpubkey_hash()?);
-            .push_int(0)
-            .push_slice(pk.wpubkey_hash().ok_or(Error::UncompressedPubkey)?);
 
         let payload = Payload::ScriptHash(builder.into_script().script_hash());
         Ok(Address(AddressInner { network, payload }, PhantomData))
@@ -802,7 +797,7 @@ mod tests {
     use secp256k1::XOnlyPublicKey;
 
     use super::*;
-    use crate::crypto::key::PublicKey;
+    use crate::crypto::key::{PublicKey, UncompressedPubkeyError};
     use crate::network::Network::{Bitcoin, Testnet};
 
     fn roundtrips(addr: &Address) {
@@ -903,7 +898,7 @@ mod tests {
 
         // Test uncompressed pubkey
         key.compressed = false;
-        assert_eq!(Address::p2wpkh(&key, Bitcoin), Err(Error::UncompressedPubkey));
+        assert_eq!(Address::p2wpkh(&key, Bitcoin).unwrap_err(), UncompressedPubkeyError.into());
     }
 
     #[test]
@@ -932,7 +927,7 @@ mod tests {
 
         // Test uncompressed pubkey
         key.compressed = false;
-        assert_eq!(Address::p2wpkh(&key, Bitcoin), Err(Error::UncompressedPubkey));
+        assert_eq!(Address::p2wpkh(&key, Bitcoin).unwrap_err(), UncompressedPubkeyError.into());
     }
 
     #[test]

bitcoin/src/crypto/key.rs

@@ -58,15 +58,13 @@ impl PublicKey {
     pub fn pubkey_hash(&self) -> PubkeyHash { self.with_serialized(PubkeyHash::hash) }
 
     /// Returns bitcoin 160-bit hash of the public key for witness program
-    pub fn wpubkey_hash(&self) -> Option<WPubkeyHash> {
+    pub fn wpubkey_hash(&self) -> Result<WPubkeyHash, UncompressedPubkeyError> {
         if self.compressed {
-            Some(WPubkeyHash::from_byte_array(
+            Ok(WPubkeyHash::from_byte_array(
                 hash160::Hash::hash(&self.inner.serialize()).to_byte_array(),
             ))
         } else {
-            // We can't create witness pubkey hashes for an uncompressed
+            Err(UncompressedPubkeyError)
-            // public keys
-            None
         }
     }
 
@@ -746,6 +744,22 @@ impl From<hex::HexToArrayError> for Error {
     fn from(e: hex::HexToArrayError) -> Self { Error::Hex(e) }
 }
 
+/// Segwit public keys must always be compressed.
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub struct UncompressedPubkeyError;
+
+impl fmt::Display for UncompressedPubkeyError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.write_str("segwit public keys must always be compressed")
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for UncompressedPubkeyError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
+}
+
 #[cfg(test)]
 mod tests {
     use std::str::FromStr;
@@ -826,7 +840,7 @@ mod tests {
             pk.wpubkey_hash().unwrap().to_string(),
             "9511aa27ef39bbfa4e4f3dd15f4d66ea57f475b4"
         );
-        assert_eq!(upk.wpubkey_hash(), None);
+        assert!(upk.wpubkey_hash().is_err());
     }
 
     #[cfg(feature = "serde")]