diff --git a/Cargo.lock b/Cargo.lock index a7d9628b6b70c..13fc0dcd51a77 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4305,6 +4305,7 @@ dependencies = [ "sr-primitives 2.0.0", "sr-staking-primitives 2.0.0", "sr-std 2.0.0", + "srml-authorship 0.1.0", "srml-session 2.0.0", "srml-support 2.0.0", "srml-system 2.0.0", diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 9bda317c3358b..a0ee20988eaf8 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -206,7 +206,7 @@ impl authorship::Trait for Runtime { type FindAuthor = session::FindAccountFromAuthorIndex; type UncleGenerations = UncleGenerations; type FilterUncle = (); - type EventHandler = Staking; + type EventHandler = (Staking, ImOnline); } impl_opaque_keys! { diff --git a/srml/im-online/Cargo.toml b/srml/im-online/Cargo.toml index b62cc2a8f9615..4c7ccf1487d9b 100644 --- a/srml/im-online/Cargo.toml +++ b/srml/im-online/Cargo.toml @@ -6,6 +6,7 @@ edition = "2018" [dependencies] app-crypto = { package = "substrate-application-crypto", path = "../../core/application-crypto", default-features = false } +authorship = { package = "srml-authorship", path = "../authorship", default-features = false } codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false, features = ["derive"] } primitives = { package="substrate-primitives", path = "../../core/primitives", default-features = false } rstd = { package = "sr-std", path = "../../core/sr-std", default-features = false } @@ -24,6 +25,7 @@ offchain = { package = "substrate-offchain", path = "../../core/offchain" } default = ["std", "session/historical"] std = [ "app-crypto/std", + "authorship/std", "codec/std", "primitives/std", "rstd/std", diff --git a/srml/im-online/src/lib.rs b/srml/im-online/src/lib.rs index 6dcaf1d3abb59..46e26ff32fecb 100644 --- a/srml/im-online/src/lib.rs +++ b/srml/im-online/src/lib.rs @@ -214,10 +214,15 @@ decl_storage! { /// The current set of keys that may issue a heartbeat. Keys get(fn keys): Vec; - /// For each session index we keep a mapping of `AuthorityId` + /// 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; + + /// For each session index, we keep a mapping of `T::ValidatorId` to the + /// number of blocks authored by the given authority. + AuthoredBlocks get(fn authored_blocks): double_map SessionIndex, + blake2_256(T::ValidatorId) => u32; } add_extra_genesis { config(keys): Vec; @@ -278,14 +283,63 @@ decl_module! { } } +/// Keep track of number of authored blocks per authority, uncles are counted as +/// well since they're a valid proof of onlineness. +impl authorship::EventHandler for Module { + fn note_author(author: T::ValidatorId) { + Self::note_authorship(author); + } + + fn note_uncle(author: T::ValidatorId, _age: T::BlockNumber) { + Self::note_authorship(author); + } +} + impl Module { + /// Returns `true` if a heartbeat has been received for the authority at + /// `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 { + let current_validators = >::validators(); + + if authority_index >= current_validators.len() as u32 { + return false; + } + + let authority = ¤t_validators[authority_index as usize]; + + Self::is_online_in_current_session_aux(authority_index, authority) + } + + fn is_online_in_current_session_aux(authority_index: AuthIndex, authority: &T::ValidatorId) -> bool { + let current_session = >::current_index(); + + ::exists(¤t_session, &authority_index) || + >::get( + ¤t_session, + authority, + ) != 0 + } + /// Returns `true` if a heartbeat has been received for the authority at `authority_index` in /// the authorities series, during the current session. Otherwise `false`. - pub fn is_online_in_current_session(authority_index: AuthIndex) -> bool { + pub fn received_heartbeat_in_current_session(authority_index: AuthIndex) -> bool { let current_session = >::current_index(); ::exists(¤t_session, &authority_index) } + /// Note that the given authority has authored a block in the current session. + fn note_authorship(author: T::ValidatorId) { + let current_session = >::current_index(); + + >::mutate( + ¤t_session, + author, + |authored| *authored += 1, + ); + } + pub(crate) fn offchain(now: T::BlockNumber) { let next_gossip = >::get(); let check = Self::check_not_yet_gossipped(now, next_gossip); @@ -461,9 +515,7 @@ impl session::OneSessionHandler for Module { let current_validators = >::validators(); for (auth_idx, validator_id) in current_validators.into_iter().enumerate() { - let auth_idx = auth_idx as u32; - let exists = ::exists(¤t_session, &auth_idx); - if !exists { + 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; @@ -477,6 +529,12 @@ impl session::OneSessionHandler for Module { } } + // Remove all received heartbeats and number of authored blocks from the + // current session, they have already been processed and won't be needed + // anymore. + ::remove_prefix(&>::current_index()); + >::remove_prefix(&>::current_index()); + if unresponsive.is_empty() { return; } @@ -489,10 +547,6 @@ impl session::OneSessionHandler for Module { }; T::ReportUnresponsiveness::report_offence(vec![], offence); - - // Remove all received heartbeats from the current session, they have - // already been processed and won't be needed anymore. - ::remove_prefix(&>::current_index()); } fn on_disabled(_i: usize) { diff --git a/srml/im-online/src/mock.rs b/srml/im-online/src/mock.rs index e50e7779e9fcd..233e055f887f1 100644 --- a/srml/im-online/src/mock.rs +++ b/srml/im-online/src/mock.rs @@ -145,6 +145,17 @@ impl session::historical::Trait for Runtime { type FullIdentificationOf = ConvertInto; } +parameter_types! { + pub const UncleGenerations: u32 = 5; +} + +impl authorship::Trait for Runtime { + type FindAuthor = (); + type UncleGenerations = UncleGenerations; + type FilterUncle = (); + type EventHandler = ImOnline; +} + impl Trait for Runtime { type AuthorityId = UintAuthorityId; type Event = (); diff --git a/srml/im-online/src/tests.rs b/srml/im-online/src/tests.rs index 652d751281294..42c1aa02276a5 100644 --- a/srml/im-online/src/tests.rs +++ b/srml/im-online/src/tests.rs @@ -243,3 +243,33 @@ fn should_cleanup_received_heartbeats_on_session_end() { assert!(ImOnline::received_heartbeats(&2, &0).is_empty()); }); } + +#[test] +fn should_mark_online_validator_when_block_is_authored() { + use authorship::EventHandler; + + new_test_ext().execute_with(|| { + advance_session(); + // given + VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3, 4, 5, 6])); + assert_eq!(Session::validators(), Vec::::new()); + // enact the change and buffer another one + advance_session(); + + assert_eq!(Session::current_index(), 2); + assert_eq!(Session::validators(), vec![1, 2, 3]); + + for i in 0..3 { + assert!(!ImOnline::is_online_in_current_session(i)); + } + + // when + ImOnline::note_author(1); + 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)); + }); +}