From 10b032bb0ded1c95ca13f0d4dd4fa52f314bab05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com> Date: Tue, 27 Aug 2019 11:57:35 +0200 Subject: [PATCH] Make sure that `on_before_session_ending` is called (#3487) * Make sure that `on_before_session_ending` is called * Move the call above the validtor set being set * Bump spec_version --- substrate/node/runtime/src/lib.rs | 4 ++-- substrate/srml/session/src/lib.rs | 29 +++++++++++++++++++++++++++++ substrate/srml/session/src/mock.rs | 14 ++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index 33088260b7d..e121967511b 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -79,8 +79,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: 151, - impl_version: 153, + spec_version: 152, + impl_version: 152, apis: RUNTIME_API_VERSIONS, }; diff --git a/substrate/srml/session/src/lib.rs b/substrate/srml/session/src/lib.rs index 6a09591e014..c474962c650 100644 --- a/substrate/srml/session/src/lib.rs +++ b/substrate/srml/session/src/lib.rs @@ -481,6 +481,9 @@ impl<T: Trait> Module<T> { let changed = QueuedChanged::get(); + // Inform the session handlers that a session is going to end. + T::SessionHandler::on_before_session_ending(); + // Get queued session keys and validators. let session_keys = <QueuedKeys<T>>::get(); let validators = session_keys.iter() @@ -663,6 +666,7 @@ mod tests { use mock::{ NEXT_VALIDATORS, SESSION_CHANGED, TEST_SESSION_CHANGED, authorities, force_new_session, set_next_validators, set_session_length, session_changed, Test, Origin, System, Session, + reset_before_session_end_called, before_session_end_called, }; fn new_test_ext() -> runtime_io::TestExternalities<Blake2Hasher> { @@ -715,6 +719,8 @@ mod tests { #[test] fn authorities_should_track_validators() { + reset_before_session_end_called(); + with_externalities(&mut new_test_ext(), || { set_next_validators(vec![1, 2]); force_new_session(); @@ -725,6 +731,8 @@ mod tests { ]); assert_eq!(Session::validators(), vec![1, 2, 3]); assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2), UintAuthorityId(3)]); + assert!(before_session_end_called()); + reset_before_session_end_called(); force_new_session(); initialize_block(2); @@ -734,6 +742,8 @@ mod tests { ]); assert_eq!(Session::validators(), vec![1, 2]); assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2)]); + assert!(before_session_end_called()); + reset_before_session_end_called(); set_next_validators(vec![1, 2, 4]); assert_ok!(Session::set_keys(Origin::signed(4), UintAuthorityId(4).into(), vec![])); @@ -746,6 +756,7 @@ mod tests { ]); assert_eq!(Session::validators(), vec![1, 2]); assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2)]); + assert!(before_session_end_called()); force_new_session(); initialize_block(4); @@ -827,45 +838,63 @@ mod tests { #[test] fn session_changed_flag_works() { + reset_before_session_end_called(); + with_externalities(&mut new_test_ext(), || { TEST_SESSION_CHANGED.with(|l| *l.borrow_mut() = true); force_new_session(); initialize_block(1); assert!(!session_changed()); + assert!(before_session_end_called()); + reset_before_session_end_called(); force_new_session(); initialize_block(2); assert!(!session_changed()); + assert!(before_session_end_called()); + reset_before_session_end_called(); Session::disable_index(0); force_new_session(); initialize_block(3); assert!(!session_changed()); + assert!(before_session_end_called()); + reset_before_session_end_called(); force_new_session(); initialize_block(4); assert!(session_changed()); + assert!(before_session_end_called()); + reset_before_session_end_called(); force_new_session(); initialize_block(5); assert!(!session_changed()); + assert!(before_session_end_called()); + reset_before_session_end_called(); assert_ok!(Session::set_keys(Origin::signed(2), UintAuthorityId(5).into(), vec![])); force_new_session(); initialize_block(6); assert!(!session_changed()); + assert!(before_session_end_called()); + reset_before_session_end_called(); // changing the keys of a validator leads to change. assert_ok!(Session::set_keys(Origin::signed(69), UintAuthorityId(69).into(), vec![])); force_new_session(); initialize_block(7); assert!(session_changed()); + assert!(before_session_end_called()); + reset_before_session_end_called(); // while changing the keys of a non-validator does not. force_new_session(); initialize_block(7); assert!(!session_changed()); + assert!(before_session_end_called()); + reset_before_session_end_called(); }); } diff --git a/substrate/srml/session/src/mock.rs b/substrate/srml/session/src/mock.rs index 13fcb54754c..445076be652 100644 --- a/substrate/srml/session/src/mock.rs +++ b/substrate/srml/session/src/mock.rs @@ -53,6 +53,8 @@ thread_local! { pub static SESSION_CHANGED: RefCell<bool> = RefCell::new(false); pub static TEST_SESSION_CHANGED: RefCell<bool> = RefCell::new(false); pub static DISABLED: RefCell<bool> = RefCell::new(false); + // Stores if `on_before_session_end` was called + pub static BEFORE_SESSION_END_CALLED: RefCell<bool> = RefCell::new(false); } pub struct TestShouldEndSession; @@ -81,6 +83,9 @@ impl SessionHandler<u64> for TestSessionHandler { fn on_disabled(_validator_index: usize) { DISABLED.with(|l| *l.borrow_mut() = true) } + fn on_before_session_ending() { + BEFORE_SESSION_END_CALLED.with(|b| *b.borrow_mut() = true); + } } pub struct TestOnSessionEnding; @@ -134,8 +139,17 @@ pub fn set_next_validators(next: Vec<u64>) { NEXT_VALIDATORS.with(|v| *v.borrow_mut() = next); } +pub fn before_session_end_called() -> bool { + BEFORE_SESSION_END_CALLED.with(|b| *b.borrow()) +} + +pub fn reset_before_session_end_called() { + BEFORE_SESSION_END_CALLED.with(|b| *b.borrow_mut() = false); +} + #[derive(Clone, Eq, PartialEq)] pub struct Test; + parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; -- GitLab