Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

exchange member with a new account and same rank in the ranked collec… #2587

Merged
merged 41 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
07b93db
exchange member with a new account and same rank in the ranked collec…
dharjeezy Dec 3, 2023
66b2f08
Merge branch 'master' into dami/change-account
dharjeezy Dec 11, 2023
fbf91f3
address concerns and benchmarks
dharjeezy Dec 11, 2023
8d68ca5
Merge remote-tracking branch 'origin/dami/change-account' into dami/c…
dharjeezy Dec 11, 2023
cd50907
change comment
dharjeezy Dec 11, 2023
05f6ee6
fmt
dharjeezy Dec 11, 2023
255ad1f
weight
dharjeezy Dec 21, 2023
4df260b
Merge branch 'master' into dami/change-account
dharjeezy Dec 21, 2023
413ce7f
Merge branch 'master' into dami/change-account
dharjeezy Dec 21, 2023
89a8c29
Merge branch 'master' into dami/change-account
dharjeezy Jan 2, 2024
6dc14e9
Merge branch 'paritytech:master' into dami/change-account
dharjeezy Jan 17, 2024
279ed33
Merge branch 'master' of https://github.com/dharjeezy/polkadot-sdk in…
dharjeezy Jan 18, 2024
f7d68ef
boolean flag to toggle emission of events
dharjeezy Jan 18, 2024
065c619
Merge remote-tracking branch 'origin/dami/change-account' into dami/c…
dharjeezy Jan 18, 2024
ec954ad
doc
dharjeezy Jan 18, 2024
205c2d8
Merge branch 'master' into dami/change-account
dharjeezy Jan 18, 2024
3d0c8f7
subtle changes
dharjeezy Jan 21, 2024
b92210e
Merge remote-tracking branch 'origin/dami/change-account' into dami/c…
dharjeezy Jan 21, 2024
3115f39
Merge branch 'master' into dami/change-account
dharjeezy Jan 21, 2024
fd68eb1
Merge branch 'master' into dami/change-account
dharjeezy Jan 22, 2024
59bb68a
Merge branch 'master' into dami/change-account
dharjeezy Jan 24, 2024
a33fe94
Merge branch 'master' into dami/change-account
dharjeezy Jan 24, 2024
b7078b0
Merge branch 'master' of https://github.com/paritytech/polkadot-sdk i…
Jan 24, 2024
7d25b5c
".git/.scripts/commands/bench/bench.sh" --subcommand=runtime --runtim…
Jan 24, 2024
fdbbe76
nit
dharjeezy Jan 24, 2024
2758890
Merge remote-tracking branch 'origin/dami/change-account' into dami/c…
dharjeezy Jan 24, 2024
239831c
Merge branch 'master' into dami/change-account
dharjeezy Jan 25, 2024
99fc1b7
nit
dharjeezy Jan 25, 2024
76963a1
Merge remote-tracking branch 'origin/dami/change-account' into dami/c…
dharjeezy Jan 25, 2024
5c6b2cf
nit
dharjeezy Jan 25, 2024
0cb7494
nit
dharjeezy Jan 25, 2024
f58c20a
nit
dharjeezy Jan 25, 2024
0da55a7
Merge branch 'master' into dami/change-account
dharjeezy Jan 25, 2024
7ed5a3d
nit
dharjeezy Jan 26, 2024
598c2bc
Merge branch 'master' into dami/change-account
dharjeezy Jan 26, 2024
3f03aaf
Merge branch 'master' into dami/change-account
dharjeezy Jan 26, 2024
6e9e14d
fmt
ggwpez Jan 26, 2024
0920307
Add prdoc
ggwpez Jan 26, 2024
43773cb
Merge branch 'master' into dami/change-account
ggwpez Jan 26, 2024
684beb3
helper method
dharjeezy Jan 30, 2024
754184f
Merge branch 'master' into dami/change-account
dharjeezy Jan 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 43 additions & 0 deletions substrate/frame/ranked-collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,8 @@ pub mod pallet {
/// The member `who` has voted for the `poll` with the given `vote` leading to an updated
/// `tally`.
Voted { who: T::AccountId, poll: PollIndexOf<T, I>, vote: VoteRecord, tally: TallyOf<T, I> },
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
/// The member `who` of given `rank` has been exchanged from the collective.
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
MemberExchanged { who: T::AccountId, new_who: T::AccountId, rank: Rank },
}

#[pallet::error]
Expand Down Expand Up @@ -650,6 +652,47 @@ pub mod pallet {
pays_fee: Pays::No,
})
}

/// Exchanges a member with a new account and the same existing rank.
///
/// - `origin`: Must be the `AdminOrigin`.
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
/// - `who`: Account of existing member of rank greater than zero to be exchanged.
/// - `new_who`: New Account of existing member of rank greater than zero to exchanged to.
/// - `min_rank`: The rank of the member or greater.
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
///
/// Weight: `O(min_rank)`.
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
#[pallet::call_index(6)]
#[pallet::weight(0)]
pub fn exchange_member(
origin: OriginFor<T>,
who: AccountIdLookupOf<T>,
new_who: AccountIdLookupOf<T>,
min_rank: Rank,
) -> DispatchResult {
// remove who
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
let max_rank = T::DemoteOrigin::ensure_origin(origin)?;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
let who = T::Lookup::lookup(who)?;
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved
let MemberRecord { rank, .. } = Self::ensure_member(&who)?;
ensure!(min_rank >= rank, Error::<T, I>::InvalidWitness);
ensure!(max_rank >= rank, Error::<T, I>::NoPermission);
ggwpez marked this conversation as resolved.
Show resolved Hide resolved

for r in 0..=rank {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you make an equivalent of do_remove_member_to_rank that does these two operations, it would be a good counterpart to do_add_member_to_rank, it would make the code a bit nicer, and you can use it in other places as well.

Self::remove_from_rank(&who, r)?;
}
Members::<T, I>::remove(&who);

// add new member
let new_who = T::Lookup::lookup(new_who)?;
Self::do_add_member(new_who.clone())?;
dharjeezy marked this conversation as resolved.
Show resolved Hide resolved

// promotes new_who to the rank of who
for _i in 1..=rank {
let _ = Self::do_promote_member(new_who.clone(), Some(max_rank.clone()))?;
}

Self::deposit_event(Event::MemberExchanged { who, new_who: new_who.clone(), rank });
Ok(())
}
}

impl<T: Config<I>, I: 'static> Pallet<T, I> {
Expand Down
20 changes: 20 additions & 0 deletions substrate/frame/ranked-collective/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,3 +567,23 @@ fn tally_support_correct() {
MinRankOfClassDelta::set(0);
});
}

#[test]
fn exchange_member_works() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be good to see a test for EnsureRanked<Test, (), 2> in action as well.

new_test_ext().execute_with(|| {
assert_ok!(Club::add_member(RuntimeOrigin::root(), 1));
assert_eq!(member_count(0), 1);

assert_ok!(Club::promote_member(RuntimeOrigin::root(), 1));

let member_record = MemberRecord { rank: 1 };
assert_eq!(Members::<Test>::get(1), Some(member_record.clone()));
assert_eq!(Members::<Test>::get(2), None);

assert_ok!(Club::exchange_member(RuntimeOrigin::root(), 1, 2, 2));
assert_eq!(member_count(0), 1);

assert_eq!(Members::<Test>::get(1), None);
assert_eq!(Members::<Test>::get(2), Some(member_record));
joepetrowski marked this conversation as resolved.
Show resolved Hide resolved
});
}