Merge rust-bitcoin/rust-bitcoin#1591: Improve downflow of information in `psbt::Error`

68abfdb0b9 Better downflow of information in `psbt::Error` (Arturo Marquez)

Pull request description:

  See third point in https://github.com/rust-bitcoin/rust-bitcoin/issues/837

  Closes: https://github.com/rust-bitcoin/rust-bitcoin/issues/1589

ACKs for top commit:
  apoelstra:
    ACK 68abfdb0b9
  tcharding:
    ACK 68abfdb0b9

Tree-SHA512: 8accfd6a1ae9c413b48a5a5861ec036a166a5cd5a73ee242169ee9d34a91f85f4f152e2feba628e20b8b89d7177694c3d2af60551a37c2d9c9a4408036f55262

bitcoin/src/psbt/error.rs

@@ -72,14 +72,34 @@ pub enum Error {
     /// Conflicting data during combine procedure:
     /// global extended public key has inconsistent key sources
     CombineInconsistentKeySources(Box<ExtendedPubKey>),
-    /// Parsing error.
-    ParseFailed(&'static str),
     /// Serialization error in bitcoin consensus-encoded structures
     ConsensusEncoding,
     /// Negative fee
     NegativeFee,
     /// Integer overflow in fee calculation
     FeeOverflow,
+    /// Parsing error indicating invalid public keys
+    InvalidPublicKey(crate::crypto::key::Error),
+    /// Parsing error indicating invalid secp256k1 public keys
+    InvalidSecp256k1PublicKey(secp256k1::Error),
+    /// Parsing error indicating invalid xonly public keys
+    InvalidXOnlyPublicKey,
+    /// Parsing error indicating invalid ECDSA signatures
+    InvalidEcdsaSignature(crate::crypto::ecdsa::Error),
+    /// Parsing error indicating invalid Schnorr signatures
+    InvalidSchnorrSignature(crate::crypto::schnorr::Error),
+    /// Parsing error indicating invalid control block
+    InvalidControlBlock,
+    /// Parsing error indicating invalid leaf version
+    InvalidLeafVersion,
+    /// Parsing error indicating a Taproot error
+    Taproot(&'static str),
+    /// Error related to an xpub key
+    XPubKey(&'static str),
+    /// Error related to PSBT version
+    Version(&'static str),
+    /// PSBT data is not consumed entirely
+    PartialDataConsumption,
 }
 
 impl fmt::Display for Error {
@@ -106,11 +126,20 @@ impl fmt::Display for Error {
                 write!(f, "Preimage {:?} does not match {:?} hash {:?}", preimage, hash_type, hash )
             },
             Error::CombineInconsistentKeySources(ref s) => { write!(f, "combine conflict: {}", s) },
-            Error::ParseFailed(ref s) => write!(f, "parse failed: {}", s),
             Error::ConsensusEncoding => f.write_str("bitcoin consensus or BIP-174 encoding error"),
             Error::NegativeFee => f.write_str("PSBT has a negative fee which is not allowed"),
             Error::FeeOverflow => f.write_str("integer overflow in fee calculation"),
-
+            Error::InvalidPublicKey(ref e) => write_err!(f, "invalid public key"; e),
+            Error::InvalidSecp256k1PublicKey(ref e) => write_err!(f, "invalid secp256k1 public key"; e),
+            Error::InvalidXOnlyPublicKey => f.write_str("invalid xonly public key"),
+            Error::InvalidEcdsaSignature(ref e) => write_err!(f, "invalid ECDSA signature"; e),
+            Error::InvalidSchnorrSignature(ref e) => write_err!(f, "invalid Schnorr signature"; e),
+            Error::InvalidControlBlock => f.write_str("invalid control block"),
+            Error::InvalidLeafVersion => f.write_str("invalid leaf version"),
+            Error::Taproot(s) => write!(f, "taproot error -  {}", s),
+            Error::XPubKey(s) => write!(f, "xpub key error -  {}", s),
+            Error::Version(s) => write!(f, "version error {}", s),
+            Error::PartialDataConsumption => f.write_str("data not consumed entirely when explicitly deserializing"),
         }
     }
 }
@@ -138,10 +167,20 @@ impl std::error::Error for Error {
             | NonStandardSighashType(_)
             | InvalidPreimageHashPair{ .. }
             | CombineInconsistentKeySources(_)
-            | ParseFailed(_)
             | ConsensusEncoding
             | NegativeFee
-            | FeeOverflow => None,
+            | FeeOverflow
+            | InvalidPublicKey(_)
+            | InvalidSecp256k1PublicKey(_)
+            | InvalidXOnlyPublicKey
+            | InvalidEcdsaSignature(_)
+            | InvalidSchnorrSignature(_)
+            | InvalidControlBlock
+            | InvalidLeafVersion
+            | Taproot(_)
+            | XPubKey(_)
+            | Version(_)
+            | PartialDataConsumption=> None
         }
     }
 }

bitcoin/src/psbt/map/global.rs

@@ -119,7 +119,7 @@ impl PartiallySignedTransaction {
                                     });
 
                                     if decoder.position() != vlen as u64 {
-                                        return Err(Error::ParseFailed("data not consumed entirely when explicitly deserializing"))
+                                        return Err(Error::PartialDataConsumption)
                                     }
                                 } else {
                                     return Err(Error::DuplicateKey(pair.key))
@@ -131,18 +131,18 @@ impl PartiallySignedTransaction {
                         PSBT_GLOBAL_XPUB => {
                             if !pair.key.key.is_empty() {
                                 let xpub = ExtendedPubKey::decode(&pair.key.key)
-                                    .map_err(|_| Error::ParseFailed(
+                                    .map_err(|_| Error::XPubKey(
                                         "Can't deserialize ExtendedPublicKey from global XPUB key data"
                                     ))?;
 
                                 if pair.value.is_empty() || pair.value.len() % 4 != 0 {
-                                    return Err(Error::ParseFailed("Incorrect length of global xpub derivation data"))
+                                    return Err(Error::XPubKey("Incorrect length of global xpub derivation data"))
                                 }
 
                                 let child_count = pair.value.len() / 4 - 1;
                                 let mut decoder = Cursor::new(pair.value);
                                 let mut fingerprint = [0u8; 4];
-                                decoder.read_exact(&mut fingerprint[..]).map_err(|_| Error::ParseFailed("Can't read global xpub fingerprint"))?;
+                                decoder.read_exact(&mut fingerprint[..]).map_err(|_| Error::XPubKey("Can't read global xpub fingerprint"))?;
                                 let mut path = Vec::<ChildNumber>::with_capacity(child_count);
                                 while let Ok(index) = u32::consensus_decode(&mut decoder) {
                                     path.push(ChildNumber::from(index))
@@ -150,10 +150,10 @@ impl PartiallySignedTransaction {
                                 let derivation = DerivationPath::from(path);
                                 // Keys, according to BIP-174, must be unique
                                 if xpub_map.insert(xpub, (Fingerprint::from(fingerprint), derivation)).is_some() {
-                                    return Err(Error::ParseFailed("Repeated global xpub key"))
+                                    return Err(Error::XPubKey("Repeated global xpub key"))
                                 }
                             } else {
-                                return Err(Error::ParseFailed("Xpub global key must contain serialized Xpub data"))
+                                return Err(Error::XPubKey("Xpub global key must contain serialized Xpub data"))
                             }
                         }
                         PSBT_GLOBAL_VERSION => {
@@ -164,13 +164,13 @@ impl PartiallySignedTransaction {
                                     let vlen: usize = pair.value.len();
                                     let mut decoder = Cursor::new(pair.value);
                                     if vlen != 4 {
-                                        return Err(Error::ParseFailed("Wrong global version value length (must be 4 bytes)"))
+                                        return Err(Error::Version("invalid global version value length (must be 4 bytes)"))
                                     }
                                     version = Some(Decodable::consensus_decode(&mut decoder)?);
                                     // We only understand version 0 PSBTs. According to BIP-174 we
                                     // should throw an error if we see anything other than version 0.
                                     if version != Some(0) {
-                                        return Err(Error::ParseFailed("PSBT versions greater than 0 are not supported"))
+                                        return Err(Error::Version("PSBT versions greater than 0 are not supported"))
                                     }
                                 } else {
                                     return Err(Error::DuplicateKey(pair.key))

bitcoin/src/psbt/mod.rs

@@ -1384,28 +1384,37 @@ mod tests {
         #[test]
         fn invalid_vectors() {
             let err = hex_psbt!("70736274ff010071020000000127744ababf3027fe0d6cf23a96eee2efb188ef52301954585883e69b6624b2420000000000ffffffff02787c01000000000016001483a7e34bd99ff03a4962ef8a1a101bb295461ece606b042a010000001600147ac369df1b20e033d6116623957b0ac49f3c52e8000000000001012b00f2052a010000002251205a2c2cf5b52cf31f83ad2e8da63ff03183ecd8f609c7510ae8a48e03910a075701172102fe349064c98d6e2a853fa3c9b12bd8b304a19c195c60efa7ee2393046d3fa232000000").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid xonly public key");
+            assert_eq!(err.to_string(), "invalid xonly public key");
             let err = hex_psbt!("70736274ff010071020000000127744ababf3027fe0d6cf23a96eee2efb188ef52301954585883e69b6624b2420000000000ffffffff02787c01000000000016001483a7e34bd99ff03a4962ef8a1a101bb295461ece606b042a010000001600147ac369df1b20e033d6116623957b0ac49f3c52e8000000000001012b00f2052a010000002251205a2c2cf5b52cf31f83ad2e8da63ff03183ecd8f609c7510ae8a48e03910a0757011342173bb3d36c074afb716fec6307a069a2e450b995f3c82785945ab8df0e24260dcd703b0cbf34de399184a9481ac2b3586db6601f026a77f7e4938481bc34751701aa000000").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid Schnorr signature length");
+            #[cfg(feature = "std")]
+            assert_eq!(err.to_string(), "invalid Schnorr signature");
+            #[cfg(not(feature = "std"))]
+            assert_eq!(err.to_string(), "invalid Schnorr signature: Invalid Schnorr signature size: 66");
             let err = hex_psbt!("70736274ff010071020000000127744ababf3027fe0d6cf23a96eee2efb188ef52301954585883e69b6624b2420000000000ffffffff02787c01000000000016001483a7e34bd99ff03a4962ef8a1a101bb295461ece606b042a010000001600147ac369df1b20e033d6116623957b0ac49f3c52e8000000000001012b00f2052a010000002251205a2c2cf5b52cf31f83ad2e8da63ff03183ecd8f609c7510ae8a48e03910a0757221602fe349064c98d6e2a853fa3c9b12bd8b304a19c195c60efa7ee2393046d3fa2321900772b2da75600008001000080000000800100000000000000000000").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid xonly public key");
+            assert_eq!(err.to_string(), "invalid xonly public key");
             let err = hex_psbt!("70736274ff01007d020000000127744ababf3027fe0d6cf23a96eee2efb188ef52301954585883e69b6624b2420000000000ffffffff02887b0100000000001600142382871c7e8421a00093f754d91281e675874b9f606b042a010000002251205a2c2cf5b52cf31f83ad2e8da63ff03183ecd8f609c7510ae8a48e03910a0757000000000001012b00f2052a010000002251205a2c2cf5b52cf31f83ad2e8da63ff03183ecd8f609c7510ae8a48e03910a0757000001052102fe349064c98d6e2a853fa3c9b12bd8b304a19c195c60efa7ee2393046d3fa23200").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid xonly public key");
+            assert_eq!(err.to_string(), "invalid xonly public key");
             let err = hex_psbt!("70736274ff01007d020000000127744ababf3027fe0d6cf23a96eee2efb188ef52301954585883e69b6624b2420000000000ffffffff02887b0100000000001600142382871c7e8421a00093f754d91281e675874b9f606b042a010000002251205a2c2cf5b52cf31f83ad2e8da63ff03183ecd8f609c7510ae8a48e03910a0757000000000001012b00f2052a010000002251205a2c2cf5b52cf31f83ad2e8da63ff03183ecd8f609c7510ae8a48e03910a07570000220702fe349064c98d6e2a853fa3c9b12bd8b304a19c195c60efa7ee2393046d3fa2321900772b2da7560000800100008000000080010000000000000000").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid xonly public key");
+            assert_eq!(err.to_string(), "invalid xonly public key");
             let err = hex_psbt!("70736274ff01005e02000000019bd48765230bf9a72e662001f972556e54f0c6f97feb56bcb5600d817f6995260100000000ffffffff0148e6052a01000000225120030da4fce4f7db28c2cb2951631e003713856597fe963882cb500e68112cca63000000000001012b00f2052a01000000225120c2247efbfd92ac47f6f40b8d42d169175a19fa9fa10e4a25d7f35eb4dd85b6924214022cb13ac68248de806aa6a3659cf3c03eb6821d09c8114a4e868febde865bb6d2cd970e15f53fc0c82f950fd560ffa919b76172be017368a89913af074f400b094089756aa3739ccc689ec0fcf3a360be32cc0b59b16e93a1e8bb4605726b2ca7a3ff706c4176649632b2cc68e1f912b8a578e3719ce7710885c7a966f49bcd43cb0000").unwrap_err();
             #[cfg(feature = "std")]
             assert_eq!(err.to_string(), "hash parse error");
             #[cfg(not(feature = "std"))]
             assert_eq!(err.to_string(), "hash parse error: bad slice length 33 (expected 32)");
             let err = hex_psbt!("70736274ff01005e02000000019bd48765230bf9a72e662001f972556e54f0c6f97feb56bcb5600d817f6995260100000000ffffffff0148e6052a01000000225120030da4fce4f7db28c2cb2951631e003713856597fe963882cb500e68112cca63000000000001012b00f2052a01000000225120c2247efbfd92ac47f6f40b8d42d169175a19fa9fa10e4a25d7f35eb4dd85b69241142cb13ac68248de806aa6a3659cf3c03eb6821d09c8114a4e868febde865bb6d2cd970e15f53fc0c82f950fd560ffa919b76172be017368a89913af074f400b094289756aa3739ccc689ec0fcf3a360be32cc0b59b16e93a1e8bb4605726b2ca7a3ff706c4176649632b2cc68e1f912b8a578e3719ce7710885c7a966f49bcd43cb01010000").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid Schnorr signature length");
+            #[cfg(feature = "std")]
+            assert_eq!(err.to_string(), "invalid Schnorr signature");
+            #[cfg(not(feature = "std"))]
+            assert_eq!(err.to_string(), "invalid Schnorr signature: Invalid Schnorr signature size: 66");
             let err = hex_psbt!("70736274ff01005e02000000019bd48765230bf9a72e662001f972556e54f0c6f97feb56bcb5600d817f6995260100000000ffffffff0148e6052a01000000225120030da4fce4f7db28c2cb2951631e003713856597fe963882cb500e68112cca63000000000001012b00f2052a01000000225120c2247efbfd92ac47f6f40b8d42d169175a19fa9fa10e4a25d7f35eb4dd85b69241142cb13ac68248de806aa6a3659cf3c03eb6821d09c8114a4e868febde865bb6d2cd970e15f53fc0c82f950fd560ffa919b76172be017368a89913af074f400b093989756aa3739ccc689ec0fcf3a360be32cc0b59b16e93a1e8bb4605726b2ca7a3ff706c4176649632b2cc68e1f912b8a578e3719ce7710885c7a966f49bcd43cb0000").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid Schnorr signature length");
+            #[cfg(feature = "std")]
+            assert_eq!(err.to_string(), "invalid Schnorr signature");
+            #[cfg(not(feature = "std"))]
+            assert_eq!(err.to_string(), "invalid Schnorr signature: Invalid Schnorr signature size: 57");
             let err = hex_psbt!("70736274ff01005e02000000019bd48765230bf9a72e662001f972556e54f0c6f97feb56bcb5600d817f6995260100000000ffffffff0148e6052a01000000225120030da4fce4f7db28c2cb2951631e003713856597fe963882cb500e68112cca63000000000001012b00f2052a01000000225120c2247efbfd92ac47f6f40b8d42d169175a19fa9fa10e4a25d7f35eb4dd85b6926315c150929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac06f7d62059e9497a1a4a267569d9876da60101aff38e3529b9b939ce7f91ae970115f2e490af7cc45c4f78511f36057ce5c5a5c56325a29fb44dfc203f356e1f80023202cb13ac68248de806aa6a3659cf3c03eb6821d09c8114a4e868febde865bb6d2acc00000").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid control block");
+            assert_eq!(err.to_string(), "invalid control block");
             let err = hex_psbt!("70736274ff01005e02000000019bd48765230bf9a72e662001f972556e54f0c6f97feb56bcb5600d817f6995260100000000ffffffff0148e6052a01000000225120030da4fce4f7db28c2cb2951631e003713856597fe963882cb500e68112cca63000000000001012b00f2052a01000000225120c2247efbfd92ac47f6f40b8d42d169175a19fa9fa10e4a25d7f35eb4dd85b6926115c150929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac06f7d62059e9497a1a4a267569d9876da60101aff38e3529b9b939ce7f91ae970115f2e490af7cc45c4f78511f36057ce5c5a5c56325a29fb44dfc203f356e123202cb13ac68248de806aa6a3659cf3c03eb6821d09c8114a4e868febde865bb6d2acc00000").unwrap_err();
-            assert_eq!(err.to_string(), "parse failed: Invalid control block");
+            assert_eq!(err.to_string(), "invalid control block");
         }
 
         fn rtt_psbt(psbt: PartiallySignedTransaction) {

bitcoin/src/psbt/serialize.rs

@@ -152,7 +152,7 @@ impl Serialize for PublicKey {
 impl Deserialize for PublicKey {
     fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
         PublicKey::from_slice(bytes)
-            .map_err(|_| Error::ParseFailed("invalid public key"))
+            .map_err(Error::InvalidPublicKey)
     }
 }
 
@@ -165,7 +165,7 @@ impl Serialize for secp256k1::PublicKey {
 impl Deserialize for secp256k1::PublicKey {
     fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
         secp256k1::PublicKey::from_slice(bytes)
-            .map_err(|_| Error::ParseFailed("invalid public key"))
+            .map_err(Error::InvalidSecp256k1PublicKey)
     }
 }
 
@@ -193,13 +193,13 @@ impl Deserialize for ecdsa::Signature {
         ecdsa::Signature::from_slice(bytes)
             .map_err(|e| match e {
                 ecdsa::Error::EmptySignature => {
-                    Error::ParseFailed("Empty partial signature data")
+                    Error::InvalidEcdsaSignature(e)
                 }
                 ecdsa::Error::NonStandardSighashType(flag) => {
                     Error::NonStandardSighashType(flag)
                 }
                 ecdsa::Error::Secp256k1(..) => {
-                    Error::ParseFailed("Invalid Ecdsa signature")
+                    Error::InvalidEcdsaSignature(e)
                 }
                 ecdsa::Error::HexEncoding(..) =>  {
                     unreachable!("Decoding from slice, not hex")
@@ -279,7 +279,7 @@ impl Serialize for XOnlyPublicKey {
 impl Deserialize for XOnlyPublicKey {
     fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
         XOnlyPublicKey::from_slice(bytes)
-            .map_err(|_| Error::ParseFailed("Invalid xonly public key"))
+            .map_err(|_| Error::InvalidXOnlyPublicKey)
     }
 }
 
@@ -297,10 +297,10 @@ impl Deserialize for schnorr::Signature {
                     Error::NonStandardSighashType(flag as u32)
                 }
                 schnorr::Error::InvalidSignatureSize(_) => {
-                    Error::ParseFailed("Invalid Schnorr signature length")
+                    Error::InvalidSchnorrSignature(e)
                 }
                 schnorr::Error::Secp256k1(..) => {
-                    Error::ParseFailed("Invalid Schnorr signature")
+                    Error::InvalidSchnorrSignature(e)
                 }
             })
     }
@@ -336,7 +336,7 @@ impl Serialize for ControlBlock {
 impl Deserialize for ControlBlock {
     fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
         Self::from_slice(bytes)
-            .map_err(|_| Error::ParseFailed("Invalid control block"))
+            .map_err(|_| Error::InvalidControlBlock)
     }
 }
 
@@ -358,7 +358,7 @@ impl Deserialize for (ScriptBuf, LeafVersion) {
         // The last byte is LeafVersion.
         let script = ScriptBuf::deserialize(&bytes[..bytes.len() - 1])?;
         let leaf_ver = LeafVersion::from_consensus(bytes[bytes.len() - 1])
-            .map_err(|_| Error::ParseFailed("invalid leaf version"))?;
+            .map_err(|_| Error::InvalidLeafVersion)?;
         Ok((script, leaf_ver))
     }
 }
@@ -409,21 +409,20 @@ impl Deserialize for TapTree {
         let mut builder = TaprootBuilder::new();
         let mut bytes_iter = bytes.iter();
         while let Some(depth) = bytes_iter.next() {
-            let version = bytes_iter.next().ok_or(Error::ParseFailed("Invalid Taproot Builder"))?;
+            let version = bytes_iter.next().ok_or(Error::Taproot("Invalid Taproot Builder"))?;
             let (script, consumed) = deserialize_partial::<ScriptBuf>(bytes_iter.as_slice())?;
             if consumed > 0 {
                 bytes_iter.nth(consumed - 1);
             }
-
             let leaf_version = LeafVersion::from_consensus(*version)
-                .map_err(|_| Error::ParseFailed("Leaf Version Error"))?;
+                .map_err(|_| Error::InvalidLeafVersion)?;
             builder = builder.add_leaf_with_ver(*depth, script, leaf_version)
-                .map_err(|_| Error::ParseFailed("Tree not in DFS order"))?;
+                .map_err(|_| Error::Taproot("Tree not in DFS order"))?;
         }
         if builder.is_finalizable() && !builder.has_hidden_nodes() {
             Ok(TapTree(builder))
         } else {
-            Err(Error::ParseFailed("Incomplete taproot Tree"))
+            Err(Error::Taproot("Incomplete taproot Tree"))
         }
     }
 }