From a71db64f6c72e842377d389533ca69227af5687a Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jun 2021 10:40:58 +1000 Subject: [PATCH 01/17] Always send our local listener with the latest time Previously, whenever there was an inbound request for peers, we would clone the address book and update it with the local listener. This had two impacts: - the listener could conflict with an existing entry, rather than unconditionally replacing it, and - the listener was briefly included in the address book metrics. As a side-effect, this change also makes sanitization slightly faster, because it avoids some useless peer filtering and sorting. --- zebra-network/src/address_book.rs | 19 ++++++++++++++++--- zebrad/src/components/inbound.rs | 6 +----- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/zebra-network/src/address_book.rs b/zebra-network/src/address_book.rs index decf9f060d7..d32f1691952 100644 --- a/zebra-network/src/address_book.rs +++ b/zebra-network/src/address_book.rs @@ -126,16 +126,29 @@ impl AddressBook { } /// Get the local listener address. - pub fn get_local_listener(&self) -> MetaAddrChange { + /// + /// This address contains minimal state, but it is not sanitized. + pub fn get_local_listener(&self) -> MetaAddr { MetaAddr::new_local_listener(&self.local_listener) + .into_new_meta_addr() + .expect("unexpected invalid new local listener addr") } /// Get the contents of `self` in random order with sanitized timestamps. pub fn sanitized(&self) -> Vec { use rand::seq::SliceRandom; let _guard = self.span.enter(); - let mut peers = self - .peers() + + let mut peers = self.by_addr.clone(); + + // Unconditionally add our local listener address to the advertised peers, + // to replace any self-connection failures + let local_listener = self.get_local_listener(); + peers.insert(local_listener.addr, local_listener); + + // Then sanitize and shuffle + let mut peers = peers + .into_values() .filter_map(|a| MetaAddr::sanitize(&a)) .collect::>(); peers.shuffle(&mut rand::thread_rng()); diff --git a/zebrad/src/components/inbound.rs b/zebrad/src/components/inbound.rs index e1e9656174a..e9b341eab91 100644 --- a/zebrad/src/components/inbound.rs +++ b/zebrad/src/components/inbound.rs @@ -240,11 +240,7 @@ impl Service for Inbound { // Briefly hold the address book threaded mutex while // cloning the address book. Then sanitize after releasing // the lock. - let mut peers = address_book.lock().unwrap().clone(); - - // Add our local listener address to the advertised peers - let local_listener = address_book.lock().unwrap().get_local_listener(); - peers.update(local_listener); + let peers = address_book.lock().unwrap().clone(); // Send a sanitized response let mut peers = peers.sanitized(); From 47eaf16f5636cff33d4a6aa643e3bb056249868b Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jun 2021 15:36:58 +1000 Subject: [PATCH 02/17] Skip listeners that are not valid for outbound connections --- zebra-network/src/meta_addr.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index e565261a3bd..49ca2edf254 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -482,11 +482,18 @@ impl MetaAddr { /// /// Returns `None` if this `MetaAddr` should not be sent to remote peers. pub fn sanitize(&self) -> Option { + // Make sure this address is valid for outbound connections + if !self.is_valid_for_outbound() { + return None; + } + + // Sanitize time let interval = crate::constants::TIMESTAMP_TRUNCATION_SECONDS; let ts = self.last_seen()?.timestamp(); // This can't underflow, because `0 <= rem_euclid < ts` let last_seen = ts - ts.rem_euclid(interval); let last_seen = DateTime32::from(last_seen); + Some(MetaAddr { addr: self.addr, // deserialization also sanitizes services to known flags From 188e1ade8f204cf2372dab02b74319a99ec53dd7 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 10 Jun 2021 15:49:21 +1000 Subject: [PATCH 03/17] Stop using unstable methods --- zebra-network/src/address_book.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-network/src/address_book.rs b/zebra-network/src/address_book.rs index d32f1691952..37b0110b29f 100644 --- a/zebra-network/src/address_book.rs +++ b/zebra-network/src/address_book.rs @@ -148,8 +148,8 @@ impl AddressBook { // Then sanitize and shuffle let mut peers = peers - .into_values() - .filter_map(|a| MetaAddr::sanitize(&a)) + .values() + .filter_map(MetaAddr::sanitize) .collect::>(); peers.shuffle(&mut rand::thread_rng()); peers From b5b560743426c9137c7aafa21a997a053cba993a Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 16 Jun 2021 12:39:19 +1000 Subject: [PATCH 04/17] Filter sanitized addresses Zebra based on address state This fix correctly prevents Zebra gossiping client addresses to peers, but still keeps the client in the address book to avoid reconnections. --- zebra-network/src/meta_addr.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index 49ca2edf254..eee4bfdac29 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -482,8 +482,7 @@ impl MetaAddr { /// /// Returns `None` if this `MetaAddr` should not be sent to remote peers. pub fn sanitize(&self) -> Option { - // Make sure this address is valid for outbound connections - if !self.is_valid_for_outbound() { + if !self.last_known_info_is_valid_for_outbound() { return None; } From 1ad55435dc6e52537af95c05e15655a81f6d2801 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 18 Jun 2021 19:22:50 +1000 Subject: [PATCH 05/17] Add a full set of DateTime32 and Duration32 calculation methods --- zebra-chain/src/serialization/date_time.rs | 64 ++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/zebra-chain/src/serialization/date_time.rs b/zebra-chain/src/serialization/date_time.rs index ce01035ae5d..45f5c0c70c2 100644 --- a/zebra-chain/src/serialization/date_time.rs +++ b/zebra-chain/src/serialization/date_time.rs @@ -78,6 +78,38 @@ impl DateTime32 { pub fn saturating_elapsed(&self) -> Duration32 { DateTime32::now().saturating_duration_since(*self) } + + /// Returns the time that is `duration` seconds after this time. + /// If the calculation overflows, returns `None`. + pub fn checked_add(&self, duration: Duration32) -> Option { + self.timestamp + .checked_add(duration.seconds) + .map(|timestamp| DateTime32 { timestamp }) + } + + /// Returns the time that is `duration` seconds after this time. + /// If the calculation overflows, returns `DateTime32::MAX`. + pub fn saturating_add(&self, duration: Duration32) -> DateTime32 { + DateTime32 { + timestamp: self.timestamp.saturating_add(duration.seconds), + } + } + + /// Returns the time that is `duration` seconds before this time. + /// If the calculation underflows, returns `None`. + pub fn checked_sub(&self, duration: Duration32) -> Option { + self.timestamp + .checked_sub(duration.seconds) + .map(|timestamp| DateTime32 { timestamp }) + } + + /// Returns the time that is `duration` seconds before this time. + /// If the calculation underflows, returns `DateTime32::MIN`. + pub fn saturating_sub(&self, duration: Duration32) -> DateTime32 { + DateTime32 { + timestamp: self.timestamp.saturating_sub(duration.seconds), + } + } } impl Duration32 { @@ -101,6 +133,38 @@ impl Duration32 { pub fn to_std(self) -> std::time::Duration { self.into() } + + /// Returns the time that is `duration` seconds after this time. + /// If the calculation overflows, returns `None`. + pub fn checked_add(&self, duration: Duration32) -> Option { + self.seconds + .checked_add(duration.seconds) + .map(|seconds| Duration32 { seconds }) + } + + /// Returns the time that is `duration` seconds after this time. + /// If the calculation overflows, returns `Duration32::MAX`. + pub fn saturating_add(&self, duration: Duration32) -> Duration32 { + Duration32 { + seconds: self.seconds.saturating_add(duration.seconds), + } + } + + /// Returns the time that is `duration` seconds before this time. + /// If the calculation underflows, returns `None`. + pub fn checked_sub(&self, duration: Duration32) -> Option { + self.seconds + .checked_sub(duration.seconds) + .map(|seconds| Duration32 { seconds }) + } + + /// Returns the time that is `duration` seconds before this time. + /// If the calculation underflows, returns `Duration32::MIN`. + pub fn saturating_sub(&self, duration: Duration32) -> Duration32 { + Duration32 { + seconds: self.seconds.saturating_sub(duration.seconds), + } + } } impl fmt::Debug for DateTime32 { From bf08196dee2fb1034deb11c5cb2b48d6c70c3b9e Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 16 Jun 2021 20:46:40 +1000 Subject: [PATCH 06/17] Refactor sanitize to use the new DateTime32/Duration32 methods --- zebra-network/src/meta_addr.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index eee4bfdac29..7f13425160c 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -487,16 +487,19 @@ impl MetaAddr { } // Sanitize time - let interval = crate::constants::TIMESTAMP_TRUNCATION_SECONDS; - let ts = self.last_seen()?.timestamp(); - // This can't underflow, because `0 <= rem_euclid < ts` - let last_seen = ts - ts.rem_euclid(interval); - let last_seen = DateTime32::from(last_seen); + let last_seen = self.last_seen()?; + let remainder = last_seen + .timestamp() + .rem_euclid(crate::constants::TIMESTAMP_TRUNCATION_SECONDS); + let last_seen = last_seen + .checked_sub(remainder.into()) + .expect("unexpected underflow: rem_euclid is strictly less than timestamp"); Some(MetaAddr { addr: self.addr, - // deserialization also sanitizes services to known flags - services: self.services & PeerServices::all(), + // TODO: split untrusted and direct services + // sanitize untrusted services to NODE_NETWORK only? (#2234) + services: self.services, // only put the last seen time in the untrusted field, // this matches deserialization, and avoids leaking internal state untrusted_last_seen: Some(last_seen), From 0582411910b8934846b61b9059e403df8946b6b3 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 16 Jun 2021 20:47:11 +1000 Subject: [PATCH 07/17] Comment fixes and ticket TODO references --- zebra-network/src/meta_addr.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index 7f13425160c..58203be15d9 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -148,7 +148,7 @@ pub struct MetaAddr { /// records, older peer versions, or buggy or malicious peers. // // TODO: make services private and optional - // split gossiped and handshake services? + // split gossiped and handshake services? (#2234) pub services: PeerServices, /// The unverified "last seen time" gossiped by the remote peer that sent us @@ -549,9 +549,10 @@ impl MetaAddrChange { NewAlternate { untrusted_services, .. } => Some(*untrusted_services), - // TODO: create a "services implemented by Zebra" constant + // TODO: create a "services implemented by Zebra" constant (#2234) NewLocal { .. } => Some(PeerServices::NODE_NETWORK), UpdateAttempt { .. } => None, + // TODO: split untrusted and direct services (#2234) UpdateResponded { services, .. } => Some(*services), UpdateFailed { services, .. } => *services, } @@ -639,7 +640,7 @@ impl MetaAddrChange { match self { NewGossiped { .. } | NewAlternate { .. } | NewLocal { .. } => Some(MetaAddr { addr: self.addr(), - // TODO: make services optional when we add a DNS seeder change/state + // TODO: make services optional when we add a DNS seeder change and state services: self .untrusted_services() .expect("unexpected missing services"), @@ -688,7 +689,7 @@ impl MetaAddrChange { // so malicious peers can't keep changing our peer connection order. Some(MetaAddr { addr: self.addr(), - // TODO: or(self.untrusted_services()) when services become optional + // TODO: or(self.untrusted_services()) when services become optional (#2234) services: previous.services, untrusted_last_seen: previous .untrusted_last_seen @@ -711,8 +712,10 @@ impl MetaAddrChange { // connection timeout, even if changes are applied out of order. Some(MetaAddr { addr: self.addr(), + // We want up-to-date services, even if they have fewer bits, + // or they are applied out of order. services: self.untrusted_services().unwrap_or(previous.services), - // only NeverAttempted changes can modify the last seen field + // Only NeverAttempted changes can modify the last seen field untrusted_last_seen: previous.untrusted_last_seen, // Since Some(time) is always greater than None, `max` prefers: // - the latest time if both are Some @@ -790,7 +793,7 @@ impl Ord for MetaAddr { // So this comparison will have no impact until Zebra implements // more service features. // - // TODO: order services by usefulness, not bit pattern values + // TODO: order services by usefulness, not bit pattern values (#2234) // Security: split gossiped and direct services let larger_services = self.services.cmp(&other.services); From e290a428e8eba2667913cfef5922441565ed4540 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 21 Jun 2021 11:02:46 +1000 Subject: [PATCH 08/17] Security: Use canonical SocketAddrs to avoid duplicate connections If we allow multiple variants for each peer address, we can make multiple connections to that peer. Also make sure sanitized MetaAddrs are valid for outbound connections. --- zebra-chain/src/serialization.rs | 2 +- zebra-chain/src/serialization/arbitrary.rs | 10 ++-- zebra-chain/src/serialization/read_zcash.rs | 17 +++++++ zebra-network/src/address_book.rs | 15 +++++- zebra-network/src/config.rs | 6 +-- zebra-network/src/meta_addr.rs | 54 ++++++++++++--------- zebra-network/src/meta_addr/arbitrary.rs | 8 +-- zebra-network/src/meta_addr/tests/check.rs | 24 +++++++-- zebra-network/src/meta_addr/tests/prop.rs | 13 ++++- 9 files changed, 105 insertions(+), 44 deletions(-) diff --git a/zebra-chain/src/serialization.rs b/zebra-chain/src/serialization.rs index f383a064183..e11bcaed2bc 100644 --- a/zebra-chain/src/serialization.rs +++ b/zebra-chain/src/serialization.rs @@ -24,7 +24,7 @@ pub mod arbitrary; pub use constraint::AtLeastOne; pub use date_time::DateTime32; pub use error::SerializationError; -pub use read_zcash::ReadZcashExt; +pub use read_zcash::{canonical_socket_addr, ReadZcashExt}; pub use write_zcash::WriteZcashExt; pub use zcash_deserialize::{ zcash_deserialize_bytes_external_count, zcash_deserialize_external_count, TrustedPreallocate, diff --git a/zebra-chain/src/serialization/arbitrary.rs b/zebra-chain/src/serialization/arbitrary.rs index 6cd6af92c62..8e396284996 100644 --- a/zebra-chain/src/serialization/arbitrary.rs +++ b/zebra-chain/src/serialization/arbitrary.rs @@ -1,6 +1,6 @@ //! Arbitrary data generation for serialization proptests -use super::{read_zcash::canonical_ip_addr, DateTime32}; +use super::{read_zcash::canonical_socket_addr, DateTime32}; use chrono::{TimeZone, Utc, MAX_DATETIME, MIN_DATETIME}; use proptest::{arbitrary::any, prelude::*}; use std::net::SocketAddr; @@ -59,10 +59,6 @@ pub fn datetime_u32() -> impl Strategy> { /// Returns a random canonical Zebra `SocketAddr`. /// /// See [`canonical_ip_addr`] for details. -pub fn canonical_socket_addr() -> impl Strategy { - use SocketAddr::*; - any::().prop_map(|addr| match addr { - V4(_) => addr, - V6(v6_addr) => SocketAddr::new(canonical_ip_addr(v6_addr.ip()), v6_addr.port()), - }) +pub fn canonical_socket_addr_strategy() -> impl Strategy { + any::().prop_map(canonical_socket_addr) } diff --git a/zebra-chain/src/serialization/read_zcash.rs b/zebra-chain/src/serialization/read_zcash.rs index 8f67730e74c..23ba352c024 100644 --- a/zebra-chain/src/serialization/read_zcash.rs +++ b/zebra-chain/src/serialization/read_zcash.rs @@ -162,3 +162,20 @@ pub fn canonical_ip_addr(v6_addr: &Ipv6Addr) -> IpAddr { 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 { + 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 +} diff --git a/zebra-network/src/address_book.rs b/zebra-network/src/address_book.rs index 37b0110b29f..f3c38789060 100644 --- a/zebra-network/src/address_book.rs +++ b/zebra-network/src/address_book.rs @@ -10,6 +10,8 @@ use std::{ use tracing::Span; +use zebra_chain::serialization::canonical_socket_addr; + use crate::{meta_addr::MetaAddrChange, types::MetaAddr, Config, PeerAddrState}; /// A database of peer listener addresses, their advertised services, and @@ -94,7 +96,7 @@ impl AddressBook { let mut new_book = AddressBook { by_addr: HashMap::default(), - local_listener: config.listen_addr, + local_listener: canonical_socket_addr(config.listen_addr), span, last_address_log: None, }; @@ -118,6 +120,11 @@ impl AddressBook { let addrs = addrs .into_iter() + .map(|mut meta_addr| { + meta_addr.addr = canonical_socket_addr(meta_addr.addr); + meta_addr + }) + .filter(MetaAddr::address_is_valid_for_outbound) .map(|meta_addr| (meta_addr.addr, meta_addr)); new_book.by_addr.extend(addrs); @@ -142,7 +149,8 @@ impl AddressBook { let mut peers = self.by_addr.clone(); // Unconditionally add our local listener address to the advertised peers, - // to replace any self-connection failures + // to replace any self-connection failures. The address book and change + // constructors make sure that the SocketAddr is canonical. let local_listener = self.get_local_listener(); peers.insert(local_listener.addr, local_listener); @@ -175,6 +183,9 @@ impl AddressBook { /// All changes should go through `update`, so that the address book /// only contains valid outbound addresses. /// + /// Change addresses must be canonical `SocketAddr`s. This makes sure that + /// each address book entry has a unique IP address. + /// /// # Security /// /// This function must apply every attempted, responded, and failed change diff --git a/zebra-network/src/config.rs b/zebra-network/src/config.rs index 3c40801b2b3..e81d9c4d7c9 100644 --- a/zebra-network/src/config.rs +++ b/zebra-network/src/config.rs @@ -7,7 +7,7 @@ use std::{ use serde::{de, Deserialize, Deserializer}; -use zebra_chain::parameters::Network; +use zebra_chain::{parameters::Network, serialization::canonical_socket_addr}; use crate::BoxError; @@ -131,7 +131,7 @@ impl Config { let fut = tokio::time::timeout(crate::constants::DNS_LOOKUP_TIMEOUT, fut); match fut.await { - Ok(Ok(ips)) => Ok(ips.collect()), + Ok(Ok(ips)) => Ok(ips.map(canonical_socket_addr).collect()), Ok(Err(e)) => { tracing::info!(?host, ?e, "DNS error resolving peer IP address"); Err(e.into()) @@ -237,7 +237,7 @@ impl<'de> Deserialize<'de> for Config { }?; Ok(Config { - listen_addr, + listen_addr: canonical_socket_addr(listen_addr), network: config.network, initial_mainnet_peers: config.initial_mainnet_peers, initial_testnet_peers: config.initial_testnet_peers, diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index 58203be15d9..74439a0302e 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -11,8 +11,8 @@ use std::{ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use zebra_chain::serialization::{ - DateTime32, ReadZcashExt, SerializationError, TrustedPreallocate, WriteZcashExt, - ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize, + canonical_socket_addr, DateTime32, ReadZcashExt, SerializationError, TrustedPreallocate, + WriteZcashExt, ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize, }; use crate::{ @@ -26,7 +26,7 @@ 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; +use zebra_chain::serialization::arbitrary::canonical_socket_addr_strategy; #[cfg(any(test, feature = "proptest-impl"))] mod arbitrary; @@ -125,12 +125,15 @@ impl PartialOrd for PeerAddrState { #[derive(Copy, Clone, Debug)] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] pub struct MetaAddr { - /// The peer's address. + /// The peer's canonical socket address. #[cfg_attr( any(test, feature = "proptest-impl"), - proptest(strategy = "canonical_socket_addr()") + proptest(strategy = "canonical_socket_addr_strategy()") )] - pub addr: SocketAddr, + // + // TODO: make addr private, so the constructors can make sure it is a + // canonical SocketAddr (#2357) + pub(crate) addr: SocketAddr, /// The services advertised by the peer. /// @@ -149,7 +152,7 @@ pub struct MetaAddr { // // TODO: make services private and optional // split gossiped and handshake services? (#2234) - pub services: PeerServices, + pub(crate) services: PeerServices, /// The unverified "last seen time" gossiped by the remote peer that sent us /// this address. @@ -173,10 +176,11 @@ pub struct MetaAddr { last_failure: Option, /// The outcome of our most recent communication attempt with this peer. - pub last_connection_state: PeerAddrState, // - // TODO: move the time and services fields into PeerAddrState? + // TODO: make services private and optional? + // move the time and services fields into PeerAddrState? // then some fields could be required in some states + pub(crate) last_connection_state: PeerAddrState, } /// A change to an existing `MetaAddr`. @@ -187,7 +191,7 @@ pub enum MetaAddrChange { NewGossiped { #[cfg_attr( any(test, feature = "proptest-impl"), - proptest(strategy = "canonical_socket_addr()") + proptest(strategy = "canonical_socket_addr_strategy()") )] addr: SocketAddr, untrusted_services: PeerServices, @@ -200,7 +204,7 @@ pub enum MetaAddrChange { NewAlternate { #[cfg_attr( any(test, feature = "proptest-impl"), - proptest(strategy = "canonical_socket_addr()") + proptest(strategy = "canonical_socket_addr_strategy()") )] addr: SocketAddr, untrusted_services: PeerServices, @@ -210,7 +214,7 @@ pub enum MetaAddrChange { NewLocal { #[cfg_attr( any(test, feature = "proptest-impl"), - proptest(strategy = "canonical_socket_addr()") + proptest(strategy = "canonical_socket_addr_strategy()") )] addr: SocketAddr, }, @@ -220,7 +224,7 @@ pub enum MetaAddrChange { UpdateAttempt { #[cfg_attr( any(test, feature = "proptest-impl"), - proptest(strategy = "canonical_socket_addr()") + proptest(strategy = "canonical_socket_addr_strategy()") )] addr: SocketAddr, }, @@ -229,7 +233,7 @@ pub enum MetaAddrChange { UpdateResponded { #[cfg_attr( any(test, feature = "proptest-impl"), - proptest(strategy = "canonical_socket_addr()") + proptest(strategy = "canonical_socket_addr_strategy()") )] addr: SocketAddr, services: PeerServices, @@ -239,7 +243,7 @@ pub enum MetaAddrChange { UpdateFailed { #[cfg_attr( any(test, feature = "proptest-impl"), - proptest(strategy = "canonical_socket_addr()") + proptest(strategy = "canonical_socket_addr_strategy()") )] addr: SocketAddr, services: Option, @@ -255,7 +259,7 @@ impl MetaAddr { untrusted_last_seen: DateTime32, ) -> MetaAddr { MetaAddr { - addr, + addr: canonical_socket_addr(addr), services: untrusted_services, untrusted_last_seen: Some(untrusted_last_seen), last_response: None, @@ -269,7 +273,7 @@ impl MetaAddr { /// `MetaAddr`. pub fn new_gossiped_change(self) -> MetaAddrChange { NewGossiped { - addr: self.addr, + addr: canonical_socket_addr(self.addr), untrusted_services: self.services, untrusted_last_seen: self .untrusted_last_seen @@ -291,7 +295,7 @@ impl MetaAddr { /// - Zebra could advertise unreachable addresses to its own peers. pub fn new_responded(addr: &SocketAddr, services: &PeerServices) -> MetaAddrChange { UpdateResponded { - addr: *addr, + addr: canonical_socket_addr(*addr), services: *services, } } @@ -299,21 +303,25 @@ impl MetaAddr { /// Returns a [`MetaAddrChange::UpdateAttempt`] for a peer that we /// want to make an outbound connection to. pub fn new_reconnect(addr: &SocketAddr) -> MetaAddrChange { - UpdateAttempt { addr: *addr } + UpdateAttempt { + addr: canonical_socket_addr(*addr), + } } /// Returns a [`MetaAddrChange::NewAlternate`] for a peer's alternate address, /// received via a `Version` message. pub fn new_alternate(addr: &SocketAddr, untrusted_services: &PeerServices) -> MetaAddrChange { NewAlternate { - addr: *addr, + addr: canonical_socket_addr(*addr), untrusted_services: *untrusted_services, } } /// Returns a [`MetaAddrChange::NewLocal`] for our own listener address. pub fn new_local_listener(addr: &SocketAddr) -> MetaAddrChange { - NewLocal { addr: *addr } + NewLocal { + addr: canonical_socket_addr(*addr), + } } /// Returns a [`MetaAddrChange::UpdateFailed`] for a peer that has just had @@ -323,7 +331,7 @@ impl MetaAddr { services: impl Into>, ) -> MetaAddrChange { UpdateFailed { - addr: *addr, + addr: canonical_socket_addr(*addr), services: services.into(), } } @@ -496,7 +504,7 @@ impl MetaAddr { .expect("unexpected underflow: rem_euclid is strictly less than timestamp"); Some(MetaAddr { - addr: self.addr, + addr: canonical_socket_addr(self.addr), // TODO: split untrusted and direct services // sanitize untrusted services to NODE_NETWORK only? (#2234) services: self.services, diff --git a/zebra-network/src/meta_addr/arbitrary.rs b/zebra-network/src/meta_addr/arbitrary.rs index 90f6a9aaee2..1604e4332b4 100644 --- a/zebra-network/src/meta_addr/arbitrary.rs +++ b/zebra-network/src/meta_addr/arbitrary.rs @@ -4,7 +4,7 @@ use proptest::{arbitrary::any, collection::vec, prelude::*}; use super::{MetaAddr, MetaAddrChange, PeerServices}; -use zebra_chain::serialization::{arbitrary::canonical_socket_addr, DateTime32}; +use zebra_chain::serialization::{arbitrary::canonical_socket_addr_strategy, DateTime32}; /// The largest number of random changes we want to apply to a MetaAddr /// @@ -17,7 +17,7 @@ impl MetaAddr { /// [`PeerAddrState::NeverAttemptedGossiped`] state. pub fn gossiped_strategy() -> BoxedStrategy { ( - canonical_socket_addr(), + canonical_socket_addr_strategy(), any::(), any::(), ) @@ -30,7 +30,7 @@ impl MetaAddr { /// Create a strategy that generates [`MetaAddr`]s in the /// [`PeerAddrState::NeverAttemptedAlternate`] state. pub fn alternate_strategy() -> BoxedStrategy { - (canonical_socket_addr(), any::()) + (canonical_socket_addr_strategy(), any::()) .prop_map(|(socket_addr, untrusted_services)| { MetaAddr::new_alternate(&socket_addr, &untrusted_services) .into_new_meta_addr() @@ -78,7 +78,7 @@ impl MetaAddrChange { /// TODO: Generate all [`MetaAddrChange`] variants, and give them ready fields. /// (After PR #2276 merges.) pub fn ready_outbound_strategy() -> BoxedStrategy { - canonical_socket_addr() + canonical_socket_addr_strategy() .prop_filter_map("failed MetaAddr::is_valid_for_outbound", |addr| { // Alternate nodes use the current time, so they're always ready // diff --git a/zebra-network/src/meta_addr/tests/check.rs b/zebra-network/src/meta_addr/tests/check.rs index e90180ba273..565443d922d 100644 --- a/zebra-network/src/meta_addr/tests/check.rs +++ b/zebra-network/src/meta_addr/tests/check.rs @@ -1,5 +1,7 @@ //! Shared test checks for MetaAddr +use std::net::SocketAddr; + use super::super::MetaAddr; use crate::{constants::TIMESTAMP_TRUNCATION_SECONDS, types::PeerServices}; @@ -68,8 +70,24 @@ pub(crate) fn sanitize_avoids_leaks(original: &MetaAddr, sanitized: &MetaAddr) { // Sanitize to the the default state, even though it's not serialized assert_eq!(sanitized.last_connection_state, Default::default()); // Sanitize to known flags - assert_eq!(sanitized.services, original.services & PeerServices::all()); + let sanitized_peer_services = original.services & PeerServices::all(); + assert_eq!(sanitized.services, sanitized_peer_services); + + // Remove IPv6 scope ID and flow information + let sanitized_socket_addr = SocketAddr::new(original.addr.ip(), original.addr.port()); + assert_eq!(sanitized.addr.ip(), original.addr.ip()); + assert_eq!(sanitized.addr.port(), original.addr.port()); + assert_eq!(sanitized.addr, sanitized_socket_addr); - // We want the other fields to be unmodified - assert_eq!(sanitized.addr, original.addr); + // We want any other fields to be their defaults + assert_eq!( + sanitized, + &MetaAddr::new_gossiped_meta_addr( + sanitized_socket_addr, + sanitized_peer_services, + sanitized + .last_seen() + .expect("unexpected missing last seen time in sanitized MetaAddr") + ), + ); } diff --git a/zebra-network/src/meta_addr/tests/prop.rs b/zebra-network/src/meta_addr/tests/prop.rs index efc57d1150d..b36eec47260 100644 --- a/zebra-network/src/meta_addr/tests/prop.rs +++ b/zebra-network/src/meta_addr/tests/prop.rs @@ -21,6 +21,7 @@ use crate::{ constants::LIVE_PEER_DURATION, meta_addr::{arbitrary::MAX_ADDR_CHANGE, MetaAddr, MetaAddrChange, PeerAddrState::*}, peer_set::candidate_set::CandidateSet, + protocol::types::PeerServices, AddressBook, Config, }; @@ -32,12 +33,22 @@ const DEFAULT_VERBOSE_TEST_PROPTEST_CASES: u32 = 256; proptest! { /// Make sure that the sanitize function reduces time and state metadata - /// leaks. + /// leaks for valid addresses. + /// + /// Make sure that the sanitize function skips invalid IP addresses, ports, + /// and client services. #[test] fn sanitize_avoids_leaks(addr in MetaAddr::arbitrary()) { zebra_test::init(); if let Some(sanitized) = addr.sanitize() { + // check that all sanitized addresses are valid for outbound + prop_assert!(addr.last_known_info_is_valid_for_outbound()); + // also check the address, port, and services individually + prop_assert!(!addr.addr.ip().is_unspecified()); + prop_assert_ne!(addr.addr.port(), 0); + prop_assert!(addr.services.contains(PeerServices::NODE_NETWORK)); + check::sanitize_avoids_leaks(&addr, &sanitized); } } From 4aa6b00b994c3e945f9daaa66575dcfa84d06e06 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 21 Jun 2021 13:08:19 +1000 Subject: [PATCH 09/17] Test that address books contain the local listener address --- zebra-network/src/meta_addr.rs | 3 +- zebra-network/src/meta_addr/arbitrary.rs | 12 +++++-- zebra-network/src/meta_addr/tests/prop.rs | 42 +++++++++++++++++++++-- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index 74439a0302e..dfb5167c315 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -574,7 +574,8 @@ impl MetaAddrChange { .. } => Some(*untrusted_last_seen), NewAlternate { .. } => None, - NewLocal { .. } => None, + // We know that our local listener is available + NewLocal { .. } => Some(DateTime32::now()), UpdateAttempt { .. } => None, UpdateResponded { .. } => None, UpdateFailed { .. } => None, diff --git a/zebra-network/src/meta_addr/arbitrary.rs b/zebra-network/src/meta_addr/arbitrary.rs index 1604e4332b4..82a10de863e 100644 --- a/zebra-network/src/meta_addr/arbitrary.rs +++ b/zebra-network/src/meta_addr/arbitrary.rs @@ -6,12 +6,18 @@ use super::{MetaAddr, MetaAddrChange, PeerServices}; use zebra_chain::serialization::{arbitrary::canonical_socket_addr_strategy, DateTime32}; -/// The largest number of random changes we want to apply to a MetaAddr +/// The largest number of random changes we want to apply to a [`MetaAddr`]. /// -/// This should be at least twice the number of [`PeerAddrState`]s, so -/// the tests can cover multiple transitions through every state. +/// This should be at least twice the number of [`PeerAddrState`]s, so the tests +/// can cover multiple transitions through every state. pub const MAX_ADDR_CHANGE: usize = 15; +/// The largest number of random addresses we want to add to an [`AddressBook`]. +/// +/// This should be at least the number of [`PeerAddrState`]s, so the tests can +/// cover interactions between addresses in different states. +pub const MAX_META_ADDR: usize = 8; + impl MetaAddr { /// Create a strategy that generates [`MetaAddr`]s in the /// [`PeerAddrState::NeverAttemptedGossiped`] state. diff --git a/zebra-network/src/meta_addr/tests/prop.rs b/zebra-network/src/meta_addr/tests/prop.rs index b36eec47260..b07d9625869 100644 --- a/zebra-network/src/meta_addr/tests/prop.rs +++ b/zebra-network/src/meta_addr/tests/prop.rs @@ -14,12 +14,16 @@ use tokio::{runtime::Runtime, time::Instant}; use tower::service_fn; use tracing::Span; -use zebra_chain::serialization::{ZcashDeserialize, ZcashSerialize}; +use zebra_chain::serialization::{canonical_socket_addr, ZcashDeserialize, ZcashSerialize}; use super::check; use crate::{ constants::LIVE_PEER_DURATION, - meta_addr::{arbitrary::MAX_ADDR_CHANGE, MetaAddr, MetaAddrChange, PeerAddrState::*}, + meta_addr::{ + arbitrary::{MAX_ADDR_CHANGE, MAX_META_ADDR}, + MetaAddr, MetaAddrChange, + PeerAddrState::*, + }, peer_set::candidate_set::CandidateSet, protocol::types::PeerServices, AddressBook, Config, @@ -256,6 +260,40 @@ proptest! { } } } + + /// Make sure that a sanitized [`AddressBook`] contains the local listener + /// [`MetaAddr`], regardless of the previous contents of the address book. + /// + /// If Zebra gossips its own listener address to peers, and gets it back, + /// its address book will contain its local listener address. This address + /// will likely be in [`PeerAddrState::Failed`], due to failed + /// self-connection attempts. + #[test] + fn sanitized_address_book_contains_local_listener( + local_listener in any::(), + address_book_addrs in vec(any::(), 0..MAX_META_ADDR), + ) { + zebra_test::init(); + + let config = Config { listen_addr: local_listener, ..Config::default() }; + let address_book = AddressBook::new_with_addrs(&config, Span::none(), address_book_addrs); + let sanitized_addrs = address_book.sanitized(); + + let expected_local_listener = address_book.get_local_listener(); + let canonical_local_listener = canonical_socket_addr(local_listener); + let book_sanitized_local_listener = sanitized_addrs.iter().find(|meta_addr| meta_addr.addr == canonical_local_listener ); + + // invalid addresses should be removed by sanitization, + // regardless of where they have come from + prop_assert_eq!( + book_sanitized_local_listener.cloned(), + expected_local_listener.sanitize(), + "address book: {:?}, sanitized_addrs: {:?}, canonical_local_listener: {:?}", + address_book, + sanitized_addrs, + canonical_local_listener, + ); + } } proptest! { From b999b09342533a3336cbbfa66bf022dffc5b4f0a Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 15 Jun 2021 19:12:29 +1000 Subject: [PATCH 10/17] Remove unused code --- zebra-network/src/address_book.rs | 34 +------------------------------ 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/zebra-network/src/address_book.rs b/zebra-network/src/address_book.rs index f3c38789060..c6a619a2b40 100644 --- a/zebra-network/src/address_book.rs +++ b/zebra-network/src/address_book.rs @@ -163,18 +163,6 @@ impl AddressBook { peers } - /// Returns true if the address book has an entry for `addr`. - pub fn contains_addr(&self, addr: &SocketAddr) -> bool { - let _guard = self.span.enter(); - self.by_addr.contains_key(addr) - } - - /// Returns the entry corresponding to `addr`, or `None` if it does not exist. - pub fn get_by_addr(&self, addr: SocketAddr) -> Option { - let _guard = self.span.enter(); - self.by_addr.get(&addr).cloned() - } - /// Apply `change` to the address book, returning the updated `MetaAddr`, /// if the change was valid. /// @@ -242,6 +230,7 @@ impl AddressBook { /// /// All address removals should go through `take`, so that the address /// book metrics are accurate. + #[allow(dead_code)] fn take(&mut self, removed_addr: SocketAddr) -> Option { let _guard = self.span.enter(); trace!( @@ -338,14 +327,6 @@ impl AddressBook { .cloned() } - /// Returns an iterator that drains entries from the address book. - /// - /// Removes entries in reconnection attempt then arbitrary order, - /// see [`peers`] for details. - pub fn drain(&'_ mut self) -> impl Iterator + '_ { - Drain { book: self } - } - /// Returns the number of entries in this address book. pub fn len(&self) -> usize { self.by_addr.len() @@ -466,16 +447,3 @@ impl Extend for AddressBook { } } } - -struct Drain<'a> { - book: &'a mut AddressBook, -} - -impl<'a> Iterator for Drain<'a> { - type Item = MetaAddr; - - fn next(&mut self) -> Option { - let next_item_addr = self.book.peers().next()?.addr; - self.book.take(next_item_addr) - } -} From 4ce3563dfadfa1d582757eb16768ad41a2dd383b Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 16 Jun 2021 20:47:11 +1000 Subject: [PATCH 11/17] Fix a comment in add_initial_peers --- zebra-network/src/peer_set/initialize.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/zebra-network/src/peer_set/initialize.rs b/zebra-network/src/peer_set/initialize.rs index 26e9190ebd2..f5fd28af32f 100644 --- a/zebra-network/src/peer_set/initialize.rs +++ b/zebra-network/src/peer_set/initialize.rs @@ -201,12 +201,16 @@ where S::Future: Send + 'static, { info!(?initial_peers, "connecting to initial peer set"); - // ## Correctness: + // # Security // - // Each `CallAll` can hold one `Buffer` or `Batch` reservation for - // an indefinite period. We can use `CallAllUnordered` without filling - // the underlying `Inbound` buffer, because we immediately drive this - // single `CallAll` to completion, and handshakes have a short timeout. + // TODO: rate-limit initial seed peer connections (#2326) + // + // # Correctness + // + // Each `FuturesUnordered` can hold one `Buffer` or `Batch` reservation for + // an indefinite period. We can use `FuturesUnordered` without filling + // the underlying network buffers, because we immediately drive this + // single `FuturesUnordered` to completion, and handshakes have a short timeout. let mut handshakes: FuturesUnordered<_> = initial_peers .into_iter() .map(|addr| { From 761ff64569d42fef109e0f90094c55c7c3dc7e56 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 18 Jun 2021 19:22:50 +1000 Subject: [PATCH 12/17] Refactor CandidateSet to use DateTime32 and Duration32 --- zebra-network/src/peer_set/candidate_set.rs | 39 ++++++++++----------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/zebra-network/src/peer_set/candidate_set.rs b/zebra-network/src/peer_set/candidate_set.rs index cc4ac3ff47f..0e5cdfd4c84 100644 --- a/zebra-network/src/peer_set/candidate_set.rs +++ b/zebra-network/src/peer_set/candidate_set.rs @@ -379,33 +379,32 @@ fn validate_addrs( /// This will consider all addresses as invalid if trying to offset their /// `last_seen` times to be before the limit causes an underflow. fn limit_last_seen_times(addrs: &mut Vec, last_seen_limit: DateTime32) { - let (oldest_reported_seen_timestamp, newest_reported_seen_timestamp) = - addrs - .iter() - .fold((u32::MAX, u32::MIN), |(oldest, newest), addr| { - let last_seen = addr - .untrusted_last_seen() - .expect("unexpected missing last seen") - .timestamp(); - (oldest.min(last_seen), newest.max(last_seen)) - }); + let last_seen_times = addrs.iter().map(|meta_addr| { + meta_addr + .untrusted_last_seen() + .expect("unexpected missing last seen: should be provided by deserialization") + }); + let oldest_seen = last_seen_times.clone().min().unwrap_or(DateTime32::MIN); + let newest_seen = last_seen_times.max().unwrap_or(DateTime32::MAX); // If any time is in the future, adjust all times, to compensate for clock skew on honest peers - if newest_reported_seen_timestamp > last_seen_limit.timestamp() { - let offset = newest_reported_seen_timestamp - last_seen_limit.timestamp(); + if newest_seen > last_seen_limit { + let offset = newest_seen + .checked_duration_since(last_seen_limit) + .expect("unexpected underflow: just checked newest_seen is greater"); - // Apply offset to oldest timestamp to check for underflow - let oldest_resulting_timestamp = oldest_reported_seen_timestamp as i64 - offset as i64; - if oldest_resulting_timestamp >= 0 { + // Check for underflow + if oldest_seen.checked_sub(offset).is_some() { // No underflow is possible, so apply offset to all addresses for addr in addrs { - let old_last_seen = addr + let last_seen = addr .untrusted_last_seen() - .expect("unexpected missing last seen") - .timestamp(); - let new_last_seen = old_last_seen - offset; + .expect("unexpected missing last seen: should be provided by deserialization"); + let last_seen = last_seen + .checked_sub(offset) + .expect("unexpected underflow: just checked oldest_seen"); - addr.set_untrusted_last_seen(new_last_seen.into()); + addr.set_untrusted_last_seen(last_seen); } } else { // An underflow will occur, so reject all gossiped peers From eddc7e711c15352c450e82921a7ab3a4ec472fdc Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 22 Jun 2021 11:16:05 +1000 Subject: [PATCH 13/17] Fix method comments Co-authored-by: Janito Vaqueiro Ferreira Filho --- zebra-chain/src/serialization/date_time.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zebra-chain/src/serialization/date_time.rs b/zebra-chain/src/serialization/date_time.rs index 45f5c0c70c2..b9c92d4b1d5 100644 --- a/zebra-chain/src/serialization/date_time.rs +++ b/zebra-chain/src/serialization/date_time.rs @@ -134,7 +134,7 @@ impl Duration32 { self.into() } - /// Returns the time that is `duration` seconds after this time. + /// Returns a duration that is `duration` longer than this duration. /// If the calculation overflows, returns `None`. pub fn checked_add(&self, duration: Duration32) -> Option { self.seconds @@ -142,7 +142,7 @@ impl Duration32 { .map(|seconds| Duration32 { seconds }) } - /// Returns the time that is `duration` seconds after this time. + /// Returns a duration that is `duration` longer than this duration. /// If the calculation overflows, returns `Duration32::MAX`. pub fn saturating_add(&self, duration: Duration32) -> Duration32 { Duration32 { @@ -150,7 +150,7 @@ impl Duration32 { } } - /// Returns the time that is `duration` seconds before this time. + /// Returns a duration that is `duration` shorter than this duration. /// If the calculation underflows, returns `None`. pub fn checked_sub(&self, duration: Duration32) -> Option { self.seconds @@ -158,7 +158,7 @@ impl Duration32 { .map(|seconds| Duration32 { seconds }) } - /// Returns the time that is `duration` seconds before this time. + /// Returns a duration that is `duration` shorter than this duration. /// If the calculation underflows, returns `Duration32::MIN`. pub fn saturating_sub(&self, duration: Duration32) -> Duration32 { Duration32 { From 7e88c3a153382b63ded7867222a5cf2044677b18 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 22 Jun 2021 11:18:11 +1000 Subject: [PATCH 14/17] Refactor checked_add Co-authored-by: Janito Vaqueiro Ferreira Filho --- zebra-chain/src/serialization/date_time.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-chain/src/serialization/date_time.rs b/zebra-chain/src/serialization/date_time.rs index b9c92d4b1d5..34f72bb0fe3 100644 --- a/zebra-chain/src/serialization/date_time.rs +++ b/zebra-chain/src/serialization/date_time.rs @@ -84,7 +84,7 @@ impl DateTime32 { pub fn checked_add(&self, duration: Duration32) -> Option { self.timestamp .checked_add(duration.seconds) - .map(|timestamp| DateTime32 { timestamp }) + .map(DateTime32::from) } /// Returns the time that is `duration` seconds after this time. From 19f1b299752718cd07864b7cbd71a340f422de95 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 22 Jun 2021 11:22:57 +1000 Subject: [PATCH 15/17] Clarify method comments --- zebra-chain/src/serialization/date_time.rs | 36 ++++++++++++---------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/zebra-chain/src/serialization/date_time.rs b/zebra-chain/src/serialization/date_time.rs index 34f72bb0fe3..415ab9e1614 100644 --- a/zebra-chain/src/serialization/date_time.rs +++ b/zebra-chain/src/serialization/date_time.rs @@ -44,14 +44,18 @@ impl DateTime32 { self.into() } - /// Returns the current time + /// Returns the current time. + /// + /// # Panics + /// + /// If the number of seconds since the UNIX epoch is greater than `u32::MAX`. pub fn now() -> DateTime32 { Utc::now() .try_into() .expect("unexpected out of range chrono::DateTime") } - /// Returns the number of seconds elapsed between `earlier` and this time, + /// Returns the duration elapsed between `earlier` and this time, /// or `None` if `earlier` is later than this time. pub fn checked_duration_since(&self, earlier: DateTime32) -> Option { self.timestamp @@ -59,7 +63,7 @@ impl DateTime32 { .map(|seconds| Duration32 { seconds }) } - /// Returns the number of seconds elapsed between `earlier` and this time, + /// Returns duration elapsed between `earlier` and this time, /// or zero if `earlier` is later than this time. pub fn saturating_duration_since(&self, earlier: DateTime32) -> Duration32 { Duration32 { @@ -67,19 +71,19 @@ impl DateTime32 { } } - /// Returns the number of seconds elapsed since this time, + /// Returns the duration elapsed since this time, /// or if this time is in the future, returns `None`. pub fn checked_elapsed(&self) -> Option { DateTime32::now().checked_duration_since(*self) } - /// Returns the number of seconds elapsed since this time, + /// Returns the duration elapsed since this time, /// or if this time is in the future, returns zero. pub fn saturating_elapsed(&self) -> Duration32 { DateTime32::now().saturating_duration_since(*self) } - /// Returns the time that is `duration` seconds after this time. + /// Returns the time that is `duration` after this time. /// If the calculation overflows, returns `None`. pub fn checked_add(&self, duration: Duration32) -> Option { self.timestamp @@ -87,7 +91,7 @@ impl DateTime32 { .map(DateTime32::from) } - /// Returns the time that is `duration` seconds after this time. + /// Returns the time that is `duration` after this time. /// If the calculation overflows, returns `DateTime32::MAX`. pub fn saturating_add(&self, duration: Duration32) -> DateTime32 { DateTime32 { @@ -95,7 +99,7 @@ impl DateTime32 { } } - /// Returns the time that is `duration` seconds before this time. + /// Returns the time that is `duration` before this time. /// If the calculation underflows, returns `None`. pub fn checked_sub(&self, duration: Duration32) -> Option { self.timestamp @@ -103,7 +107,7 @@ impl DateTime32 { .map(|timestamp| DateTime32 { timestamp }) } - /// Returns the time that is `duration` seconds before this time. + /// Returns the time that is `duration` before this time. /// If the calculation underflows, returns `DateTime32::MIN`. pub fn saturating_sub(&self, duration: Duration32) -> DateTime32 { DateTime32 { @@ -119,7 +123,7 @@ impl Duration32 { /// The latest possible `Duration32` value. pub const MAX: Duration32 = Duration32 { seconds: u32::MAX }; - /// Returns the number of seconds. + /// Returns the number of seconds in this duration. pub fn seconds(&self) -> u32 { self.seconds } @@ -253,7 +257,7 @@ impl TryFrom> for DateTime32 { /// Convert from a [`chrono::DateTime`] to a [`DateTime32`], discarding any nanoseconds. /// - /// Conversion fails if the number of seconds is outside the `u32` range. + /// Conversion fails if the number of seconds since the UNIX epoch is outside the `u32` range. fn try_from(value: chrono::DateTime) -> Result { Ok(Self { timestamp: value.timestamp().try_into()?, @@ -266,7 +270,7 @@ impl TryFrom<&chrono::DateTime> for DateTime32 { /// Convert from a [`chrono::DateTime`] to a [`DateTime32`], discarding any nanoseconds. /// - /// Conversion fails if the number of seconds is outside the `u32` range. + /// Conversion fails if the number of seconds since the UNIX epoch is outside the `u32` range. fn try_from(value: &chrono::DateTime) -> Result { (*value).try_into() } @@ -277,7 +281,7 @@ impl TryFrom for Duration32 { /// Convert from a [`chrono::Duration`] to a [`Duration32`], discarding any nanoseconds. /// - /// Conversion fails if the number of seconds is outside the `u32` range. + /// Conversion fails if the number of seconds since the UNIX epoch is outside the `u32` range. fn try_from(value: chrono::Duration) -> Result { Ok(Self { seconds: value.num_seconds().try_into()?, @@ -290,7 +294,7 @@ impl TryFrom<&chrono::Duration> for Duration32 { /// Convert from a [`chrono::Duration`] to a [`Duration32`], discarding any nanoseconds. /// - /// Conversion fails if the number of seconds is outside the `u32` range. + /// Conversion fails if the number of seconds in the duration is outside the `u32` range. fn try_from(value: &chrono::Duration) -> Result { (*value).try_into() } @@ -301,7 +305,7 @@ impl TryFrom for Duration32 { /// Convert from a [`std::time::Duration`] to a [`Duration32`], discarding any nanoseconds. /// - /// Conversion fails if the number of seconds is outside the `u32` range. + /// Conversion fails if the number of seconds in the duration is outside the `u32` range. fn try_from(value: std::time::Duration) -> Result { Ok(Self { seconds: value.as_secs().try_into()?, @@ -314,7 +318,7 @@ impl TryFrom<&std::time::Duration> for Duration32 { /// Convert from a [`std::time::Duration`] to a [`Duration32`], discarding any nanoseconds. /// - /// Conversion fails if the number of seconds is outside the `u32` range. + /// Conversion fails if the number of seconds in the duration is outside the `u32` range. fn try_from(value: &std::time::Duration) -> Result { (*value).try_into() } From 015e090bce07fc91af7819e60d38f46546588df0 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 22 Jun 2021 11:29:35 +1000 Subject: [PATCH 16/17] Simplify some conversions --- zebra-chain/src/serialization/date_time.rs | 28 +++++++--------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/zebra-chain/src/serialization/date_time.rs b/zebra-chain/src/serialization/date_time.rs index 415ab9e1614..7a485202844 100644 --- a/zebra-chain/src/serialization/date_time.rs +++ b/zebra-chain/src/serialization/date_time.rs @@ -60,15 +60,13 @@ impl DateTime32 { pub fn checked_duration_since(&self, earlier: DateTime32) -> Option { self.timestamp .checked_sub(earlier.timestamp) - .map(|seconds| Duration32 { seconds }) + .map(Duration32::from) } /// Returns duration elapsed between `earlier` and this time, /// or zero if `earlier` is later than this time. pub fn saturating_duration_since(&self, earlier: DateTime32) -> Duration32 { - Duration32 { - seconds: self.timestamp.saturating_sub(earlier.timestamp), - } + Duration32::from(self.timestamp.saturating_sub(earlier.timestamp)) } /// Returns the duration elapsed since this time, @@ -94,9 +92,7 @@ impl DateTime32 { /// Returns the time that is `duration` after this time. /// If the calculation overflows, returns `DateTime32::MAX`. pub fn saturating_add(&self, duration: Duration32) -> DateTime32 { - DateTime32 { - timestamp: self.timestamp.saturating_add(duration.seconds), - } + DateTime32::from(self.timestamp.saturating_add(duration.seconds)) } /// Returns the time that is `duration` before this time. @@ -104,15 +100,13 @@ impl DateTime32 { pub fn checked_sub(&self, duration: Duration32) -> Option { self.timestamp .checked_sub(duration.seconds) - .map(|timestamp| DateTime32 { timestamp }) + .map(DateTime32::from) } /// Returns the time that is `duration` before this time. /// If the calculation underflows, returns `DateTime32::MIN`. pub fn saturating_sub(&self, duration: Duration32) -> DateTime32 { - DateTime32 { - timestamp: self.timestamp.saturating_sub(duration.seconds), - } + DateTime32::from(self.timestamp.saturating_sub(duration.seconds)) } } @@ -143,15 +137,13 @@ impl Duration32 { pub fn checked_add(&self, duration: Duration32) -> Option { self.seconds .checked_add(duration.seconds) - .map(|seconds| Duration32 { seconds }) + .map(Duration32::from) } /// Returns a duration that is `duration` longer than this duration. /// If the calculation overflows, returns `Duration32::MAX`. pub fn saturating_add(&self, duration: Duration32) -> Duration32 { - Duration32 { - seconds: self.seconds.saturating_add(duration.seconds), - } + Duration32::from(self.seconds.saturating_add(duration.seconds)) } /// Returns a duration that is `duration` shorter than this duration. @@ -159,15 +151,13 @@ impl Duration32 { pub fn checked_sub(&self, duration: Duration32) -> Option { self.seconds .checked_sub(duration.seconds) - .map(|seconds| Duration32 { seconds }) + .map(Duration32::from) } /// Returns a duration that is `duration` shorter than this duration. /// If the calculation underflows, returns `Duration32::MIN`. pub fn saturating_sub(&self, duration: Duration32) -> Duration32 { - Duration32 { - seconds: self.seconds.saturating_sub(duration.seconds), - } + Duration32::from(self.seconds.saturating_sub(duration.seconds)) } } From 8434ad37e0d1859ab6cc38dfe42c6b0974949103 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 22 Jun 2021 11:32:28 +1000 Subject: [PATCH 17/17] fastmod new_local_listener new_local_listener_change --- zebra-network/src/address_book.rs | 2 +- zebra-network/src/meta_addr.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-network/src/address_book.rs b/zebra-network/src/address_book.rs index c6a619a2b40..ecc6dec7df9 100644 --- a/zebra-network/src/address_book.rs +++ b/zebra-network/src/address_book.rs @@ -136,7 +136,7 @@ impl AddressBook { /// /// This address contains minimal state, but it is not sanitized. pub fn get_local_listener(&self) -> MetaAddr { - MetaAddr::new_local_listener(&self.local_listener) + MetaAddr::new_local_listener_change(&self.local_listener) .into_new_meta_addr() .expect("unexpected invalid new local listener addr") } diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index dfb5167c315..ae6a3063f84 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -318,7 +318,7 @@ impl MetaAddr { } /// Returns a [`MetaAddrChange::NewLocal`] for our own listener address. - pub fn new_local_listener(addr: &SocketAddr) -> MetaAddrChange { + pub fn new_local_listener_change(addr: &SocketAddr) -> MetaAddrChange { NewLocal { addr: canonical_socket_addr(*addr), }