From 9bd4a125779a146b9d2cf656251fa03588e3f2da Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 1 Nov 2019 09:39:41 +0000 Subject: [PATCH 1/3] Add AllGood event for im_online --- core/network/src/protocol.rs | 1 - core/network/src/protocol/specialization.rs | 2 +- .../src/generic/checked_extrinsic.rs | 3 +- srml/im-online/src/lib.rs | 58 ++++++++----------- srml/im-online/src/tests.rs | 30 +++++----- 5 files changed, 41 insertions(+), 53 deletions(-) diff --git a/core/network/src/protocol.rs b/core/network/src/protocol.rs index 335c2380c8fce..6afb4c8b116ae 100644 --- a/core/network/src/protocol.rs +++ b/core/network/src/protocol.rs @@ -35,7 +35,6 @@ use sr_primitives::traits::{ }; use message::{BlockAnnounce, BlockAttributes, Direction, FromBlock, Message, RequestId}; use message::generic::{Message as GenericMessage, ConsensusMessage}; -use event::Event; use consensus_gossip::{ConsensusGossip, MessageRecipient as GossipMessageRecipient}; use light_dispatch::{LightDispatch, LightDispatchNetwork, RequestData}; use specialization::NetworkSpecialization; diff --git a/core/network/src/protocol/specialization.rs b/core/network/src/protocol/specialization.rs index 37509eeec09d4..1a15c47c87d3a 100644 --- a/core/network/src/protocol/specialization.rs +++ b/core/network/src/protocol/specialization.rs @@ -43,7 +43,7 @@ pub trait NetworkSpecialization: Send + Sync + 'static { /// Called when a network-specific event arrives. #[deprecated(note = "This method is never called; please use `with_dht_event_tx` when building the service")] - fn on_event(&mut self, event: Event) {} + fn on_event(&mut self, _event: Event) {} /// Called on abort. #[deprecated(note = "This method is never called; aborting corresponds to dropping the object")] diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 1e030ea1d8769..510a7ac8c85c9 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -36,8 +36,7 @@ pub struct CheckedExtrinsic { pub function: Call, } -impl traits::Applyable -for +impl traits::Applyable for CheckedExtrinsic where AccountId: Member + MaybeDisplay, diff --git a/srml/im-online/src/lib.rs b/srml/im-online/src/lib.rs index 46e26ff32fecb..201b10503d9e6 100644 --- a/srml/im-online/src/lib.rs +++ b/srml/im-online/src/lib.rs @@ -37,7 +37,7 @@ //! //! ### Public Functions //! -//! - `is_online_in_current_session` - True if the validator sent a heartbeat in the current session. +//! - `is_online` - True if the validator sent a heartbeat in the current session. //! //! ## Usage //! @@ -52,7 +52,7 @@ //! pub struct Module for enum Call where origin: T::Origin { //! pub fn is_online(origin, authority_index: u32) -> Result { //! let _sender = ensure_signed(origin)?; -//! let _is_online = >::is_online_in_current_session(authority_index); +//! let _is_online = >::is_online(authority_index); //! Ok(()) //! } //! } @@ -203,6 +203,8 @@ decl_event!( { /// A new heartbeat was received from `AuthorityId` HeartbeatReceived(AuthorityId), + /// At the end of the session, no offence was committed. + AllGood, } ); @@ -217,7 +219,7 @@ decl_storage! { /// For each session index, we keep a mapping of `AuthIndex` /// to `offchain::OpaqueNetworkState`. ReceivedHeartbeats get(fn received_heartbeats): double_map SessionIndex, - blake2_256(AuthIndex) => Vec; + blake2_256(AuthIndex) => Option>; /// For each session index, we keep a mapping of `T::ValidatorId` to the /// number of blocks authored by the given authority. @@ -249,8 +251,8 @@ decl_module! { &heartbeat.authority_index ); let keys = Keys::::get(); - let public = keys.get(heartbeat.authority_index as usize); - if let (true, Some(public)) = (!exists, public) { + let maybe_public = keys.get(heartbeat.authority_index as usize); + if let (false, Some(public)) = (exists, maybe_public) { let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| { public.verify(&encoded_heartbeat, &signature) }); @@ -300,7 +302,7 @@ impl Module { /// `authority_index` in the authorities series or if the authority has /// authored at least one block, during the current session. Otherwise /// `false`. - pub fn is_online_in_current_session(authority_index: AuthIndex) -> bool { + pub fn is_online(authority_index: AuthIndex) -> bool { let current_validators = >::validators(); if authority_index >= current_validators.len() as u32 { @@ -309,10 +311,10 @@ impl Module { let authority = ¤t_validators[authority_index as usize]; - Self::is_online_in_current_session_aux(authority_index, authority) + Self::is_online_aux(authority_index, authority) } - fn is_online_in_current_session_aux(authority_index: AuthIndex, authority: &T::ValidatorId) -> bool { + fn is_online_aux(authority_index: AuthIndex, authority: &T::ValidatorId) -> bool { let current_session = >::current_index(); ::exists(¤t_session, &authority_index) || @@ -507,27 +509,19 @@ impl session::OneSessionHandler for Module { } fn on_before_session_ending() { - let mut unresponsive = vec![]; - - let current_session = >::current_index(); - + let session_index = >::current_index(); let keys = Keys::::get(); let current_validators = >::validators(); - for (auth_idx, validator_id) in current_validators.into_iter().enumerate() { - if !Self::is_online_in_current_session_aux(auth_idx as u32, &validator_id) { - let full_identification = T::FullIdentificationOf::convert(validator_id.clone()) - .expect( - "we got the validator_id from current_validators; - current_validators is set of currently acting validators; - the mapping between the validator id and its full identification should be valid; - thus `FullIdentificationOf::convert` can't return `None`; - qed", - ); - - unresponsive.push((validator_id, full_identification)); - } - } + let offenders = current_validators.into_iter().enumerate() + .filter(|(index, id)| + !Self::is_online_aux(*index as u32, id) + ).filter_map(|(_, id)| + T::FullIdentificationOf::convert(id.clone()) + .map(|full_id| + (id, full_id) + ) + ).collect::>(); // Remove all received heartbeats and number of authored blocks from the // current session, they have already been processed and won't be needed @@ -535,17 +529,13 @@ impl session::OneSessionHandler for Module { ::remove_prefix(&>::current_index()); >::remove_prefix(&>::current_index()); - if unresponsive.is_empty() { + if offenders.is_empty() { + Self::deposit_event(RawEvent::AllGood); return; } let validator_set_count = keys.len() as u32; - let offence = UnresponsivenessOffence { - session_index: current_session, - validator_set_count, - offenders: unresponsive, - }; - + let offence = UnresponsivenessOffence { session_index, validator_set_count, offenders }; T::ReportUnresponsiveness::report_offence(vec![], offence); } @@ -559,7 +549,7 @@ impl support::unsigned::ValidateUnsigned for Module { fn validate_unsigned(call: &Self::Call) -> TransactionValidity { if let Call::heartbeat(heartbeat, signature) = call { - if >::is_online_in_current_session(heartbeat.authority_index) { + if >::is_online(heartbeat.authority_index) { // we already received a heartbeat for this authority return InvalidTransaction::Stale.into(); } diff --git a/srml/im-online/src/tests.rs b/srml/im-online/src/tests.rs index 42c1aa02276a5..57f2e4008b61c 100644 --- a/srml/im-online/src/tests.rs +++ b/srml/im-online/src/tests.rs @@ -134,25 +134,25 @@ fn should_mark_online_validator_when_heartbeat_is_received() { assert_eq!(Session::current_index(), 2); assert_eq!(Session::validators(), vec![1, 2, 3]); - assert!(!ImOnline::is_online_in_current_session(0)); - assert!(!ImOnline::is_online_in_current_session(1)); - assert!(!ImOnline::is_online_in_current_session(2)); + assert!(!ImOnline::is_online(0)); + assert!(!ImOnline::is_online(1)); + assert!(!ImOnline::is_online(2)); // when let _ = heartbeat(1, 2, 0, 1.into()).unwrap(); // then - assert!(ImOnline::is_online_in_current_session(0)); - assert!(!ImOnline::is_online_in_current_session(1)); - assert!(!ImOnline::is_online_in_current_session(2)); + assert!(ImOnline::is_online(0)); + assert!(!ImOnline::is_online(1)); + assert!(!ImOnline::is_online(2)); // and when let _ = heartbeat(1, 2, 2, 3.into()).unwrap(); // then - assert!(ImOnline::is_online_in_current_session(0)); - assert!(!ImOnline::is_online_in_current_session(1)); - assert!(ImOnline::is_online_in_current_session(2)); + assert!(ImOnline::is_online(0)); + assert!(!ImOnline::is_online(1)); + assert!(ImOnline::is_online(2)); }); } @@ -233,14 +233,14 @@ fn should_cleanup_received_heartbeats_on_session_end() { let _ = heartbeat(1, 2, 0, 1.into()).unwrap(); // the heartbeat is stored - assert!(!ImOnline::received_heartbeats(&2, &0).is_empty()); + assert!(!ImOnline::received_heartbeats(&2, &0).is_none()); advance_session(); // after the session has ended we have already processed the heartbeat // message, so any messages received on the previous session should have // been pruned. - assert!(ImOnline::received_heartbeats(&2, &0).is_empty()); + assert!(ImOnline::received_heartbeats(&2, &0).is_none()); }); } @@ -260,7 +260,7 @@ fn should_mark_online_validator_when_block_is_authored() { assert_eq!(Session::validators(), vec![1, 2, 3]); for i in 0..3 { - assert!(!ImOnline::is_online_in_current_session(i)); + assert!(!ImOnline::is_online(i)); } // when @@ -268,8 +268,8 @@ fn should_mark_online_validator_when_block_is_authored() { ImOnline::note_uncle(2, 0); // then - assert!(ImOnline::is_online_in_current_session(0)); - assert!(ImOnline::is_online_in_current_session(1)); - assert!(!ImOnline::is_online_in_current_session(2)); + assert!(ImOnline::is_online(0)); + assert!(ImOnline::is_online(1)); + assert!(!ImOnline::is_online(2)); }); } From f6c888a4f66a60ec57871aa82848930d772d617c Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 1 Nov 2019 10:04:52 +0000 Subject: [PATCH 2/3] Another event just in case. --- srml/im-online/src/lib.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/srml/im-online/src/lib.rs b/srml/im-online/src/lib.rs index 201b10503d9e6..fa970f4997891 100644 --- a/srml/im-online/src/lib.rs +++ b/srml/im-online/src/lib.rs @@ -200,11 +200,14 @@ pub trait Trait: system::Trait + session::historical::Trait { decl_event!( pub enum Event where ::AuthorityId, + IdentificationTuple = IdentificationTuple, { /// A new heartbeat was received from `AuthorityId` HeartbeatReceived(AuthorityId), /// At the end of the session, no offence was committed. AllGood, + /// At the end of the session, at least once validator was found to be offline. + SomeOffline(Vec), } ); @@ -517,11 +520,8 @@ impl session::OneSessionHandler for Module { .filter(|(index, id)| !Self::is_online_aux(*index as u32, id) ).filter_map(|(_, id)| - T::FullIdentificationOf::convert(id.clone()) - .map(|full_id| - (id, full_id) - ) - ).collect::>(); + T::FullIdentificationOf::convert(id.clone()).map(|full_id| (id, full_id)) + ).collect::>>(); // Remove all received heartbeats and number of authored blocks from the // current session, they have already been processed and won't be needed @@ -531,12 +531,13 @@ impl session::OneSessionHandler for Module { if offenders.is_empty() { Self::deposit_event(RawEvent::AllGood); - return; - } + } else { + Self::deposit_event(RawEvent::SomeOffline(offenders.clone())); - let validator_set_count = keys.len() as u32; - let offence = UnresponsivenessOffence { session_index, validator_set_count, offenders }; - T::ReportUnresponsiveness::report_offence(vec![], offence); + let validator_set_count = keys.len() as u32; + let offence = UnresponsivenessOffence { session_index, validator_set_count, offenders }; + T::ReportUnresponsiveness::report_offence(vec![], offence); + } } fn on_disabled(_i: usize) { From f2dba9972df0fb0c2430ab8367162ab919777ad9 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Fri, 1 Nov 2019 10:10:21 +0000 Subject: [PATCH 3/3] Bump runtime --- node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index a0ee20988eaf8..5cf6663904142 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -81,7 +81,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to equal spec_version. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 190, + spec_version: 191, impl_version: 191, apis: RUNTIME_API_VERSIONS, };