From fa9ee1b2ce83eb1c98726b466d259c55f922d15d Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Tue, 9 Apr 2019 17:32:18 +0200 Subject: [PATCH 01/19] test random usage of peerset api --- Cargo.lock | 1 + core/peerset/Cargo.toml | 3 + core/peerset/src/lib.rs | 142 +++++++++++++++++++++++++++++++++++++++- 3 files changed, 145 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index e7afec8f6a79a..688857b4c93d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4038,6 +4038,7 @@ dependencies = [ "linked-hash-map 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "lru-cache 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "rand 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.38 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/core/peerset/Cargo.toml b/core/peerset/Cargo.toml index 1b505682d85cc..5622d942cf22a 100644 --- a/core/peerset/Cargo.toml +++ b/core/peerset/Cargo.toml @@ -14,3 +14,6 @@ linked-hash-map = "0.5" log = "0.4" lru-cache = "0.1.2" serde_json = "1.0.24" + +[dev-dependencies] +rand = "0.6.5" diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index 98287bf7a6336..0ce1cce5f7b6f 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -165,7 +165,7 @@ pub enum Message { } /// Opaque identifier for an incoming connection. Allocated by the network. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub struct IncomingIndex(pub u64); impl From for IncomingIndex { @@ -491,7 +491,10 @@ impl Stream for Peerset { mod tests { use libp2p::PeerId; use futures::prelude::*; + use rand::thread_rng; + use rand::seq::SliceRandom; use super::{PeersetConfig, Peerset, Message, IncomingIndex}; + use std::collections::{HashMap, HashSet, VecDeque}; fn assert_messages(mut peerset: Peerset, messages: Vec) -> Peerset { for expected_message in messages { @@ -511,6 +514,143 @@ mod tests { Ok((message, peerset)) } + #[test] + fn test_random_api_use() { + + #[derive(Debug)] + enum TestAction { + AddReservedPeer, + RemoveReservedPeer, + SetReservedOnly(bool), + ReportPeer(i32), + DropPeer, + } + + let bootnode = PeerId::random(); + let config = PeersetConfig { + in_peers: 100, + out_peers: 100, + bootnodes: vec![bootnode.clone()], + reserved_only: false, + reserved_nodes: Vec::new(), + }; + + let (mut peerset, handle) = Peerset::from_config(config); + + let mut actions = vec![]; + actions.push((bootnode.clone(), TestAction::AddReservedPeer)); + actions.push((bootnode.clone(), TestAction::SetReservedOnly(true))); + actions.push((bootnode.clone(), TestAction::ReportPeer(-1))); + actions.push((bootnode.clone(), TestAction::SetReservedOnly(false))); + actions.push((bootnode.clone(), TestAction::RemoveReservedPeer)); + actions.push((bootnode.clone(), TestAction::DropPeer)); + for _ in 0..100 { + let peer_id = PeerId::random(); + actions.push((peer_id.clone(), TestAction::AddReservedPeer)); + actions.push((peer_id.clone(), TestAction::SetReservedOnly(true))); + actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); + actions.push((peer_id.clone(), TestAction::SetReservedOnly(false))); + actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); + actions.push((peer_id.clone(), TestAction::DropPeer)); + } + + let mut dropped_called = HashSet::new(); + let mut performed_actions = HashMap::new(); + + let mut rng = thread_rng(); + for (peer_id, action) in actions.choose_multiple(&mut rng, actions.len()) { + match action { + TestAction::AddReservedPeer => { + handle.add_reserved_peer(peer_id.clone()); + }, + TestAction::RemoveReservedPeer => { + handle.remove_reserved_peer(peer_id.clone()); + }, + TestAction::SetReservedOnly(reserved) => { + handle.set_reserved_only(reserved.clone()); + }, + TestAction::ReportPeer(diff_score) => { + handle.report_peer(peer_id.clone(), diff_score.clone()); + }, + TestAction::DropPeer => { + peerset.dropped(peer_id.clone()); + dropped_called.insert(peer_id.clone()); + }, + } + let performed = performed_actions + .entry(peer_id.clone()) + .or_insert(VecDeque::new()); + performed.push_back(action) + } + + drop(handle); + + let mut last_connected_messages = HashMap::new(); + let mut last_unconnected_messages = HashMap::new(); + loop { + let message = match next_message(peerset) { + Ok((message, p)) => { + peerset = p; + message + }, + _ => break, + }; + match message { + Message::Connect(peer_id) => { + let last_message = last_connected_messages.get(&peer_id); + match last_message { + Some(Message::Drop(_)) => {}, + _ => { + if !dropped_called.remove(&peer_id) && !(peer_id == bootnode) && !last_unconnected_messages.len() > 0 { + panic!("Unexpected Connect message after a {:?} message", last_message); + } + }, + } + last_connected_messages.insert(peer_id.clone(), Message::Connect(peer_id)); + }, + Message::Drop(peer_id) => { + let maybe_related_action = { + if let Some(actions) = performed_actions.get_mut(&peer_id) { + actions.pop_front() + } else { + None + } + }; + let last_message = last_connected_messages + .get(&peer_id); + match last_message { + Some(Message::Connect(_)) => {}, + _ => { + println!("Last unconnected: {:?}", last_unconnected_messages); + if !last_unconnected_messages.len() > 0 { + panic!("Unexpected Drop message, after a {:?} message, a perhaps related action was: {:?}", last_message, maybe_related_action); + } + }, + } + last_connected_messages.insert(peer_id.clone(), Message::Drop(peer_id)); + }, + Message::Accept(index) => { + if let Some(last_message) = last_unconnected_messages.get(&index) { + match last_message { + Message::Connect(_) => panic!("Unexpected Accept message, after a Connect message"), + _ => {}, + } + } + last_unconnected_messages.insert(index.clone(), Message::Accept(index)); + }, + Message::Reject(index) => { + if let Some(last_message) = last_unconnected_messages.get(&index) { + match last_message { + Message::Connect(_) => panic!("Unexpected Reject message, after a Connect message"), + _ => {}, + } + } + last_unconnected_messages.insert(index.clone(), Message::Reject(index)); + }, + } + } + } + #[test] fn test_peerset_from_config_with_bootnodes() { let bootnode = PeerId::random(); From e0a6821abe2b68946f03a4c620ca0a51d7041cf9 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Wed, 10 Apr 2019 11:31:42 +0200 Subject: [PATCH 02/19] add more api test actions --- core/peerset/src/lib.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index 0ce1cce5f7b6f..46f2866f34367 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -524,6 +524,8 @@ mod tests { SetReservedOnly(bool), ReportPeer(i32), DropPeer, + Incoming(IncomingIndex), + Discovered(Vec), } let bootnode = PeerId::random(); @@ -538,23 +540,33 @@ mod tests { let (mut peerset, handle) = Peerset::from_config(config); let mut actions = vec![]; + let mut discovered = vec![]; + discovered.push(bootnode.clone()); + actions.push((bootnode.clone(), TestAction::AddReservedPeer)); actions.push((bootnode.clone(), TestAction::SetReservedOnly(true))); actions.push((bootnode.clone(), TestAction::ReportPeer(-1))); actions.push((bootnode.clone(), TestAction::SetReservedOnly(false))); actions.push((bootnode.clone(), TestAction::RemoveReservedPeer)); actions.push((bootnode.clone(), TestAction::DropPeer)); - for _ in 0..100 { + actions.push((bootnode.clone(), TestAction::Incoming(IncomingIndex(1)))); + actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); + + for i in 0..100 { let peer_id = PeerId::random(); + discovered.push(peer_id.clone()); actions.push((peer_id.clone(), TestAction::AddReservedPeer)); actions.push((peer_id.clone(), TestAction::SetReservedOnly(true))); actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); actions.push((peer_id.clone(), TestAction::SetReservedOnly(false))); actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); actions.push((peer_id.clone(), TestAction::DropPeer)); + actions.push((bootnode.clone(), TestAction::Incoming(IncomingIndex(i)))); + actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); } let mut dropped_called = HashSet::new(); + let mut discovered_called = HashSet::new(); let mut performed_actions = HashMap::new(); let mut rng = thread_rng(); @@ -576,6 +588,15 @@ mod tests { peerset.dropped(peer_id.clone()); dropped_called.insert(peer_id.clone()); }, + TestAction::Incoming(index) => { + peerset.incoming(peer_id.clone(), index.clone()); + }, + TestAction::Discovered(nodes) => { + peerset.discovered(nodes.clone()); + for node in nodes { + discovered_called.insert(node.clone()); + } + }, } let performed = performed_actions .entry(peer_id.clone()) @@ -601,7 +622,8 @@ mod tests { match last_message { Some(Message::Drop(_)) => {}, _ => { - if !dropped_called.remove(&peer_id) && !(peer_id == bootnode) && !last_unconnected_messages.len() > 0 { + let relevant_api_called = dropped_called.remove(&peer_id) || discovered_called.remove(&peer_id); + if !relevant_api_called && !(peer_id == bootnode) && !last_unconnected_messages.len() > 0 { panic!("Unexpected Connect message after a {:?} message", last_message); } }, From 4afbc54c11b1cb0a06aa77023f65822c9e18f70c Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Wed, 10 Apr 2019 11:34:35 +0200 Subject: [PATCH 03/19] nits --- core/peerset/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index 46f2866f34367..a9fcda9fc08d2 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -638,12 +638,10 @@ mod tests { None } }; - let last_message = last_connected_messages - .get(&peer_id); + let last_message = last_connected_messages.get(&peer_id); match last_message { Some(Message::Connect(_)) => {}, _ => { - println!("Last unconnected: {:?}", last_unconnected_messages); if !last_unconnected_messages.len() > 0 { panic!("Unexpected Drop message, after a {:?} message, a perhaps related action was: {:?}", last_message, maybe_related_action); } From 1792868f0d180a879facfe5429d6548042aeecad Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Wed, 10 Apr 2019 11:37:49 +0200 Subject: [PATCH 04/19] notes --- core/peerset/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index a9fcda9fc08d2..fcb84d008b26c 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -651,6 +651,7 @@ mod tests { }, Message::Accept(index) => { if let Some(last_message) = last_unconnected_messages.get(&index) { + // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... match last_message { Message::Connect(_) => panic!("Unexpected Accept message, after a Connect message"), _ => {}, @@ -660,6 +661,7 @@ mod tests { }, Message::Reject(index) => { if let Some(last_message) = last_unconnected_messages.get(&index) { + // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... match last_message { Message::Connect(_) => panic!("Unexpected Reject message, after a Connect message"), _ => {}, From 419085f8228ef3e29facab74e376fbca270fcf35 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Wed, 10 Apr 2019 11:39:21 +0200 Subject: [PATCH 05/19] move note --- core/peerset/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index fcb84d008b26c..4be59044027aa 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -651,8 +651,8 @@ mod tests { }, Message::Accept(index) => { if let Some(last_message) = last_unconnected_messages.get(&index) { - // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... match last_message { + // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... Message::Connect(_) => panic!("Unexpected Accept message, after a Connect message"), _ => {}, } @@ -661,8 +661,8 @@ mod tests { }, Message::Reject(index) => { if let Some(last_message) = last_unconnected_messages.get(&index) { - // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... match last_message { + // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... Message::Connect(_) => panic!("Unexpected Reject message, after a Connect message"), _ => {}, } From 636ee239eecb9be41e60e05b9afc037ad2e5bd2f Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Wed, 10 Apr 2019 12:03:48 +0200 Subject: [PATCH 06/19] add actions for more peers than supported --- core/peerset/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index 4be59044027aa..eb7b9811b74b6 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -552,7 +552,7 @@ mod tests { actions.push((bootnode.clone(), TestAction::Incoming(IncomingIndex(1)))); actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); - for i in 0..100 { + for i in 0..125 { let peer_id = PeerId::random(); discovered.push(peer_id.clone()); actions.push((peer_id.clone(), TestAction::AddReservedPeer)); From f7cff2de4f8df2b4ffe9c7cde5f4338ca2632cc9 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Wed, 10 Apr 2019 12:06:36 +0200 Subject: [PATCH 07/19] add actions for unknonw peers --- core/peerset/src/lib.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index eb7b9811b74b6..a6c6e75a6b23f 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -565,6 +565,16 @@ mod tests { actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); } + for _ in 0..25 { + // Actions for peers that do not include a AddReservedPeer. + let peer_id = PeerId::random(); + discovered.push(peer_id.clone()); + actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); + actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); + actions.push((peer_id.clone(), TestAction::DropPeer)); + actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); + } + let mut dropped_called = HashSet::new(); let mut discovered_called = HashSet::new(); let mut performed_actions = HashMap::new(); From 98108e78054185a3ac676ba75c0a38e9eba690f0 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Wed, 10 Apr 2019 12:16:44 +0200 Subject: [PATCH 08/19] use proper peer id --- core/peerset/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index a6c6e75a6b23f..86e7fc044e419 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -561,8 +561,8 @@ mod tests { actions.push((peer_id.clone(), TestAction::SetReservedOnly(false))); actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); actions.push((peer_id.clone(), TestAction::DropPeer)); - actions.push((bootnode.clone(), TestAction::Incoming(IncomingIndex(i)))); - actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); + actions.push((peer_id.clone(), TestAction::Incoming(IncomingIndex(i)))); + actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); } for _ in 0..25 { @@ -572,7 +572,7 @@ mod tests { actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); actions.push((peer_id.clone(), TestAction::DropPeer)); - actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); + actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); } let mut dropped_called = HashSet::new(); From 49aeac2a14f9f4a0d42b916fe107144052c912ea Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Wed, 10 Apr 2019 20:11:40 +0200 Subject: [PATCH 09/19] add test folder to peerset --- core/peerset/src/lib.rs | 191 +++-------------------------------- core/peerset/src/test/mod.rs | 175 ++++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 178 deletions(-) create mode 100644 core/peerset/src/test/mod.rs diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index 86e7fc044e419..92450858be814 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -28,6 +28,9 @@ use lru_cache::LruCache; use slots::{SlotType, SlotState, Slots}; pub use serde_json::Value; +#[cfg(test)] +mod test; + const PEERSET_SCORES_CACHE_SIZE: usize = 1000; /// FIFO-ordered list of nodes that we know exist, but we are not connected to. @@ -487,13 +490,22 @@ impl Stream for Peerset { } } +#[cfg(test)] +pub fn next_message(peerset: Peerset) -> Result<(Message, Peerset), ()> { + let (next, peerset) = peerset.into_future() + .wait() + .map_err(|_| ())?; + let message = next.ok_or_else(|| ())?; + Ok((message, peerset)) +} + #[cfg(test)] mod tests { use libp2p::PeerId; use futures::prelude::*; use rand::thread_rng; use rand::seq::SliceRandom; - use super::{PeersetConfig, Peerset, Message, IncomingIndex}; + use super::{PeersetConfig, Peerset, Message, IncomingIndex, next_message}; use std::collections::{HashMap, HashSet, VecDeque}; fn assert_messages(mut peerset: Peerset, messages: Vec) -> Peerset { @@ -506,183 +518,6 @@ mod tests { peerset } - fn next_message(peerset: Peerset) -> Result<(Message, Peerset), ()> { - let (next, peerset) = peerset.into_future() - .wait() - .map_err(|_| ())?; - let message = next.ok_or_else(|| ())?; - Ok((message, peerset)) - } - - #[test] - fn test_random_api_use() { - - #[derive(Debug)] - enum TestAction { - AddReservedPeer, - RemoveReservedPeer, - SetReservedOnly(bool), - ReportPeer(i32), - DropPeer, - Incoming(IncomingIndex), - Discovered(Vec), - } - - let bootnode = PeerId::random(); - let config = PeersetConfig { - in_peers: 100, - out_peers: 100, - bootnodes: vec![bootnode.clone()], - reserved_only: false, - reserved_nodes: Vec::new(), - }; - - let (mut peerset, handle) = Peerset::from_config(config); - - let mut actions = vec![]; - let mut discovered = vec![]; - discovered.push(bootnode.clone()); - - actions.push((bootnode.clone(), TestAction::AddReservedPeer)); - actions.push((bootnode.clone(), TestAction::SetReservedOnly(true))); - actions.push((bootnode.clone(), TestAction::ReportPeer(-1))); - actions.push((bootnode.clone(), TestAction::SetReservedOnly(false))); - actions.push((bootnode.clone(), TestAction::RemoveReservedPeer)); - actions.push((bootnode.clone(), TestAction::DropPeer)); - actions.push((bootnode.clone(), TestAction::Incoming(IncomingIndex(1)))); - actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); - - for i in 0..125 { - let peer_id = PeerId::random(); - discovered.push(peer_id.clone()); - actions.push((peer_id.clone(), TestAction::AddReservedPeer)); - actions.push((peer_id.clone(), TestAction::SetReservedOnly(true))); - actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); - actions.push((peer_id.clone(), TestAction::SetReservedOnly(false))); - actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); - actions.push((peer_id.clone(), TestAction::DropPeer)); - actions.push((peer_id.clone(), TestAction::Incoming(IncomingIndex(i)))); - actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); - } - - for _ in 0..25 { - // Actions for peers that do not include a AddReservedPeer. - let peer_id = PeerId::random(); - discovered.push(peer_id.clone()); - actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); - actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); - actions.push((peer_id.clone(), TestAction::DropPeer)); - actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); - } - - let mut dropped_called = HashSet::new(); - let mut discovered_called = HashSet::new(); - let mut performed_actions = HashMap::new(); - - let mut rng = thread_rng(); - for (peer_id, action) in actions.choose_multiple(&mut rng, actions.len()) { - match action { - TestAction::AddReservedPeer => { - handle.add_reserved_peer(peer_id.clone()); - }, - TestAction::RemoveReservedPeer => { - handle.remove_reserved_peer(peer_id.clone()); - }, - TestAction::SetReservedOnly(reserved) => { - handle.set_reserved_only(reserved.clone()); - }, - TestAction::ReportPeer(diff_score) => { - handle.report_peer(peer_id.clone(), diff_score.clone()); - }, - TestAction::DropPeer => { - peerset.dropped(peer_id.clone()); - dropped_called.insert(peer_id.clone()); - }, - TestAction::Incoming(index) => { - peerset.incoming(peer_id.clone(), index.clone()); - }, - TestAction::Discovered(nodes) => { - peerset.discovered(nodes.clone()); - for node in nodes { - discovered_called.insert(node.clone()); - } - }, - } - let performed = performed_actions - .entry(peer_id.clone()) - .or_insert(VecDeque::new()); - performed.push_back(action) - } - - drop(handle); - - let mut last_connected_messages = HashMap::new(); - let mut last_unconnected_messages = HashMap::new(); - loop { - let message = match next_message(peerset) { - Ok((message, p)) => { - peerset = p; - message - }, - _ => break, - }; - match message { - Message::Connect(peer_id) => { - let last_message = last_connected_messages.get(&peer_id); - match last_message { - Some(Message::Drop(_)) => {}, - _ => { - let relevant_api_called = dropped_called.remove(&peer_id) || discovered_called.remove(&peer_id); - if !relevant_api_called && !(peer_id == bootnode) && !last_unconnected_messages.len() > 0 { - panic!("Unexpected Connect message after a {:?} message", last_message); - } - }, - } - last_connected_messages.insert(peer_id.clone(), Message::Connect(peer_id)); - }, - Message::Drop(peer_id) => { - let maybe_related_action = { - if let Some(actions) = performed_actions.get_mut(&peer_id) { - actions.pop_front() - } else { - None - } - }; - let last_message = last_connected_messages.get(&peer_id); - match last_message { - Some(Message::Connect(_)) => {}, - _ => { - if !last_unconnected_messages.len() > 0 { - panic!("Unexpected Drop message, after a {:?} message, a perhaps related action was: {:?}", last_message, maybe_related_action); - } - }, - } - last_connected_messages.insert(peer_id.clone(), Message::Drop(peer_id)); - }, - Message::Accept(index) => { - if let Some(last_message) = last_unconnected_messages.get(&index) { - match last_message { - // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... - Message::Connect(_) => panic!("Unexpected Accept message, after a Connect message"), - _ => {}, - } - } - last_unconnected_messages.insert(index.clone(), Message::Accept(index)); - }, - Message::Reject(index) => { - if let Some(last_message) = last_unconnected_messages.get(&index) { - match last_message { - // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... - Message::Connect(_) => panic!("Unexpected Reject message, after a Connect message"), - _ => {}, - } - } - last_unconnected_messages.insert(index.clone(), Message::Reject(index)); - }, - } - } - } - #[test] fn test_peerset_from_config_with_bootnodes() { let bootnode = PeerId::random(); diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs new file mode 100644 index 0000000000000..68c308f94b2b4 --- /dev/null +++ b/core/peerset/src/test/mod.rs @@ -0,0 +1,175 @@ +use libp2p::PeerId; +use futures::prelude::*; +use rand::thread_rng; +use rand::seq::SliceRandom; +use super::{PeersetConfig, Peerset, Message, IncomingIndex, next_message}; +use std::collections::{HashMap, HashSet, VecDeque}; + +#[test] +fn test_random_api_use() { + + #[derive(Debug)] + enum TestAction { + AddReservedPeer, + RemoveReservedPeer, + SetReservedOnly(bool), + ReportPeer(i32), + DropPeer, + Incoming(IncomingIndex), + Discovered(Vec), + } + + let bootnode = PeerId::random(); + let config = PeersetConfig { + in_peers: 100, + out_peers: 100, + bootnodes: vec![bootnode.clone()], + reserved_only: false, + reserved_nodes: Vec::new(), + }; + + let (mut peerset, handle) = Peerset::from_config(config); + + let mut actions = vec![]; + let mut discovered = vec![]; + discovered.push(bootnode.clone()); + + actions.push((bootnode.clone(), TestAction::AddReservedPeer)); + actions.push((bootnode.clone(), TestAction::SetReservedOnly(true))); + actions.push((bootnode.clone(), TestAction::ReportPeer(-1))); + actions.push((bootnode.clone(), TestAction::SetReservedOnly(false))); + actions.push((bootnode.clone(), TestAction::RemoveReservedPeer)); + actions.push((bootnode.clone(), TestAction::DropPeer)); + actions.push((bootnode.clone(), TestAction::Incoming(IncomingIndex(1)))); + actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); + + for i in 0..125 { + let peer_id = PeerId::random(); + discovered.push(peer_id.clone()); + actions.push((peer_id.clone(), TestAction::AddReservedPeer)); + actions.push((peer_id.clone(), TestAction::SetReservedOnly(true))); + actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); + actions.push((peer_id.clone(), TestAction::SetReservedOnly(false))); + actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); + actions.push((peer_id.clone(), TestAction::DropPeer)); + actions.push((peer_id.clone(), TestAction::Incoming(IncomingIndex(i)))); + actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); + } + + for _ in 0..25 { + // Actions for peers that do not include a AddReservedPeer. + let peer_id = PeerId::random(); + discovered.push(peer_id.clone()); + actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); + actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); + actions.push((peer_id.clone(), TestAction::DropPeer)); + actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); + } + + let mut dropped_called = HashSet::new(); + let mut discovered_called = HashSet::new(); + let mut performed_actions = HashMap::new(); + + let mut rng = thread_rng(); + for (peer_id, action) in actions.choose_multiple(&mut rng, actions.len()) { + match action { + TestAction::AddReservedPeer => { + handle.add_reserved_peer(peer_id.clone()); + }, + TestAction::RemoveReservedPeer => { + handle.remove_reserved_peer(peer_id.clone()); + }, + TestAction::SetReservedOnly(reserved) => { + handle.set_reserved_only(reserved.clone()); + }, + TestAction::ReportPeer(diff_score) => { + handle.report_peer(peer_id.clone(), diff_score.clone()); + }, + TestAction::DropPeer => { + peerset.dropped(peer_id.clone()); + dropped_called.insert(peer_id.clone()); + }, + TestAction::Incoming(index) => { + peerset.incoming(peer_id.clone(), index.clone()); + }, + TestAction::Discovered(nodes) => { + peerset.discovered(nodes.clone()); + for node in nodes { + discovered_called.insert(node.clone()); + } + }, + } + let performed = performed_actions + .entry(peer_id.clone()) + .or_insert(VecDeque::new()); + performed.push_back(action) + } + + drop(handle); + + let mut last_connected_messages = HashMap::new(); + let mut last_unconnected_messages = HashMap::new(); + loop { + let message = match next_message(peerset) { + Ok((message, p)) => { + peerset = p; + message + }, + _ => break, + }; + match message { + Message::Connect(peer_id) => { + let last_message = last_connected_messages.get(&peer_id); + match last_message { + Some(Message::Drop(_)) => {}, + _ => { + let relevant_api_called = dropped_called.remove(&peer_id) || discovered_called.remove(&peer_id); + if !relevant_api_called && !(peer_id == bootnode) && !last_unconnected_messages.len() > 0 { + panic!("Unexpected Connect message after a {:?} message", last_message); + } + }, + } + last_connected_messages.insert(peer_id.clone(), Message::Connect(peer_id)); + }, + Message::Drop(peer_id) => { + let maybe_related_action = { + if let Some(actions) = performed_actions.get_mut(&peer_id) { + actions.pop_front() + } else { + None + } + }; + let last_message = last_connected_messages.get(&peer_id); + match last_message { + Some(Message::Connect(_)) => {}, + _ => { + if !last_unconnected_messages.len() > 0 { + panic!("Unexpected Drop message, after a {:?} message, a perhaps related action was: {:?}", last_message, maybe_related_action); + } + }, + } + last_connected_messages.insert(peer_id.clone(), Message::Drop(peer_id)); + }, + Message::Accept(index) => { + if let Some(last_message) = last_unconnected_messages.get(&index) { + match last_message { + // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... + Message::Connect(_) => panic!("Unexpected Accept message, after a Connect message"), + _ => {}, + } + } + last_unconnected_messages.insert(index.clone(), Message::Accept(index)); + }, + Message::Reject(index) => { + if let Some(last_message) = last_unconnected_messages.get(&index) { + match last_message { + // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... + Message::Connect(_) => panic!("Unexpected Reject message, after a Connect message"), + _ => {}, + } + } + last_unconnected_messages.insert(index.clone(), Message::Reject(index)); + }, + } + } +} From 4627ce27550baad7d8b95cbb4d846903c066838f Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Thu, 11 Apr 2019 13:10:45 +0200 Subject: [PATCH 10/19] deal with incoming index --- core/peerset/src/lib.rs | 13 +++--- core/peerset/src/slots.rs | 1 + core/peerset/src/test/mod.rs | 91 +++++++++++++++++------------------- 3 files changed, 51 insertions(+), 54 deletions(-) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index 92450858be814..53f00e77d2a55 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -337,7 +337,9 @@ impl Peerset { fn alloc_slots(&mut self) { while let Some((peer_id, slot_type)) = self.data.discovered.pop_peer(self.data.reserved_only) { - match self.data.out_slots.add_peer(peer_id, slot_type) { + let state = self.data.out_slots.add_peer(peer_id, slot_type); + println!("State: {:?}", state); + match state { SlotState::Added(peer_id) => { self.message_queue.push_back(Message::Connect(peer_id)); }, @@ -377,6 +379,7 @@ impl Peerset { // a) it is not reserved, so we reject the connection // b) we are already connected to it, so we reject the connection if self.data.reserved_only && !self.data.discovered.is_reserved(&peer_id) { + println!("Reject: {:?} {:?}", peer_id, index); self.message_queue.push_back(Message::Reject(index)); return; } @@ -393,7 +396,9 @@ impl Peerset { SlotType::Common }; - match self.data.in_slots.add_peer(peer_id, slot_type) { + let state = self.data.in_slots.add_peer(peer_id, slot_type); + println!("State 2: {:?} Index: {:?}", state, index); + match state { SlotState::Added(peer_id) => { // reserved node may have been previously stored as normal node in discovered list self.data.discovered.remove_peer(&peer_id); @@ -502,11 +507,7 @@ pub fn next_message(peerset: Peerset) -> Result<(Message, Peerset), ()> { #[cfg(test)] mod tests { use libp2p::PeerId; - use futures::prelude::*; - use rand::thread_rng; - use rand::seq::SliceRandom; use super::{PeersetConfig, Peerset, Message, IncomingIndex, next_message}; - use std::collections::{HashMap, HashSet, VecDeque}; fn assert_messages(mut peerset: Peerset, messages: Vec) -> Peerset { for expected_message in messages { diff --git a/core/peerset/src/slots.rs b/core/peerset/src/slots.rs index 7fa655d7ff608..a46e1616bfb82 100644 --- a/core/peerset/src/slots.rs +++ b/core/peerset/src/slots.rs @@ -28,6 +28,7 @@ pub enum SlotType { Common, } +#[derive(Debug)] /// Descibes the result of `add_peer` action. pub enum SlotState { /// Returned when `add_peer` successfully adds a peer to the slot. diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index 68c308f94b2b4..693462810e826 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -1,5 +1,4 @@ use libp2p::PeerId; -use futures::prelude::*; use rand::thread_rng; use rand::seq::SliceRandom; use super::{PeersetConfig, Peerset, Message, IncomingIndex, next_message}; @@ -32,6 +31,7 @@ fn test_random_api_use() { let mut actions = vec![]; let mut discovered = vec![]; + let mut index_to_peer = HashMap::new(); discovered.push(bootnode.clone()); actions.push((bootnode.clone(), TestAction::AddReservedPeer)); @@ -43,27 +43,31 @@ fn test_random_api_use() { actions.push((bootnode.clone(), TestAction::Incoming(IncomingIndex(1)))); actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); - for i in 0..125 { + for i in 0..150 { let peer_id = PeerId::random(); + let index = IncomingIndex(i); + println!("Peer: {:?} {:?}", peer_id, index); + index_to_peer.insert(index, peer_id.clone()); discovered.push(peer_id.clone()); - actions.push((peer_id.clone(), TestAction::AddReservedPeer)); - actions.push((peer_id.clone(), TestAction::SetReservedOnly(true))); - actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); - actions.push((peer_id.clone(), TestAction::SetReservedOnly(false))); - actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); - actions.push((peer_id.clone(), TestAction::DropPeer)); - actions.push((peer_id.clone(), TestAction::Incoming(IncomingIndex(i)))); - actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); - } - - for _ in 0..25 { - // Actions for peers that do not include a AddReservedPeer. - let peer_id = PeerId::random(); - discovered.push(peer_id.clone()); - actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); - actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); - actions.push((peer_id.clone(), TestAction::DropPeer)); - actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); + if i > 75 { + // Includes AddReservedPeer. + actions.push((peer_id.clone(), TestAction::AddReservedPeer)); + actions.push((peer_id.clone(), TestAction::SetReservedOnly(true))); + actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); + actions.push((peer_id.clone(), TestAction::SetReservedOnly(false))); + actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); + actions.push((peer_id.clone(), TestAction::DropPeer)); + actions.push((peer_id.clone(), TestAction::Incoming(index))); + actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); + } else { + actions.push((peer_id.clone(), TestAction::SetReservedOnly(true))); + actions.push((peer_id.clone(), TestAction::ReportPeer(-1))); + actions.push((peer_id.clone(), TestAction::SetReservedOnly(false))); + actions.push((peer_id.clone(), TestAction::RemoveReservedPeer)); + actions.push((peer_id.clone(), TestAction::DropPeer)); + actions.push((peer_id.clone(), TestAction::Incoming(index))); + actions.push((peer_id.clone(), TestAction::Discovered(discovered.clone()))); + } } let mut dropped_called = HashSet::new(); @@ -107,11 +111,11 @@ fn test_random_api_use() { drop(handle); - let mut last_connected_messages = HashMap::new(); - let mut last_unconnected_messages = HashMap::new(); + let mut received_messages = HashMap::new(); loop { let message = match next_message(peerset) { Ok((message, p)) => { + println!("Message {:?}", message); peerset = p; message }, @@ -119,17 +123,17 @@ fn test_random_api_use() { }; match message { Message::Connect(peer_id) => { - let last_message = last_connected_messages.get(&peer_id); + let last_message = received_messages.get(&peer_id); match last_message { - Some(Message::Drop(_)) => {}, + Some(Message::Drop(_)) | Some(Message::Reject(_)) => {}, _ => { let relevant_api_called = dropped_called.remove(&peer_id) || discovered_called.remove(&peer_id); - if !relevant_api_called && !(peer_id == bootnode) && !last_unconnected_messages.len() > 0 { + if !relevant_api_called && !(peer_id == bootnode) { panic!("Unexpected Connect message after a {:?} message", last_message); } }, } - last_connected_messages.insert(peer_id.clone(), Message::Connect(peer_id)); + received_messages.insert(peer_id.clone(), Message::Connect(peer_id)); }, Message::Drop(peer_id) => { let maybe_related_action = { @@ -139,36 +143,27 @@ fn test_random_api_use() { None } }; - let last_message = last_connected_messages.get(&peer_id); + let last_message = received_messages.get(&peer_id); match last_message { - Some(Message::Connect(_)) => {}, - _ => { - if !last_unconnected_messages.len() > 0 { - panic!("Unexpected Drop message, after a {:?} message, a perhaps related action was: {:?}", last_message, maybe_related_action); - } - }, + Some(Message::Connect(_)) | Some(Message::Accept(_)) => {}, + _ => panic!("Unexpected Drop message, after a {:?} message, a perhaps related action was: {:?}", last_message, maybe_related_action), } - last_connected_messages.insert(peer_id.clone(), Message::Drop(peer_id)); + received_messages.insert(peer_id.clone(), Message::Drop(peer_id)); }, Message::Accept(index) => { - if let Some(last_message) = last_unconnected_messages.get(&index) { - match last_message { - // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... - Message::Connect(_) => panic!("Unexpected Accept message, after a Connect message"), - _ => {}, - } + let peer_id = index_to_peer.get(&index).expect("Unknown index"); + println!("ID: {:?}", peer_id); + if let Some(Message::Connect(_)) = received_messages.get(&peer_id) { + panic!("Unexpected Accept message, after a Connect message"); } - last_unconnected_messages.insert(index.clone(), Message::Accept(index)); + received_messages.insert(peer_id.clone(), Message::Accept(index)); }, Message::Reject(index) => { - if let Some(last_message) = last_unconnected_messages.get(&index) { - match last_message { - // Note: this will never hit, since Connect messages are stored by peer_id in last_connected_message... - Message::Connect(_) => panic!("Unexpected Reject message, after a Connect message"), - _ => {}, - } + let peer_id = index_to_peer.get(&index).expect("Unknown index"); + if let Some(Message::Connect(_)) = received_messages.get(&peer_id) { + panic!("Unexpected Reject message, after a Connect message"); } - last_unconnected_messages.insert(index.clone(), Message::Reject(index)); + received_messages.insert(peer_id.clone(), Message::Reject(index)); }, } } From 9aa82948109097a01f5f2f3ffcaa103215a01f1c Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Thu, 11 Apr 2019 14:10:04 +0200 Subject: [PATCH 11/19] more check --- core/peerset/src/lib.rs | 6 ++++-- core/peerset/src/test/mod.rs | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index 53f00e77d2a55..8fc721d1ebd2f 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -396,8 +396,8 @@ impl Peerset { SlotType::Common }; - let state = self.data.in_slots.add_peer(peer_id, slot_type); - println!("State 2: {:?} Index: {:?}", state, index); + let state = self.data.in_slots.add_peer(peer_id.clone(), slot_type); + println!("State 2: {:?} ID: {:?} Index: {:?}", state, peer_id, index); match state { SlotState::Added(peer_id) => { // reserved node may have been previously stored as normal node in discovered list @@ -434,8 +434,10 @@ impl Peerset { "Dropping {:?}\nin_slots={:?}\nout_slots={:?}", peer_id, self.data.in_slots, self.data.out_slots ); + println!("Dropping {:?}", peer_id); // Automatically connect back if reserved. if self.data.in_slots.is_connected_and_reserved(&peer_id) || self.data.out_slots.is_connected_and_reserved(&peer_id) { + println!("reconnecting {:?}", peer_id); self.message_queue.push_back(Message::Connect(peer_id)); return; } diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index 693462810e826..e403d676136bb 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -32,18 +32,21 @@ fn test_random_api_use() { let mut actions = vec![]; let mut discovered = vec![]; let mut index_to_peer = HashMap::new(); - discovered.push(bootnode.clone()); + let bootnode_index = IncomingIndex(0); actions.push((bootnode.clone(), TestAction::AddReservedPeer)); actions.push((bootnode.clone(), TestAction::SetReservedOnly(true))); actions.push((bootnode.clone(), TestAction::ReportPeer(-1))); actions.push((bootnode.clone(), TestAction::SetReservedOnly(false))); actions.push((bootnode.clone(), TestAction::RemoveReservedPeer)); actions.push((bootnode.clone(), TestAction::DropPeer)); - actions.push((bootnode.clone(), TestAction::Incoming(IncomingIndex(1)))); + actions.push((bootnode.clone(), TestAction::Incoming(bootnode_index))); actions.push((bootnode.clone(), TestAction::Discovered(discovered.clone()))); - for i in 0..150 { + index_to_peer.insert(bootnode_index, bootnode.clone()); + discovered.push(bootnode.clone()); + + for i in 1..150 { let peer_id = PeerId::random(); let index = IncomingIndex(i); println!("Peer: {:?} {:?}", peer_id, index); From 82b693df43a571f3e733088f872e5d21468aaaa0 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Thu, 11 Apr 2019 14:30:00 +0200 Subject: [PATCH 12/19] change var name --- core/peerset/src/test/mod.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index e403d676136bb..cb642c8a9b4c8 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -114,7 +114,7 @@ fn test_random_api_use() { drop(handle); - let mut received_messages = HashMap::new(); + let mut last_received_messages = HashMap::new(); loop { let message = match next_message(peerset) { Ok((message, p)) => { @@ -126,7 +126,7 @@ fn test_random_api_use() { }; match message { Message::Connect(peer_id) => { - let last_message = received_messages.get(&peer_id); + let last_message = last_received_messages.get(&peer_id); match last_message { Some(Message::Drop(_)) | Some(Message::Reject(_)) => {}, _ => { @@ -136,7 +136,7 @@ fn test_random_api_use() { } }, } - received_messages.insert(peer_id.clone(), Message::Connect(peer_id)); + last_received_messages.insert(peer_id.clone(), Message::Connect(peer_id)); }, Message::Drop(peer_id) => { let maybe_related_action = { @@ -146,27 +146,27 @@ fn test_random_api_use() { None } }; - let last_message = received_messages.get(&peer_id); + let last_message = last_received_messages.get(&peer_id); match last_message { Some(Message::Connect(_)) | Some(Message::Accept(_)) => {}, _ => panic!("Unexpected Drop message, after a {:?} message, a perhaps related action was: {:?}", last_message, maybe_related_action), } - received_messages.insert(peer_id.clone(), Message::Drop(peer_id)); + last_received_messages.insert(peer_id.clone(), Message::Drop(peer_id)); }, Message::Accept(index) => { let peer_id = index_to_peer.get(&index).expect("Unknown index"); println!("ID: {:?}", peer_id); - if let Some(Message::Connect(_)) = received_messages.get(&peer_id) { + if let Some(Message::Connect(_)) = last_received_messages.get(&peer_id) { panic!("Unexpected Accept message, after a Connect message"); } - received_messages.insert(peer_id.clone(), Message::Accept(index)); + last_received_messages.insert(peer_id.clone(), Message::Accept(index)); }, Message::Reject(index) => { let peer_id = index_to_peer.get(&index).expect("Unknown index"); - if let Some(Message::Connect(_)) = received_messages.get(&peer_id) { + if let Some(Message::Connect(_)) = last_received_messages.get(&peer_id) { panic!("Unexpected Reject message, after a Connect message"); } - received_messages.insert(peer_id.clone(), Message::Reject(index)); + last_received_messages.insert(peer_id.clone(), Message::Reject(index)); }, } } From 742745fafc893d3d7b85dedf3f406f8c32efb8bc Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Thu, 11 Apr 2019 14:41:44 +0200 Subject: [PATCH 13/19] add full sequence of actions --- core/peerset/src/test/mod.rs | 64 +++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index cb642c8a9b4c8..2e91c060ad1de 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -3,11 +3,10 @@ use rand::thread_rng; use rand::seq::SliceRandom; use super::{PeersetConfig, Peerset, Message, IncomingIndex, next_message}; use std::collections::{HashMap, HashSet, VecDeque}; +use std::fmt::{self, Debug}; #[test] fn test_random_api_use() { - - #[derive(Debug)] enum TestAction { AddReservedPeer, RemoveReservedPeer, @@ -18,6 +17,34 @@ fn test_random_api_use() { Discovered(Vec), } + impl Debug for TestAction { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + TestAction::AddReservedPeer => { + write!(f, "AddReservedPeer") + }, + TestAction::RemoveReservedPeer => { + write!(f, "RemoveReservedPeer") + }, + TestAction::SetReservedOnly(reserved) => { + write!(f, "SetReservedOnly {:?}", reserved) + }, + TestAction::ReportPeer(diff_score) => { + write!(f, "ReportPeer {:?}", diff_score) + }, + TestAction::DropPeer => { + write!(f, "DropPeer") + }, + TestAction::Incoming(index) => { + write!(f, "Incoming {:?}", index) + }, + TestAction::Discovered(nodes) => { + write!(f, "Discovered {:?}", nodes.len()) + }, + } + } + } + let bootnode = PeerId::random(); let config = PeersetConfig { in_peers: 100, @@ -127,21 +154,28 @@ fn test_random_api_use() { match message { Message::Connect(peer_id) => { let last_message = last_received_messages.get(&peer_id); + let action_sequence = { + if let Some(actions) = performed_actions.get_mut(&peer_id) { + Some(actions.clone()) + } else { + None + } + }; match last_message { Some(Message::Drop(_)) | Some(Message::Reject(_)) => {}, _ => { let relevant_api_called = dropped_called.remove(&peer_id) || discovered_called.remove(&peer_id); if !relevant_api_called && !(peer_id == bootnode) { - panic!("Unexpected Connect message after a {:?} message", last_message); + panic!("Unexpected Connect message after a {:?} message, sequence of actions: {:?}", last_message, action_sequence); } }, } last_received_messages.insert(peer_id.clone(), Message::Connect(peer_id)); }, Message::Drop(peer_id) => { - let maybe_related_action = { + let action_sequence = { if let Some(actions) = performed_actions.get_mut(&peer_id) { - actions.pop_front() + Some(actions.clone()) } else { None } @@ -149,22 +183,36 @@ fn test_random_api_use() { let last_message = last_received_messages.get(&peer_id); match last_message { Some(Message::Connect(_)) | Some(Message::Accept(_)) => {}, - _ => panic!("Unexpected Drop message, after a {:?} message, a perhaps related action was: {:?}", last_message, maybe_related_action), + _ => panic!("Unexpected Drop message, after a {:?} message, sequence of actions: {:?}", last_message, action_sequence), } last_received_messages.insert(peer_id.clone(), Message::Drop(peer_id)); }, Message::Accept(index) => { let peer_id = index_to_peer.get(&index).expect("Unknown index"); + let action_sequence = { + if let Some(actions) = performed_actions.get_mut(&peer_id) { + Some(actions.clone()) + } else { + None + } + }; println!("ID: {:?}", peer_id); if let Some(Message::Connect(_)) = last_received_messages.get(&peer_id) { - panic!("Unexpected Accept message, after a Connect message"); + panic!("Unexpected Accept message, after a Connect message, sequence of actions: {:?}", action_sequence); } last_received_messages.insert(peer_id.clone(), Message::Accept(index)); }, Message::Reject(index) => { let peer_id = index_to_peer.get(&index).expect("Unknown index"); + let action_sequence = { + if let Some(actions) = performed_actions.get_mut(&peer_id) { + Some(actions.clone()) + } else { + None + } + }; if let Some(Message::Connect(_)) = last_received_messages.get(&peer_id) { - panic!("Unexpected Reject message, after a Connect message"); + panic!("Unexpected Reject message, after a Connect message, sequence of actions: {:?}", action_sequence); } last_received_messages.insert(peer_id.clone(), Message::Reject(index)); }, From 3040ad5f292eff5ef1c57c330c24993bf7ad2734 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Tue, 16 Apr 2019 17:00:20 +0200 Subject: [PATCH 14/19] check for presence of more than one action --- core/peerset/src/test/mod.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index 2e91c060ad1de..a3c73fcaee40c 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -7,6 +7,8 @@ use std::fmt::{self, Debug}; #[test] fn test_random_api_use() { + + #[derive(Eq, PartialEq)] enum TestAction { AddReservedPeer, RemoveReservedPeer, @@ -76,7 +78,6 @@ fn test_random_api_use() { for i in 1..150 { let peer_id = PeerId::random(); let index = IncomingIndex(i); - println!("Peer: {:?} {:?}", peer_id, index); index_to_peer.insert(index, peer_id.clone()); discovered.push(peer_id.clone()); if i > 75 { @@ -145,7 +146,6 @@ fn test_random_api_use() { loop { let message = match next_message(peerset) { Ok((message, p)) => { - println!("Message {:?}", message); peerset = p; message }, @@ -196,8 +196,20 @@ fn test_random_api_use() { None } }; - println!("ID: {:?}", peer_id); if let Some(Message::Connect(_)) = last_received_messages.get(&peer_id) { + if let Some(action_sequence) = action_sequence.clone() { + let mut actions = action_sequence.into_iter(); + let drop_position = actions.clone().rposition(|x| x == &TestAction::DropPeer); + let incoming_position = actions.rposition(|x| x == &TestAction::Incoming(index)); + match (drop_position, incoming_position) { + (Some(drop), Some(incoming)) => { + + assert!(drop < incoming); + continue + }, + _ => {} + } + } panic!("Unexpected Accept message, after a Connect message, sequence of actions: {:?}", action_sequence); } last_received_messages.insert(peer_id.clone(), Message::Accept(index)); From bdf8b58b39070422d5604f1c05b41f4c67c9c01b Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Tue, 16 Apr 2019 17:04:30 +0200 Subject: [PATCH 15/19] remove prints --- core/peerset/src/lib.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/core/peerset/src/lib.rs b/core/peerset/src/lib.rs index 8fc721d1ebd2f..037a4b1ad7a38 100644 --- a/core/peerset/src/lib.rs +++ b/core/peerset/src/lib.rs @@ -337,9 +337,7 @@ impl Peerset { fn alloc_slots(&mut self) { while let Some((peer_id, slot_type)) = self.data.discovered.pop_peer(self.data.reserved_only) { - let state = self.data.out_slots.add_peer(peer_id, slot_type); - println!("State: {:?}", state); - match state { + match self.data.out_slots.add_peer(peer_id, slot_type) { SlotState::Added(peer_id) => { self.message_queue.push_back(Message::Connect(peer_id)); }, @@ -379,7 +377,6 @@ impl Peerset { // a) it is not reserved, so we reject the connection // b) we are already connected to it, so we reject the connection if self.data.reserved_only && !self.data.discovered.is_reserved(&peer_id) { - println!("Reject: {:?} {:?}", peer_id, index); self.message_queue.push_back(Message::Reject(index)); return; } @@ -396,9 +393,7 @@ impl Peerset { SlotType::Common }; - let state = self.data.in_slots.add_peer(peer_id.clone(), slot_type); - println!("State 2: {:?} ID: {:?} Index: {:?}", state, peer_id, index); - match state { + match self.data.in_slots.add_peer(peer_id.clone(), slot_type) { SlotState::Added(peer_id) => { // reserved node may have been previously stored as normal node in discovered list self.data.discovered.remove_peer(&peer_id); @@ -434,10 +429,8 @@ impl Peerset { "Dropping {:?}\nin_slots={:?}\nout_slots={:?}", peer_id, self.data.in_slots, self.data.out_slots ); - println!("Dropping {:?}", peer_id); // Automatically connect back if reserved. if self.data.in_slots.is_connected_and_reserved(&peer_id) || self.data.out_slots.is_connected_and_reserved(&peer_id) { - println!("reconnecting {:?}", peer_id); self.message_queue.push_back(Message::Connect(peer_id)); return; } From 91d1936d2ce98c8c1496a4c831913d330da51749 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Tue, 16 Apr 2019 18:09:04 +0200 Subject: [PATCH 16/19] check for connect message --- core/peerset/Cargo.toml | 2 +- core/peerset/src/test/mod.rs | 50 +++++++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/core/peerset/Cargo.toml b/core/peerset/Cargo.toml index 75bbd045bcdc5..5622d942cf22a 100644 --- a/core/peerset/Cargo.toml +++ b/core/peerset/Cargo.toml @@ -13,7 +13,7 @@ libp2p = { version = "0.6.0", default-features = false } linked-hash-map = "0.5" log = "0.4" lru-cache = "0.1.2" +serde_json = "1.0.24" [dev-dependencies] rand = "0.6.5" -serde_json = "1.0.24" diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index f57322d1e21b1..e1d2df15f6235 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -142,7 +142,7 @@ fn test_random_api_use() { drop(handle); - let mut last_received_messages = HashMap::new(); + let mut last_received_messages: HashMap> = HashMap::new(); loop { let message = match next_message(peerset) { Ok((message, p)) => { @@ -153,7 +153,13 @@ fn test_random_api_use() { }; match message { Message::Connect(peer_id) => { - let last_message = last_received_messages.get(&peer_id); + let last_message = { + if let Some(messages) = last_received_messages.get_mut(&peer_id) { + messages.pop_front() + } else { + None + } + }; let action_sequence = { if let Some(actions) = performed_actions.get_mut(&peer_id) { Some(actions.clone()) @@ -170,7 +176,8 @@ fn test_random_api_use() { } }, } - last_received_messages.insert(peer_id.clone(), Message::Connect(peer_id)); + let received = last_received_messages.entry(peer_id.clone()).or_insert(VecDeque::new()); + received.push_back(Message::Connect(peer_id)); }, Message::Drop(peer_id) => { let action_sequence = { @@ -180,12 +187,19 @@ fn test_random_api_use() { None } }; - let last_message = last_received_messages.get(&peer_id); + let last_message = { + if let Some(messages) = last_received_messages.get_mut(&peer_id) { + messages.pop_front() + } else { + None + } + }; match last_message { Some(Message::Connect(_)) | Some(Message::Accept(_)) => {}, _ => panic!("Unexpected Drop message, after a {:?} message, sequence of actions: {:?}", last_message, action_sequence), } - last_received_messages.insert(peer_id.clone(), Message::Drop(peer_id)); + let received = last_received_messages.entry(peer_id.clone()).or_insert(VecDeque::new()); + received.push_back(Message::Drop(peer_id)); }, Message::Accept(index) => { let peer_id = index_to_peer.get(&index).expect("Unknown index"); @@ -196,7 +210,14 @@ fn test_random_api_use() { None } }; - if let Some(Message::Connect(_)) = last_received_messages.get(&peer_id) { + let last_messages = { + if let Some(messages) = last_received_messages.get_mut(&peer_id) { + messages + } else { + continue; + } + }; + if let Some(Message::Connect(_)) = last_messages.pop_front() { if let Some(action_sequence) = action_sequence.clone() { let mut actions = action_sequence.into_iter(); let drop_position = actions.clone().rposition(|x| x == &TestAction::DropPeer); @@ -204,6 +225,8 @@ fn test_random_api_use() { match (drop_position, incoming_position) { (Some(drop), Some(incoming)) => { assert!(drop < incoming); + println!("Last messages: {:?}", last_messages); + assert!(last_messages.contains(&Message::Connect(peer_id.clone()))); continue }, _ => {} @@ -211,7 +234,8 @@ fn test_random_api_use() { } panic!("Unexpected Accept message, after a Connect message, sequence of actions: {:?}", action_sequence); } - last_received_messages.insert(peer_id.clone(), Message::Accept(index)); + let received = last_received_messages.entry(peer_id.clone()).or_insert(VecDeque::new()); + received.push_back(Message::Accept(index)); }, Message::Reject(index) => { let peer_id = index_to_peer.get(&index).expect("Unknown index"); @@ -222,10 +246,18 @@ fn test_random_api_use() { None } }; - if let Some(Message::Connect(_)) = last_received_messages.get(&peer_id) { + let last_messages = { + if let Some(messages) = last_received_messages.get_mut(&peer_id) { + messages + } else { + continue; + } + }; + if let Some(Message::Connect(_)) = last_messages.pop_front() { panic!("Unexpected Reject message, after a Connect message, sequence of actions: {:?}", action_sequence); } - last_received_messages.insert(peer_id.clone(), Message::Reject(index)); + let received = last_received_messages.entry(peer_id.clone()).or_insert(VecDeque::new()); + received.push_back(Message::Reject(index)); }, } } From b41179831cfcc3f76c6696d8c9fc1d09b674c9e6 Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Mon, 6 May 2019 22:19:27 +0800 Subject: [PATCH 17/19] fix test logic, remove print statement --- core/peerset/src/test/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index e1d2df15f6235..ad35b13c720b7 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -195,7 +195,7 @@ fn test_random_api_use() { } }; match last_message { - Some(Message::Connect(_)) | Some(Message::Accept(_)) => {}, + Some(Message::Connect(_)) | Some(Message::Accept(_)) | None => {}, _ => panic!("Unexpected Drop message, after a {:?} message, sequence of actions: {:?}", last_message, action_sequence), } let received = last_received_messages.entry(peer_id.clone()).or_insert(VecDeque::new()); @@ -225,8 +225,6 @@ fn test_random_api_use() { match (drop_position, incoming_position) { (Some(drop), Some(incoming)) => { assert!(drop < incoming); - println!("Last messages: {:?}", last_messages); - assert!(last_messages.contains(&Message::Connect(peer_id.clone()))); continue }, _ => {} From d55169f3322a57a3142afbfb0a296daaa7fd3ebe Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Mon, 6 May 2019 22:47:55 +0800 Subject: [PATCH 18/19] allow for Drop after discovered had been called for a peer. --- core/peerset/src/test/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index ad35b13c720b7..dfe895a2be589 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -195,8 +195,12 @@ fn test_random_api_use() { } }; match last_message { - Some(Message::Connect(_)) | Some(Message::Accept(_)) | None => {}, - _ => panic!("Unexpected Drop message, after a {:?} message, sequence of actions: {:?}", last_message, action_sequence), + Some(Message::Connect(_)) | Some(Message::Accept(_)) => {}, + _ => { + if !discovered_called.remove(&peer_id) { + panic!("Unexpected Drop message for {:?}, after a {:?} message, sequence of actions: {:?}", peer_id, last_message, action_sequence) + } + }, } let received = last_received_messages.entry(peer_id.clone()).or_insert(VecDeque::new()); received.push_back(Message::Drop(peer_id)); From cc5336959bc4699011c8106138a4c7a7ac8d320c Mon Sep 17 00:00:00 2001 From: Gregory Terzian Date: Tue, 7 May 2019 00:17:34 +0800 Subject: [PATCH 19/19] fix bug in test --- core/peerset/src/test/mod.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/core/peerset/src/test/mod.rs b/core/peerset/src/test/mod.rs index dfe895a2be589..ad78ec4142ba8 100644 --- a/core/peerset/src/test/mod.rs +++ b/core/peerset/src/test/mod.rs @@ -196,11 +196,7 @@ fn test_random_api_use() { }; match last_message { Some(Message::Connect(_)) | Some(Message::Accept(_)) => {}, - _ => { - if !discovered_called.remove(&peer_id) { - panic!("Unexpected Drop message for {:?}, after a {:?} message, sequence of actions: {:?}", peer_id, last_message, action_sequence) - } - }, + _ => panic!("Unexpected Drop message for {:?}, after a {:?} message, sequence of actions: {:?}", peer_id, last_message, action_sequence) } let received = last_received_messages.entry(peer_id.clone()).or_insert(VecDeque::new()); received.push_back(Message::Drop(peer_id)); @@ -214,14 +210,14 @@ fn test_random_api_use() { None } }; - let last_messages = { + let last_message = { if let Some(messages) = last_received_messages.get_mut(&peer_id) { - messages + messages.pop_front() } else { - continue; + None } }; - if let Some(Message::Connect(_)) = last_messages.pop_front() { + if let Some(Message::Connect(_)) = last_message { if let Some(action_sequence) = action_sequence.clone() { let mut actions = action_sequence.into_iter(); let drop_position = actions.clone().rposition(|x| x == &TestAction::DropPeer); @@ -229,12 +225,10 @@ fn test_random_api_use() { match (drop_position, incoming_position) { (Some(drop), Some(incoming)) => { assert!(drop < incoming); - continue }, _ => {} } } - panic!("Unexpected Accept message, after a Connect message, sequence of actions: {:?}", action_sequence); } let received = last_received_messages.entry(peer_id.clone()).or_insert(VecDeque::new()); received.push_back(Message::Accept(index));