Merge rust-bitcoin/rust-bitcoin#2039: Add a `consensus::deserialize_hex` function

16a813734c Implement consensus deserialize_hex (Tobin C. Harding)

Pull request description:

  We have `serialize_hex` and `deserialize` but no `deserialize_hex`, add it.

  Move the `IterReader` out of `consensus::serde` to the `consensus` module.

  Add some additional logic to the `DecodeError`, I'm not sure why this wasn't there before?

  Use the `HexSliceToBytesIter` by way of the `IterReader` to deserialize an arbitrary hex string. Add unit tests to check that we consume all bytes when deserializing a fixed size object (a transaction).

ACKs for top commit:
  apoelstra:
    ACK 16a813734c
  sanket1729:
    ACK 16a813734c

Tree-SHA512: 121285cb328ca01bf9fd2a716e6d625fa93113a11613d44c576e3e49a9d06dc181165d2d9bfb9beea7c3d2aff264f64ade4965acd594b05ce0d1660e7493d2e4

bitcoin/src/consensus/encode.rs

@@ -18,6 +18,7 @@
 use core::{fmt, mem, u32};
 
 use hashes::{sha256, sha256d, Hash};
+use hex::error::{InvalidCharError, OddLengthStringError};
 use internals::write_err;
 use io::{Cursor, BufRead, Read, Write};
 
@@ -25,6 +26,7 @@ use crate::bip152::{PrefilledTransaction, ShortId};
 use crate::bip158::{FilterHash, FilterHeader};
 use crate::blockdata::block::{self, BlockHash, TxMerkleNode};
 use crate::blockdata::transaction::{Transaction, TxIn, TxOut};
+use crate::consensus::{DecodeError, IterReader};
 #[cfg(feature = "std")]
 use crate::p2p::{
     address::{AddrV2Message, Address},
@@ -101,6 +103,44 @@ impl From<io::Error> for Error {
     fn from(error: io::Error) -> Self { Error::Io(error) }
 }
 
+/// Hex deserialization error.
+#[derive(Debug)]
+pub enum FromHexError {
+    /// Purported hex string had odd length.
+    OddLengthString(OddLengthStringError),
+    /// Decoding error.
+    Decode(DecodeError<InvalidCharError>)
+}
+
+impl fmt::Display for FromHexError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use FromHexError::*;
+
+        match *self {
+            OddLengthString(ref e) =>
+                write_err!(f, "odd length, failed to create bytes from hex"; e),
+            Decode(ref e) => write_err!(f, "decoding error"; e),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for FromHexError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use FromHexError::*;
+
+        match *self {
+            OddLengthString(ref e) => Some(e),
+            Decode(ref e) => Some(e),
+        }
+    }
+}
+
+impl From<OddLengthStringError> for FromHexError {
+    #[inline]
+    fn from(e: OddLengthStringError) -> Self { Self::OddLengthString(e) }
+}
+
 /// Encodes an object into a vector.
 pub fn serialize<T: Encodable + ?Sized>(data: &T) -> Vec<u8> {
     let mut encoder = Vec::new();
@@ -127,6 +167,14 @@ pub fn deserialize<T: Decodable>(data: &[u8]) -> Result<T, Error> {
     }
 }
 
+/// Deserialize any decodable type from a hex string, will error if said deserialization
+/// doesn't consume the entire vector.
+pub fn deserialize_hex<T: Decodable>(hex: &str) -> Result<T, FromHexError> {
+    let iter = hex::HexSliceToBytesIter::new(hex)?;
+    let reader = IterReader::new(iter);
+    Ok(reader.decode().map_err(FromHexError::Decode)?)
+}
+
 /// 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<T: Decodable>(data: &[u8]) -> Result<(T, usize), Error> {
@@ -1232,4 +1280,19 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    fn deserialize_tx_hex() {
+        let hex = include_str!("../../tests/data/previous_tx_0_hex"); // An arbitrary transaction.
+        assert!(deserialize_hex::<Transaction>(hex).is_ok())
+    }
+
+    #[test]
+    fn deserialize_tx_hex_too_many_bytes() {
+        use crate::consensus::DecodeError;
+
+        let mut hex = include_str!("../../tests/data/previous_tx_0_hex").to_string(); // An arbitrary transaction.
+        hex.push_str("abcdef");
+        assert!(matches!(deserialize_hex::<Transaction>(&hex).unwrap_err(), FromHexError::Decode(DecodeError::TooManyBytes)));
+    }
 }

bitcoin/src/consensus/mod.rs

@@ -13,6 +13,13 @@ pub mod serde;
 #[cfg(feature = "bitcoinconsensus")]
 pub mod validation;
 
+use core::fmt;
+
+use io::{Read, BufRead};
+use internals::write_err;
+
+use crate::consensus;
+
 #[rustfmt::skip]                // Keep public re-exports separate.
 #[doc(inline)]
 pub use self::{
@@ -25,3 +32,120 @@ pub use self::{
 pub use self::validation::{
     verify_script, verify_script_with_flags, verify_transaction, verify_transaction_with_flags,
 };
+
+struct IterReader<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> {
+    iterator: core::iter::Fuse<I>,
+    buf: Option<u8>,
+    error: Option<E>,
+}
+
+impl<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> IterReader<E, I> {
+    pub(crate) fn new(iterator: I) -> Self { IterReader { iterator: iterator.fuse(), buf: None, error: None } }
+
+    fn decode<T: Decodable>(mut self) -> Result<T, DecodeError<E>> {
+        let result = T::consensus_decode(&mut self);
+        match (result, self.error) {
+            (Ok(_), None) if self.iterator.next().is_some() => Err(DecodeError::TooManyBytes),
+            (Ok(value), None) => Ok(value),
+            (Ok(_), Some(error)) => panic!("{} silently ate the error: {:?}", core::any::type_name::<T>(), error),
+
+            (Err(consensus::encode::Error::Io(io_error)), Some(de_error)) if io_error.kind() == io::ErrorKind::Other && io_error.get_ref().is_none() => Err(DecodeError::Other(de_error)),
+            (Err(consensus_error), None) => Err(DecodeError::Consensus(consensus_error)),
+            (Err(consensus::encode::Error::Io(io_error)), de_error) => panic!("Unexpected IO error {:?} returned from {}::consensus_decode(), deserialization error: {:?}", io_error, core::any::type_name::<T>(), de_error),
+            (Err(consensus_error), Some(de_error)) => panic!("{} should've returned `Other` IO error because of deserialization error {:?} but it returned consensus error {:?} instead", core::any::type_name::<T>(), de_error, consensus_error),
+        }
+    }
+}
+
+impl<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> Read for IterReader<E, I> {
+    fn read(&mut self, mut buf: &mut [u8]) -> io::Result<usize> {
+        let mut count = 0;
+        if buf.is_empty() {
+            return Ok(0);
+        }
+
+        if let Some(first) = self.buf.take() {
+            buf[0] = first;
+            buf = &mut buf[1..];
+            count += 1;
+        }
+        for (dst, src) in buf.iter_mut().zip(&mut self.iterator) {
+            match src {
+                Ok(byte) => *dst = byte,
+                Err(error) => {
+                    self.error = Some(error);
+                    return Err(io::ErrorKind::Other.into());
+                }
+            }
+            // bounded by the length of buf
+            count += 1;
+        }
+        Ok(count)
+    }
+}
+
+impl<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> BufRead for IterReader<E, I> {
+    fn fill_buf(&mut self) -> Result<&[u8], io::Error> {
+        // matching on reference rather than using `ref` confuses borrow checker
+        if let Some(ref byte) = self.buf {
+            Ok(core::slice::from_ref(byte))
+        } else {
+            match self.iterator.next() {
+                Some(Ok(byte)) => {
+                    self.buf = Some(byte);
+                    Ok(core::slice::from_ref(self.buf.as_ref().expect("we've just filled it")))
+                },
+                Some(Err(error)) => {
+                    self.error = Some(error);
+                    Err(io::ErrorKind::Other.into())
+                },
+                None => Ok(&[]),
+            }
+        }
+    }
+
+    fn consume(&mut self, len: usize) {
+        debug_assert!(len <= 1);
+        if len > 0 {
+            self.buf = None;
+        }
+    }
+}
+
+/// Error when consensus decoding from an `[IterReader]`.
+#[derive(Debug)]
+pub enum DecodeError<E> {
+    /// Attempted to decode an object from an iterator that yielded too many bytes.
+    TooManyBytes,
+    /// Invalid consensus encoding.
+    Consensus(consensus::encode::Error),
+    /// Other decoding error.
+    Other(E),
+}
+
+internals::impl_from_infallible!(DecodeError<E>);
+
+impl<E: fmt::Debug> fmt::Display for DecodeError<E> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use DecodeError::*;
+
+        match *self {
+            TooManyBytes => write!(f, "attempted to decode object from an iterator that yielded too many bytes"),
+            Consensus(ref e) => write_err!(f, "invalid consensus encoding"; e),
+            Other(ref other) => write!(f, "other decoding error: {:?}", other),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl<E: fmt::Debug> std::error::Error for DecodeError<E> {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use DecodeError::*;
+
+        match *self {
+            TooManyBytes => None,
+            Consensus(ref e) => Some(e),
+            Other(_) => None, // TODO: Is this correct?
+        }
+    }
+}

bitcoin/src/consensus/serde.rs

@@ -12,11 +12,12 @@
 use core::fmt;
 use core::marker::PhantomData;
 
-use io::{BufRead, Read, Write};
+use io::Write;
 use serde::de::{SeqAccess, Unexpected, Visitor};
 use serde::ser::SerializeSeq;
 use serde::{Deserializer, Serializer};
 
+use crate::consensus::{DecodeError, IterReader};
 use super::encode::Error as ConsensusError;
 use super::{Decodable, Encodable};
 
@@ -352,14 +353,6 @@ impl<D: fmt::Display> serde::de::Expected for DisplayExpected<D> {
     }
 }
 
-enum DecodeError<E> {
-    TooManyBytes,
-    Consensus(ConsensusError),
-    Other(E),
-}
-
-internals::impl_from_infallible!(DecodeError<E>);
-
 // not a trait impl because we panic on some variants
 fn consensus_error_into_serde<E: serde::de::Error>(error: ConsensusError) -> E {
     match error {
@@ -409,86 +402,6 @@ where
     }
 }
 
-struct IterReader<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> {
-    iterator: core::iter::Fuse<I>,
-    buf: Option<u8>,
-    error: Option<E>,
-}
-
-impl<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> IterReader<E, I> {
-    fn new(iterator: I) -> Self { IterReader { iterator: iterator.fuse(), buf: None, error: None } }
-
-    fn decode<T: Decodable>(mut self) -> Result<T, DecodeError<E>> {
-        let result = T::consensus_decode(&mut self);
-        match (result, self.error) {
-            (Ok(_), None) if self.iterator.next().is_some() => {
-                Err(DecodeError::TooManyBytes)
-            },
-            (Ok(value), None) => Ok(value),
-            (Ok(_), Some(error)) => panic!("{} silently ate the error: {:?}", core::any::type_name::<T>(), error),
-            (Err(ConsensusError::Io(io_error)), Some(de_error)) if io_error.kind() == io::ErrorKind::Other && io_error.get_ref().is_none() => Err(DecodeError::Other(de_error)),
-            (Err(consensus_error), None) => Err(DecodeError::Consensus(consensus_error)),
-            (Err(ConsensusError::Io(io_error)), de_error) => panic!("Unexpected IO error {:?} returned from {}::consensus_decode(), deserialization error: {:?}", io_error, core::any::type_name::<T>(), de_error),
-            (Err(consensus_error), Some(de_error)) => panic!("{} should've returned `Other` IO error because of deserialization error {:?} but it returned consensus error {:?} instead", core::any::type_name::<T>(), de_error, consensus_error),
-        }
-    }
-}
-
-impl<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> Read for IterReader<E, I> {
-    fn read(&mut self, mut buf: &mut [u8]) -> io::Result<usize> {
-        let mut count = 0;
-        if buf.is_empty() {
-            return Ok(0);
-        }
-
-        if let Some(first) = self.buf.take() {
-            buf[0] = first;
-            buf = &mut buf[1..];
-            count += 1;
-        }
-        for (dst, src) in buf.iter_mut().zip(&mut self.iterator) {
-            match src {
-                Ok(byte) => *dst = byte,
-                Err(error) => {
-                    self.error = Some(error);
-                    return Err(io::ErrorKind::Other.into());
-                }
-            }
-            // bounded by the length of buf
-            count += 1;
-        }
-        Ok(count)
-    }
-}
-
-impl<E: fmt::Debug, I: Iterator<Item = Result<u8, E>>> BufRead for IterReader<E, I> {
-    fn fill_buf(&mut self) -> Result<&[u8], io::Error> {
-        // matching on reference rather than using `ref` confuses borrow checker
-        if let Some(ref byte) = self.buf {
-            Ok(core::slice::from_ref(byte))
-        } else {
-            match self.iterator.next() {
-                Some(Ok(byte)) => {
-                    self.buf = Some(byte);
-                    Ok(core::slice::from_ref(self.buf.as_ref().expect("we've just filled it")))
-                },
-                Some(Err(error)) => {
-                    self.error = Some(error);
-                    Err(io::ErrorKind::Other.into())
-                },
-                None => Ok(&[]),
-            }
-        }
-    }
-
-    fn consume(&mut self, len: usize) {
-        debug_assert!(len <= 1);
-        if len > 0 {
-            self.buf = None;
-        }
-    }
-}
-
 /// Helper for `#[serde(with = "")]`.
 ///
 /// To (de)serialize a field using consensus encoding you can write e.g.: