Replace serialize::Error::Detail with variants

- Add serialize::Error::ParseFailed(&'static str) variant for
  serialization errors without context
- Add appropriate variants to replace network::Error::Detail for
  serialization error with context
- Remove error method from SimpleDecoders

src/blockdata/transaction.rs

@@ -417,7 +417,7 @@ impl<D: SimpleDecoder> ConsensusDecodable<D> for Transaction {
                 }
                 // We don't support anything else
                 x => {
-                    Err(d.error(format!("segwit flag {:02x} not understood", x)))
+                    Err(serialize::Error::UnsupportedSegwitFlag(x))
                 }
             }
         // non-segwit

src/network/constants.rs

@@ -117,7 +117,7 @@ impl<D: SimpleDecoder> ConsensusDecodable<D> for Network {
         u32::consensus_decode(d)
             .and_then(|m| {
                 Network::from_magic(m)
-                    .ok_or(d.error(format!("Unknown network (magic {:x})", m)))
+                    .ok_or(serialize::Error::UnknownNetworkMagic(m))
             })
     }
 }

src/network/encodable.rs

@@ -146,7 +146,7 @@ impl<D: SimpleDecoder> ConsensusDecodable<D> for String {
     #[inline]
     fn consensus_decode(d: &mut D) -> Result<String, serialize::Error> {
         String::from_utf8(ConsensusDecodable::consensus_decode(d)?)
-            .map_err(|_| d.error("String was not valid UTF8".to_owned()))
+            .map_err(|_| serialize::Error::ParseFailed("String was not valid UTF8"))
     }
 }
 
@@ -205,9 +205,9 @@ impl<D: SimpleDecoder, T: ConsensusDecodable<D>> ConsensusDecodable<D> for Vec<T
         let VarInt(len): VarInt = ConsensusDecodable::consensus_decode(d)?;
         let byte_size = (len as usize)
                             .checked_mul(mem::size_of::<T>())
-                            .ok_or(d.error("Invalid length".to_owned()))?;
+                            .ok_or(serialize::Error::ParseFailed("Invalid length"))?;
         if byte_size > MAX_VEC_SIZE {
-            return Err(d.error(format!("tried to allocate vec of size {} (max {})", byte_size, MAX_VEC_SIZE)));
+            return Err(serialize::Error::OversizedVectorAllocation { requested: byte_size, max: MAX_VEC_SIZE })
         }
         let mut ret = Vec::with_capacity(len as usize);
         for _ in 0..len { ret.push(ConsensusDecodable::consensus_decode(d)?); }
@@ -226,7 +226,7 @@ impl<D: SimpleDecoder, T: ConsensusDecodable<D>> ConsensusDecodable<D> for Box<[
         let VarInt(len): VarInt = ConsensusDecodable::consensus_decode(d)?;
         let len = len as usize;
         if len > MAX_VEC_SIZE {
-            return Err(d.error(format!("tried to allocate vec of size {} (max {})", len, MAX_VEC_SIZE)));
+            return Err(serialize::Error::OversizedVectorAllocation { requested: len, max: MAX_VEC_SIZE })
         }
         let mut ret = Vec::with_capacity(len);
         for _ in 0..len { ret.push(ConsensusDecodable::consensus_decode(d)?); }
@@ -291,7 +291,10 @@ impl<D: SimpleDecoder> ConsensusDecodable<D> for CheckedData {
         for _ in 0..len { ret.push(ConsensusDecodable::consensus_decode(d)?); }
         let expected_checksum = sha2_checksum(&ret);
         if expected_checksum != checksum {
-            Err(d.error(format!("bad checksum {:?} (expected {:?})", checksum, expected_checksum)))
+            Err(serialize::Error::InvalidChecksum {
+                expected: expected_checksum,
+                actual: checksum,
+            })
         } else {
             Ok(CheckedData(ret))
         }

src/network/message.rs

@@ -196,7 +196,7 @@ impl<D: SimpleDecoder> ConsensusDecodable<D> for RawNetworkMessage {
             "pong"    => NetworkMessage::Pong(ConsensusDecodable::consensus_decode(&mut mem_d)?),
             "tx"      => NetworkMessage::Tx(ConsensusDecodable::consensus_decode(&mut mem_d)?),
             "alert"   => NetworkMessage::Alert(ConsensusDecodable::consensus_decode(&mut mem_d)?),
-            cmd => return Err(d.error(format!("unrecognized network command `{}`", cmd)))
+            _ => return Err(serialize::Error::UnrecognizedNetworkCommand(cmd)),
         };
         Ok(RawNetworkMessage {
             magic: magic,

src/network/serialize.rs

@@ -43,24 +43,57 @@ pub enum Error {
     Bech32(bitcoin_bech32::Error),
     /// Error from the `byteorder` crate
     ByteOrder(io::Error),
-    /// Network magic was not what we expected
+    /// Network magic was not expected
-    BadNetworkMagic(u32, u32),
+    UnexpectedNetworkMagic {
-    /// Network message was unrecognized
+        /// The expected network magic
-    BadNetworkMessage(String),
+        expected: u32,
+        /// The unexpected network magic
+        actual: u32,
+    },
+    /// Tried to allocate an oversized vector
+    OversizedVectorAllocation{
+        /// The capacity requested
+        requested: usize,
+        /// The maximum capacity
+        max: usize,
+    },
+    /// Checksum was invalid
+    InvalidChecksum {
+        /// The expected checksum
+        expected: [u8; 4],
+        /// The invalid checksum
+        actual: [u8; 4],
+    },
+    /// Network magic was unknown
+    UnknownNetworkMagic(u32),
     /// Parsing error
-    ParseFailed,
+    ParseFailed(&'static str),
-    /// Error propagated from subsystem
-    Detail(String, Box<Error>),
     /// Unsupported witness version
     UnsupportedWitnessVersion(u8),
+    /// Unsupported Segwit flag
+    UnsupportedSegwitFlag(u8),
+    /// Unrecognized network command
+    UnrecognizedNetworkCommand(String),
+    /// Unexpected hex digit
+    UnexpectedHexDigit(char),
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
             Error::Io(ref e) => fmt::Display::fmt(e, f),
-            Error::Detail(ref s, ref e) => write!(f, "{}: {}", s, e),
+            Error::Base58(ref e) => fmt::Display::fmt(e, f),
-            ref x => f.write_str(error::Error::description(x)),
+            Error::Bech32(ref e) => fmt::Display::fmt(e, f),
+            Error::ByteOrder(ref e) => fmt::Display::fmt(e, f),
+            Error::UnexpectedNetworkMagic { expected: ref e, actual: ref a } => write!(f, "{}: expected {}, actual {}", error::Error::description(self), e, a),
+            Error::OversizedVectorAllocation { requested: ref r, max: ref m } => write!(f, "{}: requested {}, maximum {}", error::Error::description(self), r, m),
+            Error::InvalidChecksum { expected: ref e, actual: ref a } => write!(f, "{}: expected {}, actual {}", error::Error::description(self), hex_encode(e), hex_encode(a)),
+            Error::UnknownNetworkMagic(ref m) => write!(f, "{}: {}", error::Error::description(self), m),
+            Error::ParseFailed(ref e) => write!(f, "{}: {}", error::Error::description(self), e),
+            Error::UnsupportedWitnessVersion(ref wver) => write!(f, "{}: {}", error::Error::description(self), wver),
+            Error::UnsupportedSegwitFlag(ref swflag) => write!(f, "{}: {}", error::Error::description(self), swflag),
+            Error::UnrecognizedNetworkCommand(ref nwcmd) => write!(f, "{}: {}", error::Error::description(self), nwcmd),
+            Error::UnexpectedHexDigit(ref d) => write!(f, "{}: {}", error::Error::description(self), d),
         }
     }
 }
@@ -72,7 +105,6 @@ impl error::Error for Error {
             Error::Base58(ref e) => Some(e),
             Error::Bech32(ref e) => Some(e),
             Error::ByteOrder(ref e) => Some(e),
-            Error::Detail(_, ref e) => Some(e),
             _ => None
         }
     }
@@ -80,14 +112,18 @@ impl error::Error for Error {
     fn description(&self) -> &str {
         match *self {
             Error::Io(ref e) => e.description(),
-            Error::ParseFailed => "parsing error",
-            Error::Detail(_, ref e) => e.description(),
             Error::Base58(ref e) => e.description(),
             Error::Bech32(ref e) => e.description(),
             Error::ByteOrder(ref e) => e.description(),
-            Error::BadNetworkMagic(_, _) => "incorrect network magic",
+            Error::UnexpectedNetworkMagic { .. } => "unexpected network magic",
-            Error::BadNetworkMessage(_) => "incorrect/unexpected network message",
+            Error::OversizedVectorAllocation { .. } => "allocation of oversized vector requested",
-            Error::UnsupportedWitnessVersion(_) => "unsupported witness version",
+            Error::InvalidChecksum { .. } => "invalid checksum",
+            Error::UnknownNetworkMagic(..) => "unknown network magic",
+            Error::ParseFailed(..) => "parse failed",
+            Error::UnsupportedWitnessVersion(..) => "unsupported witness version",
+            Error::UnsupportedSegwitFlag(..) => "unsupported segwit version",
+            Error::UnrecognizedNetworkCommand(..) => "unrecognized network command",
+            Error::UnexpectedHexDigit(..) => "unexpected hex digit",
         }
     }
 }
@@ -156,7 +192,7 @@ pub fn deserialize<'a, T>(data: &'a [u8]) -> Result<T, Error>
     if decoder.into_inner().position() == data.len() as u64 {
         Ok(rv)
     } else {
-        Err(Error::ParseFailed)
+        Err(Error::ParseFailed("data not consumed entirely when explicitly deserializing"))
     }
 }
 
@@ -230,9 +266,6 @@ pub trait SimpleDecoder {
 
     /// Read a boolean
     fn read_bool(&mut self) -> Result<bool, Error>;
-
-    /// Signal a decoding error
-    fn error(&mut self, err: String) -> Error;
 }
 
 macro_rules! encoder_fn {
@@ -298,11 +331,6 @@ impl<R: Read> SimpleDecoder for RawDecoder<R> {
             Err(e) => Err(Error::Io(e))
         }
     }
-
-    #[inline]
-    fn error(&mut self, err: String) -> Error {
-        Error::Detail(err, Box::new(Error::ParseFailed))
-    }
 }
 
 // Aren't really any tests here.. the main functions are serialize and

src/network/socket.rs

@@ -182,7 +182,10 @@ impl Socket {
             // Then for magic (this should come before parse error, but we can't
             // get to it if the deserialization failed). TODO restructure this
             if decoded.magic != self.magic {
-                Err(util::Error::Serialize(serialize::Error::BadNetworkMagic(self.magic, decoded.magic)))
+                Err(serialize::Error::UnexpectedNetworkMagic {
+                    expected: self.magic,
+                    actual: decoded.magic,
+                }.into())
             } else {
                 Ok(decoded.payload)
             }

src/util/misc.rs

@@ -29,24 +29,15 @@ pub fn hex_bytes(s: &str) -> Result<Vec<u8>, serialize::Error> {
         if e.is_err() { e }
         else {
             match (f.to_digit(16), s.to_digit(16)) {
-                (None, _) => Err(serialize::Error::Detail(
+                (None, _) => Err(serialize::Error::UnexpectedHexDigit(f)),
-                    format!("expected hex, got {:}", f),
+                (_, None) => Err(serialize::Error::UnexpectedHexDigit(s)),
-                    Box::new(serialize::Error::ParseFailed)
-                )),
-                (_, None) => Err(serialize::Error::Detail(
-                    format!("expected hex, got {:}", s),
-                    Box::new(serialize::Error::ParseFailed)
-                )),
                 (Some(f), Some(s)) => { v.push((f * 0x10 + s) as u8); Ok(()) }
             }
         }
     )?;
     // Check that there was no remainder
     match iter.remainder() {
-        Some(_) => Err(serialize::Error::Detail(
+        Some(_) => Err(serialize::Error::ParseFailed("hexstring of odd length")),
-            "hexstring of odd length".to_owned(),
-            Box::new(serialize::Error::ParseFailed)
-        )),
         None => Ok(v)
     }
 }

src/util/mod.rs

@@ -59,19 +59,20 @@ pub trait BitArray {
     fn one() -> Self;
 }
 
-/// A general error code
+/// A general error code, other errors should implement conversions to/from this
+/// if appropriate.
 #[derive(Debug)]
 pub enum Error {
-    /// The `target` field of a block header did not match the expected difficulty
-    SpvBadTarget,
-    /// The header hash is not below the target
-    SpvBadProofOfWork,
     /// secp-related error
     Secp256k1(secp256k1::Error),
     /// Serialization error
     Serialize(serialize::Error),
     /// Network error
     Network(network::Error),
+    /// The header hash is not below the target
+    SpvBadProofOfWork,
+    /// The `target` field of a block header did not match the expected difficulty
+    SpvBadTarget,
 }
 
 impl fmt::Display for Error {
@@ -96,10 +97,10 @@ impl error::Error for Error {
     fn description(&self) -> &str {
         match *self {
             Error::Secp256k1(ref e) => e.description(),
-            Error::SpvBadTarget => "target incorrect",
-            Error::SpvBadProofOfWork => "target correct but not attained",
             Error::Serialize(ref e) => e.description(),
             Error::Network(ref e) => e.description(),
+            Error::SpvBadProofOfWork => "target correct but not attained",
+            Error::SpvBadTarget => "target incorrect",
         }
     }
 }