Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
aadfd4b
Introduce `cancel_proposal`
gavofyork Sep 15, 2020
a67a018
Support proposal cancellation from runtime.
gavofyork Sep 15, 2020
bec8bad
Fixes
gavofyork Sep 15, 2020
06c97f2
Fixes
gavofyork Sep 15, 2020
693fc82
Fixes
gavofyork Sep 15, 2020
bbe86b6
Fixes
gavofyork Sep 16, 2020
083ba8d
Fixes
gavofyork Sep 16, 2020
3626e8e
Fixes
gavofyork Sep 16, 2020
c93eedb
Fixes
gavofyork Sep 16, 2020
2274fa3
Merge remote-tracking branch 'origin/master' into gav-cancel-proposal
gavofyork Sep 17, 2020
400d8e4
Merge remote-tracking branch 'origin/master' into gav-cancel-proposal
gavofyork Sep 17, 2020
a8e9fe8
Fix benchmarks
shawntabrizi Sep 21, 2020
764ea77
fix benchmark
shawntabrizi Sep 21, 2020
a5dfded
whitelisted caller weights
shawntabrizi Sep 21, 2020
4aaa1e8
Merge branch 'master' into gav-cancel-proposal
shawntabrizi Sep 22, 2020
a36189f
fix build
shawntabrizi Sep 22, 2020
1254cf6
Merge remote-tracking branch 'origin/master' into gav-cancel-proposal
gavofyork Sep 23, 2020
f60cccc
Fixes
gavofyork Sep 23, 2020
13cace0
Fixes
gavofyork Sep 23, 2020
02e4fe8
Fixes
gavofyork Sep 23, 2020
88dc626
Fixes
gavofyork Sep 23, 2020
a1ebcb1
Merge remote-tracking branch 'origin/master' into gav-cancel-proposal
gavofyork Sep 23, 2020
775b637
Update frame/democracy/src/lib.rs
gavofyork Sep 24, 2020
9c0cf8d
Fixes
gavofyork Sep 24, 2020
2ba9cdd
Fixes
gavofyork Sep 24, 2020
847414f
Merge branch 'gav-cancel-proposal' of github.com:paritytech/substrate…
gavofyork Sep 24, 2020
4ecbddf
Fixes
gavofyork Sep 24, 2020
2f84c91
Fixes
gavofyork Sep 24, 2020
42b9093
Fixes
gavofyork Sep 24, 2020
97af811
doc updates
shawntabrizi Sep 24, 2020
b466e1f
new weights
shawntabrizi Sep 24, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,14 @@ impl pallet_democracy::Trait for Runtime {
type FastTrackVotingPeriod = FastTrackVotingPeriod;
// To cancel a proposal which has been passed, 2/3 of the council must agree to it.
type CancellationOrigin = pallet_collective::EnsureProportionAtLeast<_2, _3, AccountId, CouncilCollective>;
// To cancel a proposal before it has been passed, the technical committee must be unanimous or
// Root must agree.
type CancelProposalOrigin = EnsureOneOf<
AccountId,
EnsureRoot<AccountId>,
pallet_collective::EnsureProportionAtLeast<_1, _1, AccountId, TechnicalCollective>,
>;
type BlacklistOrigin = EnsureRoot<AccountId>;
// Any single technical committee member may veto a coming council proposal, however they can
// only do it once and it lasts only for the cooloff period.
type VetoOrigin = pallet_collective::EnsureMember<AccountId, TechnicalCollective>;
Expand Down
12 changes: 11 additions & 1 deletion bin/node/runtime/src/weights/pallet_democracy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight};

pub struct WeightInfo;
impl pallet_democracy::WeightInfo for WeightInfo {
fn propose() -> Weight {
fn propose(p: u32, ) -> Weight {
(49113000 as Weight)
.saturating_add((220000 as Weight).saturating_mul(p as Weight))
.saturating_add(DbWeight::get().reads(2 as Weight))
.saturating_add(DbWeight::get().writes(3 as Weight))
}
Expand All @@ -48,6 +49,11 @@ impl pallet_democracy::WeightInfo for WeightInfo {
.saturating_add(DbWeight::get().reads(2 as Weight))
.saturating_add(DbWeight::get().writes(2 as Weight))
}
fn blacklist() -> Weight {
(31071000 as Weight)
.saturating_add(DbWeight::get().reads(2 as Weight))
.saturating_add(DbWeight::get().writes(2 as Weight))
}
fn external_propose(v: u32, ) -> Weight {
(14282000 as Weight)
.saturating_add((109000 as Weight).saturating_mul(v as Weight))
Expand All @@ -73,6 +79,10 @@ impl pallet_democracy::WeightInfo for WeightInfo {
.saturating_add(DbWeight::get().reads(2 as Weight))
.saturating_add(DbWeight::get().writes(2 as Weight))
}
fn cancel_proposal() -> Weight {
(20431000 as Weight)
.saturating_add(DbWeight::get().writes(1 as Weight))
}
fn cancel_referendum() -> Weight {
(20431000 as Weight)
.saturating_add(DbWeight::get().writes(1 as Weight))
Expand Down
40 changes: 38 additions & 2 deletions frame/democracy/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ fn add_proposal<T: Trait>(n: u32) -> Result<T::Hash, &'static str> {
RawOrigin::Signed(other).into(),
proposal_hash,
value.into(),
MAX_PROPOSALS,
)?;

Ok(proposal_hash)
Expand Down Expand Up @@ -100,12 +101,17 @@ benchmarks! {
_ { }

propose {
let p in 1 .. MAX_PROPOSALS;
for i in 0 .. (p - 1) {
add_proposal::<T>(i)?;
}

let caller = funded_account::<T>("caller", 0);
let proposal_hash: T::Hash = T::Hashing::hash_of(&0);
let value = T::MinimumDeposit::get();
}: _(RawOrigin::Signed(caller), proposal_hash, value.into())
}: _(RawOrigin::Signed(caller), proposal_hash, value.into(), MAX_PROPOSALS)
verify {
assert_eq!(Democracy::<T>::public_props().len(), 1, "Proposals not created.");
assert_eq!(Democracy::<T>::public_props().len(), p as usize, "Proposals not created.");
}

second {
Expand Down Expand Up @@ -206,6 +212,32 @@ benchmarks! {
assert!(Democracy::<T>::referendum_status(referendum_index).is_err());
}

blacklist {
let p in 1 .. MAX_PROPOSALS;

// Place our proposal at the end to make sure it's worst case.
for i in 1 .. p {
add_proposal::<T>(i)?;
}
add_proposal::<T>(0)?;
// We should really add a lot of seconds here, but we're not doing it elsewhere.

// Place our proposal in the external queue, too.
let hash = T::Hashing::hash_of(&0);
assert!(Democracy::<T>::external_propose(T::ExternalOrigin::successful_origin(), hash.clone()).is_ok());

// Add a referendum of our proposal.
let referendum_index = add_referendum::<T>(0)?;
assert!(Democracy::<T>::referendum_status(referendum_index).is_ok());

let call = Call::<T>::blacklist(hash, Some(referendum_index));
let origin = T::BlacklistOrigin::successful_origin();
}: { call.dispatch_bypass_filter(origin)? }
verify {
// Referendum has been canceled
assert!(Democracy::<T>::referendum_status(referendum_index).is_err());
}

// Worst case scenario, we external propose a previously blacklisted proposal
external_propose {
let v in 1 .. MAX_VETOERS as u32;
Expand Down Expand Up @@ -287,6 +319,10 @@ benchmarks! {
assert_eq!(new_vetoers.len(), (v + 1) as usize, "vetoers not added");
}

cancel_proposal {
let proposal_index = add_proposal::<T>(0)?;
}: _(RawOrigin::Root, 0)

cancel_referendum {
let referendum_index = add_referendum::<T>(0)?;
}: _(RawOrigin::Root, referendum_index)
Expand Down
12 changes: 11 additions & 1 deletion frame/democracy/src/default_weight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ use frame_support::weights::{Weight, constants::RocksDbWeight as DbWeight};
/// Default implementation of weight, this is just from an example return, values may change
/// depending on the runtime. This is not meant to be used in production.
impl crate::WeightInfo for () {
fn propose() -> Weight {
fn propose(p: u32, ) -> Weight {
(49113000 as Weight)
.saturating_add((262000 as Weight).saturating_mul(p as Weight))
.saturating_add(DbWeight::get().reads(2 as Weight))
.saturating_add(DbWeight::get().writes(3 as Weight))
}
Expand All @@ -51,6 +52,11 @@ impl crate::WeightInfo for () {
.saturating_add(DbWeight::get().reads(2 as Weight))
.saturating_add(DbWeight::get().writes(2 as Weight))
}
fn blacklist() -> Weight {
(31071000 as Weight)
.saturating_add(DbWeight::get().reads(2 as Weight))
.saturating_add(DbWeight::get().writes(2 as Weight))
}
fn external_propose(v: u32, ) -> Weight {
(14282000 as Weight)
.saturating_add((109000 as Weight).saturating_mul(v as Weight))
Expand All @@ -76,6 +82,10 @@ impl crate::WeightInfo for () {
.saturating_add(DbWeight::get().reads(2 as Weight))
.saturating_add(DbWeight::get().writes(2 as Weight))
}
fn cancel_proposal() -> Weight {
(20431000 as Weight)
.saturating_add(DbWeight::get().writes(1 as Weight))
}
fn cancel_referendum() -> Weight {
(20431000 as Weight)
.saturating_add(DbWeight::get().writes(1 as Weight))
Expand Down
121 changes: 113 additions & 8 deletions frame/democracy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
use sp_std::prelude::*;
use sp_runtime::{
DispatchResult, DispatchError, RuntimeDebug,
traits::{Zero, Hash, Dispatchable, Saturating},
traits::{Zero, Hash, Dispatchable, Saturating, Bounded},
};
use codec::{Encode, Decode, Input};
use frame_support::{
Expand Down Expand Up @@ -192,6 +192,9 @@ const DEMOCRACY_ID: LockIdentifier = *b"democrac";
/// NOTE: This is not enforced by any logic.
pub const MAX_VETOERS: u32 = 100;

/// The maximum number of public proposals that can exist at any time.
pub const MAX_PROPOSALS: u32 = 1000;

Comment thread
gavofyork marked this conversation as resolved.
Outdated
/// A proposal index.
pub type PropIndex = u32;

Expand All @@ -203,17 +206,19 @@ type NegativeImbalanceOf<T> =
<<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::NegativeImbalance;

pub trait WeightInfo {
fn propose() -> Weight;
fn propose(p: u32, ) -> Weight;
fn second(s: u32, ) -> Weight;
fn vote_new(r: u32, ) -> Weight;
fn vote_existing(r: u32, ) -> Weight;
fn emergency_cancel() -> Weight;
fn blacklist() -> Weight;
fn external_propose(v: u32, ) -> Weight;
fn external_propose_majority() -> Weight;
fn external_propose_default() -> Weight;
fn fast_track() -> Weight;
fn veto_external(v: u32, ) -> Weight;
fn cancel_referendum() -> Weight;
fn cancel_proposal() -> Weight;
fn cancel_queued(r: u32, ) -> Weight;
fn on_initialize_base(r: u32, ) -> Weight;
fn delegate(r: u32, ) -> Weight;
Expand Down Expand Up @@ -285,6 +290,12 @@ pub trait Trait: frame_system::Trait + Sized {
/// Origin from which any referendum may be cancelled in an emergency.
type CancellationOrigin: EnsureOrigin<Self::Origin>;

/// Origin from which proposals may be blacklisted.
type BlacklistOrigin: EnsureOrigin<Self::Origin>;

/// Origin from which a proposal may be cancelled and its backers slashed.
type CancelProposalOrigin: EnsureOrigin<Self::Origin>;

/// Origin for anyone able to veto proposals.
///
/// # Warning
Expand Down Expand Up @@ -414,8 +425,7 @@ decl_storage! {

/// A record of who vetoed what. Maps proposal hash to a possible existent block number
/// (until when it may not be resubmitted) and who vetoed it.
pub Blacklist get(fn blacklist):
map hasher(identity) T::Hash => Option<(T::BlockNumber, Vec<T::AccountId>)>;
pub Blacklist: map hasher(identity) T::Hash => Option<(T::BlockNumber, Vec<T::AccountId>)>;

/// Record of all proposals that have been subject to emergency cancellation.
pub Cancellations: map hasher(identity) T::Hash => bool;
Expand Down Expand Up @@ -472,6 +482,8 @@ decl_event! {
PreimageReaped(Hash, AccountId, Balance, AccountId),
/// An \[account\] has been unlocked successfully.
Unlocked(AccountId),
/// A proposal \[hash\] has been blacklisted permanently.
Blacklisted(Hash),
}
}

Expand Down Expand Up @@ -544,6 +556,10 @@ decl_error! {
WrongUpperBound,
/// Maximum number of votes reached.
MaxVotesReached,
/// The provided witness data is wrong.
InvalidWitness,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

self/random note that some other modules can generally use this terminology: Some data that is passed in for sanity/weight should be called witness data.

/// Maximum number of proposals reached.
TooManyProposals,
}
}

Expand Down Expand Up @@ -596,14 +612,28 @@ decl_module! {
/// - Db reads: `PublicPropCount`, `PublicProps`
/// - Db writes: `PublicPropCount`, `PublicProps`, `DepositOf`
/// # </weight>
#[weight = T::WeightInfo::propose()]
fn propose(origin, proposal_hash: T::Hash, #[compact] value: BalanceOf<T>) {
#[weight = T::WeightInfo::propose(*prop_count)]
fn propose(origin,
proposal_hash: T::Hash,
#[compact] value: BalanceOf<T>,
#[compact] prop_count: u32,
Comment thread
gavofyork marked this conversation as resolved.
Outdated
Comment thread
gavofyork marked this conversation as resolved.
Outdated
) {
let who = ensure_signed(origin)?;
ensure!(value >= T::MinimumDeposit::get(), Error::<T>::ValueLow);

T::Currency::reserve(&who, value)?;

let index = Self::public_prop_count();
let real_prop_count = PublicProps::<T>::decode_len().unwrap_or(0) as u32;
ensure!(real_prop_count <= prop_count, Error::<T>::InvalidWitness);
ensure!(real_prop_count < MAX_PROPOSALS, Error::<T>::TooManyProposals);

if let Some((until, _)) = <Blacklist<T>>::get(proposal_hash) {
ensure!(
<frame_system::Module<T>>::block_number() >= until,
Error::<T>::ProposalBlacklisted,
);
}

T::Currency::reserve(&who, value)?;
PublicPropCount::put(index + 1);
<DepositOf<T>>::insert(index, (&[&who][..], value));

Expand Down Expand Up @@ -688,6 +718,58 @@ decl_module! {
Self::internal_cancel_referendum(ref_index);
}

/// Permanently place a proposal into the blacklist. This prevents it from ever being
/// proposed again.
///
/// If called on an queued public or external proposal, then this will result in it being
Comment thread
gavofyork marked this conversation as resolved.
Outdated
/// removed. If the `ref_index` supplied is an active referendum with the proposal hash,
/// then it will be cancelled.
///
/// The dispatch origin of this call must be `BlacklistOrigin`.
///
/// - `proposal_hash`: The proposal hash to blacklist permanently.
/// - `ref_index`: An ongoing referendum whose hash is `proposal_hash`, which will be
/// cancelled.
Comment thread
gavofyork marked this conversation as resolved.
Outdated
#[weight = (T::WeightInfo::blacklist(), DispatchClass::Operational)]
fn blacklist(origin,
proposal_hash: T::Hash,
maybe_ref_index: Option<ReferendumIndex>,
) {
T::BlacklistOrigin::ensure_origin(origin)?;

// Insert the proposal into the blacklist.
let permanent = (T::BlockNumber::max_value(), Vec::<T::AccountId>::new());
Blacklist::<T>::insert(&proposal_hash, permanent);

// Remove the queued proposal, if it's there.
PublicProps::<T>::mutate(|props| {
Comment thread
gui1117 marked this conversation as resolved.
Outdated
if let Some(index) = props.iter().position(|p| p.1 == proposal_hash) {
let (prop_index, ..) = props.remove(index);
if let Some((whos, amount)) = DepositOf::<T>::take(prop_index) {
for who in whos.into_iter() {
T::Slash::on_unbalanced(T::Currency::slash_reserved(&who, amount).0);
}
}
}
});

// Remove the external queued referendum, if it's there.
if matches!(NextExternal::<T>::get(), Some((h, ..)) if h == proposal_hash) {
NextExternal::<T>::kill();
}

// Remove the referendum, if it's there.
if let Some(ref_index) = maybe_ref_index {
let status = Self::referendum_status(ref_index)?;
let h = status.proposal_hash;
if h == proposal_hash {
Self::internal_cancel_referendum(ref_index);
}
}

Self::deposit_event(RawEvent::Blacklisted(proposal_hash));
}

/// Schedule a referendum to be tabled once it is legal to schedule an external
/// referendum.
///
Expand Down Expand Up @@ -848,6 +930,29 @@ decl_module! {
<NextExternal<T>>::kill();
}

/// Remove a proposal.
///
/// The dispatch origin of this call must be _Root_.
///
/// - `ref_index`: The index of the proposal to cancel.
Comment thread
gavofyork marked this conversation as resolved.
Outdated
///
/// # <weight>
/// - Complexity: `O(1)`.
/// - Db writes: `ReferendumInfoOf`
Comment thread
gavofyork marked this conversation as resolved.
Outdated
/// - Base Weight: 21.57 µs
/// # </weight>
#[weight = T::WeightInfo::cancel_proposal()]
fn cancel_proposal(origin, #[compact] prop_index: PropIndex) {
Comment thread
gavofyork marked this conversation as resolved.
Outdated
T::CancelProposalOrigin::ensure_origin(origin)?;

PublicProps::<T>::mutate(|props| props.retain(|p| p.0 != prop_index));
Comment thread
gavofyork marked this conversation as resolved.
Outdated
if let Some((whos, amount)) = DepositOf::<T>::take(prop_index) {
for who in whos.into_iter() {
T::Slash::on_unbalanced(T::Currency::slash_reserved(&who, amount).0);
}
}
}

/// Remove a referendum.
///
/// The dispatch origin of this call must be _Root_.
Expand Down
6 changes: 6 additions & 0 deletions frame/democracy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ const NAY: Vote = Vote { aye: false, conviction: Conviction::None };
const BIG_AYE: Vote = Vote { aye: true, conviction: Conviction::Locked1x };
const BIG_NAY: Vote = Vote { aye: false, conviction: Conviction::Locked1x };

const MAX_PROPOSALS: u32 = 100;
Comment thread
gavofyork marked this conversation as resolved.

impl_outer_origin! {
pub enum Origin for Test where system = frame_system {}
}
Expand Down Expand Up @@ -193,6 +195,8 @@ impl super::Trait for Test {
type ExternalDefaultOrigin = EnsureSignedBy<One, u64>;
type FastTrackOrigin = EnsureSignedBy<Five, u64>;
type CancellationOrigin = EnsureSignedBy<Four, u64>;
type BlacklistOrigin = EnsureRoot<u64>;
type CancelProposalOrigin = EnsureRoot<u64>;
type VetoOrigin = EnsureSignedBy<OneToFive, u64>;
type CooloffPeriod = CooloffPeriod;
type PreimageByteDeposit = PreimageByteDeposit;
Expand Down Expand Up @@ -269,6 +273,7 @@ fn propose_set_balance(who: u64, value: u64, delay: u64) -> DispatchResult {
Origin::signed(who),
set_balance_proposal_hash(value),
delay,
MAX_PROPOSALS,
)
}

Expand All @@ -277,6 +282,7 @@ fn propose_set_balance_and_note(who: u64, value: u64, delay: u64) -> DispatchRes
Origin::signed(who),
set_balance_proposal_hash_and_note(value),
delay,
MAX_PROPOSALS,
)
}

Expand Down
Loading