From 713196be0d61313bd1873c4ec366efb4e43f6892 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 18 Oct 2024 12:27:35 +1100 Subject: [PATCH] Return DeserError from encode::deserialize The `encode::deserialize` function never returns an I/O error. Add a new error type that expresses this. --- bitcoin/src/blockdata/transaction.rs | 2 +- bitcoin/src/consensus/encode.rs | 25 +++++++++--------- bitcoin/src/consensus/error.rs | 39 ++++++++++++++++++++++++++++ bitcoin/src/consensus/mod.rs | 2 +- bitcoin/src/psbt/error.rs | 17 ++++++++++++ 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 8eabc3054..48e68161b 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -1486,7 +1486,7 @@ mod tests { let tx_bytes = hex!("0000fd000001021921212121212121212121f8b372b0239cc1dff600000000004f4f4f4f4f4f4f4f000000000000000000000000000000333732343133380d000000000000000000000000000000ff000000000009000dff000000000000000800000000000000000d"); let tx: Result = deserialize(&tx_bytes); assert!(tx.is_err()); - assert!(matches!(tx.unwrap_err(), crate::consensus::Error::Parse(_))); + assert!(matches!(tx.unwrap_err(), crate::consensus::DeserializeError::Parse(_))); } #[test] diff --git a/bitcoin/src/consensus/encode.rs b/bitcoin/src/consensus/encode.rs index 5b9902f01..462e68a31 100644 --- a/bitcoin/src/consensus/encode.rs +++ b/bitcoin/src/consensus/encode.rs @@ -36,7 +36,7 @@ use crate::taproot::TapLeafHash; use crate::transaction::{Transaction, TxIn, TxOut}; #[rustfmt::skip] // Keep public re-exports separate. -pub use super::{Error, FromHexError, ParseError}; +pub use super::{Error, FromHexError, ParseError, DeserializeError}; /// Encodes an object into a vector. pub fn serialize(data: &T) -> Vec { @@ -53,14 +53,14 @@ pub fn serialize_hex(data: &T) -> String { /// Deserializes an object from a vector, will error if said deserialization /// doesn't consume the entire vector. -pub fn deserialize(data: &[u8]) -> Result { +pub fn deserialize(data: &[u8]) -> Result { let (rv, consumed) = deserialize_partial(data)?; // Fail if data are not consumed entirely. if consumed == data.len() { Ok(rv) } else { - Err(super::parse_failed_error("data not consumed entirely when explicitly deserializing")) + Err(DeserializeError::Unconsumed) } } @@ -74,10 +74,15 @@ pub fn deserialize_hex(hex: &str) -> Result { /// Deserializes an object from a vector, but will not report an error if said deserialization /// doesn't consume the entire vector. -pub fn deserialize_partial(data: &[u8]) -> Result<(T, usize), Error> { +pub fn deserialize_partial(data: &[u8]) -> Result<(T, usize), ParseError> { let mut decoder = Cursor::new(data); - let rv = Decodable::consensus_decode_from_finite_reader(&mut decoder)?; + let rv = match Decodable::consensus_decode_from_finite_reader(&mut decoder) { + Ok(rv) => rv, + Err(Error::Parse(e)) => return Err(e), + Err(Error::Io(_)) => + unreachable!("consensus_decode code never returns an I/O error for in-memory reads"), + }; let consumed = decoder.position() as usize; Ok((rv, consumed)) @@ -1033,10 +1038,7 @@ mod tests { // by making sure it fails with `MissingData` and not an `OversizedVectorAllocation` Error. let err = deserialize::(&serialize(&(super::MAX_VEC_SIZE as u32))).unwrap_err(); - match err { - Error::Io(e) => panic!("unexpected I/O error {}", e), - Error::Parse(e) => assert_eq!(e, ParseError::MissingData), - } + assert_eq!(err, DeserializeError::Parse(ParseError::MissingData)); test_len_is_max_vec::(); test_len_is_max_vec::(); @@ -1061,10 +1063,7 @@ mod tests { let mut buf = Vec::new(); buf.emit_compact_size(super::MAX_VEC_SIZE / mem::size_of::()).unwrap(); let err = deserialize::>(&buf).unwrap_err(); - match err { - Error::Io(e) => panic!("unexpected I/O error {}", e), - Error::Parse(e) => assert_eq!(e, ParseError::MissingData), - } + assert_eq!(err, DeserializeError::Parse(ParseError::MissingData)); } #[test] diff --git a/bitcoin/src/consensus/error.rs b/bitcoin/src/consensus/error.rs index 75dad6f69..4321cf776 100644 --- a/bitcoin/src/consensus/error.rs +++ b/bitcoin/src/consensus/error.rs @@ -11,6 +11,45 @@ use internals::write_err; #[cfg(doc)] use super::IterReader; +/// Error deserializing from a slice. +#[derive(Debug, PartialEq, Eq)] +#[non_exhaustive] +pub enum DeserializeError { + /// Error parsing encoded object. + Parse(ParseError), + /// Data unconsumed error. + Unconsumed, +} + +internals::impl_from_infallible!(DeserializeError); + +impl fmt::Display for DeserializeError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use DeserializeError::*; + + match *self { + Parse(ref e) => write_err!(f, "error parsing encoded object"; e), + Unconsumed => write!(f, "data not consumed entirely when deserializing"), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for DeserializeError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use DeserializeError::*; + + match *self { + Parse(ref e) => Some(e), + Unconsumed => None, + } + } +} + +impl From for DeserializeError { + fn from(e: ParseError) -> Self { Self::Parse(e) } +} + /// Error when consensus decoding from an `[IterReader]`. #[derive(Debug)] pub enum DecodeError { diff --git a/bitcoin/src/consensus/mod.rs b/bitcoin/src/consensus/mod.rs index 7ef30ada6..04dbd5479 100644 --- a/bitcoin/src/consensus/mod.rs +++ b/bitcoin/src/consensus/mod.rs @@ -20,7 +20,7 @@ use crate::consensus; #[doc(inline)] pub use self::{ encode::{deserialize, deserialize_partial, serialize, Decodable, Encodable, ReadExt, WriteExt}, - error::{Error, FromHexError, DecodeError, ParseError}, + error::{Error, FromHexError, DecodeError, ParseError, DeserializeError}, }; pub(crate) use self::error::parse_failed_error; diff --git a/bitcoin/src/psbt/error.rs b/bitcoin/src/psbt/error.rs index 1804459e4..e1d266650 100644 --- a/bitcoin/src/psbt/error.rs +++ b/bitcoin/src/psbt/error.rs @@ -71,6 +71,10 @@ pub enum Error { CombineInconsistentKeySources(Box), /// Serialization error in bitcoin consensus-encoded structures ConsensusEncoding(encode::Error), + /// Deserialization error in bitcoin consensus-encoded structures. + ConsensusDeserialize(encode::DeserializeError), + /// Error parsing bitcoin consensus-encoded object. + ConsensusParse(encode::ParseError), /// Negative fee NegativeFee, /// Integer overflow in fee calculation @@ -141,6 +145,9 @@ impl fmt::Display for Error { write!(f, "combine conflict: {}", s) } ConsensusEncoding(ref e) => write_err!(f, "bitcoin consensus encoding error"; e), + ConsensusDeserialize(ref e) => + write_err!(f, "bitcoin consensus deserializaton error"; e), + ConsensusParse(ref e) => write_err!(f, "error parsing bitcoin consensus encoded object"; e), NegativeFee => f.write_str("PSBT has a negative fee which is not allowed"), FeeOverflow => f.write_str("integer overflow in fee calculation"), InvalidPublicKey(ref e) => write_err!(f, "invalid public key"; e), @@ -169,6 +176,8 @@ impl std::error::Error for Error { match *self { InvalidHash(ref e) => Some(e), ConsensusEncoding(ref e) => Some(e), + ConsensusDeserialize(ref e) => Some(e), + ConsensusParse(ref e) => Some(e), Io(ref e) => Some(e), InvalidMagic | MissingUtxo @@ -211,6 +220,14 @@ impl From for Error { fn from(e: encode::Error) -> Self { Error::ConsensusEncoding(e) } } +impl From for Error { + fn from(e: encode::DeserializeError) -> Self { Error::ConsensusDeserialize(e) } +} + +impl From for Error { + fn from(e: encode::ParseError) -> Self { Error::ConsensusParse(e) } +} + impl From for Error { fn from(e: io::Error) -> Self { Error::Io(e) } }