From 656a3d33031475b858904bd9586bd865e1dd4623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= <andre.beat@gmail.com> Date: Mon, 30 Sep 2019 17:03:40 +0100 Subject: [PATCH] srml: im-online: fix received heartbeats pruning (#3724) * srml: im-online: fix pruning of received heartbeats * srml: im-online: add test for received heartbeats pruning * srml: im-online: remove unused variables from test * node: bump spec_version --- substrate/node/runtime/src/lib.rs | 4 ++-- substrate/srml/im-online/src/lib.rs | 7 ++++--- substrate/srml/im-online/src/tests.rs | 29 +++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index a8999576a7b..c7314270337 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -84,8 +84,8 @@ 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: 165, - impl_version: 167, + spec_version: 166, + impl_version: 166, apis: RUNTIME_API_VERSIONS, }; diff --git a/substrate/srml/im-online/src/lib.rs b/substrate/srml/im-online/src/lib.rs index 611a2aaa16a..1a093497017 100644 --- a/substrate/srml/im-online/src/lib.rs +++ b/substrate/srml/im-online/src/lib.rs @@ -437,9 +437,6 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> { fn on_new_session<'a, I: 'a>(_changed: bool, validators: I, _queued_validators: I) where I: Iterator<Item=(&'a T::AccountId, T::AuthorityId)> { - // Reset heartbeats - <ReceivedHeartbeats>::remove_prefix(&<session::Module<T>>::current_index()); - // Tell the offchain worker to start making the next session's heartbeats. <GossipAt<T>>::put(<system::Module<T>>::block_number()); @@ -484,6 +481,10 @@ impl<T: Trait> session::OneSessionHandler<T::AccountId> for Module<T> { }; 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. + <ReceivedHeartbeats>::remove_prefix(&<session::Module<T>>::current_index()); } fn on_disabled(_i: usize) { diff --git a/substrate/srml/im-online/src/tests.rs b/substrate/srml/im-online/src/tests.rs index 4f6702780ff..c6405c34fa6 100644 --- a/substrate/srml/im-online/src/tests.rs +++ b/substrate/srml/im-online/src/tests.rs @@ -216,3 +216,32 @@ fn should_generate_heartbeats() { }); }); } + +#[test] +fn should_cleanup_received_heartbeats_on_session_end() { + with_externalities(&mut new_test_ext(), || { + advance_session(); + + VALIDATORS.with(|l| *l.borrow_mut() = Some(vec![1, 2, 3])); + assert_eq!(Session::validators(), Vec::<u64>::new()); + + // enact the change and buffer another one + advance_session(); + + assert_eq!(Session::current_index(), 2); + assert_eq!(Session::validators(), vec![1, 2, 3]); + + // send an heartbeat from authority id 0 at session 2 + let _ = heartbeat(1, 2, 0, 1.into()).unwrap(); + + // the heartbeat is stored + assert!(!ImOnline::received_heartbeats(&2, &0).is_empty()); + + 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()); + }); +} -- GitLab