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 3 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
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));
});
}