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

fix(network): Ignore out of order Address Book changes, unless they are concurrent #6717

Merged
merged 13 commits into from
May 24, 2023
Merged
13 changes: 7 additions & 6 deletions zebra-network/src/address_book.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use ordered_map::OrderedMap;
use tokio::sync::watch;
use tracing::Span;

use zebra_chain::parameters::Network;
use zebra_chain::{parameters::Network, serialization::DateTime32};

use crate::{
constants,
Expand Down Expand Up @@ -222,10 +222,11 @@ impl AddressBook {
/// Get the local listener address.
///
/// This address contains minimal state, but it is not sanitized.
pub fn local_listener_meta_addr(&self) -> MetaAddr {
pub fn local_listener_meta_addr(&self, now: chrono::DateTime<Utc>) -> MetaAddr {
let now: DateTime32 = now.try_into().expect("will succeed until 2038");

MetaAddr::new_local_listener_change(self.local_listener)
.into_new_meta_addr()
.expect("unexpected invalid new local listener addr")
.local_listener_into_new_meta_addr(now)
}

/// Get the local listener [`SocketAddr`].
Expand All @@ -243,7 +244,7 @@ impl AddressBook {
// Unconditionally add our local listener address to the advertised peers,
// to replace any self-connection failures. The address book and change
// constructors make sure that the SocketAddr is canonical.
let local_listener = self.local_listener_meta_addr();
let local_listener = self.local_listener_meta_addr(now);
peers.insert(local_listener.addr, local_listener);

// Then sanitize and shuffle
Expand Down Expand Up @@ -307,7 +308,7 @@ impl AddressBook {
let instant_now = Instant::now();
let chrono_now = Utc::now();

let updated = change.apply_to_meta_addr(previous);
let updated = change.apply_to_meta_addr(previous, instant_now, chrono_now);

trace!(
?change,
Expand Down
18 changes: 18 additions & 0 deletions zebra-network/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,24 @@ pub const REQUEST_TIMEOUT: Duration = Duration::from_secs(20);
/// nodes, and on testnet.
pub const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(3);

/// The maximum time difference for two address book changes to be considered concurrent.
///
/// This prevents simultaneous or nearby important changes or connection progress
/// being overridden by less important changes.
///
/// This timeout should be less than:
/// - the [peer reconnection delay](MIN_PEER_RECONNECTION_DELAY), and
/// - the [peer keepalive/heartbeat interval](HEARTBEAT_INTERVAL).
///
/// But more than:
/// - the amount of time between connection events and address book updates,
/// even under heavy load (in tests, we have observed delays up to 500ms),
/// - the delay between an outbound connection failing,
/// and the [CandidateSet](crate::peer_set::CandidateSet) registering the failure, and
/// - the delay between the application closing a connection,
/// and any remaining positive changes from the peer.
pub const CONCURRENT_ADDRESS_CHANGE_PERIOD: Duration = Duration::from_secs(5);

/// We expect to receive a message from a live peer at least once in this time duration.
///
/// This is the sum of:
Expand Down
Loading