Implement addr v1 serialization using a separate AddrV1 type

zebra-chain/src/serialization.rs

@@ -29,7 +29,7 @@ pub use compact_size::{CompactSize64, CompactSizeMessage};
 pub use constraint::AtLeastOne;
 pub use date_time::{DateTime32, Duration32};
 pub use error::SerializationError;
-pub use read_zcash::{canonical_socket_addr, ReadZcashExt};
+pub use read_zcash::ReadZcashExt;
 pub use write_zcash::WriteZcashExt;
 pub use zcash_deserialize::{
     zcash_deserialize_bytes_external_count, zcash_deserialize_external_count, TrustedPreallocate,

zebra-chain/src/serialization/arbitrary.rs

@@ -1,13 +1,11 @@
 //! Arbitrary data generation for serialization proptests
 
-use std::{convert::TryInto, net::SocketAddr};
+use std::convert::TryInto;
 
 use chrono::{TimeZone, Utc, MAX_DATETIME, MIN_DATETIME};
 use proptest::{arbitrary::any, prelude::*};
 
-use super::{
-    read_zcash::canonical_socket_addr, CompactSizeMessage, DateTime32, MAX_PROTOCOL_MESSAGE_LEN,
-};
+use super::{CompactSizeMessage, DateTime32, MAX_PROTOCOL_MESSAGE_LEN};
 
 impl Arbitrary for DateTime32 {
     type Parameters = ();
@@ -60,13 +58,6 @@ pub fn datetime_u32() -> impl Strategy<Value = chrono::DateTime<Utc>> {
     any::<DateTime32>().prop_map(Into::into)
 }
 
-/// Returns a random canonical Zebra `SocketAddr`.
-///
-/// See [`canonical_ip_addr`] for details.
-pub fn canonical_socket_addr_strategy() -> impl Strategy<Value = SocketAddr> {
-    any::<SocketAddr>().prop_map(canonical_socket_addr)
-}
-
 impl Arbitrary for CompactSizeMessage {
     type Parameters = ();
 

zebra-chain/src/serialization/read_zcash.rs

@@ -1,32 +1,9 @@
-use std::{
-    io,
-    net::{IpAddr, Ipv6Addr, SocketAddr},
-};
-
-use byteorder::{BigEndian, ReadBytesExt};
+use std::io;
 
 /// Extends [`Read`] with methods for writing Zcash/Bitcoin types.
 ///
 /// [`Read`]: https://doc.rust-lang.org/std/io/trait.Read.html
 pub trait ReadZcashExt: io::Read {
-    /// Read an IP address in Bitcoin format.
-    #[inline]
-    fn read_ip_addr(&mut self) -> io::Result<IpAddr> {
-        let mut octets = [0u8; 16];
-        self.read_exact(&mut octets)?;
-        let v6_addr = Ipv6Addr::from(octets);
-
-        Ok(canonical_ip_addr(&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))
-    }
-
     /// Convenience method to read a `[u8; 4]`.
     #[inline]
     fn read_4_bytes(&mut self) -> io::Result<[u8; 4]> {
@@ -62,37 +39,3 @@ pub trait ReadZcashExt: io::Read {
 
 /// Mark all types implementing `Read` as implementing the extension.
 impl<R: io::Read + ?Sized> ReadZcashExt for R {}
-
-/// Transform a Zcash-deserialized IPv6 address into a canonical Zebra IP address.
-///
-/// Zcash uses IPv6-mapped IPv4 addresses in its network protocol. Zebra converts
-/// those addresses to `Ipv4Addr`s, for maximum compatibility with systems that
-/// don't understand IPv6.
-pub fn canonical_ip_addr(v6_addr: &Ipv6Addr) -> IpAddr {
-    use IpAddr::*;
-
-    // TODO: replace with `to_ipv4_mapped` when that stabilizes
-    // https://github.com/rust-lang/rust/issues/27709
-    match v6_addr.to_ipv4() {
-        // workaround for unstable `to_ipv4_mapped`
-        Some(v4_addr) if v4_addr.to_ipv6_mapped() == *v6_addr => V4(v4_addr),
-        Some(_) | None => V6(*v6_addr),
-    }
-}
-
-/// Transform a `SocketAddr` into a canonical Zebra `SocketAddr`, converting
-/// IPv6-mapped IPv4 addresses, and removing IPv6 scope IDs and flow information.
-///
-/// See [`canonical_ip_addr`] for detailed info on IPv6-mapped IPv4 addresses.
-pub fn canonical_socket_addr(socket_addr: impl Into<SocketAddr>) -> SocketAddr {
-    use SocketAddr::*;
-
-    let mut socket_addr = socket_addr.into();
-    if let V6(v6_socket_addr) = socket_addr {
-        let canonical_ip = canonical_ip_addr(v6_socket_addr.ip());
-        // creating a new SocketAddr removes scope IDs and flow information
-        socket_addr = SocketAddr::new(canonical_ip, socket_addr.port());
-    }
-
-    socket_addr
-}

zebra-chain/src/serialization/write_zcash.rs

@@ -1,32 +1,9 @@
-use std::{
-    io,
-    net::{IpAddr, SocketAddr},
-};
-
-use byteorder::{BigEndian, WriteBytesExt};
+use std::io;
 
 /// Extends [`Write`] with methods for writing Zcash/Bitcoin types.
 ///
 /// [`Write`]: https://doc.rust-lang.org/std/io/trait.Write.html
 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())
-    }
-
     /// Convenience method to write exactly 32 u8's.
     #[inline]
     fn write_32_bytes(&mut self, bytes: &[u8; 32]) -> io::Result<()> {

zebra-chain/src/serialization/zcash_deserialize.rs

@@ -1,6 +1,7 @@
 use std::{
     convert::{TryFrom, TryInto},
     io,
+    net::Ipv6Addr,
 };
 
 use super::{AtLeastOne, CompactSizeMessage, SerializationError, MAX_PROTOCOL_MESSAGE_LEN};
@@ -128,6 +129,19 @@ impl ZcashDeserialize for String {
     }
 }
 
+// We don't impl ZcashDeserialize for Ipv4Addr or SocketAddrs,
+// because the IPv4 and port formats are different in addr (v1) and addrv2 messages.
+
+/// Read a Bitcoin-encoded IPv6 address.
+impl ZcashDeserialize for Ipv6Addr {
+    fn zcash_deserialize<R: io::Read>(mut reader: R) -> Result<Self, SerializationError> {
+        let mut ipv6_addr = [0u8; 16];
+        reader.read_exact(&mut ipv6_addr)?;
+
+        Ok(Ipv6Addr::from(ipv6_addr))
+    }
+}
+
 /// Helper for deserializing more succinctly via type inference
 pub trait ZcashDeserializeInto {
     /// Deserialize based on type inference

zebra-chain/src/serialization/zcash_serialize.rs

@@ -1,4 +1,4 @@
-use std::{convert::TryInto, io};
+use std::{convert::TryInto, io, net::Ipv6Addr};
 
 use super::{AtLeastOne, CompactSizeMessage};
 
@@ -174,3 +174,13 @@ impl ZcashSerialize for String {
         self.as_str().zcash_serialize(&mut writer)
     }
 }
+
+// We don't impl ZcashSerialize for Ipv4Addr or SocketAddrs,
+// because the IPv4 and port formats are different in addr (v1) and addrv2 messages.
+
+/// Write a Bitcoin-encoded IPv6 address.
+impl ZcashSerialize for Ipv6Addr {
+    fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), std::io::Error> {
+        writer.write_all(&self.octets())
+    }
+}

zebra-network/src/address_book.rs

@@ -10,9 +10,10 @@ use std::{
 
 use tracing::Span;
 
-use zebra_chain::serialization::canonical_socket_addr;
-
-use crate::{meta_addr::MetaAddrChange, types::MetaAddr, PeerAddrState};
+use crate::{
+    meta_addr::MetaAddrChange, protocol::external::canonical_socket_addr, types::MetaAddr,
+    PeerAddrState,
+};
 
 #[cfg(test)]
 mod tests;

zebra-network/src/config.rs

@@ -7,9 +7,9 @@ use std::{
 
 use serde::{de, Deserialize, Deserializer};
 
-use zebra_chain::{parameters::Network, serialization::canonical_socket_addr};
+use zebra_chain::parameters::Network;
 
-use crate::{constants, BoxError};
+use crate::{constants, protocol::external::canonical_socket_addr, BoxError};
 
 #[cfg(test)]
 mod tests;

zebra-network/src/isolated.rs

@@ -98,20 +98,26 @@ impl Service<Request> for Wrapper {
 
 #[cfg(test)]
 mod tests {
+
     use super::*;
 
     #[tokio::test]
     async fn connect_isolated_sends_minimally_distinguished_version_message() {
+        use std::net::SocketAddr;
+
+        use futures::stream::StreamExt;
+        use tokio_util::codec::Framed;
+
         use crate::{
-            protocol::external::{Codec, Message},
+            protocol::external::{AddrInVersion, Codec, Message},
             types::PeerServices,
         };
-        use futures::stream::StreamExt;
-        use tokio_util::codec::Framed;
 
         let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
         let listen_addr = listener.local_addr().unwrap();
 
+        let fixed_isolated_addr: SocketAddr = "0.0.0.0:8233".parse().unwrap();
+
         let conn = tokio::net::TcpStream::connect(listen_addr).await.unwrap();
 
         tokio::spawn(connect_isolated(conn, "".to_string()));
@@ -139,7 +145,7 @@ mod tests {
             assert_eq!(timestamp.timestamp() % (5 * 60), 0);
             assert_eq!(
                 address_from,
-                (PeerServices::empty(), "0.0.0.0:8233".parse().unwrap())
+                AddrInVersion::new(fixed_isolated_addr, PeerServices::empty()),
             );
             assert_eq!(user_agent, "");
             assert_eq!(start_height.0, 0);

zebra-network/src/meta_addr.rs

@@ -7,22 +7,27 @@ use std::{
     time::Instant,
 };
 
-use zebra_chain::serialization::{canonical_socket_addr, DateTime32};
+use zebra_chain::serialization::DateTime32;
 
-use crate::{constants, protocol::types::PeerServices};
+use crate::{
+    constants,
+    protocol::{external::canonical_socket_addr, types::PeerServices},
+};
 
 use MetaAddrChange::*;
 use PeerAddrState::*;
 
 #[cfg(any(test, feature = "proptest-impl"))]
 use proptest_derive::Arbitrary;
+
 #[cfg(any(test, feature = "proptest-impl"))]
-use zebra_chain::serialization::arbitrary::canonical_socket_addr_strategy;
+use crate::protocol::external::arbitrary::canonical_socket_addr_strategy;
+
 #[cfg(any(test, feature = "proptest-impl"))]
 pub(crate) mod arbitrary;
 
 #[cfg(test)]
-mod tests;
+pub(crate) mod tests;
 
 /// Peer connection state, based on our interactions with the peer.
 ///
@@ -112,6 +117,8 @@ impl PartialOrd for PeerAddrState {
 
 /// An address with metadata on its advertised services and last-seen time.
 ///
+/// This struct can be created from `addr` or `addrv2` messages.
+///
 /// [Bitcoin reference](https://en.bitcoin.it/wiki/Protocol_documentation#Network_address)
 #[derive(Copy, Clone, Debug)]
 #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
@@ -131,8 +138,8 @@ pub struct MetaAddr {
     /// The exact meaning depends on `last_connection_state`:
     ///   - `Responded`: the services advertised by this peer, the last time we
     ///      performed a handshake with it
-    ///   - `NeverAttempted`: the unverified services provided by the remote peer
-    ///     that sent us this address
+    ///   - `NeverAttempted`: the unverified services advertised by another peer,
+    ///      then gossiped by the peer that sent us this address
     ///   - `Failed` or `AttemptPending`: unverified services via another peer,
     ///      or services advertised in a previous handshake
     ///

zebra-network/src/meta_addr/arbitrary.rs

@@ -2,9 +2,11 @@ use std::net::SocketAddr;
 
 use proptest::{arbitrary::any, collection::vec, prelude::*};
 
-use super::{MetaAddr, MetaAddrChange, PeerServices};
+use zebra_chain::serialization::DateTime32;
+
+use crate::protocol::external::arbitrary::canonical_socket_addr_strategy;
 
-use zebra_chain::serialization::{arbitrary::canonical_socket_addr_strategy, DateTime32};
+use super::{MetaAddr, MetaAddrChange, PeerServices};
 
 /// The largest number of random changes we want to apply to a [`MetaAddr`].
 ///

zebra-network/src/meta_addr/tests.rs

@@ -1,3 +1,6 @@
-mod check;
+//! Tests for MetaAddrs
+
+pub(crate) mod check;
+
 mod prop;
 mod vectors;