Merge rust-bitcoin#5027: units: Improve the decoders

b8af0d7 Update API text files (Tobin C. Harding)
6175586 units: Re-export the AmountDecoder (Tobin C. Harding)
d340c4d Update API text files (Tobin C. Harding)
b02b3ad units: Implement constructor for decoders (Tobin C. Harding)
c6ce97e units: Wrap decoder error types (Tobin C. Harding)
11811e0 units: Hide amount decoder error internals (Tobin C. Harding)

Pull request description:

  Improve all the decoders and decoder error types in `units`. Based on recent discussion. 
  
  This PR introduces policy on how we implement decoder error types (patch 1 and 2).
  
  LFG, decoders done by Nashville!


ACKs for top commit:
  apoelstra:
    ACK b8af0d7; successfully ran local tests


Tree-SHA512: bcc18be8b29ec067cf30d66d59d14da88c3edfbaf0dcdc5363ee6d2380cf149c8b0c63bb92f5d571c85167b74a59b36d097d58a21524895c9da20c7ac77f94c7

Author: apoelstra

api/units/all-features.txt

@@ -394,8 +394,24 @@ impl std::error::Error for PossiblyConfusingDenominationError {
 /// An error consensus decoding an `Amount`.
 #[cfg(feature = "encoding")]
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[non_exhaustive]
-pub enum AmountDecoderError {
+pub struct AmountDecoderError(pub(super) AmountDecoderErrorInner);
+
+#[cfg(feature = "encoding")]
+impl AmountDecoderError {
+    /// Constructs an EOF error.
+    pub(super) fn eof(e: encoding::UnexpectedEofError) -> Self {
+        Self(AmountDecoderErrorInner::UnexpectedEof(e))
+    }
+
+    /// Constructs an out of range (`Amount::from_sat`) error.
+    pub(super) fn out_of_range(e: OutOfRangeError) -> Self {
+        Self(AmountDecoderErrorInner::OutOfRange(e))
+    }
+}
+
+#[cfg(feature = "encoding")]
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub(super) enum AmountDecoderErrorInner {
     /// Not enough bytes given to decoder.
     UnexpectedEof(encoding::UnexpectedEofError),
     /// Decoded amount is too big.
@@ -407,27 +423,26 @@ impl From<Infallible> for AmountDecoderError {
     fn from(never: Infallible) -> Self { match never {} }
 }
 
-#[cfg(feature = "encoding")]
-impl From<encoding::UnexpectedEofError> for AmountDecoderError {
-    fn from(e: encoding::UnexpectedEofError) -> Self { Self::UnexpectedEof(e) }
-}
-
 #[cfg(feature = "encoding")]
 impl fmt::Display for AmountDecoderError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match *self {
-            Self::UnexpectedEof(ref e) => write_err!(f, "decode error"; e),
-            Self::OutOfRange(ref e) => write_err!(f, "decode error"; e),
+        use AmountDecoderErrorInner as E;
+
+        match self.0 {
+            E::UnexpectedEof(ref e) => write_err!(f, "decode error"; e),
+            E::OutOfRange(ref e) => write_err!(f, "decode error"; e),
         }
     }
 }
 
 #[cfg(all(feature = "std", feature = "encoding"))]
 impl std::error::Error for AmountDecoderError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match *self {
-            Self::UnexpectedEof(ref e) => Some(e),
-            Self::OutOfRange(ref e) => Some(e),
+        use AmountDecoderErrorInner as E;
+
+        match self.0 {
+            E::UnexpectedEof(ref e) => Some(e),
+            E::OutOfRange(ref e) => Some(e),
         }
     }
 }

units/src/amount/error.rs

@@ -37,6 +37,9 @@ pub use self::{
     signed::SignedAmount,
     unsigned::Amount,
 };
+#[doc(inline)]
+#[cfg(feature = "encoding")]
+pub use self::unsigned::AmountDecoder;
 #[cfg(feature = "encoding")]
 #[doc(no_inline)]
 pub use self::error::AmountDecoderError;

units/src/amount/mod.rs

@@ -582,20 +582,31 @@ impl encoding::Encodable for Amount {
 #[cfg(feature = "encoding")]
 pub struct AmountDecoder(encoding::ArrayDecoder<8>);
 
+#[cfg(feature = "encoding")]
+impl AmountDecoder {
+    /// Constructs a new [`Amount`] decoder.
+    pub fn new() -> Self { Self(encoding::ArrayDecoder::new()) }
+}
+
+#[cfg(feature = "encoding")]
+impl Default for AmountDecoder {
+    fn default() -> Self { Self::new() }
+}
+
 #[cfg(feature = "encoding")]
 impl encoding::Decoder for AmountDecoder {
     type Output = Amount;
     type Error = AmountDecoderError;
 
     #[inline]
     fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
-        Ok(self.0.push_bytes(bytes)?)
+        self.0.push_bytes(bytes).map_err(AmountDecoderError::eof)
     }
 
     #[inline]
     fn end(self) -> Result<Self::Output, Self::Error> {
-        let a = u64::from_le_bytes(self.0.end()?);
-        Ok(Amount::from_sat(a).map_err(AmountDecoderError::OutOfRange)?)
+        let a = u64::from_le_bytes(self.0.end().map_err(AmountDecoderError::eof)?);
+        Amount::from_sat(a).map_err(AmountDecoderError::out_of_range)
     }
 }
 

units/src/amount/unsigned.rs

@@ -11,10 +11,14 @@
 //! The difference between these types and the locktime types is that these types are thin wrappers
 //! whereas the locktime types contain more complex locktime specific abstractions.
 
+#[cfg(feature = "encoding")]
+use core::convert::Infallible;
 use core::{fmt, ops};
 
 #[cfg(feature = "arbitrary")]
 use arbitrary::{Arbitrary, Unstructured};
+#[cfg(feature = "encoding")]
+use internals::write_err;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
@@ -161,19 +165,30 @@ impl encoding::Encodable for BlockHeight {
 #[cfg(feature = "encoding")]
 pub struct BlockHeightDecoder(encoding::ArrayDecoder<4>);
 
+#[cfg(feature = "encoding")]
+impl Default for BlockHeightDecoder {
+    fn default() -> Self { Self::new() }
+}
+
+#[cfg(feature = "encoding")]
+impl BlockHeightDecoder {
+    /// Constructs a new [`BlockHeight`] decoder.
+    pub fn new() -> Self { Self(encoding::ArrayDecoder::new()) }
+}
+
 #[cfg(feature = "encoding")]
 impl encoding::Decoder for BlockHeightDecoder {
     type Output = BlockHeight;
-    type Error = encoding::UnexpectedEofError;
+    type Error = BlockHeightDecoderError;
 
     #[inline]
     fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
-        self.0.push_bytes(bytes)
+        self.0.push_bytes(bytes).map_err(BlockHeightDecoderError)
     }
 
     #[inline]
     fn end(self) -> Result<Self::Output, Self::Error> {
-        let n = u32::from_le_bytes(self.0.end()?);
+        let n = u32::from_le_bytes(self.0.end().map_err(BlockHeightDecoderError)?);
         Ok(BlockHeight::from_u32(n))
     }
 }
@@ -184,6 +199,28 @@ impl encoding::Decodable for BlockHeight {
     fn decoder() -> Self::Decoder { BlockHeightDecoder(encoding::ArrayDecoder::<4>::new()) }
 }
 
+/// An error consensus decoding an `BlockHeight`.
+#[cfg(feature = "encoding")]
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct BlockHeightDecoderError(encoding::UnexpectedEofError);
+
+#[cfg(feature = "encoding")]
+impl From<Infallible> for BlockHeightDecoderError {
+    fn from(never: Infallible) -> Self { match never {} }
+}
+
+#[cfg(feature = "encoding")]
+impl fmt::Display for BlockHeightDecoderError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write_err!(f, "block height decoder error"; self.0)
+    }
+}
+
+#[cfg(all(feature = "std", feature = "encoding"))]
+impl std::error::Error for BlockHeightDecoderError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
+}
+
 impl_u32_wrapper! {
     /// An unsigned block interval.
     ///

units/src/block.rs

@@ -6,10 +6,34 @@ use core::convert::Infallible;
 use core::fmt;
 
 use internals::error::InputString;
+#[cfg(feature = "encoding")]
+use internals::write_err;
 
 use super::{Height, MedianTimePast, LOCK_TIME_THRESHOLD};
 use crate::parse_int::ParseIntError;
 
+/// An error consensus decoding an `LockTime`.
+#[cfg(feature = "encoding")]
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct LockTimeDecoderError(pub(super) encoding::UnexpectedEofError);
+
+#[cfg(feature = "encoding")]
+impl From<Infallible> for LockTimeDecoderError {
+    fn from(never: Infallible) -> Self { match never {} }
+}
+
+#[cfg(feature = "encoding")]
+impl fmt::Display for LockTimeDecoderError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write_err!(f, "lock time decoder error"; self.0)
+    }
+}
+
+#[cfg(all(feature = "std", feature = "encoding"))]
+impl std::error::Error for LockTimeDecoderError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
+}
+
 /// Tried to satisfy a lock-by-time lock using a height value.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct IncompatibleHeightError {

units/src/locktime/absolute/error.rs

@@ -24,6 +24,8 @@ use crate::parse_int::{self, PrefixedHexError, UnprefixedHexError};
 pub use self::error::{
     ConversionError, IncompatibleHeightError, IncompatibleTimeError, ParseHeightError, ParseTimeError,
 };
+#[cfg(feature = "encoding")]
+pub use self::error::LockTimeDecoderError;
 
 /// The Threshold for deciding whether a lock time value is a height or a time (see [Bitcoin Core]).
 ///
@@ -421,19 +423,30 @@ impl encoding::Encodable for LockTime {
 #[cfg(feature = "encoding")]
 pub struct LockTimeDecoder(encoding::ArrayDecoder<4>);
 
+#[cfg(feature = "encoding")]
+impl LockTimeDecoder {
+    /// Constructs a new [`LockTime`] decoder.
+    pub fn new() -> Self { Self(encoding::ArrayDecoder::new()) }
+}
+
+#[cfg(feature = "encoding")]
+impl Default for LockTimeDecoder {
+    fn default() -> Self { Self::new() }
+}
+
 #[cfg(feature = "encoding")]
 impl encoding::Decoder for LockTimeDecoder {
     type Output = LockTime;
-    type Error = encoding::UnexpectedEofError;
+    type Error = LockTimeDecoderError;
 
     #[inline]
     fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
-        self.0.push_bytes(bytes)
+        Ok(self.0.push_bytes(bytes).map_err(LockTimeDecoderError)?)
     }
 
     #[inline]
     fn end(self) -> Result<Self::Output, Self::Error> {
-        let n = u32::from_le_bytes(self.0.end()?);
+        let n = u32::from_le_bytes(self.0.end().map_err(LockTimeDecoderError)?);
         Ok(LockTime::from_consensus(n))
     }
 }

units/src/locktime/absolute/mod.rs

@@ -14,10 +14,14 @@
 //! [BIP-0068]: <https://github.com/bitcoin/bips/blob/master/bip-0068.mediawiki>
 //! [BIP-0125]: <https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki>
 
+#[cfg(feature = "encoding")]
+use core::convert::Infallible;
 use core::fmt;
 
 #[cfg(feature = "arbitrary")]
 use arbitrary::{Arbitrary, Unstructured};
+#[cfg(feature = "encoding")]
+use internals::write_err;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
@@ -284,19 +288,30 @@ impl encoding::Encodable for Sequence {
 #[cfg(feature = "encoding")]
 pub struct SequenceDecoder(encoding::ArrayDecoder<4>);
 
+#[cfg(feature = "encoding")]
+impl Default for SequenceDecoder {
+    fn default() -> Self { Self::new() }
+}
+
+#[cfg(feature = "encoding")]
+impl SequenceDecoder {
+    /// Constructs a new [`Sequence`] decoder.
+    pub fn new() -> Self { Self(encoding::ArrayDecoder::new()) }
+}
+
 #[cfg(feature = "encoding")]
 impl encoding::Decoder for SequenceDecoder {
     type Output = Sequence;
-    type Error = encoding::UnexpectedEofError;
+    type Error = SequenceDecoderError;
 
     #[inline]
     fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
-        self.0.push_bytes(bytes)
+        self.0.push_bytes(bytes).map_err(SequenceDecoderError)
     }
 
     #[inline]
     fn end(self) -> Result<Self::Output, Self::Error> {
-        let n = u32::from_le_bytes(self.0.end()?);
+        let n = u32::from_le_bytes(self.0.end().map_err(SequenceDecoderError)?);
         Ok(Sequence::from_consensus(n))
     }
 }
@@ -307,6 +322,28 @@ impl encoding::Decodable for Sequence {
     fn decoder() -> Self::Decoder { SequenceDecoder(encoding::ArrayDecoder::<4>::new()) }
 }
 
+/// An error consensus decoding an `Sequence`.
+#[cfg(feature = "encoding")]
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct SequenceDecoderError(encoding::UnexpectedEofError);
+
+#[cfg(feature = "encoding")]
+impl From<Infallible> for SequenceDecoderError {
+    fn from(never: Infallible) -> Self { match never {} }
+}
+
+#[cfg(feature = "encoding")]
+impl fmt::Display for SequenceDecoderError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write_err!(f, "sequence decoder error"; self.0)
+    }
+}
+
+#[cfg(all(feature = "std", feature = "encoding"))]
+impl std::error::Error for SequenceDecoderError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
+}
+
 #[cfg(feature = "arbitrary")]
 #[cfg(feature = "alloc")]
 impl<'a> Arbitrary<'a> for Sequence {