diff --git a/zebra-network/src/address_book.rs b/zebra-network/src/address_book.rs index 7a2cd556e19..50d18be2ada 100644 --- a/zebra-network/src/address_book.rs +++ b/zebra-network/src/address_book.rs @@ -112,9 +112,6 @@ pub struct AddressMetrics { /// The number of addresses in the `NeverAttemptedGossiped` state. pub never_attempted_gossiped: usize, - /// The number of addresses in the `NeverAttemptedAlternate` state. - pub never_attempted_alternate: usize, - /// The number of addresses in the `Failed` state. pub failed: usize, @@ -667,9 +664,6 @@ impl AddressBook { let never_attempted_gossiped = self .state_peers(PeerAddrState::NeverAttemptedGossiped) .count(); - let never_attempted_alternate = self - .state_peers(PeerAddrState::NeverAttemptedAlternate) - .count(); let failed = self.state_peers(PeerAddrState::Failed).count(); let attempt_pending = self.state_peers(PeerAddrState::AttemptPending).count(); @@ -683,7 +677,6 @@ impl AddressBook { AddressMetrics { responded, never_attempted_gossiped, - never_attempted_alternate, failed, attempt_pending, recently_live, @@ -705,10 +698,6 @@ impl AddressBook { // TODO: rename to address_book.[state_name] metrics::gauge!("candidate_set.responded", m.responded as f64); metrics::gauge!("candidate_set.gossiped", m.never_attempted_gossiped as f64); - metrics::gauge!( - "candidate_set.alternate", - m.never_attempted_alternate as f64, - ); metrics::gauge!("candidate_set.failed", m.failed as f64); metrics::gauge!("candidate_set.pending", m.attempt_pending as f64); @@ -754,12 +743,7 @@ impl AddressBook { self.last_address_log = Some(now); // if all peers have failed - if m.responded - + m.attempt_pending - + m.never_attempted_gossiped - + m.never_attempted_alternate - == 0 - { + if m.responded + m.attempt_pending + m.never_attempted_gossiped == 0 { warn!( address_metrics = ?m, "all peer addresses have failed. Hint: check your network connection" diff --git a/zebra-network/src/address_book_updater.rs b/zebra-network/src/address_book_updater.rs index f0729b3efb0..2feeb49e8e0 100644 --- a/zebra-network/src/address_book_updater.rs +++ b/zebra-network/src/address_book_updater.rs @@ -106,10 +106,9 @@ impl AddressBookUpdater { .set_pos(u64::try_from(address_info.num_addresses).expect("fits in u64")); // .set_len(u64::try_from(address_info.address_limit).expect("fits in u64")); - let never_attempted = address_info.never_attempted_alternate - + address_info.never_attempted_gossiped; - - never_bar.set_pos(u64::try_from(never_attempted).expect("fits in u64")); + never_bar.set_pos( + u64::try_from(address_info.never_attempted_gossiped).expect("fits in u64"), + ); // .set_len(u64::try_from(address_info.address_limit).expect("fits in u64")); failed_bar.set_pos(u64::try_from(address_info.failed).expect("fits in u64")); diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index 98d3f32224a..507620f29a0 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -54,15 +54,10 @@ pub enum PeerAddrState { Responded, /// The peer's address has just been fetched from a DNS seeder, or via peer - /// gossip, but we haven't attempted to connect to it yet. + /// gossip, or as part of a `Version` message, or guessed from an inbound remote IP, + /// but we haven't attempted to connect to it yet. NeverAttemptedGossiped, - /// The peer's address has just been received as part of a `Version` message, - /// so we might already be connected to this peer. - /// - /// Alternate addresses are attempted after gossiped addresses. - NeverAttemptedAlternate, - /// The peer's TCP connection failed, or the peer sent us an unexpected /// Zcash protocol message, so we failed the connection. Failed, @@ -75,7 +70,7 @@ impl PeerAddrState { /// Return true if this state is a "never attempted" state. pub fn is_never_attempted(&self) -> bool { match self { - NeverAttemptedGossiped | NeverAttemptedAlternate => true, + NeverAttemptedGossiped => true, AttemptPending | Responded | Failed => false, } } @@ -88,17 +83,8 @@ impl PeerAddrState { use Ordering::*; match (self, other) { _ if self == other => Equal, - // Peers start in one of the "never attempted" states, + // Peers start in the "never attempted" state, // then typically progress towards a "responded" or "failed" state. - // - // # Security - // - // Prefer gossiped addresses to alternate addresses, - // so that peers can't replace the addresses of other peers. - // (This is currently checked explicitly by the address update code, - // but we respect the same order here as a precaution.) - (NeverAttemptedAlternate, _) => Less, - (_, NeverAttemptedAlternate) => Greater, (NeverAttemptedGossiped, _) => Less, (_, NeverAttemptedGossiped) => Greater, (AttemptPending, _) => Less, @@ -139,8 +125,6 @@ impl Ord for PeerAddrState { (_, Responded) => Greater, (NeverAttemptedGossiped, _) => Less, (_, NeverAttemptedGossiped) => Greater, - (NeverAttemptedAlternate, _) => Less, - (_, NeverAttemptedAlternate) => Greater, (Failed, _) => Less, (_, Failed) => Greater, // These patterns are redundant, but Rust doesn't assume that `==` is reflexive, @@ -249,18 +233,6 @@ pub enum MetaAddrChange { untrusted_last_seen: DateTime32, }, - /// Creates new alternate `MetaAddr`. - /// - /// Based on the canonical peer address in `Version` messages. - NewAlternate { - #[cfg_attr( - any(test, feature = "proptest-impl"), - proptest(strategy = "canonical_peer_addr_strategy()") - )] - addr: PeerSocketAddr, - untrusted_services: PeerServices, - }, - /// Creates new local listener `MetaAddr`. NewLocal { #[cfg_attr( @@ -398,18 +370,6 @@ impl MetaAddr { } } - /// Returns a [`MetaAddrChange::NewAlternate`] for a peer's alternate address, - /// received via a `Version` message. - pub fn new_alternate( - addr: PeerSocketAddr, - untrusted_services: &PeerServices, - ) -> MetaAddrChange { - NewAlternate { - addr: canonical_peer_addr(*addr), - untrusted_services: *untrusted_services, - } - } - /// Returns a [`MetaAddrChange::NewLocal`] for our own listener address. pub fn new_local_listener_change(addr: impl Into) -> MetaAddrChange { NewLocal { @@ -715,7 +675,6 @@ impl MetaAddrChange { match self { NewInitial { addr } | NewGossiped { addr, .. } - | NewAlternate { addr, .. } | NewLocal { addr, .. } | UpdateAttempt { addr } | UpdateConnected { addr, .. } @@ -732,7 +691,6 @@ impl MetaAddrChange { match self { NewInitial { addr } | NewGossiped { addr, .. } - | NewAlternate { addr, .. } | NewLocal { addr, .. } | UpdateAttempt { addr } | UpdateConnected { addr, .. } @@ -748,9 +706,6 @@ impl MetaAddrChange { // TODO: split untrusted and direct services (#2324) NewGossiped { untrusted_services, .. - } - | NewAlternate { - untrusted_services, .. } => Some(*untrusted_services), // TODO: create a "services implemented by Zebra" constant (#2324) NewLocal { .. } => Some(PeerServices::NODE_NETWORK), @@ -769,7 +724,6 @@ impl MetaAddrChange { untrusted_last_seen, .. } => Some(*untrusted_last_seen), - NewAlternate { .. } => None, // We know that our local listener is available NewLocal { .. } => Some(now), UpdateAttempt { .. } @@ -801,7 +755,7 @@ impl MetaAddrChange { /// Return the last attempt for this change, if available. pub fn last_attempt(&self, now: Instant) -> Option { match self { - NewInitial { .. } | NewGossiped { .. } | NewAlternate { .. } | NewLocal { .. } => None, + NewInitial { .. } | NewGossiped { .. } | NewLocal { .. } => None, // Attempt changes are applied before we start the handshake to the // peer address. So the attempt time is a lower bound for the actual // handshake time. @@ -813,11 +767,7 @@ impl MetaAddrChange { /// Return the last response for this change, if available. pub fn last_response(&self, now: DateTime32) -> Option { match self { - NewInitial { .. } - | NewGossiped { .. } - | NewAlternate { .. } - | NewLocal { .. } - | UpdateAttempt { .. } => None, + NewInitial { .. } | NewGossiped { .. } | NewLocal { .. } | UpdateAttempt { .. } => None, // If there is a large delay applying this change, then: // - the peer might stay in the `AttemptPending` state for longer, // - we might send outdated last seen times to our peers, and @@ -833,7 +783,6 @@ impl MetaAddrChange { match self { NewInitial { .. } | NewGossiped { .. } - | NewAlternate { .. } | NewLocal { .. } | UpdateAttempt { .. } | UpdateConnected { .. } @@ -852,7 +801,6 @@ impl MetaAddrChange { match self { NewInitial { .. } => NeverAttemptedGossiped, NewGossiped { .. } => NeverAttemptedGossiped, - NewAlternate { .. } => NeverAttemptedAlternate, // local listeners get sanitized, so the state doesn't matter here NewLocal { .. } => NeverAttemptedGossiped, UpdateAttempt { .. } => AttemptPending, diff --git a/zebra-network/src/meta_addr/arbitrary.rs b/zebra-network/src/meta_addr/arbitrary.rs index d7d4dd840e8..985b36693ea 100644 --- a/zebra-network/src/meta_addr/arbitrary.rs +++ b/zebra-network/src/meta_addr/arbitrary.rs @@ -1,6 +1,6 @@ //! Randomised test data generation for MetaAddr. -use std::{net::IpAddr, time::Instant}; +use std::net::IpAddr; use proptest::{arbitrary::any, collection::vec, prelude::*}; @@ -45,28 +45,6 @@ impl MetaAddr { }) .boxed() } - - /// Create a strategy that generates [`MetaAddr`]s in the - /// [`NeverAttemptedAlternate`][1] state. - /// - /// [1]: super::PeerAddrState::NeverAttemptedAlternate - pub fn alternate_strategy() -> BoxedStrategy { - ( - canonical_peer_addr_strategy(), - any::(), - any::(), - any::(), - ) - .prop_map( - |(socket_addr, untrusted_services, instant_now, local_now)| { - // instant_now is not actually used for this variant, - // so we could just provide a default value - MetaAddr::new_alternate(socket_addr, &untrusted_services) - .into_new_meta_addr(instant_now, local_now) - }, - ) - .boxed() - } } impl MetaAddrChange { @@ -109,33 +87,27 @@ impl MetaAddrChange { .boxed() } - /// Create a strategy that generates port numbers for [`MetaAddrChange`]s which are ready for + /// Create a strategy that generates port numbers for [`MetaAddr`]s which are ready for /// outbound connections. /// - /// Currently, all generated changes are the [`NewAlternate`][1] variant. - /// TODO: Generate all [`MetaAddrChange`] variants, and give them ready - /// fields. (After PR #2276 merges.) + /// Currently, all generated [`MetaAddr`]s are the [`NeverAttemptedGossiped`][1] variant. + /// + /// TODO: Generate all [`MetaAddr`] variants, and give them ready fields. /// - /// [1]: super::NewAlternate + /// [1]: super::NeverAttemptedGossiped pub fn ready_outbound_strategy_port() -> BoxedStrategy { ( canonical_peer_addr_strategy(), - any::(), + any::(), any::(), ) .prop_filter_map( "failed MetaAddr::is_valid_for_outbound", - |(addr, instant_now, local_now)| { - // Alternate nodes use the current time, so they're always ready - // - // TODO: create a "Zebra supported services" constant + |(addr, services, local_now)| { + let addr = MetaAddr::new_gossiped_meta_addr(addr, services, local_now); - let change = MetaAddr::new_alternate(addr, &PeerServices::NODE_NETWORK); - if change - .into_new_meta_addr(instant_now, local_now) - .last_known_info_is_valid_for_outbound(Mainnet) - { - Some(addr.port()) + if addr.last_known_info_is_valid_for_outbound(Mainnet) { + Some(addr.addr.port()) } else { None } diff --git a/zebra-network/src/meta_addr/tests/vectors.rs b/zebra-network/src/meta_addr/tests/vectors.rs index 40e00b66513..d5a81bfd996 100644 --- a/zebra-network/src/meta_addr/tests/vectors.rs +++ b/zebra-network/src/meta_addr/tests/vectors.rs @@ -74,25 +74,6 @@ fn new_local_listener_is_gossipable() { assert!(peer.is_active_for_gossip(chrono_now)); } -/// Test if a recently received alternate peer address is not gossipable. -/// -/// Such [`MetaAddr`] is only considered gossipable after Zebra has tried to connect to it and -/// confirmed that the address is reachable. -#[test] -fn new_alternate_peer_address_is_not_gossipable() { - let _init_guard = zebra_test::init(); - - let instant_now = Instant::now(); - let chrono_now = Utc::now(); - let local_now: DateTime32 = chrono_now.try_into().expect("will succeed until 2038"); - - let address = PeerSocketAddr::from(([192, 168, 180, 9], 10_000)); - let peer = MetaAddr::new_alternate(address, &PeerServices::NODE_NETWORK) - .into_new_meta_addr(instant_now, local_now); - - assert!(!peer.is_active_for_gossip(chrono_now)); -} - /// Test if recently received gossiped peer is gossipable. #[test] fn gossiped_peer_reportedly_to_be_seen_recently_is_gossipable() { @@ -166,8 +147,7 @@ fn recently_responded_peer_is_gossipable() { let local_now: DateTime32 = chrono_now.try_into().expect("will succeed until 2038"); let address = PeerSocketAddr::from(([192, 168, 180, 9], 10_000)); - let peer_seed = MetaAddr::new_alternate(address, &PeerServices::NODE_NETWORK) - .into_new_meta_addr(instant_now, local_now); + let peer_seed = MetaAddr::new_initial_peer(address).into_new_meta_addr(instant_now, local_now); // Create a peer that has responded let peer = MetaAddr::new_responded(address) @@ -187,8 +167,7 @@ fn not_so_recently_responded_peer_is_still_gossipable() { let local_now: DateTime32 = chrono_now.try_into().expect("will succeed until 2038"); let address = PeerSocketAddr::from(([192, 168, 180, 9], 10_000)); - let peer_seed = MetaAddr::new_alternate(address, &PeerServices::NODE_NETWORK) - .into_new_meta_addr(instant_now, local_now); + let peer_seed = MetaAddr::new_initial_peer(address).into_new_meta_addr(instant_now, local_now); // Create a peer that has responded let mut peer = MetaAddr::new_responded(address) @@ -218,8 +197,7 @@ fn responded_long_ago_peer_is_not_gossipable() { let local_now: DateTime32 = chrono_now.try_into().expect("will succeed until 2038"); let address = PeerSocketAddr::from(([192, 168, 180, 9], 10_000)); - let peer_seed = MetaAddr::new_alternate(address, &PeerServices::NODE_NETWORK) - .into_new_meta_addr(instant_now, local_now); + let peer_seed = MetaAddr::new_initial_peer(address).into_new_meta_addr(instant_now, local_now); // Create a peer that has responded let mut peer = MetaAddr::new_responded(address) @@ -249,8 +227,7 @@ fn long_delayed_change_is_not_applied() { let local_now: DateTime32 = chrono_now.try_into().expect("will succeed until 2038"); let address = PeerSocketAddr::from(([192, 168, 180, 9], 10_000)); - let peer_seed = MetaAddr::new_alternate(address, &PeerServices::NODE_NETWORK) - .into_new_meta_addr(instant_now, local_now); + let peer_seed = MetaAddr::new_initial_peer(address).into_new_meta_addr(instant_now, local_now); // Create a peer that has responded let peer = MetaAddr::new_responded(address) @@ -293,8 +270,7 @@ fn later_revert_change_is_applied() { let local_now: DateTime32 = chrono_now.try_into().expect("will succeed until 2038"); let address = PeerSocketAddr::from(([192, 168, 180, 9], 10_000)); - let peer_seed = MetaAddr::new_alternate(address, &PeerServices::NODE_NETWORK) - .into_new_meta_addr(instant_now, local_now); + let peer_seed = MetaAddr::new_initial_peer(address).into_new_meta_addr(instant_now, local_now); // Create a peer that has responded let peer = MetaAddr::new_responded(address) @@ -336,8 +312,7 @@ fn concurrent_state_revert_change_is_not_applied() { let local_now: DateTime32 = chrono_now.try_into().expect("will succeed until 2038"); let address = PeerSocketAddr::from(([192, 168, 180, 9], 10_000)); - let peer_seed = MetaAddr::new_alternate(address, &PeerServices::NODE_NETWORK) - .into_new_meta_addr(instant_now, local_now); + let peer_seed = MetaAddr::new_initial_peer(address).into_new_meta_addr(instant_now, local_now); // Create a peer that has responded let peer = MetaAddr::new_responded(address) @@ -396,8 +371,7 @@ fn concurrent_state_progress_change_is_applied() { let local_now: DateTime32 = chrono_now.try_into().expect("will succeed until 2038"); let address = PeerSocketAddr::from(([192, 168, 180, 9], 10_000)); - let peer_seed = MetaAddr::new_alternate(address, &PeerServices::NODE_NETWORK) - .into_new_meta_addr(instant_now, local_now); + let peer_seed = MetaAddr::new_initial_peer(address).into_new_meta_addr(instant_now, local_now); // Create a peer that has responded let peer = MetaAddr::new_responded(address) diff --git a/zebra-network/src/peer_set/candidate_set.rs b/zebra-network/src/peer_set/candidate_set.rs index 9458bbeaa8c..b347508916b 100644 --- a/zebra-network/src/peer_set/candidate_set.rs +++ b/zebra-network/src/peer_set/candidate_set.rs @@ -1,6 +1,6 @@ //! Candidate peer selection for outbound connections using the [`CandidateSet`]. -use std::{cmp::min, sync::Arc}; +use std::{any::type_name, cmp::min, sync::Arc}; use chrono::Utc; use futures::stream::{FuturesUnordered, StreamExt}; @@ -26,13 +26,12 @@ mod tests; /// /// 1. [`Responded`] peers, which we have had an outbound connection to. /// 2. [`NeverAttemptedGossiped`] peers, which we learned about from other peers -/// but have never connected to. -/// 3. [`NeverAttemptedAlternate`] peers, canonical addresses which we learned -/// from the [`Version`] messages of inbound and outbound connections, -/// but have never connected to. -/// 4. [`Failed`] peers, which failed a connection attempt, or had an error +/// but have never connected to. This includes gossiped peers, DNS seeder peers, +/// cached peers, canonical addresses from the [`Version`] messages of inbound +/// and outbound connections, and remote IP addresses of inbound connections. +/// 3. [`Failed`] peers, which failed a connection attempt, or had an error /// during an outbound connection. -/// 5. [`AttemptPending`] peers, which we've recently queued for a connection. +/// 4. [`AttemptPending`] peers, which we've recently queued for a connection. /// /// Never attempted peers are always available for connection. /// @@ -68,8 +67,7 @@ mod tests; /// │ disjoint `PeerAddrState`s ▼ │ /// │ ┌─────────────┐ ┌─────────────────────────┐ ┌─────────────┐ │ /// │ │ `Responded` │ │`NeverAttemptedGossiped` │ │ `Failed` │ │ -/// ┌┼▶│ Peers │ │`NeverAttemptedAlternate`│ │ Peers │◀┼┐ -/// ││ │ │ │ Peers │ │ │ ││ +/// ┌┼▶│ Peers │ │ Peers │ │ Peers │◀┼┐ /// ││ └─────────────┘ └─────────────────────────┘ └─────────────┘ ││ /// ││ │ │ │ ││ /// ││ #1 oldest_first #2 newest_first #3 oldest_first ││ @@ -115,25 +113,52 @@ mod tests; /// [`Responded`]: crate::PeerAddrState::Responded /// [`Version`]: crate::protocol::external::types::Version /// [`NeverAttemptedGossiped`]: crate::PeerAddrState::NeverAttemptedGossiped -/// [`NeverAttemptedAlternate`]: crate::PeerAddrState::NeverAttemptedAlternate /// [`Failed`]: crate::PeerAddrState::Failed /// [`AttemptPending`]: crate::PeerAddrState::AttemptPending // TODO: // * show all possible transitions between Attempt/Responded/Failed, // except Failed -> Responded is invalid, must go through Attempt +// +// Note: the CandidateSet can't be cloned, because there needs to be a single +// instance of its timers, so that rate limits are enforced correctly. pub(crate) struct CandidateSet where S: Service + Send, S::Future: Send + 'static, { - // Correctness: the address book must be private, - // so all operations are performed on a blocking thread (see #1976). + /// The outbound address book for this peer set. + /// + /// # Correctness + /// + /// The address book must be private, so all operations are performed on a blocking thread + /// (see #1976). address_book: Arc>, + + /// The peer set used to crawl the network for peers. peer_service: S, + + /// A timer that enforces a rate-limit on new outbound connections. min_next_handshake: Instant, + + /// A timer that enforces a rate-limit on peer set requests for more peers. min_next_crawl: Instant, } +impl std::fmt::Debug for CandidateSet +where + S: Service + Send, + S::Future: Send + 'static, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CandidateSet") + .field("address_book", &self.address_book) + .field("peer_service", &type_name::()) + .field("min_next_handshake", &self.min_next_handshake) + .field("min_next_crawl", &self.min_next_crawl) + .finish() + } +} + impl CandidateSet where S: Service + Send, diff --git a/zebra-network/src/peer_set/candidate_set/tests/prop.rs b/zebra-network/src/peer_set/candidate_set/tests/prop.rs index e5201e046ba..d434e9866e2 100644 --- a/zebra-network/src/peer_set/candidate_set/tests/prop.rs +++ b/zebra-network/src/peer_set/candidate_set/tests/prop.rs @@ -21,7 +21,6 @@ use crate::{ canonical_peer_addr, constants::{DEFAULT_MAX_CONNS_PER_IP, MIN_OUTBOUND_PEER_CONNECTION_INTERVAL}, meta_addr::{MetaAddr, MetaAddrChange}, - protocol::types::PeerServices, AddressBook, BoxError, Request, Response, }; @@ -107,7 +106,7 @@ proptest! { let _guard = runtime.enter(); let peers = peers.into_iter().map(|(ip, port)| { - MetaAddr::new_alternate(canonical_peer_addr(SocketAddr::new(ip, port)), &PeerServices::NODE_NETWORK) + MetaAddr::new_initial_peer(canonical_peer_addr(SocketAddr::new(ip, port))) }).collect::>(); let peer_service = tower::service_fn(|_| async {