Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

WIP Make ChangeMembers trait fallible #5985

Closed
wants to merge 10 commits into from
Closed

WIP Make ChangeMembers trait fallible #5985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4aac2f0
add ensure_can_change_members function to ChangeMembers trait and adj…
apopiak May 11, 2020
3a9fabb
adjust comments and return length
apopiak May 12, 2020
880d4bb
bump runtime spec version
apopiak May 12, 2020
787c0ef
remove todo, usage is safe
apopiak May 12, 2020
542088e
add MaxMembers limit to collective
apopiak May 12, 2020
a5fc3ac
return the new members from change_members instead of the length
apopiak May 12, 2020
b547837
remove unnecessary let
apopiak May 12, 2020
eae912d
Update frame/support/src/traits.rs
apopiak May 13, 2020
eed63fe
add some comments on safety, add an ensure check
apopiak May 13, 2020
bfe5153
Merge branch 'apopiak-change-members' of github.com:paritytech/substr…
apopiak May 13, 2020
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
7 changes: 5 additions & 2 deletions bin/node/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// and set impl_version to 0. If only runtime
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 247,
spec_version: 248,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down Expand Up @@ -377,6 +377,7 @@ impl pallet_democracy::Trait for Runtime {

parameter_types! {
pub const CouncilMotionDuration: BlockNumber = 5 * DAYS;
pub const DesiredMembers: u32 = 13;
}

type CouncilCollective = pallet_collective::Instance1;
Expand All @@ -385,13 +386,13 @@ impl pallet_collective::Trait<CouncilCollective> for Runtime {
type Proposal = Call;
type Event = Event;
type MotionDuration = CouncilMotionDuration;
type MaxMembers = DesiredMembers;
}

parameter_types! {
pub const CandidacyBond: Balance = 10 * DOLLARS;
pub const VotingBond: Balance = 1 * DOLLARS;
pub const TermDuration: BlockNumber = 7 * DAYS;
pub const DesiredMembers: u32 = 13;
pub const DesiredRunnersUp: u32 = 7;
pub const ElectionsPhragmenModuleId: LockIdentifier = *b"phrelect";
}
Expand All @@ -417,6 +418,7 @@ impl pallet_elections_phragmen::Trait for Runtime {

parameter_types! {
pub const TechnicalMotionDuration: BlockNumber = 5 * DAYS;
pub const TechnicalMaxMembers: u32 = 100;
}

type TechnicalCollective = pallet_collective::Instance2;
Expand All @@ -425,6 +427,7 @@ impl pallet_collective::Trait<TechnicalCollective> for Runtime {
type Proposal = Call;
type Event = Event;
type MotionDuration = TechnicalMotionDuration;
type MaxMembers = TechnicalMaxMembers;
}

impl pallet_membership::Trait<pallet_membership::Instance1> for Runtime {
Expand Down
55 changes: 27 additions & 28 deletions frame/collective/src/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use crate::Module as Collective;

const SEED: u32 = 0;

const MAX_MEMBERS: u32 = 1000;
const MAX_PROPOSALS: u32 = 100;
const MAX_BYTES: u32 = 1_024;

Expand All @@ -44,8 +43,8 @@ benchmarks_instance! {
_{ }

set_members {
let m in 1 .. MAX_MEMBERS;
let n in 1 .. MAX_MEMBERS;
let m in 1 .. T::MaxMembers::get();
let n in 1 .. T::MaxMembers::get();

// Set old members.
// We compute the difference of old and new members, so it should influence timing.
Expand Down Expand Up @@ -74,12 +73,12 @@ benchmarks_instance! {
}

execute {
let m in 1 .. MAX_MEMBERS;
let m in 1 .. T::MaxMembers::get();
let b in 1 .. MAX_BYTES;

// Construct `members`.
let mut members = vec![];
for i in 0 .. m {
for i in 0 .. m - 1 {
let member = account("member", i, SEED);
members.push(member);
}
Expand All @@ -100,12 +99,12 @@ benchmarks_instance! {

// This tests when execution would happen immediately after proposal
propose_execute {
let m in 1 .. MAX_MEMBERS;
let m in 1 .. T::MaxMembers::get();
let b in 1 .. MAX_BYTES;

// Construct `members`.
let mut members = vec![];
for i in 0 .. m {
for i in 0 .. m - 1 {
let member = account("member", i, SEED);
members.push(member);
}
Expand All @@ -127,13 +126,13 @@ benchmarks_instance! {

// This tests when proposal is created and queued as "proposed"
propose_proposed {
let m in 1 .. MAX_MEMBERS;
let m in 1 .. T::MaxMembers::get();
let p in 0 .. MAX_PROPOSALS;
let b in 1 .. MAX_BYTES;

// Construct `members`.
let mut members = vec![];
for i in 0 .. m {
for i in 0 .. m - 1 {
let member = account("member", i, SEED);
members.push(member);
}
Expand Down Expand Up @@ -164,13 +163,13 @@ benchmarks_instance! {
}

vote_insert {
let m in 2 .. MAX_MEMBERS;
let m in 5 .. T::MaxMembers::get();
let p in 1 .. MAX_PROPOSALS;
let b in 1 .. MAX_BYTES;

// Construct `members`.
let mut members = vec![];
for i in 0 .. m {
for i in 0 .. m - 1 {
let member = account("member", i, SEED);
members.push(member);
}
Expand All @@ -180,7 +179,7 @@ benchmarks_instance! {
Collective::<T, _>::set_members(SystemOrigin::Root.into(), members.clone(), None)?;

// Threshold is 1 less than the number of members so that one person can vote nay
let threshold = m;
let threshold = m - 1;

// Add previous proposals
let mut last_hash = T::Hash::default();
Expand All @@ -192,7 +191,7 @@ benchmarks_instance! {
}

// Have everyone vote aye on last proposal, while keeping it from passing
for j in 2 .. m {
for j in 2 .. m - 1 {
let voter = &members[j as usize];
let approve = true;
Collective::<T, _>::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), p - 1, approve)?;
Expand All @@ -209,18 +208,18 @@ benchmarks_instance! {
// All proposals exist and the last proposal has just been updated.
assert_eq!(Collective::<T, _>::proposals().len(), p as usize);
let voting = Collective::<T, _>::voting(&last_hash).ok_or(Error::<T, I>::ProposalMissing)?;
assert_eq!(voting.ayes.len(), (m - 2) as usize);
assert_eq!(voting.ayes.len(), (m - 3) as usize);
assert_eq!(voting.nays.len(), 1);
}

vote_disapproved {
let m in 2 .. MAX_MEMBERS;
let m in 2 .. T::MaxMembers::get();
let p in 1 .. MAX_PROPOSALS;
let b in 1 .. MAX_BYTES;

// Construct `members`.
let mut members = vec![];
for i in 0 .. m {
for i in 0 .. m - 1 {
let member = account("member", i, SEED);
members.push(member);
}
Expand All @@ -230,7 +229,7 @@ benchmarks_instance! {
Collective::<T, _>::set_members(SystemOrigin::Root.into(), members.clone(), None)?;

// Threshold is total members so that one nay will disapprove the vote
let threshold = m + 1;
let threshold = m;

// Add previous proposals
let mut last_hash = T::Hash::default();
Expand All @@ -242,7 +241,7 @@ benchmarks_instance! {
}

// Have everyone vote aye on last proposal, while keeping it from passing
for j in 1 .. m {
for j in 1 .. m - 1 {
let voter = &members[j as usize];
let approve = true;
Collective::<T, _>::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), p - 1, approve)?;
Expand All @@ -262,13 +261,13 @@ benchmarks_instance! {
}

vote_approved {
let m in 2 .. MAX_MEMBERS;
let m in 4 .. T::MaxMembers::get();
let p in 1 .. MAX_PROPOSALS;
let b in 1 .. MAX_BYTES;

// Construct `members`.
let mut members = vec![];
for i in 0 .. m {
for i in 0 .. m - 1 {
let member = account("member", i, SEED);
members.push(member);
}
Expand All @@ -293,7 +292,7 @@ benchmarks_instance! {
Collective::<T, _>::vote(SystemOrigin::Signed(caller.clone()).into(), last_hash.clone(), p - 1, false)?;

// Have everyone vote nay on last proposal, while keeping it from failing
for j in 2 .. m {
for j in 2 .. m - 1 {
let voter = &members[j as usize];
let approve = false;
Collective::<T, _>::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), p - 1, approve)?;
Expand All @@ -316,13 +315,13 @@ benchmarks_instance! {
}

close_disapproved {
let m in 2 .. MAX_MEMBERS;
let m in 4 .. T::MaxMembers::get();
let p in 1 .. MAX_PROPOSALS;
let b in 1 .. MAX_BYTES;

// Construct `members`.
let mut members = vec![];
for i in 0 .. m {
for i in 0 .. m - 1 {
let member = account("member", i, SEED);
members.push(member);
}
Expand All @@ -332,7 +331,7 @@ benchmarks_instance! {
Collective::<T, _>::set_members(SystemOrigin::Root.into(), members.clone(), Some(caller.clone()))?;

// Threshold is one less than total members so that two nays will disapprove the vote
let threshold = m;
let threshold = m - 1;

// Add proposals
let mut last_hash = T::Hash::default();
Expand All @@ -345,7 +344,7 @@ benchmarks_instance! {

// Have everyone vote aye on last proposal, while keeping it from passing
// A few abstainers will be the nay votes needed to fail the vote
for j in 2 .. m {
for j in 2 .. m - 1 {
let voter = &members[j as usize];
let approve = true;
Collective::<T, _>::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), p - 1, approve)?;
Expand All @@ -366,13 +365,13 @@ benchmarks_instance! {


close_approved {
let m in 2 .. MAX_MEMBERS;
let m in 4 .. T::MaxMembers::get();
let p in 1 .. MAX_PROPOSALS;
let b in 1 .. MAX_BYTES;

// Construct `members`.
let mut members = vec![];
for i in 0 .. m {
for i in 0 .. m - 1 {
let member = account("member", i, SEED);
members.push(member);
}
Expand All @@ -395,7 +394,7 @@ benchmarks_instance! {

// Have everyone vote nay on last proposal, while keeping it from failing
// A few abstainers will be the aye votes needed to pass the vote
for j in 2 .. m {
for j in 2 .. m - 1 {
let voter = &members[j as usize];
let approve = false;
Collective::<T, _>::vote(SystemOrigin::Signed(voter.clone()).into(), last_hash.clone(), p - 1, approve)?;
Expand Down
Loading