Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add events for im_online #3991

Merged
merged 4 commits into from
Nov 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion core/network/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion core/network/src/protocol/specialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub trait NetworkSpecialization<B: BlockT>: 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")]
Expand Down
3 changes: 1 addition & 2 deletions core/sr-primitives/src/generic/checked_extrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ pub struct CheckedExtrinsic<AccountId, Call, Extra> {
pub function: Call,
}

impl<AccountId, Call, Extra, Origin> traits::Applyable
for
impl<AccountId, Call, Extra, Origin> traits::Applyable for
CheckedExtrinsic<AccountId, Call, Extra>
where
AccountId: Member + MaybeDisplay,
Expand Down
2 changes: 1 addition & 1 deletion node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
67 changes: 29 additions & 38 deletions srml/im-online/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//!
Expand All @@ -52,7 +52,7 @@
//! pub struct Module<T: Trait> for enum Call where origin: T::Origin {
//! pub fn is_online(origin, authority_index: u32) -> Result {
//! let _sender = ensure_signed(origin)?;
//! let _is_online = <im_online::Module<T>>::is_online_in_current_session(authority_index);
//! let _is_online = <im_online::Module<T>>::is_online(authority_index);
//! Ok(())
//! }
//! }
Expand Down Expand Up @@ -200,9 +200,14 @@ pub trait Trait: system::Trait + session::historical::Trait {
decl_event!(
pub enum Event<T> where
<T as Trait>::AuthorityId,
IdentificationTuple = IdentificationTuple<T>,
{
/// 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<IdentificationTuple>),
}
);

Expand All @@ -217,7 +222,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<u8>;
blake2_256(AuthIndex) => Option<Vec<u8>>;

/// For each session index, we keep a mapping of `T::ValidatorId` to the
/// number of blocks authored by the given authority.
Expand Down Expand Up @@ -249,8 +254,8 @@ decl_module! {
&heartbeat.authority_index
);
let keys = Keys::<T>::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)
});
Expand Down Expand Up @@ -300,7 +305,7 @@ impl<T: Trait> Module<T> {
/// `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 = <session::Module<T>>::validators();

if authority_index >= current_validators.len() as u32 {
Expand All @@ -309,10 +314,10 @@ impl<T: Trait> Module<T> {

let authority = &current_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 = <session::Module<T>>::current_index();

<ReceivedHeartbeats>::exists(&current_session, &authority_index) ||
Expand Down Expand Up @@ -507,46 +512,32 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> {
}

fn on_before_session_ending() {
let mut unresponsive = vec![];

let current_session = <session::Module<T>>::current_index();

let session_index = <session::Module<T>>::current_index();
let keys = Keys::<T>::get();
let current_validators = <session::Module<T>>::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(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a weak and dangerous proof; it should never have been here.

nothing should be able to panic in initialize_block/finalize_block.

"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::<Vec<IdentificationTuple<T>>>();

// Remove all received heartbeats and number of authored blocks from the
// current session, they have already been processed and won't be needed
// anymore.
<ReceivedHeartbeats>::remove_prefix(&<session::Module<T>>::current_index());
<AuthoredBlocks<T>>::remove_prefix(&<session::Module<T>>::current_index());

if unresponsive.is_empty() {
return;
}

let validator_set_count = keys.len() as u32;
let offence = UnresponsivenessOffence {
session_index: current_session,
validator_set_count,
offenders: unresponsive,
};
if offenders.is_empty() {
Self::deposit_event(RawEvent::AllGood);
} else {
Self::deposit_event(RawEvent::SomeOffline(offenders.clone()));

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) {
Expand All @@ -559,7 +550,7 @@ impl<T: Trait> support::unsigned::ValidateUnsigned for Module<T> {

fn validate_unsigned(call: &Self::Call) -> TransactionValidity {
if let Call::heartbeat(heartbeat, signature) = call {
if <Module<T>>::is_online_in_current_session(heartbeat.authority_index) {
if <Module<T>>::is_online(heartbeat.authority_index) {
// we already received a heartbeat for this authority
return InvalidTransaction::Stale.into();
}
Expand Down
30 changes: 15 additions & 15 deletions srml/im-online/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});
}

Expand Down Expand Up @@ -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());
});
}

Expand All @@ -260,16 +260,16 @@ 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
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));
assert!(ImOnline::is_online(0));
assert!(ImOnline::is_online(1));
assert!(!ImOnline::is_online(2));
});
}