From 7ed0dd2aa2e055b434bb6b584d8ebd16ed30e01b Mon Sep 17 00:00:00 2001
From: librelois <c@elo.tf>
Date: Sat, 22 Jan 2022 11:55:10 +0100
Subject: [PATCH] =?UTF-8?q?feat(authority-members):=C2=A0add=20param=20Max?=
=?UTF-8?q?KeysLife?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
pallets/authority-members/Cargo.toml | 1 +
pallets/authority-members/src/lib.rs | 51 ++++++++++++++------
pallets/authority-members/src/mock.rs | 3 +-
pallets/authority-members/src/tests.rs | 66 ++++++++++++++++++++++++++
pallets/authority-members/src/types.rs | 10 ++--
runtime/common/src/pallets_config.rs | 26 +++++-----
6 files changed, 120 insertions(+), 37 deletions(-)
diff --git a/pallets/authority-members/Cargo.toml b/pallets/authority-members/Cargo.toml
index c6deae911..26b80e4cb 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 fd9f76a8f..2fbb814bf 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 5da294071..326b26066 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 dcd20ebf1..6f01e8b17 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 e22484a07..5e5fd7007 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 b5fff7fab..226d575d8 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 {
--
GitLab