From 4b7956bfaada0f1994bcbb1882f113c26c168351 Mon Sep 17 00:00:00 2001 From: timorl Date: Fri, 25 Nov 2022 17:25:09 +0100 Subject: [PATCH 1/3] Unify MockMultiaddress --- .../src/network/manager/compatibility.rs | 2 +- .../src/network/manager/connections.rs | 9 +- .../src/network/manager/discovery.rs | 10 +- finality-aleph/src/network/manager/session.rs | 7 +- finality-aleph/src/network/mock.rs | 85 +++-------------- finality-aleph/src/network/service.rs | 84 ++++++++--------- .../src/testing/mocks/validator_network.rs | 86 ++++++++--------- finality-aleph/src/testing/network.rs | 28 +++--- .../validator_network/manager/direction.rs | 88 +++++++++--------- .../src/validator_network/manager/legacy.rs | 25 +++-- .../src/validator_network/manager/mod.rs | 23 +++-- finality-aleph/src/validator_network/mock.rs | 92 ++++++++++++++----- .../validator_network/protocols/handshake.rs | 74 +++++++-------- .../src/validator_network/protocols/v0/mod.rs | 52 +++++------ .../src/validator_network/protocols/v1/mod.rs | 48 +++++----- 15 files changed, 343 insertions(+), 370 deletions(-) diff --git a/finality-aleph/src/network/manager/compatibility.rs b/finality-aleph/src/network/manager/compatibility.rs index 8d489d34d6..5c68521dea 100644 --- a/finality-aleph/src/network/manager/compatibility.rs +++ b/finality-aleph/src/network/manager/compatibility.rs @@ -140,11 +140,11 @@ mod test { crypto::AuthorityVerifier, network::{ manager::{compatibility::MAX_AUTHENTICATION_SIZE, SessionHandler}, - mock::MockMultiaddress, NetworkIdentity, }, nodes::testing::new_pen, tcp_network::{testing::new_identity, TcpMultiaddress}, + testing::mocks::validator_network::MockMultiaddress, NodeIndex, SessionId, Version, }; diff --git a/finality-aleph/src/network/manager/connections.rs b/finality-aleph/src/network/manager/connections.rs index 0982acede3..4023a54ca4 100644 --- a/finality-aleph/src/network/manager/connections.rs +++ b/finality-aleph/src/network/manager/connections.rs @@ -56,10 +56,13 @@ mod tests { use std::collections::HashSet; use super::Connections; - use crate::{network::mock::MockPeerId, SessionId}; + use crate::{ + validator_network::mock::{random_keys, MockPublicKey}, + SessionId, + }; - fn random_peer_ids(num: usize) -> HashSet { - (0..num).map(|_| MockPeerId::random()).collect() + fn random_peer_ids(num: usize) -> HashSet { + random_keys(num).into_keys().collect() } #[test] diff --git a/finality-aleph/src/network/manager/discovery.rs b/finality-aleph/src/network/manager/discovery.rs index 4a4f1bb833..fd8942dcff 100644 --- a/finality-aleph/src/network/manager/discovery.rs +++ b/finality-aleph/src/network/manager/discovery.rs @@ -136,10 +136,8 @@ mod tests { use super::{Discovery, DiscoveryMessage}; use crate::{ - network::{ - manager::SessionHandler, - mock::{crypto_basics, MockMultiaddress, MockPeerId}, - }, + network::{manager::SessionHandler, mock::crypto_basics}, + testing::mocks::validator_network::{random_identity, MockMultiaddress}, SessionId, }; @@ -148,7 +146,7 @@ mod tests { fn addresses() -> Vec { (0..NUM_NODES) - .map(|_| MockMultiaddress::random_with_id(MockPeerId::random())) + .map(|_| random_identity().0[0].clone()) .collect() } @@ -177,7 +175,7 @@ mod tests { None, crypto_basics.1.clone(), SessionId(43), - vec![MockMultiaddress::random_with_id(MockPeerId::random())], + random_identity().0, ) .await .unwrap(); diff --git a/finality-aleph/src/network/manager/session.rs b/finality-aleph/src/network/manager/session.rs index 209ecc1658..3998f7b71c 100644 --- a/finality-aleph/src/network/manager/session.rs +++ b/finality-aleph/src/network/manager/session.rs @@ -275,9 +275,10 @@ mod tests { use super::{get_common_peer_id, Handler, HandlerError}; use crate::{ network::{ - mock::{crypto_basics, MockMultiaddress, MockNetworkIdentity, MockPeerId}, + mock::{crypto_basics, MockNetworkIdentity}, NetworkIdentity, }, + testing::mocks::validator_network::{random_identity, MockMultiaddress}, NodeIndex, SessionId, }; @@ -379,8 +380,8 @@ mod tests { async fn fails_to_create_with_non_unique_peer_id() { let mut crypto_basics = crypto_basics(NUM_NODES).await; let addresses = vec![ - MockMultiaddress::random_with_id(MockPeerId::random()), - MockMultiaddress::random_with_id(MockPeerId::random()), + random_identity().0[0].clone(), + random_identity().0[0].clone(), ]; assert!(matches!( Handler::new( diff --git a/finality-aleph/src/network/mock.rs b/finality-aleph/src/network/mock.rs index 885b5bbcea..2f42392da7 100644 --- a/finality-aleph/src/network/mock.rs +++ b/finality-aleph/src/network/mock.rs @@ -7,13 +7,11 @@ use std::{ use aleph_primitives::KEY_TYPE; use async_trait::async_trait; -use codec::{Decode, Encode}; use futures::{ channel::{mpsc, oneshot}, StreamExt, }; use parking_lot::Mutex; -use rand::random; use sp_keystore::{testing::KeyStore, CryptoStore}; use tokio::time::timeout; @@ -21,84 +19,31 @@ use crate::{ crypto::{AuthorityPen, AuthorityVerifier}, network::{ manager::VersionedAuthentication, AddressedData, ConnectionCommand, Event, EventStream, - Multiaddress, Network, NetworkIdentity, NetworkSender, NetworkServiceIO, PeerId, Protocol, + Multiaddress, Network, NetworkIdentity, NetworkSender, NetworkServiceIO, Protocol, }, + testing::mocks::validator_network::{random_identity, MockMultiaddress}, + validator_network::mock::MockPublicKey, AuthorityId, NodeIndex, }; -#[derive(PartialEq, Eq, Copy, Clone, Debug, Hash, Encode, Decode)] -pub struct MockPeerId(u32); - -impl MockPeerId { - pub fn random() -> Self { - MockPeerId(random()) - } -} -impl fmt::Display for MockPeerId { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -impl PeerId for MockPeerId {} - -#[derive(PartialEq, Eq, Clone, Debug, Hash, Encode, Decode)] -pub struct MockMultiaddress { - peer_id: Option, - address: u32, -} - -impl MockMultiaddress { - pub fn random_with_id(peer_id: MockPeerId) -> Self { - MockMultiaddress { - peer_id: Some(peer_id), - address: random(), - } - } -} - -impl Multiaddress for MockMultiaddress { - type PeerId = MockPeerId; - - fn get_peer_id(&self) -> Option { - self.peer_id - } - - fn add_matching_peer_id(mut self, peer_id: Self::PeerId) -> Option { - match self.peer_id { - Some(old_peer_id) => match old_peer_id == peer_id { - true => Some(self), - false => None, - }, - None => { - self.peer_id = Some(peer_id); - Some(self) - } - } - } -} - pub struct MockNetworkIdentity { addresses: Vec, - peer_id: MockPeerId, + peer_id: MockPublicKey, } impl MockNetworkIdentity { pub fn new() -> Self { - let peer_id = MockPeerId::random(); - let addresses = (0..3) - .map(|_| MockMultiaddress::random_with_id(peer_id)) - .collect(); + let (addresses, peer_id) = random_identity(); MockNetworkIdentity { addresses, peer_id } } } impl NetworkIdentity for MockNetworkIdentity { - type PeerId = MockPeerId; + type PeerId = MockPublicKey; type Multiaddress = MockMultiaddress; fn identity(&self) -> (Vec, Self::PeerId) { - (self.addresses.clone(), self.peer_id) + (self.addresses.clone(), self.peer_id.clone()) } } @@ -152,7 +97,7 @@ impl Default for Channel { } } -pub type MockEvent = Event; +pub type MockEvent = Event; pub type MockData = Vec; @@ -196,15 +141,15 @@ impl MockIO { pub struct MockEventStream(mpsc::UnboundedReceiver); #[async_trait] -impl EventStream for MockEventStream { +impl EventStream for MockEventStream { async fn next_event(&mut self) -> Option { self.0.next().await } } pub struct MockNetworkSender { - sender: mpsc::UnboundedSender<(Vec, MockPeerId, Protocol)>, - peer_id: MockPeerId, + sender: mpsc::UnboundedSender<(Vec, MockPublicKey, Protocol)>, + peer_id: MockPublicKey, protocol: Protocol, error: Result<(), MockSenderError>, } @@ -219,7 +164,7 @@ impl NetworkSender for MockNetworkSender { ) -> Result<(), MockSenderError> { self.error?; self.sender - .unbounded_send((data.into(), self.peer_id, self.protocol)) + .unbounded_send((data.into(), self.peer_id.clone(), self.protocol)) .unwrap(); Ok(()) } @@ -228,8 +173,8 @@ impl NetworkSender for MockNetworkSender { #[derive(Clone)] pub struct MockNetwork { pub add_reserved: Channel<(HashSet, Protocol)>, - pub remove_reserved: Channel<(HashSet, Protocol)>, - pub send_message: Channel<(Vec, MockPeerId, Protocol)>, + pub remove_reserved: Channel<(HashSet, Protocol)>, + pub send_message: Channel<(Vec, MockPublicKey, Protocol)>, pub event_sinks: Arc>>>, event_stream_taken_oneshot: Arc>>>, pub create_sender_errors: Arc>>, @@ -256,7 +201,7 @@ impl std::error::Error for MockSenderError {} impl Network for MockNetwork { type SenderError = MockSenderError; type NetworkSender = MockNetworkSender; - type PeerId = MockPeerId; + type PeerId = MockPublicKey; type Multiaddress = MockMultiaddress; type EventStream = MockEventStream; diff --git a/finality-aleph/src/network/service.rs b/finality-aleph/src/network/service.rs index 154ac69b12..6a39cd09a0 100644 --- a/finality-aleph/src/network/service.rs +++ b/finality-aleph/src/network/service.rs @@ -348,16 +348,12 @@ mod tests { use crate::{ network::{ manager::{SessionHandler, VersionedAuthentication}, - mock::{ - crypto_basics, MockData, MockEvent, MockIO, - MockMultiaddress as MockAuthMultiaddress, MockNetwork, - MockPeerId as MockAuthPeerId, MockSenderError, - }, + mock::{crypto_basics, MockData, MockEvent, MockIO, MockNetwork, MockSenderError}, testing::DiscoveryMessage, NetworkIdentity, Protocol, }, testing::mocks::validator_network::{ - random_authority_id, MockMultiaddress, MockNetwork as MockValidatorNetwork, + random_identity, random_peer_id, MockMultiaddress, MockNetwork as MockValidatorNetwork, }, SessionId, }; @@ -419,7 +415,7 @@ mod tests { // We do this only to make sure that NotificationStreamOpened/NotificationStreamClosed events are handled async fn wait_for_events_handled(&mut self) { - let address = MockAuthMultiaddress::random_with_id(MockAuthPeerId::random()); + let address = random_identity().0[0].clone(); self.network .emit_event(MockEvent::Connected(address.clone())); assert_eq!( @@ -458,7 +454,7 @@ mod tests { async fn test_sync_connected() { let mut test_data = TestData::prepare().await; - let address = MockAuthMultiaddress::random_with_id(MockAuthPeerId::random()); + let address = random_identity().0[0].clone(); test_data .network .emit_event(MockEvent::Connected(address.clone())); @@ -482,11 +478,11 @@ mod tests { async fn test_sync_disconnected() { let mut test_data = TestData::prepare().await; - let peer_id = MockAuthPeerId::random(); + let peer_id = random_peer_id(); test_data .network - .emit_event(MockEvent::Disconnected(peer_id)); + .emit_event(MockEvent::Disconnected(peer_id.clone())); let expected = (iter::once(peer_id).collect(), Protocol::Authentication); @@ -507,12 +503,13 @@ mod tests { async fn test_notification_stream_opened() { let mut test_data = TestData::prepare().await; - let peer_ids: Vec<_> = (0..3).map(|_| MockAuthPeerId::random()).collect(); + let peer_ids: Vec<_> = (0..3).map(|_| random_peer_id()).collect(); peer_ids.iter().for_each(|peer_id| { - test_data - .network - .emit_event(MockEvent::StreamOpened(*peer_id, Protocol::Authentication)); + test_data.network.emit_event(MockEvent::StreamOpened( + peer_id.clone(), + Protocol::Authentication, + )); }); // We do this only to make sure that NotificationStreamOpened events are handled @@ -549,22 +546,24 @@ mod tests { async fn test_notification_stream_closed() { let mut test_data = TestData::prepare().await; - let peer_ids: Vec<_> = (0..3).map(|_| MockAuthPeerId::random()).collect(); + let peer_ids: Vec<_> = (0..3).map(|_| random_peer_id()).collect(); let opened_authorities_n = 2; peer_ids.iter().for_each(|peer_id| { - test_data - .network - .emit_event(MockEvent::StreamOpened(*peer_id, Protocol::Authentication)); + test_data.network.emit_event(MockEvent::StreamOpened( + peer_id.clone(), + Protocol::Authentication, + )); }); peer_ids .iter() .skip(opened_authorities_n) .for_each(|peer_id| { - test_data - .network - .emit_event(MockEvent::StreamClosed(*peer_id, Protocol::Authentication)); + test_data.network.emit_event(MockEvent::StreamClosed( + peer_id.clone(), + Protocol::Authentication, + )); }); // We do this only to make sure that NotificationStreamClosed events are handled @@ -602,7 +601,7 @@ mod tests { async fn test_send_validator_data() { let mut test_data = TestData::prepare().await; - let peer_id = random_authority_id().await; + let peer_id = random_peer_id(); let message = message(1); @@ -658,16 +657,15 @@ mod tests { .lock() .push_back(MockSenderError::SomeError); - let peer_id = MockAuthPeerId::random(); + let peer_id = random_peer_id(); - let message_1 = - authentication(vec![(random_authority_id().await, String::from("other_1"))]).await; - let message_2 = - authentication(vec![(random_authority_id().await, String::from("other_2"))]).await; + let message_1 = authentication(vec![(random_peer_id(), String::from("other_1"))]).await; + let message_2 = authentication(vec![(random_peer_id(), String::from("other_2"))]).await; - test_data - .network - .emit_event(MockEvent::StreamOpened(peer_id, Protocol::Authentication)); + test_data.network.emit_event(MockEvent::StreamOpened( + peer_id.clone(), + Protocol::Authentication, + )); // We do this only to make sure that NotificationStreamOpened events are handled test_data.wait_for_events_handled().await; @@ -709,16 +707,15 @@ mod tests { .lock() .push_back(MockSenderError::SomeError); - let peer_id = MockAuthPeerId::random(); + let peer_id = random_peer_id(); - let message_1 = - authentication(vec![(random_authority_id().await, String::from("other_1"))]).await; - let message_2 = - authentication(vec![(random_authority_id().await, String::from("other_2"))]).await; + let message_1 = authentication(vec![(random_peer_id(), String::from("other_1"))]).await; + let message_2 = authentication(vec![(random_peer_id(), String::from("other_2"))]).await; - test_data - .network - .emit_event(MockEvent::StreamOpened(peer_id, Protocol::Authentication)); + test_data.network.emit_event(MockEvent::StreamOpened( + peer_id.clone(), + Protocol::Authentication, + )); // We do this only to make sure that NotificationStreamOpened events are handled test_data.wait_for_events_handled().await; @@ -754,11 +751,7 @@ mod tests { async fn test_notification_received() { let mut test_data = TestData::prepare().await; - let message = authentication(vec![( - random_authority_id().await, - String::from("other_addr"), - )]) - .await; + let message = authentication(vec![(random_peer_id(), String::from("other_addr"))]).await; test_data.network.emit_event(MockEvent::Messages(vec![( Protocol::Authentication, @@ -782,8 +775,7 @@ mod tests { async fn test_command_add_reserved() { let mut test_data = TestData::prepare().await; - let multiaddress: MockMultiaddress = - (random_authority_id().await, String::from("other_addr")); + let multiaddress: MockMultiaddress = (random_peer_id(), String::from("other_addr")); test_data .mock_io @@ -812,7 +804,7 @@ mod tests { async fn test_command_remove_reserved() { let mut test_data = TestData::prepare().await; - let peer_id = random_authority_id().await; + let peer_id = random_peer_id(); test_data .mock_io diff --git a/finality-aleph/src/testing/mocks/validator_network.rs b/finality-aleph/src/testing/mocks/validator_network.rs index 2ea14775b3..d71f194b9e 100644 --- a/finality-aleph/src/testing/mocks/validator_network.rs +++ b/finality-aleph/src/testing/mocks/validator_network.rs @@ -2,11 +2,9 @@ use std::{ collections::{BTreeMap, HashMap, HashSet}, io::Result as IoResult, pin::Pin, - sync::Arc, task::{Context, Poll}, }; -use aleph_primitives::{AuthorityId, KEY_TYPE}; use codec::{Decode, Encode, Output}; use futures::{ channel::{mpsc, oneshot}, @@ -15,7 +13,6 @@ use futures::{ use log::info; use rand::{thread_rng, Rng}; use sc_service::{SpawnTaskHandle, TaskManager}; -use sp_keystore::{testing::KeyStore, CryptoStore}; use tokio::{ io::{duplex, AsyncRead, AsyncWrite, DuplexStream, ReadBuf}, runtime::Handle, @@ -23,18 +20,18 @@ use tokio::{ }; use crate::{ - crypto::AuthorityPen, network::{mock::Channel, Data, Multiaddress, NetworkIdentity}, validator_network::{ - mock::random_keys, ConnectionInfo, Dialer as DialerT, Listener as ListenerT, Network, - PeerAddressInfo, Service, Splittable, + mock::{key, random_keys, MockPublicKey, MockSecretKey}, + ConnectionInfo, Dialer as DialerT, Listener as ListenerT, Network, PeerAddressInfo, + SecretKey, Service, Splittable, }, }; -pub type MockMultiaddress = (AuthorityId, String); +pub type MockMultiaddress = (MockPublicKey, String); impl Multiaddress for MockMultiaddress { - type PeerId = AuthorityId; + type PeerId = MockPublicKey; fn get_peer_id(&self) -> Option { Some(self.0.clone()) @@ -50,25 +47,25 @@ impl Multiaddress for MockMultiaddress { #[derive(Clone)] pub struct MockNetwork { - pub add_connection: Channel<(AuthorityId, Vec)>, - pub remove_connection: Channel, - pub send: Channel<(D, AuthorityId)>, + pub add_connection: Channel<(MockPublicKey, Vec)>, + pub remove_connection: Channel, + pub send: Channel<(D, MockPublicKey)>, pub next: Channel, - id: AuthorityId, + id: MockPublicKey, addresses: Vec, } #[async_trait::async_trait] -impl Network for MockNetwork { - fn add_connection(&mut self, peer: AuthorityId, addresses: Vec) { +impl Network for MockNetwork { + fn add_connection(&mut self, peer: MockPublicKey, addresses: Vec) { self.add_connection.send((peer, addresses)); } - fn remove_connection(&mut self, peer: AuthorityId) { + fn remove_connection(&mut self, peer: MockPublicKey) { self.remove_connection.send(peer); } - fn send(&self, data: D, recipient: AuthorityId) { + fn send(&self, data: D, recipient: MockPublicKey) { self.send.send((data, recipient)); } @@ -78,7 +75,7 @@ impl Network for MockNetwork { } impl NetworkIdentity for MockNetwork { - type PeerId = AuthorityId; + type PeerId = MockPublicKey; type Multiaddress = MockMultiaddress; fn identity(&self) -> (Vec, Self::PeerId) { @@ -86,23 +83,28 @@ impl NetworkIdentity for MockNetwork { } } -pub async fn random_authority_id() -> AuthorityId { - let key_store = Arc::new(KeyStore::new()); - key_store - .ed25519_generate_new(KEY_TYPE, None) - .await - .unwrap() - .into() +pub fn random_peer_id() -> MockPublicKey { + key().0 } -pub async fn random_identity(address: String) -> (Vec, AuthorityId) { - let id = random_authority_id().await; +pub fn random_identity_with_address(address: String) -> (Vec, MockPublicKey) { + let id = random_peer_id(); (vec![(id.clone(), address)], id) } +pub fn random_identity() -> (Vec, MockPublicKey) { + random_identity_with_address( + rand::thread_rng() + .sample_iter(&rand::distributions::Alphanumeric) + .map(char::from) + .take(43) + .collect(), + ) +} + impl MockNetwork { pub async fn new(address: &str) -> Self { - let id = random_authority_id().await; + let id = random_peer_id(); let addresses = vec![(id.clone(), String::from(address))]; MockNetwork { add_connection: Channel::new(), @@ -114,7 +116,7 @@ impl MockNetwork { } } - pub fn from(addresses: Vec, id: AuthorityId) -> Self { + pub fn from(addresses: Vec, id: MockPublicKey) -> Self { MockNetwork { add_connection: Channel::new(), remove_connection: Channel::new(), @@ -272,8 +274,8 @@ impl Splittable for UnreliableSplittable { } type Address = u32; -type Addresses = HashMap>; -type Callers = HashMap; +type Addresses = HashMap>; +type Callers = HashMap; type Connection = UnreliableSplittable; const TWICE_MAX_DATA_SIZE: usize = 32 * 1024 * 1024; @@ -319,7 +321,7 @@ pub struct UnreliableConnectionMaker { } impl UnreliableConnectionMaker { - pub fn new(ids: Vec) -> (Self, Callers, Addresses) { + pub fn new(ids: Vec) -> (Self, Callers, Addresses) { let mut listeners = Vec::with_capacity(ids.len()); let mut callers = HashMap::with_capacity(ids.len()); let (tx_dialer, dialers) = mpsc::unbounded(); @@ -422,18 +424,18 @@ impl Decode for MockData { #[allow(clippy::too_many_arguments)] fn spawn_peer( - pen: AuthorityPen, + secret_key: MockSecretKey, addr: Addresses, n_msg: usize, large_message_interval: Option, corrupted_message_interval: Option, dialer: MockDialer, listener: MockListener, - report: mpsc::UnboundedSender<(AuthorityId, usize)>, + report: mpsc::UnboundedSender<(MockPublicKey, usize)>, spawn_handle: SpawnTaskHandle, ) { - let our_id = pen.authority_id(); - let (service, mut interface) = Service::new(dialer, listener, pen, spawn_handle); + let our_id = secret_key.public_key(); + let (service, mut interface) = Service::new(dialer, listener, secret_key, spawn_handle); // run the service tokio::spawn(async { let (_exit, rx) = oneshot::channel(); @@ -470,7 +472,7 @@ fn spawn_peer( } let data: MockData = MockData::new(thread_rng().gen_range(0..n_msg) as u32, filler_size, decodes); // choose a peer - let peer: AuthorityId = peer_ids[thread_rng().gen_range(0..peer_ids.len())].clone(); + let peer: MockPublicKey = peer_ids[thread_rng().gen_range(0..peer_ids.len())].clone(); // send interface.send(data, peer); }, @@ -502,7 +504,7 @@ pub async fn scenario( let spawn_handle = task_manager.spawn_handle(); // create peer identities info!(target: "validator-network", "generating keys..."); - let keys = random_keys(n_peers).await; + let keys = random_keys(n_peers); info!(target: "validator-network", "done"); // prepare and run the manager let (mut connection_manager, mut callers, addr) = @@ -511,17 +513,17 @@ pub async fn scenario( connection_manager.run(broken_connection_interval).await; }); // channel for receiving status updates from spawned peers - let (tx_report, mut rx_report) = mpsc::unbounded::<(AuthorityId, usize)>(); - let mut reports: BTreeMap = + let (tx_report, mut rx_report) = mpsc::unbounded::<(MockPublicKey, usize)>(); + let mut reports: BTreeMap = keys.keys().cloned().map(|id| (id, 0)).collect(); // spawn peers - for (id, pen) in keys.into_iter() { + for (id, secret_key) in keys.into_iter() { let mut addr = addr.clone(); // do not connect with itself - addr.remove(&pen.authority_id()); + addr.remove(&secret_key.public_key()); let (dialer, listener) = callers.remove(&id).expect("should contain all ids"); spawn_peer( - pen, + secret_key, addr, n_msg, large_message_interval, diff --git a/finality-aleph/src/testing/network.rs b/finality-aleph/src/testing/network.rs index d1f6b36648..1dedafca0b 100644 --- a/finality-aleph/src/testing/network.rs +++ b/finality-aleph/src/testing/network.rs @@ -4,7 +4,6 @@ use std::{ time::Duration, }; -use aleph_primitives::AuthorityId as MockPeerId; use codec::{Decode, Encode}; use futures::channel::oneshot; use sc_service::TaskManager; @@ -13,15 +12,16 @@ use tokio::{runtime::Handle, task::JoinHandle, time::timeout}; use crate::{ crypto::{AuthorityPen, AuthorityVerifier}, network::{ - mock::{crypto_basics, MockData, MockEvent, MockNetwork, MockPeerId as MockAuthPeerId}, + mock::{crypto_basics, MockData, MockEvent, MockNetwork}, setup_io, testing::{DataInSession, DiscoveryMessage, SessionHandler, VersionedAuthentication}, ConnectionManager, ConnectionManagerConfig, DataNetwork, NetworkIdentity, Protocol, Service as NetworkService, SessionManager, }, testing::mocks::validator_network::{ - random_identity, MockMultiaddress, MockNetwork as MockValidatorNetwork, + random_identity_with_address, MockMultiaddress, MockNetwork as MockValidatorNetwork, }, + validator_network::mock::{key, MockPublicKey}, MillisecsPerBlock, NodeIndex, Recipient, SessionId, SessionPeriod, }; @@ -34,8 +34,8 @@ const NODES_N: usize = 3; struct Authority { pen: AuthorityPen, addresses: Vec, - peer_id: MockPeerId, - auth_peer_id: MockAuthPeerId, + peer_id: MockPublicKey, + auth_peer_id: MockPublicKey, } impl Authority { @@ -47,17 +47,17 @@ impl Authority { self.addresses.clone() } - fn peer_id(&self) -> MockPeerId { + fn peer_id(&self) -> MockPublicKey { self.peer_id.clone() } - fn auth_peer_id(&self) -> MockAuthPeerId { - self.auth_peer_id + fn auth_peer_id(&self) -> MockPublicKey { + self.auth_peer_id.clone() } } impl NetworkIdentity for Authority { - type PeerId = MockPeerId; + type PeerId = MockPublicKey; type Multiaddress = MockMultiaddress; fn identity(&self) -> (Vec, Self::PeerId) { @@ -84,8 +84,8 @@ async fn prepare_one_session_test_data() -> TestData { let (authority_pens, authority_verifier) = crypto_basics(NODES_N).await; let mut authorities = Vec::new(); for (index, p) in authority_pens { - let identity = random_identity(index.0.to_string()).await; - let auth_peer_id = MockAuthPeerId::random(); + let identity = random_identity_with_address(index.0.to_string()); + let auth_peer_id = key().0; authorities.push(Authority { pen: p, addresses: identity.0, @@ -152,7 +152,7 @@ async fn prepare_one_session_test_data() -> TestData { } impl TestData { - fn connect_identity_to_network(&mut self, peer_id: MockAuthPeerId, protocol: Protocol) { + fn connect_identity_to_network(&mut self, peer_id: MockPublicKey, protocol: Protocol) { self.network .emit_event(MockEvent::StreamOpened(peer_id, protocol)); } @@ -248,7 +248,7 @@ impl TestData { &mut self, ) -> Option<( VersionedAuthentication, - MockAuthPeerId, + MockPublicKey, Protocol, )> { loop { @@ -286,7 +286,7 @@ async fn test_sends_discovery_message() { let session_id = 43; let mut test_data = prepare_one_session_test_data().await; let connected_peer_id = test_data.authorities[1].auth_peer_id(); - test_data.connect_identity_to_network(connected_peer_id, Protocol::Authentication); + test_data.connect_identity_to_network(connected_peer_id.clone(), Protocol::Authentication); let mut data_network = test_data.start_validator_session(0, session_id).await; let handler = test_data.get_session_handler(0, session_id).await; diff --git a/finality-aleph/src/validator_network/manager/direction.rs b/finality-aleph/src/validator_network/manager/direction.rs index 7d32eab0f1..d36148f92d 100644 --- a/finality-aleph/src/validator_network/manager/direction.rs +++ b/finality-aleph/src/validator_network/manager/direction.rs @@ -84,15 +84,13 @@ impl DirectedPeers { #[cfg(test)] mod tests { - use aleph_primitives::AuthorityId; - use super::DirectedPeers; - use crate::validator_network::mock::key; + use crate::validator_network::mock::{key, MockPublicKey}; type Address = String; - async fn container_with_id() -> (DirectedPeers, AuthorityId) { - let (id, _) = key().await; + fn container_with_id() -> (DirectedPeers, MockPublicKey) { + let (id, _) = key(); let container = DirectedPeers::new(id.clone()); (container, id) } @@ -105,10 +103,10 @@ mod tests { ] } - #[tokio::test] - async fn exactly_one_direction_attempts_connections() { - let (mut container0, id0) = container_with_id().await; - let (mut container1, id1) = container_with_id().await; + #[test] + fn exactly_one_direction_attempts_connections() { + let (mut container0, id0) = container_with_id(); + let (mut container1, id1) = container_with_id(); let addresses = some_addresses(); assert!( container0.add_peer(id1, addresses.clone()) @@ -116,10 +114,10 @@ mod tests { ); } - async fn container_with_added_connecting_peer( - ) -> (DirectedPeers, AuthorityId) { - let (mut container0, id0) = container_with_id().await; - let (mut container1, id1) = container_with_id().await; + fn container_with_added_connecting_peer( + ) -> (DirectedPeers, MockPublicKey) { + let (mut container0, id0) = container_with_id(); + let (mut container1, id1) = container_with_id(); let addresses = some_addresses(); match container0.add_peer(id1.clone(), addresses.clone()) { true => (container0, id1), @@ -130,10 +128,10 @@ mod tests { } } - async fn container_with_added_nonconnecting_peer( - ) -> (DirectedPeers, AuthorityId) { - let (mut container0, id0) = container_with_id().await; - let (mut container1, id1) = container_with_id().await; + fn container_with_added_nonconnecting_peer( + ) -> (DirectedPeers, MockPublicKey) { + let (mut container0, id0) = container_with_id(); + let (mut container1, id1) = container_with_id(); let addresses = some_addresses(); match container0.add_peer(id1.clone(), addresses.clone()) { false => (container0, id1), @@ -144,61 +142,61 @@ mod tests { } } - #[tokio::test] - async fn no_connecting_on_subsequent_add() { - let (mut container0, id1) = container_with_added_connecting_peer().await; + #[test] + fn no_connecting_on_subsequent_add() { + let (mut container0, id1) = container_with_added_connecting_peer(); let addresses = some_addresses(); assert!(!container0.add_peer(id1, addresses)); } - #[tokio::test] - async fn peer_addresses_when_connecting() { - let (container0, id1) = container_with_added_connecting_peer().await; + #[test] + fn peer_addresses_when_connecting() { + let (container0, id1) = container_with_added_connecting_peer(); assert!(container0.peer_addresses(&id1).is_some()); } - #[tokio::test] - async fn no_peer_addresses_when_nonconnecting() { - let (container0, id1) = container_with_added_nonconnecting_peer().await; + #[test] + fn no_peer_addresses_when_nonconnecting() { + let (container0, id1) = container_with_added_nonconnecting_peer(); assert!(container0.peer_addresses(&id1).is_none()); } - #[tokio::test] - async fn interested_in_connecting() { - let (container0, id1) = container_with_added_connecting_peer().await; + #[test] + fn interested_in_connecting() { + let (container0, id1) = container_with_added_connecting_peer(); assert!(container0.interested(&id1)); } - #[tokio::test] - async fn interested_in_nonconnecting() { - let (container0, id1) = container_with_added_nonconnecting_peer().await; + #[test] + fn interested_in_nonconnecting() { + let (container0, id1) = container_with_added_nonconnecting_peer(); assert!(container0.interested(&id1)); } - #[tokio::test] - async fn uninterested_in_unknown() { - let (container0, _) = container_with_id().await; - let (_, id1) = container_with_id().await; + #[test] + fn uninterested_in_unknown() { + let (container0, _) = container_with_id(); + let (_, id1) = container_with_id(); assert!(!container0.interested(&id1)); } - #[tokio::test] - async fn connecting_are_outgoing() { - let (container0, id1) = container_with_added_connecting_peer().await; + #[test] + fn connecting_are_outgoing() { + let (container0, id1) = container_with_added_connecting_peer(); assert_eq!(container0.outgoing_peers().collect::>(), vec![&id1]); assert_eq!(container0.incoming_peers().next(), None); } - #[tokio::test] - async fn nonconnecting_are_incoming() { - let (container0, id1) = container_with_added_nonconnecting_peer().await; + #[test] + fn nonconnecting_are_incoming() { + let (container0, id1) = container_with_added_nonconnecting_peer(); assert_eq!(container0.incoming_peers().collect::>(), vec![&id1]); assert_eq!(container0.outgoing_peers().next(), None); } - #[tokio::test] - async fn uninterested_in_removed() { - let (mut container0, id1) = container_with_added_connecting_peer().await; + #[test] + fn uninterested_in_removed() { + let (mut container0, id1) = container_with_added_connecting_peer(); assert!(container0.interested(&id1)); container0.remove_peer(&id1); assert!(!container0.interested(&id1)); diff --git a/finality-aleph/src/validator_network/manager/legacy.rs b/finality-aleph/src/validator_network/manager/legacy.rs index 6e7e13697c..e4d774ffbc 100644 --- a/finality-aleph/src/validator_network/manager/legacy.rs +++ b/finality-aleph/src/validator_network/manager/legacy.rs @@ -215,20 +215,19 @@ impl Manager { #[cfg(test)] mod tests { - use aleph_primitives::AuthorityId; use futures::{channel::mpsc, StreamExt}; use super::{AddResult::*, Manager, SendError}; - use crate::validator_network::mock::key; + use crate::validator_network::mock::{key, MockPublicKey}; type Data = String; type Address = String; - #[tokio::test] - async fn add_remove() { - let mut manager = Manager::::new(); - let (peer_id, _) = key().await; - let (peer_id_b, _) = key().await; + #[test] + fn add_remove() { + let mut manager = Manager::::new(); + let (peer_id, _) = key(); + let (peer_id_b, _) = key(); let addresses = vec![ String::from(""), String::from("a/b/c"), @@ -250,9 +249,9 @@ mod tests { #[tokio::test] async fn outgoing() { - let mut manager = Manager::::new(); + let mut manager = Manager::::new(); let data = String::from("DATA"); - let (peer_id, _) = key().await; + let (peer_id, _) = key(); let addresses = vec![ String::from(""), String::from("a/b/c"), @@ -279,10 +278,10 @@ mod tests { assert!(rx.next().await.is_none()); } - #[tokio::test] - async fn incoming() { - let mut manager = Manager::::new(); - let (peer_id, _) = key().await; + #[test] + fn incoming() { + let mut manager = Manager::::new(); + let (peer_id, _) = key(); let addresses = vec![ String::from(""), String::from("a/b/c"), diff --git a/finality-aleph/src/validator_network/manager/mod.rs b/finality-aleph/src/validator_network/manager/mod.rs index dd9288e7d7..a9b13732d2 100644 --- a/finality-aleph/src/validator_network/manager/mod.rs +++ b/finality-aleph/src/validator_network/manager/mod.rs @@ -239,21 +239,20 @@ impl Manager { #[cfg(test)] mod tests { - use aleph_primitives::AuthorityId; use futures::{channel::mpsc, StreamExt}; use super::{AddResult::*, Manager, SendError}; - use crate::validator_network::mock::key; + use crate::validator_network::mock::{key, MockPublicKey}; type Data = String; type Address = String; - #[tokio::test] - async fn add_remove() { - let (own_id, _) = key().await; - let mut manager = Manager::::new(own_id); - let (peer_id, _) = key().await; - let (peer_id_b, _) = key().await; + #[test] + fn add_remove() { + let (own_id, _) = key(); + let mut manager = Manager::::new(own_id); + let (peer_id, _) = key(); + let (peer_id_b, _) = key(); let addresses = vec![ String::from(""), String::from("a/b/c"), @@ -282,12 +281,12 @@ mod tests { #[tokio::test] async fn send_receive() { - let (mut connecting_id, _) = key().await; + let (mut connecting_id, _) = key(); let mut connecting_manager = - Manager::::new(connecting_id.clone()); - let (mut listening_id, _) = key().await; + Manager::::new(connecting_id.clone()); + let (mut listening_id, _) = key(); let mut listening_manager = - Manager::::new(listening_id.clone()); + Manager::::new(listening_id.clone()); let data = String::from("DATA"); let addresses = vec![ String::from(""), diff --git a/finality-aleph/src/validator_network/mock.rs b/finality-aleph/src/validator_network/mock.rs index 383a365501..37858f5858 100644 --- a/finality-aleph/src/validator_network/mock.rs +++ b/finality-aleph/src/validator_network/mock.rs @@ -1,43 +1,87 @@ -use std::sync::Arc; -#[cfg(test)] use std::{ collections::HashMap, + fmt::{Display, Error as FmtError, Formatter}, io::Result as IoResult, pin::Pin, task::{Context, Poll}, }; -use aleph_primitives::{AuthorityId, KEY_TYPE}; -use sp_keystore::{testing::KeyStore, CryptoStore}; +use codec::{Decode, Encode}; use tokio::io::{duplex, AsyncRead, AsyncWrite, DuplexStream, ReadBuf}; use crate::{ - crypto::AuthorityPen, - validator_network::{ConnectionInfo, PeerAddressInfo, Splittable}, + network::PeerId, + validator_network::{ConnectionInfo, PeerAddressInfo, PublicKey, SecretKey, Splittable}, }; -/// Create a random authority id and pen pair. -pub async fn key() -> (AuthorityId, AuthorityPen) { - let keystore = Arc::new(KeyStore::new()); - let id: AuthorityId = keystore - .ed25519_generate_new(KEY_TYPE, None) - .await - .unwrap() - .into(); - let pen = AuthorityPen::new(id.clone(), keystore) - .await - .expect("keys shoud sign successfully"); - (id, pen) +/// A mock secret key that is able to sign messages. +#[derive(Debug, PartialEq, Eq, Clone, Hash)] +pub struct MockSecretKey([u8; 4]); + +/// A mock public key for verifying signatures. +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Hash, Encode, Decode)] +pub struct MockPublicKey([u8; 4]); + +impl Display for MockPublicKey { + fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> { + write!(f, "PublicKey({:?})", self.0) + } +} + +impl AsRef<[u8]> for MockPublicKey { + fn as_ref(&self) -> &[u8] { + self.0.as_ref() + } +} + +/// A mock signature, able to discern whether the correct key has been used to sign a specific +/// message. +#[derive(Debug, PartialEq, Eq, Clone, Hash, Encode, Decode)] +pub struct MockSignature { + message: Vec, + key_id: [u8; 4], +} + +impl PublicKey for MockPublicKey { + type Signature = MockSignature; + + fn verify(&self, message: &[u8], signature: &Self::Signature) -> bool { + (message == signature.message.as_slice()) && (self.0 == signature.key_id) + } +} + +impl PeerId for MockPublicKey {} + +#[async_trait::async_trait] +impl SecretKey for MockSecretKey { + type Signature = MockSignature; + type PublicKey = MockPublicKey; + + async fn sign(&self, message: &[u8]) -> Self::Signature { + MockSignature { + message: message.to_vec(), + key_id: self.0, + } + } + + fn public_key(&self) -> Self::PublicKey { + MockPublicKey(self.0) + } +} + +/// Create a random key pair. +pub fn key() -> (MockPublicKey, MockSecretKey) { + let secret_key = MockSecretKey(rand::random()); + (secret_key.public_key(), secret_key) } -/// Create a HashMap with authority ids as keys and pens as values. -pub async fn random_keys(n_peers: usize) -> HashMap { +/// Create a HashMap with public keys as keys and secret keys as values. +pub fn random_keys(n_peers: usize) -> HashMap { let mut result = HashMap::with_capacity(n_peers); - for _ in 0..n_peers { - let (id, pen) = key().await; - result.insert(id, pen); + while result.len() < n_peers { + let (pk, sk) = key(); + result.insert(pk, sk); } - assert_eq!(result.len(), n_peers); result } diff --git a/finality-aleph/src/validator_network/protocols/handshake.rs b/finality-aleph/src/validator_network/protocols/handshake.rs index 3286d5733e..d436fca91b 100644 --- a/finality-aleph/src/validator_network/protocols/handshake.rs +++ b/finality-aleph/src/validator_network/protocols/handshake.rs @@ -182,23 +182,19 @@ pub async fn v0_handshake_outgoing( #[cfg(test)] mod tests { - use aleph_primitives::AuthorityId; use futures::{join, try_join}; use super::{ execute_v0_handshake_incoming, execute_v0_handshake_outgoing, Challenge, HandshakeError, Response, }; - use crate::{ - crypto::AuthorityPen, - validator_network::{ - io::{receive_data, send_data}, - mock::{key, MockSplittable}, - Splittable, - }, + use crate::validator_network::{ + io::{receive_data, send_data}, + mock::{key, MockPublicKey, MockSecretKey, MockSplittable}, + SecretKey, Splittable, }; - fn assert_send_error(result: Result>) { + fn assert_send_error(result: Result>) { match result { Err(HandshakeError::SendError(_)) => (), x => panic!( @@ -208,7 +204,7 @@ mod tests { }; } - fn assert_receive_error(result: Result>) { + fn assert_receive_error(result: Result>) { match result { Err(HandshakeError::ReceiveError(_)) => (), x => panic!( @@ -218,7 +214,9 @@ mod tests { }; } - fn assert_signature_error(result: Result>) { + fn assert_signature_error( + result: Result>, + ) { match result { Err(HandshakeError::SignatureError) => (), x => panic!( @@ -228,7 +226,9 @@ mod tests { }; } - fn assert_challenge_error(result: Result>) { + fn assert_challenge_error( + result: Result>, + ) { match result { Err(HandshakeError::ChallengeError(_, _)) => (), x => panic!( @@ -241,8 +241,8 @@ mod tests { #[tokio::test] async fn handshake() { let (stream_a, stream_b) = MockSplittable::new(4096); - let (id_a, pen_a) = key().await; - let (id_b, pen_b) = key().await; + let (id_a, pen_a) = key(); + let (id_b, pen_b) = key(); assert_ne!(id_a, id_b); let ((_, _, received_id_b), (_, _)) = try_join!( execute_v0_handshake_incoming(stream_a, pen_a), @@ -255,7 +255,7 @@ mod tests { #[tokio::test] async fn handshake_with_malicious_server_peer() { async fn execute_malicious_v0_handshake_incoming(stream: S) { - let (fake_id, _) = key().await; + let (fake_id, _) = key(); // send challenge with incorrect id let our_challenge = Challenge::new(fake_id); send_data(stream, our_challenge.clone()) @@ -266,8 +266,8 @@ mod tests { } let (stream_a, stream_b) = MockSplittable::new(4096); - let (id_a, _) = key().await; - let (_, pen_b) = key().await; + let (id_a, _) = key(); + let (_, pen_b) = key(); tokio::select! { _ = execute_malicious_v0_handshake_incoming(stream_a) => panic!("should wait"), result = execute_v0_handshake_outgoing(stream_b, pen_b, id_a) => assert_challenge_error(result), @@ -278,24 +278,24 @@ mod tests { async fn handshake_with_malicious_client_peer_fake_challenge() { pub async fn execute_malicious_v0_handshake_outgoing_fake_challenge( stream: S, - authority_pen: AuthorityPen, + secret_key: MockSecretKey, ) { // receive challenge - let (stream, _) = receive_data::<_, Challenge>(stream) + let (stream, _) = receive_data::<_, Challenge>(stream) .await .expect("should receive"); // prepare fake challenge - let (fake_id, _) = key().await; + let (fake_id, _) = key(); let fake_challenge = Challenge::new(fake_id); // send response with substituted challenge - let our_response = Response::new(&authority_pen, &fake_challenge).await; + let our_response = Response::new(&secret_key, &fake_challenge).await; send_data(stream, our_response).await.expect("should send"); futures::future::pending::<()>().await; } let (stream_a, stream_b) = MockSplittable::new(4096); - let (_, pen_a) = key().await; - let (_, pen_b) = key().await; + let (_, pen_a) = key(); + let (_, pen_b) = key(); tokio::select! { result = execute_v0_handshake_incoming(stream_a, pen_a) => assert_signature_error(result), _ = execute_malicious_v0_handshake_outgoing_fake_challenge(stream_b, pen_b) => panic!("should wait"), @@ -306,24 +306,24 @@ mod tests { async fn handshake_with_malicious_client_peer_fake_signature() { pub async fn execute_malicious_v0_handshake_outgoing_fake_signature( stream: S, - authority_pen: AuthorityPen, + secret_key: MockSecretKey, ) { // receive challenge - let (stream, challenge) = receive_data::<_, Challenge>(stream) + let (stream, challenge) = receive_data::<_, Challenge>(stream) .await .expect("should receive"); // prepare fake id - let (fake_id, _) = key().await; + let (fake_id, _) = key(); // send response with substituted id - let mut our_response = Response::new(&authority_pen, &challenge).await; + let mut our_response = Response::new(&secret_key, &challenge).await; our_response.public_key = fake_id; send_data(stream, our_response).await.expect("should send"); futures::future::pending::<()>().await; } let (stream_a, stream_b) = MockSplittable::new(4096); - let (_, pen_a) = key().await; - let (_, pen_b) = key().await; + let (_, pen_a) = key(); + let (_, pen_b) = key(); tokio::select! { result = execute_v0_handshake_incoming(stream_a, pen_a) => assert_signature_error(result), _ = execute_malicious_v0_handshake_outgoing_fake_signature(stream_b, pen_b) => panic!("should wait"), @@ -334,19 +334,19 @@ mod tests { async fn broken_incoming_connection_step_one() { // break the connection even before the handshake starts by dropping the stream let (stream_a, _) = MockSplittable::new(4096); - let (_, pen_a) = key().await; + let (_, pen_a) = key(); assert_send_error(execute_v0_handshake_incoming(stream_a, pen_a).await); } #[tokio::test] async fn broken_incoming_connection_step_two() { let (stream_a, stream_b) = MockSplittable::new(4096); - let (_, pen_a) = key().await; + let (_, pen_a) = key(); let (result, _) = join!( execute_v0_handshake_incoming(stream_a, pen_a), // mock outgoing handshake: receive the first message and terminate async { - receive_data::<_, Challenge>(stream_b) + receive_data::<_, Challenge>(stream_b) .await .expect("should receive"); }, @@ -358,18 +358,18 @@ mod tests { async fn broken_outgoing_connection_step_one() { // break the connection even before the handshake starts by dropping the stream let (stream_a, _) = MockSplittable::new(4096); - let (_, pen_a) = key().await; - let (id_b, _) = key().await; + let (_, pen_a) = key(); + let (id_b, _) = key(); assert_receive_error(execute_v0_handshake_outgoing(stream_a, pen_a, id_b).await); } #[tokio::test] async fn broken_outgoing_connection_step_two() { let (stream_a, stream_b) = MockSplittable::new(4096); - let (id_a, pen_a) = key().await; - let (_, pen_b) = key().await; + let (id_a, pen_a) = key(); + let (_, pen_b) = key(); // mock incoming handshake: send the first message and terminate - send_data(stream_a, Challenge::new(pen_a.authority_id())) + send_data(stream_a, Challenge::new(pen_a.public_key())) .await .expect("should send"); assert_send_error(execute_v0_handshake_outgoing(stream_b, pen_b, id_a).await); diff --git a/finality-aleph/src/validator_network/protocols/v0/mod.rs b/finality-aleph/src/validator_network/protocols/v0/mod.rs index c0c1af3ea0..7510f71b6e 100644 --- a/finality-aleph/src/validator_network/protocols/v0/mod.rs +++ b/finality-aleph/src/validator_network/protocols/v0/mod.rs @@ -113,36 +113,32 @@ pub async fn incoming( #[cfg(test)] mod tests { - use aleph_primitives::AuthorityId; use futures::{ channel::{mpsc, mpsc::UnboundedReceiver}, pin_mut, FutureExt, StreamExt, }; use super::{incoming, outgoing, ProtocolError}; - use crate::{ - crypto::AuthorityPen, - validator_network::{ - mock::{key, MockSplittable}, - protocols::{ConnectionType, ResultForService}, - Data, - }, + use crate::validator_network::{ + mock::{key, MockPublicKey, MockSecretKey, MockSplittable}, + protocols::{ConnectionType, ResultForService}, + Data, }; - async fn prepare() -> ( - AuthorityId, - AuthorityPen, - AuthorityId, - AuthorityPen, - impl futures::Future>>, - impl futures::Future>>, + fn prepare() -> ( + MockPublicKey, + MockSecretKey, + MockPublicKey, + MockSecretKey, + impl futures::Future>>, + impl futures::Future>>, UnboundedReceiver, - UnboundedReceiver>, - UnboundedReceiver>, + UnboundedReceiver>, + UnboundedReceiver>, ) { let (stream_incoming, stream_outgoing) = MockSplittable::new(4096); - let (id_incoming, pen_incoming) = key().await; - let (id_outgoing, pen_outgoing) = key().await; + let (id_incoming, pen_incoming) = key(); + let (id_outgoing, pen_outgoing) = key(); assert_ne!(id_incoming, id_outgoing); let (incoming_result_for_service, result_from_incoming) = mpsc::unbounded(); let (outgoing_result_for_service, result_from_outgoing) = mpsc::unbounded(); @@ -184,7 +180,7 @@ mod tests { mut data_from_incoming, _result_from_incoming, mut result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); let incoming_handle = incoming_handle.fuse(); let outgoing_handle = outgoing_handle.fuse(); pin_mut!(incoming_handle); @@ -233,7 +229,7 @@ mod tests { _data_from_incoming, mut result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); let incoming_handle = incoming_handle.fuse(); let outgoing_handle = outgoing_handle.fuse(); pin_mut!(incoming_handle); @@ -265,7 +261,7 @@ mod tests { _data_from_incoming, result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); std::mem::drop(result_from_incoming); let incoming_handle = incoming_handle.fuse(); let outgoing_handle = outgoing_handle.fuse(); @@ -293,7 +289,7 @@ mod tests { data_from_incoming, _result_from_incoming, mut result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); std::mem::drop(data_from_incoming); let incoming_handle = incoming_handle.fuse(); let outgoing_handle = outgoing_handle.fuse(); @@ -334,7 +330,7 @@ mod tests { _data_from_incoming, _result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); std::mem::drop(outgoing_handle); match incoming_handle.await { Err(ProtocolError::HandshakeError(_)) => (), @@ -355,7 +351,7 @@ mod tests { _data_from_incoming, mut result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); let incoming_handle = incoming_handle.fuse(); pin_mut!(incoming_handle); let (_, _exit, connection_type) = tokio::select! { @@ -384,7 +380,7 @@ mod tests { _data_from_incoming, _result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); std::mem::drop(incoming_handle); match outgoing_handle.await { Err(ProtocolError::HandshakeError(_)) => (), @@ -405,7 +401,7 @@ mod tests { _data_from_incoming, mut result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); let outgoing_handle = outgoing_handle.fuse(); pin_mut!(outgoing_handle); let (_, _exit, connection_type) = tokio::select! { @@ -436,7 +432,7 @@ mod tests { _data_from_incoming, mut result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); let outgoing_handle = outgoing_handle.fuse(); pin_mut!(outgoing_handle); let (_, _exit, connection_type) = tokio::select! { diff --git a/finality-aleph/src/validator_network/protocols/v1/mod.rs b/finality-aleph/src/validator_network/protocols/v1/mod.rs index 372a00dfac..ca138f9170 100644 --- a/finality-aleph/src/validator_network/protocols/v1/mod.rs +++ b/finality-aleph/src/validator_network/protocols/v1/mod.rs @@ -133,37 +133,33 @@ pub async fn incoming( #[cfg(test)] mod tests { - use aleph_primitives::AuthorityId; use futures::{ channel::{mpsc, mpsc::UnboundedReceiver}, pin_mut, FutureExt, StreamExt, }; use super::{incoming, outgoing, ProtocolError}; - use crate::{ - crypto::AuthorityPen, - validator_network::{ - mock::{key, MockSplittable}, - protocols::{ConnectionType, ResultForService}, - Data, - }, + use crate::validator_network::{ + mock::{key, MockPublicKey, MockSecretKey, MockSplittable}, + protocols::{ConnectionType, ResultForService}, + Data, }; - async fn prepare() -> ( - AuthorityId, - AuthorityPen, - AuthorityId, - AuthorityPen, - impl futures::Future>>, - impl futures::Future>>, + fn prepare() -> ( + MockPublicKey, + MockSecretKey, + MockPublicKey, + MockSecretKey, + impl futures::Future>>, + impl futures::Future>>, UnboundedReceiver, UnboundedReceiver, - UnboundedReceiver>, - UnboundedReceiver>, + UnboundedReceiver>, + UnboundedReceiver>, ) { let (stream_incoming, stream_outgoing) = MockSplittable::new(4096); - let (id_incoming, pen_incoming) = key().await; - let (id_outgoing, pen_outgoing) = key().await; + let (id_incoming, pen_incoming) = key(); + let (id_outgoing, pen_outgoing) = key(); assert_ne!(id_incoming, id_outgoing); let (incoming_result_for_service, result_from_incoming) = mpsc::unbounded(); let (outgoing_result_for_service, result_from_outgoing) = mpsc::unbounded(); @@ -209,7 +205,7 @@ mod tests { mut data_from_outgoing, mut result_from_incoming, mut result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); let incoming_handle = incoming_handle.fuse(); let outgoing_handle = outgoing_handle.fuse(); pin_mut!(incoming_handle); @@ -289,7 +285,7 @@ mod tests { _data_from_outgoing, mut result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); let incoming_handle = incoming_handle.fuse(); let outgoing_handle = outgoing_handle.fuse(); pin_mut!(incoming_handle); @@ -322,7 +318,7 @@ mod tests { _data_from_outgoing, result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); std::mem::drop(result_from_incoming); let incoming_handle = incoming_handle.fuse(); let outgoing_handle = outgoing_handle.fuse(); @@ -351,7 +347,7 @@ mod tests { _data_from_outgoing, _result_from_incoming, mut result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); std::mem::drop(data_from_incoming); let incoming_handle = incoming_handle.fuse(); let outgoing_handle = outgoing_handle.fuse(); @@ -393,7 +389,7 @@ mod tests { _data_from_outgoing, _result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); std::mem::drop(outgoing_handle); match incoming_handle.await { Err(ProtocolError::HandshakeError(_)) => (), @@ -415,7 +411,7 @@ mod tests { _data_from_outgoing, mut result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); let incoming_handle = incoming_handle.fuse(); pin_mut!(incoming_handle); let (_, _exit, connection_type) = tokio::select! { @@ -445,7 +441,7 @@ mod tests { _data_from_outgoing, _result_from_incoming, _result_from_outgoing, - ) = prepare::>().await; + ) = prepare::>(); std::mem::drop(incoming_handle); match outgoing_handle.await { Err(ProtocolError::HandshakeError(_)) => (), From 2c5a15091d27367f5fbc49fe810994f4e04b62ab Mon Sep 17 00:00:00 2001 From: timorl Date: Mon, 28 Nov 2022 13:40:38 +0100 Subject: [PATCH 2/3] Random multiaddress function --- finality-aleph/src/network/manager/discovery.rs | 8 ++++---- finality-aleph/src/network/manager/session.rs | 7 ++----- finality-aleph/src/network/service.rs | 7 ++++--- finality-aleph/src/testing/mocks/validator_network.rs | 4 ++++ 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/finality-aleph/src/network/manager/discovery.rs b/finality-aleph/src/network/manager/discovery.rs index fd8942dcff..112fceadf3 100644 --- a/finality-aleph/src/network/manager/discovery.rs +++ b/finality-aleph/src/network/manager/discovery.rs @@ -137,7 +137,9 @@ mod tests { use super::{Discovery, DiscoveryMessage}; use crate::{ network::{manager::SessionHandler, mock::crypto_basics}, - testing::mocks::validator_network::{random_identity, MockMultiaddress}, + testing::mocks::validator_network::{ + random_identity, random_multiaddress, MockMultiaddress, + }, SessionId, }; @@ -145,9 +147,7 @@ mod tests { const MS_COOLDOWN: u64 = 200; fn addresses() -> Vec { - (0..NUM_NODES) - .map(|_| random_identity().0[0].clone()) - .collect() + (0..NUM_NODES).map(|_| random_multiaddress()).collect() } async fn build_number( diff --git a/finality-aleph/src/network/manager/session.rs b/finality-aleph/src/network/manager/session.rs index 3998f7b71c..1c8f84906d 100644 --- a/finality-aleph/src/network/manager/session.rs +++ b/finality-aleph/src/network/manager/session.rs @@ -278,7 +278,7 @@ mod tests { mock::{crypto_basics, MockNetworkIdentity}, NetworkIdentity, }, - testing::mocks::validator_network::{random_identity, MockMultiaddress}, + testing::mocks::validator_network::{random_multiaddress, MockMultiaddress}, NodeIndex, SessionId, }; @@ -379,10 +379,7 @@ mod tests { #[tokio::test] async fn fails_to_create_with_non_unique_peer_id() { let mut crypto_basics = crypto_basics(NUM_NODES).await; - let addresses = vec![ - random_identity().0[0].clone(), - random_identity().0[0].clone(), - ]; + let addresses = vec![random_multiaddress(), random_multiaddress()]; assert!(matches!( Handler::new( Some(crypto_basics.0.pop().unwrap()), diff --git a/finality-aleph/src/network/service.rs b/finality-aleph/src/network/service.rs index 6a39cd09a0..42c2ef954b 100644 --- a/finality-aleph/src/network/service.rs +++ b/finality-aleph/src/network/service.rs @@ -353,7 +353,8 @@ mod tests { NetworkIdentity, Protocol, }, testing::mocks::validator_network::{ - random_identity, random_peer_id, MockMultiaddress, MockNetwork as MockValidatorNetwork, + random_multiaddress, random_peer_id, MockMultiaddress, + MockNetwork as MockValidatorNetwork, }, SessionId, }; @@ -415,7 +416,7 @@ mod tests { // We do this only to make sure that NotificationStreamOpened/NotificationStreamClosed events are handled async fn wait_for_events_handled(&mut self) { - let address = random_identity().0[0].clone(); + let address = random_multiaddress(); self.network .emit_event(MockEvent::Connected(address.clone())); assert_eq!( @@ -454,7 +455,7 @@ mod tests { async fn test_sync_connected() { let mut test_data = TestData::prepare().await; - let address = random_identity().0[0].clone(); + let address = random_multiaddress(); test_data .network .emit_event(MockEvent::Connected(address.clone())); diff --git a/finality-aleph/src/testing/mocks/validator_network.rs b/finality-aleph/src/testing/mocks/validator_network.rs index d71f194b9e..7de0771818 100644 --- a/finality-aleph/src/testing/mocks/validator_network.rs +++ b/finality-aleph/src/testing/mocks/validator_network.rs @@ -102,6 +102,10 @@ pub fn random_identity() -> (Vec, MockPublicKey) { ) } +pub fn random_multiaddress() -> MockMultiaddress { + random_identity().0.pop().expect("we created an address") +} + impl MockNetwork { pub async fn new(address: &str) -> Self { let id = random_peer_id(); From edadbfba67b8c53eaaf7c5e0e309178819b099be Mon Sep 17 00:00:00 2001 From: timorl Date: Mon, 28 Nov 2022 14:16:47 +0100 Subject: [PATCH 3/3] Some clippy warnings --- finality-aleph/src/network/mock.rs | 2 +- finality-aleph/src/testing/network.rs | 2 +- finality-aleph/src/validator_network/manager/direction.rs | 5 +---- finality-aleph/src/validator_network/manager/legacy.rs | 2 +- finality-aleph/src/validator_network/manager/mod.rs | 8 ++------ 5 files changed, 6 insertions(+), 13 deletions(-) diff --git a/finality-aleph/src/network/mock.rs b/finality-aleph/src/network/mock.rs index 2f42392da7..83db534947 100644 --- a/finality-aleph/src/network/mock.rs +++ b/finality-aleph/src/network/mock.rs @@ -78,7 +78,7 @@ impl Channel { self.1.lock().await.by_ref().take(n).collect::>(), ) .await - .unwrap_or(Vec::new()) + .unwrap_or_default() } pub async fn try_next(&self) -> Option { diff --git a/finality-aleph/src/testing/network.rs b/finality-aleph/src/testing/network.rs index 1dedafca0b..13a243001f 100644 --- a/finality-aleph/src/testing/network.rs +++ b/finality-aleph/src/testing/network.rs @@ -275,7 +275,7 @@ impl TestData { self.network_service_exit_tx.send(()).unwrap(); self.network_manager_handle.await.unwrap(); self.network_service_handle.await.unwrap(); - while let Some(_) = self.network.send_message.try_next().await {} + while self.network.send_message.try_next().await.is_some() {} self.network.close_channels().await; self.validator_network.close_channels().await; } diff --git a/finality-aleph/src/validator_network/manager/direction.rs b/finality-aleph/src/validator_network/manager/direction.rs index d36148f92d..3197b1a12d 100644 --- a/finality-aleph/src/validator_network/manager/direction.rs +++ b/finality-aleph/src/validator_network/manager/direction.rs @@ -108,10 +108,7 @@ mod tests { let (mut container0, id0) = container_with_id(); let (mut container1, id1) = container_with_id(); let addresses = some_addresses(); - assert!( - container0.add_peer(id1, addresses.clone()) - != container1.add_peer(id0, addresses.clone()) - ); + assert!(container0.add_peer(id1, addresses.clone()) != container1.add_peer(id0, addresses)); } fn container_with_added_connecting_peer( diff --git a/finality-aleph/src/validator_network/manager/legacy.rs b/finality-aleph/src/validator_network/manager/legacy.rs index e4d774ffbc..3162161983 100644 --- a/finality-aleph/src/validator_network/manager/legacy.rs +++ b/finality-aleph/src/validator_network/manager/legacy.rs @@ -293,7 +293,7 @@ mod tests { // rx should fail assert!(rx.try_next().expect("channel should be closed").is_none()); // add peer, this time for real - assert!(manager.add_peer(peer_id.clone(), addresses.clone())); + assert!(manager.add_peer(peer_id.clone(), addresses)); let (tx, mut rx) = mpsc::unbounded(); // should just add assert_eq!(manager.add_incoming(peer_id.clone(), tx), Added); diff --git a/finality-aleph/src/validator_network/manager/mod.rs b/finality-aleph/src/validator_network/manager/mod.rs index a9b13732d2..c309ac5af5 100644 --- a/finality-aleph/src/validator_network/manager/mod.rs +++ b/finality-aleph/src/validator_network/manager/mod.rs @@ -310,12 +310,8 @@ mod tests { } else { // We need to switch the names around, because the connection was randomly the // other way around. - let temp_id = connecting_id; - connecting_id = listening_id; - listening_id = temp_id; - let temp_manager = connecting_manager; - connecting_manager = listening_manager; - listening_manager = temp_manager; + std::mem::swap(&mut connecting_id, &mut listening_id); + std::mem::swap(&mut connecting_manager, &mut listening_manager); assert!(connecting_manager.add_peer(listening_id.clone(), addresses.clone())); } // add outgoing to connecting