octo
/
Zebra
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Stop ignoring some peers when updating the address book (#3292) 

* Make sure MetaAddrChanges are correctly applied to an empty address book

* Add all initial peers to the address book
This commit is contained in:
teor 2022-01-06 09:12:59 +10:00 committed by GitHub
parent 144c532de4
commit 7e63182cdc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 132 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
zebra-network/src/meta_addr.rs
View File

@ -752,21 +752,15 @@ impl MetaAddrChange {
/// If this change can create a new `MetaAddr`, return that address.
pub fn into_new_meta_addr(self) -> Option<MetaAddr> {
match self {
NewInitial { .. } | NewGossiped { .. } | NewAlternate { .. } | NewLocal { .. } => {
Some(MetaAddr {
addr: self.addr(),
services: self.untrusted_services(),
untrusted_last_seen: self.untrusted_last_seen(),
last_response: None,
last_attempt: None,
last_failure: None,
last_connection_state: self.peer_addr_state(),
})
}
UpdateAttempt { .. } | UpdateResponded { .. } | UpdateFailed { .. } => None,
}
Some(MetaAddr {
addr: self.addr(),
services: self.untrusted_services(),
untrusted_last_seen: self.untrusted_last_seen(),
last_response: self.last_response(),
last_attempt: self.last_attempt(),
last_failure: self.last_failure(),
last_connection_state: self.peer_addr_state(),
})
}
/// Apply this change to a previous `MetaAddr` from the address book,

101
zebra-network/src/meta_addr/tests/prop.rs
View File

@ -1,4 +1,4 @@
//! Randomised property tests for MetaAddr.
//! Randomised property tests for MetaAddr and MetaAddrChange.
use std::{
collections::HashMap, convert::TryFrom, env, net::SocketAddr, str::FromStr, sync::Arc,
@ -174,10 +174,107 @@ proptest! {
canonical_local_listener,
);
}
/// Make sure that [`MetaAddrChange`]s are correctly applied
/// when there is no [`MetaAddr`] in the address book.
///
/// TODO: Make sure that [`MetaAddrChange`]s are correctly applied,
/// regardless of the [`MetaAddr`] that is currently in the address book.
#[test]
fn new_meta_addr_from_meta_addr_change(
(addr, changes) in MetaAddrChange::addr_changes_strategy(MAX_ADDR_CHANGE)
) {
zebra_test::init();
let local_listener = "0.0.0.0:0".parse().expect("unexpected invalid SocketAddr");
for change in changes {
// Check direct application
let new_addr = change.apply_to_meta_addr(None);
prop_assert!(
new_addr.is_some(),
"applying a change to `None` should always result in a new address,\n \
change: {:?}",
change,
);
let new_addr = new_addr.expect("just checked is_some");
prop_assert_eq!(new_addr.addr, addr.addr);
// Check address book update - return value
let mut address_book = AddressBook::new_with_addrs(
local_listener,
1,
Span::none(),
Vec::new(),
);
let expected_result = new_addr;
let book_result = address_book.update(change);
let book_contents: Vec<MetaAddr> = address_book.peers().collect();
// Ignore the same addresses that the address book ignores
let expected_result = if !expected_result.address_is_valid_for_outbound()
|| ( !expected_result.last_known_info_is_valid_for_outbound()
&& expected_result.last_connection_state.is_never_attempted())
{
None
} else {
Some(expected_result)
};
prop_assert_eq!(
book_result.is_some(),
expected_result.is_some(),
"applying a change to an empty address book should return a new address,\n \
unless its info is invalid,\n \
change: {:?},\n \
address book returned: {:?},\n \
expected result: {:?}",
change,
book_result,
expected_result,
);
if let Some(book_result) = book_result {
prop_assert_eq!(book_result.addr, addr.addr);
// TODO: pass times to MetaAddrChange::apply_to_meta_addr and AddressBook::update,
// so the times are equal
// prop_assert_eq!(new_addr, book_result);
}
// Check address book update - address book contents
prop_assert_eq!(
!book_contents.is_empty(), expected_result.is_some(),
"applying a change to an empty address book should add a new address,\n \
unless its info is invalid,\n \
change: {:?},\n \
address book contains: {:?},\n \
expected result: {:?}",
change,
book_contents,
expected_result,
);
if let Some(book_contents) = book_contents.first() {
prop_assert_eq!(book_contents.addr, addr.addr);
// TODO: pass times to MetaAddrChange::apply_to_meta_addr and AddressBook::update,
// so the times are equal
//prop_assert_eq!(new_addr, *book_contents);
}
prop_assert_eq!(book_result.as_ref(), book_contents.first());
// TODO: do we need to check each field is calculated correctly as well?
}
}
}
proptest! {
// These tests can produce a lot of debug output, so we use a smaller number of cases by default.
// These tests can produce a lot of debug output,
// so we let developers configure them with a smaller number of cases.
//
// Set the PROPTEST_CASES env var to override this default.
#![proptest_config(proptest::test_runner::Config::with_cases(env::var("PROPTEST_CASES")
.ok()

2
zebra-network/src/meta_addr/tests/vectors.rs
View File

@ -1,4 +1,4 @@
//! Test vectors for MetaAddr.
//! Fixed test cases for MetaAddr and MetaAddrChange.
use std::net::SocketAddr;

43
zebra-network/src/peer_set/initialize.rs
View File

@ -237,7 +237,7 @@ where
/// Use the provided `outbound_connector` to connect to the configured initial peers,
/// then send the resulting peer connections over `peerset_tx`.
///
/// Sends any unused initial peers to the `address_book_updater`.
/// Also sends every initial peer address to the `address_book_updater`.
#[instrument(skip(config, outbound_connector, peerset_tx, address_book_updater))]
async fn add_initial_peers<S>(
config: Config,
@ -375,9 +375,10 @@ where
/// Limit the number of `initial_peers` addresses entries to the configured
/// `peerset_initial_target_size`.
///
/// Returns randomly chosen entries from the provided set of addresses.
/// Returns randomly chosen entries from the provided set of addresses,
/// in a random order.
///
/// Sends any unused entries to the `address_book_updater`.
/// Also sends every initial peer to the `address_book_updater`.
async fn limit_initial_peers(
config: &Config,
address_book_updater: tokio::sync::mpsc::Sender<MetaAddrChange>,
@ -385,27 +386,29 @@ async fn limit_initial_peers(
let all_peers = config.initial_peers().await;
let peers_count = all_peers.len();
if peers_count <= config.peerset_initial_target_size {
return all_peers;
// # Correctness
//
// We can't exit early if we only have a few peers,
// because we still need to shuffle the connection order.
if all_peers.len() > config.peerset_initial_target_size {
info!(
"limiting the initial peers list from {} to {}",
peers_count, config.peerset_initial_target_size
);
}
// Limit the number of initial peers to `config.peerset_initial_target_size`
info!(
"Limiting the initial peers list from {} to {}",
peers_count, config.peerset_initial_target_size
);
// Split out the `initial_peers` that will be shuffled and returned.
let mut initial_peers: Vec<SocketAddr> = all_peers.iter().cloned().collect();
let (initial_peers, _unused_peers) =
initial_peers.partial_shuffle(&mut rand::thread_rng(), config.peerset_initial_target_size);
// Split all the peers into the `initial_peers` that will be returned and
// `unused_peers` that will be sent to the address book.
let mut all_peers: Vec<SocketAddr> = all_peers.into_iter().collect();
let (initial_peers, unused_peers) =
all_peers.partial_shuffle(&mut rand::thread_rng(), config.peerset_initial_target_size);
// Send the unused peers to the address book.
for peer in unused_peers {
let peer_addr = MetaAddr::new_initial_peer(*peer);
// Send every initial peer to the address book.
// (This treats initial peers the same way we treat gossiped peers.)
for peer in all_peers {
let peer_addr = MetaAddr::new_initial_peer(peer);
// `send` only waits when the channel is full.
// The address book updater is a separate task, so we will only wait for a short time.
// The address book updater runs in its own thread, so we will only wait for a short time.
let _ = address_book_updater.send(peer_addr).await;
}