Integrate modified scheduler to reversible-transfers pallet (#15)

Author: dastansam

Cargo.lock

@@ -50,9 +50,9 @@ pallet-conviction-voting = { version = "39.1.0", default-features = false }
 pallet-ranked-collective = { version = "39.0.0", default-features = false }
 pallet-preimage = { version = "39.0.0", default-features = false }
 pallet-referenda = { version = "39.1.0", default-features = false }
+pallet-treasury = { version = "38.1.0", default-features = false}
 pallet-recovery = { version = "39.1.0", default-features = false }
 pallet-scheduler = { path = "./pallets/scheduler", default-features = false }
-pallet-treasury = { version = "38.1.0", default-features = false }
 pallet-utility = { version = "39.1.0", default-features = false }
 parking_lot = { version = "0.12.1", default-features = false }
 pallet-vesting = { version = "39.1.0", default-features = false }
@@ -86,7 +86,7 @@ sp-blockchain = { version = "38.0.0", default-features = false }
 sp-consensus = { version = "0.41.0", default-features = false }
 sp-consensus-pow = { path = "./primitives/consensus/pow", default-features = false }
 sp-consensus-qpow = { path = "./primitives/consensus/qpow", default-features = false }
-sp-common = { path = "./primitives/common", default-features = false }
+qp-scheduler = { path = "./primitives/scheduler", default-features = false }
 sp-faucet = { path = "./primitives/faucet", default-features = false }
 sp-core = { version = "35.0.0", default-features = false }
 sp-genesis-builder = { version = "0.16.0", default-features = false }

Cargo.toml

@@ -20,6 +20,7 @@ frame-support.workspace = true
 frame-system.workspace = true
 sp-runtime.workspace = true
 pallet-balances = { workspace = true }
+qp-scheduler = { workspace = true, default-features = false }
 
 [dev-dependencies]
 frame-support = { workspace = true, features = ["experimental"], default-features = true }
@@ -29,7 +30,7 @@ pallet-scheduler = { workspace = true, default-features = false }
 pallet-timestamp = { workspace = true, default-features = false }
 pallet-balances = { workspace = true, features = ["std"] }
 pallet-preimage = { workspace = true, default-features = false }
-sp-common = { workspace = true, default-features = false }
+qp-scheduler = { workspace = true, default-features = false }
 
 [features]
 default = ["std"]
@@ -42,6 +43,8 @@ std = [
 	"scale-info/std",
 	"sp-runtime/std",
 	"pallet-balances/std",
+	"qp-scheduler/std",
+	"pallet-scheduler/std",
 ]
 # Enable support for setting the existential deposit to zero.
 insecure_zero_ed = []

pallets/reversible-transfers/Cargo.toml

@@ -22,13 +22,18 @@ pub mod weights;
 pub use weights::WeightInfo;
 
 use alloc::vec::Vec;
+use frame_support::pallet_prelude::*;
 use frame_support::traits::tokens::{Fortitude, Restriction};
-use frame_support::{pallet_prelude::*, traits::schedule::DispatchTime};
 use frame_system::pallet_prelude::*;
+use qp_scheduler::{BlockNumberOrTimestamp, DispatchTime, ScheduleNamed};
 use sp_runtime::traits::StaticLookup;
 
+/// Type alias for this config's `BlockNumberOrTimestamp`.
+pub type BlockNumberOrTimestampOf<T> =
+    BlockNumberOrTimestamp<BlockNumberFor<T>, <T as Config>::Moment>;
+
 /// How to delay transactions
-/// - `Explicit`: Only delay transactions explicitly using `schedule_transfer`.
+/// - `Explicit`: Only delay transactions explicitly using this pallet's `schedule_transfer` extrinsic.
 /// - `Intercept`: Intercept and delay transactions at the `TransactionExtension` level.
 ///
 /// For example, for a reversible account with `DelayPolicy::Intercept`, the transaction
@@ -49,11 +54,11 @@ pub enum DelayPolicy {
 
 /// Reversible account details
 #[derive(Encode, Decode, MaxEncodedLen, Clone, Default, TypeInfo, Debug, PartialEq, Eq)]
-pub struct ReversibleAccountData<AccountId, BlockNumber> {
+pub struct ReversibleAccountData<AccountId, Delay> {
     /// The account that can reverse the transaction. `None` means the account itself.
     pub explicit_reverser: Option<AccountId>,
     /// The delay period for the account
-    pub delay: BlockNumber,
+    pub delay: Delay,
     /// The policy for the account
     pub policy: DelayPolicy,
 }
@@ -78,17 +83,15 @@ type BalanceOf<T> = <T as pallet_balances::Config>::Balance;
 #[frame_support::pallet]
 pub mod pallet {
     use super::*;
+    use crate::BlockNumberOrTimestampOf;
     use frame_support::dispatch::PostDispatchInfo;
     use frame_support::traits::fungible::MutateHold;
     use frame_support::traits::tokens::Precision;
-    use frame_support::traits::{Bounded, CallerTrait, QueryPreimage, StorePreimage};
-    use frame_support::{
-        traits::schedule::v3::{Named, TaskName},
-        PalletId,
-    };
-    use sp_runtime::traits::AccountIdConversion;
-    use sp_runtime::traits::Hash;
+    use frame_support::traits::{Bounded, CallerTrait, QueryPreimage, StorePreimage, Time};
+    use frame_support::{traits::schedule::v3::TaskName, PalletId};
+    use sp_runtime::traits::{AccountIdConversion, AtLeast32Bit};
     use sp_runtime::traits::{BlockNumberProvider, Dispatchable};
+    use sp_runtime::traits::{Hash, Scale};
     use sp_runtime::Saturating;
 
     #[pallet::pallet]
@@ -106,8 +109,9 @@ pub mod pallet {
         type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
 
         /// Scheduler for the runtime. We use the Named scheduler for cancellability.
-        type Scheduler: Named<
+        type Scheduler: ScheduleNamed<
             BlockNumberFor<Self>,
+            Self::Moment,
             Self::RuntimeCall,
             Self::SchedulerOrigin,
             Hasher = Self::Hashing,
@@ -126,12 +130,18 @@ pub mod pallet {
         type MaxPendingPerAccount: Get<u32>;
 
         /// The default delay period for reversible transactions if none is specified.
+        ///
+        /// NOTE: default delay is always in blocks.
+        #[pallet::constant]
+        type DefaultDelay: Get<BlockNumberOrTimestampOf<Self>>;
+
+        /// The minimum delay period allowed for reversible transactions, in blocks.
         #[pallet::constant]
-        type DefaultDelay: Get<BlockNumberFor<Self>>;
+        type MinDelayPeriodBlocks: Get<BlockNumberFor<Self>>;
 
-        /// The minimum delay period allowed for reversible transactions.
+        /// The minimum delay period allowed for reversible transactions, in milliseconds.
         #[pallet::constant]
-        type MinDelayPeriod: Get<BlockNumberFor<Self>>;
+        type MinDelayPeriodMoment: Get<Self::Moment>;
 
         /// Pallet Id
         type PalletId: Get<PalletId>;
@@ -144,6 +154,17 @@ pub mod pallet {
 
         /// Hold reason for the reversible transactions.
         type RuntimeHoldReason: From<HoldReason>;
+
+        /// Moment type for scheduling.
+        type Moment: Saturating
+            + Copy
+            + Parameter
+            + AtLeast32Bit
+            + Scale<BlockNumberFor<Self>, Output = Self::Moment>
+            + MaxEncodedLen;
+
+        /// Time provider for scheduling.
+        type TimeProvider: Time<Moment = Self::Moment>;
     }
 
     /// Maps accounts to their chosen reversibility delay period (in milliseconds).
@@ -154,7 +175,7 @@ pub mod pallet {
         _,
         Blake2_128Concat,
         T::AccountId,
-        ReversibleAccountData<T::AccountId, BlockNumberFor<T>>,
+        ReversibleAccountData<T::AccountId, BlockNumberOrTimestampOf<T>>,
         OptionQuery,
     >;
 
@@ -184,14 +205,14 @@ pub mod pallet {
         /// [who, maybe_delay: None means disabled]
         ReversibilitySet {
             who: T::AccountId,
-            data: ReversibleAccountData<T::AccountId, BlockNumberFor<T>>,
+            data: ReversibleAccountData<T::AccountId, BlockNumberOrTimestampOf<T>>,
         },
         /// A transaction has been intercepted and scheduled for delayed execution.
         /// [who, tx_id, execute_at_moment]
         TransactionScheduled {
             who: T::AccountId,
             tx_id: T::Hash,
-            execute_at: DispatchTime<BlockNumberFor<T>>,
+            execute_at: DispatchTime<BlockNumberFor<T>, T::Moment>,
         },
         /// A scheduled transaction has been successfully cancelled by the owner.
         /// [who, tx_id]
@@ -248,24 +269,36 @@ pub mod pallet {
         #[pallet::weight(<T as Config>::WeightInfo::set_reversibility())]
         pub fn set_reversibility(
             origin: OriginFor<T>,
-            delay: Option<BlockNumberFor<T>>,
+            delay: Option<BlockNumberOrTimestampOf<T>>,
             policy: DelayPolicy,
             reverser: Option<T::AccountId>,
         ) -> DispatchResult {
             let who = ensure_signed(origin)?;
 
-            ensure!(
-                !ReversibleAccounts::<T>::contains_key(&who),
-                Error::<T>::AccountAlreadyReversible
-            );
             ensure!(
                 reverser != Some(who.clone()),
                 Error::<T>::ExplicitReverserCanNotBeSelf
             );
-
+            ensure!(
+                !ReversibleAccounts::<T>::contains_key(&who),
+                Error::<T>::AccountAlreadyReversible
+            );
             let delay = delay.unwrap_or(T::DefaultDelay::get());
 
-            ensure!(delay >= T::MinDelayPeriod::get(), Error::<T>::DelayTooShort);
+            match delay {
+                BlockNumberOrTimestamp::BlockNumber(x) => {
+                    ensure!(
+                        x > T::MinDelayPeriodBlocks::get(),
+                        Error::<T>::DelayTooShort
+                    )
+                }
+                BlockNumberOrTimestamp::Timestamp(t) => {
+                    ensure!(
+                        t > T::MinDelayPeriodMoment::get(),
+                        Error::<T>::DelayTooShort
+                    )
+                }
+            }
 
             let reversible_account_data = ReversibleAccountData {
                 explicit_reverser: reverser,
@@ -327,12 +360,16 @@ pub mod pallet {
     impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
         fn integrity_test() {
             assert!(
-                T::MinDelayPeriod::get() > Zero::zero(),
-                "`T::MinDelayPeriod` must be greater than 0"
+                !T::MinDelayPeriodBlocks::get().is_zero()
+                    && !T::MinDelayPeriodMoment::get().is_zero(),
+                "Minimum delay periods must be greater than 0"
             );
+
+            // NOTE: default delay is always in blocks
             assert!(
-                T::MinDelayPeriod::get() <= T::DefaultDelay::get(),
-                "`T::MinDelayPeriod` must be less or equal to `T::DefaultDelay`"
+                BlockNumberOrTimestampOf::<T>::BlockNumber(T::MinDelayPeriodBlocks::get())
+                    <= T::DefaultDelay::get(),
+                "Minimum delay periods must be less or equal to `T::DefaultDelay`"
             );
         }
     }
@@ -352,7 +389,7 @@ pub mod pallet {
         /// Check if an account has reversibility enabled and return its delay.
         pub fn is_reversible(
             who: &T::AccountId,
-        ) -> Option<ReversibleAccountData<T::AccountId, BlockNumberFor<T>>> {
+        ) -> Option<ReversibleAccountData<T::AccountId, BlockNumberOrTimestampOf<T>>> {
             ReversibleAccounts::<T>::get(who)
         }
 
@@ -430,18 +467,8 @@ pub mod pallet {
             amount: BalanceOf<T>,
         ) -> DispatchResult {
             let who = ensure_signed(origin)?;
-            let ReversibleAccountData {
-                delay,
-                explicit_reverser,
-                policy: _,
-            } = Self::reversible_accounts(&who).ok_or(Error::<T>::AccountNotReversible)?;
-
-            match explicit_reverser {
-                Some(reverser) => {
-                    ensure!(who != reverser, Error::<T>::InvalidReverser);
-                }
-                None => {}
-            };
+            let ReversibleAccountData { delay, .. } =
+                Self::reversible_accounts(&who).ok_or(Error::<T>::AccountNotReversible)?;
 
             let transfer_call: T::RuntimeCall = pallet_balances::Call::<T>::transfer_keep_alive {
                 dest: dest.clone(),
@@ -461,9 +488,16 @@ pub mod pallet {
                 Ok(())
             })?;
 
-            let dispatch_time = DispatchTime::At(
-                T::BlockNumberProvider::current_block_number().saturating_add(delay),
-            );
+            let dispatch_time = match delay {
+                BlockNumberOrTimestamp::BlockNumber(blocks) => DispatchTime::At(
+                    T::BlockNumberProvider::current_block_number().saturating_add(blocks),
+                ),
+                BlockNumberOrTimestamp::Timestamp(millis) => {
+                    DispatchTime::After(BlockNumberOrTimestamp::Timestamp(
+                        T::TimeProvider::now().saturating_add(millis),
+                    ))
+                }
+            };
 
             let call = T::Preimages::bound(transfer_call)?;
 
@@ -592,6 +626,7 @@ pub mod pallet {
     #[derive(frame_support::DefaultNoBound)]
     pub struct GenesisConfig<T: Config> {
         /// Configure initial reversible accounts. [AccountId, Delay]
+        /// NOTE: using `(bool, BlockNumberFor<T>)` where `bool` indicates if the delay is in block numbers
         pub initial_reversible_accounts: Vec<(T::AccountId, BlockNumberFor<T>)>,
     }
 
@@ -600,20 +635,22 @@ pub mod pallet {
         fn build(&self) {
             for (who, delay) in &self.initial_reversible_accounts {
                 // Basic validation, ensure delay is reasonable if needed
-                if *delay >= T::MinDelayPeriod::get() {
+                let wrapped_delay = BlockNumberOrTimestampOf::<T>::BlockNumber(*delay);
+
+                if delay >= &T::MinDelayPeriodBlocks::get() {
                     ReversibleAccounts::<T>::insert(
                         who,
                         ReversibleAccountData {
                             explicit_reverser: None,
-                            delay: *delay,
+                            delay: wrapped_delay,
                             policy: DelayPolicy::Explicit,
                         },
                     );
                 } else {
                     // Optionally log a warning during genesis build
                     log::warn!(
-                        "Genesis config for account {:?} has delay {:?} below MinDelayPeriod {:?}, skipping.",
-                        who, delay, T::MinDelayPeriod::get()
+                        "Genesis config for account {:?} has delay {:?} below MinDelayPeriodBlocks {:?}, skipping.",
+                        who, wrapped_delay, T::MinDelayPeriodBlocks::get()
                      );
                 }
             }