From ddcfe825cba657bee258fcf839c1b9e9791c44cc Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 25 Jan 2022 19:19:15 -0300 Subject: [PATCH 1/4] add a test for peerset always broadcast while there are available peers --- zebra-network/src/peer_set/set/tests/prop.rs | 85 ++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/zebra-network/src/peer_set/set/tests/prop.rs b/zebra-network/src/peer_set/set/tests/prop.rs index b803444cf63..73c30ae65bd 100644 --- a/zebra-network/src/peer_set/set/tests/prop.rs +++ b/zebra-network/src/peer_set/set/tests/prop.rs @@ -166,6 +166,91 @@ proptest! { Ok::<_, TestCaseError>(()) })?; } + + /// Test the peerset will always broadcast iff there is at least one + /// peer in the set. + /// Peerset will panic a if a rquest is sent and no more peers are available. + #[test] + #[should_panic(expected = "requests must be routed to at least one peer")] + fn peerset_always_broadcast( + total_number_of_peers in (2..10usize) + ) { + // Get a dummy block hash to help us construct a valid request to be broadcasted + let block: block::Block = zebra_test::vectors::BLOCK_MAINNET_10_BYTES + .zcash_deserialize_into() + .unwrap(); + let block_hash = block::Hash::from(&block); + + // Start the runtime + let runtime = zebra_test::init_async(); + let _guard = runtime.enter(); + + // All peers will have the current version + let peer_versions = vec![CURRENT_NETWORK_PROTOCOL_VERSION; total_number_of_peers]; + let peer_versions = PeerVersions { + peer_versions, + }; + + // Get peers and handles + let (discovered_peers, mut handles) = peer_versions.mock_peer_discovery(); + let (minimum_peer_version, _best_tip_height) = + MinimumPeerVersion::with_mock_chain_tip(Network::Mainnet); + + // Build a peerset + runtime.block_on(async move { + let (mut peer_set, _peer_set_guard) = PeerSetBuilder::new() + .with_discover(discovered_peers) + .with_minimum_peer_version(minimum_peer_version.clone()) + .build(); + + // Remove peers, test broadcast until there is only 1 peer left in the peerset + let mut new_number_of_peers = total_number_of_peers; + for port in 1u16..total_number_of_peers as u16 { + peer_set.remove(&SocketAddr::new([127, 0, 0, 1].into(), port)); + handles.remove(0); + + // poll the peers + check_if_only_up_to_date_peers_are_live( + &mut peer_set, + &mut handles, + CURRENT_NETWORK_PROTOCOL_VERSION, + )?; + + // Get the new number of active peers after removal + let number_of_peers_to_broadcast = peer_set.number_of_peers_to_broadcast(); + + // Send a request to all peers we have now + let _ = peer_set.route_broadcast(Request::AdvertiseBlock(block_hash)); + + // Check how many peers received the request + let mut received = 0; + for h in &mut handles { + if let ReceiveRequestAttempt::Request(client_request) = h.try_to_receive_outbound_client_request() { + prop_assert_eq!(client_request.request, Request::AdvertiseBlock(block_hash)); + received += 1; + }; + } + + // Make sure the message was broadcasted to the right number of peers + prop_assert_eq!(received, number_of_peers_to_broadcast); + + // Exit the loop when we have only 1 peer left + new_number_of_peers -= 1; + if new_number_of_peers == 1 { + break; + } + } + + // Remove the last peer we have left in the peerset + peer_set.remove(&SocketAddr::new([127, 0, 0, 1].into(), total_number_of_peers as u16)); + handles.remove(0); + + // this will panic as expected + let _ = peer_set.route_broadcast(Request::AdvertiseBlock(block_hash)); + + Ok::<_, TestCaseError>(()) + })?; + } } /// Check if only peers with up-to-date protocol versions are live. From a79615ad1664d5ccfd2a75142706939057a90a9c Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 31 Jan 2022 20:05:18 -0300 Subject: [PATCH 2/4] fix minors from review Co-authored-by: Janito Vaqueiro Ferreira Filho --- zebra-network/src/peer_set/set/tests/prop.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/zebra-network/src/peer_set/set/tests/prop.rs b/zebra-network/src/peer_set/set/tests/prop.rs index 73c30ae65bd..f7053eeaefd 100644 --- a/zebra-network/src/peer_set/set/tests/prop.rs +++ b/zebra-network/src/peer_set/set/tests/prop.rs @@ -172,7 +172,7 @@ proptest! { /// Peerset will panic a if a rquest is sent and no more peers are available. #[test] #[should_panic(expected = "requests must be routed to at least one peer")] - fn peerset_always_broadcast( + fn peerset_always_broadcasts( total_number_of_peers in (2..10usize) ) { // Get a dummy block hash to help us construct a valid request to be broadcasted @@ -196,15 +196,14 @@ proptest! { let (minimum_peer_version, _best_tip_height) = MinimumPeerVersion::with_mock_chain_tip(Network::Mainnet); - // Build a peerset runtime.block_on(async move { + // Build a peerset let (mut peer_set, _peer_set_guard) = PeerSetBuilder::new() .with_discover(discovered_peers) .with_minimum_peer_version(minimum_peer_version.clone()) .build(); // Remove peers, test broadcast until there is only 1 peer left in the peerset - let mut new_number_of_peers = total_number_of_peers; for port in 1u16..total_number_of_peers as u16 { peer_set.remove(&SocketAddr::new([127, 0, 0, 1].into(), port)); handles.remove(0); @@ -233,12 +232,6 @@ proptest! { // Make sure the message was broadcasted to the right number of peers prop_assert_eq!(received, number_of_peers_to_broadcast); - - // Exit the loop when we have only 1 peer left - new_number_of_peers -= 1; - if new_number_of_peers == 1 { - break; - } } // Remove the last peer we have left in the peerset From 756d9b90a0b4621ef1a816dd15c6e221b9b820cf Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 1 Feb 2022 17:12:44 -0300 Subject: [PATCH 3/4] split the test into two --- zebra-network/src/peer_set/set/tests/prop.rs | 48 ++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/zebra-network/src/peer_set/set/tests/prop.rs b/zebra-network/src/peer_set/set/tests/prop.rs index f7053eeaefd..e2b03ac6e7c 100644 --- a/zebra-network/src/peer_set/set/tests/prop.rs +++ b/zebra-network/src/peer_set/set/tests/prop.rs @@ -169,9 +169,7 @@ proptest! { /// Test the peerset will always broadcast iff there is at least one /// peer in the set. - /// Peerset will panic a if a rquest is sent and no more peers are available. #[test] - #[should_panic(expected = "requests must be routed to at least one peer")] fn peerset_always_broadcasts( total_number_of_peers in (2..10usize) ) { @@ -230,10 +228,54 @@ proptest! { }; } - // Make sure the message was broadcasted to the right number of peers + // Make sure the message is always broadcasted to the right number of peers prop_assert_eq!(received, number_of_peers_to_broadcast); } + Ok::<_, TestCaseError>(()) + })?; + } + + /// Test the peerset panics if a request is sent and no more peers are available. + #[test] + #[should_panic(expected = "requests must be routed to at least one peer")] + fn panics_when_broadcasting_to_no_peers( + total_number_of_peers in (2..10usize) + ) { + // Get a dummy block hash to help us construct a valid request to be broadcasted + let block: block::Block = zebra_test::vectors::BLOCK_MAINNET_10_BYTES + .zcash_deserialize_into() + .unwrap(); + let block_hash = block::Hash::from(&block); + + // Start the runtime + let runtime = zebra_test::init_async(); + let _guard = runtime.enter(); + + // All peers will have the current version + let peer_versions = vec![CURRENT_NETWORK_PROTOCOL_VERSION; total_number_of_peers]; + let peer_versions = PeerVersions { + peer_versions, + }; + + // Get peers and handles + let (discovered_peers, mut handles) = peer_versions.mock_peer_discovery(); + let (minimum_peer_version, _best_tip_height) = + MinimumPeerVersion::with_mock_chain_tip(Network::Mainnet); + + runtime.block_on(async move { + // Build a peerset + let (mut peer_set, _peer_set_guard) = PeerSetBuilder::new() + .with_discover(discovered_peers) + .with_minimum_peer_version(minimum_peer_version.clone()) + .build(); + + // Remove peers + for port in 1u16..total_number_of_peers as u16 { + peer_set.remove(&SocketAddr::new([127, 0, 0, 1].into(), port)); + handles.remove(0); + } + // Remove the last peer we have left in the peerset peer_set.remove(&SocketAddr::new([127, 0, 0, 1].into(), total_number_of_peers as u16)); handles.remove(0); From 7875b65235faf05c677cb4bec76544add178fe67 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 1 Feb 2022 18:49:02 -0300 Subject: [PATCH 4/4] simplify some code Co-authored-by: Janito Vaqueiro Ferreira Filho --- zebra-network/src/peer_set/set/tests/prop.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/zebra-network/src/peer_set/set/tests/prop.rs b/zebra-network/src/peer_set/set/tests/prop.rs index e2b03ac6e7c..f7ab9e128df 100644 --- a/zebra-network/src/peer_set/set/tests/prop.rs +++ b/zebra-network/src/peer_set/set/tests/prop.rs @@ -271,15 +271,11 @@ proptest! { .build(); // Remove peers - for port in 1u16..total_number_of_peers as u16 { + for port in 1u16..=total_number_of_peers as u16 { peer_set.remove(&SocketAddr::new([127, 0, 0, 1].into(), port)); handles.remove(0); } - // Remove the last peer we have left in the peerset - peer_set.remove(&SocketAddr::new([127, 0, 0, 1].into(), total_number_of_peers as u16)); - handles.remove(0); - // this will panic as expected let _ = peer_set.route_broadcast(Request::AdvertiseBlock(block_hash));