diff --git a/substrate/client/authority-discovery/src/lib.rs b/substrate/client/authority-discovery/src/lib.rs index e5b7c986a4..bc76c14331 100644 --- a/substrate/client/authority-discovery/src/lib.rs +++ b/substrate/client/authority-discovery/src/lib.rs @@ -58,7 +58,7 @@ use futures::task::{Context, Poll}; use futures::{Future, FutureExt, ready, Stream, StreamExt}; use futures_timer::Delay; -use codec::{Decode, Encode}; +use codec::Decode; use error::{Error, Result}; use log::{debug, error, log_enabled}; use prometheus_endpoint::{Counter, CounterVec, Gauge, Opts, U64, register}; @@ -262,16 +262,16 @@ where ) .map_err(|_| Error::Signing)?; - for (sign_result, key) in signatures.iter().zip(keys) { + for (sign_result, key) in signatures.into_iter().zip(keys) { let mut signed_addresses = vec![]; // sign_with_all returns Result signature // is generated for a public key that is supported. // Verify that all signatures exist for all provided keys. - let signature = sign_result.as_ref().map_err(|_| Error::MissingSignature(key.clone()))?; + let signature = sign_result.map_err(|_| Error::MissingSignature(key.clone()))?; schema::SignedAuthorityAddresses { addresses: serialized_addresses.clone(), - signature: Encode::encode(&signature), + signature, } .encode(&mut signed_addresses) .map_err(Error::EncodingProto)?; diff --git a/substrate/client/authority-discovery/src/tests.rs b/substrate/client/authority-discovery/src/tests.rs index c9b5e392d8..c13bca894c 100644 --- a/substrate/client/authority-discovery/src/tests.rs +++ b/substrate/client/authority-discovery/src/tests.rs @@ -162,8 +162,8 @@ sp_api::mock_impl_runtime_apis! { } } -#[derive(Default)] struct TestNetwork { + peer_id: PeerId, // Whenever functions on `TestNetwork` are called, the function arguments are added to the // vectors below. pub put_value_call: Arc)>>>, @@ -171,6 +171,17 @@ struct TestNetwork { pub set_priority_group_call: Arc)>>>, } +impl Default for TestNetwork { + fn default() -> Self { + TestNetwork { + peer_id: PeerId::random(), + put_value_call: Default::default(), + get_value_call: Default::default(), + set_priority_group_call: Default::default(), + } + } +} + impl NetworkProvider for TestNetwork { fn set_priority_group( &self, @@ -193,11 +204,11 @@ impl NetworkProvider for TestNetwork { impl NetworkStateInfo for TestNetwork { fn local_peer_id(&self) -> PeerId { - PeerId::random() + self.peer_id.clone() } fn external_addresses(&self) -> Vec { - vec![] + vec!["/ip6/2001:db8::".parse().unwrap()] } } @@ -224,34 +235,6 @@ fn new_registers_metrics() { assert!(registry.gather().len() > 0); } -#[test] -fn publish_ext_addresses_puts_record_on_dht() { - let (_dht_event_tx, dht_event_rx) = channel(1000); - let network: Arc = Arc::new(Default::default()); - let key_store = KeyStore::new(); - let public = key_store - .write() - .sr25519_generate_new(key_types::AUTHORITY_DISCOVERY, None) - .unwrap(); - let test_api = Arc::new(TestApi { - authorities: vec![public.into()], - }); - - let mut authority_discovery = AuthorityDiscovery::new( - test_api, - network.clone(), - vec![], - dht_event_rx.boxed(), - Role::Authority(key_store), - None, - ); - - authority_discovery.publish_ext_addresses().unwrap(); - - // Expect authority discovery to put a new record onto the dht. - assert_eq!(network.put_value_call.lock().unwrap().len(), 1); -} - #[test] fn request_addresses_of_others_triggers_dht_get_query() { let _ = ::env_logger::try_init(); @@ -284,15 +267,57 @@ fn request_addresses_of_others_triggers_dht_get_query() { } #[test] -fn handle_dht_events_with_value_found_should_call_set_priority_group() { +fn publish_discover_cycle() { let _ = ::env_logger::try_init(); - // Create authority discovery. + // Node A publishing its address. + + let (_dht_event_tx, dht_event_rx) = channel(1000); + + let network: Arc = Arc::new(Default::default()); + let node_a_multiaddr = { + let peer_id = network.local_peer_id(); + let address = network.external_addresses().pop().unwrap(); + + address.with(libp2p::core::multiaddr::Protocol::P2p( + peer_id.into(), + )) + }; + + let key_store = KeyStore::new(); + let node_a_public = key_store + .write() + .sr25519_generate_new(key_types::AUTHORITY_DISCOVERY, None) + .unwrap(); + let test_api = Arc::new(TestApi { + authorities: vec![node_a_public.into()], + }); + + let mut authority_discovery = AuthorityDiscovery::new( + test_api, + network.clone(), + vec![], + dht_event_rx.boxed(), + Role::Authority(key_store), + None, + ); + + authority_discovery.publish_ext_addresses().unwrap(); + + // Expect authority discovery to put a new record onto the dht. + assert_eq!(network.put_value_call.lock().unwrap().len(), 1); + + let dht_event = { + let (key, value) = network.put_value_call.lock().unwrap().pop().unwrap(); + sc_network::DhtEvent::ValueFound(vec![(key, value)]) + }; + + // Node B discovering node A's address. let (mut dht_event_tx, dht_event_rx) = channel(1000); - let key_pair = AuthorityPair::from_seed_slice(&[1; 32]).unwrap(); let test_api = Arc::new(TestApi { - authorities: vec![key_pair.public()], + // Make sure node B identifies node A as an authority. + authorities: vec![node_a_public.into()], }); let network: Arc = Arc::new(Default::default()); let key_store = KeyStore::new(); @@ -306,32 +331,10 @@ fn handle_dht_events_with_value_found_should_call_set_priority_group() { None, ); - // Create sample dht event. - - let authority_id_1 = hash_authority_id(key_pair.public().as_ref()); - let address_1: Multiaddr = "/ip6/2001:db8::".parse().unwrap(); - - let mut serialized_addresses = vec![]; - schema::AuthorityAddresses { - addresses: vec![address_1.to_vec()], - } - .encode(&mut serialized_addresses) - .unwrap(); - - let signature = key_pair.sign(serialized_addresses.as_ref()).encode(); - let mut signed_addresses = vec![]; - schema::SignedAuthorityAddresses { - addresses: serialized_addresses, - signature, - } - .encode(&mut signed_addresses) - .unwrap(); - - let dht_event = sc_network::DhtEvent::ValueFound(vec![(authority_id_1, signed_addresses)]); dht_event_tx.try_send(dht_event).unwrap(); - // Make authority discovery handle the event. let f = |cx: &mut Context<'_>| -> Poll<()> { + // Make authority discovery handle the event. if let Poll::Ready(e) = authority_discovery.handle_dht_events(cx) { panic!("Unexpected error: {:?}", e); } @@ -343,7 +346,7 @@ fn handle_dht_events_with_value_found_should_call_set_priority_group() { network.set_priority_group_call.lock().unwrap()[0], ( "authorities".to_string(), - HashSet::from_iter(vec![address_1.clone()].into_iter()) + HashSet::from_iter(vec![node_a_multiaddr.clone()].into_iter()) ) ); diff --git a/substrate/primitives/core/src/traits.rs b/substrate/primitives/core/src/traits.rs index cca1a8fa8c..133a4a4e19 100644 --- a/substrate/primitives/core/src/traits.rs +++ b/substrate/primitives/core/src/traits.rs @@ -120,7 +120,7 @@ pub trait BareCryptoStore: Send + Sync { /// Given a list of public keys, find the first supported key and /// sign the provided message with that key. /// - /// Returns a tuple of the used key and the signature + /// Returns a tuple of the used key and the SCALE encoded signature. fn sign_with_any( &self, id: KeyTypeId, @@ -144,8 +144,8 @@ pub trait BareCryptoStore: Send + Sync { /// Provided a list of public keys, sign a message with /// each key given that the key is supported. /// - /// Returns a list of `Result`s each representing the signature of each key or - /// a BareCryptoStoreError for non-supported keys. + /// Returns a list of `Result`s each representing the SCALE encoded + /// signature of each key or a BareCryptoStoreError for non-supported keys. fn sign_with_all( &self, id: KeyTypeId,