From 68abfdb0b9dab9c4141c99a6be191327760bcf95 Mon Sep 17 00:00:00 2001 From: Arturo Marquez Date: Tue, 17 Jan 2023 10:21:52 -0600 Subject: [PATCH] Better downflow of information in `psbt::Error` See third point in `https://github.com/rust-bitcoin/rust-bitcoin/issues/837` --- bitcoin/src/psbt/error.rs | 51 ++++++++++++++++++++++++++++++---- bitcoin/src/psbt/map/global.rs | 16 +++++------ bitcoin/src/psbt/mod.rs | 27 ++++++++++++------ bitcoin/src/psbt/serialize.rs | 27 +++++++++--------- 4 files changed, 84 insertions(+), 37 deletions(-) diff --git a/bitcoin/src/psbt/error.rs b/bitcoin/src/psbt/error.rs index 29b3d7cc..e1a1614b 100644 --- a/bitcoin/src/psbt/error.rs +++ b/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), - /// 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 } } } diff --git a/bitcoin/src/psbt/map/global.rs b/bitcoin/src/psbt/map/global.rs index e443e6c9..908b2a7c 100644 --- a/bitcoin/src/psbt/map/global.rs +++ b/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::::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)) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index 6f1c3d2c..d239a780 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/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) { diff --git a/bitcoin/src/psbt/serialize.rs b/bitcoin/src/psbt/serialize.rs index 93c6ac7c..e31bce6a 100644 --- a/bitcoin/src/psbt/serialize.rs +++ b/bitcoin/src/psbt/serialize.rs @@ -152,7 +152,7 @@ impl Serialize for PublicKey { impl Deserialize for PublicKey { fn deserialize(bytes: &[u8]) -> Result { 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 { 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 { 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::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::(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")) } } }