Skip to content

Commit

Permalink
Include submitting origin in PollStatus
Browse files Browse the repository at this point in the history
  • Loading branch information
olanod committed Feb 23, 2025
1 parent 21f6f07 commit f50beaa
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 87 deletions.
17 changes: 17 additions & 0 deletions prdoc/pr_4961.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
title: Include executing origin in PollStatus

doc:
- audience: Runtime Dev
description: |
Adds an extra field to `PollStatus::Ongoing` variant `Origin`. It allows
users of the `Polling` trait know what origin is executing the poll.

crates:
- name: frame-support
bump: minor
- name: pallet-referenda
bump: minor
- name: pallet-conviction-voting
bump: patch
- name: pallet-ranked-collective
bump: patch
9 changes: 5 additions & 4 deletions substrate/frame/conviction-voting/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Error::<T, I>::InsufficientFunds
);
T::Polls::try_access_poll(poll_index, |poll_status| {
let (tally, class) = poll_status.ensure_ongoing().ok_or(Error::<T, I>::NotOngoing)?;
let (tally, class, _) =
poll_status.ensure_ongoing().ok_or(Error::<T, I>::NotOngoing)?;
VotingFor::<T, I>::try_mutate(who, &class, |voting| {
if let Voting::Casting(Casting { ref mut votes, delegations, .. }) = voting {
match votes.binary_search_by_key(&poll_index, |i| i.0) {
Expand Down Expand Up @@ -469,7 +470,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
let v = votes.remove(i);

T::Polls::try_access_poll(poll_index, |poll_status| match poll_status {
PollStatus::Ongoing(tally, _) => {
PollStatus::Ongoing(tally, _, _) => {
ensure!(matches!(scope, UnvoteScope::Any), Error::<T, I>::NoPermission);
// Shouldn't be possible to fail, but we handle it gracefully.
tally.remove(v.1).ok_or(ArithmeticError::Underflow)?;
Expand Down Expand Up @@ -520,7 +521,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
for &(poll_index, account_vote) in votes.iter() {
if let AccountVote::Standard { vote, .. } = account_vote {
T::Polls::access_poll(poll_index, |poll_status| {
if let PollStatus::Ongoing(tally, _) = poll_status {
if let PollStatus::Ongoing(tally, _, _) = poll_status {
tally.increase(vote.aye, amount);
}
});
Expand Down Expand Up @@ -548,7 +549,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
for &(poll_index, account_vote) in votes.iter() {
if let AccountVote::Standard { vote, .. } = account_vote {
T::Polls::access_poll(poll_index, |poll_status| {
if let PollStatus::Ongoing(tally, _) = poll_status {
if let PollStatus::Ongoing(tally, _, _) = poll_status {
tally.reduce(vote.aye, amount);
}
});
Expand Down
104 changes: 54 additions & 50 deletions substrate/frame/conviction-voting/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,12 @@ impl pallet_balances::Config for Test {
type AccountStore = System;
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Origin;

#[derive(Clone, PartialEq, Eq, Debug)]
pub enum TestPollState {
Ongoing(TallyOf<Test>, u8),
Ongoing(TallyOf<Test>, u8, Origin),
Completed(u64, bool),
}
use TestPollState::*;
Expand All @@ -70,7 +73,7 @@ parameter_types! {
pub static Polls: BTreeMap<u8, TestPollState> = vec![
(1, Completed(1, true)),
(2, Completed(2, false)),
(3, Ongoing(Tally::from_parts(0, 0, 0), 0)),
(3, Ongoing(Tally::from_parts(0, 0, 0), 0, Origin)),
].into_iter().collect();
}

Expand All @@ -80,27 +83,28 @@ impl Polling<TallyOf<Test>> for TestPolls {
type Votes = u64;
type Moment = u64;
type Class = u8;
type Origin = Origin;
fn classes() -> Vec<u8> {
vec![0, 1, 2]
}
fn as_ongoing(index: u8) -> Option<(TallyOf<Test>, Self::Class)> {
fn as_ongoing(index: u8) -> Option<(TallyOf<Test>, Self::Class, Self::Origin)> {
Polls::get().remove(&index).and_then(|x| {
if let TestPollState::Ongoing(t, c) = x {
Some((t, c))
if let TestPollState::Ongoing(t, c, o) = x {
Some((t, c, o))
} else {
None
}
})
}
fn access_poll<R>(
index: Self::Index,
f: impl FnOnce(PollStatus<&mut TallyOf<Test>, u64, u8>) -> R,
f: impl FnOnce(PollStatus<&mut TallyOf<Test>, u64, u8, &Origin>) -> R,
) -> R {
let mut polls = Polls::get();
let entry = polls.get_mut(&index);
let r = match entry {
Some(Ongoing(ref mut tally_mut_ref, class)) =>
f(PollStatus::Ongoing(tally_mut_ref, *class)),
Some(Ongoing(ref mut tally_mut_ref, class, origin)) =>
f(PollStatus::Ongoing(tally_mut_ref, *class, &origin)),
Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)),
None => f(PollStatus::None),
};
Expand All @@ -109,13 +113,13 @@ impl Polling<TallyOf<Test>> for TestPolls {
}
fn try_access_poll<R>(
index: Self::Index,
f: impl FnOnce(PollStatus<&mut TallyOf<Test>, u64, u8>) -> Result<R, DispatchError>,
f: impl FnOnce(PollStatus<&mut TallyOf<Test>, u64, u8, &Origin>) -> Result<R, DispatchError>,
) -> Result<R, DispatchError> {
let mut polls = Polls::get();
let entry = polls.get_mut(&index);
let r = match entry {
Some(Ongoing(ref mut tally_mut_ref, class)) =>
f(PollStatus::Ongoing(tally_mut_ref, *class)),
Some(Ongoing(ref mut tally_mut_ref, class, origin)) =>
f(PollStatus::Ongoing(tally_mut_ref, *class, &origin)),
Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)),
None => f(PollStatus::None),
}?;
Expand All @@ -127,7 +131,7 @@ impl Polling<TallyOf<Test>> for TestPolls {
fn create_ongoing(class: Self::Class) -> Result<Self::Index, ()> {
let mut polls = Polls::get();
let i = polls.keys().rev().next().map_or(0, |x| x + 1);
polls.insert(i, Ongoing(Tally::new(0), class));
polls.insert(i, Ongoing(Tally::new(0), class, Origin));
Polls::set(polls);
Ok(i)
}
Expand Down Expand Up @@ -455,10 +459,10 @@ fn classwise_delegation_works() {
new_test_ext().execute_with(|| {
Polls::set(
vec![
(0, Ongoing(Tally::new(0), 0)),
(1, Ongoing(Tally::new(0), 1)),
(2, Ongoing(Tally::new(0), 2)),
(3, Ongoing(Tally::new(0), 2)),
(0, Ongoing(Tally::new(0), 0, Origin)),
(1, Ongoing(Tally::new(0), 1, Origin)),
(2, Ongoing(Tally::new(0), 2, Origin)),
(3, Ongoing(Tally::new(0), 2, Origin)),
]
.into_iter()
.collect(),
Expand All @@ -482,10 +486,10 @@ fn classwise_delegation_works() {
assert_eq!(
Polls::get(),
vec![
(0, Ongoing(Tally::from_parts(6, 2, 15), 0)),
(1, Ongoing(Tally::from_parts(6, 2, 15), 1)),
(2, Ongoing(Tally::from_parts(6, 2, 15), 2)),
(3, Ongoing(Tally::from_parts(0, 0, 0), 2)),
(0, Ongoing(Tally::from_parts(6, 2, 15), 0, Origin)),
(1, Ongoing(Tally::from_parts(6, 2, 15), 1, Origin)),
(2, Ongoing(Tally::from_parts(6, 2, 15), 2, Origin)),
(3, Ongoing(Tally::from_parts(0, 0, 0), 2, Origin)),
]
.into_iter()
.collect()
Expand All @@ -496,10 +500,10 @@ fn classwise_delegation_works() {
assert_eq!(
Polls::get(),
vec![
(0, Ongoing(Tally::from_parts(6, 2, 15), 0)),
(1, Ongoing(Tally::from_parts(6, 2, 15), 1)),
(2, Ongoing(Tally::from_parts(6, 2, 15), 2)),
(3, Ongoing(Tally::from_parts(0, 6, 0), 2)),
(0, Ongoing(Tally::from_parts(6, 2, 15), 0, Origin)),
(1, Ongoing(Tally::from_parts(6, 2, 15), 1, Origin)),
(2, Ongoing(Tally::from_parts(6, 2, 15), 2, Origin)),
(3, Ongoing(Tally::from_parts(0, 6, 0), 2, Origin)),
]
.into_iter()
.collect()
Expand All @@ -511,10 +515,10 @@ fn classwise_delegation_works() {
assert_eq!(
Polls::get(),
vec![
(0, Ongoing(Tally::from_parts(6, 2, 15), 0)),
(1, Ongoing(Tally::from_parts(6, 2, 15), 1)),
(2, Ongoing(Tally::from_parts(1, 7, 10), 2)),
(3, Ongoing(Tally::from_parts(0, 1, 0), 2)),
(0, Ongoing(Tally::from_parts(6, 2, 15), 0, Origin)),
(1, Ongoing(Tally::from_parts(6, 2, 15), 1, Origin)),
(2, Ongoing(Tally::from_parts(1, 7, 10), 2, Origin)),
(3, Ongoing(Tally::from_parts(0, 1, 0), 2, Origin)),
]
.into_iter()
.collect()
Expand All @@ -530,10 +534,10 @@ fn classwise_delegation_works() {
assert_eq!(
Polls::get(),
vec![
(0, Ongoing(Tally::from_parts(4, 2, 13), 0)),
(1, Ongoing(Tally::from_parts(4, 2, 13), 1)),
(2, Ongoing(Tally::from_parts(4, 2, 13), 2)),
(3, Ongoing(Tally::from_parts(0, 4, 0), 2)),
(0, Ongoing(Tally::from_parts(4, 2, 13), 0, Origin)),
(1, Ongoing(Tally::from_parts(4, 2, 13), 1, Origin)),
(2, Ongoing(Tally::from_parts(4, 2, 13), 2, Origin)),
(3, Ongoing(Tally::from_parts(0, 4, 0), 2, Origin)),
]
.into_iter()
.collect()
Expand Down Expand Up @@ -562,10 +566,10 @@ fn classwise_delegation_works() {
assert_eq!(
Polls::get(),
vec![
(0, Ongoing(Tally::from_parts(7, 2, 16), 0)),
(1, Ongoing(Tally::from_parts(8, 2, 17), 1)),
(2, Ongoing(Tally::from_parts(9, 2, 18), 2)),
(3, Ongoing(Tally::from_parts(0, 9, 0), 2)),
(0, Ongoing(Tally::from_parts(7, 2, 16), 0, Origin)),
(1, Ongoing(Tally::from_parts(8, 2, 17), 1, Origin)),
(2, Ongoing(Tally::from_parts(9, 2, 18), 2, Origin)),
(3, Ongoing(Tally::from_parts(0, 9, 0), 2, Origin)),
]
.into_iter()
.collect()
Expand All @@ -576,7 +580,7 @@ fn classwise_delegation_works() {
#[test]
fn redelegation_after_vote_ending_should_keep_lock() {
new_test_ext().execute_with(|| {
Polls::set(vec![(0, Ongoing(Tally::new(0), 0))].into_iter().collect());
Polls::set(vec![(0, Ongoing(Tally::new(0), 0, Origin))].into_iter().collect());
assert_ok!(Voting::delegate(RuntimeOrigin::signed(1), 0, 2, Conviction::Locked1x, 5));
assert_ok!(Voting::vote(RuntimeOrigin::signed(2), 0, aye(10, 1)));
Polls::set(vec![(0, Completed(1, true))].into_iter().collect());
Expand All @@ -594,9 +598,9 @@ fn lock_amalgamation_valid_with_multiple_removed_votes() {
new_test_ext().execute_with(|| {
Polls::set(
vec![
(0, Ongoing(Tally::new(0), 0)),
(1, Ongoing(Tally::new(0), 0)),
(2, Ongoing(Tally::new(0), 0)),
(0, Ongoing(Tally::new(0), 0, Origin)),
(1, Ongoing(Tally::new(0), 0, Origin)),
(2, Ongoing(Tally::new(0), 0, Origin)),
]
.into_iter()
.collect(),
Expand Down Expand Up @@ -666,7 +670,7 @@ fn lock_amalgamation_valid_with_multiple_delegations() {
#[test]
fn lock_amalgamation_valid_with_move_roundtrip_to_delegation() {
new_test_ext().execute_with(|| {
Polls::set(vec![(0, Ongoing(Tally::new(0), 0))].into_iter().collect());
Polls::set(vec![(0, Ongoing(Tally::new(0), 0, Origin))].into_iter().collect());
assert_ok!(Voting::vote(RuntimeOrigin::signed(1), 0, aye(5, 1)));
Polls::set(vec![(0, Completed(1, true))].into_iter().collect());
assert_ok!(Voting::remove_vote(RuntimeOrigin::signed(1), Some(0), 0));
Expand All @@ -678,7 +682,7 @@ fn lock_amalgamation_valid_with_move_roundtrip_to_delegation() {
assert_ok!(Voting::unlock(RuntimeOrigin::signed(1), 0, 1));
assert_eq!(Balances::usable_balance(1), 0);

Polls::set(vec![(1, Ongoing(Tally::new(0), 0))].into_iter().collect());
Polls::set(vec![(1, Ongoing(Tally::new(0), 0, Origin))].into_iter().collect());
assert_ok!(Voting::vote(RuntimeOrigin::signed(1), 1, aye(5, 2)));
Polls::set(vec![(1, Completed(1, true))].into_iter().collect());
assert_ok!(Voting::remove_vote(RuntimeOrigin::signed(1), Some(0), 1));
Expand Down Expand Up @@ -706,7 +710,7 @@ fn lock_amalgamation_valid_with_move_roundtrip_to_casting() {
assert_ok!(Voting::unlock(RuntimeOrigin::signed(1), 0, 1));
assert_eq!(Balances::usable_balance(1), 5);

Polls::set(vec![(0, Ongoing(Tally::new(0), 0))].into_iter().collect());
Polls::set(vec![(0, Ongoing(Tally::new(0), 0, Origin))].into_iter().collect());
assert_ok!(Voting::vote(RuntimeOrigin::signed(1), 0, aye(10, 1)));
Polls::set(vec![(0, Completed(1, true))].into_iter().collect());
assert_ok!(Voting::remove_vote(RuntimeOrigin::signed(1), Some(0), 0));
Expand Down Expand Up @@ -767,9 +771,9 @@ fn lock_aggregation_over_different_classes_with_casting_works() {
new_test_ext().execute_with(|| {
Polls::set(
vec![
(0, Ongoing(Tally::new(0), 0)),
(1, Ongoing(Tally::new(0), 1)),
(2, Ongoing(Tally::new(0), 2)),
(0, Ongoing(Tally::new(0), 0, Origin)),
(1, Ongoing(Tally::new(0), 1, Origin)),
(2, Ongoing(Tally::new(0), 2, Origin)),
]
.into_iter()
.collect(),
Expand Down Expand Up @@ -835,10 +839,10 @@ fn errors_with_vote_work() {
assert_ok!(Voting::undelegate(RuntimeOrigin::signed(1), 0));
Polls::set(
vec![
(0, Ongoing(Tally::new(0), 0)),
(1, Ongoing(Tally::new(0), 0)),
(2, Ongoing(Tally::new(0), 0)),
(3, Ongoing(Tally::new(0), 0)),
(0, Ongoing(Tally::new(0), 0, Origin)),
(1, Ongoing(Tally::new(0), 0, Origin)),
(2, Ongoing(Tally::new(0), 0, Origin)),
(3, Ongoing(Tally::new(0), 0, Origin)),
]
.into_iter()
.collect(),
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/ranked-collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ pub mod pallet {
match status {
PollStatus::None | PollStatus::Completed(..) =>
Err(Error::<T, I>::NotPolling)?,
PollStatus::Ongoing(ref mut tally, class) => {
PollStatus::Ongoing(ref mut tally, class, _) => {
match Voting::<T, I>::get(&poll, &who) {
Some(Aye(votes)) => {
tally.bare_ayes.saturating_dec();
Expand Down
28 changes: 16 additions & 12 deletions substrate/frame/ranked-collective/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,12 @@ impl frame_system::Config for Test {
type Block = Block;
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Origin;

#[derive(Clone, PartialEq, Eq, Debug)]
pub enum TestPollState {
Ongoing(TallyOf<Test>, Rank),
Ongoing(TallyOf<Test>, Rank, Origin),
Completed(u64, bool),
}
use TestPollState::*;
Expand All @@ -59,7 +62,7 @@ parameter_types! {
pub static Polls: BTreeMap<u8, TestPollState> = vec![
(1, Completed(1, true)),
(2, Completed(2, false)),
(3, Ongoing(Tally::from_parts(0, 0, 0), 1)),
(3, Ongoing(Tally::from_parts(0, 0, 0), 1, Origin)),
].into_iter().collect();
}

Expand All @@ -69,27 +72,28 @@ impl Polling<TallyOf<Test>> for TestPolls {
type Votes = Votes;
type Moment = u64;
type Class = Class;
type Origin = Origin;
fn classes() -> Vec<Self::Class> {
vec![0, 1, 2]
}
fn as_ongoing(index: u8) -> Option<(TallyOf<Test>, Self::Class)> {
fn as_ongoing(index: u8) -> Option<(TallyOf<Test>, Self::Class, Self::Origin)> {
Polls::get().remove(&index).and_then(|x| {
if let TestPollState::Ongoing(t, c) = x {
Some((t, c))
if let TestPollState::Ongoing(t, c, o) = x {
Some((t, c, o))
} else {
None
}
})
}
fn access_poll<R>(
index: Self::Index,
f: impl FnOnce(PollStatus<&mut TallyOf<Test>, Self::Moment, Self::Class>) -> R,
f: impl FnOnce(PollStatus<&mut TallyOf<Test>, Self::Moment, Self::Class, &Self::Origin>) -> R,
) -> R {
let mut polls = Polls::get();
let entry = polls.get_mut(&index);
let r = match entry {
Some(Ongoing(ref mut tally_mut_ref, class)) =>
f(PollStatus::Ongoing(tally_mut_ref, *class)),
Some(Ongoing(ref mut tally_mut_ref, class, origin)) =>
f(PollStatus::Ongoing(tally_mut_ref, *class, &origin)),
Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)),
None => f(PollStatus::None),
};
Expand All @@ -99,14 +103,14 @@ impl Polling<TallyOf<Test>> for TestPolls {
fn try_access_poll<R>(
index: Self::Index,
f: impl FnOnce(
PollStatus<&mut TallyOf<Test>, Self::Moment, Self::Class>,
PollStatus<&mut TallyOf<Test>, Self::Moment, Self::Class, &Self::Origin>,
) -> Result<R, DispatchError>,
) -> Result<R, DispatchError> {
let mut polls = Polls::get();
let entry = polls.get_mut(&index);
let r = match entry {
Some(Ongoing(ref mut tally_mut_ref, class)) =>
f(PollStatus::Ongoing(tally_mut_ref, *class)),
Some(Ongoing(ref mut tally_mut_ref, class, origin)) =>
f(PollStatus::Ongoing(tally_mut_ref, *class, &origin)),
Some(Completed(when, succeeded)) => f(PollStatus::Completed(*when, *succeeded)),
None => f(PollStatus::None),
}?;
Expand All @@ -118,7 +122,7 @@ impl Polling<TallyOf<Test>> for TestPolls {
fn create_ongoing(class: Self::Class) -> Result<Self::Index, ()> {
let mut polls = Polls::get();
let i = polls.keys().rev().next().map_or(0, |x| x + 1);
polls.insert(i, Ongoing(Tally::new(class), class));
polls.insert(i, Ongoing(Tally::new(class), class, Origin));
Polls::set(polls);
Ok(i)
}
Expand Down
Loading

0 comments on commit f50beaa

Please sign in to comment.