From 774c5f565e18de95451df68ec8f08aa2f681cccb Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Sun, 29 Aug 2021 22:25:18 +0800 Subject: [PATCH 01/10] Use the precise number of approvals when constructing RawOrgin::Members Close #9604 --- frame/collective/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 39da8e2c45fb7..2a183fa15c91f 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -643,7 +643,7 @@ decl_module! { )?; Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); let (proposal_weight, proposal_count) = - Self::do_approve_proposal(seats, voting, proposal_hash, proposal); + Self::do_approve_proposal(seats, yes_votes, proposal_hash, proposal); return Ok(( Some(T::WeightInfo::close_early_approved(len as u32, seats, proposal_count) .saturating_add(proposal_weight)), @@ -682,7 +682,7 @@ decl_module! { )?; Self::deposit_event(RawEvent::Closed(proposal_hash, yes_votes, no_votes)); let (proposal_weight, proposal_count) = - Self::do_approve_proposal(seats, voting, proposal_hash, proposal); + Self::do_approve_proposal(seats, yes_votes, proposal_hash, proposal); return Ok(( Some(T::WeightInfo::close_approved(len as u32, seats, proposal_count) .saturating_add(proposal_weight)), @@ -764,14 +764,14 @@ impl, I: Instance> Module { /// - `P` is number of active proposals fn do_approve_proposal( seats: MemberCount, - voting: Votes, + yes_votes: MemberCount, proposal_hash: T::Hash, proposal: >::Proposal, ) -> (Weight, u32) { Self::deposit_event(RawEvent::Approved(proposal_hash)); let dispatch_weight = proposal.get_dispatch_info().weight; - let origin = RawOrigin::Members(voting.threshold, seats).into(); + let origin = RawOrigin::Members(yes_votes, seats).into(); let result = proposal.dispatch(origin); Self::deposit_event(RawEvent::Executed( proposal_hash, From 4ce523bf06353312c5d5ea0fab79c9011a6bac2d Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 10 Sep 2021 13:55:06 +0800 Subject: [PATCH 02/10] Split out tests.rs --- frame/collective/src/lib.rs | 1182 +-------------------------------- frame/collective/src/tests.rs | 1175 ++++++++++++++++++++++++++++++++ 2 files changed, 1178 insertions(+), 1179 deletions(-) create mode 100644 frame/collective/src/tests.rs diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 2a183fa15c91f..fc40fd5541293 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -61,6 +61,9 @@ use frame_support::{ }; use frame_system::{self as system, ensure_root, ensure_signed}; +#[cfg(test)] +mod tests; + #[cfg(feature = "runtime-benchmarks")] mod benchmarking; @@ -987,1182 +990,3 @@ impl< O::from(RawOrigin::Members(0u32, 0u32)) } } - -#[cfg(test)] -mod tests { - use super::*; - use crate as collective; - use frame_support::{assert_noop, assert_ok, parameter_types, Hashable}; - use frame_system::{self as system, EventRecord, Phase}; - use hex_literal::hex; - use sp_core::H256; - use sp_runtime::{ - testing::Header, - traits::{BlakeTwo256, IdentityLookup}, - BuildStorage, - }; - - parameter_types! { - pub const BlockHashCount: u64 = 250; - pub const MotionDuration: u64 = 3; - pub const MaxProposals: u32 = 100; - pub const MaxMembers: u32 = 100; - pub BlockWeights: frame_system::limits::BlockWeights = - frame_system::limits::BlockWeights::simple_max(1024); - } - impl frame_system::Config for Test { - type BaseCallFilter = frame_support::traits::Everything; - type BlockWeights = (); - type BlockLength = (); - type DbWeight = (); - type Origin = Origin; - type Index = u64; - type BlockNumber = u64; - type Call = Call; - type Hash = H256; - type Hashing = BlakeTwo256; - type AccountId = u64; - type Lookup = IdentityLookup; - type Header = Header; - type Event = Event; - type BlockHashCount = BlockHashCount; - type Version = (); - type PalletInfo = PalletInfo; - type AccountData = (); - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = (); - type OnSetCode = (); - } - impl Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = PrimeDefaultVote; - type WeightInfo = (); - } - impl Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = MoreThanMajorityThenPrimeDefaultVote; - type WeightInfo = (); - } - impl Config for Test { - type Origin = Origin; - type Proposal = Call; - type Event = Event; - type MotionDuration = MotionDuration; - type MaxProposals = MaxProposals; - type MaxMembers = MaxMembers; - type DefaultVote = PrimeDefaultVote; - type WeightInfo = (); - } - - pub type Block = sp_runtime::generic::Block; - pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; - - frame_support::construct_runtime!( - pub enum Test where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic - { - System: system::{Pallet, Call, Event}, - Collective: collective::::{Pallet, Call, Event, Origin, Config}, - CollectiveMajority: collective::::{Pallet, Call, Event, Origin, Config}, - DefaultCollective: collective::{Pallet, Call, Event, Origin, Config}, - } - ); - - pub fn new_test_ext() -> sp_io::TestExternalities { - let mut ext: sp_io::TestExternalities = GenesisConfig { - collective: collective::GenesisConfig { - members: vec![1, 2, 3], - phantom: Default::default(), - }, - collective_majority: collective::GenesisConfig { - members: vec![1, 2, 3, 4, 5], - phantom: Default::default(), - }, - default_collective: Default::default(), - } - .build_storage() - .unwrap() - .into(); - ext.execute_with(|| System::set_block_number(1)); - ext - } - - fn make_proposal(value: u64) -> Call { - Call::System(frame_system::Call::remark(value.encode())) - } - - #[test] - fn motions_basic_environment_works() { - new_test_ext().execute_with(|| { - assert_eq!(Collective::members(), vec![1, 2, 3]); - assert_eq!(*Collective::proposals(), Vec::::new()); - }); - } - - #[test] - fn close_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(3); - assert_noop!( - Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - ), - Error::::TooEarly - ); - - System::set_block_number(4); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!( - System::events(), - vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::Collective(RawEvent::Disapproved(hash.clone()))) - ] - ); - }); - } - - #[test] - fn proposal_weight_limit_works_on_approve() { - new_test_ext().execute_with(|| { - let proposal = - Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - // Set 1 as prime voter - Prime::::set(Some(1)); - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - // With 1's prime vote, this should pass - System::set_block_number(4); - assert_noop!( - Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight - 100, - proposal_len - ), - Error::::WrongProposalWeight - ); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - }) - } - - #[test] - fn proposal_weight_limit_ignored_on_disapprove() { - new_test_ext().execute_with(|| { - let proposal = - Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - // No votes, this proposal wont pass - System::set_block_number(4); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight - 100, - proposal_len - )); - }) - } - - #[test] - fn close_with_prime_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members( - Origin::root(), - vec![1, 2, 3], - Some(3), - MaxMembers::get() - )); - - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!( - System::events(), - vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::Collective(RawEvent::Disapproved(hash.clone()))) - ] - ); - }); - } - - #[test] - fn close_with_voting_prime_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::set_members( - Origin::root(), - vec![1, 2, 3], - Some(1), - MaxMembers::get() - )); - - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!( - System::events(), - vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash.clone(), 3, 0))), - record(Event::Collective(RawEvent::Approved(hash.clone()))), - record(Event::Collective(RawEvent::Executed( - hash.clone(), - Err(DispatchError::BadOrigin) - ))) - ] - ); - }); - } - - #[test] - fn close_with_no_prime_but_majority_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(CollectiveMajority::set_members( - Origin::root(), - vec![1, 2, 3, 4, 5], - Some(5), - MaxMembers::get() - )); - - assert_ok!(CollectiveMajority::propose( - Origin::signed(1), - 5, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(CollectiveMajority::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); - - System::set_block_number(4); - assert_ok!(CollectiveMajority::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!( - System::events(), - vec![ - record(Event::CollectiveMajority(RawEvent::Proposed(1, 0, hash.clone(), 5))), - record(Event::CollectiveMajority(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::CollectiveMajority(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::CollectiveMajority(RawEvent::Voted(3, hash.clone(), true, 3, 0))), - record(Event::CollectiveMajority(RawEvent::Closed(hash.clone(), 5, 0))), - record(Event::CollectiveMajority(RawEvent::Approved(hash.clone()))), - record(Event::CollectiveMajority(RawEvent::Executed( - hash.clone(), - Err(DispatchError::BadOrigin) - ))) - ] - ); - }); - } - - #[test] - fn removal_of_old_voters_votes_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - let end = 4; - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) - ); - Collective::change_members_sorted(&[4], &[1], &[2, 3, 4]); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) - ); - - let proposal = make_proposal(69); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::propose( - Origin::signed(2), - 2, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 1, true)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) - ); - Collective::change_members_sorted(&[], &[3], &[2, 4]); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) - ); - }); - } - - #[test] - fn removal_of_old_voters_votes_works_with_set_members() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - let end = 4; - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) - ); - assert_ok!(Collective::set_members( - Origin::root(), - vec![2, 3, 4], - None, - MaxMembers::get() - )); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) - ); - - let proposal = make_proposal(69); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = BlakeTwo256::hash_of(&proposal); - assert_ok!(Collective::propose( - Origin::signed(2), - 2, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 1, true)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) - ); - assert_ok!(Collective::set_members( - Origin::root(), - vec![2, 4], - None, - MaxMembers::get() - )); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) - ); - }); - } - - #[test] - fn propose_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_eq!(*Collective::proposals(), vec![hash]); - assert_eq!(Collective::proposal_of(&hash), Some(proposal)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 3, ayes: vec![], nays: vec![], end }) - ); - - assert_eq!( - System::events(), - vec![EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"] - .into(), - 3, - )), - topics: vec![], - }] - ); - }); - } - - #[test] - fn limit_active_proposals() { - new_test_ext().execute_with(|| { - for i in 0..MaxProposals::get() { - let proposal = make_proposal(i as u64); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - } - let proposal = make_proposal(MaxProposals::get() as u64 + 1); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_noop!( - Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len), - Error::::TooManyProposals - ); - }) - } - - #[test] - fn correct_validate_and_get_proposal() { - new_test_ext().execute_with(|| { - let proposal = - Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); - let length = proposal.encode().len() as u32; - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - length - )); - - let hash = BlakeTwo256::hash_of(&proposal); - let weight = proposal.get_dispatch_info().weight; - assert_noop!( - Collective::validate_and_get_proposal( - &BlakeTwo256::hash_of(&vec![3; 4]), - length, - weight - ), - Error::::ProposalMissing - ); - assert_noop!( - Collective::validate_and_get_proposal(&hash, length - 2, weight), - Error::::WrongProposalLength - ); - assert_noop!( - Collective::validate_and_get_proposal(&hash, length, weight - 10), - Error::::WrongProposalWeight - ); - let res = Collective::validate_and_get_proposal(&hash, length, weight); - assert_ok!(res.clone()); - let (retrieved_proposal, len) = res.unwrap(); - assert_eq!(length as usize, len); - assert_eq!(proposal, retrieved_proposal); - }) - } - - #[test] - fn motions_ignoring_non_collective_proposals_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - assert_noop!( - Collective::propose( - Origin::signed(42), - 3, - Box::new(proposal.clone()), - proposal_len - ), - Error::::NotMember - ); - }); - } - - #[test] - fn motions_ignoring_non_collective_votes_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_noop!( - Collective::vote(Origin::signed(42), hash.clone(), 0, true), - Error::::NotMember, - ); - }); - } - - #[test] - fn motions_ignoring_bad_index_collective_vote_works() { - new_test_ext().execute_with(|| { - System::set_block_number(3); - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_noop!( - Collective::vote(Origin::signed(2), hash.clone(), 1, true), - Error::::WrongIndex, - ); - }); - } - - #[test] - fn motions_vote_after_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len - )); - // Initially there a no votes when the motion is proposed. - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![], end }) - ); - // Cast first aye vote. - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) - ); - // Try to cast a duplicate aye vote. - assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, true), - Error::::DuplicateVote, - ); - // Cast a nay vote. - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) - ); - // Try to cast a duplicate nay vote. - assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, false), - Error::::DuplicateVote, - ); - - assert_eq!( - System::events(), - vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed( - 1, - 0, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 2, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 1, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - true, - 1, - 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 1, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - false, - 0, - 1, - )), - topics: vec![], - } - ] - ); - }); - } - - #[test] - fn motions_all_first_vote_free_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - let end = 4; - assert_ok!(Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len, - )); - assert_eq!( - Collective::voting(&hash), - Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![], end }) - ); - - // For the motion, acc 2's first vote, expecting Ok with Pays::No. - let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(2), hash.clone(), 0, true); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); - - // Duplicate vote, expecting error with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(2), hash.clone(), 0, true); - assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); - - // Modifying vote, expecting ok with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(2), hash.clone(), 0, false); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); - - // For the motion, acc 3's first vote, expecting Ok with Pays::No. - let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(3), hash.clone(), 0, true); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); - - // acc 3 modify the vote, expecting Ok with Pays::Yes. - let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(3), hash.clone(), 0, false); - assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); - - // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info - - let proposal_weight = proposal.get_dispatch_info().weight; - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); - assert_eq!(close_rval.unwrap().pays_fee, Pays::No); - - // trying to close the proposal, which is already closed. - // Expecting error "ProposalMissing" with Pays::Yes - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); - assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); - }); - } - - #[test] - fn motions_reproposing_disapproved_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - assert_eq!(*Collective::proposals(), vec![]); - assert_ok!(Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len - )); - assert_eq!(*Collective::proposals(), vec![hash]); - }); - } - - #[test] - fn motions_disapproval_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - - assert_eq!( - System::events(), - vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed( - 1, - 0, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 3, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 1, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - true, - 1, - 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 2, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - false, - 1, - 1, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Closed( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 1, - 1, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Disapproved( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - )), - topics: vec![], - } - ] - ); - }); - } - - #[test] - fn motions_approval_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len - )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - - assert_eq!( - System::events(), - vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed( - 1, - 0, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 2, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 1, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - true, - 1, - 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 2, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - true, - 2, - 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Closed( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 2, - 0, - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Approved( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - )), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Executed( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - Err(DispatchError::BadOrigin), - )), - topics: vec![], - } - ] - ); - }); - } - - #[test] - fn motion_with_no_votes_closes_with_disapproval() { - new_test_ext().execute_with(|| { - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let proposal_weight = proposal.get_dispatch_info().weight; - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - proposal_len - )); - assert_eq!( - System::events()[0], - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))) - ); - - // Closing the motion too early is not possible because it has neither - // an approving or disapproving simple majority due to the lack of votes. - assert_noop!( - Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - ), - Error::::TooEarly - ); - - // Once the motion duration passes, - let closing_block = System::block_number() + MotionDuration::get(); - System::set_block_number(closing_block); - // we can successfully close the motion. - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); - - // Events show that the close ended in a disapproval. - assert_eq!( - System::events()[1], - record(Event::Collective(RawEvent::Closed(hash.clone(), 0, 3))) - ); - assert_eq!( - System::events()[2], - record(Event::Collective(RawEvent::Disapproved(hash.clone()))) - ); - }) - } - - #[test] - fn close_disapprove_does_not_care_about_weight_or_len() { - // This test confirms that if you close a proposal that would be disapproved, - // we do not care about the proposal length or proposal weight since it will - // not be read from storage or executed. - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len - )); - // First we make the proposal succeed - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - // It will not close with bad weight/len information - assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), - Error::::WrongProposalLength, - ); - assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), - Error::::WrongProposalWeight, - ); - // Now we make the proposal fail - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - // It can close even if the weight/len information is bad - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); - }) - } - - #[test] - fn disapprove_proposal_works() { - new_test_ext().execute_with(|| { - let proposal = make_proposal(42); - let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); - let hash: H256 = proposal.blake2_256().into(); - assert_ok!(Collective::propose( - Origin::signed(1), - 2, - Box::new(proposal.clone()), - proposal_len - )); - // Proposal would normally succeed - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - // But Root can disapprove and remove it anyway - assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; - assert_eq!( - System::events(), - vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 2))), - record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::Collective(RawEvent::Disapproved(hash.clone()))), - ] - ); - }) - } - - #[test] - #[should_panic(expected = "Members cannot contain duplicate accounts.")] - fn genesis_build_panics_with_duplicate_members() { - collective::GenesisConfig:: { - members: vec![1, 2, 3, 1], - phantom: Default::default(), - } - .build_storage() - .unwrap(); - } -} diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs new file mode 100644 index 0000000000000..880897e7b94e8 --- /dev/null +++ b/frame/collective/src/tests.rs @@ -0,0 +1,1175 @@ +use super::*; +use crate as collective; +use frame_support::{assert_noop, assert_ok, parameter_types, Hashable}; +use frame_system::{self as system, EventRecord, Phase}; +use hex_literal::hex; +use sp_core::H256; +use sp_runtime::{ + testing::Header, + traits::{BlakeTwo256, IdentityLookup}, + BuildStorage, +}; + +parameter_types! { + pub const BlockHashCount: u64 = 250; + pub const MotionDuration: u64 = 3; + pub const MaxProposals: u32 = 100; + pub const MaxMembers: u32 = 100; + pub BlockWeights: frame_system::limits::BlockWeights = + frame_system::limits::BlockWeights::simple_max(1024); +} +impl frame_system::Config for Test { + type BaseCallFilter = frame_support::traits::Everything; + type BlockWeights = (); + type BlockLength = (); + type DbWeight = (); + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Call = Call; + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type Event = Event; + type BlockHashCount = BlockHashCount; + type Version = (); + type PalletInfo = PalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type SS58Prefix = (); + type OnSetCode = (); +} +impl Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = PrimeDefaultVote; + type WeightInfo = (); +} +impl Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = MoreThanMajorityThenPrimeDefaultVote; + type WeightInfo = (); +} +impl Config for Test { + type Origin = Origin; + type Proposal = Call; + type Event = Event; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = PrimeDefaultVote; + type WeightInfo = (); +} + +pub type Block = sp_runtime::generic::Block; +pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; + +frame_support::construct_runtime!( + pub enum Test where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: system::{Pallet, Call, Event}, + Collective: collective::::{Pallet, Call, Event, Origin, Config}, + CollectiveMajority: collective::::{Pallet, Call, Event, Origin, Config}, + DefaultCollective: collective::{Pallet, Call, Event, Origin, Config}, + } +); + +pub fn new_test_ext() -> sp_io::TestExternalities { + let mut ext: sp_io::TestExternalities = GenesisConfig { + collective: collective::GenesisConfig { + members: vec![1, 2, 3], + phantom: Default::default(), + }, + collective_majority: collective::GenesisConfig { + members: vec![1, 2, 3, 4, 5], + phantom: Default::default(), + }, + default_collective: Default::default(), + } + .build_storage() + .unwrap() + .into(); + ext.execute_with(|| System::set_block_number(1)); + ext +} + +fn make_proposal(value: u64) -> Call { + Call::System(frame_system::Call::remark(value.encode())) +} + +#[test] +fn motions_basic_environment_works() { + new_test_ext().execute_with(|| { + assert_eq!(Collective::members(), vec![1, 2, 3]); + assert_eq!(*Collective::proposals(), Vec::::new()); + }); +} + +#[test] +fn close_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(3); + assert_noop!( + Collective::close( + Origin::signed(4), + hash.clone(), + 0, + proposal_weight, + proposal_len + ), + Error::::TooEarly + ); + + System::set_block_number(4); + assert_ok!(Collective::close( + Origin::signed(4), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + + let record = + |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!( + System::events(), + vec![ + record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::Collective(RawEvent::Closed(hash.clone(), 2, 1))), + record(Event::Collective(RawEvent::Disapproved(hash.clone()))) + ] + ); + }); +} + +#[test] +fn proposal_weight_limit_works_on_approve() { + new_test_ext().execute_with(|| { + let proposal = + Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + // Set 1 as prime voter + Prime::::set(Some(1)); + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + // With 1's prime vote, this should pass + System::set_block_number(4); + assert_noop!( + Collective::close( + Origin::signed(4), + hash.clone(), + 0, + proposal_weight - 100, + proposal_len + ), + Error::::WrongProposalWeight + ); + assert_ok!(Collective::close( + Origin::signed(4), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + }) +} + +#[test] +fn proposal_weight_limit_ignored_on_disapprove() { + new_test_ext().execute_with(|| { + let proposal = + Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + // No votes, this proposal wont pass + System::set_block_number(4); + assert_ok!(Collective::close( + Origin::signed(4), + hash.clone(), + 0, + proposal_weight - 100, + proposal_len + )); + }) +} + +#[test] +fn close_with_prime_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::set_members( + Origin::root(), + vec![1, 2, 3], + Some(3), + MaxMembers::get() + )); + + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(Collective::close( + Origin::signed(4), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + + let record = + |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!( + System::events(), + vec![ + record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::Collective(RawEvent::Closed(hash.clone(), 2, 1))), + record(Event::Collective(RawEvent::Disapproved(hash.clone()))) + ] + ); + }); +} + +#[test] +fn close_with_voting_prime_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::set_members( + Origin::root(), + vec![1, 2, 3], + Some(1), + MaxMembers::get() + )); + + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(Collective::close( + Origin::signed(4), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + + let record = + |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!( + System::events(), + vec![ + record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), + record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::Collective(RawEvent::Closed(hash.clone(), 3, 0))), + record(Event::Collective(RawEvent::Approved(hash.clone()))), + record(Event::Collective(RawEvent::Executed( + hash.clone(), + Err(DispatchError::BadOrigin) + ))) + ] + ); + }); +} + +#[test] +fn close_with_no_prime_but_majority_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(CollectiveMajority::set_members( + Origin::root(), + vec![1, 2, 3, 4, 5], + Some(5), + MaxMembers::get() + )); + + assert_ok!(CollectiveMajority::propose( + Origin::signed(1), + 5, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(CollectiveMajority::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); + + System::set_block_number(4); + assert_ok!(CollectiveMajority::close( + Origin::signed(4), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + + let record = + |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!( + System::events(), + vec![ + record(Event::CollectiveMajority(RawEvent::Proposed(1, 0, hash.clone(), 5))), + record(Event::CollectiveMajority(RawEvent::Voted(1, hash.clone(), true, 1, 0))), + record(Event::CollectiveMajority(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::CollectiveMajority(RawEvent::Voted(3, hash.clone(), true, 3, 0))), + record(Event::CollectiveMajority(RawEvent::Closed(hash.clone(), 5, 0))), + record(Event::CollectiveMajority(RawEvent::Approved(hash.clone()))), + record(Event::CollectiveMajority(RawEvent::Executed( + hash.clone(), + Err(DispatchError::BadOrigin) + ))) + ] + ); + }); +} + +#[test] +fn removal_of_old_voters_votes_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + let end = 4; + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) + ); + Collective::change_members_sorted(&[4], &[1], &[2, 3, 4]); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) + ); + + let proposal = make_proposal(69); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::propose( + Origin::signed(2), + 2, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 1, true)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) + ); + Collective::change_members_sorted(&[], &[3], &[2, 4]); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) + ); + }); +} + +#[test] +fn removal_of_old_voters_votes_works_with_set_members() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + let end = 4; + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) + ); + assert_ok!(Collective::set_members( + Origin::root(), + vec![2, 3, 4], + None, + MaxMembers::get() + )); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) + ); + + let proposal = make_proposal(69); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = BlakeTwo256::hash_of(&proposal); + assert_ok!(Collective::propose( + Origin::signed(2), + 2, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 1, true)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) + ); + assert_ok!(Collective::set_members( + Origin::root(), + vec![2, 4], + None, + MaxMembers::get() + )); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) + ); + }); +} + +#[test] +fn propose_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_eq!(*Collective::proposals(), vec![hash]); + assert_eq!(Collective::proposal_of(&hash), Some(proposal)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 3, ayes: vec![], nays: vec![], end }) + ); + + assert_eq!( + System::events(), + vec![EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Proposed( + 1, + 0, + hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"] + .into(), + 3, + )), + topics: vec![], + }] + ); + }); +} + +#[test] +fn limit_active_proposals() { + new_test_ext().execute_with(|| { + for i in 0..MaxProposals::get() { + let proposal = make_proposal(i as u64); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + } + let proposal = make_proposal(MaxProposals::get() as u64 + 1); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_noop!( + Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len), + Error::::TooManyProposals + ); + }) +} + +#[test] +fn correct_validate_and_get_proposal() { + new_test_ext().execute_with(|| { + let proposal = + Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); + let length = proposal.encode().len() as u32; + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + length + )); + + let hash = BlakeTwo256::hash_of(&proposal); + let weight = proposal.get_dispatch_info().weight; + assert_noop!( + Collective::validate_and_get_proposal( + &BlakeTwo256::hash_of(&vec![3; 4]), + length, + weight + ), + Error::::ProposalMissing + ); + assert_noop!( + Collective::validate_and_get_proposal(&hash, length - 2, weight), + Error::::WrongProposalLength + ); + assert_noop!( + Collective::validate_and_get_proposal(&hash, length, weight - 10), + Error::::WrongProposalWeight + ); + let res = Collective::validate_and_get_proposal(&hash, length, weight); + assert_ok!(res.clone()); + let (retrieved_proposal, len) = res.unwrap(); + assert_eq!(length as usize, len); + assert_eq!(proposal, retrieved_proposal); + }) +} + +#[test] +fn motions_ignoring_non_collective_proposals_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + assert_noop!( + Collective::propose( + Origin::signed(42), + 3, + Box::new(proposal.clone()), + proposal_len + ), + Error::::NotMember + ); + }); +} + +#[test] +fn motions_ignoring_non_collective_votes_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_noop!( + Collective::vote(Origin::signed(42), hash.clone(), 0, true), + Error::::NotMember, + ); + }); +} + +#[test] +fn motions_ignoring_bad_index_collective_vote_works() { + new_test_ext().execute_with(|| { + System::set_block_number(3); + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_noop!( + Collective::vote(Origin::signed(2), hash.clone(), 1, true), + Error::::WrongIndex, + ); + }); +} + +#[test] +fn motions_vote_after_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len + )); + // Initially there a no votes when the motion is proposed. + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![], end }) + ); + // Cast first aye vote. + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) + ); + // Try to cast a duplicate aye vote. + assert_noop!( + Collective::vote(Origin::signed(1), hash.clone(), 0, true), + Error::::DuplicateVote, + ); + // Cast a nay vote. + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) + ); + // Try to cast a duplicate nay vote. + assert_noop!( + Collective::vote(Origin::signed(1), hash.clone(), 0, false), + Error::::DuplicateVote, + ); + + assert_eq!( + System::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Proposed( + 1, + 0, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + 2, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted( + 1, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + true, + 1, + 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted( + 1, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + false, + 0, + 1, + )), + topics: vec![], + } + ] + ); + }); +} + +#[test] +fn motions_all_first_vote_free_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + let end = 4; + assert_ok!(Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len, + )); + assert_eq!( + Collective::voting(&hash), + Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![], end }) + ); + + // For the motion, acc 2's first vote, expecting Ok with Pays::No. + let vote_rval: DispatchResultWithPostInfo = + Collective::vote(Origin::signed(2), hash.clone(), 0, true); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); + + // Duplicate vote, expecting error with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = + Collective::vote(Origin::signed(2), hash.clone(), 0, true); + assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); + + // Modifying vote, expecting ok with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = + Collective::vote(Origin::signed(2), hash.clone(), 0, false); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); + + // For the motion, acc 3's first vote, expecting Ok with Pays::No. + let vote_rval: DispatchResultWithPostInfo = + Collective::vote(Origin::signed(3), hash.clone(), 0, true); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); + + // acc 3 modify the vote, expecting Ok with Pays::Yes. + let vote_rval: DispatchResultWithPostInfo = + Collective::vote(Origin::signed(3), hash.clone(), 0, false); + assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); + + // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info + + let proposal_weight = proposal.get_dispatch_info().weight; + let close_rval: DispatchResultWithPostInfo = Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len, + ); + assert_eq!(close_rval.unwrap().pays_fee, Pays::No); + + // trying to close the proposal, which is already closed. + // Expecting error "ProposalMissing" with Pays::Yes + let close_rval: DispatchResultWithPostInfo = Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len, + ); + assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); + }); +} + +#[test] +fn motions_reproposing_disapproved_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + assert_eq!(*Collective::proposals(), vec![]); + assert_ok!(Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len + )); + assert_eq!(*Collective::proposals(), vec![hash]); + }); +} + +#[test] +fn motions_disapproval_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + + assert_eq!( + System::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Proposed( + 1, + 0, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + 3, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted( + 1, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + true, + 1, + 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted( + 2, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + false, + 1, + 1, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Closed( + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + 1, + 1, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Disapproved( + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + )), + topics: vec![], + } + ] + ); + }); +} + +#[test] +fn motions_approval_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + + assert_eq!( + System::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Proposed( + 1, + 0, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + 2, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted( + 1, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + true, + 1, + 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted( + 2, + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + true, + 2, + 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Closed( + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + 2, + 0, + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Approved( + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + )), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Executed( + hex![ + "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" + ] + .into(), + Err(DispatchError::BadOrigin), + )), + topics: vec![], + } + ] + ); + }); +} + +#[test] +fn motion_with_no_votes_closes_with_disapproval() { + new_test_ext().execute_with(|| { + let record = + |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose( + Origin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + assert_eq!( + System::events()[0], + record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))) + ); + + // Closing the motion too early is not possible because it has neither + // an approving or disapproving simple majority due to the lack of votes. + assert_noop!( + Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len + ), + Error::::TooEarly + ); + + // Once the motion duration passes, + let closing_block = System::block_number() + MotionDuration::get(); + System::set_block_number(closing_block); + // we can successfully close the motion. + assert_ok!(Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + + // Events show that the close ended in a disapproval. + assert_eq!( + System::events()[1], + record(Event::Collective(RawEvent::Closed(hash.clone(), 0, 3))) + ); + assert_eq!( + System::events()[2], + record(Event::Collective(RawEvent::Disapproved(hash.clone()))) + ); + }) +} + +#[test] +fn close_disapprove_does_not_care_about_weight_or_len() { + // This test confirms that if you close a proposal that would be disapproved, + // we do not care about the proposal length or proposal weight since it will + // not be read from storage or executed. + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len + )); + // First we make the proposal succeed + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + // It will not close with bad weight/len information + assert_noop!( + Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), + Error::::WrongProposalLength, + ); + assert_noop!( + Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), + Error::::WrongProposalWeight, + ); + // Now we make the proposal fail + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + // It can close even if the weight/len information is bad + assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); + }) +} + +#[test] +fn disapprove_proposal_works() { + new_test_ext().execute_with(|| { + let proposal = make_proposal(42); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let hash: H256 = proposal.blake2_256().into(); + assert_ok!(Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len + )); + // Proposal would normally succeed + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + // But Root can disapprove and remove it anyway + assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); + let record = + |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_eq!( + System::events(), + vec![ + record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 2))), + record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), + record(Event::Collective(RawEvent::Disapproved(hash.clone()))), + ] + ); + }) +} + +#[test] +#[should_panic(expected = "Members cannot contain duplicate accounts.")] +fn genesis_build_panics_with_duplicate_members() { + collective::GenesisConfig:: { + members: vec![1, 2, 3, 1], + phantom: Default::default(), + } + .build_storage() + .unwrap(); +} From 118c272d5917ff705f461b71febcbee458963790 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 10 Sep 2021 15:16:39 +0800 Subject: [PATCH 03/10] Add a test for dispatching with yes votes instead of voting threshold --- frame/collective/src/tests.rs | 186 +++++++++++++++++++++++++--------- 1 file changed, 140 insertions(+), 46 deletions(-) diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs index 880897e7b94e8..f4597f2e9c304 100644 --- a/frame/collective/src/tests.rs +++ b/frame/collective/src/tests.rs @@ -3,13 +3,71 @@ use crate as collective; use frame_support::{assert_noop, assert_ok, parameter_types, Hashable}; use frame_system::{self as system, EventRecord, Phase}; use hex_literal::hex; -use sp_core::H256; +use sp_core::{u32_trait::{_3, _4}, H256}; use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, BuildStorage, }; +pub type Block = sp_runtime::generic::Block; +pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; + +frame_support::construct_runtime!( + pub enum Test where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: system::{Pallet, Call, Event}, + Collective: collective::::{Pallet, Call, Event, Origin, Config}, + CollectiveMajority: collective::::{Pallet, Call, Event, Origin, Config}, + DefaultCollective: collective::{Pallet, Call, Event, Origin, Config}, + Democracy: mock_democracy::{Pallet, Call, Event}, + } +); + +mod mock_democracy { + pub use pallet::*; + #[frame_support::pallet] + pub mod pallet { + use frame_support::{ + pallet_prelude::*, + traits::EnsureOrigin, + }; + use frame_system::pallet_prelude::*; + use sp_runtime::DispatchResult; + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config + Sized { + type Event: From> + IsType<::Event>; + type ExternalMajorityOrigin: EnsureOrigin; + } + + #[pallet::call] + impl Pallet { + #[pallet::weight(0)] + pub fn external_propose_majority( + origin: OriginFor, + ) -> DispatchResult { + T::ExternalMajorityOrigin::ensure_origin(origin)?; + Self::deposit_event(Event::::ExternalProposed); + Ok(()) + } + } + + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { + ExternalProposed, + } + } +} + parameter_types! { pub const BlockHashCount: u64 = 250; pub const MotionDuration: u64 = 3; @@ -63,6 +121,10 @@ impl Config for Test { type DefaultVote = MoreThanMajorityThenPrimeDefaultVote; type WeightInfo = (); } +impl mock_democracy::Config for Test { + type Event = Event; + type ExternalMajorityOrigin = EnsureProportionAtLeast<_3, _4, u64, Instance1>; +} impl Config for Test { type Origin = Origin; type Proposal = Call; @@ -74,22 +136,6 @@ impl Config for Test { type WeightInfo = (); } -pub type Block = sp_runtime::generic::Block; -pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; - -frame_support::construct_runtime!( - pub enum Test where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic - { - System: system::{Pallet, Call, Event}, - Collective: collective::::{Pallet, Call, Event, Origin, Config}, - CollectiveMajority: collective::::{Pallet, Call, Event, Origin, Config}, - DefaultCollective: collective::{Pallet, Call, Event, Origin, Config}, - } -); - pub fn new_test_ext() -> sp_io::TestExternalities { let mut ext: sp_io::TestExternalities = GenesisConfig { collective: collective::GenesisConfig { @@ -697,43 +743,17 @@ fn motions_vote_after_works() { vec![ EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed( - 1, - 0, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 2, - )), + event: Event::Collective(RawEvent::Proposed(1, 0, hash, 2)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 1, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - true, - 1, - 0, - )), + event: Event::Collective(RawEvent::Voted(1, hash, true, 1, 0)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 1, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - false, - 0, - 1, - )), + event: Event::Collective(RawEvent::Voted(1, hash, false, 0, 1)), topics: vec![], } ] @@ -841,6 +861,80 @@ fn motions_reproposing_disapproved_works() { }); } +#[test] +fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { + new_test_ext().execute_with(|| { + let proposal = Call::Democracy(mock_democracy::Call::external_propose_majority()); + let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); + let proposal_weight = proposal.get_dispatch_info().weight; + let hash: H256 = proposal.blake2_256().into(); + // The voting threshold is 2, but the required votes for `ExternalMajorityOrigin` is 3. + // The proposal will be executed regardless of the voting threshold + // as long as we have enough yes votes. + assert_ok!(Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len + )); + assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 0, true)); + assert_ok!(Collective::close( + Origin::signed(2), + hash.clone(), + 0, + proposal_weight, + proposal_len + )); + assert_eq!( + System::events(), + vec![ + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Proposed(1, 0, hash, 2)), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted(1, hash, true, 1, 0)), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted(2, hash, true, 2, 0)), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Voted(3, hash, true, 3, 0)), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Closed(hash, 3, 0)), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Approved(hash)), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Democracy(mock_democracy::pallet::Event::::ExternalProposed), + topics: vec![], + }, + EventRecord { + phase: Phase::Initialization, + event: Event::Collective(RawEvent::Executed(hash, Ok(()))), + topics: vec![], + } + ] + ); + }); +} + #[test] fn motions_disapproval_works() { new_test_ext().execute_with(|| { From 3cc7ccc0031a031d1b4d1d0d9a66e7dfc1d1e14b Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 10 Sep 2021 15:29:35 +0800 Subject: [PATCH 04/10] Simplify tests - Also add copyright header. --- frame/collective/src/tests.rs | 482 +++++++++++----------------------- 1 file changed, 147 insertions(+), 335 deletions(-) diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs index f4597f2e9c304..c10d8d0b19330 100644 --- a/frame/collective/src/tests.rs +++ b/frame/collective/src/tests.rs @@ -1,9 +1,29 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + use super::*; use crate as collective; use frame_support::{assert_noop, assert_ok, parameter_types, Hashable}; use frame_system::{self as system, EventRecord, Phase}; use hex_literal::hex; -use sp_core::{u32_trait::{_3, _4}, H256}; +use sp_core::{ + u32_trait::{_3, _4}, + H256, +}; use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup}, @@ -31,10 +51,7 @@ mod mock_democracy { pub use pallet::*; #[frame_support::pallet] pub mod pallet { - use frame_support::{ - pallet_prelude::*, - traits::EnsureOrigin, - }; + use frame_support::{pallet_prelude::*, traits::EnsureOrigin}; use frame_system::pallet_prelude::*; use sp_runtime::DispatchResult; @@ -51,9 +68,7 @@ mod mock_democracy { #[pallet::call] impl Pallet { #[pallet::weight(0)] - pub fn external_propose_majority( - origin: OriginFor, - ) -> DispatchResult { + pub fn external_propose_majority(origin: OriginFor) -> DispatchResult { T::ExternalMajorityOrigin::ensure_origin(origin)?; Self::deposit_event(Event::::ExternalProposed); Ok(()) @@ -181,40 +196,27 @@ fn close_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); System::set_block_number(3); assert_noop!( - Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - ), + Collective::close(Origin::signed(4), hash, 0, proposal_weight, proposal_len), Error::::TooEarly ); System::set_block_number(4); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::close(Origin::signed(4), hash, 0, proposal_weight, proposal_len)); - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::Collective(RawEvent::Disapproved(hash.clone()))) + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(RawEvent::Closed(hash, 2, 1))), + record(Event::Collective(RawEvent::Disapproved(hash))) ] ); }); @@ -236,26 +238,14 @@ fn proposal_weight_limit_works_on_approve() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); // With 1's prime vote, this should pass System::set_block_number(4); assert_noop!( - Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight - 100, - proposal_len - ), + Collective::close(Origin::signed(4), hash, 0, proposal_weight - 100, proposal_len), Error::::WrongProposalWeight ); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::close(Origin::signed(4), hash, 0, proposal_weight, proposal_len)); }) } @@ -278,7 +268,7 @@ fn proposal_weight_limit_ignored_on_disapprove() { System::set_block_number(4); assert_ok!(Collective::close( Origin::signed(4), - hash.clone(), + hash, 0, proposal_weight - 100, proposal_len @@ -306,28 +296,21 @@ fn close_with_prime_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); System::set_block_number(4); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::close(Origin::signed(4), hash, 0, proposal_weight, proposal_len)); - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash.clone(), 2, 1))), - record(Event::Collective(RawEvent::Disapproved(hash.clone()))) + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(RawEvent::Closed(hash, 2, 1))), + record(Event::Collective(RawEvent::Disapproved(hash))) ] ); }); @@ -353,32 +336,22 @@ fn close_with_voting_prime_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); System::set_block_number(4); - assert_ok!(Collective::close( - Origin::signed(4), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::close(Origin::signed(4), hash, 0, proposal_weight, proposal_len)); - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))), - record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::Collective(RawEvent::Closed(hash.clone(), 3, 0))), - record(Event::Collective(RawEvent::Approved(hash.clone()))), - record(Event::Collective(RawEvent::Executed( - hash.clone(), - Err(DispatchError::BadOrigin) - ))) + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(RawEvent::Closed(hash, 3, 0))), + record(Event::Collective(RawEvent::Approved(hash))), + record(Event::Collective(RawEvent::Executed(hash, Err(DispatchError::BadOrigin)))) ] ); }); @@ -404,32 +377,31 @@ fn close_with_no_prime_but_majority_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(CollectiveMajority::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash.clone(), 0, true)); + assert_ok!(CollectiveMajority::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(CollectiveMajority::vote(Origin::signed(2), hash, 0, true)); + assert_ok!(CollectiveMajority::vote(Origin::signed(3), hash, 0, true)); System::set_block_number(4); assert_ok!(CollectiveMajority::close( Origin::signed(4), - hash.clone(), + hash, 0, proposal_weight, proposal_len )); - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ - record(Event::CollectiveMajority(RawEvent::Proposed(1, 0, hash.clone(), 5))), - record(Event::CollectiveMajority(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::CollectiveMajority(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::CollectiveMajority(RawEvent::Voted(3, hash.clone(), true, 3, 0))), - record(Event::CollectiveMajority(RawEvent::Closed(hash.clone(), 5, 0))), - record(Event::CollectiveMajority(RawEvent::Approved(hash.clone()))), + record(Event::CollectiveMajority(RawEvent::Proposed(1, 0, hash, 5))), + record(Event::CollectiveMajority(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::CollectiveMajority(RawEvent::Voted(2, hash, true, 2, 0))), + record(Event::CollectiveMajority(RawEvent::Voted(3, hash, true, 3, 0))), + record(Event::CollectiveMajority(RawEvent::Closed(hash, 5, 0))), + record(Event::CollectiveMajority(RawEvent::Approved(hash))), record(Event::CollectiveMajority(RawEvent::Executed( - hash.clone(), + hash, Err(DispatchError::BadOrigin) ))) ] @@ -450,8 +422,8 @@ fn removal_of_old_voters_votes_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); assert_eq!( Collective::voting(&hash), Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) @@ -471,8 +443,8 @@ fn removal_of_old_voters_votes_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 1, true)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 1, true)); + assert_ok!(Collective::vote(Origin::signed(3), hash, 1, false)); assert_eq!( Collective::voting(&hash), Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) @@ -498,18 +470,13 @@ fn removal_of_old_voters_votes_works_with_set_members() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); assert_eq!( Collective::voting(&hash), Some(Votes { index: 0, threshold: 3, ayes: vec![1, 2], nays: vec![], end }) ); - assert_ok!(Collective::set_members( - Origin::root(), - vec![2, 3, 4], - None, - MaxMembers::get() - )); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 3, 4], None, MaxMembers::get())); assert_eq!( Collective::voting(&hash), Some(Votes { index: 0, threshold: 3, ayes: vec![2], nays: vec![], end }) @@ -524,18 +491,13 @@ fn removal_of_old_voters_votes_works_with_set_members() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 1, true)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 1, false)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 1, true)); + assert_ok!(Collective::vote(Origin::signed(3), hash, 1, false)); assert_eq!( Collective::voting(&hash), Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![3], end }) ); - assert_ok!(Collective::set_members( - Origin::root(), - vec![2, 4], - None, - MaxMembers::get() - )); + assert_ok!(Collective::set_members(Origin::root(), vec![2, 4], None, MaxMembers::get())); assert_eq!( Collective::voting(&hash), Some(Votes { index: 1, threshold: 2, ayes: vec![2], nays: vec![], end }) @@ -567,13 +529,7 @@ fn propose_works() { System::events(), vec![EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed( - 1, - 0, - hex!["68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35"] - .into(), - 3, - )), + event: Event::Collective(RawEvent::Proposed(1, 0, hash, 3)), topics: vec![], }] ); @@ -608,12 +564,7 @@ fn correct_validate_and_get_proposal() { let proposal = Call::Collective(crate::Call::set_members(vec![1, 2, 3], None, MaxMembers::get())); let length = proposal.encode().len() as u32; - assert_ok!(Collective::propose( - Origin::signed(1), - 3, - Box::new(proposal.clone()), - length - )); + assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), length)); let hash = BlakeTwo256::hash_of(&proposal); let weight = proposal.get_dispatch_info().weight; @@ -647,12 +598,7 @@ fn motions_ignoring_non_collective_proposals_works() { let proposal = make_proposal(42); let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); assert_noop!( - Collective::propose( - Origin::signed(42), - 3, - Box::new(proposal.clone()), - proposal_len - ), + Collective::propose(Origin::signed(42), 3, Box::new(proposal.clone()), proposal_len), Error::::NotMember ); }); @@ -671,7 +617,7 @@ fn motions_ignoring_non_collective_votes_works() { proposal_len )); assert_noop!( - Collective::vote(Origin::signed(42), hash.clone(), 0, true), + Collective::vote(Origin::signed(42), hash, 0, true), Error::::NotMember, ); }); @@ -691,7 +637,7 @@ fn motions_ignoring_bad_index_collective_vote_works() { proposal_len )); assert_noop!( - Collective::vote(Origin::signed(2), hash.clone(), 1, true), + Collective::vote(Origin::signed(2), hash, 1, true), Error::::WrongIndex, ); }); @@ -716,25 +662,25 @@ fn motions_vote_after_works() { Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![], end }) ); // Cast first aye vote. - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); assert_eq!( Collective::voting(&hash), Some(Votes { index: 0, threshold: 2, ayes: vec![1], nays: vec![], end }) ); // Try to cast a duplicate aye vote. assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, true), + Collective::vote(Origin::signed(1), hash, 0, true), Error::::DuplicateVote, ); // Cast a nay vote. - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, false)); assert_eq!( Collective::voting(&hash), Some(Votes { index: 0, threshold: 2, ayes: vec![], nays: vec![1], end }) ); // Try to cast a duplicate nay vote. assert_noop!( - Collective::vote(Origin::signed(1), hash.clone(), 0, false), + Collective::vote(Origin::signed(1), hash, 0, false), Error::::DuplicateVote, ); @@ -781,50 +727,40 @@ fn motions_all_first_vote_free_works() { // For the motion, acc 2's first vote, expecting Ok with Pays::No. let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(2), hash.clone(), 0, true); + Collective::vote(Origin::signed(2), hash, 0, true); assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); // Duplicate vote, expecting error with Pays::Yes. let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(2), hash.clone(), 0, true); + Collective::vote(Origin::signed(2), hash, 0, true); assert_eq!(vote_rval.unwrap_err().post_info.pays_fee, Pays::Yes); // Modifying vote, expecting ok with Pays::Yes. let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(2), hash.clone(), 0, false); + Collective::vote(Origin::signed(2), hash, 0, false); assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); // For the motion, acc 3's first vote, expecting Ok with Pays::No. let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(3), hash.clone(), 0, true); + Collective::vote(Origin::signed(3), hash, 0, true); assert_eq!(vote_rval.unwrap().pays_fee, Pays::No); // acc 3 modify the vote, expecting Ok with Pays::Yes. let vote_rval: DispatchResultWithPostInfo = - Collective::vote(Origin::signed(3), hash.clone(), 0, false); + Collective::vote(Origin::signed(3), hash, 0, false); assert_eq!(vote_rval.unwrap().pays_fee, Pays::Yes); // Test close() Extrincis | Check DispatchResultWithPostInfo with Pay Info let proposal_weight = proposal.get_dispatch_info().weight; - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); + let close_rval: DispatchResultWithPostInfo = + Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len); assert_eq!(close_rval.unwrap().pays_fee, Pays::No); // trying to close the proposal, which is already closed. // Expecting error "ProposalMissing" with Pays::Yes - let close_rval: DispatchResultWithPostInfo = Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len, - ); + let close_rval: DispatchResultWithPostInfo = + Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len); assert_eq!(close_rval.unwrap_err().post_info.pays_fee, Pays::Yes); }); } @@ -842,14 +778,8 @@ fn motions_reproposing_disapproved_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, false)); + assert_ok!(Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len)); assert_eq!(*Collective::proposals(), vec![]); assert_ok!(Collective::propose( Origin::signed(1), @@ -877,16 +807,10 @@ fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(3), hash.clone(), 0, true)); - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(3), hash, 0, true)); + assert_ok!(Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len)); assert_eq!( System::events(), vec![ @@ -922,7 +846,9 @@ fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { }, EventRecord { phase: Phase::Initialization, - event: Event::Democracy(mock_democracy::pallet::Event::::ExternalProposed), + event: Event::Democracy( + mock_democracy::pallet::Event::::ExternalProposed + ), topics: vec![], }, EventRecord { @@ -948,80 +874,36 @@ fn motions_disapproval_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, false)); + assert_ok!(Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len)); assert_eq!( System::events(), vec![ EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed( - 1, - 0, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 3, - )), + event: Event::Collective(RawEvent::Proposed(1, 0, hash, 3)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 1, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - true, - 1, - 0, - )), + event: Event::Collective(RawEvent::Voted(1, hash, true, 1, 0)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 2, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - false, - 1, - 1, - )), + event: Event::Collective(RawEvent::Voted(2, hash, false, 1, 1)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Closed( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 1, - 1, - )), + event: Event::Collective(RawEvent::Closed(hash, 1, 1)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Disapproved( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - )), + event: Event::Collective(RawEvent::Disapproved(hash)), topics: vec![], } ] @@ -1042,90 +924,43 @@ fn motions_approval_works() { Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); + assert_ok!(Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len)); assert_eq!( System::events(), vec![ EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed( - 1, - 0, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 2, - )), + event: Event::Collective(RawEvent::Proposed(1, 0, hash, 2)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 1, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - true, - 1, - 0, - )), + event: Event::Collective(RawEvent::Voted(1, hash, true, 1, 0)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted( - 2, - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - true, - 2, - 0, - )), + event: Event::Collective(RawEvent::Voted(2, hash, true, 2, 0)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Closed( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - 2, - 0, - )), + event: Event::Collective(RawEvent::Closed(hash, 2, 0)), topics: vec![], }, EventRecord { phase: Phase::Initialization, - event: Event::Collective(RawEvent::Approved( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - )), + event: Event::Collective(RawEvent::Approved(hash)), topics: vec![], }, EventRecord { phase: Phase::Initialization, event: Event::Collective(RawEvent::Executed( - hex![ - "68eea8f20b542ec656c6ac2d10435ae3bd1729efc34d1354ab85af840aad2d35" - ] - .into(), - Err(DispatchError::BadOrigin), + hash, + Err(DispatchError::BadOrigin) )), topics: vec![], } @@ -1137,8 +972,7 @@ fn motions_approval_works() { #[test] fn motion_with_no_votes_closes_with_disapproval() { new_test_ext().execute_with(|| { - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; let proposal = make_proposal(42); let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); let proposal_weight = proposal.get_dispatch_info().weight; @@ -1151,19 +985,13 @@ fn motion_with_no_votes_closes_with_disapproval() { )); assert_eq!( System::events()[0], - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 3))) + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))) ); // Closing the motion too early is not possible because it has neither // an approving or disapproving simple majority due to the lack of votes. assert_noop!( - Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - ), + Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len), Error::::TooEarly ); @@ -1171,23 +999,11 @@ fn motion_with_no_votes_closes_with_disapproval() { let closing_block = System::block_number() + MotionDuration::get(); System::set_block_number(closing_block); // we can successfully close the motion. - assert_ok!(Collective::close( - Origin::signed(2), - hash.clone(), - 0, - proposal_weight, - proposal_len - )); + assert_ok!(Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len)); // Events show that the close ended in a disapproval. - assert_eq!( - System::events()[1], - record(Event::Collective(RawEvent::Closed(hash.clone(), 0, 3))) - ); - assert_eq!( - System::events()[2], - record(Event::Collective(RawEvent::Disapproved(hash.clone()))) - ); + assert_eq!(System::events()[1], record(Event::Collective(RawEvent::Closed(hash, 0, 3)))); + assert_eq!(System::events()[2], record(Event::Collective(RawEvent::Disapproved(hash)))); }) } @@ -1207,22 +1023,22 @@ fn close_disapprove_does_not_care_about_weight_or_len() { proposal_len )); // First we make the proposal succeed - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); // It will not close with bad weight/len information assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0), + Collective::close(Origin::signed(2), hash, 0, 0, 0), Error::::WrongProposalLength, ); assert_noop!( - Collective::close(Origin::signed(2), hash.clone(), 0, 0, proposal_len), + Collective::close(Origin::signed(2), hash, 0, 0, proposal_len), Error::::WrongProposalWeight, ); // Now we make the proposal fail - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, false)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, false)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, false)); // It can close even if the weight/len information is bad - assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, 0, 0)); + assert_ok!(Collective::close(Origin::signed(2), hash, 0, 0, 0)); }) } @@ -1239,19 +1055,18 @@ fn disapprove_proposal_works() { proposal_len )); // Proposal would normally succeed - assert_ok!(Collective::vote(Origin::signed(1), hash.clone(), 0, true)); - assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, true)); + assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); // But Root can disapprove and remove it anyway - assert_ok!(Collective::disapprove_proposal(Origin::root(), hash.clone())); - let record = - |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; + assert_ok!(Collective::disapprove_proposal(Origin::root(), hash)); + let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ - record(Event::Collective(RawEvent::Proposed(1, 0, hash.clone(), 2))), - record(Event::Collective(RawEvent::Voted(1, hash.clone(), true, 1, 0))), - record(Event::Collective(RawEvent::Voted(2, hash.clone(), true, 2, 0))), - record(Event::Collective(RawEvent::Disapproved(hash.clone()))), + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(RawEvent::Disapproved(hash))), ] ); }) @@ -1260,10 +1075,7 @@ fn disapprove_proposal_works() { #[test] #[should_panic(expected = "Members cannot contain duplicate accounts.")] fn genesis_build_panics_with_duplicate_members() { - collective::GenesisConfig:: { - members: vec![1, 2, 3, 1], - phantom: Default::default(), - } - .build_storage() - .unwrap(); + collective::GenesisConfig:: { members: vec![1, 2, 3, 1], phantom: Default::default() } + .build_storage() + .unwrap(); } From 2c9da01bb6eb8fd2151e3b45a62d99ab2fe0ab49 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 10 Sep 2021 15:39:56 +0800 Subject: [PATCH 05/10] Remove unused hex_literal::hex in collective tests --- Cargo.lock | 1 - frame/collective/Cargo.toml | 3 --- frame/collective/src/tests.rs | 1 - 3 files changed, 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 61863994c60c4..d5594bb9fde6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5003,7 +5003,6 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", - "hex-literal", "log 0.4.14", "parity-scale-codec", "sp-core", diff --git a/frame/collective/Cargo.toml b/frame/collective/Cargo.toml index 4b1051e793044..5c543e4e783ad 100644 --- a/frame/collective/Cargo.toml +++ b/frame/collective/Cargo.toml @@ -25,9 +25,6 @@ frame-support = { version = "4.0.0-dev", default-features = false, path = "../su frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } log = { version = "0.4.14", default-features = false } -[dev-dependencies] -hex-literal = "0.3.1" - [features] default = ["std"] std = [ diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs index c10d8d0b19330..cc873c420eb2e 100644 --- a/frame/collective/src/tests.rs +++ b/frame/collective/src/tests.rs @@ -19,7 +19,6 @@ use super::*; use crate as collective; use frame_support::{assert_noop, assert_ok, parameter_types, Hashable}; use frame_system::{self as system, EventRecord, Phase}; -use hex_literal::hex; use sp_core::{ u32_trait::{_3, _4}, H256, From 34b2eb34248a3d5fd64ab125a2b4e1e0c17c6cf8 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 10 Sep 2021 15:56:30 +0800 Subject: [PATCH 06/10] Extract the helper function record() --- frame/collective/src/tests.rs | 153 ++++++---------------------------- 1 file changed, 27 insertions(+), 126 deletions(-) diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs index cc873c420eb2e..857f7a23633a5 100644 --- a/frame/collective/src/tests.rs +++ b/frame/collective/src/tests.rs @@ -173,6 +173,10 @@ fn make_proposal(value: u64) -> Call { Call::System(frame_system::Call::remark(value.encode())) } +fn record(event: Event) -> EventRecord { + EventRecord { phase: Phase::Initialization, event, topics: vec![] } +} + #[test] fn motions_basic_environment_works() { new_test_ext().execute_with(|| { @@ -207,7 +211,6 @@ fn close_works() { System::set_block_number(4); assert_ok!(Collective::close(Origin::signed(4), hash, 0, proposal_weight, proposal_len)); - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ @@ -301,7 +304,6 @@ fn close_with_prime_works() { System::set_block_number(4); assert_ok!(Collective::close(Origin::signed(4), hash, 0, proposal_weight, proposal_len)); - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ @@ -341,7 +343,6 @@ fn close_with_voting_prime_works() { System::set_block_number(4); assert_ok!(Collective::close(Origin::signed(4), hash, 0, proposal_weight, proposal_len)); - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ @@ -389,7 +390,6 @@ fn close_with_no_prime_but_majority_works() { proposal_len )); - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ @@ -526,11 +526,7 @@ fn propose_works() { assert_eq!( System::events(), - vec![EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed(1, 0, hash, 3)), - topics: vec![], - }] + vec![record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3)))] ); }); } @@ -686,21 +682,9 @@ fn motions_vote_after_works() { assert_eq!( System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed(1, 0, hash, 2)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(1, hash, true, 1, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(1, hash, false, 0, 1)), - topics: vec![], - } + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(1, hash, false, 0, 1))), ] ); }); @@ -813,48 +797,14 @@ fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { assert_eq!( System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed(1, 0, hash, 2)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(1, hash, true, 1, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(2, hash, true, 2, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(3, hash, true, 3, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Closed(hash, 3, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Approved(hash)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Democracy( - mock_democracy::pallet::Event::::ExternalProposed - ), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Executed(hash, Ok(()))), - topics: vec![], - } + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(RawEvent::Voted(3, hash, true, 3, 0))), + record(Event::Collective(RawEvent::Closed(hash, 3, 0))), + record(Event::Collective(RawEvent::Approved(hash))), + record(Event::Democracy(mock_democracy::pallet::Event::::ExternalProposed)), + record(Event::Collective(RawEvent::Executed(hash, Ok(())))), ] ); }); @@ -880,31 +830,11 @@ fn motions_disapproval_works() { assert_eq!( System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed(1, 0, hash, 3)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(1, hash, true, 1, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(2, hash, false, 1, 1)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Closed(hash, 1, 1)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Disapproved(hash)), - topics: vec![], - } + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 3))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash, false, 1, 1))), + record(Event::Collective(RawEvent::Closed(hash, 1, 1))), + record(Event::Collective(RawEvent::Disapproved(hash))), ] ); }); @@ -930,39 +860,12 @@ fn motions_approval_works() { assert_eq!( System::events(), vec![ - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Proposed(1, 0, hash, 2)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(1, hash, true, 1, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Voted(2, hash, true, 2, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Closed(hash, 2, 0)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Approved(hash)), - topics: vec![], - }, - EventRecord { - phase: Phase::Initialization, - event: Event::Collective(RawEvent::Executed( - hash, - Err(DispatchError::BadOrigin) - )), - topics: vec![], - } + record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(RawEvent::Closed(hash, 2, 0))), + record(Event::Collective(RawEvent::Approved(hash))), + record(Event::Collective(RawEvent::Executed(hash, Err(DispatchError::BadOrigin)))), ] ); }); @@ -971,7 +874,6 @@ fn motions_approval_works() { #[test] fn motion_with_no_votes_closes_with_disapproval() { new_test_ext().execute_with(|| { - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; let proposal = make_proposal(42); let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32); let proposal_weight = proposal.get_dispatch_info().weight; @@ -1058,7 +960,6 @@ fn disapprove_proposal_works() { assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); // But Root can disapprove and remove it anyway assert_ok!(Collective::disapprove_proposal(Origin::root(), hash)); - let record = |event| EventRecord { phase: Phase::Initialization, event, topics: vec![] }; assert_eq!( System::events(), vec![ From 4def2a0d33f23cb6a53791baed09269c5e3e0cb5 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 10 Sep 2021 16:25:56 +0800 Subject: [PATCH 07/10] Try fixing ci --- primitives/state-machine/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 032899faeb523..ec426d8e1022a 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1579,7 +1579,6 @@ mod tests { let mut seed = [0; 16]; for i in 0..50u32 { let mut child_infos = Vec::new(); - &seed[0..4].copy_from_slice(&i.to_be_bytes()[..]); let mut rand = SmallRng::from_seed(seed); let nb_child_trie = rand.next_u32() as usize % 25; From e81e06ac9e2cee86a894180e188e696e08fdbc98 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 10 Sep 2021 23:16:32 +0800 Subject: [PATCH 08/10] Add a test case with only two votes --- frame/collective/src/tests.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs index 857f7a23633a5..e82ddc3d7bf5c 100644 --- a/frame/collective/src/tests.rs +++ b/frame/collective/src/tests.rs @@ -784,6 +784,8 @@ fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { // The voting threshold is 2, but the required votes for `ExternalMajorityOrigin` is 3. // The proposal will be executed regardless of the voting threshold // as long as we have enough yes votes. + // + // Failed to execute with only 2 yes votes. assert_ok!(Collective::propose( Origin::signed(1), 2, @@ -792,7 +794,6 @@ fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { )); assert_ok!(Collective::vote(Origin::signed(1), hash, 0, true)); assert_ok!(Collective::vote(Origin::signed(2), hash, 0, true)); - assert_ok!(Collective::vote(Origin::signed(3), hash, 0, true)); assert_ok!(Collective::close(Origin::signed(2), hash, 0, proposal_weight, proposal_len)); assert_eq!( System::events(), @@ -800,6 +801,32 @@ fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { record(Event::Collective(RawEvent::Proposed(1, 0, hash, 2))), record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), + record(Event::Collective(RawEvent::Closed(hash, 2, 0))), + record(Event::Collective(RawEvent::Approved(hash))), + record(Event::Collective(RawEvent::Executed(hash, Err(DispatchError::BadOrigin)))), + ] + ); + + System::reset_events(); + + // Execute with 3 yes votes. + assert_ok!(Collective::propose( + Origin::signed(1), + 2, + Box::new(proposal.clone()), + proposal_len + )); + + assert_ok!(Collective::vote(Origin::signed(1), hash, 1, true)); + assert_ok!(Collective::vote(Origin::signed(2), hash, 1, true)); + assert_ok!(Collective::vote(Origin::signed(3), hash, 1, true)); + assert_ok!(Collective::close(Origin::signed(2), hash, 1, proposal_weight, proposal_len)); + assert_eq!( + System::events(), + vec![ + record(Event::Collective(RawEvent::Proposed(1, 1, hash, 2))), + record(Event::Collective(RawEvent::Voted(1, hash, true, 1, 0))), + record(Event::Collective(RawEvent::Voted(2, hash, true, 2, 0))), record(Event::Collective(RawEvent::Voted(3, hash, true, 3, 0))), record(Event::Collective(RawEvent::Closed(hash, 3, 0))), record(Event::Collective(RawEvent::Approved(hash))), From 9a65763f94770d8be2a826b9e2d488a7a65adbf4 Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Fri, 10 Sep 2021 23:26:52 +0800 Subject: [PATCH 09/10] Nit --- frame/collective/src/tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs index e82ddc3d7bf5c..7cce2f363189d 100644 --- a/frame/collective/src/tests.rs +++ b/frame/collective/src/tests.rs @@ -809,14 +809,13 @@ fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { System::reset_events(); - // Execute with 3 yes votes. + // Executed with 3 yes votes. assert_ok!(Collective::propose( Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len )); - assert_ok!(Collective::vote(Origin::signed(1), hash, 1, true)); assert_ok!(Collective::vote(Origin::signed(2), hash, 1, true)); assert_ok!(Collective::vote(Origin::signed(3), hash, 1, true)); From 46862cf7b93c4f56b61cebb1a9008c5b0e900f6d Mon Sep 17 00:00:00 2001 From: Liu-Cheng Xu Date: Mon, 13 Sep 2021 08:39:41 +0800 Subject: [PATCH 10/10] Fix typo --- frame/collective/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/collective/src/tests.rs b/frame/collective/src/tests.rs index 7cce2f363189d..aa6ea090f4eea 100644 --- a/frame/collective/src/tests.rs +++ b/frame/collective/src/tests.rs @@ -775,7 +775,7 @@ fn motions_reproposing_disapproved_works() { } #[test] -fn motions_approval_with_enought_votes_and_lower_voting_threshold_works() { +fn motions_approval_with_enough_votes_and_lower_voting_threshold_works() { new_test_ext().execute_with(|| { let proposal = Call::Democracy(mock_democracy::Call::external_propose_majority()); let proposal_len: u32 = proposal.using_encoded(|p| p.len() as u32);