diff --git a/Cargo.lock b/Cargo.lock index 6a5b562d0a791..87cc3701f711d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6327,6 +6327,8 @@ dependencies = [ "frame-support", "frame-system", "pallet-balances", + "pallet-collective", + "pallet-timestamp", "parity-scale-codec", "scale-info", "sp-core", diff --git a/frame/utility/Cargo.toml b/frame/utility/Cargo.toml index cbe2892d72dc7..ac4f52c6bb9f3 100644 --- a/frame/utility/Cargo.toml +++ b/frame/utility/Cargo.toml @@ -25,6 +25,8 @@ sp-std = { version = "4.0.0", default-features = false, path = "../../primitives [dev-dependencies] pallet-balances = { version = "4.0.0-dev", path = "../balances" } +pallet-collective = { version = "4.0.0-dev", path = "../collective" } +pallet-timestamp = { version = "4.0.0-dev", path = "../timestamp" } sp-core = { version = "6.0.0", path = "../../primitives/core" } [features] diff --git a/frame/utility/src/lib.rs b/frame/utility/src/lib.rs index 544add1da68d3..41710be930b90 100644 --- a/frame/utility/src/lib.rs +++ b/frame/utility/src/lib.rs @@ -164,13 +164,13 @@ pub mod pallet { impl Pallet { /// Send a batch of dispatch calls. /// - /// May be called from any origin. + /// May be called from any origin except `None`. /// /// - `calls`: The calls to be dispatched from the same origin. The number of call must not /// exceed the constant: `batched_calls_limit` (available in constant metadata). /// - /// If origin is root then call are dispatch without checking origin filter. (This includes - /// bypassing `frame_system::Config::BaseCallFilter`). + /// If origin is root then the calls are dispatched without checking origin filter. (This + /// includes bypassing `frame_system::Config::BaseCallFilter`). /// /// # /// - Complexity: O(C) where C is the number of calls to be batched. @@ -291,13 +291,13 @@ pub mod pallet { /// Send a batch of dispatch calls and atomically execute them. /// The whole transaction will rollback and fail if any of the calls failed. /// - /// May be called from any origin. + /// May be called from any origin except `None`. /// /// - `calls`: The calls to be dispatched from the same origin. The number of call must not /// exceed the constant: `batched_calls_limit` (available in constant metadata). /// - /// If origin is root then call are dispatch without checking origin filter. (This includes - /// bypassing `frame_system::Config::BaseCallFilter`). + /// If origin is root then the calls are dispatched without checking origin filter. (This + /// includes bypassing `frame_system::Config::BaseCallFilter`). /// /// # /// - Complexity: O(C) where C is the number of calls to be batched. @@ -403,13 +403,13 @@ pub mod pallet { /// Send a batch of dispatch calls. /// Unlike `batch`, it allows errors and won't interrupt. /// - /// May be called from any origin. + /// May be called from any origin except `None`. /// /// - `calls`: The calls to be dispatched from the same origin. The number of call must not /// exceed the constant: `batched_calls_limit` (available in constant metadata). /// - /// If origin is root then call are dispatch without checking origin filter. (This includes - /// bypassing `frame_system::Config::BaseCallFilter`). + /// If origin is root then the calls are dispatch without checking origin filter. (This + /// includes bypassing `frame_system::Config::BaseCallFilter`). /// /// # /// - Complexity: O(C) where C is the number of calls to be batched. diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 81cec1c295c30..c374f5ae21099 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -27,15 +27,18 @@ use frame_support::{ dispatch::{DispatchError, DispatchErrorWithPostInfo, Dispatchable, Pays}, error::BadOrigin, parameter_types, storage, - traits::{ConstU32, ConstU64, Contains}, + traits::{ConstU32, ConstU64, Contains, GenesisBuild}, weights::Weight, }; +use pallet_collective::{EnsureProportionAtLeast, Instance1}; use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, IdentityLookup}, + traits::{BlakeTwo256, Hash, IdentityLookup}, }; +type BlockNumber = u64; + // example module to test behaviors. #[frame_support::pallet] pub mod example { @@ -82,6 +85,42 @@ pub mod example { } } +mod mock_democracy { + pub use pallet::*; + #[frame_support::pallet] + pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config + Sized { + type RuntimeEvent: From> + + IsType<::RuntimeEvent>; + 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, + } + } +} + type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -92,9 +131,12 @@ frame_support::construct_runtime!( UncheckedExtrinsic = UncheckedExtrinsic, { System: frame_system::{Pallet, Call, Config, Storage, Event}, + Timestamp: pallet_timestamp::{Call, Inherent}, Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, + Council: pallet_collective::, Utility: utility::{Pallet, Call, Event}, Example: example::{Pallet, Call}, + Democracy: mock_democracy::{Pallet, Call, Event}, } ); @@ -140,10 +182,34 @@ impl pallet_balances::Config for Test { type AccountStore = System; type WeightInfo = (); } + +impl pallet_timestamp::Config for Test { + type Moment = u64; + type OnTimestampSet = (); + type MinimumPeriod = ConstU64<3>; + type WeightInfo = (); +} + +const MOTION_DURATION_IN_BLOCKS: BlockNumber = 3; parameter_types! { pub const MultisigDepositBase: u64 = 1; pub const MultisigDepositFactor: u64 = 1; pub const MaxSignatories: u32 = 3; + pub const MotionDuration: BlockNumber = MOTION_DURATION_IN_BLOCKS; + pub const MaxProposals: u32 = 100; + pub const MaxMembers: u32 = 100; +} + +type CouncilCollective = pallet_collective::Instance1; +impl pallet_collective::Config for Test { + type RuntimeOrigin = RuntimeOrigin; + type Proposal = RuntimeCall; + type RuntimeEvent = RuntimeEvent; + type MotionDuration = MotionDuration; + type MaxProposals = MaxProposals; + type MaxMembers = MaxMembers; + type DefaultVote = pallet_collective::PrimeDefaultVote; + type WeightInfo = (); } impl example::Config for Test {} @@ -159,10 +225,16 @@ impl Contains for TestBaseCallFilter { RuntimeCall::System(frame_system::Call::remark { .. }) => true, // For tests RuntimeCall::Example(_) => true, + // For council origin tests. + RuntimeCall::Democracy(_) => true, _ => false, } } } +impl mock_democracy::Config for Test { + type RuntimeEvent = RuntimeEvent; + type ExternalMajorityOrigin = EnsureProportionAtLeast; +} impl Config for Test { type RuntimeEvent = RuntimeEvent; type RuntimeCall = RuntimeCall; @@ -175,6 +247,7 @@ type UtilityCall = crate::Call; use frame_system::Call as SystemCall; use pallet_balances::{Call as BalancesCall, Error as BalancesError}; +use pallet_timestamp::Call as TimestampCall; pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); @@ -183,6 +256,14 @@ pub fn new_test_ext() -> sp_io::TestExternalities { } .assimilate_storage(&mut t) .unwrap(); + + pallet_collective::GenesisConfig:: { + members: vec![1, 2, 3], + phantom: Default::default(), + } + .assimilate_storage(&mut t) + .unwrap(); + let mut ext = sp_io::TestExternalities::new(t); ext.execute_with(|| System::set_block_number(1)); ext @@ -679,3 +760,139 @@ fn none_origin_does_not_work() { assert_noop!(Utility::batch_all(RuntimeOrigin::none(), vec![]), BadOrigin); }) } + +#[test] +fn batch_doesnt_work_with_inherents() { + new_test_ext().execute_with(|| { + // fails because inherents expect the origin to be none. + assert_ok!(Utility::batch( + RuntimeOrigin::signed(1), + vec![RuntimeCall::Timestamp(TimestampCall::set { now: 42 }),] + )); + System::assert_last_event( + utility::Event::BatchInterrupted { + index: 0, + error: frame_system::Error::::CallFiltered.into(), + } + .into(), + ); + }) +} + +#[test] +fn force_batch_doesnt_work_with_inherents() { + new_test_ext().execute_with(|| { + // fails because inherents expect the origin to be none. + assert_ok!(Utility::force_batch( + RuntimeOrigin::root(), + vec![RuntimeCall::Timestamp(TimestampCall::set { now: 42 }),] + )); + System::assert_last_event(utility::Event::BatchCompletedWithErrors.into()); + }) +} + +#[test] +fn batch_all_doesnt_work_with_inherents() { + new_test_ext().execute_with(|| { + let batch_all = RuntimeCall::Utility(UtilityCall::batch_all { + calls: vec![RuntimeCall::Timestamp(TimestampCall::set { now: 42 })], + }); + let info = batch_all.get_dispatch_info(); + + // fails because inherents expect the origin to be none. + assert_noop!( + batch_all.dispatch(RuntimeOrigin::signed(1)), + DispatchErrorWithPostInfo { + post_info: PostDispatchInfo { + actual_weight: Some(info.weight), + pays_fee: Pays::Yes + }, + error: frame_system::Error::::CallFiltered.into(), + } + ); + }) +} + +#[test] +fn batch_works_with_council_origin() { + new_test_ext().execute_with(|| { + let proposal = RuntimeCall::Utility(UtilityCall::batch { + calls: vec![RuntimeCall::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 = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Council::propose( + RuntimeOrigin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + + assert_ok!(Council::vote(RuntimeOrigin::signed(1), hash, 0, true)); + assert_ok!(Council::vote(RuntimeOrigin::signed(2), hash, 0, true)); + assert_ok!(Council::vote(RuntimeOrigin::signed(3), hash, 0, true)); + + System::set_block_number(4); + assert_ok!(Council::close( + RuntimeOrigin::signed(4), + hash, + 0, + proposal_weight, + proposal_len + )); + + System::assert_last_event(RuntimeEvent::Council(pallet_collective::Event::Executed { + proposal_hash: hash, + result: Ok(()), + })); + }) +} + +#[test] +fn force_batch_works_with_council_origin() { + new_test_ext().execute_with(|| { + let proposal = RuntimeCall::Utility(UtilityCall::force_batch { + calls: vec![RuntimeCall::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 = BlakeTwo256::hash_of(&proposal); + + assert_ok!(Council::propose( + RuntimeOrigin::signed(1), + 3, + Box::new(proposal.clone()), + proposal_len + )); + + assert_ok!(Council::vote(RuntimeOrigin::signed(1), hash, 0, true)); + assert_ok!(Council::vote(RuntimeOrigin::signed(2), hash, 0, true)); + assert_ok!(Council::vote(RuntimeOrigin::signed(3), hash, 0, true)); + + System::set_block_number(4); + assert_ok!(Council::close( + RuntimeOrigin::signed(4), + hash, + 0, + proposal_weight, + proposal_len + )); + + System::assert_last_event(RuntimeEvent::Council(pallet_collective::Event::Executed { + proposal_hash: hash, + result: Ok(()), + })); + }) +} + +#[test] +fn batch_all_works_with_council_origin() { + new_test_ext().execute_with(|| { + assert_ok!(Utility::batch_all( + RuntimeOrigin::from(pallet_collective::RawOrigin::Members(3, 3)), + vec![RuntimeCall::Democracy(mock_democracy::Call::external_propose_majority {})] + )); + }) +}