Introduce scheduler and use it for the democracy dispatch queue (#5412)

* Initial draft of the logic

* Build and tests

* Make work with new initialize infratructure.

* Update frame/scheduler/src/lib.rs

Co-Authored-By: Marcio Diaz <marcio.diaz@gmail.com>

* Update frame/scheduler/src/lib.rs

Co-Authored-By: Marcio Diaz <marcio.diaz@gmail.com>

* Update frame/scheduler/src/lib.rs

Co-Authored-By: Marcio Diaz <marcio.diaz@gmail.com>

* Update frame/scheduler/src/lib.rs

Co-Authored-By: Marcio Diaz <marcio.diaz@gmail.com>

* Fix test

* Update frame/scheduler/src/lib.rs

Co-Authored-By: Marcio Diaz <marcio.diaz@gmail.com>

* Rejig interface to make it more useful for democracy.

* Try to get democraxy module to make use of scheduler.

* Make democracy use scheduler.

* Use actual max weight for enactent

* Remove TODO

* Fix runtime build

* Minor cleanup

* Fix scheduler.

* Fix benchmarks

* Fix

* Fix

* Fix

* More bench fixes

* Fix

* Fix.

* Add more bench constants.

* Fix cancel_queued bench.

* Fix test comment.

* Update frame/scheduler/src/lib.rs

Co-Authored-By: Marcio Diaz <marcio.diaz@gmail.com>

Co-authored-by: Marcio Diaz <marcio.diaz@gmail.com>
This commit is contained in:
Gavin Wood
2020-04-01 15:52:39 +02:00
committed by GitHub
parent 433824511e
commit eac1a1964e
16 changed files with 825 additions and 121 deletions
+31 -41
View File
@@ -29,6 +29,9 @@ const SEED: u32 = 0;
const MAX_USERS: u32 = 1000;
const MAX_REFERENDUMS: u32 = 100;
const MAX_PROPOSALS: u32 = 100;
const MAX_SECONDERS: u32 = 100;
const MAX_VETOERS: u32 = 100;
const MAX_BYTES: u32 = 16_384;
fn funded_account<T: Trait>(name: &'static str, index: u32) -> T::AccountId {
let caller: T::AccountId = account(name, index, SEED);
@@ -57,6 +60,13 @@ fn add_referendum<T: Trait>(n: u32) -> Result<ReferendumIndex, &'static str> {
0.into(),
);
let referendum_index: ReferendumIndex = ReferendumCount::get() - 1;
let _ = T::Scheduler::schedule_named(
(DEMOCRACY_ID, referendum_index),
0.into(),
None,
63,
Call::enact_proposal(proposal_hash, referendum_index).into(),
);
Ok(referendum_index)
}
@@ -89,7 +99,7 @@ benchmarks! {
let p in 1 .. MAX_PROPOSALS;
// Add p proposals
for i in 0..p {
for i in 0 .. p {
add_proposal::<T>(i)?;
}
@@ -99,10 +109,10 @@ benchmarks! {
}: _(RawOrigin::Signed(caller), proposal_hash, value.into())
second {
let s in 0 .. 100;
let s in 0 .. MAX_SECONDERS;
// Create s existing "seconds"
for i in 0..s {
for i in 0 .. s {
let seconder = funded_account::<T>("seconder", i);
Democracy::<T>::second(RawOrigin::Signed(seconder).into(), 0)?;
}
@@ -202,7 +212,7 @@ benchmarks! {
veto_external {
// Existing veto-ers
let v in 0 .. 100;
let v in 0 .. MAX_VETOERS;
let proposal_hash: T::Hash = T::Hashing::hash_of(&v);
@@ -210,7 +220,7 @@ benchmarks! {
Democracy::<T>::external_propose_default(origin_propose, proposal_hash.clone())?;
let mut vetoers: Vec<T::AccountId> = Vec::new();
for i in 0..v {
for i in 0 .. v {
vetoers.push(account("vetoer", i, SEED));
}
Blacklist::<T>::insert(proposal_hash, (T::BlockNumber::zero(), vetoers));
@@ -228,12 +238,9 @@ benchmarks! {
}: _(RawOrigin::Root, referendum_index)
cancel_queued {
let d in 0 .. 100;
let u in 1 .. MAX_USERS;
let referendum_index = add_referendum::<T>(d)?;
let block_number: T::BlockNumber = 0.into();
let hash: T::Hash = T::Hashing::hash_of(&d);
<DispatchQueue<T>>::put(vec![(block_number, hash, referendum_index.clone()); d as usize]);
let referendum_index = add_referendum::<T>(u)?;
}: _(RawOrigin::Root, referendum_index)
open_proxy {
@@ -297,15 +304,17 @@ benchmarks! {
}: _(RawOrigin::Signed(delegator))
clear_public_proposals {
let p in 0 .. 100;
let p in 0 .. MAX_PROPOSALS;
for i in 0 .. p {
add_proposal::<T>(i)?;
}
}: _(RawOrigin::Root)
note_preimage {
// Num of bytes in encoded proposal
let b in 0 .. 16_384;
let b in 0 .. MAX_BYTES;
let caller = funded_account::<T>("caller", b);
let encoded_proposal = vec![0; b as usize];
@@ -313,51 +322,32 @@ benchmarks! {
note_imminent_preimage {
// Num of bytes in encoded proposal
let b in 0 .. 16_384;
// Length of dispatch queue
let d in 0 .. 100;
let b in 0 .. MAX_BYTES;
let mut dispatch_queue = Vec::new();
// d + 1 to include the one we are testing
for i in 0 .. d + 1 {
let encoded_proposal = vec![0; i as usize];
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
let block_number = T::BlockNumber::zero();
let referendum_index: ReferendumIndex = 0;
dispatch_queue.push((block_number, proposal_hash, referendum_index))
}
<DispatchQueue<T>>::put(dispatch_queue);
let encoded_proposal = vec![0; b as usize];
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
let block_number = T::BlockNumber::one();
Preimages::<T>::insert(&proposal_hash, PreimageStatus::Missing(block_number));
let caller = funded_account::<T>("caller", b);
let encoded_proposal = vec![0; d as usize];
let encoded_proposal = vec![0; b as usize];
}: _(RawOrigin::Signed(caller), encoded_proposal)
reap_preimage {
// Num of bytes in encoded proposal
let b in 0 .. 16_384;
// Length of dispatch queue
let d in 0 .. 100;
let b in 0 .. MAX_BYTES;
let mut dispatch_queue = Vec::new();
for i in 0 .. d {
let encoded_proposal = vec![0; i as usize];
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
let block_number = T::BlockNumber::zero();
let referendum_index: ReferendumIndex = 0;
dispatch_queue.push((block_number, proposal_hash, referendum_index))
}
<DispatchQueue<T>>::put(dispatch_queue);
let encoded_proposal = vec![0; b as usize];
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
let caller = funded_account::<T>("caller", d);
let encoded_proposal = vec![0; d as usize];
let caller = funded_account::<T>("caller", b);
Democracy::<T>::note_preimage(RawOrigin::Signed(caller.clone()).into(), encoded_proposal.clone())?;
// We need to set this otherwise we get `Early` error.
let block_number = T::VotingPeriod::get() + T::EnactmentPeriod::get() + T::BlockNumber::one();
System::<T>::set_block_number(block_number.into());
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
}: _(RawOrigin::Signed(caller), proposal_hash)
unlock {
+109 -67
View File
@@ -165,15 +165,16 @@
use sp_std::prelude::*;
use sp_runtime::{
DispatchResult, DispatchError, traits::{Zero, EnsureOrigin, Hash, Dispatchable, Saturating},
DispatchResult, DispatchError, RuntimeDebug,
traits::{Zero, EnsureOrigin, Hash, Dispatchable, Saturating},
};
use codec::{Ref, Decode};
use codec::{Ref, Encode, Decode};
use frame_support::{
decl_module, decl_storage, decl_event, decl_error, ensure, Parameter,
weights::{SimpleDispatchInfo, Weight, WeighData},
traits::{
Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get,
OnUnbalanced, BalanceStatus
OnUnbalanced, BalanceStatus, schedule::Named as ScheduleNamed
}
};
use frame_system::{self as system, ensure_signed, ensure_root};
@@ -206,7 +207,7 @@ type NegativeImbalanceOf<T> =
<<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::NegativeImbalance;
pub trait Trait: frame_system::Trait + Sized {
type Proposal: Parameter + Dispatchable<Origin=Self::Origin>;
type Proposal: Parameter + Dispatchable<Origin=Self::Origin> + From<Call<Self>>;
type Event: From<Event<Self>> + Into<<Self as frame_system::Trait>::Event>;
/// Currency type for this module.
@@ -273,6 +274,33 @@ pub trait Trait: frame_system::Trait + Sized {
/// Handler for the unbalanced reduction when slashing a preimage deposit.
type Slash: OnUnbalanced<NegativeImbalanceOf<Self>>;
/// The Scheduler.
type Scheduler: ScheduleNamed<Self::BlockNumber, Self::Proposal>;
}
#[derive(Clone, Encode, Decode, RuntimeDebug)]
pub enum PreimageStatus<AccountId, Balance, BlockNumber> {
/// The preimage is imminently needed at the argument.
Missing(BlockNumber),
/// The preimage is available.
Available {
data: Vec<u8>,
provider: AccountId,
deposit: Balance,
since: BlockNumber,
/// None if it's not imminent.
expiry: Option<BlockNumber>,
},
}
impl<AccountId, Balance, BlockNumber> PreimageStatus<AccountId, Balance, BlockNumber> {
fn to_missing_expiry(self) -> Option<BlockNumber> {
match self {
PreimageStatus::Missing(expiry) => Some(expiry),
_ => None,
}
}
}
decl_storage! {
@@ -293,7 +321,7 @@ decl_storage! {
// https://github.com/paritytech/substrate/issues/5322
pub Preimages:
map hasher(identity) T::Hash
=> Option<(Vec<u8>, T::AccountId, BalanceOf<T>, T::BlockNumber)>;
=> Option<PreimageStatus<T::AccountId, BalanceOf<T>, T::BlockNumber>>;
/// The next free referendum index, aka the number of referenda started so far.
pub ReferendumCount get(fn referendum_count) build(|_| 0 as ReferendumIndex): ReferendumIndex;
@@ -306,11 +334,6 @@ decl_storage! {
map hasher(twox_64_concat) ReferendumIndex
=> Option<ReferendumInfo<T::BlockNumber, T::Hash, BalanceOf<T>>>;
// TODO: Refactor DispatchQueue into its own pallet.
// https://github.com/paritytech/substrate/issues/5322
/// Queue of successful referenda to be dispatched. Stored ordered by block number.
pub DispatchQueue get(fn dispatch_queue): Vec<(T::BlockNumber, T::Hash, ReferendumIndex)>;
/// All votes for a particular voter. We store the balance for the number of votes that we
/// have recorded. The second item is the total amount of delegations, that will be added.
pub VotingOf: map hasher(twox_64_concat) T::AccountId => Voting<BalanceOf<T>, T::AccountId, T::BlockNumber>;
@@ -816,11 +839,8 @@ decl_module! {
#[weight = SimpleDispatchInfo::FixedOperational(10_000)]
fn cancel_queued(origin, which: ReferendumIndex) {
ensure_root(origin)?;
let mut items = <DispatchQueue<T>>::get();
let original_len = items.len();
items.retain(|i| i.2 != which);
ensure!(items.len() < original_len, Error::<T>::ProposalMissing);
<DispatchQueue<T>>::put(items);
T::Scheduler::cancel_named((DEMOCRACY_ID, which))
.map_err(|_| Error::<T>::ProposalMissing)?;
}
fn on_initialize(n: T::BlockNumber) -> Weight {
@@ -986,7 +1006,14 @@ decl_module! {
T::Currency::reserve(&who, deposit)?;
let now = <frame_system::Module<T>>::block_number();
<Preimages<T>>::insert(proposal_hash, (encoded_proposal, who.clone(), deposit, now));
let a = PreimageStatus::Available {
data: encoded_proposal,
provider: who.clone(),
deposit,
since: now,
expiry: None,
};
<Preimages<T>>::insert(proposal_hash, a);
Self::deposit_event(RawEvent::PreimageNoted(proposal_hash, who, deposit));
}
@@ -1007,13 +1034,19 @@ decl_module! {
fn note_imminent_preimage(origin, encoded_proposal: Vec<u8>) {
let who = ensure_signed(origin)?;
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
ensure!(!<Preimages<T>>::contains_key(&proposal_hash), Error::<T>::DuplicatePreimage);
let queue = <DispatchQueue<T>>::get();
ensure!(queue.iter().any(|item| &item.1 == &proposal_hash), Error::<T>::NotImminent);
let status = Preimages::<T>::get(&proposal_hash).ok_or(Error::<T>::NotImminent)?;
let expiry = status.to_missing_expiry().ok_or(Error::<T>::DuplicatePreimage)?;
let now = <frame_system::Module<T>>::block_number();
let free = <BalanceOf<T>>::zero();
<Preimages<T>>::insert(proposal_hash, (encoded_proposal, who.clone(), free, now));
let a = PreimageStatus::Available {
data: encoded_proposal,
provider: who.clone(),
deposit: Zero::zero(),
since: now,
expiry: Some(expiry),
};
<Preimages<T>>::insert(proposal_hash, a);
Self::deposit_event(RawEvent::PreimageNoted(proposal_hash, who, free));
}
@@ -1036,20 +1069,22 @@ decl_module! {
#[weight = SimpleDispatchInfo::FixedNormal(10_000)]
fn reap_preimage(origin, proposal_hash: T::Hash) {
let who = ensure_signed(origin)?;
let (provider, deposit, since, expiry) = <Preimages<T>>::get(&proposal_hash)
.and_then(|m| match m {
PreimageStatus::Available { provider, deposit, since, expiry, .. }
=> Some((provider, deposit, since, expiry)),
_ => None,
}).ok_or(Error::<T>::PreimageMissing)?;
let (_, old, deposit, then) = <Preimages<T>>::get(&proposal_hash)
.ok_or(Error::<T>::PreimageMissing)?;
let now = <frame_system::Module<T>>::block_number();
let (voting, enactment) = (T::VotingPeriod::get(), T::EnactmentPeriod::get());
let additional = if who == old { Zero::zero() } else { enactment };
ensure!(now >= then + voting + additional, Error::<T>::TooEarly);
let additional = if who == provider { Zero::zero() } else { enactment };
ensure!(now >= since + voting + additional, Error::<T>::TooEarly);
ensure!(expiry.map_or(true, |e| now > e), Error::<T>::Imminent);
let queue = <DispatchQueue<T>>::get();
ensure!(!queue.iter().any(|item| &item.1 == &proposal_hash), Error::<T>::Imminent);
let _ = T::Currency::repatriate_reserved(&old, &who, deposit, BalanceStatus::Free);
let _ = T::Currency::repatriate_reserved(&provider, &who, deposit, BalanceStatus::Free);
<Preimages<T>>::remove(&proposal_hash);
Self::deposit_event(RawEvent::PreimageReaped(proposal_hash, old, deposit, who));
Self::deposit_event(RawEvent::PreimageReaped(proposal_hash, provider, deposit, who));
}
/// Unlock tokens that have an expired lock.
@@ -1222,6 +1257,13 @@ decl_module! {
let target = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::<T>::NotProxy)?;
Self::try_remove_vote(&target, index, UnvoteScope::Any)
}
/// Enact a proposal from a referendum. For now we just make the weight be the maximum.
#[weight = SimpleDispatchInfo::MaxNormal]
fn enact_proposal(origin, proposal_hash: T::Hash, index: ReferendumIndex) -> DispatchResult {
ensure_root(origin)?;
Self::do_enact_proposal(proposal_hash, index)
}
}
}
@@ -1544,28 +1586,6 @@ impl<T: Trait> Module<T> {
ref_index
}
/// Enact a proposal from a referendum.
fn enact_proposal(proposal_hash: T::Hash, index: ReferendumIndex) -> DispatchResult {
if let Some((encoded_proposal, who, amount, _)) = <Preimages<T>>::take(&proposal_hash) {
if let Ok(proposal) = T::Proposal::decode(&mut &encoded_proposal[..]) {
let _ = T::Currency::unreserve(&who, amount);
Self::deposit_event(RawEvent::PreimageUsed(proposal_hash, who, amount));
let ok = proposal.dispatch(frame_system::RawOrigin::Root.into()).is_ok();
Self::deposit_event(RawEvent::Executed(index, ok));
Ok(())
} else {
T::Slash::on_unbalanced(T::Currency::slash_reserved(&who, amount).0);
Self::deposit_event(RawEvent::PreimageInvalid(proposal_hash, index));
Err(Error::<T>::PreimageInvalid.into())
}
} else {
Self::deposit_event(RawEvent::PreimageMissing(proposal_hash, index));
Err(Error::<T>::PreimageMissing.into())
}
}
/// Table the next waiting proposal for a vote.
fn launch_next(now: T::BlockNumber) -> DispatchResult {
if LastTabledWasExternal::take() {
@@ -1622,6 +1642,28 @@ impl<T: Trait> Module<T> {
}
}
fn do_enact_proposal(proposal_hash: T::Hash, index: ReferendumIndex) -> DispatchResult {
let preimage = <Preimages<T>>::take(&proposal_hash);
if let Some(PreimageStatus::Available { data, provider, deposit, .. }) = preimage {
if let Ok(proposal) = T::Proposal::decode(&mut &data[..]) {
let _ = T::Currency::unreserve(&provider, deposit);
Self::deposit_event(RawEvent::PreimageUsed(proposal_hash, provider, deposit));
let ok = proposal.dispatch(frame_system::RawOrigin::Root.into()).is_ok();
Self::deposit_event(RawEvent::Executed(index, ok));
Ok(())
} else {
T::Slash::on_unbalanced(T::Currency::slash_reserved(&provider, deposit).0);
Self::deposit_event(RawEvent::PreimageInvalid(proposal_hash, index));
Err(Error::<T>::PreimageInvalid.into())
}
} else {
Self::deposit_event(RawEvent::PreimageMissing(proposal_hash, index));
Err(Error::<T>::PreimageMissing.into())
}
}
fn bake_referendum(
now: T::BlockNumber,
index: ReferendumIndex,
@@ -1633,13 +1675,24 @@ impl<T: Trait> Module<T> {
if approved {
Self::deposit_event(RawEvent::Passed(index));
if status.delay.is_zero() {
let _ = Self::enact_proposal(status.proposal_hash, index);
let _ = Self::do_enact_proposal(status.proposal_hash, index);
} else {
let item = (now + status.delay, status.proposal_hash, index);
<DispatchQueue<T>>::mutate(|queue| {
let pos = queue.binary_search_by_key(&item.0, |x| x.0).unwrap_or_else(|e| e);
queue.insert(pos, item);
let when = now + status.delay;
// Note that we need the preimage now.
Preimages::<T>::mutate_exists(&status.proposal_hash, |maybe_pre| match *maybe_pre {
Some(PreimageStatus::Available { ref mut expiry, .. }) => *expiry = Some(when),
ref mut a => *a = Some(PreimageStatus::Missing(when)),
});
if T::Scheduler::schedule_named(
(DEMOCRACY_ID, index),
when,
None,
63,
Call::enact_proposal(status.proposal_hash, index).into(),
).is_err() {
frame_support::print("LOGIC ERROR: bake_referendum/schedule_named failed");
}
}
} else {
Self::deposit_event(RawEvent::NotPassed(index));
@@ -1662,17 +1715,6 @@ impl<T: Trait> Module<T> {
let approved = Self::bake_referendum(now, index, info)?;
ReferendumInfoOf::<T>::insert(index, ReferendumInfo::Finished { end: now, approved });
}
let queue = <DispatchQueue<T>>::get();
let mut used = 0;
// It's stored in order, so the earliest will always be at the start.
for &(_, proposal_hash, index) in queue.iter().take_while(|x| x.0 == now) {
let _ = Self::enact_proposal(proposal_hash.clone(), index);
used += 1;
}
if used != 0 {
<DispatchQueue<T>>::put(&queue[used..]);
}
Ok(())
}
}
+12 -2
View File
@@ -21,7 +21,7 @@ use std::cell::RefCell;
use codec::Encode;
use frame_support::{
impl_outer_origin, impl_outer_dispatch, assert_noop, assert_ok, parameter_types,
ord_parameter_types, traits::Contains, weights::Weight,
ord_parameter_types, traits::{Contains, OnInitialize}, weights::Weight,
};
use sp_core::H256;
use sp_runtime::{
@@ -71,7 +71,7 @@ impl frame_system::Trait for Test {
type Origin = Origin;
type Index = u64;
type BlockNumber = u64;
type Call = ();
type Call = Call;
type Hash = H256;
type Hashing = BlakeTwo256;
type AccountId = u64;
@@ -90,6 +90,13 @@ impl frame_system::Trait for Test {
}
parameter_types! {
pub const ExistentialDeposit: u64 = 1;
pub const MaximumWeight: u32 = 1000000;
}
impl pallet_scheduler::Trait for Test {
type Event = ();
type Origin = Origin;
type Call = Call;
type MaximumWeight = MaximumWeight;
}
impl pallet_balances::Trait for Test {
type Balance = u64;
@@ -152,6 +159,7 @@ impl super::Trait for Test {
type Slash = ();
type InstantOrigin = EnsureSignedBy<Six, u64>;
type InstantAllowed = InstantAllowed;
type Scheduler = Scheduler;
}
fn new_test_ext() -> sp_io::TestExternalities {
@@ -167,6 +175,7 @@ fn new_test_ext() -> sp_io::TestExternalities {
type System = frame_system::Module<Test>;
type Balances = pallet_balances::Module<Test>;
type Scheduler = pallet_scheduler::Module<Test>;
type Democracy = Module<Test>;
#[test]
@@ -215,6 +224,7 @@ fn propose_set_balance_and_note(who: u64, value: u64, delay: u64) -> DispatchRes
fn next_block() {
System::set_block_number(System::block_number() + 1);
Scheduler::on_initialize(System::block_number());
assert_eq!(Democracy::begin_block(System::block_number()), Ok(()));
}
@@ -50,13 +50,11 @@ fn cancel_queued_should_work() {
fast_forward_to(4);
assert_eq!(Democracy::dispatch_queue(), vec![
(6, set_balance_proposal_hash_and_note(2), 0)
]);
assert!(pallet_scheduler::Agenda::<Test>::get(6)[0].is_some());
assert_noop!(Democracy::cancel_queued(Origin::ROOT, 1), Error::<Test>::ProposalMissing);
assert_ok!(Democracy::cancel_queued(Origin::ROOT, 0));
assert_eq!(Democracy::dispatch_queue(), vec![]);
assert!(pallet_scheduler::Agenda::<Test>::get(6)[0].is_none());
});
}
@@ -152,7 +152,6 @@ fn reaping_imminent_preimage_should_fail() {
assert_ok!(Democracy::vote(Origin::signed(1), r, aye(1)));
next_block();
next_block();
// now imminent.
assert_noop!(Democracy::reap_preimage(Origin::signed(6), h), Error::<Test>::Imminent);
});
}
@@ -85,9 +85,7 @@ fn single_proposal_should_work() {
fast_forward_to(4);
assert!(Democracy::referendum_status(0).is_err());
assert_eq!(Democracy::dispatch_queue(), vec![
(6, set_balance_proposal_hash_and_note(2), 0)
]);
assert!(pallet_scheduler::Agenda::<Test>::get(6)[0].is_some());
// referendum passes and wait another two blocks for enactment.
fast_forward_to(6);