Skip to content

Commit

Permalink
Alliance pallet: fix func name, fix migration weights (#12174)
Browse files Browse the repository at this point in the history
* Alliance pallet: fix func name, fix migration weights

* update comment order
  • Loading branch information
muharem authored Sep 2, 2022
1 parent e986f05 commit 91b27bc
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 19 deletions.
2 changes: 1 addition & 1 deletion frame/alliance/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ benchmarks_instance_pallet! {
).unwrap();
}

assert_eq!(Alliance::<T, I>::votable_members_count(), cc);
assert_eq!(Alliance::<T, I>::voting_members_count(), cc);
assert_eq!(Alliance::<T, I>::ally_members_count(), m);
}

Expand Down
16 changes: 8 additions & 8 deletions frame/alliance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ pub mod pallet {
Error::<T, I>::BadWitness
);
ensure!(
Self::votable_members_count() <= witness.voting_members,
Self::voting_members_count() <= witness.voting_members,
Error::<T, I>::BadWitness
);
ensure!(
Expand All @@ -697,7 +697,7 @@ pub mod pallet {
T::ProposalProvider::veto_proposal(*hash);
}

let mut members = Self::votable_members();
let mut members = Self::voting_members();
T::MembershipChanged::change_members_sorted(&[], &members, &[]);

members.append(&mut Self::members_of(MemberRole::Ally));
Expand Down Expand Up @@ -1052,7 +1052,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

/// Count of all members who have voting rights.
fn votable_members_count() -> u32 {
fn voting_members_count() -> u32 {
Members::<T, I>::decode_len(MemberRole::Founder)
.unwrap_or(0)
.saturating_add(Members::<T, I>::decode_len(MemberRole::Fellow).unwrap_or(0)) as u32
Expand All @@ -1064,16 +1064,16 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

/// Collect all members who have voting rights into one list.
fn votable_members() -> Vec<T::AccountId> {
fn voting_members() -> Vec<T::AccountId> {
let mut founders = Self::members_of(MemberRole::Founder);
let mut fellows = Self::members_of(MemberRole::Fellow);
founders.append(&mut fellows);
founders
}

/// Collect all members who have voting rights into one sorted list.
fn votable_members_sorted() -> Vec<T::AccountId> {
let mut members = Self::votable_members();
fn voting_members_sorted() -> Vec<T::AccountId> {
let mut members = Self::voting_members();
members.sort();
members
}
Expand All @@ -1089,7 +1089,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
})?;

if role == MemberRole::Founder || role == MemberRole::Fellow {
let members = Self::votable_members_sorted();
let members = Self::voting_members_sorted();
T::MembershipChanged::change_members_sorted(&[who.clone()], &[], &members[..]);
}
Ok(())
Expand All @@ -1104,7 +1104,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
})?;

if matches!(role, MemberRole::Founder | MemberRole::Fellow) {
let members = Self::votable_members_sorted();
let members = Self::voting_members_sorted();
T::MembershipChanged::change_members_sorted(&[], &[who.clone()], &members[..]);
}
Ok(())
Expand Down
19 changes: 13 additions & 6 deletions frame/alliance/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,29 @@ mod v0_to_v1 {
use super::*;

pub fn migrate<T: Config<I>, I: 'static>() -> Weight {
if migration::clear_storage_prefix(
log::info!(target: LOG_TARGET, "Running migration v0_to_v1.");

let res = migration::clear_storage_prefix(
<Pallet<T, I>>::name().as_bytes(),
b"UpForKicking",
b"",
None,
None,
)
.maybe_cursor
.is_some()
{
);

log::info!(
target: LOG_TARGET,
"Cleared '{}' entries from 'UpForKicking' storage prefix",
res.unique
);

if res.maybe_cursor.is_some() {
log::error!(
target: LOG_TARGET,
"Storage prefix 'UpForKicking' is not completely cleared."
);
}

T::DbWeight::get().writes(1)
T::DbWeight::get().writes(res.unique.into())
}
}
8 changes: 4 additions & 4 deletions frame/alliance/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type AllianceMotionEvent = pallet_collective::Event<Test, pallet_collective::Ins
fn force_set_members_works() {
new_test_ext().execute_with(|| {
// ensure alliance is set
assert_eq!(Alliance::votable_members_sorted(), vec![1, 2, 3]);
assert_eq!(Alliance::voting_members_sorted(), vec![1, 2, 3]);

// creating and proposing proposals
let (proposal, proposal_len, hash) = make_remark_proposal(42);
Expand Down Expand Up @@ -141,7 +141,7 @@ fn force_set_members_works() {
));

// assert new set of voting members
assert_eq!(Alliance::votable_members_sorted(), vec![4, 5, 8]);
assert_eq!(Alliance::voting_members_sorted(), vec![4, 5, 8]);
// assert new ally member
assert!(Alliance::is_ally(&2));
// assert a retiring member from previous Alliance not removed
Expand Down Expand Up @@ -176,7 +176,7 @@ fn propose_works() {
new_test_ext().execute_with(|| {
let (proposal, proposal_len, hash) = make_remark_proposal(42);

// only votable member can propose proposal, 4 is ally not have vote rights
// only voting member can propose proposal, 4 is ally not have vote rights
assert_noop!(
Alliance::propose(Origin::signed(4), 3, Box::new(proposal.clone()), proposal_len),
Error::<Test, ()>::NoVotingRights
Expand Down Expand Up @@ -465,7 +465,7 @@ fn nominate_ally_works() {
Error::<Test, ()>::AlreadyMember
);

// only votable member(founder/fellow) have nominate right
// only voting member(founder/fellow) have nominate right
assert_noop!(
Alliance::nominate_ally(Origin::signed(5), 4),
Error::<Test, ()>::NoVotingRights
Expand Down

0 comments on commit 91b27bc

Please sign in to comment.