diff --git a/substrate/frame/referenda/src/benchmarking.rs b/substrate/frame/referenda/src/benchmarking.rs index 76a8173f16..08612e0614 100644 --- a/substrate/frame/referenda/src/benchmarking.rs +++ b/substrate/frame/referenda/src/benchmarking.rs @@ -48,7 +48,7 @@ fn create_referendum() -> (T::AccountId, ReferendumIndex) { RawOrigin::Signed(caller.clone()).into(), RawOrigin::Root.into(), T::Hashing::hash_of(&0), - AtOrAfter::After(0u32.into()) + DispatchTime::After(0u32.into()) )); let index = ReferendumCount::::get() - 1; (caller, index) @@ -182,7 +182,7 @@ benchmarks! { RawOrigin::Signed(caller), RawOrigin::Root.into(), T::Hashing::hash_of(&0), - AtOrAfter::After(0u32.into()) + DispatchTime::After(0u32.into()) ) verify { let index = ReferendumCount::::get().checked_sub(1).unwrap(); assert_matches!(ReferendumInfoFor::::get(index), Some(ReferendumInfo::Ongoing(_))); diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index 2d4d29b0bc..fb19d2b9ed 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -68,8 +68,8 @@ use frame_support::{ v2::{Anon as ScheduleAnon, Named as ScheduleNamed}, DispatchTime, MaybeHashed, }, - Currency, Get, LockIdentifier, LockableCurrency, OnUnbalanced, OriginTrait, PollStatus, - Polling, ReservableCurrency, VoteTally, + Currency, Get, LockIdentifier, OnUnbalanced, OriginTrait, PollStatus, Polling, + ReservableCurrency, VoteTally, }, BoundedVec, }; @@ -86,7 +86,7 @@ pub mod weights; use branch::{BeginDecidingBranch, OneFewerDecidingBranch, ServiceBranch}; pub use pallet::*; pub use types::{ - AtOrAfter, BalanceOf, CallOf, Curve, DecidingStatus, DecidingStatusOf, Deposit, InsertSorted, + BalanceOf, CallOf, Curve, DecidingStatus, DecidingStatusOf, Deposit, InsertSorted, NegativeImbalanceOf, PalletsOriginOf, ReferendumIndex, ReferendumInfo, ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, ScheduleAddressOf, TallyOf, TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf, @@ -126,9 +126,7 @@ pub mod pallet { type Scheduler: ScheduleAnon, PalletsOriginOf, Hash = Self::Hash> + ScheduleNamed, PalletsOriginOf, Hash = Self::Hash>; /// Currency type for this pallet. - type Currency: ReservableCurrency - + LockableCurrency; - + type Currency: ReservableCurrency; // Origins and unbalances. /// Origin from which any vote may be cancelled. type CancelOrigin: EnsureOrigin; @@ -175,8 +173,6 @@ pub mod pallet { pub type ReferendumCount = StorageValue<_, ReferendumIndex, ValueQuery>; /// Information concerning any given referendum. - /// - /// TWOX-NOTE: SAFE as indexes are not under an attacker’s control. #[pallet::storage] pub type ReferendumInfoFor = StorageMap<_, Blake2_128Concat, ReferendumIndex, ReferendumInfoOf>; @@ -301,7 +297,7 @@ pub mod pallet { /// Referendum is not ongoing. NotOngoing, /// Referendum's decision deposit is already paid. - HaveDeposit, + HasDeposit, /// The track identifier given was invalid. BadTrack, /// There are already a full complement of referendums in progress for this track. @@ -338,7 +334,7 @@ pub mod pallet { origin: OriginFor, proposal_origin: PalletsOriginOf, proposal_hash: T::Hash, - enactment_moment: AtOrAfter, + enactment_moment: DispatchTime, ) -> DispatchResult { let who = ensure_signed(origin)?; @@ -385,7 +381,7 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let mut status = Self::ensure_ongoing(index)?; - ensure!(status.decision_deposit.is_none(), Error::::HaveDeposit); + ensure!(status.decision_deposit.is_none(), Error::::HasDeposit); let track = Self::track(status.track).ok_or(Error::::NoTrack)?; status.decision_deposit = Some(Self::take_deposit(who.clone(), track.decision_deposit)?); @@ -598,7 +594,7 @@ impl Polling for Pallet { track: class, origin: frame_support::dispatch::RawOrigin::Root.into(), proposal_hash: ::hash_of(&index), - enactment: AtOrAfter::After(Zero::zero()), + enactment: DispatchTime::After(Zero::zero()), submitted: now, submission_deposit: Deposit { who: dummy_account_id, amount: Zero::zero() }, decision_deposit: None, @@ -651,7 +647,7 @@ impl Pallet { fn schedule_enactment( index: ReferendumIndex, track: &TrackInfoOf, - desired: AtOrAfter, + desired: DispatchTime, origin: PalletsOriginOf, call_hash: T::Hash, ) { diff --git a/substrate/frame/referenda/src/mock.rs b/substrate/frame/referenda/src/mock.rs index 063b124f2b..fdd14fdadf 100644 --- a/substrate/frame/referenda/src/mock.rs +++ b/substrate/frame/referenda/src/mock.rs @@ -306,7 +306,7 @@ pub fn propose_set_balance(who: u64, value: u64, delay: u64) -> DispatchResult { Origin::signed(who), frame_system::RawOrigin::Root.into(), set_balance_proposal_hash(value), - AtOrAfter::After(delay), + DispatchTime::After(delay), ) } @@ -434,7 +434,7 @@ impl RefState { Origin::signed(1), frame_support::dispatch::RawOrigin::Root.into(), set_balance_proposal_hash(1), - AtOrAfter::At(10), + DispatchTime::At(10), )); assert_ok!(Referenda::place_decision_deposit(Origin::signed(2), 0)); if matches!(self, RefState::Confirming { immediate: true }) { diff --git a/substrate/frame/referenda/src/tests.rs b/substrate/frame/referenda/src/tests.rs index cea071ced1..96edd4ce87 100644 --- a/substrate/frame/referenda/src/tests.rs +++ b/substrate/frame/referenda/src/tests.rs @@ -47,7 +47,7 @@ fn basic_happy_path_works() { Origin::signed(1), RawOrigin::Root.into(), set_balance_proposal_hash(1), - AtOrAfter::At(10), + DispatchTime::At(10), )); assert_eq!(Balances::reserved_balance(&1), 2); assert_eq!(ReferendumCount::::get(), 1); @@ -178,7 +178,7 @@ fn queueing_works() { Origin::signed(5), RawOrigin::Root.into(), set_balance_proposal_hash(0), - AtOrAfter::After(0), + DispatchTime::After(0), )); assert_ok!(Referenda::place_decision_deposit(Origin::signed(5), 0)); @@ -190,7 +190,7 @@ fn queueing_works() { Origin::signed(i), RawOrigin::Root.into(), set_balance_proposal_hash(i), - AtOrAfter::After(0), + DispatchTime::After(0), )); assert_ok!(Referenda::place_decision_deposit(Origin::signed(i), i as u32)); // TODO: decision deposit after some initial votes with a non-highest voted coming @@ -275,7 +275,7 @@ fn auto_timeout_should_happen_with_nothing_but_submit() { Origin::signed(1), RawOrigin::Root.into(), set_balance_proposal_hash(1), - AtOrAfter::At(20), + DispatchTime::At(20), )); run_to(20); assert_matches!(ReferendumInfoFor::::get(0), Some(ReferendumInfo::Ongoing(..))); @@ -295,13 +295,13 @@ fn tracks_are_distinguished() { Origin::signed(1), RawOrigin::Root.into(), set_balance_proposal_hash(1), - AtOrAfter::At(10), + DispatchTime::At(10), )); assert_ok!(Referenda::submit( Origin::signed(2), RawOrigin::None.into(), set_balance_proposal_hash(2), - AtOrAfter::At(20), + DispatchTime::At(20), )); assert_ok!(Referenda::place_decision_deposit(Origin::signed(3), 0)); @@ -318,7 +318,7 @@ fn tracks_are_distinguished() { track: 0, origin: OriginCaller::system(RawOrigin::Root), proposal_hash: set_balance_proposal_hash(1), - enactment: AtOrAfter::At(10), + enactment: DispatchTime::At(10), submitted: 1, submission_deposit: Deposit { who: 1, amount: 2 }, decision_deposit: Some(Deposit { who: 3, amount: 10 }), @@ -334,7 +334,7 @@ fn tracks_are_distinguished() { track: 1, origin: OriginCaller::system(RawOrigin::None), proposal_hash: set_balance_proposal_hash(2), - enactment: AtOrAfter::At(20), + enactment: DispatchTime::At(20), submitted: 1, submission_deposit: Deposit { who: 2, amount: 2 }, decision_deposit: Some(Deposit { who: 4, amount: 1 }), @@ -355,13 +355,18 @@ fn submit_errors_work() { let h = set_balance_proposal_hash(1); // No track for Signed origins. assert_noop!( - Referenda::submit(Origin::signed(1), RawOrigin::Signed(2).into(), h, AtOrAfter::At(10),), + Referenda::submit( + Origin::signed(1), + RawOrigin::Signed(2).into(), + h, + DispatchTime::At(10), + ), Error::::NoTrack ); // No funds for deposit assert_noop!( - Referenda::submit(Origin::signed(10), RawOrigin::Root.into(), h, AtOrAfter::At(10),), + Referenda::submit(Origin::signed(10), RawOrigin::Root.into(), h, DispatchTime::At(10),), BalancesError::::InsufficientBalance ); }); @@ -378,13 +383,13 @@ fn decision_deposit_errors_work() { Origin::signed(1), RawOrigin::Root.into(), h, - AtOrAfter::At(10), + DispatchTime::At(10), )); let e = BalancesError::::InsufficientBalance; assert_noop!(Referenda::place_decision_deposit(Origin::signed(10), 0), e); assert_ok!(Referenda::place_decision_deposit(Origin::signed(2), 0)); - let e = Error::::HaveDeposit; + let e = Error::::HasDeposit; assert_noop!(Referenda::place_decision_deposit(Origin::signed(2), 0), e); }); } @@ -400,7 +405,7 @@ fn refund_deposit_works() { Origin::signed(1), RawOrigin::Root.into(), h, - AtOrAfter::At(10), + DispatchTime::At(10), )); let e = Error::::NoDeposit; assert_noop!(Referenda::refund_decision_deposit(Origin::signed(2), 0), e); @@ -422,7 +427,7 @@ fn cancel_works() { Origin::signed(1), RawOrigin::Root.into(), h, - AtOrAfter::At(10), + DispatchTime::At(10), )); assert_ok!(Referenda::place_decision_deposit(Origin::signed(2), 0)); @@ -441,7 +446,7 @@ fn cancel_errors_works() { Origin::signed(1), RawOrigin::Root.into(), h, - AtOrAfter::At(10), + DispatchTime::At(10), )); assert_ok!(Referenda::place_decision_deposit(Origin::signed(2), 0)); assert_noop!(Referenda::cancel(Origin::signed(1), 0), BadOrigin); @@ -459,7 +464,7 @@ fn kill_works() { Origin::signed(1), RawOrigin::Root.into(), h, - AtOrAfter::At(10), + DispatchTime::At(10), )); assert_ok!(Referenda::place_decision_deposit(Origin::signed(2), 0)); @@ -479,7 +484,7 @@ fn kill_errors_works() { Origin::signed(1), RawOrigin::Root.into(), h, - AtOrAfter::At(10), + DispatchTime::At(10), )); assert_ok!(Referenda::place_decision_deposit(Origin::signed(2), 0)); assert_noop!(Referenda::kill(Origin::signed(4), 0), BadOrigin); diff --git a/substrate/frame/referenda/src/types.rs b/substrate/frame/referenda/src/types.rs index 367fa2a4ba..8ea9fc3faf 100644 --- a/substrate/frame/referenda/src/types.rs +++ b/substrate/frame/referenda/src/types.rs @@ -185,26 +185,6 @@ pub trait TracksInfo { } } -/// Indication of either a specific moment or a delay from a implicitly defined moment. -#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)] -pub enum AtOrAfter { - /// Indiciates that the event should occur at the moment given. - At(Moment), - /// Indiciates that the event should occur some period of time (defined by the parameter) after - /// a prior event. The prior event is defined by the context, but for the purposes of - /// referendum proposals, the "prior event" is the passing of the referendum. - After(Moment), -} - -impl AtOrAfter { - pub fn evaluate(&self, since: Moment) -> Moment { - match &self { - Self::At(m) => *m, - Self::After(m) => m.saturating_add(since), - } - } -} - /// Info regarding an ongoing referendum. #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub struct ReferendumStatus< @@ -224,7 +204,7 @@ pub struct ReferendumStatus< /// The hash of the proposal up for referendum. pub(crate) proposal_hash: Hash, /// The time the proposal should be scheduled for enactment. - pub(crate) enactment: AtOrAfter, + pub(crate) enactment: DispatchTime, /// The time of submission. Once `UndecidingTimeout` passes, it may be closed by anyone if it /// `deciding` is `None`. pub(crate) submitted: Moment, diff --git a/substrate/frame/support/src/traits/schedule.rs b/substrate/frame/support/src/traits/schedule.rs index 3b8e6da3e2..c2d0d4bc3b 100644 --- a/substrate/frame/support/src/traits/schedule.rs +++ b/substrate/frame/support/src/traits/schedule.rs @@ -19,7 +19,7 @@ use codec::{Codec, Decode, Encode, EncodeLike, MaxEncodedLen}; use scale_info::TypeInfo; -use sp_runtime::{DispatchError, RuntimeDebug}; +use sp_runtime::{traits::Saturating, DispatchError, RuntimeDebug}; use sp_std::{fmt::Debug, prelude::*, result::Result}; /// Information relating to the period of a scheduled task. First item is the length of the @@ -32,7 +32,7 @@ pub type Period = (BlockNumber, u32); pub type Priority = u8; /// The dispatch time of a scheduled task. -#[derive(Encode, Decode, Copy, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)] +#[derive(Encode, Decode, Copy, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub enum DispatchTime { /// At specified block. At(BlockNumber), @@ -40,6 +40,15 @@ pub enum DispatchTime { After(BlockNumber), } +impl DispatchTime { + pub fn evaluate(&self, since: BlockNumber) -> BlockNumber { + match &self { + Self::At(m) => *m, + Self::After(m) => m.saturating_add(since), + } + } +} + /// The highest priority. We invert the value so that normal sorting will place the highest /// priority at the beginning of the list. pub const HIGHEST_PRIORITY: Priority = 0;