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.

Sign up for GitHub

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

Include submitting origin in PollStatus #4961

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Include submitting origin in PollStatus #4961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions prdoc/pr_4961.prdoc
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading