Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpc(getpeerinfo): Add inbound peers to method response #9214

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 74 additions & 9 deletions zebra-network/src/address_book.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ mod tests;
///
/// # Security
///
/// Address book state must be based on outbound connections to peers.
/// Address book state must be based on outbound connections to peers iff
/// the book type is `AddressBookType::Outbound`. A separated address book
/// state for inbound connections can be created by using the
/// `AddressBookType::Inbound` book type.
///
/// If the address book is updated incorrectly:
/// - malicious peers can interfere with other peers' `AddressBook` state,
/// or
/// - Zebra can advertise unreachable addresses to its own peers.
///
/// ## Adding Addresses
/// ## Adding addresses to an outbound book
///
/// The address book should only contain Zcash listener port addresses from peers
/// on the configured network. These addresses can come from:
Expand All @@ -48,20 +51,38 @@ mod tests;
/// - the canonical address (`Version.address_from`) provided by each peer,
/// particularly peers on inbound connections.
///
/// The remote addresses of inbound connections must not be added to the address
/// book, because they contain ephemeral outbound ports, not listener ports.
/// The remote addresses of inbound connections must not be added to this type
/// of address book, because they contain ephemeral outbound ports, not listener
/// ports.
///
/// Isolated connections must not add addresses or update the address book.
/// Isolated connections must not add addresses or update the address book in any
/// case.
///
/// ## Updating Address State
/// ## Updating address state of an outbound book
///
/// Updates to address state must be based on outbound connections to peers.
///
/// Updates must not be based on:
/// - the remote addresses of inbound connections, or
/// - the canonical address of any connection.
///
/// ## Adding addresses to an inbound book
///
/// This type of address book should only contain remote peer addresses connected to
/// Zebra's listener port.
///
/// ## Updating address state of an inbound book
///
/// Updates to address state must be based on inbound connections from peers.

#[derive(Debug)]
pub struct AddressBook {
/// The type of this address book.
///
/// - An outbound book must only contain Zcash listener port addresses from peers on the configured network.
/// - An inbound book must only contain remote peer addresses connected to Zebra's listener port.
book_type: AddressBookType,

/// Peer listener addresses, suitable for outbound connections,
/// in connection attempt order.
///
Expand Down Expand Up @@ -102,6 +123,25 @@ pub struct AddressBook {
last_address_log: Option<Instant>,
}

/// The type of an [`AddressBook`].
#[derive(Clone, Debug)]
pub enum AddressBookType {
/// The address book is used for outbound connections.
Outbound,
/// The address book is used for inbound connections.
Inbound,
}

impl AddressBookType {
/// Return a string representation of this address book type.
pub fn as_str(&self) -> &'static str {
match self {
AddressBookType::Outbound => "outbound",
AddressBookType::Inbound => "inbound",
}
}
}

/// Metrics about the states of the addresses in an [`AddressBook`].
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Hash)]
pub struct AddressMetrics {
Expand Down Expand Up @@ -140,6 +180,7 @@ impl AddressBook {
network: &Network,
max_connections_per_ip: usize,
span: Span,
book_type: AddressBookType,
) -> AddressBook {
let constructor_span = span.clone();
let _guard = constructor_span.enter();
Expand All @@ -154,6 +195,7 @@ impl AddressBook {
// Avoid initiating outbound handshakes when max_connections_per_ip is 1.
let should_limit_outbound_conns_per_ip = max_connections_per_ip == 1;
let mut new_book = AddressBook {
book_type,
by_addr: OrderedMap::new(|meta_addr| Reverse(*meta_addr)),
local_listener: canonical_socket_addr(local_listener),
network: network.clone(),
Expand All @@ -168,8 +210,8 @@ impl AddressBook {
new_book
}

/// Construct an [`AddressBook`] with the given `local_listener`, `network`,
/// `addr_limit`, [`tracing::Span`], and addresses.
/// Construct an outbound [`AddressBook`] with the given `local_listener`,
/// `network`, `addr_limit`, [`tracing::Span`], and addresses.
///
/// `addr_limit` is enforced by this method, and by [`AddressBook::update`].
///
Expand Down Expand Up @@ -197,7 +239,13 @@ impl AddressBook {
// The maximum number of addresses should be always greater than 0
assert!(addr_limit > 0);

let mut new_book = AddressBook::new(local_listener, network, max_connections_per_ip, span);
let mut new_book = AddressBook::new(
local_listener,
network,
max_connections_per_ip,
span,
AddressBookType::Outbound,
);
new_book.addr_limit = addr_limit;

let addrs = addrs
Expand Down Expand Up @@ -763,6 +811,16 @@ impl AddressBookPeers for AddressBook {
.cloned()
.collect()
}

fn currently_live_peers(&self, now: chrono::DateTime<Utc>) -> Vec<MetaAddr> {
let _guard = self.span.enter();

self.by_addr
.descending_values()
.filter(|peer| peer.has_connection_recently_responded(now))
.cloned()
.collect()
}
}

impl AddressBookPeers for Arc<Mutex<AddressBook>> {
Expand All @@ -771,6 +829,12 @@ impl AddressBookPeers for Arc<Mutex<AddressBook>> {
.expect("panic in a previous thread that was holding the mutex")
.recently_live_peers(now)
}

fn currently_live_peers(&self, now: chrono::DateTime<Utc>) -> Vec<MetaAddr> {
self.lock()
.expect("panic in a previous thread that was holding the mutex")
.currently_live_peers(now)
}
}

impl Extend<MetaAddrChange> for AddressBook {
Expand All @@ -797,6 +861,7 @@ impl Clone for AddressBook {
watch::channel(*self.address_metrics_tx.borrow());

AddressBook {
book_type: self.book_type.clone(),
by_addr: self.by_addr.clone(),
local_listener: self.local_listener,
network: self.network.clone(),
Expand Down
3 changes: 2 additions & 1 deletion zebra-network/src/address_book/tests/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
constants::{DEFAULT_MAX_CONNS_PER_IP, MAX_ADDRS_IN_ADDRESS_BOOK},
meta_addr::MetaAddr,
protocol::external::types::PeerServices,
AddressBook,
AddressBook, AddressBookType,
};

/// Make sure an empty address book is actually empty.
Expand All @@ -25,6 +25,7 @@ fn address_book_empty() {
&Mainnet,
DEFAULT_MAX_CONNS_PER_IP,
Span::current(),
AddressBookType::Outbound,
);

assert_eq!(
Expand Down
3 changes: 3 additions & 0 deletions zebra-network/src/address_book_peers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,7 @@ pub use mock::MockAddressBookPeers;
pub trait AddressBookPeers {
/// Return an Vec of peers we've seen recently, in reconnection attempt order.
fn recently_live_peers(&self, now: chrono::DateTime<Utc>) -> Vec<MetaAddr>;

/// Return an Vec of peers that are likely to be live, in reconnection attempt order.
fn currently_live_peers(&self, now: chrono::DateTime<Utc>) -> Vec<MetaAddr>;
}
9 changes: 8 additions & 1 deletion zebra-network/src/address_book_peers/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ use crate::{meta_addr::MetaAddr, AddressBookPeers};
pub struct MockAddressBookPeers {
/// Return value for mock `recently_live_peers` method.
recently_live_peers: Vec<MetaAddr>,

/// Return value for mock `currently_live_peers` method.
currently_live_peers: Vec<MetaAddr>,
}

impl MockAddressBookPeers {
/// Creates a new [`MockAddressBookPeers`]
pub fn new(recently_live_peers: Vec<MetaAddr>) -> Self {
pub fn new(recently_live_peers: Vec<MetaAddr>, currently_live_peers: Vec<MetaAddr>) -> Self {
Self {
recently_live_peers,
currently_live_peers,
}
}
}
Expand All @@ -22,4 +26,7 @@ impl AddressBookPeers for MockAddressBookPeers {
fn recently_live_peers(&self, _now: chrono::DateTime<chrono::Utc>) -> Vec<MetaAddr> {
self.recently_live_peers.clone()
}
fn currently_live_peers(&self, _now: chrono::DateTime<chrono::Utc>) -> Vec<MetaAddr> {
self.currently_live_peers.clone()
}
}
20 changes: 13 additions & 7 deletions zebra-network/src/address_book_updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use tokio::{
use tracing::{Level, Span};

use crate::{
address_book::AddressMetrics, meta_addr::MetaAddrChange, AddressBook, BoxError, Config,
address_book::{AddressBookType, AddressMetrics},
meta_addr::MetaAddrChange,
AddressBook, BoxError, Config,
};

/// The minimum size of the address book updater channel.
Expand Down Expand Up @@ -41,6 +43,7 @@ impl AddressBookUpdater {
pub fn spawn(
config: &Config,
local_listener: SocketAddr,
book_type: AddressBookType,
) -> (
Arc<std::sync::Mutex<AddressBook>>,
mpsc::Sender<MetaAddrChange>,
Expand All @@ -59,26 +62,29 @@ impl AddressBookUpdater {
&config.network,
config.max_connections_per_ip,
span!(Level::TRACE, "address book"),
book_type.clone(),
);
let address_metrics = address_book.address_metrics_watcher();
let address_book = Arc::new(std::sync::Mutex::new(address_book));

#[cfg(feature = "progress-bar")]
let (mut address_info, address_bar, never_bar, failed_bar) = {
let address_bar = howudoin::new_root().label("Known Peers");
let never_bar =
howudoin::new_with_parent(address_bar.id()).label("Never Attempted Peers");
let failed_bar = howudoin::new_with_parent(never_bar.id()).label("Failed Peers");
let address_bar =
howudoin::new_root().label(format!("Known Peers ({})", book_type.as_str()));
let never_bar = howudoin::new_with_parent(address_bar.id())
.label(format!("Never Attempted Peers ({})", book_type.as_str()));
let failed_bar = howudoin::new_with_parent(never_bar.id())
.label(format!("Failed Peers ({})", book_type.as_str()));

(address_metrics.clone(), address_bar, never_bar, failed_bar)
};

let worker_address_book = address_book.clone();
let worker = move || {
info!("starting the address book updater");
info!("starting the {} address book updater", book_type.as_str());

while let Some(event) = worker_rx.blocking_recv() {
trace!(?event, "got address book change");
trace!(?event, "got {} address book change", book_type.as_str());

// # Correctness
//
Expand Down
2 changes: 1 addition & 1 deletion zebra-network/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ pub use crate::{
};

pub use crate::{
address_book::AddressBook,
address_book::{AddressBook, AddressBookType},
address_book_peers::AddressBookPeers,
config::{CacheDir, Config},
isolated::{connect_isolated, connect_isolated_tcp_direct},
Expand Down
6 changes: 3 additions & 3 deletions zebra-network/src/peer_set/candidate_set/tests/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{
canonical_peer_addr,
constants::{DEFAULT_MAX_CONNS_PER_IP, MIN_OUTBOUND_PEER_CONNECTION_INTERVAL},
meta_addr::{MetaAddr, MetaAddrChange},
AddressBook, BoxError, Request, Response,
AddressBook, AddressBookType, BoxError, Request, Response,
};

use super::super::{validate_addrs, CandidateSet};
Expand Down Expand Up @@ -71,7 +71,7 @@ proptest! {
});

// Since the address book is empty, there won't be any available peers
let address_book = AddressBook::new(SocketAddr::from_str("0.0.0.0:0").unwrap(), &Mainnet, DEFAULT_MAX_CONNS_PER_IP, Span::none());
let address_book = AddressBook::new(SocketAddr::from_str("0.0.0.0:0").unwrap(), &Mainnet, DEFAULT_MAX_CONNS_PER_IP, Span::none(), AddressBookType::Outbound);

let mut candidate_set = CandidateSet::new(Arc::new(std::sync::Mutex::new(address_book)), peer_service);

Expand Down Expand Up @@ -113,7 +113,7 @@ proptest! {
unreachable!("Mock peer service is never used");
});

let mut address_book = AddressBook::new(SocketAddr::from_str("0.0.0.0:0").unwrap(), &Mainnet, DEFAULT_MAX_CONNS_PER_IP, Span::none());
let mut address_book = AddressBook::new(SocketAddr::from_str("0.0.0.0:0").unwrap(), &Mainnet, DEFAULT_MAX_CONNS_PER_IP, Span::none(), AddressBookType::Outbound);
address_book.extend(peers);

let mut candidate_set = CandidateSet::new(Arc::new(std::sync::Mutex::new(address_book)), peer_service);
Expand Down
4 changes: 3 additions & 1 deletion zebra-network/src/peer_set/candidate_set/tests/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use zebra_test::mock_service::{MockService, PanicAssertion};
use crate::{
constants::{DEFAULT_MAX_CONNS_PER_IP, GET_ADDR_FANOUT, MIN_PEER_GET_ADDR_INTERVAL},
types::{MetaAddr, PeerServices},
AddressBook, Request, Response,
AddressBook, AddressBookType, Request, Response,
};

use super::super::{validate_addrs, CandidateSet};
Expand Down Expand Up @@ -143,6 +143,7 @@ fn candidate_set_updates_are_rate_limited() {
&Mainnet,
DEFAULT_MAX_CONNS_PER_IP,
Span::none(),
AddressBookType::Outbound,
);
let mut peer_service = MockService::build().for_unit_tests();
let mut candidate_set = CandidateSet::new(
Expand Down Expand Up @@ -189,6 +190,7 @@ fn candidate_set_update_after_update_initial_is_rate_limited() {
&Mainnet,
DEFAULT_MAX_CONNS_PER_IP,
Span::none(),
AddressBookType::Outbound,
);
let mut peer_service = MockService::build().for_unit_tests();
let mut candidate_set = CandidateSet::new(
Expand Down
Loading
Loading