From b9cf9ac346aa3d67e7e21310eb71aa68da35e803 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 25 Mar 2021 18:36:06 +1000 Subject: [PATCH] Make MetaAddr.last_seen into a private field --- zebra-network/src/address_book.rs | 4 +- zebra-network/src/meta_addr.rs | 68 ++++++++++++++++----- zebra-network/src/peer/handshake.rs | 9 +-- zebra-network/src/peer_set/candidate_set.rs | 5 +- 4 files changed, 58 insertions(+), 28 deletions(-) diff --git a/zebra-network/src/address_book.rs b/zebra-network/src/address_book.rs index 997b37c7d6a..f18d365465b 100644 --- a/zebra-network/src/address_book.rs +++ b/zebra-network/src/address_book.rs @@ -105,7 +105,7 @@ impl AddressBook { ); if let Some(prev) = self.get_by_addr(new.addr) { - if prev.last_seen > new.last_seen { + if prev.get_last_seen() > new.get_last_seen() { return; } } @@ -160,7 +160,7 @@ impl AddressBook { // NeverAttempted, Failed, and AttemptPending peers should never be live Some(peer) => { peer.last_connection_state == PeerAddrState::Responded - && peer.last_seen > AddressBook::liveness_cutoff_time() + && peer.get_last_seen() > AddressBook::liveness_cutoff_time() } } } diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index 2ee589a8546..37b39369a3a 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -104,6 +104,42 @@ pub struct MetaAddr { /// records, older peer versions, or buggy or malicious peers. pub services: PeerServices, + /// The last time we interacted with this peer. + /// + /// See `get_last_seen` for details. + last_seen: DateTime, + + /// The outcome of our most recent communication attempt with this peer. + pub last_connection_state: PeerAddrState, +} + +impl MetaAddr { + /// Create a new `MetaAddr` from the deserialized fields in an `Addr` + /// message. + pub fn new_gossiped( + addr: &SocketAddr, + services: &PeerServices, + last_seen: &DateTime, + ) -> MetaAddr { + MetaAddr { + addr: *addr, + services: *services, + last_seen: *last_seen, + // the state is Zebra-specific, it isn't part of the Zcash network protocol + last_connection_state: NeverAttempted, + } + } + + /// Create a new `MetaAddr` for a peer that has just `Responded`. + pub fn new_responded(addr: &SocketAddr, services: &PeerServices) -> MetaAddr { + MetaAddr { + addr: *addr, + services: *services, + last_seen: Utc::now(), + last_connection_state: Responded, + } + } + /// The last time we interacted with this peer. /// /// The exact meaning depends on `last_connection_state`: @@ -118,17 +154,19 @@ pub struct MetaAddr { /// /// `last_seen` times from `NeverAttempted` peers may be invalid due to /// clock skew, or buggy or malicious peers. - pub last_seen: DateTime, + pub fn get_last_seen(&self) -> DateTime { + self.last_seen + } - /// The outcome of our most recent communication attempt with this peer. - pub last_connection_state: PeerAddrState, -} + /// Update the last time we interacted with this peer to the current time. + pub fn update_last_seen(&mut self) { + self.last_seen = Utc::now(); + } -impl MetaAddr { /// Return a sanitized version of this `MetaAddr`, for sending to a remote peer. pub fn sanitize(&self) -> MetaAddr { let interval = crate::constants::TIMESTAMP_TRUNCATION_SECONDS; - let ts = self.last_seen.timestamp(); + let ts = self.get_last_seen().timestamp(); let last_seen = Utc.timestamp(ts - ts.rem_euclid(interval), 0); MetaAddr { addr: self.addr, @@ -151,7 +189,7 @@ impl Ord for MetaAddr { fn cmp(&self, other: &Self) -> Ordering { use std::net::IpAddr::{V4, V6}; - let oldest_first = self.last_seen.cmp(&other.last_seen); + let oldest_first = self.get_last_seen().cmp(&other.get_last_seen()); let newest_first = oldest_first.reverse(); let connection_state = self.last_connection_state.cmp(&other.last_connection_state); @@ -187,7 +225,7 @@ impl PartialOrd for MetaAddr { impl ZcashSerialize for MetaAddr { fn zcash_serialize(&self, mut writer: W) -> Result<(), std::io::Error> { - writer.write_u32::(self.last_seen.timestamp() as u32)?; + writer.write_u32::(self.get_last_seen().timestamp() as u32)?; writer.write_u64::(self.services.bits())?; writer.write_socket_addr(self.addr)?; Ok(()) @@ -196,13 +234,11 @@ impl ZcashSerialize for MetaAddr { impl ZcashDeserialize for MetaAddr { fn zcash_deserialize(mut reader: R) -> Result { - Ok(MetaAddr { - last_seen: Utc.timestamp(reader.read_u32::()? as i64, 0), - // Discard unknown service bits. - services: PeerServices::from_bits_truncate(reader.read_u64::()?), - addr: reader.read_socket_addr()?, - last_connection_state: Default::default(), - }) + let last_seen = Utc.timestamp(reader.read_u32::()? as i64, 0); + let services = PeerServices::from_bits_truncate(reader.read_u64::()?); + let addr = reader.read_socket_addr()?; + + Ok(MetaAddr::new_gossiped(&addr, &services, &last_seen)) } } @@ -227,7 +263,7 @@ mod tests { // We want the sanitized timestamp to be a multiple of the truncation interval. assert_eq!( - entry.last_seen.timestamp() % crate::constants::TIMESTAMP_TRUNCATION_SECONDS, + entry.get_last_seen().timestamp() % crate::constants::TIMESTAMP_TRUNCATION_SECONDS, 0 ); // We want the state to be the default diff --git a/zebra-network/src/peer/handshake.rs b/zebra-network/src/peer/handshake.rs index 49f1f52c9a5..af7ac25deba 100644 --- a/zebra-network/src/peer/handshake.rs +++ b/zebra-network/src/peer/handshake.rs @@ -27,7 +27,7 @@ use crate::{ internal::{Request, Response}, }, types::MetaAddr, - BoxError, Config, PeerAddrState, + BoxError, Config, }; use super::{Client, Connection, ErrorSlot, HandshakeError, PeerError}; @@ -390,12 +390,7 @@ where ); use futures::sink::SinkExt; let _ = timestamp_collector - .send(MetaAddr { - addr, - services: remote_services, - last_seen: Utc::now(), - last_connection_state: PeerAddrState::Responded, - }) + .send(MetaAddr::new_responded(&addr, &remote_services)) .await; } msg diff --git a/zebra-network/src/peer_set/candidate_set.rs b/zebra-network/src/peer_set/candidate_set.rs index acfd6ec3d46..a7dafe4adf2 100644 --- a/zebra-network/src/peer_set/candidate_set.rs +++ b/zebra-network/src/peer_set/candidate_set.rs @@ -4,7 +4,6 @@ use std::{ time::Duration, }; -use chrono::Utc; use futures::stream::{FuturesUnordered, StreamExt}; use tokio::time::{sleep, sleep_until, Sleep}; use tower::{Service, ServiceExt}; @@ -221,7 +220,7 @@ where // instead of yielding the next connection. let mut reconnect = peer_set_guard.reconnection_peers().next()?; - reconnect.last_seen = Utc::now(); + reconnect.update_last_seen(); reconnect.last_connection_state = PeerAddrState::AttemptPending; peer_set_guard.update(reconnect); reconnect @@ -235,7 +234,7 @@ where /// Mark `addr` as a failed peer. pub fn report_failed(&mut self, mut addr: MetaAddr) { - addr.last_seen = Utc::now(); + addr.update_last_seen(); addr.last_connection_state = PeerAddrState::Failed; self.peer_set.lock().unwrap().update(addr); }