Move serialization to zebra-chain, rework traits.

The core serialization logic is now in zebra-chain and consists of two
pairs of traits:

These are analogues of the Serde `Serialize` and `Deserialize` traits,
but explicitly intended for consensus-critical serialization formats.
Thus some struct `Foo` may have derived `Serialize` and `Deserialize`
implementations for (internal) use with Serde, and explicitly-written
`ZcashSerialize` and `ZcashDeserialize` implementations for use in
consensus-critical contexts.  The consensus-critical implementations
provide `zcash`-prefixed `zcash_serialize` and `zcash_deserialize`
methods to make it clear in client contexts that the serialization is
consensus-critical.

These are utility traits, analogous to the `ReadBytesExt` and
`WriteBytesExt` traits provided by `byteorder`.  A generic
implementation is provided for any `io::Read` or `io::Write`, so that
bringing the traits into scope adds additional Zcash-specific traits to
generic readers and writers -- for instance, writing a `u64` in the
Bitcoin "CompactSize" format.

zebra-chain/Cargo.toml

@@ -8,3 +8,5 @@ edition = "2018"
 
 [dependencies]
 sha2 = "0.8"
+byteorder = "1.3"
+failure = "0.1"

zebra-chain/src/lib.rs

@@ -1,5 +1,9 @@
 //! Blockchain-related datastructures for Zebra. 🦓
 #![deny(missing_docs)]
 
+#[macro_use]
+extern crate failure;
+
 pub mod block;
 pub mod types;
+pub mod serialization;

zebra-chain/src/serialization.rs

@@ -1,10 +1,15 @@
-//! Serializat
+//! Serialization for Zcash.
+//!
+//! This module contains four traits: `ZcashSerialize` and `ZcashDeserialize`,
+//! analogs of the Serde `Serialize` and `Deserialize` traits but intended for
+//! consensus-critical Zcash serialization formats, and `WriteZcashExt` and
+//! `ReadZcashExt`, extension traits for `io::Read` and `io::Write` with utility functions
+//! for reading and writing data (e.g., the Bitcoin variable-integer format).
 
 use std::io;
+use std::net::{IpAddr, SocketAddr};
 
-use crate::types::{Magic, Version};
+use byteorder::{BigEndian, LittleEndian, ReadBytesExt, WriteBytesExt};
-
-use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 
 /// A serialization error.
 // XXX refine error types -- better to use boxed errors?
@@ -14,6 +19,7 @@ pub enum SerializationError {
     #[fail(display = "io error {}", _0)]
     IoError(io::Error),
     /// The data to be deserialized was malformed.
+    // XXX refine errors
     #[fail(display = "parse error: {}", _0)]
     ParseError(&'static str),
 }
@@ -25,6 +31,36 @@ impl From<io::Error> for SerializationError {
     }
 }
 
+/// Consensus-critical serialization for Zcash.
+///
+/// This trait provides a generic serialization for consensus-critical
+/// formats, such as network messages, transactions, blocks, etc. It is intended
+/// for use only in consensus-critical contexts; in other contexts, such as
+/// internal storage, it would be preferable to use Serde.
+pub trait ZcashSerialize: Sized {
+    /// Write `self` to the given `writer` using the canonical format.
+    ///
+    /// This function has a `zcash_` prefix to alert the reader that the
+    /// serialization in use is consensus-critical serialization, rather than
+    /// some other kind of serialization.
+    fn zcash_serialize<W: io::Write>(&self, writer: W) -> Result<(), SerializationError>;
+}
+
+/// Consensus-critical serialization for Zcash.
+///
+/// This trait provides a generic deserialization for consensus-critical
+/// formats, such as network messages, transactions, blocks, etc. It is intended
+/// for use only in consensus-critical contexts; in other contexts, such as
+/// internal storage, it would be preferable to use Serde.
+pub trait ZcashDeserialize: Sized {
+    /// Try to read `self` from the given `reader`.
+    ///
+    /// This function has a `zcash_` prefix to alert the reader that the
+    /// serialization in use is consensus-critical serialization, rather than
+    /// some other kind of serialization.
+    fn zcash_deserialize<R: io::Read>(reader: R) -> Result<Self, SerializationError>;
+}
+
 /// Extends [`Write`] with methods for writing Zcash/Bitcoin types.
 ///
 /// [`Write`]: https://doc.rust-lang.org/std/io/trait.Write.html
@@ -34,7 +70,7 @@ pub trait WriteZcashExt: io::Write {
     /// # Examples
     ///
     /// ```rust
-    /// use zebra_network::serialization::WriteZcashExt;
+    /// use zebra_chain::serialization::WriteZcashExt;
     ///
     /// let mut buf = Vec::new();
     /// buf.write_compactsize(0x12).unwrap();
@@ -74,6 +110,31 @@ pub trait WriteZcashExt: io::Write {
             }
         }
     }
+
+    /// Write an `IpAddr` in Bitcoin format.
+    #[inline]
+    fn write_ip_addr(&mut self, addr: IpAddr) -> io::Result<()> {
+        use std::net::IpAddr::*;
+        let v6_addr = match addr {
+            V4(ref v4) => v4.to_ipv6_mapped(),
+            V6(v6) => v6,
+        };
+        self.write_all(&v6_addr.octets())
+    }
+
+    /// Write a `SocketAddr` in Bitcoin format.
+    #[inline]
+    fn write_socket_addr(&mut self, addr: SocketAddr) -> io::Result<()> {
+        self.write_ip_addr(addr.ip())?;
+        self.write_u16::<BigEndian>(addr.port())
+    }
+
+    /// Write a string in Bitcoin format.
+    #[inline]
+    fn write_string(&mut self, string: &str) -> io::Result<()> {
+        self.write_compactsize(string.len() as u64)?;
+        self.write_all(string.as_bytes())
+    }
 }
 
 /// Mark all types implementing `Write` as implementing the extension.
@@ -88,7 +149,7 @@ pub trait ReadZcashExt: io::Read {
     /// # Examples
     ///
     /// ```rust
-    /// use zebra_network::serialization::ReadZcashExt;
+    /// use zebra_chain::serialization::ReadZcashExt;
     ///
     /// use std::io::Cursor;
     /// assert_eq!(
@@ -118,82 +179,23 @@ pub trait ReadZcashExt: io::Read {
     /// );
     /// ```
     #[inline]
-    fn read_compactsize(&mut self) -> Result<u64, SerializationError> {
+    fn read_compactsize(&mut self) -> io::Result<u64> {
         let flag_byte = self.read_u8()?;
         match flag_byte {
-            0xff => Ok(self.read_u64::<LittleEndian>()?),
+            0xff => self.read_u64::<LittleEndian>(),
             0xfe => Ok(self.read_u32::<LittleEndian>()? as u64),
             0xfd => Ok(self.read_u16::<LittleEndian>()? as u64),
             n => Ok(n as u64),
         }
     }
-}
 
-/// Mark all types implementing `Read` as implementing the extension.
+    /// Read an IP address in Bitcoin format.
-impl<R: io::Read + ?Sized> ReadZcashExt for R {}
+    #[inline]
-
+    fn read_ip_addr(&mut self) -> io::Result<IpAddr> {
-/// Consensus-critical (de)serialization for Zcash.
-///
-/// This trait provides a generic (de)serialization for consensus-critical
-/// formats, such as network messages, transactions, blocks, etc. It is intended
-/// for use only in consensus-critical contexts; in other contexts, such as
-/// internal storage, it would be preferable to use Serde.
-///
-/// # Questions
-///
-/// ## Should this live here in `zebra-network` or in `zebra-chain`?
-///
-/// This is a proxy question for "is this serialization logic required outside of
-/// networking contexts", which requires mapping out the "network context"
-/// concept more precisely.
-///
-/// ## Should the `version` and `magic` parameters always be passed?
-///
-/// These are required for, e.g., serializing message headers, but possibly not
-/// for serializing transactions?
-pub trait ZcashSerialization: Sized {
-    /// Write `self` to the given `writer` using the canonical format.
-    fn write<W: io::Write>(
-        &self,
-        mut writer: W,
-        magic: Magic,
-        version: Version,
-    ) -> Result<(), SerializationError>;
-
-    /// Try to read `self` from the given `reader`.
-    fn try_read<R: io::Read>(
-        reader: R,
-        magic: Magic,
-        version: Version,
-    ) -> Result<Self, SerializationError>;
-}
-
-impl ZcashSerialization for std::net::IpAddr {
-    fn write<W: io::Write>(
-        &self,
-        mut writer: W,
-        _magic: Magic,
-        _version: Version,
-    ) -> Result<(), SerializationError> {
-        use std::net::IpAddr::*;
-        let v6_addr = match *self {
-            V4(ref addr) => addr.to_ipv6_mapped(),
-            V6(addr) => addr,
-        };
-        writer.write_all(&v6_addr.octets())?;
-        Ok(())
-    }
-
-    /// Try to read `self` from the given `reader`.
-    fn try_read<R: io::Read>(
-        mut reader: R,
-        _magic: Magic,
-        _version: Version,
-    ) -> Result<Self, SerializationError> {
         use std::net::{IpAddr::*, Ipv6Addr};
 
         let mut octets = [0u8; 16];
-        reader.read_exact(&mut octets)?;
+        self.read_exact(&mut octets)?;
         let v6_addr = std::net::Ipv6Addr::from(octets);
 
         match v6_addr.to_ipv4() {
@@ -201,33 +203,24 @@ impl ZcashSerialization for std::net::IpAddr {
             None => Ok(V6(v6_addr)),
         }
     }
+
+    /// Read a Bitcoin-encoded `SocketAddr`.
+    #[inline]
+    fn read_socket_addr(&mut self) -> io::Result<SocketAddr> {
+        let ip_addr = self.read_ip_addr()?;
+        let port = self.read_u16::<BigEndian>()?;
+        Ok(SocketAddr::new(ip_addr, port))
     }
 
-// XXX because the serialization trait has both read and write methods, we have
+    /// Read a Bitcoin-encoded UTF-8 string.
-// to implement it for String rather than impl<'a> ... for &'a str (as in that
+    #[inline]
-// case we can't deserialize into a borrowed &'str, only an owned String), so we
+    fn read_string(&mut self) -> io::Result<String> {
-// can't serialize 'static str
+        let len = self.read_compactsize()?;
-impl ZcashSerialization for String {
-    fn write<W: io::Write>(
-        &self,
-        mut writer: W,
-        _magic: Magic,
-        _version: Version,
-    ) -> Result<(), SerializationError> {
-        writer.write_compactsize(self.len() as u64)?;
-        writer.write_all(self.as_bytes())?;
-        Ok(())
-    }
-
-    /// Try to read `self` from the given `reader`.
-    fn try_read<R: io::Read>(
-        mut reader: R,
-        _magic: Magic,
-        _version: Version,
-    ) -> Result<Self, SerializationError> {
-        let len = reader.read_compactsize()?;
         let mut buf = vec![0; len as usize];
-        reader.read_exact(&mut buf)?;
+        self.read_exact(&mut buf)?;
-        String::from_utf8(buf).map_err(|e| SerializationError::ParseError("invalid utf-8"))
+        String::from_utf8(buf).map_err(|_| io::ErrorKind::InvalidData.into())
     }
 }
+
+/// Mark all types implementing `Read` as implementing the extension.
+impl<R: io::Read + ?Sized> ReadZcashExt for R {}

zebra-network/src/lib.rs

@@ -5,7 +5,6 @@
 #[macro_use]
 extern crate failure;
 
-pub mod serialization;
 pub mod message;
 pub mod types;
 mod constants;

zebra-network/src/message.rs

@@ -5,10 +5,14 @@ use std::io;
 use byteorder::{BigEndian, LittleEndian, WriteBytesExt};
 use chrono::{DateTime, Utc};
 
-use zebra_chain::types::Sha256dChecksum;
+use zebra_chain::{
+    serialization::{
+        ReadZcashExt, SerializationError, WriteZcashExt, ZcashDeserialize, ZcashSerialize,
+    },
+    types::Sha256dChecksum,
+};
 
 use crate::meta_addr::MetaAddr;
-use crate::serialization::{ReadZcashExt, SerializationError, WriteZcashExt, ZcashSerialization};
 use crate::types::*;
 
 /// A Bitcoin-like network message for the Zcash protocol.
@@ -264,8 +268,13 @@ pub enum RejectReason {
 //
 // Maybe just write some functions and refactor later?
 
-impl ZcashSerialization for Message {
+impl Message {
-    fn write<W: io::Write>(
+    /// Serialize `self` into the given writer, similarly to `ZcashSerialize`.
+    ///
+    /// This is similar to [`ZcashSerialize::zcash_serialize`], but not part of
+    /// that trait, because message serialization requires additional parameters
+    /// (the network magic and the network version).
+    pub fn zcash_serialize<W: io::Write>(
         &self,
         mut writer: W,
         magic: Magic,
@@ -356,15 +365,13 @@ impl Message {
                 // However, the version message encodes net_addrs without the
                 // timestamp field, so we encode the `MetaAddr`s manually here.
                 writer.write_u64::<LittleEndian>(address_receiving.services.0)?;
-                address_receiving.addr.ip().write(&mut writer, m, v)?;
+                writer.write_socket_addr(address_receiving.addr)?;
-                writer.write_u16::<BigEndian>(address_receiving.addr.port())?;
 
                 writer.write_u64::<LittleEndian>(address_from.services.0)?;
-                address_from.addr.ip().write(&mut writer, m, v)?;
+                writer.write_socket_addr(address_from.addr)?;
-                writer.write_u16::<BigEndian>(address_from.addr.port())?;
 
                 writer.write_u64::<LittleEndian>(nonce.0)?;
-                user_agent.write(&mut writer, m, v)?;
+                writer.write_string(&user_agent)?;
                 writer.write_u32::<LittleEndian>(start_height.0)?;
                 writer.write_u8(*relay as u8)?;
             }
@@ -373,8 +380,20 @@ impl Message {
         Ok(())
     }
 
-    /// Try to deserialize a [`Message`] from the given reader.
+    /// Try to deserialize a [`Message`] from the given reader, similarly to `ZcashDeserialize`.
-    pub fn try_read_body<R: io::Read>(
+    ///
+    /// This is similar to [`ZcashSerialize::zcash_serialize`], but not part of
+    /// that trait, because message serialization requires additional parameters
+    /// (the network magic and the network version).
+    pub fn zcash_deserialize<R: io::Read>(
+        _reader: R,
+        _magic: Magic,
+        _version: Version,
+    ) -> Result<Self, SerializationError> {
+        unimplemented!()
+    }
+
+    fn deserialize_body<R: io::Read>(
         _reader: R,
         _magic: Magic,
         _version: Version,