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

NoOp Impl Polling Trait #5311

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
16 changes: 16 additions & 0 deletions prdoc/pr_5311.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
title: No-op Impl Polling Trait

doc:
- audience: Runtime Dev
description: |
Provide a NoOp implementation of the Polling trait for unit where the trait is defined and skiping benchmarks that necessitate it's definition.

crates:
- name: pallet-core-fellowship
bump: minor
- name: pallet-ranked-collective
bump: minor
- name: pallet-salary
bump: minor
- name: frame-support
bump: minor
47 changes: 4 additions & 43 deletions substrate/frame/core-fellowship/src/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ use frame_support::{
assert_noop, assert_ok, derive_impl, hypothetically, ord_parameter_types,
pallet_prelude::Weight,
parameter_types,
traits::{ConstU16, EitherOf, IsInVec, MapSuccess, PollStatus, Polling, TryMapSuccess},
traits::{ConstU16, EitherOf, IsInVec, MapSuccess, TryMapSuccess},
};
use frame_system::EnsureSignedBy;
use pallet_ranked_collective::{EnsureRanked, Geometric, Rank, TallyOf, Votes};
use pallet_ranked_collective::{EnsureRanked, Geometric, Rank};
use sp_core::{ConstU32, Get};
use sp_runtime::{
bounded_vec,
traits::{Convert, ReduceBy, ReplaceWithDefault, TryMorphInto},
BuildStorage, DispatchError,
BuildStorage,
};
type Class = Rank;

Expand Down Expand Up @@ -83,45 +83,6 @@ impl Config for Test {
type MaxRank = ConstU32<9>;
}

pub struct TestPolls;
impl Polling<TallyOf<Test>> for TestPolls {
type Index = u8;
type Votes = Votes;
type Moment = u64;
type Class = Class;

fn classes() -> Vec<Self::Class> {
unimplemented!()
}
fn as_ongoing(_: u8) -> Option<(TallyOf<Test>, Self::Class)> {
unimplemented!()
}
fn access_poll<R>(
_: Self::Index,
_: impl FnOnce(PollStatus<&mut TallyOf<Test>, Self::Moment, Self::Class>) -> R,
) -> R {
unimplemented!()
}
fn try_access_poll<R>(
_: Self::Index,
_: impl FnOnce(
PollStatus<&mut TallyOf<Test>, Self::Moment, Self::Class>,
) -> Result<R, DispatchError>,
) -> Result<R, DispatchError> {
unimplemented!()
}

#[cfg(feature = "runtime-benchmarks")]
fn create_ongoing(_: Self::Class) -> Result<Self::Index, ()> {
unimplemented!()
}

#[cfg(feature = "runtime-benchmarks")]
fn end_ongoing(_: Self::Index, _: bool) -> Result<(), ()> {
unimplemented!()
}
}

/// Convert the tally class into the minimum rank required to vote on the poll.
/// MinRank(Class) = Class - Delta
pub struct MinRankOfClass<Delta>(PhantomData<Delta>);
Expand Down Expand Up @@ -154,7 +115,7 @@ impl pallet_ranked_collective::Config for Test {
// Members can exchange up to the rank of 2 below them.
MapSuccess<EnsureRanked<Test, (), 2>, ReduceBy<ConstU16<2>>>,
>;
type Polls = TestPolls;
type Polls = ();
type MinRankOfClass = MinRankOfClass<MinRankOfClassDelta>;
type MemberSwappedHandler = CoreFellowship;
type VoteWeight = Geometric;
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/ranked-collective/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ benchmarks_instance_pallet! {
}

vote {
let class = T::Polls::classes().into_iter().next().unwrap();
let class = T::Polls::classes().into_iter().next().ok_or(BenchmarkError::Skip)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

what weight Skip will result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Secretary Collective requires skipping these extrinsics to generate weights for the ranked_collective pallet instance which uses this NoOp implementation.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I have not checked the actual code, what we need to do here if classes() returns nothing, we need to pass some invalid poll index. The function is still trying to access the storage etc.

let rank = T::MinRankOfClass::convert(class.clone());

let caller = make_member::<T, I>(rank);
Expand All @@ -147,7 +147,7 @@ benchmarks_instance_pallet! {
let n in 0 .. 100;

// Create a poll
let class = T::Polls::classes().into_iter().next().unwrap();
let class = T::Polls::classes().into_iter().next().ok_or(BenchmarkError::Skip)?;
let rank = T::MinRankOfClass::convert(class.clone());
let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll");

Expand Down
47 changes: 4 additions & 43 deletions substrate/frame/salary/src/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ use frame_support::{
assert_noop, assert_ok, derive_impl, hypothetically,
pallet_prelude::Weight,
parameter_types,
traits::{ConstU64, EitherOf, MapSuccess, PollStatus, Polling},
traits::{ConstU64, EitherOf, MapSuccess},
};
use pallet_ranked_collective::{EnsureRanked, Geometric, TallyOf, Votes};
use pallet_ranked_collective::{EnsureRanked, Geometric};
use sp_core::{ConstU16, Get};
use sp_runtime::{
traits::{Convert, ReduceBy, ReplaceWithDefault},
BuildStorage, DispatchError,
BuildStorage,
};

use crate as pallet_salary;
Expand Down Expand Up @@ -55,45 +55,6 @@ impl frame_system::Config for Test {
type Block = Block;
}

pub struct TestPolls;
impl Polling<TallyOf<Test>> for TestPolls {
type Index = u8;
type Votes = Votes;
type Moment = u64;
type Class = Rank;

fn classes() -> Vec<Self::Class> {
unimplemented!()
}
fn as_ongoing(_index: u8) -> Option<(TallyOf<Test>, Self::Class)> {
unimplemented!()
}
fn access_poll<R>(
_index: Self::Index,
_f: impl FnOnce(PollStatus<&mut TallyOf<Test>, Self::Moment, Self::Class>) -> R,
) -> R {
unimplemented!()
}
fn try_access_poll<R>(
_index: Self::Index,
_f: impl FnOnce(
PollStatus<&mut TallyOf<Test>, Self::Moment, Self::Class>,
) -> Result<R, DispatchError>,
) -> Result<R, DispatchError> {
unimplemented!()
}

#[cfg(feature = "runtime-benchmarks")]
fn create_ongoing(_class: Self::Class) -> Result<Self::Index, ()> {
unimplemented!()
}

#[cfg(feature = "runtime-benchmarks")]
fn end_ongoing(_index: Self::Index, _approved: bool) -> Result<(), ()> {
unimplemented!()
}
}

pub struct MinRankOfClass<Delta>(PhantomData<Delta>);
impl<Delta: Get<Rank>> Convert<u16, Rank> for MinRankOfClass<Delta> {
fn convert(a: u16) -> Rank {
Expand Down Expand Up @@ -176,7 +137,7 @@ impl pallet_ranked_collective::Config for Test {
// Members can exchange up to the rank of 2 below them.
MapSuccess<EnsureRanked<Test, (), 2>, ReduceBy<ConstU16<2>>>,
>;
type Polls = TestPolls;
type Polls = ();
type MinRankOfClass = MinRankOfClass<MinRankOfClassDelta>;
type MemberSwappedHandler = Salary;
type VoteWeight = Geometric;
Expand Down
46 changes: 45 additions & 1 deletion substrate/frame/support/src/traits/voting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
//! votes.

use crate::dispatch::Parameter;
use alloc::vec::Vec;
use alloc::{vec, vec::Vec};
use codec::{HasCompact, MaxEncodedLen};
use sp_arithmetic::Perbill;
use sp_runtime::{traits::Member, DispatchError};
Expand Down Expand Up @@ -126,3 +126,47 @@ pub trait Polling<Tally> {
(Self::classes().into_iter().next().expect("Always one class"), u32::max_value())
}
}

impl<Tally> Polling<Tally> for () {
type Index = u32;
type Votes = u32;
type Class = u16;
type Moment = u64;

fn classes() -> Vec<Self::Class> {
vec![]
}

fn as_ongoing(_index: Self::Index) -> Option<(Tally, Self::Class)> {
None
}

fn access_poll<R>(
_index: Self::Index,
f: impl FnOnce(PollStatus<&mut Tally, Self::Moment, Self::Class>) -> R,
) -> R {
f(PollStatus::None)
}

fn try_access_poll<R>(
_index: Self::Index,
f: impl FnOnce(PollStatus<&mut Tally, Self::Moment, Self::Class>) -> Result<R, DispatchError>,
) -> Result<R, DispatchError> {
f(PollStatus::None)
}

#[cfg(feature = "runtime-benchmarks")]
fn create_ongoing(_class: Self::Class) -> Result<Self::Index, ()> {
Err(())
}

#[cfg(feature = "runtime-benchmarks")]
fn end_ongoing(_index: Self::Index, _approved: bool) -> Result<(), ()> {
Err(())
}

#[cfg(feature = "runtime-benchmarks")]
fn max_ongoing() -> (Self::Class, u32) {
(0, 0)
}
}
Loading