diff --git a/pallets/authority-members/Cargo.toml b/pallets/authority-members/Cargo.toml index c6deae9112a89c06c570b96fd7f20598430f7594..26b80e4cb5530ed6ce6ed43763628657e79a4b5c 100644 --- a/pallets/authority-members/Cargo.toml +++ b/pallets/authority-members/Cargo.toml @@ -57,6 +57,7 @@ branch = 'duniter-monthly-2022-01' [dependencies.pallet-session] default-features = false +features = ["historical"] git = 'https://github.com/librelois/substrate.git' branch = 'duniter-monthly-2022-01' diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs index fd9f76a8f7e0b0f5e2f8da20c6d6391ed5a58bc6..2fbb814bfa42370e5cf95458462e46885b97cac7 100644 --- a/pallets/authority-members/src/lib.rs +++ b/pallets/authority-members/src/lib.rs @@ -66,9 +66,11 @@ pub mod pallet { type IsMember: IsMember<Self::MemberId>; type OnRemovedMember: OnRemovedMember<Self::MemberId>; type OwnerKeyOf: Convert<Self::MemberId, Option<Self::AccountId>>; - type MemberId: Copy + MaybeSerializeDeserialize + Parameter + Ord; + #[pallet::constant] + type MaxKeysLife: Get<SessionIndex>; #[pallet::constant] type MaxOfflineSessions: Get<SessionIndex>; + type MemberId: Copy + MaybeSerializeDeserialize + Parameter + Ord; type RefreshValidatorIdOrigin: EnsureOrigin<Self::Origin>; type RemoveMemberOrigin: EnsureOrigin<Self::Origin>; } @@ -93,7 +95,10 @@ pub mod pallet { impl<T: Config> GenesisBuild<T> for GenesisConfig<T> { fn build(&self) { for (member_id, (validator_id, _is_online)) in &self.initial_authorities { - Members::<T>::insert(member_id, MemberData::new_genesis(validator_id.clone())); + Members::<T>::insert( + member_id, + MemberData::new_genesis(T::MaxKeysLife::get(), validator_id.clone()), + ); } let mut members_ids = self .initial_authorities @@ -110,7 +115,8 @@ pub mod pallet { .collect::<Vec<T::MemberId>>(); members_ids.sort(); - OnlineAuthorities::<T>::put(members_ids); + OnlineAuthorities::<T>::put(members_ids.clone()); + MustRotateKeysBefore::<T>::insert(T::MaxKeysLife::get(), members_ids); } } @@ -138,6 +144,11 @@ pub mod pallet { pub type MembersExpireOn<T: Config> = StorageMap<_, Blake2_128Concat, SessionIndex, Vec<T::MemberId>, ValueQuery>; + #[pallet::storage] + #[pallet::getter(fn must_rotate_keys_before)] + pub type MustRotateKeysBefore<T: Config> = + StorageMap<_, Blake2_128Concat, SessionIndex, Vec<T::MemberId>, ValueQuery>; + // HOOKS // // EVENTS // @@ -271,16 +282,22 @@ pub mod pallet { } .dispatch_bypass_filter(origin)?; - let expire_on_session = pallet_session::Pallet::<T>::current_index() - .saturating_add(T::MaxOfflineSessions::get()); + let current_session_index = pallet_session::Pallet::<T>::current_index(); + let expire_on_session = + current_session_index.saturating_add(T::MaxOfflineSessions::get()); + let must_rotate_keys_before = + current_session_index.saturating_add(T::MaxKeysLife::get()); Members::<T>::mutate_exists(member_id, |member_data_opt| { let mut member_data = member_data_opt.get_or_insert(MemberData { expire_on_session, + must_rotate_keys_before, validator_id: validator_id.clone(), }); + member_data.must_rotate_keys_before = must_rotate_keys_before; member_data.validator_id = validator_id; }); + MustRotateKeysBefore::<T>::append(must_rotate_keys_before, member_id); Ok(().into()) } @@ -299,17 +316,14 @@ pub mod pallet { return Err(Error::<T>::NotMember.into()); } - let expire_on_session = pallet_session::Pallet::<T>::current_index() - .saturating_add(T::MaxOfflineSessions::get()); - Members::<T>::mutate(member_id, |member_data_opt| { - let validator_id_clone = validator_id.clone(); - member_data_opt - .get_or_insert(MemberData::new(validator_id, expire_on_session)) - .validator_id = validator_id_clone; - }); - - Ok(().into()) + if let Some(member_data) = member_data_opt { + member_data.validator_id = validator_id; + Ok(().into()) + } else { + Err(Error::<T>::MemberNotFound.into()) + } + }) } #[pallet::weight(0)] pub fn remove_member( @@ -360,6 +374,13 @@ pub mod pallet { } } } + for member_id in MustRotateKeysBefore::<T>::take(current_session_index) { + if let Some(member_data) = Members::<T>::get(member_id) { + if member_data.must_rotate_keys_before == current_session_index { + Self::do_remove_member(member_id); + } + } + } } fn insert_in(member_id: T::MemberId) -> bool { let not_already_inserted = IncomingAuthorities::<T>::mutate(|members_ids| { diff --git a/pallets/authority-members/src/mock.rs b/pallets/authority-members/src/mock.rs index 5da294071911a5eca2cbf750710ed7c6319b75c9..326b26066f82f24905e2ba66b4b9913a5052c687 100644 --- a/pallets/authority-members/src/mock.rs +++ b/pallets/authority-members/src/mock.rs @@ -149,8 +149,9 @@ impl IsMember<u64> for TestIsSmithMember { impl pallet_authority_members::Config for Test { type Event = Event; - type KeysWrapper = MockSessionKeys; + type KeysWrapper = MockSessionKeys; type IsMember = TestIsSmithMember; + type MaxKeysLife = ConstU32<5>; type MaxOfflineSessions = ConstU32<2>; type MemberId = u64; type OnRemovedMember = (); diff --git a/pallets/authority-members/src/tests.rs b/pallets/authority-members/src/tests.rs index dcd20ebf139031a28af22e1304108d86a8b84b7c..6f01e8b174e5ac4d2b95c5976f8c71f14b763ff9 100644 --- a/pallets/authority-members/src/tests.rs +++ b/pallets/authority-members/src/tests.rs @@ -34,6 +34,7 @@ fn test_genesis_build() { AuthorityMembers::member(3), Some(MemberData { expire_on_session: 0, + must_rotate_keys_before: 5, validator_id: 3, }) ); @@ -41,6 +42,7 @@ fn test_genesis_build() { AuthorityMembers::member(6), Some(MemberData { expire_on_session: 0, + must_rotate_keys_before: 5, validator_id: 6, }) ); @@ -48,6 +50,7 @@ fn test_genesis_build() { AuthorityMembers::member(9), Some(MemberData { expire_on_session: 0, + must_rotate_keys_before: 5, validator_id: 9, }) ); @@ -68,6 +71,63 @@ fn test_new_session_shoud_not_change_authorities_set() { }); } +#[test] +fn test_max_keys_life_rule() { + new_test_ext(3).execute_with(|| { + run_to_block(11); + + assert_eq!(Session::current_index(), 2); + assert_eq!(Session::validators(), vec![3, 6, 9]); + + // Member 3 and 6 rotate their sessions keys + assert_ok!(AuthorityMembers::set_session_keys( + Origin::signed(3), + 3, + UintAuthorityId(3).into() + ),); + assert_ok!(AuthorityMembers::set_session_keys( + Origin::signed(6), + 6, + UintAuthorityId(6).into() + ),); + + // Verify state + assert_eq!( + AuthorityMembers::member(3), + Some(MemberData { + expire_on_session: 0, + must_rotate_keys_before: 7, + validator_id: 3, + }) + ); + assert_eq!( + AuthorityMembers::member(6), + Some(MemberData { + expire_on_session: 0, + must_rotate_keys_before: 7, + validator_id: 6, + }) + ); + + // Member 9 should expire at session 5 + run_to_block(26); + assert_eq!(Session::current_index(), 5); + assert_eq!(AuthorityMembers::online(), vec![3, 6]); + assert_eq!(AuthorityMembers::member(9), None); + + // Member 9 should be "deprogrammed" but still in the authorities set for 1 session + assert_eq!(Session::queued_keys().len(), 2); + assert_eq!(Session::queued_keys()[0].0, 3); + assert_eq!(Session::queued_keys()[1].0, 6); + assert_eq!(Session::validators(), vec![3, 6, 9]); + + // Member 9 should be **effectively** out at session 6 + run_to_block(31); + assert_eq!(Session::current_index(), 6); + assert_eq!(Session::validators(), vec![3, 6]); + }); +} + #[test] fn test_go_offline() { new_test_ext(3).execute_with(|| { @@ -84,6 +144,7 @@ fn test_go_offline() { AuthorityMembers::member(9), Some(MemberData { expire_on_session: 0, + must_rotate_keys_before: 5, validator_id: 9, }) ); @@ -94,6 +155,7 @@ fn test_go_offline() { AuthorityMembers::member(9), Some(MemberData { expire_on_session: 4, + must_rotate_keys_before: 5, validator_id: 9, }) ); @@ -133,6 +195,7 @@ fn test_go_online() { AuthorityMembers::member(12), Some(MemberData { expire_on_session: 2, + must_rotate_keys_before: 5, validator_id: 12, }) ); @@ -148,6 +211,7 @@ fn test_go_online() { AuthorityMembers::member(12), Some(MemberData { expire_on_session: 2, + must_rotate_keys_before: 5, validator_id: 12, }) ); @@ -196,6 +260,7 @@ fn test_go_online_then_go_offline_in_same_session() { AuthorityMembers::member(12), Some(MemberData { expire_on_session: 2, + must_rotate_keys_before: 5, validator_id: 12, }) ); @@ -223,6 +288,7 @@ fn test_go_offline_then_go_online_in_same_session() { AuthorityMembers::member(9), Some(MemberData { expire_on_session: 0, + must_rotate_keys_before: 5, validator_id: 9, }) ); diff --git a/pallets/authority-members/src/types.rs b/pallets/authority-members/src/types.rs index e22484a072fef49b4f7eb070733cee84277c0f3c..5e5fd70074dff37c90a93a0ba251460e7286244d 100644 --- a/pallets/authority-members/src/types.rs +++ b/pallets/authority-members/src/types.rs @@ -26,19 +26,15 @@ use sp_staking::SessionIndex; #[derive(Encode, Decode, Clone, PartialEq, Eq, TypeInfo)] pub struct MemberData<ValidatorId: Decode + Encode + TypeInfo> { pub expire_on_session: SessionIndex, + pub must_rotate_keys_before: SessionIndex, pub validator_id: ValidatorId, } impl<ValidatorId: Decode + Encode + TypeInfo> MemberData<ValidatorId> { - pub fn new(validator_id: ValidatorId, expire_on_session: SessionIndex) -> Self { - MemberData { - expire_on_session, - validator_id, - } - } - pub fn new_genesis(validator_id: ValidatorId) -> Self { + pub fn new_genesis(must_rotate_keys_before: SessionIndex, validator_id: ValidatorId) -> Self { MemberData { expire_on_session: 0, + must_rotate_keys_before, validator_id, } } diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index b5fff7fab9c8cc2b61dad7a32c861238fb2abf18..226d575d8016d1fbb26fde15224e216b99f8ae25 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -176,13 +176,24 @@ macro_rules! pallets_config { impl pallet_authority_discovery::Config for Runtime { type MaxAuthorities = MaxAuthorities; } + impl pallet_authority_members::Config for Runtime { + type Event = Event; + type KeysWrapper = opaque::SessionKeysWrapper; + type IsMember = SmithsMembership; + type OnRemovedMember = OnRemovedAUthorityMemberHandler<Runtime>; + type OwnerKeyOf = OwnerKeyOfImpl<Runtime>; + type MemberId = IdtyIndex; + type MaxKeysLife = frame_support::pallet_prelude::ConstU32<1_000>; + type MaxOfflineSessions = frame_support::pallet_prelude::ConstU32<100>; + type RefreshValidatorIdOrigin = EnsureRoot<Self::AccountId>; + type RemoveMemberOrigin = EnsureRoot<Self::AccountId>; + } impl pallet_authorship::Config for Runtime { type FindAuthor = pallet_session::FindAccountFromAuthorIndex<Self, Babe>; type UncleGenerations = UncleGenerations; type FilterUncle = (); type EventHandler = ImOnline; } - impl pallet_im_online::Config for Runtime { type AuthorityId = ImOnlineId; type Event = Event; @@ -215,7 +226,6 @@ macro_rules! pallets_config { type FullIdentification = ValidatorFullIdentification; type FullIdentificationOf = FullIdentificationOfImpl; } - impl pallet_grandpa::Config for Runtime { type Event = Event; type Call = Call; @@ -237,18 +247,6 @@ macro_rules! pallets_config { type MaxAuthorities = MaxAuthorities; } - impl pallet_authority_members::Config for Runtime { - type Event = Event; - type KeysWrapper = opaque::SessionKeysWrapper; - type IsMember = SmithsMembership; - type OnRemovedMember = OnRemovedAUthorityMemberHandler<Runtime>; - type OwnerKeyOf = OwnerKeyOfImpl<Runtime>; - type MemberId = IdtyIndex; - type MaxOfflineSessions = frame_support::pallet_prelude::ConstU32<100>; - type RefreshValidatorIdOrigin = EnsureRoot<Self::AccountId>; - type RemoveMemberOrigin = EnsureRoot<Self::AccountId>; - } - // UTILITIES // impl pallet_utility::Config for Runtime {