From b575a32aa1c070721c573ba0457b2eb2348e00a8 Mon Sep 17 00:00:00 2001 From: tuxmain <tuxmain@zettascript.org> Date: Tue, 8 Aug 2023 18:31:03 +0200 Subject: [PATCH] feat: remove session key and authority expiration --- docs/user/smith.md | 3 +- pallets/authority-members/README.md | 4 - pallets/authority-members/src/lib.rs | 69 +--------- pallets/authority-members/src/mock.rs | 2 - pallets/authority-members/src/tests.rs | 170 +++---------------------- pallets/authority-members/src/types.rs | 13 +- runtime/common/src/pallets_config.rs | 2 - 7 files changed, 25 insertions(+), 238 deletions(-) diff --git a/docs/user/smith.md b/docs/user/smith.md index 7aff6714d..02411c2f0 100644 --- a/docs/user/smith.md +++ b/docs/user/smith.md @@ -38,8 +38,7 @@ rotateKeys can be called anytime you want. Then you have to call setSessionKeys Once all the previous steps are completed, you can start to actually author blocks. -1. In the UI: developer > extrinsics > YOUR_SMITH_ACCOUNT > authorityMembers > goOnline() -2. Every less than 2 months, make the RPC call author.rotateKeys then the extrinsic authorityMembers.setSessionKeys. +In the UI: developer > extrinsics > YOUR_SMITH_ACCOUNT > authorityMembers > goOnline() If you're not able to monitor, reboot, act on your node, goOffline() to avoid penalty to the blockchain and to you. It will take effect after 2h, so please do it in advance if you plan to disconnect your server soon. goOnline can always be called after this, but after 100 days without authoring blocks you will loose your smith membership. diff --git a/pallets/authority-members/README.md b/pallets/authority-members/README.md index 133701b9c..37210275f 100644 --- a/pallets/authority-members/README.md +++ b/pallets/authority-members/README.md @@ -16,10 +16,6 @@ To become part of Duniter authorities, one has to complete these steps: Then one can "go online" and "go offline" to enter or leave two sessions after. -## Staying in the set of authorities - -If a smith is offline more than `MaxOfflineSessions`, he leaves the set of authorities. - ## Some vocabulary *Smiths* are people allowed to forge blocks, but in details this is: diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs index d4f299c36..1102c85e7 100644 --- a/pallets/authority-members/src/lib.rs +++ b/pallets/authority-members/src/lib.rs @@ -75,10 +75,6 @@ pub mod pallet { /// Max number of authorities allowed #[pallet::constant] type MaxAuthorities: Get<u32>; - #[pallet::constant] - type MaxKeysLife: Get<SessionIndex>; - #[pallet::constant] - type MaxOfflineSessions: Get<SessionIndex>; type MemberId: Copy + Ord + MaybeSerializeDeserialize + Parameter; type MemberIdOf: Convert<Self::AccountId, Option<Self::MemberId>>; type RemoveMemberOrigin: EnsureOrigin<Self::RuntimeOrigin>; @@ -106,10 +102,7 @@ pub mod pallet { impl<T: Config> GenesisBuild<T> for GenesisConfig<T> { fn build(&self) { for (member_id, (account_id, _is_online)) in &self.initial_authorities { - Members::<T>::insert( - member_id, - MemberData::new_genesis(T::MaxKeysLife::get(), account_id.to_owned()), - ); + Members::<T>::insert(member_id, MemberData::new_genesis(account_id.to_owned())); } let mut members_ids = self .initial_authorities @@ -128,7 +121,6 @@ pub mod pallet { AuthoritiesCounter::<T>::put(members_ids.len() as u32); OnlineAuthorities::<T>::put(members_ids.clone()); - MustRotateKeysBefore::<T>::insert(T::MaxKeysLife::get(), members_ids); } } @@ -166,18 +158,6 @@ pub mod pallet { pub type Members<T: Config> = StorageMap<_, Twox64Concat, T::MemberId, MemberData<T::AccountId>, OptionQuery>; - /// maps session index to the list of member id set to expire at this session - #[pallet::storage] - #[pallet::getter(fn members_expire_on)] - pub type MembersExpireOn<T: Config> = - StorageMap<_, Twox64Concat, SessionIndex, Vec<T::MemberId>, ValueQuery>; - - /// maps session index to the list of member id forced to rotate keys before this session - #[pallet::storage] - #[pallet::getter(fn must_rotate_keys_before)] - pub type MustRotateKeysBefore<T: Config> = - StorageMap<_, Twox64Concat, SessionIndex, Vec<T::MemberId>, ValueQuery>; - // Blacklist. #[pallet::storage] #[pallet::getter(fn blacklist)] @@ -327,21 +307,7 @@ pub mod pallet { } .dispatch_bypass_filter(origin)?; - 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, - owner_key: who, - }); - member_data.must_rotate_keys_before = must_rotate_keys_before; - }); - MustRotateKeysBefore::<T>::append(must_rotate_keys_before, member_id); + Members::<T>::insert(member_id, MemberData { owner_key: who }); Ok(().into()) } @@ -453,23 +419,6 @@ pub mod pallet { Weight::zero() } - /// perform planned expiration of authority member for the given session - pub(super) fn expire_memberships(current_session_index: SessionIndex) { - for member_id in MembersExpireOn::<T>::take(current_session_index) { - if let Some(member_data) = Members::<T>::get(member_id) { - if member_data.expire_on_session == current_session_index { - Self::do_remove_member(member_id, member_data.owner_key); - } - } - } - 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, member_data.owner_key); - } - } - } - } /// perform incoming authorities insertion fn insert_in(member_id: T::MemberId) -> bool { let not_already_inserted = IncomingAuthorities::<T>::mutate(|members_ids| { @@ -584,7 +533,7 @@ impl<T: Config> pallet_session::SessionManager<T::ValidatorId> for Pallet<T> { /// /// `new_session(session)` is guaranteed to be called before `end_session(session-1)`. In other /// words, a new session must always be planned before an ongoing one can be finished. - fn new_session(session_index: SessionIndex) -> Option<Vec<T::ValidatorId>> { + fn new_session(_session_index: SessionIndex) -> Option<Vec<T::ValidatorId>> { let members_ids_to_add = IncomingAuthorities::<T>::take(); let members_ids_to_del = OutgoingAuthorities::<T>::take(); @@ -593,17 +542,6 @@ impl<T: Config> pallet_session::SessionManager<T::ValidatorId> for Pallet<T> { // when no change to the set of autorities, return None return None; } else { - for member_id in &members_ids_to_del { - // Apply MaxOfflineSessions rule - let expire_on_session = - session_index.saturating_add(T::MaxOfflineSessions::get()); - Members::<T>::mutate_exists(member_id, |member_data_opt| { - if let Some(ref mut member_data) = member_data_opt { - member_data.expire_on_session = expire_on_session; - } - }); - MembersExpireOn::<T>::append(expire_on_session, member_id); - } Self::deposit_event(Event::OutgoingAuthorities(members_ids_to_del.clone())); } } else { @@ -660,7 +598,6 @@ impl<T: Config> pallet_session::SessionManager<T::ValidatorId> for Pallet<T> { /// /// The session start to be used for validation. fn start_session(start_index: SessionIndex) { - Self::expire_memberships(start_index); T::OnNewSession::on_new_session(start_index); } } diff --git a/pallets/authority-members/src/mock.rs b/pallets/authority-members/src/mock.rs index 5cc6fd83f..c5cfce266 100644 --- a/pallets/authority-members/src/mock.rs +++ b/pallets/authority-members/src/mock.rs @@ -154,8 +154,6 @@ impl pallet_authority_members::Config for Test { type KeysWrapper = MockSessionKeys; type IsMember = TestIsSmithMember; type MaxAuthorities = ConstU32<4>; - type MaxKeysLife = ConstU32<5>; - type MaxOfflineSessions = ConstU32<2>; type MemberId = u64; type MemberIdOf = ConvertInto; type OnNewSession = (); diff --git a/pallets/authority-members/src/tests.rs b/pallets/authority-members/src/tests.rs index 6c99f0528..e85e48a45 100644 --- a/pallets/authority-members/src/tests.rs +++ b/pallets/authority-members/src/tests.rs @@ -35,27 +35,15 @@ fn test_genesis_build() { assert_eq!(AuthorityMembers::outgoing(), EMPTY); assert_eq!( AuthorityMembers::member(3), - Some(MemberData { - expire_on_session: 0, - must_rotate_keys_before: 5, - owner_key: 3, - }) + Some(MemberData { owner_key: 3 }) ); assert_eq!( AuthorityMembers::member(6), - Some(MemberData { - expire_on_session: 0, - must_rotate_keys_before: 5, - owner_key: 6, - }) + Some(MemberData { owner_key: 6 }) ); assert_eq!( AuthorityMembers::member(9), - Some(MemberData { - expire_on_session: 0, - must_rotate_keys_before: 5, - owner_key: 9, - }) + Some(MemberData { owner_key: 9 }) ); // Verify Session state @@ -74,61 +62,6 @@ 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( - RuntimeOrigin::signed(3), - UintAuthorityId(3).into() - ),); - assert_ok!(AuthorityMembers::set_session_keys( - RuntimeOrigin::signed(6), - UintAuthorityId(6).into() - ),); - - // Verify state - assert_eq!( - AuthorityMembers::member(3), - Some(MemberData { - expire_on_session: 0, - must_rotate_keys_before: 7, - owner_key: 3, - }) - ); - assert_eq!( - AuthorityMembers::member(6), - Some(MemberData { - expire_on_session: 0, - must_rotate_keys_before: 7, - owner_key: 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]); - }); -} - /// tests consequences of go_offline call #[test] fn test_go_offline() { @@ -146,43 +79,25 @@ fn test_go_offline() { assert_eq!(AuthorityMembers::blacklist(), EMPTY); assert_eq!( AuthorityMembers::member(9), - Some(MemberData { - expire_on_session: 0, - must_rotate_keys_before: 5, - owner_key: 9, - }) + Some(MemberData { owner_key: 9 }) ); - // Member 9 should be "deprogrammed" at the next session (session 1) - // it should be out at session 2 and - // the expiry should be 2 sessions after that (session 4) + // Member 9 should be outgoing at the next session (session 1). + // They should be out at session 2. run_to_block(5); assert_eq!( AuthorityMembers::member(9), - Some(MemberData { - expire_on_session: 4, - must_rotate_keys_before: 5, - owner_key: 9, - }) + Some(MemberData { owner_key: 9 }) ); - assert_eq!(AuthorityMembers::members_expire_on(4), vec![9],); assert_eq!(Session::current_index(), 1); assert_eq!(Session::validators(), vec![3, 6, 9]); assert_eq!(Session::queued_keys().len(), 2); assert_eq!(Session::queued_keys()[0].0, 3); assert_eq!(Session::queued_keys()[1].0, 6); - // Member 9 should be **effectively** out at session 2 run_to_block(10); assert_eq!(Session::current_index(), 2); assert_eq!(Session::validators(), vec![3, 6]); - - // Member 9 should be removed at session 4 - run_to_block(20); - assert_eq!(Session::current_index(), 4); - assert_eq!(Session::validators(), vec![3, 6]); - assert_eq!(AuthorityMembers::members_expire_on(4), EMPTY); - assert_eq!(AuthorityMembers::member(9), None); }); } @@ -199,11 +114,7 @@ fn test_go_online() { )); assert_eq!( AuthorityMembers::member(12), - Some(MemberData { - expire_on_session: 2, - must_rotate_keys_before: 5, - owner_key: 12, - }) + Some(MemberData { owner_key: 12 }) ); // Member 12 should be able to go online @@ -215,11 +126,7 @@ fn test_go_online() { assert_eq!(AuthorityMembers::outgoing(), EMPTY); assert_eq!( AuthorityMembers::member(12), - Some(MemberData { - expire_on_session: 2, - must_rotate_keys_before: 5, - owner_key: 12, - }) + Some(MemberData { owner_key: 12 }) ); // Member 12 should be "programmed" at the next session @@ -295,11 +202,7 @@ fn test_go_online_then_go_offline_in_same_session() { assert_eq!(AuthorityMembers::outgoing(), EMPTY); assert_eq!( AuthorityMembers::member(12), - Some(MemberData { - expire_on_session: 2, - must_rotate_keys_before: 5, - owner_key: 12, - }) + Some(MemberData { owner_key: 12 }) ); }); } @@ -323,11 +226,7 @@ fn test_go_offline_then_go_online_in_same_session() { assert_eq!(AuthorityMembers::outgoing(), EMPTY); assert_eq!( AuthorityMembers::member(9), - Some(MemberData { - expire_on_session: 0, - must_rotate_keys_before: 5, - owner_key: 9, - }) + Some(MemberData { owner_key: 9 }) ); }); } @@ -363,43 +262,26 @@ fn test_offence_disconnect() { assert_eq!(AuthorityMembers::outgoing(), vec![3, 9]); assert_eq!(AuthorityMembers::blacklist(), EMPTY); - // Member 9 and 3 should be "deprogrammed" at the next session + // Member 9 and 3 should be outgoing at the next session run_to_block(5); assert_eq!( AuthorityMembers::member(9), - Some(MemberData { - expire_on_session: 4, - must_rotate_keys_before: 5, - owner_key: 9, - }) + Some(MemberData { owner_key: 9 }) ); assert_eq!( AuthorityMembers::member(3), - Some(MemberData { - expire_on_session: 4, - must_rotate_keys_before: 5, - owner_key: 3, - }) + Some(MemberData { owner_key: 3 }) ); - assert_eq!(AuthorityMembers::members_expire_on(4), vec![3, 9],); assert_eq!(Session::current_index(), 1); assert_eq!(Session::validators(), vec![3, 6, 9]); assert_eq!(Session::queued_keys().len(), 1); assert_eq!(Session::queued_keys()[0].0, 6); - // Member 9 and 3 should be **effectively** out at session 2 + // Member 9 and 3 should be out at session 2 run_to_block(10); assert_eq!(Session::current_index(), 2); assert_eq!(Session::validators(), vec![6]); - // Member 9 and 3 should be removed at session 4 - run_to_block(20); - assert_eq!(Session::current_index(), 4); - assert_eq!(Session::validators(), vec![6]); - assert_eq!(AuthorityMembers::members_expire_on(4), EMPTY); - assert_eq!(AuthorityMembers::member(3), None); - assert_eq!(AuthorityMembers::member(9), None); - // Member 9 and 3 should be allowed to set session keys and go online run_to_block(25); assert_ok!(AuthorityMembers::set_session_keys( @@ -423,9 +305,8 @@ fn test_offence_disconnect() { pallet_offences::SlashStrategy::Disconnect, ); - // Verify state, 6 is out of life now, only 3 should be outgoing now assert_eq!(AuthorityMembers::incoming(), EMPTY); - assert_eq!(AuthorityMembers::online(), vec![3, 9]); + assert_eq!(AuthorityMembers::online(), vec![3, 6, 9]); assert_eq!(AuthorityMembers::outgoing(), vec![3]); assert_eq!(AuthorityMembers::blacklist(), EMPTY); }); @@ -453,26 +334,17 @@ fn test_offence_black_list() { assert_eq!(AuthorityMembers::outgoing(), vec![9]); assert_eq!(AuthorityMembers::blacklist(), vec![9]); - // Member 9 should be "deprogrammed" at the next session + // Member 9 should be outgoing at the next session run_to_block(5); - assert_eq!(AuthorityMembers::members_expire_on(4), vec![9],); assert_eq!(Session::current_index(), 1); assert_eq!(Session::validators(), vec![3, 6, 9]); assert_eq!(AuthorityMembers::blacklist(), vec![9]); // still in blacklist - // Member 9 should be **effectively** out at session 2 + // Member 9 should be out at session 2 run_to_block(10); assert_eq!(Session::current_index(), 2); assert_eq!(Session::validators(), vec![3, 6]); assert_eq!(AuthorityMembers::blacklist(), vec![9]); // still in blacklist - - // Member 9 should be removed at session 4 - run_to_block(20); - assert_eq!(Session::current_index(), 4); - assert_eq!(Session::validators(), vec![3, 6]); - assert_eq!(AuthorityMembers::members_expire_on(4), EMPTY); - assert_eq!(AuthorityMembers::member(9), None); - assert_eq!(AuthorityMembers::blacklist(), vec![9]); // still in blacklist }); } @@ -497,11 +369,7 @@ fn test_offence_black_list_prevent_from_going_online() { assert_eq!(AuthorityMembers::blacklist(), vec![9]); assert_eq!( AuthorityMembers::member(9), - Some(MemberData { - expire_on_session: 0, - must_rotate_keys_before: 5, - owner_key: 9, - }) + Some(MemberData { owner_key: 9 }) ); // for detail, see `test_go_offline` diff --git a/pallets/authority-members/src/types.rs b/pallets/authority-members/src/types.rs index f77996d6d..bec91587a 100644 --- a/pallets/authority-members/src/types.rs +++ b/pallets/authority-members/src/types.rs @@ -20,25 +20,16 @@ use codec::{Decode, Encode}; use scale_info::TypeInfo; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; -use sp_staking::SessionIndex; #[cfg_attr(feature = "std", derive(Debug, Deserialize, Serialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, TypeInfo)] pub struct MemberData<AccountId> { - /// session at which the membership expires - pub expire_on_session: SessionIndex, - /// session before which the member must have rotated keys - pub must_rotate_keys_before: SessionIndex, /// pubkey of the member pub owner_key: AccountId, } impl<AccountId> MemberData<AccountId> { - pub fn new_genesis(must_rotate_keys_before: SessionIndex, owner_key: AccountId) -> Self { - MemberData { - expire_on_session: 0, - must_rotate_keys_before, - owner_key, - } + pub fn new_genesis(owner_key: AccountId) -> Self { + MemberData { owner_key } } } diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index 55fcb8bca..c243beb1d 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -205,8 +205,6 @@ macro_rules! pallets_config { type MemberId = IdtyIndex; type MemberIdOf = common_runtime::providers::IdentityIndexOf<Self>; type MaxAuthorities = MaxAuthorities; - type MaxKeysLife = frame_support::pallet_prelude::ConstU32<1_500>; - type MaxOfflineSessions = frame_support::pallet_prelude::ConstU32<2_400>; type RemoveMemberOrigin = EnsureRoot<Self::AccountId>; type RuntimeEvent = RuntimeEvent; type WeightInfo = common_runtime::weights::pallet_authority_members::WeightInfo<Runtime>; -- GitLab