Unverified Commit d3a9b5be authored by Bastian Köcher's avatar Bastian Köcher Committed by GitHub
Browse files

Introduce `OriginPrivilegeCmp` (#4166)

* Introduce `OriginPrivilegeCmp`

Make use of the new `OriginPrivilegeCmp` feature of pallet scheduler.
The idea is to make sure that a council origin with more yes votes has
higher privileges than a council origin with less yes votes. This solves
a problem that happened recently on Kusama where the council tried to
cancel a scheduled task, but that required that the same council origin
was used while the cancel motion had more yes votes than the origin
motion that scheduled this task. With this origin privilege compare it
should now be solved by checking the yes votes directly.

* Feedback

* update lockfile for substrate

Co-authored-by: parity-processbot <>
parent e9f329f7
Pipeline #163995 passed with stages
in 41 minutes and 58 seconds
This diff is collapsed.
......@@ -34,7 +34,7 @@ use runtime_common::{
OffchainSolutionWeightLimit, RocksDbWeight, SlowAdjustingFeeUpdate, ToAuthor,
};
use sp_core::u32_trait::{_1, _2, _3, _5};
use sp_std::{collections::btree_map::BTreeMap, prelude::*};
use sp_std::{cmp::Ordering, collections::btree_map::BTreeMap, prelude::*};
use runtime_parachains::{
configuration as parachains_configuration, dmp as parachains_dmp, hrmp as parachains_hrmp,
......@@ -49,7 +49,10 @@ use authority_discovery_primitives::AuthorityId as AuthorityDiscoveryId;
use beefy_primitives::crypto::AuthorityId as BeefyId;
use frame_support::{
construct_runtime, match_type, parameter_types,
traits::{Contains, Everything, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing},
traits::{
Contains, Everything, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing,
PrivilegeCmp,
},
weights::Weight,
PalletId, RuntimeDebug,
};
......@@ -195,6 +198,29 @@ type ScheduleOrigin = EnsureOneOf<
pallet_collective::EnsureProportionAtLeast<_1, _2, AccountId, CouncilCollective>,
>;
/// Used the compare the privilege of an origin inside the scheduler.
pub struct OriginPrivilegeCmp;
impl PrivilegeCmp<OriginCaller> for OriginPrivilegeCmp {
fn cmp_privilege(left: &OriginCaller, right: &OriginCaller) -> Option<Ordering> {
if left == right {
return Some(Ordering::Equal)
}
match (left, right) {
// Root is greater than anything.
(OriginCaller::system(frame_system::RawOrigin::Root), _) => Some(Ordering::Greater),
// Check which one has more yes votes.
(
OriginCaller::Council(pallet_collective::RawOrigin::Members(l_yes_votes, l_count)),
OriginCaller::Council(pallet_collective::RawOrigin::Members(r_yes_votes, r_count)),
) => Some((l_yes_votes * r_count).cmp(&(r_yes_votes * l_count))),
// For every other origin we don't care, as they are not used for `ScheduleOrigin`.
_ => None,
}
}
}
impl pallet_scheduler::Config for Runtime {
type Event = Event;
type Origin = Origin;
......@@ -204,6 +230,7 @@ impl pallet_scheduler::Config for Runtime {
type ScheduleOrigin = ScheduleOrigin;
type MaxScheduledPerBlock = MaxScheduledPerBlock;
type WeightInfo = weights::pallet_scheduler::WeightInfo<Runtime>;
type OriginPrivilegeCmp = OriginPrivilegeCmp;
}
parameter_types! {
......
......@@ -40,7 +40,7 @@ use authority_discovery_primitives::AuthorityId as AuthorityDiscoveryId;
use beefy_primitives::crypto::AuthorityId as BeefyId;
use frame_support::{
construct_runtime, parameter_types,
traits::{Contains, KeyOwnerProofSystem, LockIdentifier, OnRuntimeUpgrade},
traits::{Contains, KeyOwnerProofSystem, LockIdentifier, OnRuntimeUpgrade, PrivilegeCmp},
weights::Weight,
PalletId, RuntimeDebug,
};
......@@ -73,7 +73,7 @@ use sp_runtime::{
ApplyExtrinsicResult, KeyTypeId, Perbill, Percent, Permill,
};
use sp_staking::SessionIndex;
use sp_std::{collections::btree_map::BTreeMap, prelude::*};
use sp_std::{cmp::Ordering, collections::btree_map::BTreeMap, prelude::*};
#[cfg(any(feature = "std", test))]
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;
......@@ -229,6 +229,29 @@ type ScheduleOrigin = EnsureOneOf<
pallet_collective::EnsureProportionAtLeast<_1, _2, AccountId, CouncilCollective>,
>;
/// Used the compare the privilege of an origin inside the scheduler.
pub struct OriginPrivilegeCmp;
impl PrivilegeCmp<OriginCaller> for OriginPrivilegeCmp {
fn cmp_privilege(left: &OriginCaller, right: &OriginCaller) -> Option<Ordering> {
if left == right {
return Some(Ordering::Equal)
}
match (left, right) {
// Root is greater than anything.
(OriginCaller::system(frame_system::RawOrigin::Root), _) => Some(Ordering::Greater),
// Check which one has more yes votes.
(
OriginCaller::Council(pallet_collective::RawOrigin::Members(l_yes_votes, l_count)),
OriginCaller::Council(pallet_collective::RawOrigin::Members(r_yes_votes, r_count)),
) => Some((l_yes_votes * r_count).cmp(&(r_yes_votes * l_count))),
// For every other origin we don't care, as they are not used for `ScheduleOrigin`.
_ => None,
}
}
}
impl pallet_scheduler::Config for Runtime {
type Event = Event;
type Origin = Origin;
......@@ -238,6 +261,7 @@ impl pallet_scheduler::Config for Runtime {
type ScheduleOrigin = ScheduleOrigin;
type MaxScheduledPerBlock = MaxScheduledPerBlock;
type WeightInfo = weights::pallet_scheduler::WeightInfo<Runtime>;
type OriginPrivilegeCmp = OriginPrivilegeCmp;
}
parameter_types! {
......
......@@ -192,6 +192,7 @@ impl pallet_scheduler::Config for Runtime {
type ScheduleOrigin = EnsureRoot<AccountId>;
type MaxScheduledPerBlock = MaxScheduledPerBlock;
type WeightInfo = weights::pallet_scheduler::WeightInfo<Runtime>;
type OriginPrivilegeCmp = frame_support::traits::EqualPrivilegeOnly;
}
parameter_types! {
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment