From 874d1c930735734bf90eff67dd7c45d917dffeba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Moreau?= <cem.moreau@gmail.com> Date: Tue, 9 Jan 2024 21:07:57 +0100 Subject: [PATCH] fix(#148) "use counted maps instead of counters in authority members" (nodes/rust/duniter-v2s!224) * fix(#148): update metadata.scale * fix(#148): remove AuthoritiesCounter * fix(#148): reveal bug with a test --- pallets/authority-members/src/lib.rs | 22 +++++++--------------- pallets/authority-members/src/tests.rs | 2 ++ resources/metadata.scale | Bin 132175 -> 132114 bytes 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/pallets/authority-members/src/lib.rs b/pallets/authority-members/src/lib.rs index 8aea8ef95..29c4ecaab 100644 --- a/pallets/authority-members/src/lib.rs +++ b/pallets/authority-members/src/lib.rs @@ -116,18 +116,12 @@ pub mod pallet { .collect::<Vec<T::MemberId>>(); members_ids.sort(); - AuthoritiesCounter::<T>::put(members_ids.len() as u32); OnlineAuthorities::<T>::put(members_ids.clone()); } } // STORAGE // - /// count the number of authorities - #[pallet::storage] - #[pallet::getter(fn authorities_counter)] - pub type AuthoritiesCounter<T: Config> = StorageValue<_, u32, ValueQuery>; - /// list incoming authorities #[pallet::storage] #[pallet::getter(fn incoming)] @@ -264,7 +258,7 @@ pub mod pallet { if Self::is_online(member_id) && !is_outgoing { return Err(Error::<T>::AlreadyOnline.into()); } - if AuthoritiesCounter::<T>::get() >= T::MaxAuthorities::get() { + if Self::authorities_counter() >= T::MaxAuthorities::get() { return Err(Error::<T>::TooManyAuthorities.into()); } @@ -371,6 +365,12 @@ pub mod pallet { Ok(().into()) } + + pub fn authorities_counter() -> u32 { + let count = OnlineAuthorities::<T>::get().len() + IncomingAuthorities::<T>::get().len() + - OutgoingAuthorities::<T>::get().len(); + count as u32 + } } // INTERNAL FUNCTIONS // @@ -414,7 +414,6 @@ pub mod pallet { } }); if not_already_inserted { - AuthoritiesCounter::<T>::mutate(|counter| *counter += 1); Self::deposit_event(Event::MemberGoOnline { member: member_id }); } not_already_inserted @@ -430,11 +429,6 @@ pub mod pallet { } }); if not_already_inserted { - AuthoritiesCounter::<T>::mutate(|counter| { - if *counter > 0 { - *counter -= 1 - } - }); Self::deposit_event(Event::MemberGoOffline { member: member_id }); } not_already_inserted @@ -463,7 +457,6 @@ pub mod pallet { } /// perform removal from incoming authorities fn remove_in(member_id: T::MemberId) { - AuthoritiesCounter::<T>::mutate(|counter| counter.saturating_sub(1)); IncomingAuthorities::<T>::mutate(|members_ids| { if let Ok(index) = members_ids.binary_search(&member_id) { members_ids.remove(index); @@ -472,7 +465,6 @@ pub mod pallet { } /// perform removal from online authorities fn remove_online(member_id: T::MemberId) { - AuthoritiesCounter::<T>::mutate(|counter| counter.saturating_add(1)); OnlineAuthorities::<T>::mutate(|members_ids| { if let Ok(index) = members_ids.binary_search(&member_id) { members_ids.remove(index); diff --git a/pallets/authority-members/src/tests.rs b/pallets/authority-members/src/tests.rs index b404b96f3..ab1218ccd 100644 --- a/pallets/authority-members/src/tests.rs +++ b/pallets/authority-members/src/tests.rs @@ -175,6 +175,8 @@ fn test_too_many_authorities() { assert_eq!(AuthorityMembers::authorities_counter(), 3); assert_ok!(AuthorityMembers::go_online(RuntimeOrigin::signed(15)),); assert_eq!(AuthorityMembers::authorities_counter(), 4); + assert_ok!(AuthorityMembers::remove_member(RawOrigin::Root.into(), 15)); + assert_eq!(AuthorityMembers::authorities_counter(), 3); }); } diff --git a/resources/metadata.scale b/resources/metadata.scale index fb91b61a49c3a512a3c28298a25c7e83b34b9e69..7f579695264b0823a205907a6cfb8014e4f750cd 100644 GIT binary patch delta 28 kcmX@#!7-_WqhSl9(HTaO$&TtG(_fuo<lR2;4C6j?0HGucJ^%m! delta 72 zcmbQ#!EwHWqhSl9(HRX1569AyjQpa^lFZa%=ls&VlGGwb1_1#EAYf@wNCt^0lw_n% ZH#o~EHrY{KWcu1OjJ(@J&M@vX2LOti7@`0G -- GitLab