From 0d1ca5cdf267cde7ca7de87b2015aced4dc4f463 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Thu, 7 Mar 2024 01:45:08 +0800 Subject: [PATCH 1/8] add delay-tasks --- Cargo.dev.toml | 1 + delay-tasks/Cargo.toml | 48 ++++++ delay-tasks/src/lib.rs | 196 +++++++++++++++++++++++++ delay-tasks/src/mock.rs | 1 + delay-tasks/src/tests.rs | 1 + traits/src/delay_tasks.rs | 70 +++++++++ traits/src/lib.rs | 1 + xtokens/src/lib.rs | 171 ++++++++++++++++++--- xtokens/src/mock/para.rs | 23 ++- xtokens/src/mock/para_relative_view.rs | 19 ++- xtokens/src/mock/para_teleport.rs | 22 ++- 11 files changed, 522 insertions(+), 31 deletions(-) create mode 100644 delay-tasks/Cargo.toml create mode 100644 delay-tasks/src/lib.rs create mode 100644 delay-tasks/src/mock.rs create mode 100644 delay-tasks/src/tests.rs create mode 100644 traits/src/delay_tasks.rs diff --git a/Cargo.dev.toml b/Cargo.dev.toml index 728aec898..89f6f765f 100644 --- a/Cargo.dev.toml +++ b/Cargo.dev.toml @@ -6,6 +6,7 @@ members = [ "benchmarking", "build-script-utils", "currencies", + "delay-tasks", "gradually-update", "nft", "oracle", diff --git a/delay-tasks/Cargo.toml b/delay-tasks/Cargo.toml new file mode 100644 index 000000000..1667fc6f4 --- /dev/null +++ b/delay-tasks/Cargo.toml @@ -0,0 +1,48 @@ +[package] +name = "orml-delay-tasks" +description = "Delayed xtokens transfer assets executor." +repository = "https://github.com/open-web3-stack/open-runtime-module-library/tree/master/auction" +license = "Apache-2.0" +version = "0.7.0" +authors = ["Acala Developers"] +edition = "2021" + +[dependencies] +log = { workspace = true } +parity-scale-codec = { workspace = true } +scale-info = { workspace = true } +serde = { workspace = true, optional = true } + +frame-support = { workspace = true } +frame-system = { workspace = true } +sp-runtime = { workspace = true } +sp-std = { workspace = true } + +xcm = { workspace = true } + +orml-traits = { path = "../traits", version = "0.7.0", default-features = false } +orml-xtokens = { path = "../xtokens", version = "0.7.0", default-features = false } + +[dev-dependencies] +sp-core = { workspace = true, features = ["std"] } +sp-io = { workspace = true, features = ["std"] } + +[features] +default = [ "std" ] +std = [ + "frame-support/std", + "frame-system/std", + "orml-traits/std", + "orml-xtokens/std", + "parity-scale-codec/std", + "scale-info/std", + "serde", + "sp-runtime/std", + "sp-std/std", + "xcm/std", +] +try-runtime = [ + "frame-support/try-runtime", + "frame-system/try-runtime", + "sp-runtime/try-runtime", +] diff --git a/delay-tasks/src/lib.rs b/delay-tasks/src/lib.rs new file mode 100644 index 000000000..0be5fde0d --- /dev/null +++ b/delay-tasks/src/lib.rs @@ -0,0 +1,196 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +use frame_support::{pallet_prelude::*, traits::schedule::DispatchTime, transactional, weights::Weight}; +use frame_system::{ensure_signed, pallet_prelude::*}; +use orml_traits::{ + delay_tasks::{DelayTasks, DelayedTask}, + NamedMultiReservableCurrency, +}; +use parity_scale_codec::{Decode, Encode, FullCodec}; +use scale_info::TypeInfo; +use sp_runtime::{ + traits::{CheckedAdd, Convert, Zero}, + ArithmeticError, +}; +use sp_std::fmt::Debug; +use xcm::v4::prelude::*; + +pub use module::*; + +mod mock; +mod tests; + +#[frame_support::pallet] +pub mod module { + use super::*; + + type Nonce = u64; + + #[pallet::config] + pub trait Config: frame_system::Config + orml_xtokens::Config { + type RuntimeEvent: From> + IsType<::RuntimeEvent>; + + type GovernanceOrigin: EnsureOrigin<::RuntimeOrigin>; + + type Task: DelayedTask + FullCodec + Debug + Clone + PartialEq + TypeInfo; + } + + #[pallet::error] + pub enum Error { + BelowMinBondThreshold, + InvalidDelayBlock, + InvalidId, + } + + #[pallet::event] + #[pallet::generate_deposit(pub(crate) fn deposit_event)] + pub enum Event { + /// A task has been dispatched on_idle. + DelayedTaskAdded { + id: Nonce, + task: T::Task, + }, + DelayedTaskExecuted { + id: Nonce, + result: DispatchResult, + }, + DelayedTaskPreExecuteFailed { + id: Nonce, + }, + } + + #[pallet::pallet] + #[pallet::without_storage_info] + pub struct Pallet(_); + + #[pallet::hooks] + impl Hooks> for Pallet { + /// `on_initialize` to return the weight used in `on_finalize`. + fn on_initialize(now: BlockNumberFor) -> Weight { + Weight::zero() + } + + fn on_finalize(now: BlockNumberFor) { + Self::_on_finalize(now); + } + } + + #[pallet::storage] + #[pallet::getter(fn next_delayed_task_id)] + pub type NextDelayedTaskId = StorageValue<_, Nonce, ValueQuery>; + + #[pallet::storage] + #[pallet::getter(fn delayed_tasks)] + pub type DelayedTasks = StorageMap<_, Twox64Concat, Nonce, (T::Task, BlockNumberFor), OptionQuery>; + + #[pallet::storage] + #[pallet::getter(fn delayed_task_queue)] + pub type DelayedTaskQueue = + StorageDoubleMap<_, Twox64Concat, BlockNumberFor, Twox64Concat, Nonce, (), OptionQuery>; + + #[pallet::call] + impl Pallet { + #[pallet::call_index(0)] + #[pallet::weight(Weight::zero())] + pub fn reset_execute_block( + origin: OriginFor, + id: Nonce, + when: DispatchTime>, + ) -> DispatchResult { + T::GovernanceOrigin::ensure_origin(origin)?; + + DelayedTasks::::try_mutate_exists(id, |maybe_task| -> DispatchResult { + let (_, execute_block) = maybe_task.as_mut().ok_or(Error::::InvalidId)?; + + let now = frame_system::Pallet::::block_number(); + let new_execute_block = match when { + DispatchTime::At(x) => x, + DispatchTime::After(x) => x.checked_add(&now).ok_or(ArithmeticError::Overflow)?, + }; + ensure!(new_execute_block > now, Error::::InvalidDelayBlock); + + DelayedTaskQueue::::remove(*execute_block, id); + DelayedTaskQueue::::insert(new_execute_block, id, ()); + *execute_block = new_execute_block; + + Ok(()) + })?; + + Ok(()) + } + + #[pallet::call_index(1)] + #[pallet::weight(Weight::zero())] + pub fn cancel_delayed_task(origin: OriginFor, id: Nonce) -> DispatchResult { + T::GovernanceOrigin::ensure_origin(origin)?; + + let (delay_task, execute_block) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; + delay_task.on_cancel()?; + DelayedTaskQueue::::remove(execute_block, id); + + Ok(()) + } + } + + impl Pallet { + fn _on_finalize(now: BlockNumberFor) { + for (id, _) in DelayedTaskQueue::::drain_prefix(now) { + match Self::do_execute_delayed_task(id) { + Ok(result) => { + Self::deposit_event(Event::::DelayedTaskExecuted { id, result }); + } + Err(e) => { + Self::deposit_event(Event::::DelayedTaskPreExecuteFailed { id }); + } + } + } + } + + #[transactional] + fn do_execute_delayed_task(id: Nonce) -> sp_std::result::Result { + let (delayed_task, _) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; + delayed_task.pre_delayed_execute().map_err(|e| { + log::debug!( + target: "delay-tasks", + "delayed task#{:?}:\n {:?}\n do pre_execute_delayed failed for: {:?}", + id, + delayed_task, + e + ); + e + })?; + + Ok(delayed_task.delayed_execute()) + } + + /// Retrieves the next delayed task ID from storage, and increment it by + /// one. + fn get_next_delayed_task_id() -> Result { + NextDelayedTaskId::::mutate(|current| -> Result { + let id = *current; + + *current = current.checked_add(1).ok_or(ArithmeticError::Overflow)?; + Ok(id) + }) + } + } + + impl DelayTasks> for Pallet { + fn schedule_delay_task(task: T::Task, delay_blocks: BlockNumberFor) -> DispatchResult { + ensure!(!delay_blocks.is_zero(), Error::::InvalidDelayBlock); + + task.pre_delay()?; + + let id = Self::get_next_delayed_task_id()?; + let execute_block_number = frame_system::Pallet::::block_number() + .checked_add(&delay_blocks) + .ok_or(ArithmeticError::Overflow)?; + + DelayedTasks::::insert(id, (&task, execute_block_number)); + DelayedTaskQueue::::insert(execute_block_number, id, ()); + + Self::deposit_event(Event::::DelayedTaskAdded { id, task }); + Ok(()) + } + } +} diff --git a/delay-tasks/src/mock.rs b/delay-tasks/src/mock.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/delay-tasks/src/mock.rs @@ -0,0 +1 @@ + diff --git a/delay-tasks/src/tests.rs b/delay-tasks/src/tests.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/delay-tasks/src/tests.rs @@ -0,0 +1 @@ + diff --git a/traits/src/delay_tasks.rs b/traits/src/delay_tasks.rs new file mode 100644 index 000000000..9255acb99 --- /dev/null +++ b/traits/src/delay_tasks.rs @@ -0,0 +1,70 @@ +use sp_runtime::DispatchResult; + +pub trait DelayedTask { + fn pre_delay(&self) -> DispatchResult; + fn pre_delayed_execute(&self) -> DispatchResult; + fn delayed_execute(&self) -> DispatchResult; + fn on_cancel(&self) -> DispatchResult; +} + +pub trait DelayTasks { + fn schedule_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; +} + +#[macro_export] +macro_rules! define_combined_delayed_task { + ( + $(#[$meta:meta])* + $vis:vis enum $combined_name:ident { + $( + $task:ident ( $vtask:ident $(<$($generic:tt),*>)? ) + ),+ $(,)? + } + ) => { + $(#[$meta])* + $vis enum $combined_name { + $( + $task($vtask $(<$($generic),*>)?), + )* + } + + impl DelayedTask for $combined_name { + fn pre_delay(&self) -> DispatchResult { + match self { + $( + $combined_name::$task(t) => t.pre_delay(), + )* + } + } + fn pre_delayed_execute(&self) -> DispatchResult { + match self { + $( + $combined_name::$task(t) => t.pre_delayed_execute(), + )* + } + } + fn delayed_execute(&self) -> DispatchResult { + match self { + $( + $combined_name::$task(t) => t.delayed_execute(), + )* + } + } + fn on_cancel(&self) -> DispatchResult { + match self { + $( + $combined_name::$task(t) => t.on_cancel(), + )* + } + } + } + + $( + impl From<$vtask $(<$($generic),*>)?> for $combined_name { + fn from(t: $vtask $(<$($generic),*>)?) -> Self{ + $combined_name::$task(t) + } + } + )* + }; +} diff --git a/traits/src/lib.rs b/traits/src/lib.rs index c59e7340b..aba54777d 100644 --- a/traits/src/lib.rs +++ b/traits/src/lib.rs @@ -32,6 +32,7 @@ pub mod asset_registry; pub mod auction; pub mod currency; pub mod data_provider; +pub mod delay_tasks; pub mod get_by_key; pub mod location; pub mod multi_asset; diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 38aa0a0f8..2db7c7806 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -43,8 +43,10 @@ use sp_runtime::{ traits::{AtLeast32BitUnsigned, Bounded, Convert, MaybeSerializeDeserialize, Member, Zero}, DispatchError, }; -use sp_std::{prelude::*, result::Result}; +use sp_std::{fmt::Debug, prelude::*, result::Result}; +use parity_scale_codec::{Decode, Encode, FullCodec}; +use scale_info::TypeInfo; use xcm::{ v4::{prelude::*, Weight}, VersionedAsset, VersionedAssets, VersionedLocation, @@ -53,9 +55,10 @@ use xcm_executor::traits::WeightBounds; pub use module::*; use orml_traits::{ + delay_tasks::{DelayTasks, DelayedTask}, location::{Parse, Reserve}, xcm_transfer::{Transferred, XtokensWeightInfo}, - GetByKey, RateLimiter, XcmTransfer, + GetByKey, NamedMultiReservableCurrency, RateLimiter, XcmTransfer, }; mod mock; @@ -75,6 +78,97 @@ use TransferKind::*; pub mod module { use super::*; + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum XtokensDelayedTask { + TransferAssets { + who: T::AccountId, + assets: Assets, + fee: Asset, + dest: Location, + dest_weight_limit: WeightLimit, + }, + } + + impl DelayedTask for XtokensDelayedTask { + fn pre_delay(&self) -> DispatchResult { + match self { + XtokensDelayedTask::TransferAssets { who, assets, .. } => { + let asset_len = assets.len(); + for i in 0..asset_len { + let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; + let currency_id: T::CurrencyId = T::CurrencyIdConvert::convert(asset.id.0.clone()) + .ok_or(Error::::AssetIndexNonExistent)?; + let amount: T::Balance = match asset.fun { + Fungibility::Fungible(amount) => { + amount.try_into().map_err(|_| Error::::AssetIndexNonExistent)? + } + Fungibility::NonFungible(_) => return Err(Error::::AssetIndexNonExistent.into()), + }; + + T::Currency::reserve_named(&T::ReserveIdentifier::get(), currency_id, who, amount)?; + } + } + } + Ok(()) + } + + fn pre_delayed_execute(&self) -> DispatchResult { + match self { + XtokensDelayedTask::TransferAssets { who, assets, .. } => { + let asset_len = assets.len(); + for i in 0..asset_len { + let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; + let currency_id: T::CurrencyId = T::CurrencyIdConvert::convert(asset.id.0.clone()) + .ok_or(Error::::AssetIndexNonExistent)?; + let amount: T::Balance = match asset.fun { + Fungibility::Fungible(amount) => { + amount.try_into().map_err(|_| Error::::AssetIndexNonExistent)? + } + Fungibility::NonFungible(_) => return Err(Error::::AssetIndexNonExistent.into()), + }; + + T::Currency::unreserve_named(&T::ReserveIdentifier::get(), currency_id, who, amount); + } + } + } + Ok(()) + } + + fn delayed_execute(&self) -> DispatchResult { + match self { + XtokensDelayedTask::TransferAssets { + who, + assets, + fee, + dest, + dest_weight_limit, + } => { + // execute `do_transfer_assets` without delay check + Pallet::::do_transfer_assets( + who.clone(), + assets.clone(), + fee.clone(), + dest.clone(), + dest_weight_limit.clone(), + false, + ) + .map(|_| ()) + } + } + } + + fn on_cancel(&self) -> DispatchResult { + self.pre_delayed_execute() + } + } + + pub struct DisabledTransferAssets(sp_std::marker::PhantomData); + impl DelayTasks> for DisabledTransferAssets { + fn schedule_delay_task(_task: T::DelayedTask, _delay_blocks: BlockNumberFor) -> DispatchResult { + Err(Error::::RateLimited.into()) + } + } + #[pallet::config] pub trait Config: frame_system::Config { type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -92,7 +186,8 @@ pub mod module { type CurrencyId: Parameter + Member + Clone; /// Convert `T::CurrencyId` to `Location`. - type CurrencyIdConvert: Convert>; + type CurrencyIdConvert: Convert> + + Convert>; /// Convert `T::AccountId` to `Location`. type AccountIdToLocation: Convert; @@ -137,6 +232,28 @@ pub mod module { /// The id of the RateLimiter. #[pallet::constant] type RateLimiterId: Get<::RateLimiterId>; + + type DelayedTask: DelayedTask + + FullCodec + + Debug + + Clone + + PartialEq + + TypeInfo + + From>; + + type DelayTasks: DelayTasks>; + + type DelayBlocks: Get>; + + type Currency: NamedMultiReservableCurrency< + Self::AccountId, + Balance = Self::Balance, + CurrencyId = Self::CurrencyId, + >; + + type ReserveIdentifier: Get< + >::ReserveIdentifier, + >; } #[pallet::event] @@ -394,7 +511,7 @@ pub mod module { // We first grab the fee let fee: &Asset = assets.get(fee_item as usize).ok_or(Error::::AssetIndexNonExistent)?; - Self::do_transfer_assets(who, assets.clone(), fee.clone(), dest, dest_weight_limit).map(|_| ()) + Self::do_transfer_assets(who, assets.clone(), fee.clone(), dest, dest_weight_limit, true).map(|_| ()) } } @@ -413,7 +530,7 @@ pub mod module { ensure!(T::LocationsFilter::contains(&dest), Error::::NotSupportedLocation); let asset: Asset = (location, amount.into()).into(); - Self::do_transfer_assets(who, vec![asset.clone()].into(), asset, dest, dest_weight_limit) + Self::do_transfer_assets(who, vec![asset.clone()].into(), asset, dest, dest_weight_limit, true) } fn do_transfer_with_fee( @@ -439,7 +556,7 @@ pub mod module { assets.push(asset); assets.push(fee_asset.clone()); - Self::do_transfer_assets(who, assets, fee_asset, dest, dest_weight_limit) + Self::do_transfer_assets(who, assets, fee_asset, dest, dest_weight_limit, true) } fn do_transfer_asset( @@ -448,7 +565,7 @@ pub mod module { dest: Location, dest_weight_limit: WeightLimit, ) -> Result, DispatchError> { - Self::do_transfer_assets(who, vec![asset.clone()].into(), asset, dest, dest_weight_limit) + Self::do_transfer_assets(who, vec![asset.clone()].into(), asset, dest, dest_weight_limit, true) } fn do_transfer_asset_with_fee( @@ -463,7 +580,7 @@ pub mod module { assets.push(asset); assets.push(fee.clone()); - Self::do_transfer_assets(who, assets, fee, dest, dest_weight_limit) + Self::do_transfer_assets(who, assets, fee, dest, dest_weight_limit, true) } fn do_transfer_multicurrencies( @@ -502,7 +619,7 @@ pub mod module { let fee: Asset = (fee_location, (*fee_amount).into()).into(); - Self::do_transfer_assets(who, assets, fee, dest, dest_weight_limit) + Self::do_transfer_assets(who, assets, fee, dest, dest_weight_limit, true) } fn do_transfer_assets( @@ -511,6 +628,7 @@ pub mod module { fee: Asset, dest: Location, dest_weight_limit: WeightLimit, + delay_check: bool, ) -> Result, DispatchError> { ensure!( assets.len() <= T::MaxAssetsForTransfer::get(), @@ -549,18 +667,29 @@ pub mod module { Error::::DistinctReserveForAssetAndFee ); } + } - // per asset check - let amount = match asset.fun { - Fungibility::Fungible(amount) => amount, - Fungibility::NonFungible(_) => 1, - }; - - let rate_limiter_id = T::RateLimiterId::get(); - - // try consume quota of the rate limiter. - T::RateLimiter::try_consume(rate_limiter_id, asset.id.clone(), amount, Some(&who)) - .map_err(|_| Error::::RateLimited)?; + if delay_check { + // delay the transfer assets if limited + if Self::transfer_assets_delay_check(&who, assets.clone()).is_err() { + return T::DelayTasks::schedule_delay_task( + XtokensDelayedTask::TransferAssets { + who: who.clone(), + assets: assets.clone(), + fee: fee.clone(), + dest: dest.clone(), + dest_weight_limit, + } + .into(), + T::DelayBlocks::get(), + ) + .map(|_| Transferred { + sender: who, + assets, + fee, + dest, + }); + } } let fee_reserve = T::ReserveProvider::reserve(&fee); @@ -1023,7 +1152,7 @@ pub mod module { dest: Location, dest_weight_limit: WeightLimit, ) -> Result, DispatchError> { - Self::do_transfer_assets(who, assets, fee, dest, dest_weight_limit) + Self::do_transfer_assets(who, assets, fee, dest, dest_weight_limit, true) } } } diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 5ece9ab6a..547bdcb9a 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -1,6 +1,6 @@ use super::{ - AllowTopLevelPaidExecution, Amount, Balance, CurrencyId, CurrencyIdConvert, ParachainXcmRouter, RateLimiter, - CHARLIE, + AllowTopLevelPaidExecution, Amount, Balance, CurrencyId, CurrencyIdConvert, DisabledTransferAssets, + ParachainXcmRouter, RateLimiter, XtokensDelayedTask, CHARLIE, }; use crate as orml_xtokens; @@ -10,11 +10,12 @@ use frame_support::{ }; use frame_system::EnsureRoot; use pallet_xcm::XcmPassthrough; -use parity_scale_codec::Encode; +use parity_scale_codec::{Decode, Encode}; use polkadot_parachain_primitives::primitives::Sibling; +use scale_info::TypeInfo; use sp_runtime::{ traits::{Convert, IdentityLookup}, - AccountId32, + AccountId32, DispatchResult, RuntimeDebug, }; use sp_std::cell::RefCell; use xcm::v4::{prelude::*, Weight}; @@ -26,7 +27,10 @@ use xcm_builder::{ use xcm_executor::{Config, XcmExecutor}; use crate::mock::AllTokensAreCreatedEqualToWeight; -use orml_traits::{location::AbsoluteReserveProvider, parameter_type_with_key, RateLimiterError}; +use orml_traits::{ + define_combined_delayed_task, delay_tasks::DelayedTask, location::AbsoluteReserveProvider, parameter_type_with_key, + RateLimiterError, +}; use orml_xcm_support::{IsNativeConcrete, MultiCurrencyAdapter, MultiNativeAsset}; pub type AccountId = AccountId32; @@ -265,6 +269,13 @@ parameter_types! { pub const XtokensRateLimiterId: u8 = 0; } +define_combined_delayed_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum DelayedTasks { + XtokensDelayedTask(XtokensDelayedTask), + } +} + impl orml_xtokens::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; @@ -282,6 +293,8 @@ impl orml_xtokens::Config for Runtime { type ReserveProvider = AbsoluteReserveProvider; type RateLimiter = MockRateLimiter; type RateLimiterId = XtokensRateLimiterId; + type DelayedTask = DelayedTasks; + type DelayTasks = DisabledTransferAssets; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/para_relative_view.rs b/xtokens/src/mock/para_relative_view.rs index 248cef833..8b29ff3bd 100644 --- a/xtokens/src/mock/para_relative_view.rs +++ b/xtokens/src/mock/para_relative_view.rs @@ -1,4 +1,6 @@ -use super::{Amount, Balance, CurrencyId, CurrencyIdConvert, ParachainXcmRouter}; +use super::{ + Amount, Balance, CurrencyId, CurrencyIdConvert, DisabledTransferAssets, ParachainXcmRouter, XtokensDelayedTask, +}; use crate as orml_xtokens; use frame_support::{ @@ -7,10 +9,12 @@ use frame_support::{ }; use frame_system::EnsureRoot; use pallet_xcm::XcmPassthrough; +use parity_scale_codec::{Decode, Encode}; use polkadot_parachain_primitives::primitives::Sibling; +use scale_info::TypeInfo; use sp_runtime::{ traits::{Convert, IdentityLookup}, - AccountId32, BoundedVec, + AccountId32, BoundedVec, DispatchResult, RuntimeDebug, }; use xcm::v4::{prelude::*, Weight}; use xcm_builder::{ @@ -22,6 +26,8 @@ use xcm_executor::{Config, XcmExecutor}; use crate::mock::AllTokensAreCreatedEqualToWeight; use orml_traits::{ + define_combined_delayed_task, + delay_tasks::DelayedTask, location::{AbsoluteReserveProvider, RelativeReserveProvider}, parameter_type_with_key, }; @@ -339,6 +345,13 @@ parameter_type_with_key! { }; } +define_combined_delayed_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum DelayedTasks { + XtokensDelayedTask(XtokensDelayedTask), + } +} + impl orml_xtokens::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; @@ -356,6 +369,8 @@ impl orml_xtokens::Config for Runtime { type ReserveProvider = RelativeReserveProvider; type RateLimiter = (); type RateLimiterId = (); + type DelayedTask = DelayedTasks; + type DelayTasks = DisabledTransferAssets; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/para_teleport.rs b/xtokens/src/mock/para_teleport.rs index 6fcb6f7ef..ff6b3a401 100644 --- a/xtokens/src/mock/para_teleport.rs +++ b/xtokens/src/mock/para_teleport.rs @@ -1,4 +1,7 @@ -use super::{AllowTopLevelPaidExecution, Amount, Balance, CurrencyId, CurrencyIdConvert, ParachainXcmRouter}; +use super::{ + AllowTopLevelPaidExecution, Amount, Balance, CurrencyId, CurrencyIdConvert, DisabledTransferAssets, + ParachainXcmRouter, XtokensDelayedTask, +}; use crate as orml_xtokens; use frame_support::{ @@ -7,10 +10,12 @@ use frame_support::{ }; use frame_system::EnsureRoot; use pallet_xcm::XcmPassthrough; +use parity_scale_codec::{Decode, Encode}; use polkadot_parachain_primitives::primitives::Sibling; +use scale_info::TypeInfo; use sp_runtime::{ traits::{Convert, IdentityLookup}, - AccountId32, + AccountId32, DispatchResult, RuntimeDebug, }; use xcm::v4::{prelude::*, Weight}; use xcm_builder::{ @@ -22,7 +27,9 @@ use xcm_executor::{Config, XcmExecutor}; use crate::mock::teleport_currency_adapter::MultiTeleportCurrencyAdapter; use crate::mock::AllTokensAreCreatedEqualToWeight; -use orml_traits::{location::AbsoluteReserveProvider, parameter_type_with_key}; +use orml_traits::{ + define_combined_delayed_task, delay_tasks::DelayedTask, location::AbsoluteReserveProvider, parameter_type_with_key, +}; use orml_xcm_support::{DisabledParachainFee, IsNativeConcrete, MultiNativeAsset}; pub type AccountId = AccountId32; @@ -207,6 +214,13 @@ impl Contains for ParentOrParachains { } } +define_combined_delayed_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum DelayedTasks { + XtokensDelayedTask(XtokensDelayedTask), + } +} + impl orml_xtokens::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; @@ -224,6 +238,8 @@ impl orml_xtokens::Config for Runtime { type ReserveProvider = AbsoluteReserveProvider; type RateLimiter = (); type RateLimiterId = (); + type DelayedTask = DelayedTasks; + type DelayTasks = DisabledTransferAssets; } impl orml_xcm::Config for Runtime { From cb41ec5c74413a98562ef634d116fb6dd19103d5 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Thu, 7 Mar 2024 01:49:09 +0800 Subject: [PATCH 2/8] update --- xtokens/src/lib.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 2db7c7806..61ed3e65e 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -36,7 +36,7 @@ use frame_support::{ pallet_prelude::*, require_transactional, traits::{Contains, Get}, - Parameter, + transactional, Parameter, }; use frame_system::{ensure_signed, pallet_prelude::*}; use sp_runtime::{ @@ -516,6 +516,28 @@ pub mod module { } impl Pallet { + #[transactional] + fn transfer_assets_delay_check(who: &T::AccountId, assets: Assets) -> DispatchResult { + let rate_limiter_id = T::RateLimiterId::get(); + let asset_len = assets.len(); + for i in 0..asset_len { + let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; + + // per asset check + let amount = match asset.fun { + Fungibility::Fungible(amount) => amount, + Fungibility::NonFungible(_) => 1, + }; + + // try consume quota of the rate limiter. + // NOTE: use AssetId as the key, use AccountId as whitelist filter key. + T::RateLimiter::try_consume(rate_limiter_id, asset.id.clone(), amount, Some(who)) + .map_err(|_| Error::::RateLimited)?; + } + + Ok(()) + } + fn do_transfer( who: T::AccountId, currency_id: T::CurrencyId, From 5d8c7ff1758c23ece4cab3966e922461be88bc52 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Wed, 13 Mar 2024 02:13:04 +0800 Subject: [PATCH 3/8] xtokens impl DelayedTask --- asset-registry/src/mock/para.rs | 23 +- delay-tasks/Cargo.toml | 2 - delay-tasks/src/lib.rs | 70 ++--- delay-tasks/src/mock.rs | 192 ++++++++++++++ delay-tasks/src/tests.rs | 350 +++++++++++++++++++++++++ traits/src/delay_tasks.rs | 4 +- xtokens/src/lib.rs | 19 +- xtokens/src/mock/para.rs | 5 + xtokens/src/mock/para_relative_view.rs | 5 + xtokens/src/mock/para_teleport.rs | 5 + 10 files changed, 630 insertions(+), 45 deletions(-) diff --git a/asset-registry/src/mock/para.rs b/asset-registry/src/mock/para.rs index cd428225c..2ab4699fb 100644 --- a/asset-registry/src/mock/para.rs +++ b/asset-registry/src/mock/para.rs @@ -11,17 +11,21 @@ use frame_support::{ }; use frame_system::{EnsureRoot, EnsureSignedBy}; use orml_traits::{ + define_combined_delayed_task, + delay_tasks::DelayedTask, location::{AbsoluteReserveProvider, RelativeReserveProvider}, parameter_type_with_key, FixedConversionRateProvider, MultiCurrency, }; use orml_xcm_support::{IsNativeConcrete, MultiCurrencyAdapter, MultiNativeAsset}; +use orml_xtokens::XtokensDelayedTask; use pallet_xcm::XcmPassthrough; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use polkadot_parachain_primitives::primitives::Sibling; +use scale_info::TypeInfo; use sp_core::Get; use sp_runtime::{ traits::{AccountIdConversion, Convert, IdentityLookup}, - AccountId32, + AccountId32, DispatchResult, RuntimeDebug, }; use xcm::v4::{prelude::*, Weight}; use xcm_builder::{ @@ -310,6 +314,18 @@ parameter_type_with_key! { }; } +define_combined_delayed_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum DelayedTasks { + XtokensDelayedTask(XtokensDelayedTask), + } +} + +parameter_types! { + pub const GetDelayBlocks: u64 = 1000; + pub const GetReserveId: [u8; 8] = *b"xtokensr"; +} + impl orml_xtokens::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; @@ -327,6 +343,11 @@ impl orml_xtokens::Config for Runtime { type ReserveProvider = RelativeReserveProvider; type RateLimiter = (); type RateLimiterId = (); + type DelayedTask = DelayedTasks; + type DelayTasks = orml_xtokens::DisabledTransferAssets; + type DelayBlocks = GetDelayBlocks; + type Currency = Tokens; + type ReserveId = GetReserveId; } impl orml_xcm::Config for Runtime { diff --git a/delay-tasks/Cargo.toml b/delay-tasks/Cargo.toml index 1667fc6f4..f6ad3e240 100644 --- a/delay-tasks/Cargo.toml +++ b/delay-tasks/Cargo.toml @@ -21,7 +21,6 @@ sp-std = { workspace = true } xcm = { workspace = true } orml-traits = { path = "../traits", version = "0.7.0", default-features = false } -orml-xtokens = { path = "../xtokens", version = "0.7.0", default-features = false } [dev-dependencies] sp-core = { workspace = true, features = ["std"] } @@ -33,7 +32,6 @@ std = [ "frame-support/std", "frame-system/std", "orml-traits/std", - "orml-xtokens/std", "parity-scale-codec/std", "scale-info/std", "serde", diff --git a/delay-tasks/src/lib.rs b/delay-tasks/src/lib.rs index 0be5fde0d..47354a3e5 100644 --- a/delay-tasks/src/lib.rs +++ b/delay-tasks/src/lib.rs @@ -1,19 +1,15 @@ #![cfg_attr(not(feature = "std"), no_std)] use frame_support::{pallet_prelude::*, traits::schedule::DispatchTime, transactional, weights::Weight}; -use frame_system::{ensure_signed, pallet_prelude::*}; -use orml_traits::{ - delay_tasks::{DelayTasks, DelayedTask}, - NamedMultiReservableCurrency, -}; -use parity_scale_codec::{Decode, Encode, FullCodec}; +use frame_system::pallet_prelude::*; +use orml_traits::delay_tasks::{DelayTasksManager, DelayedTask}; +use parity_scale_codec::FullCodec; use scale_info::TypeInfo; use sp_runtime::{ - traits::{CheckedAdd, Convert, Zero}, + traits::{CheckedAdd, Zero}, ArithmeticError, }; use sp_std::fmt::Debug; -use xcm::v4::prelude::*; pub use module::*; @@ -27,7 +23,7 @@ pub mod module { type Nonce = u64; #[pallet::config] - pub trait Config: frame_system::Config + orml_xtokens::Config { + pub trait Config: frame_system::Config { type RuntimeEvent: From> + IsType<::RuntimeEvent>; type GovernanceOrigin: EnsureOrigin<::RuntimeOrigin>; @@ -37,7 +33,6 @@ pub mod module { #[pallet::error] pub enum Error { - BelowMinBondThreshold, InvalidDelayBlock, InvalidId, } @@ -45,16 +40,24 @@ pub mod module { #[pallet::event] #[pallet::generate_deposit(pub(crate) fn deposit_event)] pub enum Event { - /// A task has been dispatched on_idle. DelayedTaskAdded { id: Nonce, task: T::Task, + execute_block: BlockNumberFor, }, DelayedTaskExecuted { id: Nonce, result: DispatchResult, }, - DelayedTaskPreExecuteFailed { + DelayedTaskReDelayed { + id: Nonce, + execute_block: BlockNumberFor, + }, + DelayedTaskTryExecuteFailed { + id: Nonce, + error: DispatchError, + }, + DelayedTaskCanceled { id: Nonce, }, } @@ -113,6 +116,10 @@ pub mod module { DelayedTaskQueue::::insert(new_execute_block, id, ()); *execute_block = new_execute_block; + Self::deposit_event(Event::::DelayedTaskReDelayed { + id, + execute_block: new_execute_block, + }); Ok(()) })?; @@ -128,6 +135,7 @@ pub mod module { delay_task.on_cancel()?; DelayedTaskQueue::::remove(execute_block, id); + Self::deposit_event(Event::::DelayedTaskCanceled { id }); Ok(()) } } @@ -140,25 +148,23 @@ pub mod module { Self::deposit_event(Event::::DelayedTaskExecuted { id, result }); } Err(e) => { - Self::deposit_event(Event::::DelayedTaskPreExecuteFailed { id }); + log::debug!( + target: "delay-tasks", + "try executing delayed task {:?} failed for: {:?}. The delayed task still exists, but needs to be canceled or reset delay block.", + id, + e + ); + Self::deposit_event(Event::::DelayedTaskTryExecuteFailed { id, error: e }); } } } } #[transactional] - fn do_execute_delayed_task(id: Nonce) -> sp_std::result::Result { + pub(crate) fn do_execute_delayed_task(id: Nonce) -> sp_std::result::Result { let (delayed_task, _) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; - delayed_task.pre_delayed_execute().map_err(|e| { - log::debug!( - target: "delay-tasks", - "delayed task#{:?}:\n {:?}\n do pre_execute_delayed failed for: {:?}", - id, - delayed_task, - e - ); - e - })?; + + delayed_task.pre_delayed_execute()?; Ok(delayed_task.delayed_execute()) } @@ -175,21 +181,25 @@ pub mod module { } } - impl DelayTasks> for Pallet { - fn schedule_delay_task(task: T::Task, delay_blocks: BlockNumberFor) -> DispatchResult { + impl DelayTasksManager> for Pallet { + fn add_delay_task(task: T::Task, delay_blocks: BlockNumberFor) -> DispatchResult { ensure!(!delay_blocks.is_zero(), Error::::InvalidDelayBlock); task.pre_delay()?; let id = Self::get_next_delayed_task_id()?; - let execute_block_number = frame_system::Pallet::::block_number() + let execute_block = frame_system::Pallet::::block_number() .checked_add(&delay_blocks) .ok_or(ArithmeticError::Overflow)?; - DelayedTasks::::insert(id, (&task, execute_block_number)); - DelayedTaskQueue::::insert(execute_block_number, id, ()); + DelayedTasks::::insert(id, (&task, execute_block)); + DelayedTaskQueue::::insert(execute_block, id, ()); - Self::deposit_event(Event::::DelayedTaskAdded { id, task }); + Self::deposit_event(Event::::DelayedTaskAdded { + id, + task, + execute_block, + }); Ok(()) } } diff --git a/delay-tasks/src/mock.rs b/delay-tasks/src/mock.rs index 8b1378917..a5b14c1c8 100644 --- a/delay-tasks/src/mock.rs +++ b/delay-tasks/src/mock.rs @@ -1 +1,193 @@ +//! Mocks for the delay tasks module. +#![cfg(test)] + +use super::*; +use frame_support::{construct_runtime, derive_impl}; +use frame_system::EnsureRoot; +use orml_traits::define_combined_delayed_task; +use sp_runtime::{traits::IdentityLookup, BuildStorage, DispatchError}; +use sp_std::cell::RefCell; + +use crate as delay_tasks; + +pub type AccountId = u128; + +#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] +impl frame_system::Config for Runtime { + type Nonce = u64; + type AccountId = AccountId; + type Lookup = IdentityLookup; + type Block = Block; +} + +thread_local! { + pub static PRE_DELAYED: RefCell = RefCell::new(false); + pub static PRE_DELAYED_EXECUTED: RefCell = RefCell::new(false); + pub static EXECUTED: RefCell = RefCell::new(false); + pub static ON_CANCEL: RefCell = RefCell::new(false); +} + +pub(crate) fn reset_delay_process_records() { + PRE_DELAYED.with(|v| *v.borrow_mut() = false); + PRE_DELAYED_EXECUTED.with(|v| *v.borrow_mut() = false); + EXECUTED.with(|v| *v.borrow_mut() = false); + ON_CANCEL.with(|v| *v.borrow_mut() = false); +} + +define_combined_delayed_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum MockDelayedTaskType { + Success(SuccessTask), + FailedPreDelay(FailedPreDelayTask), + FailedPreDelayedExecute(FailedPreDelayedExecuteTask), + FailedDelayedExecute(FailedDelayedExecuteTask), + FailedOnCancel(FailedOnCancelTask), + } +} + +#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] +pub struct SuccessTask; +impl DelayedTask for SuccessTask { + fn pre_delay(&self) -> DispatchResult { + PRE_DELAYED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn pre_delayed_execute(&self) -> DispatchResult { + PRE_DELAYED_EXECUTED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn delayed_execute(&self) -> DispatchResult { + EXECUTED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn on_cancel(&self) -> DispatchResult { + ON_CANCEL.with(|v| *v.borrow_mut() = true); + Ok(()) + } +} + +#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] +pub struct FailedPreDelayTask; +impl DelayedTask for FailedPreDelayTask { + fn pre_delay(&self) -> DispatchResult { + Err(DispatchError::Other("pre_delay failed")) + } + + fn pre_delayed_execute(&self) -> DispatchResult { + unimplemented!() + } + + fn delayed_execute(&self) -> DispatchResult { + unimplemented!() + } + + fn on_cancel(&self) -> DispatchResult { + unimplemented!() + } +} + +#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] +pub struct FailedPreDelayedExecuteTask; +impl DelayedTask for FailedPreDelayedExecuteTask { + fn pre_delay(&self) -> DispatchResult { + PRE_DELAYED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn pre_delayed_execute(&self) -> DispatchResult { + Err(DispatchError::Other("pre_delayed_execute failed")) + } + + fn delayed_execute(&self) -> DispatchResult { + EXECUTED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn on_cancel(&self) -> DispatchResult { + ON_CANCEL.with(|v| *v.borrow_mut() = true); + Ok(()) + } +} + +#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] +pub struct FailedDelayedExecuteTask; +impl DelayedTask for FailedDelayedExecuteTask { + fn pre_delay(&self) -> DispatchResult { + PRE_DELAYED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn pre_delayed_execute(&self) -> DispatchResult { + PRE_DELAYED_EXECUTED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn delayed_execute(&self) -> DispatchResult { + Err(DispatchError::Other("delayed_execute failed")) + } + + fn on_cancel(&self) -> DispatchResult { + ON_CANCEL.with(|v| *v.borrow_mut() = true); + Ok(()) + } +} + +#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] +pub struct FailedOnCancelTask; +impl DelayedTask for FailedOnCancelTask { + fn pre_delay(&self) -> DispatchResult { + PRE_DELAYED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn pre_delayed_execute(&self) -> DispatchResult { + PRE_DELAYED_EXECUTED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn delayed_execute(&self) -> DispatchResult { + EXECUTED.with(|v| *v.borrow_mut() = true); + Ok(()) + } + + fn on_cancel(&self) -> DispatchResult { + Err(DispatchError::Other("on_cancel failed")) + } +} + +impl Config for Runtime { + type RuntimeEvent = RuntimeEvent; + type GovernanceOrigin = EnsureRoot; + type Task = MockDelayedTaskType; +} + +type Block = frame_system::mocking::MockBlock; + +construct_runtime!( + pub enum Runtime { + System: frame_system, + DelayTasks: delay_tasks, + } +); + +pub struct ExtBuilder; + +impl Default for ExtBuilder { + fn default() -> Self { + ExtBuilder + } +} + +impl ExtBuilder { + pub fn build(self) -> sp_io::TestExternalities { + let t = frame_system::GenesisConfig::::default() + .build_storage() + .unwrap(); + + t.into() + } +} diff --git a/delay-tasks/src/tests.rs b/delay-tasks/src/tests.rs index 8b1378917..eb1a79f25 100644 --- a/delay-tasks/src/tests.rs +++ b/delay-tasks/src/tests.rs @@ -1 +1,351 @@ +//! Unit tests for the delay tasks. +#![cfg(test)] +use super::*; +use frame_support::{assert_noop, assert_ok}; +use mock::*; +use sp_runtime::traits::{BadOrigin, Bounded}; + +#[test] +fn add_delay_task_work() { + ExtBuilder::default().build().execute_with(|| { + System::set_block_number(1); + assert_eq!(DelayTasks::next_delayed_task_id(), 0); + assert_eq!(DelayTasks::delayed_tasks(0), None); + + assert_noop!( + DelayTasks::add_delay_task(MockDelayedTaskType::Success(SuccessTask), 0), + Error::::InvalidDelayBlock + ); + assert_eq!(PRE_DELAYED.with(|v| *v.borrow()), false); + + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::Success(SuccessTask), + 100 + )); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskAdded { + id: 0, + task: MockDelayedTaskType::Success(SuccessTask), + execute_block: 101, + })); + + assert_eq!(DelayTasks::next_delayed_task_id(), 1); + assert_eq!( + DelayTasks::delayed_tasks(0), + Some((MockDelayedTaskType::Success(SuccessTask), 101)) + ); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); + assert_eq!(PRE_DELAYED.with(|v| *v.borrow()), true); + + reset_delay_process_records(); + assert_noop!( + DelayTasks::add_delay_task(MockDelayedTaskType::FailedPreDelay(FailedPreDelayTask), 200), + DispatchError::Other("pre_delay failed") + ); + assert_eq!(PRE_DELAYED.with(|v| *v.borrow()), false); + }); +} + +#[test] +fn reset_execute_block_work() { + ExtBuilder::default().build().execute_with(|| { + System::set_block_number(1); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::Success(SuccessTask), + 100 + )); + assert_eq!( + DelayTasks::delayed_tasks(0), + Some((MockDelayedTaskType::Success(SuccessTask), 101)) + ); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); + assert_eq!(DelayTasks::delayed_task_queue(10, 0), None); + + assert_noop!( + DelayTasks::reset_execute_block(RuntimeOrigin::signed(1), 0, DispatchTime::At(10)), + BadOrigin + ); + + assert_noop!( + DelayTasks::reset_execute_block(RuntimeOrigin::root(), 1, DispatchTime::At(10)), + Error::::InvalidId + ); + + assert_noop!( + DelayTasks::reset_execute_block(RuntimeOrigin::root(), 0, DispatchTime::After(Bounded::max_value())), + ArithmeticError::Overflow + ); + + assert_ok!(DelayTasks::reset_execute_block( + RuntimeOrigin::root(), + 0, + DispatchTime::At(10) + )); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskReDelayed { + id: 0, + execute_block: 10, + })); + + assert_eq!( + DelayTasks::delayed_tasks(0), + Some((MockDelayedTaskType::Success(SuccessTask), 10)) + ); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), None); + assert_eq!(DelayTasks::delayed_task_queue(10, 0), Some(())); + + System::set_block_number(100); + assert_noop!( + DelayTasks::reset_execute_block(RuntimeOrigin::root(), 0, DispatchTime::At(100)), + Error::::InvalidDelayBlock + ); + + assert_noop!( + DelayTasks::reset_execute_block(RuntimeOrigin::root(), 0, DispatchTime::After(0)), + Error::::InvalidDelayBlock + ); + + assert_ok!(DelayTasks::reset_execute_block( + RuntimeOrigin::root(), + 0, + DispatchTime::After(1) + )); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskReDelayed { + id: 0, + execute_block: 101, + })); + + assert_eq!( + DelayTasks::delayed_tasks(0), + Some((MockDelayedTaskType::Success(SuccessTask), 101)) + ); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); + assert_eq!(DelayTasks::delayed_task_queue(10, 0), None); + }); +} + +#[test] +fn cancel_delayed_task_work() { + ExtBuilder::default().build().execute_with(|| { + System::set_block_number(1); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::Success(SuccessTask), + 100 + )); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), + 200 + )); + + assert_eq!( + DelayTasks::delayed_tasks(0), + Some((MockDelayedTaskType::Success(SuccessTask), 101)) + ); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some((MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), 201)) + ); + assert_eq!(DelayTasks::delayed_task_queue(201, 1), Some(())); + + assert_noop!(DelayTasks::cancel_delayed_task(RuntimeOrigin::signed(1), 0), BadOrigin); + + assert_noop!( + DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 2), + Error::::InvalidId + ); + + assert_ok!(DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 0)); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskCanceled { id: 0 })); + assert_eq!(ON_CANCEL.with(|v| *v.borrow()), true); + assert_eq!(DelayTasks::delayed_tasks(0), None); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), None); + + reset_delay_process_records(); + assert_noop!( + DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 1), + DispatchError::Other("on_cancel failed") + ); + assert_eq!(ON_CANCEL.with(|v| *v.borrow()), false); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some((MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), 201)) + ); + assert_eq!(DelayTasks::delayed_task_queue(201, 1), Some(())); + }); +} + +#[test] +fn do_execute_delayed_task_work() { + ExtBuilder::default().build().execute_with(|| { + System::set_block_number(1); + + assert_noop!(DelayTasks::do_execute_delayed_task(0), Error::::InvalidId); + + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::Success(SuccessTask), + 100 + )); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), + 100 + )); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::FailedDelayedExecute(FailedDelayedExecuteTask), + 100 + )); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), + 100 + )); + + assert_eq!( + DelayTasks::delayed_tasks(0), + Some((MockDelayedTaskType::Success(SuccessTask), 101)) + ); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some(( + MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), + 101 + )) + ); + assert_eq!( + DelayTasks::delayed_tasks(2), + Some((MockDelayedTaskType::FailedDelayedExecute(FailedDelayedExecuteTask), 101)) + ); + assert_eq!( + DelayTasks::delayed_tasks(3), + Some((MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), 101)) + ); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); + assert_eq!(DelayTasks::delayed_task_queue(101, 1), Some(())); + assert_eq!(DelayTasks::delayed_task_queue(101, 2), Some(())); + assert_eq!(DelayTasks::delayed_task_queue(101, 3), Some(())); + + // execute delayed task + reset_delay_process_records(); + assert_ok!(DelayTasks::do_execute_delayed_task(0)); + assert_eq!(PRE_DELAYED_EXECUTED.with(|v| *v.borrow()), true); + assert_eq!(EXECUTED.with(|v| *v.borrow()), true); + assert_eq!(DelayTasks::delayed_tasks(0), None); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); // do_execute_delayed_task doesn't clear delayed_task_queue + + // failed execute delayed task for failed pre_delayed_execute + reset_delay_process_records(); + assert_noop!( + DelayTasks::do_execute_delayed_task(1), + DispatchError::Other("pre_delayed_execute failed") + ); + assert_eq!(PRE_DELAYED_EXECUTED.with(|v| *v.borrow()), false); + assert_eq!(EXECUTED.with(|v| *v.borrow()), false); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some(( + MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), + 101 + )) + ); + assert_eq!(DelayTasks::delayed_task_queue(101, 1), Some(())); // do_execute_delayed_task doesn't clear delayed_task_queue + + // execute delayed task but task failed + reset_delay_process_records(); + assert_eq!( + DelayTasks::do_execute_delayed_task(2), + Ok(Err(DispatchError::Other("delayed_execute failed"))) + ); + assert_eq!(PRE_DELAYED_EXECUTED.with(|v| *v.borrow()), true); + assert_eq!(EXECUTED.with(|v| *v.borrow()), false); + assert_eq!(DelayTasks::delayed_tasks(2), None); + assert_eq!(DelayTasks::delayed_task_queue(101, 2), Some(())); // do_execute_delayed_task doesn't clear delayed_task_queue + + // execute delayed task + reset_delay_process_records(); + assert_ok!(DelayTasks::do_execute_delayed_task(3)); + assert_eq!(PRE_DELAYED_EXECUTED.with(|v| *v.borrow()), true); + assert_eq!(EXECUTED.with(|v| *v.borrow()), true); + assert_eq!(DelayTasks::delayed_tasks(3), None); + assert_eq!(DelayTasks::delayed_task_queue(101, 3), Some(())); // do_execute_delayed_task doesn't clear delayed_task_queue + }); +} + +#[test] +fn on_finalize_work() { + ExtBuilder::default().build().execute_with(|| { + System::set_block_number(1); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::Success(SuccessTask), + 100 + )); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), + 100 + )); + System::set_block_number(2); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::FailedDelayedExecute(FailedDelayedExecuteTask), + 100 + )); + assert_ok!(DelayTasks::add_delay_task( + MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), + 100 + )); + + assert_eq!( + DelayTasks::delayed_tasks(0), + Some((MockDelayedTaskType::Success(SuccessTask), 101)) + ); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some(( + MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), + 101 + )) + ); + assert_eq!( + DelayTasks::delayed_tasks(2), + Some((MockDelayedTaskType::FailedDelayedExecute(FailedDelayedExecuteTask), 102)) + ); + assert_eq!( + DelayTasks::delayed_tasks(3), + Some((MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), 102)) + ); + assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); + assert_eq!(DelayTasks::delayed_task_queue(101, 1), Some(())); + assert_eq!(DelayTasks::delayed_task_queue(102, 2), Some(())); + assert_eq!(DelayTasks::delayed_task_queue(102, 3), Some(())); + + DelayTasks::on_finalize(101); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { + id: 0, + result: Ok(()), + })); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskTryExecuteFailed { + id: 1, + error: DispatchError::Other(""), + })); + assert_eq!(DelayTasks::delayed_tasks(0), None); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some(( + MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), + 101 + )) + ); // keep the storage for failed executed delayed task + assert_eq!(DelayTasks::delayed_task_queue(101, 0), None); + assert_eq!(DelayTasks::delayed_task_queue(101, 1), None); + + DelayTasks::on_finalize(102); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { + id: 2, + result: Err(DispatchError::Other("").into()), + })); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { + id: 3, + result: Ok(()), + })); + assert_eq!(DelayTasks::delayed_tasks(2), None); + assert_eq!(DelayTasks::delayed_tasks(3), None); + assert_eq!(DelayTasks::delayed_task_queue(101, 2), None); + assert_eq!(DelayTasks::delayed_task_queue(101, 3), None); + }); +} diff --git a/traits/src/delay_tasks.rs b/traits/src/delay_tasks.rs index 9255acb99..98ac89243 100644 --- a/traits/src/delay_tasks.rs +++ b/traits/src/delay_tasks.rs @@ -7,8 +7,8 @@ pub trait DelayedTask { fn on_cancel(&self) -> DispatchResult; } -pub trait DelayTasks { - fn schedule_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; +pub trait DelayTasksManager { + fn add_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; } #[macro_export] diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 61ed3e65e..55e9d5b00 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -55,7 +55,7 @@ use xcm_executor::traits::WeightBounds; pub use module::*; use orml_traits::{ - delay_tasks::{DelayTasks, DelayedTask}, + delay_tasks::{DelayTasksManager, DelayedTask}, location::{Parse, Reserve}, xcm_transfer::{Transferred, XtokensWeightInfo}, GetByKey, NamedMultiReservableCurrency, RateLimiter, XcmTransfer, @@ -105,7 +105,7 @@ pub mod module { Fungibility::NonFungible(_) => return Err(Error::::AssetIndexNonExistent.into()), }; - T::Currency::reserve_named(&T::ReserveIdentifier::get(), currency_id, who, amount)?; + T::Currency::reserve_named(&T::ReserveId::get(), currency_id, who, amount)?; } } } @@ -127,7 +127,7 @@ pub mod module { Fungibility::NonFungible(_) => return Err(Error::::AssetIndexNonExistent.into()), }; - T::Currency::unreserve_named(&T::ReserveIdentifier::get(), currency_id, who, amount); + T::Currency::unreserve_named(&T::ReserveId::get(), currency_id, who, amount); } } } @@ -163,8 +163,8 @@ pub mod module { } pub struct DisabledTransferAssets(sp_std::marker::PhantomData); - impl DelayTasks> for DisabledTransferAssets { - fn schedule_delay_task(_task: T::DelayedTask, _delay_blocks: BlockNumberFor) -> DispatchResult { + impl DelayTasksManager> for DisabledTransferAssets { + fn add_delay_task(_task: T::DelayedTask, _delay_blocks: BlockNumberFor) -> DispatchResult { Err(Error::::RateLimited.into()) } } @@ -241,8 +241,9 @@ pub mod module { + TypeInfo + From>; - type DelayTasks: DelayTasks>; + type DelayTasks: DelayTasksManager>; + #[pallet::constant] type DelayBlocks: Get>; type Currency: NamedMultiReservableCurrency< @@ -251,9 +252,7 @@ pub mod module { CurrencyId = Self::CurrencyId, >; - type ReserveIdentifier: Get< - >::ReserveIdentifier, - >; + type ReserveId: Get<>::ReserveIdentifier>; } #[pallet::event] @@ -694,7 +693,7 @@ pub mod module { if delay_check { // delay the transfer assets if limited if Self::transfer_assets_delay_check(&who, assets.clone()).is_err() { - return T::DelayTasks::schedule_delay_task( + return T::DelayTasks::add_delay_task( XtokensDelayedTask::TransferAssets { who: who.clone(), assets: assets.clone(), diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 547bdcb9a..d629fb386 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -267,6 +267,8 @@ impl RateLimiter for MockRateLimiter { parameter_types! { pub const XtokensRateLimiterId: u8 = 0; + pub const GetDelayBlocks: u64 = 1000; + pub const GetReserveId: [u8; 8] = *b"xtokensr"; } define_combined_delayed_task! { @@ -295,6 +297,9 @@ impl orml_xtokens::Config for Runtime { type RateLimiterId = XtokensRateLimiterId; type DelayedTask = DelayedTasks; type DelayTasks = DisabledTransferAssets; + type DelayBlocks = GetDelayBlocks; + type Currency = Tokens; + type ReserveId = GetReserveId; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/para_relative_view.rs b/xtokens/src/mock/para_relative_view.rs index 8b29ff3bd..e236af8a1 100644 --- a/xtokens/src/mock/para_relative_view.rs +++ b/xtokens/src/mock/para_relative_view.rs @@ -316,6 +316,8 @@ impl Convert> for RelativeCurrencyIdConvert { parameter_types! { pub SelfLocation: Location = Location::here(); pub const MaxAssetsForTransfer: usize = 2; + pub const GetDelayBlocks: u64 = 1000; + pub const GetReserveId: [u8; 8] = *b"xtokensr"; } pub struct ParentOrParachains; @@ -371,6 +373,9 @@ impl orml_xtokens::Config for Runtime { type RateLimiterId = (); type DelayedTask = DelayedTasks; type DelayTasks = DisabledTransferAssets; + type DelayBlocks = GetDelayBlocks; + type Currency = Tokens; + type ReserveId = GetReserveId; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/para_teleport.rs b/xtokens/src/mock/para_teleport.rs index ff6b3a401..b50881a7a 100644 --- a/xtokens/src/mock/para_teleport.rs +++ b/xtokens/src/mock/para_teleport.rs @@ -196,6 +196,8 @@ impl Convert for AccountIdToLocation { parameter_types! { pub SelfLocation: Location = Location::new(1, [Parachain(MsgQueue::get().into())]).into(); pub const MaxAssetsForTransfer: usize = 3; + pub const GetDelayBlocks: u64 = 1000; + pub const GetReserveId: [u8; 8] = *b"xtokensr"; } pub struct ParentOrParachains; @@ -240,6 +242,9 @@ impl orml_xtokens::Config for Runtime { type RateLimiterId = (); type DelayedTask = DelayedTasks; type DelayTasks = DisabledTransferAssets; + type DelayBlocks = GetDelayBlocks; + type Currency = Tokens; + type ReserveId = GetReserveId; } impl orml_xcm::Config for Runtime { From 8523a1b004ab683d89c8b7f4f79768dd82cfcc08 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Fri, 15 Mar 2024 02:12:59 +0800 Subject: [PATCH 4/8] rate-limit return delay error --- asset-registry/src/mock/para.rs | 25 ++-- delay-tasks/src/mock.rs | 4 +- rate-limit/src/lib.rs | 81 +++++++++--- rate-limit/src/tests.rs | 52 +++++++- traits/src/delay_tasks.rs | 70 ---------- traits/src/lib.rs | 2 +- traits/src/rate_limit.rs | 20 +-- traits/src/task.rs | 174 +++++++++++++++++++++++++ xtokens/src/lib.rs | 147 ++++++++------------- xtokens/src/mock/para.rs | 34 ++--- xtokens/src/mock/para_relative_view.rs | 21 ++- xtokens/src/mock/para_teleport.rs | 24 ++-- xtokens/src/tests.rs | 2 +- 13 files changed, 395 insertions(+), 261 deletions(-) delete mode 100644 traits/src/delay_tasks.rs create mode 100644 traits/src/task.rs diff --git a/asset-registry/src/mock/para.rs b/asset-registry/src/mock/para.rs index 2ab4699fb..a240abc20 100644 --- a/asset-registry/src/mock/para.rs +++ b/asset-registry/src/mock/para.rs @@ -11,13 +11,14 @@ use frame_support::{ }; use frame_system::{EnsureRoot, EnsureSignedBy}; use orml_traits::{ - define_combined_delayed_task, - delay_tasks::DelayedTask, + define_combined_task, location::{AbsoluteReserveProvider, RelativeReserveProvider}, - parameter_type_with_key, FixedConversionRateProvider, MultiCurrency, + parameter_type_with_key, + task::{DispatchableTask, TaskResult}, + FixedConversionRateProvider, MultiCurrency, }; use orml_xcm_support::{IsNativeConcrete, MultiCurrencyAdapter, MultiNativeAsset}; -use orml_xtokens::XtokensDelayedTask; +use orml_xtokens::XtokensTask; use pallet_xcm::XcmPassthrough; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use polkadot_parachain_primitives::primitives::Sibling; @@ -314,18 +315,13 @@ parameter_type_with_key! { }; } -define_combined_delayed_task! { +define_combined_task! { #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub enum DelayedTasks { - XtokensDelayedTask(XtokensDelayedTask), + Xtokens(XtokensTask), } } -parameter_types! { - pub const GetDelayBlocks: u64 = 1000; - pub const GetReserveId: [u8; 8] = *b"xtokensr"; -} - impl orml_xtokens::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; @@ -343,11 +339,8 @@ impl orml_xtokens::Config for Runtime { type ReserveProvider = RelativeReserveProvider; type RateLimiter = (); type RateLimiterId = (); - type DelayedTask = DelayedTasks; - type DelayTasks = orml_xtokens::DisabledTransferAssets; - type DelayBlocks = GetDelayBlocks; - type Currency = Tokens; - type ReserveId = GetReserveId; + type Task = (); + type DelayTasks = orml_xtokens::DisabledDelayTask; } impl orml_xcm::Config for Runtime { diff --git a/delay-tasks/src/mock.rs b/delay-tasks/src/mock.rs index a5b14c1c8..fbb722f4a 100644 --- a/delay-tasks/src/mock.rs +++ b/delay-tasks/src/mock.rs @@ -5,7 +5,7 @@ use super::*; use frame_support::{construct_runtime, derive_impl}; use frame_system::EnsureRoot; -use orml_traits::define_combined_delayed_task; +use orml_traits::define_combined_task; use sp_runtime::{traits::IdentityLookup, BuildStorage, DispatchError}; use sp_std::cell::RefCell; @@ -35,7 +35,7 @@ pub(crate) fn reset_delay_process_records() { ON_CANCEL.with(|v| *v.borrow_mut() = false); } -define_combined_delayed_task! { +define_combined_task! { #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub enum MockDelayedTaskType { Success(SuccessTask), diff --git a/rate-limit/src/lib.rs b/rate-limit/src/lib.rs index 50d49dcbe..33b2656b4 100644 --- a/rate-limit/src/lib.rs +++ b/rate-limit/src/lib.rs @@ -112,6 +112,8 @@ pub mod module { /// Exceed the allowed maximum number of KeyFilter configured to a /// RateLimiterId. MaxFilterExceeded, + /// Delay block must not be zero. + ZeroDelayBlock, } #[pallet::event] @@ -129,6 +131,10 @@ pub mod module { WhitelistFilterRemoved { rate_limiter_id: T::RateLimiterId }, /// The whitelist of bypass rate limit has been reset. WhitelistFilterReset { rate_limiter_id: T::RateLimiterId }, + AllowDelayBlockChanged { + rate_limiter_id: T::RateLimiterId, + update: Option>, + }, } /// The rate limit rule for specific RateLimiterId and encoded key. @@ -156,6 +162,12 @@ pub mod module { pub type LimitWhitelist = StorageMap<_, Twox64Concat, T::RateLimiterId, OrderedSet, ValueQuery>; + /// Allow delay (not consume quota) when limit check for specific + /// RateLimiterId + #[pallet::storage] + #[pallet::getter(fn allow_delay)] + pub type AllowDelay = StorageMap<_, Twox64Concat, T::RateLimiterId, BlockNumberFor, OptionQuery>; + #[pallet::pallet] #[pallet::without_storage_info] pub struct Pallet(_); @@ -317,6 +329,37 @@ pub mod module { Self::deposit_event(Event::WhitelistFilterReset { rate_limiter_id }); Ok(()) } + + /// Allow delayed when limit check. + /// + /// Requires `GovernanceOrigin` + /// + /// Parameters: + /// - `rate_limiter_id`: rate limiter id. + /// - `allow_delayed_block`: allow delay when limit check. + #[pallet::call_index(4)] + #[pallet::weight(T::WeightInfo::reset_whitelist())] + #[transactional] + pub fn allow_delay_block( + origin: OriginFor, + rate_limiter_id: T::RateLimiterId, + allow_delayed_block: Option>, + ) -> DispatchResult { + T::GovernanceOrigin::ensure_origin(origin)?; + + if let Some(dely_block) = allow_delayed_block { + ensure!(!dely_block.is_zero(), Error::::ZeroDelayBlock); + AllowDelay::::insert(rate_limiter_id, dely_block); + } else { + AllowDelay::::remove(rate_limiter_id); + } + + Self::deposit_event(Event::AllowDelayBlockChanged { + rate_limiter_id, + update: allow_delayed_block, + }); + Ok(()) + } } impl Pallet { @@ -381,7 +424,7 @@ pub mod module { } } - impl RateLimiter for Pallet { + impl RateLimiter> for Pallet { type RateLimiterId = T::RateLimiterId; fn is_whitelist(limiter_id: Self::RateLimiterId, key: impl Encode) -> bool { @@ -410,31 +453,33 @@ pub mod module { false } - fn can_consume(limiter_id: Self::RateLimiterId, key: impl Encode, value: u128) -> Result<(), RateLimiterError> { + fn can_consume( + limiter_id: Self::RateLimiterId, + key: impl Encode, + value: u128, + ) -> Result<(), RateLimiterError>> { let encoded_key: Vec = key.encode(); - let allowed = match RateLimitRules::::get(limiter_id, &encoded_key) { + match RateLimitRules::::get(limiter_id, &encoded_key) { Some(rate_limit_rule @ RateLimitRule::PerPeriod { .. }) | Some(rate_limit_rule @ RateLimitRule::TokenBucket { .. }) => { let remainer_quota = Self::access_remainer_quota_after_update(rate_limit_rule, &limiter_id, &encoded_key); - value <= remainer_quota - } - Some(RateLimitRule::Unlimited) => true, - Some(RateLimitRule::NotAllowed) => { - // always return false, even if the value is zero. - false - } - None => { - // if doesn't rate limit rule, always return true. - true - } - }; - - ensure!(allowed, RateLimiterError::ExceedLimit); + if value > remainer_quota { + if let Some(delay_block) = AllowDelay::::get(limiter_id) { + return Err(RateLimiterError::Delay { duration: delay_block }); + } else { + return Err(RateLimiterError::Deny); + } + } - Ok(()) + Ok(()) + } + Some(RateLimitRule::Unlimited) => Ok(()), + Some(RateLimitRule::NotAllowed) => Err(RateLimiterError::Deny), + None => Ok(()), + } } fn consume(limiter_id: Self::RateLimiterId, key: impl Encode, value: u128) { diff --git a/rate-limit/src/tests.rs b/rate-limit/src/tests.rs index 4a4817b55..08eb9d041 100644 --- a/rate-limit/src/tests.rs +++ b/rate-limit/src/tests.rs @@ -392,6 +392,37 @@ fn reset_whitelist_work() { }); } +#[test] +fn allow_delay_block_work() { + ExtBuilder::default().build().execute_with(|| { + assert_noop!( + RateLimit::allow_delay_block(RuntimeOrigin::signed(ALICE), 0, Some(1),), + BadOrigin + ); + + // delay block is zero + assert_noop!( + RateLimit::allow_delay_block(RuntimeOrigin::root(), 0, Some(0),), + Error::::ZeroDelayBlock + ); + + assert_eq!(RateLimit::allow_delay(0), None); + assert_ok!(RateLimit::allow_delay_block(RuntimeOrigin::root(), 0, Some(100),)); + System::assert_last_event(RuntimeEvent::RateLimit(crate::Event::AllowDelayBlockChanged { + rate_limiter_id: 0, + update: Some(100), + })); + assert_eq!(RateLimit::allow_delay(0), Some(100)); + + assert_ok!(RateLimit::allow_delay_block(RuntimeOrigin::root(), 0, None,)); + System::assert_last_event(RuntimeEvent::RateLimit(crate::Event::AllowDelayBlockChanged { + rate_limiter_id: 0, + update: None, + })); + assert_eq!(RateLimit::allow_delay(0), None); + }); +} + #[test] fn is_whitelist_work() { ExtBuilder::default().build().execute_with(|| { @@ -799,12 +830,12 @@ fn can_consume_work() { assert_eq!(RateLimit::rate_limit_quota(0, DOT.encode()), (0, 0)); assert_ok!(RateLimit::can_consume(0, DOT, 0)); - assert_eq!(RateLimit::can_consume(0, DOT, 500), Err(RateLimiterError::ExceedLimit),); + assert_eq!(RateLimit::can_consume(0, DOT, 500), Err(RateLimiterError::Deny),); assert_eq!(RateLimit::rate_limit_quota(0, DOT.encode()), (0, 0)); assert_eq!(RateLimit::rate_limit_quota(1, DOT.encode()), (0, 0)); assert_ok!(RateLimit::can_consume(1, DOT, 0)); - assert_eq!(RateLimit::can_consume(1, DOT, 501), Err(RateLimiterError::ExceedLimit),); + assert_eq!(RateLimit::can_consume(1, DOT, 501), Err(RateLimiterError::Deny),); assert_eq!(RateLimit::rate_limit_quota(1, DOT.encode()), (0, 0)); System::set_block_number(100); @@ -813,20 +844,29 @@ fn can_consume_work() { assert_eq!(RateLimit::rate_limit_quota(0, DOT.encode()), (0, 0)); assert_ok!(RateLimit::can_consume(0, DOT, 500)); assert_eq!(RateLimit::rate_limit_quota(0, DOT.encode()), (100, 500)); - assert_eq!(RateLimit::can_consume(0, DOT, 501), Err(RateLimiterError::ExceedLimit),); + assert_eq!(RateLimit::can_consume(0, DOT, 501), Err(RateLimiterError::Deny),); assert_eq!(RateLimit::rate_limit_quota(0, DOT.encode()), (100, 500)); assert_eq!(RateLimit::rate_limit_quota(1, DOT.encode()), (0, 0)); assert_ok!(RateLimit::can_consume(1, DOT, 501)); assert_eq!(RateLimit::rate_limit_quota(1, DOT.encode()), (100, 1000)); - assert_eq!(RateLimit::can_consume(1, DOT, 1001), Err(RateLimiterError::ExceedLimit),); + assert_eq!(RateLimit::can_consume(1, DOT, 1001), Err(RateLimiterError::Deny),); assert_eq!(RateLimit::rate_limit_quota(1, DOT.encode()), (100, 1000)); + // if config allow delay for rate limit id 0, return RateLimiterError::Delay + assert_ok!(RateLimit::allow_delay_block(RuntimeOrigin::root(), 0, Some(10000),)); + assert_eq!(RateLimit::rate_limit_quota(0, DOT.encode()), (100, 500)); + assert_ok!(RateLimit::can_consume(0, DOT, 500)); + assert_eq!( + RateLimit::can_consume(0, DOT, 501), + Err(RateLimiterError::Delay { duration: 10000 }) + ); + // NotAllowed always return error, even if value is 0 RateLimitQuota::::mutate(0, BTC.encode(), |(_, remainer_quota)| *remainer_quota = 10000); assert_eq!(RateLimit::rate_limit_quota(0, BTC.encode()), (0, 10000)); - assert_eq!(RateLimit::can_consume(0, BTC, 0), Err(RateLimiterError::ExceedLimit),); - assert_eq!(RateLimit::can_consume(0, BTC, 100), Err(RateLimiterError::ExceedLimit),); + assert_eq!(RateLimit::can_consume(0, BTC, 0), Err(RateLimiterError::Deny),); + assert_eq!(RateLimit::can_consume(0, BTC, 100), Err(RateLimiterError::Deny),); // Unlimited always return true assert_eq!(RateLimit::rate_limit_quota(1, BTC.encode()), (0, 0)); diff --git a/traits/src/delay_tasks.rs b/traits/src/delay_tasks.rs deleted file mode 100644 index 98ac89243..000000000 --- a/traits/src/delay_tasks.rs +++ /dev/null @@ -1,70 +0,0 @@ -use sp_runtime::DispatchResult; - -pub trait DelayedTask { - fn pre_delay(&self) -> DispatchResult; - fn pre_delayed_execute(&self) -> DispatchResult; - fn delayed_execute(&self) -> DispatchResult; - fn on_cancel(&self) -> DispatchResult; -} - -pub trait DelayTasksManager { - fn add_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; -} - -#[macro_export] -macro_rules! define_combined_delayed_task { - ( - $(#[$meta:meta])* - $vis:vis enum $combined_name:ident { - $( - $task:ident ( $vtask:ident $(<$($generic:tt),*>)? ) - ),+ $(,)? - } - ) => { - $(#[$meta])* - $vis enum $combined_name { - $( - $task($vtask $(<$($generic),*>)?), - )* - } - - impl DelayedTask for $combined_name { - fn pre_delay(&self) -> DispatchResult { - match self { - $( - $combined_name::$task(t) => t.pre_delay(), - )* - } - } - fn pre_delayed_execute(&self) -> DispatchResult { - match self { - $( - $combined_name::$task(t) => t.pre_delayed_execute(), - )* - } - } - fn delayed_execute(&self) -> DispatchResult { - match self { - $( - $combined_name::$task(t) => t.delayed_execute(), - )* - } - } - fn on_cancel(&self) -> DispatchResult { - match self { - $( - $combined_name::$task(t) => t.on_cancel(), - )* - } - } - } - - $( - impl From<$vtask $(<$($generic),*>)?> for $combined_name { - fn from(t: $vtask $(<$($generic),*>)?) -> Self{ - $combined_name::$task(t) - } - } - )* - }; -} diff --git a/traits/src/lib.rs b/traits/src/lib.rs index aba54777d..63c3453ae 100644 --- a/traits/src/lib.rs +++ b/traits/src/lib.rs @@ -32,7 +32,6 @@ pub mod asset_registry; pub mod auction; pub mod currency; pub mod data_provider; -pub mod delay_tasks; pub mod get_by_key; pub mod location; pub mod multi_asset; @@ -41,6 +40,7 @@ pub mod parameters; pub mod price; pub mod rate_limit; pub mod rewards; +pub mod task; pub mod xcm_transfer; /// New data handler diff --git a/traits/src/rate_limit.rs b/traits/src/rate_limit.rs index 2abac8363..8e896fbb7 100644 --- a/traits/src/rate_limit.rs +++ b/traits/src/rate_limit.rs @@ -3,12 +3,13 @@ use parity_scale_codec::Encode; use sp_runtime::{traits::Member, RuntimeDebug}; #[derive(PartialEq, Eq, RuntimeDebug)] -pub enum RateLimiterError { - ExceedLimit, +pub enum RateLimiterError { + Deny, + Delay { duration: Duration }, } /// Rate Limiter -pub trait RateLimiter { +pub trait RateLimiter { /// The type for the rate limiter. type RateLimiterId: Parameter + Member + Copy; @@ -22,7 +23,7 @@ pub trait RateLimiter { limiter_id: Self::RateLimiterId, limit_key: impl Encode, value: u128, - ) -> Result<(), RateLimiterError>; + ) -> Result<(), RateLimiterError>; /// The handler function to consume quota. fn consume(limiter_id: Self::RateLimiterId, limit_key: impl Encode, value: u128); @@ -33,29 +34,30 @@ pub trait RateLimiter { limit_key: impl Encode + Clone, value: u128, whitelist_check: Option, - ) -> Result<(), RateLimiterError> { + ) -> Result<(), RateLimiterError> { let need_consume = match whitelist_check { Some(whitelist_key) => !Self::is_whitelist(limiter_id, whitelist_key), None => true, }; if need_consume { - Self::can_consume(limiter_id, limit_key.clone(), value)?; - Self::consume(limiter_id, limit_key, value); + Self::can_consume(limiter_id, limit_key.clone(), value).map(|_| { + Self::consume(limiter_id, limit_key, value); + })?; } Ok(()) } } -impl RateLimiter for () { +impl RateLimiter for () { type RateLimiterId = (); fn is_whitelist(_: Self::RateLimiterId, _: impl Encode) -> bool { true } - fn can_consume(_: Self::RateLimiterId, _: impl Encode, _: u128) -> Result<(), RateLimiterError> { + fn can_consume(_: Self::RateLimiterId, _: impl Encode, _: u128) -> Result<(), RateLimiterError> { Ok(()) } diff --git a/traits/src/task.rs b/traits/src/task.rs new file mode 100644 index 000000000..d922fe01d --- /dev/null +++ b/traits/src/task.rs @@ -0,0 +1,174 @@ +use frame_support::weights::Weight; +use parity_scale_codec::{Decode, Encode}; +use scale_info::TypeInfo; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; +use sp_runtime::DispatchResult; +use sp_runtime::RuntimeDebug; + +#[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub struct TaskResult { + pub result: DispatchResult, + pub used_weight: Weight, + pub finished: bool, +} + +/// Dispatchable tasks +pub trait DispatchableTask { + fn dispatch(self, weight: Weight) -> TaskResult; +} + +#[cfg(feature = "std")] +impl DispatchableTask for () { + fn dispatch(self, _weight: Weight) -> TaskResult { + unimplemented!() + } +} + +#[macro_export] +macro_rules! define_combined_task { + ( + $(#[$meta:meta])* + $vis:vis enum $combined_name:ident { + $( + $task:ident ( $vtask:ident $(<$($generic:tt),*>)? ) + ),+ $(,)? + } + ) => { + $(#[$meta])* + $vis enum $combined_name { + $( + $task($vtask $(<$($generic),*>)?), + )* + } + + impl DispatchableTask for $combined_name { + fn dispatch(self, weight: Weight) -> TaskResult { + match self { + $( + $combined_name::$task(t) => t.dispatch(weight), + )* + } + } + } + + $( + impl From<$vtask $(<$($generic),*>)?> for $combined_name { + fn from(t: $vtask $(<$($generic),*>)?) -> Self{ + $combined_name::$task(t) + } + } + )* + }; +} + +pub trait DelayTasksManager { + fn add_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; +} + +// pub trait DelayedTask { +// fn pre_delay(&self) -> DispatchResult; +// fn pre_delayed_execute(&self) -> DispatchResult; +// fn delayed_execute(&self) -> DispatchResult; +// fn on_cancel(&self) -> DispatchResult; +// } + +// pub trait DelayTasksManager { +// fn add_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; +// } + +// #[macro_export] +// macro_rules! define_combined_task { +// ( +// $(#[$meta:meta])* +// $vis:vis enum $combined_name:ident { +// $( +// $task:ident ( $vtask:ident $(<$($generic:tt),*>)? ) +// ),+ $(,)? +// } +// ) => { +// $(#[$meta])* +// $vis enum $combined_name { +// $( +// $task($vtask $(<$($generic),*>)?), +// )* +// } + +// impl DispatchableTask for $combined_name { +// fn dispatch(self, weight: Weight) -> TaskResult { +// match self { +// $( +// $combined_name::$task(t) => t.dispatch(weight), +// )* +// } +// } +// } + +// $( +// impl From<$vtask $(<$($generic),*>)?> for $combined_name { +// fn from(t: $vtask $(<$($generic),*>)?) -> Self{ +// $combined_name::$task(t) +// } +// } +// )* +// }; +// } + +// #[macro_export] +// macro_rules! define_combined_delayed_task { +// ( +// $(#[$meta:meta])* +// $vis:vis enum $combined_name:ident { +// $( +// $task:ident ( $vtask:ident $(<$($generic:tt),*>)? ) +// ),+ $(,)? +// } +// ) => { +// $(#[$meta])* +// $vis enum $combined_name { +// $( +// $task($vtask $(<$($generic),*>)?), +// )* +// } + +// impl DelayedTask for $combined_name { +// fn pre_delay(&self) -> DispatchResult { +// match self { +// $( +// $combined_name::$task(t) => t.pre_delay(), +// )* +// } +// } +// fn pre_delayed_execute(&self) -> DispatchResult { +// match self { +// $( +// $combined_name::$task(t) => t.pre_delayed_execute(), +// )* +// } +// } +// fn delayed_execute(&self) -> DispatchResult { +// match self { +// $( +// $combined_name::$task(t) => t.delayed_execute(), +// )* +// } +// } +// fn on_cancel(&self) -> DispatchResult { +// match self { +// $( +// $combined_name::$task(t) => t.on_cancel(), +// )* +// } +// } +// } + +// $( +// impl From<$vtask $(<$($generic),*>)?> for $combined_name { +// fn from(t: $vtask $(<$($generic),*>)?) -> Self{ +// $combined_name::$task(t) +// } +// } +// )* +// }; +// } diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 55e9d5b00..0eadb4945 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -55,10 +55,10 @@ use xcm_executor::traits::WeightBounds; pub use module::*; use orml_traits::{ - delay_tasks::{DelayTasksManager, DelayedTask}, location::{Parse, Reserve}, + task::{DelayTasksManager, DispatchableTask, TaskResult}, xcm_transfer::{Transferred, XtokensWeightInfo}, - GetByKey, NamedMultiReservableCurrency, RateLimiter, XcmTransfer, + GetByKey, RateLimiter, RateLimiterError, XcmTransfer, }; mod mock; @@ -79,7 +79,7 @@ pub mod module { use super::*; #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] - pub enum XtokensDelayedTask { + pub enum XtokensTask { TransferAssets { who: T::AccountId, assets: Assets, @@ -89,54 +89,17 @@ pub mod module { }, } - impl DelayedTask for XtokensDelayedTask { - fn pre_delay(&self) -> DispatchResult { - match self { - XtokensDelayedTask::TransferAssets { who, assets, .. } => { - let asset_len = assets.len(); - for i in 0..asset_len { - let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; - let currency_id: T::CurrencyId = T::CurrencyIdConvert::convert(asset.id.0.clone()) - .ok_or(Error::::AssetIndexNonExistent)?; - let amount: T::Balance = match asset.fun { - Fungibility::Fungible(amount) => { - amount.try_into().map_err(|_| Error::::AssetIndexNonExistent)? - } - Fungibility::NonFungible(_) => return Err(Error::::AssetIndexNonExistent.into()), - }; - - T::Currency::reserve_named(&T::ReserveId::get(), currency_id, who, amount)?; - } - } - } - Ok(()) - } - - fn pre_delayed_execute(&self) -> DispatchResult { - match self { - XtokensDelayedTask::TransferAssets { who, assets, .. } => { - let asset_len = assets.len(); - for i in 0..asset_len { - let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; - let currency_id: T::CurrencyId = T::CurrencyIdConvert::convert(asset.id.0.clone()) - .ok_or(Error::::AssetIndexNonExistent)?; - let amount: T::Balance = match asset.fun { - Fungibility::Fungible(amount) => { - amount.try_into().map_err(|_| Error::::AssetIndexNonExistent)? - } - Fungibility::NonFungible(_) => return Err(Error::::AssetIndexNonExistent.into()), - }; - - T::Currency::unreserve_named(&T::ReserveId::get(), currency_id, who, amount); - } - } - } - Ok(()) + #[cfg(feature = "std")] + impl From> for () { + fn from(_task: XtokensTask) -> Self { + unimplemented!() } + } - fn delayed_execute(&self) -> DispatchResult { + impl DispatchableTask for XtokensTask { + fn dispatch(self, weight: Weight) -> TaskResult { match self { - XtokensDelayedTask::TransferAssets { + XtokensTask::TransferAssets { who, assets, fee, @@ -144,28 +107,23 @@ pub mod module { dest_weight_limit, } => { // execute `do_transfer_assets` without delay check - Pallet::::do_transfer_assets( - who.clone(), - assets.clone(), - fee.clone(), - dest.clone(), - dest_weight_limit.clone(), - false, - ) - .map(|_| ()) + let result = + Pallet::::do_transfer_assets(who, assets, fee, dest, dest_weight_limit, false).map(|_| ()); + + TaskResult { + result, + used_weight: weight, // TODO: update + finished: true, + } } } } - - fn on_cancel(&self) -> DispatchResult { - self.pre_delayed_execute() - } } - pub struct DisabledTransferAssets(sp_std::marker::PhantomData); - impl DelayTasksManager> for DisabledTransferAssets { - fn add_delay_task(_task: T::DelayedTask, _delay_blocks: BlockNumberFor) -> DispatchResult { - Err(Error::::RateLimited.into()) + pub struct DisabledDelayTask(sp_std::marker::PhantomData); + impl DelayTasksManager> for DisabledDelayTask { + fn add_delay_task(_task: T::Task, _delay_blocks: BlockNumberFor) -> DispatchResult { + Err(Error::::RateLimiterDeny.into()) } } @@ -227,32 +185,16 @@ pub mod module { type ReserveProvider: Reserve; /// The rate limiter used to limit the cross-chain transfer asset. - type RateLimiter: RateLimiter; + type RateLimiter: RateLimiter>; /// The id of the RateLimiter. #[pallet::constant] - type RateLimiterId: Get<::RateLimiterId>; - - type DelayedTask: DelayedTask - + FullCodec - + Debug - + Clone - + PartialEq - + TypeInfo - + From>; - - type DelayTasks: DelayTasksManager>; - - #[pallet::constant] - type DelayBlocks: Get>; + type RateLimiterId: Get<>>::RateLimiterId>; - type Currency: NamedMultiReservableCurrency< - Self::AccountId, - Balance = Self::Balance, - CurrencyId = Self::CurrencyId, - >; + /// Dispatchable tasks + type Task: DispatchableTask + FullCodec + Debug + Clone + PartialEq + TypeInfo + From>; - type ReserveId: Get<>::ReserveIdentifier>; + type DelayTasks: DelayTasksManager>; } #[pallet::event] @@ -310,8 +252,8 @@ pub mod module { NotSupportedLocation, /// MinXcmFee not registered for certain reserve location MinXcmFeeNotDefined, - /// Asset transfer is limited by RateLimiter. - RateLimited, + /// Asset transfer is denied by RateLimiter. + RateLimiterDeny, } #[pallet::hooks] @@ -516,9 +458,14 @@ pub mod module { impl Pallet { #[transactional] - fn transfer_assets_delay_check(who: &T::AccountId, assets: Assets) -> DispatchResult { + fn transfer_assets_delay_check( + who: &T::AccountId, + assets: Assets, + ) -> Result>, DispatchError> { let rate_limiter_id = T::RateLimiterId::get(); let asset_len = assets.len(); + + let mut need_delay: Option> = None; for i in 0..asset_len { let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; @@ -530,11 +477,20 @@ pub mod module { // try consume quota of the rate limiter. // NOTE: use AssetId as the key, use AccountId as whitelist filter key. - T::RateLimiter::try_consume(rate_limiter_id, asset.id.clone(), amount, Some(who)) - .map_err(|_| Error::::RateLimited)?; + match T::RateLimiter::try_consume(rate_limiter_id, asset.id.clone(), amount, Some(who)) { + Ok(_) => {} + Err(_e @ RateLimiterError::Deny) => { + return Err(Error::::RateLimiterDeny.into()); + } + Err(ref _e @ RateLimiterError::Delay { duration }) => { + if duration > need_delay.unwrap_or_default() { + need_delay = Some(duration); + } + } + }; } - Ok(()) + Ok(need_delay) } fn do_transfer( @@ -691,10 +647,9 @@ pub mod module { } if delay_check { - // delay the transfer assets if limited - if Self::transfer_assets_delay_check(&who, assets.clone()).is_err() { + if let Some(delay_block) = Self::transfer_assets_delay_check(&who, assets.clone())? { return T::DelayTasks::add_delay_task( - XtokensDelayedTask::TransferAssets { + XtokensTask::TransferAssets { who: who.clone(), assets: assets.clone(), fee: fee.clone(), @@ -702,7 +657,7 @@ pub mod module { dest_weight_limit, } .into(), - T::DelayBlocks::get(), + delay_block, ) .map(|_| Transferred { sender: who, diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index d629fb386..9c59a4a70 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -1,6 +1,6 @@ use super::{ - AllowTopLevelPaidExecution, Amount, Balance, CurrencyId, CurrencyIdConvert, DisabledTransferAssets, - ParachainXcmRouter, RateLimiter, XtokensDelayedTask, CHARLIE, + AllowTopLevelPaidExecution, Amount, Balance, BlockNumberFor, CurrencyId, CurrencyIdConvert, DisabledDelayTask, + ParachainXcmRouter, RateLimiter, XtokensTask, CHARLIE, }; use crate as orml_xtokens; @@ -15,7 +15,7 @@ use polkadot_parachain_primitives::primitives::Sibling; use scale_info::TypeInfo; use sp_runtime::{ traits::{Convert, IdentityLookup}, - AccountId32, DispatchResult, RuntimeDebug, + AccountId32, RuntimeDebug, }; use sp_std::cell::RefCell; use xcm::v4::{prelude::*, Weight}; @@ -28,7 +28,10 @@ use xcm_executor::{Config, XcmExecutor}; use crate::mock::AllTokensAreCreatedEqualToWeight; use orml_traits::{ - define_combined_delayed_task, delay_tasks::DelayedTask, location::AbsoluteReserveProvider, parameter_type_with_key, + define_combined_task, + location::AbsoluteReserveProvider, + parameter_type_with_key, + task::{DispatchableTask, TaskResult}, RateLimiterError, }; use orml_xcm_support::{IsNativeConcrete, MultiCurrencyAdapter, MultiNativeAsset}; @@ -230,7 +233,7 @@ thread_local! { } pub struct MockRateLimiter; -impl RateLimiter for MockRateLimiter { +impl RateLimiter> for MockRateLimiter { type RateLimiterId = u8; fn is_whitelist(_: Self::RateLimiterId, key: impl Encode) -> bool { @@ -239,7 +242,11 @@ impl RateLimiter for MockRateLimiter { encoded_key != encoded_charlie } - fn can_consume(_: Self::RateLimiterId, limit_key: impl Encode, value: u128) -> Result<(), RateLimiterError> { + fn can_consume( + _: Self::RateLimiterId, + limit_key: impl Encode, + value: u128, + ) -> Result<(), RateLimiterError>> { let encoded_limit_key = limit_key.encode(); let r_multi_location: Location = CurrencyIdConvert::convert(CurrencyId::R).unwrap(); let r_asset_id = AssetId(r_multi_location); @@ -247,7 +254,7 @@ impl RateLimiter for MockRateLimiter { if encoded_limit_key == encoded_r_asset_id { let accumulation = R_ACCUMULATION.with(|v| *v.borrow()); - ensure!((accumulation + value) <= 2000, RateLimiterError::ExceedLimit); + ensure!((accumulation + value) <= 2000, RateLimiterError::Deny); } Ok(()) @@ -267,14 +274,12 @@ impl RateLimiter for MockRateLimiter { parameter_types! { pub const XtokensRateLimiterId: u8 = 0; - pub const GetDelayBlocks: u64 = 1000; - pub const GetReserveId: [u8; 8] = *b"xtokensr"; } -define_combined_delayed_task! { +define_combined_task! { #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub enum DelayedTasks { - XtokensDelayedTask(XtokensDelayedTask), + Xtokens(XtokensTask), } } @@ -295,11 +300,8 @@ impl orml_xtokens::Config for Runtime { type ReserveProvider = AbsoluteReserveProvider; type RateLimiter = MockRateLimiter; type RateLimiterId = XtokensRateLimiterId; - type DelayedTask = DelayedTasks; - type DelayTasks = DisabledTransferAssets; - type DelayBlocks = GetDelayBlocks; - type Currency = Tokens; - type ReserveId = GetReserveId; + type Task = DelayedTasks; + type DelayTasks = DisabledDelayTask; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/para_relative_view.rs b/xtokens/src/mock/para_relative_view.rs index e236af8a1..9f8dab9cd 100644 --- a/xtokens/src/mock/para_relative_view.rs +++ b/xtokens/src/mock/para_relative_view.rs @@ -1,6 +1,4 @@ -use super::{ - Amount, Balance, CurrencyId, CurrencyIdConvert, DisabledTransferAssets, ParachainXcmRouter, XtokensDelayedTask, -}; +use super::{Amount, Balance, CurrencyId, CurrencyIdConvert, DisabledDelayTask, ParachainXcmRouter, XtokensTask}; use crate as orml_xtokens; use frame_support::{ @@ -14,7 +12,7 @@ use polkadot_parachain_primitives::primitives::Sibling; use scale_info::TypeInfo; use sp_runtime::{ traits::{Convert, IdentityLookup}, - AccountId32, BoundedVec, DispatchResult, RuntimeDebug, + AccountId32, BoundedVec, RuntimeDebug, }; use xcm::v4::{prelude::*, Weight}; use xcm_builder::{ @@ -26,10 +24,10 @@ use xcm_executor::{Config, XcmExecutor}; use crate::mock::AllTokensAreCreatedEqualToWeight; use orml_traits::{ - define_combined_delayed_task, - delay_tasks::DelayedTask, + define_combined_task, location::{AbsoluteReserveProvider, RelativeReserveProvider}, parameter_type_with_key, + task::{DispatchableTask, TaskResult}, }; use orml_xcm_support::{IsNativeConcrete, MultiCurrencyAdapter, MultiNativeAsset}; @@ -347,10 +345,10 @@ parameter_type_with_key! { }; } -define_combined_delayed_task! { +define_combined_task! { #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub enum DelayedTasks { - XtokensDelayedTask(XtokensDelayedTask), + Xtokens(XtokensTask), } } @@ -371,11 +369,8 @@ impl orml_xtokens::Config for Runtime { type ReserveProvider = RelativeReserveProvider; type RateLimiter = (); type RateLimiterId = (); - type DelayedTask = DelayedTasks; - type DelayTasks = DisabledTransferAssets; - type DelayBlocks = GetDelayBlocks; - type Currency = Tokens; - type ReserveId = GetReserveId; + type Task = (); + type DelayTasks = DisabledDelayTask; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/mock/para_teleport.rs b/xtokens/src/mock/para_teleport.rs index b50881a7a..822f600ba 100644 --- a/xtokens/src/mock/para_teleport.rs +++ b/xtokens/src/mock/para_teleport.rs @@ -1,6 +1,6 @@ use super::{ - AllowTopLevelPaidExecution, Amount, Balance, CurrencyId, CurrencyIdConvert, DisabledTransferAssets, - ParachainXcmRouter, XtokensDelayedTask, + AllowTopLevelPaidExecution, Amount, Balance, CurrencyId, CurrencyIdConvert, DisabledDelayTask, ParachainXcmRouter, + XtokensTask, }; use crate as orml_xtokens; @@ -15,7 +15,7 @@ use polkadot_parachain_primitives::primitives::Sibling; use scale_info::TypeInfo; use sp_runtime::{ traits::{Convert, IdentityLookup}, - AccountId32, DispatchResult, RuntimeDebug, + AccountId32, RuntimeDebug, }; use xcm::v4::{prelude::*, Weight}; use xcm_builder::{ @@ -28,7 +28,10 @@ use xcm_executor::{Config, XcmExecutor}; use crate::mock::teleport_currency_adapter::MultiTeleportCurrencyAdapter; use crate::mock::AllTokensAreCreatedEqualToWeight; use orml_traits::{ - define_combined_delayed_task, delay_tasks::DelayedTask, location::AbsoluteReserveProvider, parameter_type_with_key, + define_combined_task, + location::AbsoluteReserveProvider, + parameter_type_with_key, + task::{DispatchableTask, TaskResult}, }; use orml_xcm_support::{DisabledParachainFee, IsNativeConcrete, MultiNativeAsset}; @@ -196,8 +199,6 @@ impl Convert for AccountIdToLocation { parameter_types! { pub SelfLocation: Location = Location::new(1, [Parachain(MsgQueue::get().into())]).into(); pub const MaxAssetsForTransfer: usize = 3; - pub const GetDelayBlocks: u64 = 1000; - pub const GetReserveId: [u8; 8] = *b"xtokensr"; } pub struct ParentOrParachains; @@ -216,10 +217,10 @@ impl Contains for ParentOrParachains { } } -define_combined_delayed_task! { +define_combined_task! { #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub enum DelayedTasks { - XtokensDelayedTask(XtokensDelayedTask), + Xtokens(XtokensTask), } } @@ -240,11 +241,8 @@ impl orml_xtokens::Config for Runtime { type ReserveProvider = AbsoluteReserveProvider; type RateLimiter = (); type RateLimiterId = (); - type DelayedTask = DelayedTasks; - type DelayTasks = DisabledTransferAssets; - type DelayBlocks = GetDelayBlocks; - type Currency = Tokens; - type ReserveId = GetReserveId; + type Task = (); + type DelayTasks = DisabledDelayTask; } impl orml_xcm::Config for Runtime { diff --git a/xtokens/src/tests.rs b/xtokens/src/tests.rs index 5f2e08b15..3060a452c 100644 --- a/xtokens/src/tests.rs +++ b/xtokens/src/tests.rs @@ -1780,7 +1780,7 @@ fn send_relay_chain_asset_to_relay_chain_at_rate_limit() { ), WeightLimit::Unlimited ), - Error::::RateLimited + Error::::RateLimiterDeny ); assert_eq!(ParaTokens::free_balance(CurrencyId::R, &CHARLIE), 1200); assert_eq!(R_ACCUMULATION.with(|v| *v.borrow()), 1800); From ffd4ebbea34a1247c984ace118fef2ba692b9e6b Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Tue, 19 Mar 2024 01:31:15 +0800 Subject: [PATCH 5/8] refactor delay-tasks --- delay-tasks/Cargo.toml | 6 +- delay-tasks/src/lib.rs | 199 ++++++++++++++++------- delay-tasks/src/mock.rs | 183 ++++++++------------- delay-tasks/src/tests.rs | 334 +++++++++++++++------------------------ traits/src/task.rs | 120 ++------------ xtokens/src/lib.rs | 3 +- 6 files changed, 357 insertions(+), 488 deletions(-) diff --git a/delay-tasks/Cargo.toml b/delay-tasks/Cargo.toml index f6ad3e240..f7cfdcb5e 100644 --- a/delay-tasks/Cargo.toml +++ b/delay-tasks/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "orml-delay-tasks" -description = "Delayed xtokens transfer assets executor." +description = "Scheduler delay task and execute." repository = "https://github.com/open-web3-stack/open-runtime-module-library/tree/master/auction" license = "Apache-2.0" version = "0.7.0" @@ -21,10 +21,13 @@ sp-std = { workspace = true } xcm = { workspace = true } orml-traits = { path = "../traits", version = "0.7.0", default-features = false } +orml-xtokens = { path = "../xtokens", version = "0.7.0", default-features = false } [dev-dependencies] sp-core = { workspace = true, features = ["std"] } sp-io = { workspace = true, features = ["std"] } +pallet-preimage = { workspace = true, features = ["std"] } +pallet-scheduler = { workspace = true, features = ["std"] } [features] default = [ "std" ] @@ -32,6 +35,7 @@ std = [ "frame-support/std", "frame-system/std", "orml-traits/std", + "orml-xtokens/std", "parity-scale-codec/std", "scale-info/std", "serde", diff --git a/delay-tasks/src/lib.rs b/delay-tasks/src/lib.rs index 47354a3e5..d441c2cb2 100644 --- a/delay-tasks/src/lib.rs +++ b/delay-tasks/src/lib.rs @@ -1,40 +1,103 @@ #![cfg_attr(not(feature = "std"), no_std)] -use frame_support::{pallet_prelude::*, traits::schedule::DispatchTime, transactional, weights::Weight}; +use frame_support::{ + pallet_prelude::*, + traits::{ + schedule::{v1::Named as ScheduleNamed, DispatchTime}, + OriginTrait, + }, + weights::Weight, +}; use frame_system::pallet_prelude::*; -use orml_traits::delay_tasks::{DelayTasksManager, DelayedTask}; +use orml_traits::{ + task::{DelayTaskHooks, DelayTasksManager, DispatchableTask, TaskResult}, + MultiCurrency, NamedMultiReservableCurrency, +}; use parity_scale_codec::FullCodec; use scale_info::TypeInfo; use sp_runtime::{ - traits::{CheckedAdd, Zero}, + traits::{CheckedAdd, Convert, Zero}, ArithmeticError, }; use sp_std::fmt::Debug; +use sp_std::marker::PhantomData; +use xcm::v4::prelude::*; pub use module::*; mod mock; mod tests; +pub const DELAY_TASK_ID: [u8; 8] = *b"orml/dts"; + +/// A delayed origin. Can only be dispatched via `dispatch_as` with a delay. +#[derive(PartialEq, Eq, Clone, RuntimeDebug, Encode, Decode, TypeInfo, MaxEncodedLen)] +pub struct DelayedExecuteOrigin; + +pub struct EnsureDelayed; +impl> + From> EnsureOrigin for EnsureDelayed { + type Success = (); + fn try_origin(o: O) -> Result { + o.into().and_then(|_| Ok(())) + } + + #[cfg(feature = "runtime-benchmarks")] + fn try_successful_origin() -> Result { + Ok(O::from(DelayedExecuteOrigin)) + } +} + #[frame_support::pallet] pub mod module { use super::*; type Nonce = u64; + /// Origin for the delay tasks module. + #[pallet::origin] + pub type Origin = DelayedExecuteOrigin; + #[pallet::config] pub trait Config: frame_system::Config { type RuntimeEvent: From> + IsType<::RuntimeEvent>; + type RuntimeCall: Parameter + From>; + + /// The outer origin type. + type RuntimeOrigin: From + + From<::RuntimeOrigin> + + OriginTrait; + + /// The caller origin, overarching type of all pallets origins. + type PalletsOrigin: Parameter + Into<::RuntimeOrigin>; + + type DelayOrigin: EnsureOrigin<::RuntimeOrigin>; + type GovernanceOrigin: EnsureOrigin<::RuntimeOrigin>; - type Task: DelayedTask + FullCodec + Debug + Clone + PartialEq + TypeInfo; + type Task: DispatchableTask + FullCodec + Debug + Clone + PartialEq + TypeInfo; + + /// The Scheduler. + type Scheduler: ScheduleNamed, ::RuntimeCall, Self::PalletsOrigin>; + + type DelayTaskHooks: DelayTaskHooks; + + /// Convert `Location` to `CurrencyId`. + type CurrencyIdConvert: Convert< + Location, + Option<>::CurrencyId>, + >; + + type Currency: NamedMultiReservableCurrency; + + type ReserveId: Get<>::ReserveIdentifier>; } #[pallet::error] pub enum Error { InvalidDelayBlock, InvalidId, + FailedToSchedule, } #[pallet::event] @@ -53,10 +116,6 @@ pub mod module { id: Nonce, execute_block: BlockNumberFor, }, - DelayedTaskTryExecuteFailed { - id: Nonce, - error: DispatchError, - }, DelayedTaskCanceled { id: Nonce, }, @@ -67,16 +126,7 @@ pub mod module { pub struct Pallet(_); #[pallet::hooks] - impl Hooks> for Pallet { - /// `on_initialize` to return the weight used in `on_finalize`. - fn on_initialize(now: BlockNumberFor) -> Weight { - Weight::zero() - } - - fn on_finalize(now: BlockNumberFor) { - Self::_on_finalize(now); - } - } + impl Hooks> for Pallet {} #[pallet::storage] #[pallet::getter(fn next_delayed_task_id)] @@ -86,16 +136,25 @@ pub mod module { #[pallet::getter(fn delayed_tasks)] pub type DelayedTasks = StorageMap<_, Twox64Concat, Nonce, (T::Task, BlockNumberFor), OptionQuery>; - #[pallet::storage] - #[pallet::getter(fn delayed_task_queue)] - pub type DelayedTaskQueue = - StorageDoubleMap<_, Twox64Concat, BlockNumberFor, Twox64Concat, Nonce, (), OptionQuery>; - #[pallet::call] impl Pallet { #[pallet::call_index(0)] #[pallet::weight(Weight::zero())] - pub fn reset_execute_block( + pub fn delayed_execute(origin: OriginFor, id: Nonce) -> DispatchResult { + T::DelayOrigin::ensure_origin(origin)?; + + let execute_result = Self::do_delayed_execute(id)?; + + Self::deposit_event(Event::::DelayedTaskExecuted { + id, + result: execute_result.result, + }); + Ok(()) + } + + #[pallet::call_index(1)] + #[pallet::weight(Weight::zero())] + pub fn reschedule_delay_task( origin: OriginFor, id: Nonce, when: DispatchTime>, @@ -112,10 +171,11 @@ pub mod module { }; ensure!(new_execute_block > now, Error::::InvalidDelayBlock); - DelayedTaskQueue::::remove(*execute_block, id); - DelayedTaskQueue::::insert(new_execute_block, id, ()); *execute_block = new_execute_block; + T::Scheduler::reschedule_named((&DELAY_TASK_ID, id).encode(), DispatchTime::At(new_execute_block)) + .map_err(|_| Error::::FailedToSchedule)?; + Self::deposit_event(Event::::DelayedTaskReDelayed { id, execute_block: new_execute_block, @@ -126,14 +186,17 @@ pub mod module { Ok(()) } - #[pallet::call_index(1)] + #[pallet::call_index(2)] #[pallet::weight(Weight::zero())] pub fn cancel_delayed_task(origin: OriginFor, id: Nonce) -> DispatchResult { T::GovernanceOrigin::ensure_origin(origin)?; - let (delay_task, execute_block) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; - delay_task.on_cancel()?; - DelayedTaskQueue::::remove(execute_block, id); + let (task, _) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; + + // pre cancel + T::DelayTaskHooks::on_cancel(&task)?; + + T::Scheduler::cancel_named((&DELAY_TASK_ID, id).encode()).map_err(|_| Error::::FailedToSchedule)?; Self::deposit_event(Event::::DelayedTaskCanceled { id }); Ok(()) @@ -141,32 +204,13 @@ pub mod module { } impl Pallet { - fn _on_finalize(now: BlockNumberFor) { - for (id, _) in DelayedTaskQueue::::drain_prefix(now) { - match Self::do_execute_delayed_task(id) { - Ok(result) => { - Self::deposit_event(Event::::DelayedTaskExecuted { id, result }); - } - Err(e) => { - log::debug!( - target: "delay-tasks", - "try executing delayed task {:?} failed for: {:?}. The delayed task still exists, but needs to be canceled or reset delay block.", - id, - e - ); - Self::deposit_event(Event::::DelayedTaskTryExecuteFailed { id, error: e }); - } - } - } - } - - #[transactional] - pub(crate) fn do_execute_delayed_task(id: Nonce) -> sp_std::result::Result { + pub(crate) fn do_delayed_execute(id: Nonce) -> sp_std::result::Result { let (delayed_task, _) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; - delayed_task.pre_delayed_execute()?; + // pre delayed dispatch + T::DelayTaskHooks::pre_delayed_execute(&delayed_task)?; - Ok(delayed_task.delayed_execute()) + Ok(delayed_task.dispatch(Weight::zero())) } /// Retrieves the next delayed task ID from storage, and increment it by @@ -184,16 +228,28 @@ pub mod module { impl DelayTasksManager> for Pallet { fn add_delay_task(task: T::Task, delay_blocks: BlockNumberFor) -> DispatchResult { ensure!(!delay_blocks.is_zero(), Error::::InvalidDelayBlock); - - task.pre_delay()?; - - let id = Self::get_next_delayed_task_id()?; let execute_block = frame_system::Pallet::::block_number() .checked_add(&delay_blocks) .ok_or(ArithmeticError::Overflow)?; + // pre schedule delay task + T::DelayTaskHooks::pre_delay(&task)?; + + let id = Self::get_next_delayed_task_id()?; + let delayed_origin: ::RuntimeOrigin = From::from(DelayedExecuteOrigin); + let pallets_origin = delayed_origin.caller().clone(); + + T::Scheduler::schedule_named( + (&DELAY_TASK_ID, id).encode(), + DispatchTime::At(execute_block), + None, + Zero::zero(), + pallets_origin, + ::RuntimeCall::from(Call::::delayed_execute { id }), + ) + .map_err(|_| Error::::FailedToSchedule)?; + DelayedTasks::::insert(id, (&task, execute_block)); - DelayedTaskQueue::::insert(execute_block, id, ()); Self::deposit_event(Event::::DelayedTaskAdded { id, @@ -203,4 +259,31 @@ pub mod module { Ok(()) } } + + pub struct DelayedXtokensTaskHooks(PhantomData); + impl DelayTaskHooks> for DelayedXtokensTaskHooks { + fn pre_delay(task: &orml_xtokens::XtokensTask) -> DispatchResult { + match task { + orml_xtokens::XtokensTask::::TransferAssets { who, assets, fee, .. } => {} + } + + Ok(()) + } + + fn pre_delayed_execute(task: &orml_xtokens::XtokensTask) -> DispatchResult { + match task { + orml_xtokens::XtokensTask::::TransferAssets { who, assets, fee, .. } => {} + } + + Ok(()) + } + + fn on_cancel(task: &orml_xtokens::XtokensTask) -> DispatchResult { + match task { + orml_xtokens::XtokensTask::::TransferAssets { who, assets, fee, .. } => {} + } + + Ok(()) + } + } } diff --git a/delay-tasks/src/mock.rs b/delay-tasks/src/mock.rs index fbb722f4a..61ea16e0e 100644 --- a/delay-tasks/src/mock.rs +++ b/delay-tasks/src/mock.rs @@ -3,7 +3,7 @@ #![cfg(test)] use super::*; -use frame_support::{construct_runtime, derive_impl}; +use frame_support::{construct_runtime, derive_impl, parameter_types, traits::EqualPrivilegeOnly}; use frame_system::EnsureRoot; use orml_traits::define_combined_task; use sp_runtime::{traits::IdentityLookup, BuildStorage, DispatchError}; @@ -21,148 +21,83 @@ impl frame_system::Config for Runtime { type Block = Block; } -thread_local! { - pub static PRE_DELAYED: RefCell = RefCell::new(false); - pub static PRE_DELAYED_EXECUTED: RefCell = RefCell::new(false); - pub static EXECUTED: RefCell = RefCell::new(false); - pub static ON_CANCEL: RefCell = RefCell::new(false); +impl pallet_preimage::Config for Runtime { + type RuntimeEvent = RuntimeEvent; + type WeightInfo = (); + type Currency = (); + type ManagerOrigin = EnsureRoot; + type Consideration = (); } -pub(crate) fn reset_delay_process_records() { - PRE_DELAYED.with(|v| *v.borrow_mut() = false); - PRE_DELAYED_EXECUTED.with(|v| *v.borrow_mut() = false); - EXECUTED.with(|v| *v.borrow_mut() = false); - ON_CANCEL.with(|v| *v.borrow_mut() = false); +parameter_types! { + pub BlockWeights: frame_system::limits::BlockWeights = + frame_system::limits::BlockWeights::simple_max(Weight::from_parts(2_000_000_000_000, 0).set_proof_size(u64::MAX)); + pub MaximumSchedulerWeight: Weight = BlockWeights::get().max_block; } -define_combined_task! { - #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] - pub enum MockDelayedTaskType { - Success(SuccessTask), - FailedPreDelay(FailedPreDelayTask), - FailedPreDelayedExecute(FailedPreDelayedExecuteTask), - FailedDelayedExecute(FailedDelayedExecuteTask), - FailedOnCancel(FailedOnCancelTask), - } -} - -#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] -pub struct SuccessTask; -impl DelayedTask for SuccessTask { - fn pre_delay(&self) -> DispatchResult { - PRE_DELAYED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn pre_delayed_execute(&self) -> DispatchResult { - PRE_DELAYED_EXECUTED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn delayed_execute(&self) -> DispatchResult { - EXECUTED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn on_cancel(&self) -> DispatchResult { - ON_CANCEL.with(|v| *v.borrow_mut() = true); - Ok(()) - } +impl pallet_scheduler::Config for Runtime { + type RuntimeEvent = RuntimeEvent; + type RuntimeOrigin = RuntimeOrigin; + type PalletsOrigin = OriginCaller; + type RuntimeCall = RuntimeCall; + type MaximumWeight = MaximumSchedulerWeight; + type ScheduleOrigin = EnsureRoot; + type MaxScheduledPerBlock = ConstU32<10>; + type WeightInfo = (); + type OriginPrivilegeCmp = EqualPrivilegeOnly; + type Preimages = Preimage; } -#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] -pub struct FailedPreDelayTask; -impl DelayedTask for FailedPreDelayTask { - fn pre_delay(&self) -> DispatchResult { - Err(DispatchError::Other("pre_delay failed")) - } - - fn pre_delayed_execute(&self) -> DispatchResult { - unimplemented!() - } - - fn delayed_execute(&self) -> DispatchResult { - unimplemented!() - } - - fn on_cancel(&self) -> DispatchResult { - unimplemented!() - } +thread_local! { + pub static SUCCEEDED: RefCell = RefCell::new(0); + pub static FAILED: RefCell = RefCell::new(0); } -#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] -pub struct FailedPreDelayedExecuteTask; -impl DelayedTask for FailedPreDelayedExecuteTask { - fn pre_delay(&self) -> DispatchResult { - PRE_DELAYED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn pre_delayed_execute(&self) -> DispatchResult { - Err(DispatchError::Other("pre_delayed_execute failed")) - } - - fn delayed_execute(&self) -> DispatchResult { - EXECUTED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn on_cancel(&self) -> DispatchResult { - ON_CANCEL.with(|v| *v.borrow_mut() = true); - Ok(()) +define_combined_task! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum MockTaskType { + Success(SuccessTask), + Fail(FailTask), } } #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] -pub struct FailedDelayedExecuteTask; -impl DelayedTask for FailedDelayedExecuteTask { - fn pre_delay(&self) -> DispatchResult { - PRE_DELAYED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn pre_delayed_execute(&self) -> DispatchResult { - PRE_DELAYED_EXECUTED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn delayed_execute(&self) -> DispatchResult { - Err(DispatchError::Other("delayed_execute failed")) - } +pub struct SuccessTask; +impl DispatchableTask for SuccessTask { + fn dispatch(self, _weight: Weight) -> TaskResult { + SUCCEEDED.with(|v| *v.borrow_mut() += 1); - fn on_cancel(&self) -> DispatchResult { - ON_CANCEL.with(|v| *v.borrow_mut() = true); - Ok(()) + TaskResult { + result: Ok(()), + used_weight: Weight::zero(), + finished: true, + } } } #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] -pub struct FailedOnCancelTask; -impl DelayedTask for FailedOnCancelTask { - fn pre_delay(&self) -> DispatchResult { - PRE_DELAYED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn pre_delayed_execute(&self) -> DispatchResult { - PRE_DELAYED_EXECUTED.with(|v| *v.borrow_mut() = true); - Ok(()) - } - - fn delayed_execute(&self) -> DispatchResult { - EXECUTED.with(|v| *v.borrow_mut() = true); - Ok(()) - } +pub struct FailTask; +impl DispatchableTask for FailTask { + fn dispatch(self, _weight: Weight) -> TaskResult { + FAILED.with(|v| *v.borrow_mut() += 1); - fn on_cancel(&self) -> DispatchResult { - Err(DispatchError::Other("on_cancel failed")) + TaskResult { + result: Err(DispatchError::Other("execute failed")), + used_weight: Weight::zero(), + finished: true, + } } } impl Config for Runtime { type RuntimeEvent = RuntimeEvent; + type RuntimeCall = RuntimeCall; + type RuntimeOrigin = RuntimeOrigin; + type PalletsOrigin = OriginCaller; + type DelayOrigin = EnsureDelayed; type GovernanceOrigin = EnsureRoot; - type Task = MockDelayedTaskType; + type Task = MockTaskType; + type Scheduler = Scheduler; } type Block = frame_system::mocking::MockBlock; @@ -171,6 +106,8 @@ construct_runtime!( pub enum Runtime { System: frame_system, DelayTasks: delay_tasks, + Scheduler: pallet_scheduler, + Preimage: pallet_preimage, } ); @@ -191,3 +128,11 @@ impl ExtBuilder { t.into() } } + +pub fn run_to_block(n: u64) { + while System::block_number() < n { + Scheduler::on_finalize(System::block_number()); + System::set_block_number(System::block_number() + 1); + Scheduler::on_initialize(System::block_number()); + } +} diff --git a/delay-tasks/src/tests.rs b/delay-tasks/src/tests.rs index eb1a79f25..7fb2192bc 100644 --- a/delay-tasks/src/tests.rs +++ b/delay-tasks/src/tests.rs @@ -4,6 +4,7 @@ use super::*; use frame_support::{assert_noop, assert_ok}; use mock::*; +use sp_io::hashing::blake2_256; use sp_runtime::traits::{BadOrigin, Bounded}; #[test] @@ -14,73 +15,64 @@ fn add_delay_task_work() { assert_eq!(DelayTasks::delayed_tasks(0), None); assert_noop!( - DelayTasks::add_delay_task(MockDelayedTaskType::Success(SuccessTask), 0), + DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 0), Error::::InvalidDelayBlock ); - assert_eq!(PRE_DELAYED.with(|v| *v.borrow()), false); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::Success(SuccessTask), - 100 + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 10)); + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Scheduled { when: 11, index: 0 }, )); System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskAdded { id: 0, - task: MockDelayedTaskType::Success(SuccessTask), - execute_block: 101, + task: MockTaskType::Success(SuccessTask), + execute_block: 11, })); assert_eq!(DelayTasks::next_delayed_task_id(), 1); assert_eq!( DelayTasks::delayed_tasks(0), - Some((MockDelayedTaskType::Success(SuccessTask), 101)) + Some((MockTaskType::Success(SuccessTask), 11)) ); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); - assert_eq!(PRE_DELAYED.with(|v| *v.borrow()), true); - - reset_delay_process_records(); - assert_noop!( - DelayTasks::add_delay_task(MockDelayedTaskType::FailedPreDelay(FailedPreDelayTask), 200), - DispatchError::Other("pre_delay failed") - ); - assert_eq!(PRE_DELAYED.with(|v| *v.borrow()), false); }); } #[test] -fn reset_execute_block_work() { +fn reschedule_delay_task_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::Success(SuccessTask), - 100 - )); + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 100)); assert_eq!( DelayTasks::delayed_tasks(0), - Some((MockDelayedTaskType::Success(SuccessTask), 101)) + Some((MockTaskType::Success(SuccessTask), 101)) ); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); - assert_eq!(DelayTasks::delayed_task_queue(10, 0), None); assert_noop!( - DelayTasks::reset_execute_block(RuntimeOrigin::signed(1), 0, DispatchTime::At(10)), + DelayTasks::reschedule_delay_task(RuntimeOrigin::signed(1), 0, DispatchTime::At(10)), BadOrigin ); assert_noop!( - DelayTasks::reset_execute_block(RuntimeOrigin::root(), 1, DispatchTime::At(10)), + DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 1, DispatchTime::At(10)), Error::::InvalidId ); assert_noop!( - DelayTasks::reset_execute_block(RuntimeOrigin::root(), 0, DispatchTime::After(Bounded::max_value())), + DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 0, DispatchTime::After(Bounded::max_value())), ArithmeticError::Overflow ); - assert_ok!(DelayTasks::reset_execute_block( + assert_ok!(DelayTasks::reschedule_delay_task( RuntimeOrigin::root(), 0, DispatchTime::At(10) )); + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Canceled { when: 101, index: 0 }, + )); + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Scheduled { when: 10, index: 0 }, + )); System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskReDelayed { id: 0, execute_block: 10, @@ -88,23 +80,21 @@ fn reset_execute_block_work() { assert_eq!( DelayTasks::delayed_tasks(0), - Some((MockDelayedTaskType::Success(SuccessTask), 10)) + Some((MockTaskType::Success(SuccessTask), 10)) ); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), None); - assert_eq!(DelayTasks::delayed_task_queue(10, 0), Some(())); System::set_block_number(100); assert_noop!( - DelayTasks::reset_execute_block(RuntimeOrigin::root(), 0, DispatchTime::At(100)), + DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 0, DispatchTime::At(100)), Error::::InvalidDelayBlock ); assert_noop!( - DelayTasks::reset_execute_block(RuntimeOrigin::root(), 0, DispatchTime::After(0)), + DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 0, DispatchTime::After(0)), Error::::InvalidDelayBlock ); - assert_ok!(DelayTasks::reset_execute_block( + assert_ok!(DelayTasks::reschedule_delay_task( RuntimeOrigin::root(), 0, DispatchTime::After(1) @@ -116,10 +106,8 @@ fn reset_execute_block_work() { assert_eq!( DelayTasks::delayed_tasks(0), - Some((MockDelayedTaskType::Success(SuccessTask), 101)) + Some((MockTaskType::Success(SuccessTask), 101)) ); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); - assert_eq!(DelayTasks::delayed_task_queue(10, 0), None); }); } @@ -127,25 +115,11 @@ fn reset_execute_block_work() { fn cancel_delayed_task_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::Success(SuccessTask), - 100 - )); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), - 200 - )); - + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 100)); assert_eq!( DelayTasks::delayed_tasks(0), - Some((MockDelayedTaskType::Success(SuccessTask), 101)) + Some((MockTaskType::Success(SuccessTask), 101)) ); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); - assert_eq!( - DelayTasks::delayed_tasks(1), - Some((MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), 201)) - ); - assert_eq!(DelayTasks::delayed_task_queue(201, 1), Some(())); assert_noop!(DelayTasks::cancel_delayed_task(RuntimeOrigin::signed(1), 0), BadOrigin); @@ -155,197 +129,149 @@ fn cancel_delayed_task_work() { ); assert_ok!(DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 0)); + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Canceled { when: 101, index: 0 }, + )); System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskCanceled { id: 0 })); - assert_eq!(ON_CANCEL.with(|v| *v.borrow()), true); assert_eq!(DelayTasks::delayed_tasks(0), None); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), None); - - reset_delay_process_records(); - assert_noop!( - DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 1), - DispatchError::Other("on_cancel failed") - ); - assert_eq!(ON_CANCEL.with(|v| *v.borrow()), false); - assert_eq!( - DelayTasks::delayed_tasks(1), - Some((MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), 201)) - ); - assert_eq!(DelayTasks::delayed_task_queue(201, 1), Some(())); }); } #[test] -fn do_execute_delayed_task_work() { +fn do_delayed_execute_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); - assert_noop!(DelayTasks::do_execute_delayed_task(0), Error::::InvalidId); + assert_noop!(DelayTasks::do_delayed_execute(0), Error::::InvalidId); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::Success(SuccessTask), - 100 - )); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), - 100 - )); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::FailedDelayedExecute(FailedDelayedExecuteTask), - 100 - )); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), - 100 - )); + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 100)); + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Fail(FailTask), 100)); assert_eq!( DelayTasks::delayed_tasks(0), - Some((MockDelayedTaskType::Success(SuccessTask), 101)) - ); - assert_eq!( - DelayTasks::delayed_tasks(1), - Some(( - MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), - 101 - )) + Some((MockTaskType::Success(SuccessTask), 101)) ); - assert_eq!( - DelayTasks::delayed_tasks(2), - Some((MockDelayedTaskType::FailedDelayedExecute(FailedDelayedExecuteTask), 101)) - ); - assert_eq!( - DelayTasks::delayed_tasks(3), - Some((MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), 101)) - ); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); - assert_eq!(DelayTasks::delayed_task_queue(101, 1), Some(())); - assert_eq!(DelayTasks::delayed_task_queue(101, 2), Some(())); - assert_eq!(DelayTasks::delayed_task_queue(101, 3), Some(())); - - // execute delayed task - reset_delay_process_records(); - assert_ok!(DelayTasks::do_execute_delayed_task(0)); - assert_eq!(PRE_DELAYED_EXECUTED.with(|v| *v.borrow()), true); - assert_eq!(EXECUTED.with(|v| *v.borrow()), true); - assert_eq!(DelayTasks::delayed_tasks(0), None); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); // do_execute_delayed_task doesn't clear delayed_task_queue + assert_eq!(DelayTasks::delayed_tasks(1), Some((MockTaskType::Fail(FailTask), 101))); - // failed execute delayed task for failed pre_delayed_execute - reset_delay_process_records(); - assert_noop!( - DelayTasks::do_execute_delayed_task(1), - DispatchError::Other("pre_delayed_execute failed") - ); - assert_eq!(PRE_DELAYED_EXECUTED.with(|v| *v.borrow()), false); - assert_eq!(EXECUTED.with(|v| *v.borrow()), false); + assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 0); assert_eq!( - DelayTasks::delayed_tasks(1), - Some(( - MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), - 101 - )) + DelayTasks::do_delayed_execute(0), + Ok(TaskResult { + result: Ok(()), + used_weight: Weight::zero(), + finished: true, + }) ); - assert_eq!(DelayTasks::delayed_task_queue(101, 1), Some(())); // do_execute_delayed_task doesn't clear delayed_task_queue + assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 1); + assert_eq!(DelayTasks::delayed_tasks(0), None); - // execute delayed task but task failed - reset_delay_process_records(); + assert_eq!(FAILED.with(|v| *v.borrow()), 0); assert_eq!( - DelayTasks::do_execute_delayed_task(2), - Ok(Err(DispatchError::Other("delayed_execute failed"))) + DelayTasks::do_delayed_execute(1), + Ok(TaskResult { + result: Err(DispatchError::Other("execute failed").into()), + used_weight: Weight::zero(), + finished: true, + }) ); - assert_eq!(PRE_DELAYED_EXECUTED.with(|v| *v.borrow()), true); - assert_eq!(EXECUTED.with(|v| *v.borrow()), false); - assert_eq!(DelayTasks::delayed_tasks(2), None); - assert_eq!(DelayTasks::delayed_task_queue(101, 2), Some(())); // do_execute_delayed_task doesn't clear delayed_task_queue - - // execute delayed task - reset_delay_process_records(); - assert_ok!(DelayTasks::do_execute_delayed_task(3)); - assert_eq!(PRE_DELAYED_EXECUTED.with(|v| *v.borrow()), true); - assert_eq!(EXECUTED.with(|v| *v.borrow()), true); - assert_eq!(DelayTasks::delayed_tasks(3), None); - assert_eq!(DelayTasks::delayed_task_queue(101, 3), Some(())); // do_execute_delayed_task doesn't clear delayed_task_queue + assert_eq!(FAILED.with(|v| *v.borrow()), 1); + assert_eq!(DelayTasks::delayed_tasks(1), None); }); } #[test] -fn on_finalize_work() { +fn delayed_execute_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::Success(SuccessTask), - 100 - )); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), - 100 - )); - System::set_block_number(2); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::FailedDelayedExecute(FailedDelayedExecuteTask), - 100 - )); - assert_ok!(DelayTasks::add_delay_task( - MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), - 100 - )); - assert_eq!( - DelayTasks::delayed_tasks(0), - Some((MockDelayedTaskType::Success(SuccessTask), 101)) - ); - assert_eq!( - DelayTasks::delayed_tasks(1), - Some(( - MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), - 101 - )) - ); - assert_eq!( - DelayTasks::delayed_tasks(2), - Some((MockDelayedTaskType::FailedDelayedExecute(FailedDelayedExecuteTask), 102)) + assert_noop!(DelayTasks::delayed_execute(RuntimeOrigin::root(), 0), BadOrigin); + + assert_noop!(DelayTasks::delayed_execute(RuntimeOrigin::signed(1), 0), BadOrigin); + + assert_noop!( + DelayTasks::delayed_execute(RuntimeOrigin::from(DelayedExecuteOrigin), 0), + Error::::InvalidId ); + + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 100)); + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Fail(FailTask), 100)); + assert_eq!( - DelayTasks::delayed_tasks(3), - Some((MockDelayedTaskType::FailedOnCancel(FailedOnCancelTask), 102)) + DelayTasks::delayed_tasks(0), + Some((MockTaskType::Success(SuccessTask), 101)) ); - assert_eq!(DelayTasks::delayed_task_queue(101, 0), Some(())); - assert_eq!(DelayTasks::delayed_task_queue(101, 1), Some(())); - assert_eq!(DelayTasks::delayed_task_queue(102, 2), Some(())); - assert_eq!(DelayTasks::delayed_task_queue(102, 3), Some(())); + assert_eq!(DelayTasks::delayed_tasks(1), Some((MockTaskType::Fail(FailTask), 101))); - DelayTasks::on_finalize(101); + assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 0); + assert_ok!(DelayTasks::delayed_execute( + RuntimeOrigin::from(DelayedExecuteOrigin), + 0 + )); System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { id: 0, result: Ok(()), })); - System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskTryExecuteFailed { - id: 1, - error: DispatchError::Other(""), - })); + assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 1); assert_eq!(DelayTasks::delayed_tasks(0), None); - assert_eq!( - DelayTasks::delayed_tasks(1), - Some(( - MockDelayedTaskType::FailedPreDelayedExecute(FailedPreDelayedExecuteTask), - 101 - )) - ); // keep the storage for failed executed delayed task - assert_eq!(DelayTasks::delayed_task_queue(101, 0), None); - assert_eq!(DelayTasks::delayed_task_queue(101, 1), None); - - DelayTasks::on_finalize(102); + + assert_eq!(FAILED.with(|v| *v.borrow()), 0); + assert_ok!(DelayTasks::delayed_execute( + RuntimeOrigin::from(DelayedExecuteOrigin), + 1 + )); System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { - id: 2, + id: 1, result: Err(DispatchError::Other("").into()), })); + assert_eq!(FAILED.with(|v| *v.borrow()), 1); + assert_eq!(DelayTasks::delayed_tasks(1), None); + }); +} + +#[test] +fn dispatch_delayed_execute_work() { + ExtBuilder::default().build().execute_with(|| { + System::set_block_number(1); + + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 10)); + assert_ok!(DelayTasks::add_delay_task(MockTaskType::Fail(FailTask), 10)); + + assert_eq!( + DelayTasks::delayed_tasks(0), + Some((MockTaskType::Success(SuccessTask), 11)) + ); + assert_eq!(DelayTasks::delayed_tasks(1), Some((MockTaskType::Fail(FailTask), 11))); + + assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 0); + assert_eq!(FAILED.with(|v| *v.borrow()), 0); + run_to_block(11); + + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Dispatched { + task: (11, 0), + id: Some(blake2_256(&(&DELAY_TASK_ID, 0u64).encode())), + result: Ok(()), + }, + )); System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { - id: 3, + id: 0, result: Ok(()), })); - assert_eq!(DelayTasks::delayed_tasks(2), None); - assert_eq!(DelayTasks::delayed_tasks(3), None); - assert_eq!(DelayTasks::delayed_task_queue(101, 2), None); - assert_eq!(DelayTasks::delayed_task_queue(101, 3), None); + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Dispatched { + task: (11, 1), + id: Some(blake2_256(&(&DELAY_TASK_ID, 1u64).encode())), + result: Ok(()), + }, + )); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { + id: 1, + result: Err(DispatchError::Other("").into()), + })); + + assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 1); + assert_eq!(FAILED.with(|v| *v.borrow()), 1); + assert_eq!(DelayTasks::delayed_tasks(0), None); + assert_eq!(DelayTasks::delayed_tasks(1), None); }); } diff --git a/traits/src/task.rs b/traits/src/task.rs index d922fe01d..0a050b311 100644 --- a/traits/src/task.rs +++ b/traits/src/task.rs @@ -67,108 +67,20 @@ pub trait DelayTasksManager { fn add_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; } -// pub trait DelayedTask { -// fn pre_delay(&self) -> DispatchResult; -// fn pre_delayed_execute(&self) -> DispatchResult; -// fn delayed_execute(&self) -> DispatchResult; -// fn on_cancel(&self) -> DispatchResult; -// } - -// pub trait DelayTasksManager { -// fn add_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; -// } - -// #[macro_export] -// macro_rules! define_combined_task { -// ( -// $(#[$meta:meta])* -// $vis:vis enum $combined_name:ident { -// $( -// $task:ident ( $vtask:ident $(<$($generic:tt),*>)? ) -// ),+ $(,)? -// } -// ) => { -// $(#[$meta])* -// $vis enum $combined_name { -// $( -// $task($vtask $(<$($generic),*>)?), -// )* -// } - -// impl DispatchableTask for $combined_name { -// fn dispatch(self, weight: Weight) -> TaskResult { -// match self { -// $( -// $combined_name::$task(t) => t.dispatch(weight), -// )* -// } -// } -// } - -// $( -// impl From<$vtask $(<$($generic),*>)?> for $combined_name { -// fn from(t: $vtask $(<$($generic),*>)?) -> Self{ -// $combined_name::$task(t) -// } -// } -// )* -// }; -// } - -// #[macro_export] -// macro_rules! define_combined_delayed_task { -// ( -// $(#[$meta:meta])* -// $vis:vis enum $combined_name:ident { -// $( -// $task:ident ( $vtask:ident $(<$($generic:tt),*>)? ) -// ),+ $(,)? -// } -// ) => { -// $(#[$meta])* -// $vis enum $combined_name { -// $( -// $task($vtask $(<$($generic),*>)?), -// )* -// } - -// impl DelayedTask for $combined_name { -// fn pre_delay(&self) -> DispatchResult { -// match self { -// $( -// $combined_name::$task(t) => t.pre_delay(), -// )* -// } -// } -// fn pre_delayed_execute(&self) -> DispatchResult { -// match self { -// $( -// $combined_name::$task(t) => t.pre_delayed_execute(), -// )* -// } -// } -// fn delayed_execute(&self) -> DispatchResult { -// match self { -// $( -// $combined_name::$task(t) => t.delayed_execute(), -// )* -// } -// } -// fn on_cancel(&self) -> DispatchResult { -// match self { -// $( -// $combined_name::$task(t) => t.on_cancel(), -// )* -// } -// } -// } +pub trait DelayTaskHooks { + fn pre_delay(task: &Task) -> DispatchResult; + fn pre_delayed_execute(task: &Task) -> DispatchResult; + fn on_cancel(task: &Task) -> DispatchResult; +} -// $( -// impl From<$vtask $(<$($generic),*>)?> for $combined_name { -// fn from(t: $vtask $(<$($generic),*>)?) -> Self{ -// $combined_name::$task(t) -// } -// } -// )* -// }; -// } +impl DelayTaskHooks for () { + fn pre_delay(_: &Task) -> DispatchResult { + Ok(()) + } + fn pre_delayed_execute(_: &Task) -> DispatchResult { + Ok(()) + } + fn on_cancel(_: &Task) -> DispatchResult { + Ok(()) + } +} diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 0eadb4945..06571a7b3 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -144,8 +144,7 @@ pub mod module { type CurrencyId: Parameter + Member + Clone; /// Convert `T::CurrencyId` to `Location`. - type CurrencyIdConvert: Convert> - + Convert>; + type CurrencyIdConvert: Convert>; /// Convert `T::AccountId` to `Location`. type AccountIdToLocation: Convert; From 9d70e37703a0551997ff20bce6515ccb0e24b45a Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Thu, 21 Mar 2024 02:30:04 +0800 Subject: [PATCH 6/8] impl DelayTaskHooks for Task to process --- asset-registry/src/mock/para.rs | 16 +- delay-tasks/Cargo.toml | 19 +++ delay-tasks/src/lib.rs | 109 ++++++++---- delay-tasks/src/mock.rs | 289 +++++++++++++++++++++++++++++--- delay-tasks/src/tests.rs | 256 +++++++++++++++------------- traits/src/task.rs | 97 +++++++++-- 6 files changed, 587 insertions(+), 199 deletions(-) diff --git a/asset-registry/src/mock/para.rs b/asset-registry/src/mock/para.rs index a240abc20..69c4c5f5e 100644 --- a/asset-registry/src/mock/para.rs +++ b/asset-registry/src/mock/para.rs @@ -11,22 +11,17 @@ use frame_support::{ }; use frame_system::{EnsureRoot, EnsureSignedBy}; use orml_traits::{ - define_combined_task, location::{AbsoluteReserveProvider, RelativeReserveProvider}, - parameter_type_with_key, - task::{DispatchableTask, TaskResult}, - FixedConversionRateProvider, MultiCurrency, + parameter_type_with_key, FixedConversionRateProvider, MultiCurrency, }; use orml_xcm_support::{IsNativeConcrete, MultiCurrencyAdapter, MultiNativeAsset}; -use orml_xtokens::XtokensTask; use pallet_xcm::XcmPassthrough; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use polkadot_parachain_primitives::primitives::Sibling; -use scale_info::TypeInfo; use sp_core::Get; use sp_runtime::{ traits::{AccountIdConversion, Convert, IdentityLookup}, - AccountId32, DispatchResult, RuntimeDebug, + AccountId32, }; use xcm::v4::{prelude::*, Weight}; use xcm_builder::{ @@ -315,13 +310,6 @@ parameter_type_with_key! { }; } -define_combined_task! { - #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] - pub enum DelayedTasks { - Xtokens(XtokensTask), - } -} - impl orml_xtokens::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; diff --git a/delay-tasks/Cargo.toml b/delay-tasks/Cargo.toml index f7cfdcb5e..fdb5f9424 100644 --- a/delay-tasks/Cargo.toml +++ b/delay-tasks/Cargo.toml @@ -26,8 +26,22 @@ orml-xtokens = { path = "../xtokens", version = "0.7.0", default-features = fals [dev-dependencies] sp-core = { workspace = true, features = ["std"] } sp-io = { workspace = true, features = ["std"] } +pallet-balances = { workspace = true, features = ["std"] } pallet-preimage = { workspace = true, features = ["std"] } pallet-scheduler = { workspace = true, features = ["std"] } +orml-tokens = { path = "../tokens", version = "0.7.0", features = ["std"] } + +cumulus-pallet-xcm = { workspace = true, features = ["std"] } +orml-xcm-mock-message-queue = { path = "../xcm-mock-message-queue" } +orml-xcm = { path = "../xcm" } +orml-xcm-support = { path = "../xcm-support" } +pallet-xcm = { workspace = true, features = ["std"] } +xcm-builder = { workspace = true, features = ["std"] } +xcm-executor = { workspace = true, features = ["std"] } +polkadot-parachain-primitives = { workspace = true, features = ["std"] } +polkadot-runtime-parachains = { workspace = true, features = ["std"] } +polkadot-runtime-common = { workspace = true, features = ["std"] } +xcm-simulator = { workspace = true } [features] default = [ "std" ] @@ -43,6 +57,11 @@ std = [ "sp-std/std", "xcm/std", ] +runtime-benchmarks = [ + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks", +] try-runtime = [ "frame-support/try-runtime", "frame-system/try-runtime", diff --git a/delay-tasks/src/lib.rs b/delay-tasks/src/lib.rs index d441c2cb2..97c758741 100644 --- a/delay-tasks/src/lib.rs +++ b/delay-tasks/src/lib.rs @@ -10,7 +10,7 @@ use frame_support::{ }; use frame_system::pallet_prelude::*; use orml_traits::{ - task::{DelayTaskHooks, DelayTasksManager, DispatchableTask, TaskResult}, + task::{DelayTaskHooks, DelayTasksManager, DispatchableTask}, MultiCurrency, NamedMultiReservableCurrency, }; use parity_scale_codec::FullCodec; @@ -38,7 +38,7 @@ pub struct EnsureDelayed; impl> + From> EnsureOrigin for EnsureDelayed { type Success = (); fn try_origin(o: O) -> Result { - o.into().and_then(|_| Ok(())) + o.into().map(|_| ()) } #[cfg(feature = "runtime-benchmarks")] @@ -98,6 +98,8 @@ pub mod module { InvalidDelayBlock, InvalidId, FailedToSchedule, + AssetIndexNonExistent, + AssetConvertFailed, } #[pallet::event] @@ -119,6 +121,10 @@ pub mod module { DelayedTaskCanceled { id: Nonce, }, + DelayedTaskStuck { + id: Nonce, + error: DispatchError, + }, } #[pallet::pallet] @@ -143,12 +149,21 @@ pub mod module { pub fn delayed_execute(origin: OriginFor, id: Nonce) -> DispatchResult { T::DelayOrigin::ensure_origin(origin)?; - let execute_result = Self::do_delayed_execute(id)?; + let (delayed_task, _) = DelayedTasks::::get(id).ok_or(Error::::InvalidId)?; + + // pre delayed execute + if let Err(error) = T::DelayTaskHooks::pre_delayed_execute(&delayed_task) { + Self::deposit_event(Event::::DelayedTaskStuck { id, error }); + } else { + let execute_result = delayed_task.dispatch(Weight::zero()); + + DelayedTasks::::remove(id); + Self::deposit_event(Event::::DelayedTaskExecuted { + id, + result: execute_result.result, + }); + } - Self::deposit_event(Event::::DelayedTaskExecuted { - id, - result: execute_result.result, - }); Ok(()) } @@ -171,11 +186,11 @@ pub mod module { }; ensure!(new_execute_block > now, Error::::InvalidDelayBlock); - *execute_block = new_execute_block; - T::Scheduler::reschedule_named((&DELAY_TASK_ID, id).encode(), DispatchTime::At(new_execute_block)) .map_err(|_| Error::::FailedToSchedule)?; + *execute_block = new_execute_block; + Self::deposit_event(Event::::DelayedTaskReDelayed { id, execute_block: new_execute_block, @@ -188,15 +203,19 @@ pub mod module { #[pallet::call_index(2)] #[pallet::weight(Weight::zero())] - pub fn cancel_delayed_task(origin: OriginFor, id: Nonce) -> DispatchResult { + pub fn cancel_delayed_task(origin: OriginFor, id: Nonce, skip_pre_cancel: bool) -> DispatchResult { T::GovernanceOrigin::ensure_origin(origin)?; - let (task, _) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; + let (task, execute_block) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; - // pre cancel - T::DelayTaskHooks::on_cancel(&task)?; + if !skip_pre_cancel { + T::DelayTaskHooks::pre_cancel(&task)?; + } - T::Scheduler::cancel_named((&DELAY_TASK_ID, id).encode()).map_err(|_| Error::::FailedToSchedule)?; + if frame_system::Pallet::::block_number() < execute_block { + // if now < execute_block, need cancel scheduler + T::Scheduler::cancel_named((&DELAY_TASK_ID, id).encode()).map_err(|_| Error::::FailedToSchedule)?; + } Self::deposit_event(Event::::DelayedTaskCanceled { id }); Ok(()) @@ -204,15 +223,6 @@ pub mod module { } impl Pallet { - pub(crate) fn do_delayed_execute(id: Nonce) -> sp_std::result::Result { - let (delayed_task, _) = DelayedTasks::::take(id).ok_or(Error::::InvalidId)?; - - // pre delayed dispatch - T::DelayTaskHooks::pre_delayed_execute(&delayed_task)?; - - Ok(delayed_task.dispatch(Weight::zero())) - } - /// Retrieves the next delayed task ID from storage, and increment it by /// one. fn get_next_delayed_task_id() -> Result { @@ -261,10 +271,33 @@ pub mod module { } pub struct DelayedXtokensTaskHooks(PhantomData); - impl DelayTaskHooks> for DelayedXtokensTaskHooks { + impl DelayTaskHooks> for DelayedXtokensTaskHooks + where + ::Currency: MultiCurrency< + T::AccountId, + CurrencyId = ::CurrencyId, + Balance = ::Balance, + >, + { fn pre_delay(task: &orml_xtokens::XtokensTask) -> DispatchResult { match task { - orml_xtokens::XtokensTask::::TransferAssets { who, assets, fee, .. } => {} + orml_xtokens::XtokensTask::::TransferAssets { who, assets, .. } => { + let asset_len = assets.len(); + for i in 0..asset_len { + let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; + let currency_id: >::CurrencyId = + ::CurrencyIdConvert::convert(asset.id.0.clone()) + .ok_or(Error::::AssetConvertFailed)?; + let amount: T::Balance = match asset.fun { + Fungibility::Fungible(amount) => { + amount.try_into().map_err(|_| Error::::AssetConvertFailed)? + } + Fungibility::NonFungible(_) => return Err(Error::::AssetConvertFailed.into()), + }; + + T::Currency::reserve_named(&T::ReserveId::get(), currency_id, who, amount)?; + } + } } Ok(()) @@ -272,18 +305,30 @@ pub mod module { fn pre_delayed_execute(task: &orml_xtokens::XtokensTask) -> DispatchResult { match task { - orml_xtokens::XtokensTask::::TransferAssets { who, assets, fee, .. } => {} + orml_xtokens::XtokensTask::::TransferAssets { who, assets, .. } => { + let asset_len = assets.len(); + for i in 0..asset_len { + let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; + let currency_id: >::CurrencyId = + ::CurrencyIdConvert::convert(asset.id.0.clone()) + .ok_or(Error::::AssetConvertFailed)?; + let amount: T::Balance = match asset.fun { + Fungibility::Fungible(amount) => { + amount.try_into().map_err(|_| Error::::AssetConvertFailed)? + } + Fungibility::NonFungible(_) => return Err(Error::::AssetConvertFailed.into()), + }; + + T::Currency::unreserve_named(&T::ReserveId::get(), currency_id, who, amount); + } + } } Ok(()) } - fn on_cancel(task: &orml_xtokens::XtokensTask) -> DispatchResult { - match task { - orml_xtokens::XtokensTask::::TransferAssets { who, assets, fee, .. } => {} - } - - Ok(()) + fn pre_cancel(task: &orml_xtokens::XtokensTask) -> DispatchResult { + Self::pre_delayed_execute(task) } } } diff --git a/delay-tasks/src/mock.rs b/delay-tasks/src/mock.rs index 61ea16e0e..08b16531c 100644 --- a/delay-tasks/src/mock.rs +++ b/delay-tasks/src/mock.rs @@ -3,15 +3,25 @@ #![cfg(test)] use super::*; -use frame_support::{construct_runtime, derive_impl, parameter_types, traits::EqualPrivilegeOnly}; +use frame_support::{ + construct_runtime, derive_impl, parameter_types, + traits::{ConstU128, EqualPrivilegeOnly, Everything}, +}; use frame_system::EnsureRoot; -use orml_traits::define_combined_task; -use sp_runtime::{traits::IdentityLookup, BuildStorage, DispatchError}; +use orml_traits::{define_combined_task_and_bind_delay_hooks, parameter_type_with_key, task::TaskResult}; +use serde::{Deserialize, Serialize}; +use sp_runtime::{traits::IdentityLookup, AccountId32, BuildStorage, DispatchError}; use sp_std::cell::RefCell; use crate as delay_tasks; -pub type AccountId = u128; +pub type AccountId = AccountId32; +pub type Amount = i128; +pub type Balance = u128; +pub type ReserveIdentifier = [u8; 8]; + +pub const ALICE: AccountId32 = AccountId32::new([0u8; 32]); +pub const BOB: AccountId32 = AccountId32::new([1u8; 32]); #[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] impl frame_system::Config for Runtime { @@ -19,13 +29,30 @@ impl frame_system::Config for Runtime { type AccountId = AccountId; type Lookup = IdentityLookup; type Block = Block; + type AccountData = pallet_balances::AccountData; +} + +impl pallet_balances::Config for Runtime { + type MaxLocks = ConstU32<50>; + type Balance = Balance; + type RuntimeEvent = RuntimeEvent; + type DustRemoval = (); + type ExistentialDeposit = ConstU128<1>; + type AccountStore = System; + type WeightInfo = (); + type MaxReserves = ConstU32<50>; + type ReserveIdentifier = [u8; 8]; + type RuntimeHoldReason = RuntimeHoldReason; + type RuntimeFreezeReason = RuntimeFreezeReason; + type FreezeIdentifier = [u8; 8]; + type MaxFreezes = (); } impl pallet_preimage::Config for Runtime { type RuntimeEvent = RuntimeEvent; type WeightInfo = (); type Currency = (); - type ManagerOrigin = EnsureRoot; + type ManagerOrigin = EnsureRoot; type Consideration = (); } @@ -41,31 +68,132 @@ impl pallet_scheduler::Config for Runtime { type PalletsOrigin = OriginCaller; type RuntimeCall = RuntimeCall; type MaximumWeight = MaximumSchedulerWeight; - type ScheduleOrigin = EnsureRoot; + type ScheduleOrigin = EnsureRoot; type MaxScheduledPerBlock = ConstU32<10>; type WeightInfo = (); type OriginPrivilegeCmp = EqualPrivilegeOnly; type Preimages = Preimage; } -thread_local! { - pub static SUCCEEDED: RefCell = RefCell::new(0); - pub static FAILED: RefCell = RefCell::new(0); +#[derive( + Encode, + Decode, + Eq, + PartialEq, + Copy, + Clone, + RuntimeDebug, + PartialOrd, + Ord, + parity_scale_codec::MaxEncodedLen, + TypeInfo, +)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub enum CurrencyId { + /// Relay chain token. + R, + /// Parachain A token. + A, + /// Parachain B token. + B, } -define_combined_task! { - #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] - pub enum MockTaskType { - Success(SuccessTask), - Fail(FailTask), +pub struct CurrencyIdConvert; +impl Convert> for CurrencyIdConvert { + fn convert(id: CurrencyId) -> Option { + match id { + CurrencyId::R => Some(Parent.into()), + CurrencyId::A => Some( + ( + Parent, + Parachain(1), + Junction::from(BoundedVec::try_from(b"A".to_vec()).unwrap()), + ) + .into(), + ), + CurrencyId::B => Some( + ( + Parent, + Parachain(2), + Junction::from(BoundedVec::try_from(b"B".to_vec()).unwrap()), + ) + .into(), + ), + } } } +impl Convert> for CurrencyIdConvert { + fn convert(l: Location) -> Option { + let mut a: Vec = "A".into(); + a.resize(32, 0); + let mut b: Vec = "B".into(); + b.resize(32, 0); + + if l == Location::parent() { + return Some(CurrencyId::R); + } + match l.unpack() { + (parents, interior) if parents == 1 => match interior { + [Parachain(1), GeneralKey { data, .. }] if data.to_vec() == a => Some(CurrencyId::A), + [Parachain(2), GeneralKey { data, .. }] if data.to_vec() == b => Some(CurrencyId::B), + _ => None, + }, + (parents, interior) if parents == 0 => match interior { + [GeneralKey { data, .. }] if data.to_vec() == a => Some(CurrencyId::A), + [GeneralKey { data, .. }] if data.to_vec() == b => Some(CurrencyId::B), + _ => None, + }, + _ => None, + } + } +} +impl Convert> for CurrencyIdConvert { + fn convert(a: Asset) -> Option { + if let Asset { + fun: Fungible(_), + id: AssetId(id), + } = a + { + Self::convert(id) + } else { + Option::None + } + } +} + +parameter_type_with_key! { + pub ExistentialDeposits: |_currency_id: CurrencyId| -> Balance { + Default::default() + }; +} + +impl orml_tokens::Config for Runtime { + type RuntimeEvent = RuntimeEvent; + type Balance = Balance; + type Amount = Amount; + type CurrencyId = CurrencyId; + type WeightInfo = (); + type ExistentialDeposits = ExistentialDeposits; + type CurrencyHooks = (); + type MaxLocks = ConstU32<50>; + type MaxReserves = ConstU32<50>; + type ReserveIdentifier = ReserveIdentifier; + type DustRemovalWhitelist = Everything; +} + +thread_local! { + pub static DISPATCH_SUCCEEDED: RefCell = RefCell::new(0); + pub static DISPATCH_FAILED: RefCell = RefCell::new(0); + pub static PRE_DELAY_SUCCEEDED: RefCell = RefCell::new(0); + pub static PRE_DELAYED_EXECUTE_SUCCEEDED: RefCell = RefCell::new(0); + pub static PRE_CANCEL_SUCCEEDED: RefCell = RefCell::new(0); +} #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] pub struct SuccessTask; impl DispatchableTask for SuccessTask { fn dispatch(self, _weight: Weight) -> TaskResult { - SUCCEEDED.with(|v| *v.borrow_mut() += 1); + DISPATCH_SUCCEEDED.with(|v| *v.borrow_mut() += 1); TaskResult { result: Ok(()), @@ -74,22 +202,137 @@ impl DispatchableTask for SuccessTask { } } } +pub struct SuccessTaskHook; +impl DelayTaskHooks for SuccessTaskHook { + fn pre_delay(_: &SuccessTask) -> DispatchResult { + PRE_DELAY_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } + fn pre_delayed_execute(_: &SuccessTask) -> DispatchResult { + PRE_DELAYED_EXECUTE_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } + fn pre_cancel(_: &SuccessTask) -> DispatchResult { + PRE_CANCEL_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } +} #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] -pub struct FailTask; -impl DispatchableTask for FailTask { +pub struct FailDispatchTask; +impl DispatchableTask for FailDispatchTask { fn dispatch(self, _weight: Weight) -> TaskResult { - FAILED.with(|v| *v.borrow_mut() += 1); + DISPATCH_FAILED.with(|v| *v.borrow_mut() += 1); TaskResult { - result: Err(DispatchError::Other("execute failed")), + result: Err(DispatchError::Other("dispatch failed")), used_weight: Weight::zero(), finished: true, } } } +pub struct FailDispatchTaskHook; +impl DelayTaskHooks for FailDispatchTaskHook { + fn pre_delay(_: &FailDispatchTask) -> DispatchResult { + PRE_DELAY_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } + fn pre_delayed_execute(_: &FailDispatchTask) -> DispatchResult { + PRE_DELAYED_EXECUTE_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } + fn pre_cancel(_: &FailDispatchTask) -> DispatchResult { + PRE_CANCEL_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } +} + +#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] +pub struct FailPreDelayTask; +impl DispatchableTask for FailPreDelayTask { + fn dispatch(self, _weight: Weight) -> TaskResult { + unimplemented!() + } +} +pub struct FailPreDelayTaskHook; +impl DelayTaskHooks for FailPreDelayTaskHook { + fn pre_delay(_: &FailPreDelayTask) -> DispatchResult { + Err(DispatchError::Other("pre_delay failed")) + } + fn pre_delayed_execute(_: &FailPreDelayTask) -> DispatchResult { + unimplemented!() + } + fn pre_cancel(_: &FailPreDelayTask) -> DispatchResult { + unimplemented!() + } +} + +#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] +pub struct FailPreDelayedExecuteTask; +impl DispatchableTask for FailPreDelayedExecuteTask { + fn dispatch(self, _weight: Weight) -> TaskResult { + unimplemented!() + } +} +pub struct FailPreDelayedExecuteTaskHook; +impl DelayTaskHooks for FailPreDelayedExecuteTaskHook { + fn pre_delay(_: &FailPreDelayedExecuteTask) -> DispatchResult { + PRE_DELAY_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } + fn pre_delayed_execute(_: &FailPreDelayedExecuteTask) -> DispatchResult { + Err(DispatchError::Other("pre_delayed_execute failed")) + } + fn pre_cancel(_: &FailPreDelayedExecuteTask) -> DispatchResult { + PRE_CANCEL_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } +} + +#[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] +pub struct FailPreCancelTask; +impl DispatchableTask for FailPreCancelTask { + fn dispatch(self, _weight: Weight) -> TaskResult { + DISPATCH_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + + TaskResult { + result: Ok(()), + used_weight: Weight::zero(), + finished: true, + } + } +} +pub struct FailPreCancelTaskHook; +impl DelayTaskHooks for FailPreCancelTaskHook { + fn pre_delay(_: &FailPreCancelTask) -> DispatchResult { + PRE_DELAY_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } + fn pre_delayed_execute(_: &FailPreCancelTask) -> DispatchResult { + PRE_DELAYED_EXECUTE_SUCCEEDED.with(|v| *v.borrow_mut() += 1); + Ok(()) + } + fn pre_cancel(_: &FailPreCancelTask) -> DispatchResult { + Err(DispatchError::Other("pre_cancel failed")) + } +} + +define_combined_task_and_bind_delay_hooks! { + #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + pub enum MockTaskType { + Success(SuccessTask, SuccessTaskHook), + FailDispatch(FailDispatchTask, FailDispatchTaskHook), + FailPreDelay(FailPreDelayTask, FailPreDelayTaskHook), + FailPreDelayedExecute(FailPreDelayedExecuteTask, FailPreDelayedExecuteTaskHook), + FailPreCancel(FailPreCancelTask, FailPreCancelTaskHook), + } +} + +parameter_types! { + pub ReserveId: ReserveIdentifier = [1u8;8]; +} -impl Config for Runtime { +impl delay_tasks::Config for Runtime { type RuntimeEvent = RuntimeEvent; type RuntimeCall = RuntimeCall; type RuntimeOrigin = RuntimeOrigin; @@ -98,6 +341,10 @@ impl Config for Runtime { type GovernanceOrigin = EnsureRoot; type Task = MockTaskType; type Scheduler = Scheduler; + type DelayTaskHooks = MockTaskType; + type CurrencyIdConvert = CurrencyIdConvert; + type Currency = Tokens; + type ReserveId = ReserveId; } type Block = frame_system::mocking::MockBlock; @@ -108,6 +355,8 @@ construct_runtime!( DelayTasks: delay_tasks, Scheduler: pallet_scheduler, Preimage: pallet_preimage, + Tokens: orml_tokens, + Balances: pallet_balances, } ); diff --git a/delay-tasks/src/tests.rs b/delay-tasks/src/tests.rs index 7fb2192bc..816e81e1e 100644 --- a/delay-tasks/src/tests.rs +++ b/delay-tasks/src/tests.rs @@ -11,14 +11,20 @@ use sp_runtime::traits::{BadOrigin, Bounded}; fn add_delay_task_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); - assert_eq!(DelayTasks::next_delayed_task_id(), 0); - assert_eq!(DelayTasks::delayed_tasks(0), None); assert_noop!( DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 0), Error::::InvalidDelayBlock ); + assert_noop!( + DelayTasks::add_delay_task(MockTaskType::FailPreDelay(FailPreDelayTask), 0), + Error::::InvalidDelayBlock + ); + + assert_eq!(PRE_DELAY_SUCCEEDED.with(|v| *v.borrow()), 0); + assert_eq!(DelayTasks::next_delayed_task_id(), 0); + assert_eq!(DelayTasks::delayed_tasks(0), None); assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 10)); System::assert_has_event(mock::RuntimeEvent::Scheduler( pallet_scheduler::Event::::Scheduled { when: 11, index: 0 }, @@ -29,6 +35,7 @@ fn add_delay_task_work() { execute_block: 11, })); + assert_eq!(PRE_DELAY_SUCCEEDED.with(|v| *v.borrow()), 1); assert_eq!(DelayTasks::next_delayed_task_id(), 1); assert_eq!( DelayTasks::delayed_tasks(0), @@ -42,18 +49,26 @@ fn reschedule_delay_task_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 100)); + assert_ok!(DelayTasks::add_delay_task( + MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), + 9 + )); assert_eq!( DelayTasks::delayed_tasks(0), Some((MockTaskType::Success(SuccessTask), 101)) ); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some((MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), 10)) + ); assert_noop!( - DelayTasks::reschedule_delay_task(RuntimeOrigin::signed(1), 0, DispatchTime::At(10)), + DelayTasks::reschedule_delay_task(RuntimeOrigin::signed(ALICE), 0, DispatchTime::At(10)), BadOrigin ); assert_noop!( - DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 1, DispatchTime::At(10)), + DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 2, DispatchTime::At(10)), Error::::InvalidId ); @@ -83,30 +98,31 @@ fn reschedule_delay_task_work() { Some((MockTaskType::Success(SuccessTask), 10)) ); - System::set_block_number(100); - assert_noop!( - DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 0, DispatchTime::At(100)), - Error::::InvalidDelayBlock - ); + run_to_block(10); + assert_eq!(DelayTasks::delayed_tasks(0), None); - assert_noop!( - DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 0, DispatchTime::After(0)), - Error::::InvalidDelayBlock + // scheduler dispatched delayed_execute call for task#1, + // but task#1 stuck for failed at pre_delayed_execute + assert_eq!( + DelayTasks::delayed_tasks(1), + Some((MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), 10)) ); - - assert_ok!(DelayTasks::reschedule_delay_task( - RuntimeOrigin::root(), - 0, - DispatchTime::After(1) + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Dispatched { + task: (10, 0), + id: Some(blake2_256(&(&DELAY_TASK_ID, 1u64).encode())), + result: Ok(()), + }, )); - System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskReDelayed { - id: 0, - execute_block: 101, + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskStuck { + id: 1, + error: DispatchError::Other(""), })); - assert_eq!( - DelayTasks::delayed_tasks(0), - Some((MockTaskType::Success(SuccessTask), 101)) + // cannot rescheduler stucked task + assert_noop!( + DelayTasks::reschedule_delay_task(RuntimeOrigin::root(), 1, DispatchTime::At(100)), + Error::::FailedToSchedule ); }); } @@ -116,77 +132,96 @@ fn cancel_delayed_task_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 100)); + assert_ok!(DelayTasks::add_delay_task( + MockTaskType::FailPreCancel(FailPreCancelTask), + 100 + )); + assert_ok!(DelayTasks::add_delay_task( + MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), + 100 + )); assert_eq!( DelayTasks::delayed_tasks(0), Some((MockTaskType::Success(SuccessTask), 101)) ); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some((MockTaskType::FailPreCancel(FailPreCancelTask), 101)) + ); + assert_eq!( + DelayTasks::delayed_tasks(2), + Some((MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), 101)) + ); - assert_noop!(DelayTasks::cancel_delayed_task(RuntimeOrigin::signed(1), 0), BadOrigin); + assert_noop!( + DelayTasks::cancel_delayed_task(RuntimeOrigin::signed(ALICE), 0, false), + BadOrigin + ); assert_noop!( - DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 2), + DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 3, false), Error::::InvalidId ); - assert_ok!(DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 0)); + assert_eq!(PRE_CANCEL_SUCCEEDED.with(|v| *v.borrow()), 0); + assert_ok!(DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 0, false)); System::assert_has_event(mock::RuntimeEvent::Scheduler( pallet_scheduler::Event::::Canceled { when: 101, index: 0 }, )); System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskCanceled { id: 0 })); assert_eq!(DelayTasks::delayed_tasks(0), None); - }); -} + assert_eq!(PRE_CANCEL_SUCCEEDED.with(|v| *v.borrow()), 1); -#[test] -fn do_delayed_execute_work() { - ExtBuilder::default().build().execute_with(|| { - System::set_block_number(1); + // failed cancel for failed on pre_cancel + assert_noop!( + DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 1, false), + DispatchError::Other("pre_cancel failed"), + ); - assert_noop!(DelayTasks::do_delayed_execute(0), Error::::InvalidId); + // cancel by skip pre_cancel + assert_ok!(DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 1, true)); + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Canceled { when: 101, index: 1 }, + )); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskCanceled { id: 1 })); + assert_eq!(DelayTasks::delayed_tasks(1), None); + assert_eq!(PRE_CANCEL_SUCCEEDED.with(|v| *v.borrow()), 1); // skip pre_cancel - assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 100)); - assert_ok!(DelayTasks::add_delay_task(MockTaskType::Fail(FailTask), 100)); + run_to_block(101); + // scheduler dispatched delayed_execute call for task#2, + // but task#2 stuck for failed at pre_delayed_execute assert_eq!( - DelayTasks::delayed_tasks(0), - Some((MockTaskType::Success(SuccessTask), 101)) + DelayTasks::delayed_tasks(2), + Some((MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), 101)) ); - assert_eq!(DelayTasks::delayed_tasks(1), Some((MockTaskType::Fail(FailTask), 101))); - - assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 0); - assert_eq!( - DelayTasks::do_delayed_execute(0), - Ok(TaskResult { + System::assert_has_event(mock::RuntimeEvent::Scheduler( + pallet_scheduler::Event::::Dispatched { + task: (101, 2), + id: Some(blake2_256(&(&DELAY_TASK_ID, 2u64).encode())), result: Ok(()), - used_weight: Weight::zero(), - finished: true, - }) - ); - assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 1); - assert_eq!(DelayTasks::delayed_tasks(0), None); + }, + )); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskStuck { + id: 2, + error: DispatchError::Other(""), + })); - assert_eq!(FAILED.with(|v| *v.borrow()), 0); - assert_eq!( - DelayTasks::do_delayed_execute(1), - Ok(TaskResult { - result: Err(DispatchError::Other("execute failed").into()), - used_weight: Weight::zero(), - finished: true, - }) - ); - assert_eq!(FAILED.with(|v| *v.borrow()), 1); - assert_eq!(DelayTasks::delayed_tasks(1), None); + // cancel stuck task#2 + assert_ok!(DelayTasks::cancel_delayed_task(RuntimeOrigin::root(), 2, false)); + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskCanceled { id: 2 })); + assert_eq!(DelayTasks::delayed_tasks(2), None); + assert_eq!(PRE_CANCEL_SUCCEEDED.with(|v| *v.borrow()), 2); }); } #[test] -fn delayed_execute_work() { +fn do_delayed_execute_work() { ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); assert_noop!(DelayTasks::delayed_execute(RuntimeOrigin::root(), 0), BadOrigin); - - assert_noop!(DelayTasks::delayed_execute(RuntimeOrigin::signed(1), 0), BadOrigin); + assert_noop!(DelayTasks::delayed_execute(RuntimeOrigin::signed(ALICE), 0), BadOrigin); assert_noop!( DelayTasks::delayed_execute(RuntimeOrigin::from(DelayedExecuteOrigin), 0), @@ -194,15 +229,33 @@ fn delayed_execute_work() { ); assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 100)); - assert_ok!(DelayTasks::add_delay_task(MockTaskType::Fail(FailTask), 100)); + assert_ok!(DelayTasks::add_delay_task( + MockTaskType::FailDispatch(FailDispatchTask), + 100 + )); + assert_ok!(DelayTasks::add_delay_task( + MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), + 100 + )); assert_eq!( DelayTasks::delayed_tasks(0), Some((MockTaskType::Success(SuccessTask), 101)) ); - assert_eq!(DelayTasks::delayed_tasks(1), Some((MockTaskType::Fail(FailTask), 101))); + assert_eq!( + DelayTasks::delayed_tasks(1), + Some((MockTaskType::FailDispatch(FailDispatchTask), 101)) + ); + assert_eq!( + DelayTasks::delayed_tasks(2), + Some((MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), 101)) + ); - assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 0); + assert_eq!(DISPATCH_SUCCEEDED.with(|v| *v.borrow()), 0); + assert_eq!(DISPATCH_FAILED.with(|v| *v.borrow()), 0); + assert_eq!(PRE_DELAYED_EXECUTE_SUCCEEDED.with(|v| *v.borrow()), 0); + + // delayed task executed, and succeeded assert_ok!(DelayTasks::delayed_execute( RuntimeOrigin::from(DelayedExecuteOrigin), 0 @@ -211,67 +264,40 @@ fn delayed_execute_work() { id: 0, result: Ok(()), })); - assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 1); assert_eq!(DelayTasks::delayed_tasks(0), None); + assert_eq!(DISPATCH_SUCCEEDED.with(|v| *v.borrow()), 1); + assert_eq!(DISPATCH_FAILED.with(|v| *v.borrow()), 0); + assert_eq!(PRE_DELAYED_EXECUTE_SUCCEEDED.with(|v| *v.borrow()), 1); - assert_eq!(FAILED.with(|v| *v.borrow()), 0); + // delayed task executed, and failed assert_ok!(DelayTasks::delayed_execute( RuntimeOrigin::from(DelayedExecuteOrigin), 1 )); System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { id: 1, - result: Err(DispatchError::Other("").into()), + result: Err(DispatchError::Other("")), })); - assert_eq!(FAILED.with(|v| *v.borrow()), 1); assert_eq!(DelayTasks::delayed_tasks(1), None); - }); -} + assert_eq!(DISPATCH_SUCCEEDED.with(|v| *v.borrow()), 1); + assert_eq!(DISPATCH_FAILED.with(|v| *v.borrow()), 1); + assert_eq!(PRE_DELAYED_EXECUTE_SUCCEEDED.with(|v| *v.borrow()), 2); -#[test] -fn dispatch_delayed_execute_work() { - ExtBuilder::default().build().execute_with(|| { - System::set_block_number(1); - - assert_ok!(DelayTasks::add_delay_task(MockTaskType::Success(SuccessTask), 10)); - assert_ok!(DelayTasks::add_delay_task(MockTaskType::Fail(FailTask), 10)); - - assert_eq!( - DelayTasks::delayed_tasks(0), - Some((MockTaskType::Success(SuccessTask), 11)) - ); - assert_eq!(DelayTasks::delayed_tasks(1), Some((MockTaskType::Fail(FailTask), 11))); - - assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 0); - assert_eq!(FAILED.with(|v| *v.borrow()), 0); - run_to_block(11); - - System::assert_has_event(mock::RuntimeEvent::Scheduler( - pallet_scheduler::Event::::Dispatched { - task: (11, 0), - id: Some(blake2_256(&(&DELAY_TASK_ID, 0u64).encode())), - result: Ok(()), - }, - )); - System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { - id: 0, - result: Ok(()), - })); - System::assert_has_event(mock::RuntimeEvent::Scheduler( - pallet_scheduler::Event::::Dispatched { - task: (11, 1), - id: Some(blake2_256(&(&DELAY_TASK_ID, 1u64).encode())), - result: Ok(()), - }, + // delayed task stuck for failed pre_delayed_execute + assert_ok!(DelayTasks::delayed_execute( + RuntimeOrigin::from(DelayedExecuteOrigin), + 2 )); - System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskExecuted { - id: 1, - result: Err(DispatchError::Other("").into()), + System::assert_has_event(mock::RuntimeEvent::DelayTasks(Event::DelayedTaskStuck { + id: 2, + error: DispatchError::Other(""), })); - - assert_eq!(SUCCEEDED.with(|v| *v.borrow()), 1); - assert_eq!(FAILED.with(|v| *v.borrow()), 1); - assert_eq!(DelayTasks::delayed_tasks(0), None); - assert_eq!(DelayTasks::delayed_tasks(1), None); + assert_eq!( + DelayTasks::delayed_tasks(2), + Some((MockTaskType::FailPreDelayedExecute(FailPreDelayedExecuteTask), 101)) + ); + assert_eq!(DISPATCH_SUCCEEDED.with(|v| *v.borrow()), 1); + assert_eq!(DISPATCH_FAILED.with(|v| *v.borrow()), 1); + assert_eq!(PRE_DELAYED_EXECUTE_SUCCEEDED.with(|v| *v.borrow()), 2); }); } diff --git a/traits/src/task.rs b/traits/src/task.rs index 0a050b311..0cbba710e 100644 --- a/traits/src/task.rs +++ b/traits/src/task.rs @@ -26,6 +26,28 @@ impl DispatchableTask for () { } } +pub trait DelayTasksManager { + fn add_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; +} + +pub trait DelayTaskHooks { + fn pre_delay(task: &Task) -> DispatchResult; + fn pre_delayed_execute(task: &Task) -> DispatchResult; + fn pre_cancel(task: &Task) -> DispatchResult; +} + +impl DelayTaskHooks for () { + fn pre_delay(_: &Task) -> DispatchResult { + Ok(()) + } + fn pre_delayed_execute(_: &Task) -> DispatchResult { + Ok(()) + } + fn pre_cancel(_: &Task) -> DispatchResult { + Ok(()) + } +} + #[macro_export] macro_rules! define_combined_task { ( @@ -63,24 +85,63 @@ macro_rules! define_combined_task { }; } -pub trait DelayTasksManager { - fn add_delay_task(task: Task, delay_blocks: BlockNumber) -> DispatchResult; -} +#[macro_export] +macro_rules! define_combined_task_and_bind_delay_hooks { + ( + $(#[$meta:meta])* + $vis:vis enum $combined_name:ident { + $( + $task:ident ( $vtask:ident $(<$($generic:tt),*>)? , $hook:ty ) + ),+ $(,)? + } + ) => { + $(#[$meta])* + $vis enum $combined_name { + $( + $task($vtask $(<$($generic),*>)?), + )* + } -pub trait DelayTaskHooks { - fn pre_delay(task: &Task) -> DispatchResult; - fn pre_delayed_execute(task: &Task) -> DispatchResult; - fn on_cancel(task: &Task) -> DispatchResult; -} + impl DispatchableTask for $combined_name { + fn dispatch(self, weight: Weight) -> TaskResult { + match self { + $( + $combined_name::$task(t) => t.dispatch(weight), + )* + } + } + } -impl DelayTaskHooks for () { - fn pre_delay(_: &Task) -> DispatchResult { - Ok(()) - } - fn pre_delayed_execute(_: &Task) -> DispatchResult { - Ok(()) - } - fn on_cancel(_: &Task) -> DispatchResult { - Ok(()) - } + impl DelayTaskHooks<$combined_name> for $combined_name { + fn pre_delay(task: &$combined_name) -> DispatchResult { + match task { + $( + $combined_name::$task(t) => <$hook>::pre_delay(t), + )* + } + } + fn pre_delayed_execute(task: &$combined_name) -> DispatchResult { + match task { + $( + $combined_name::$task(t) => <$hook>::pre_delayed_execute(t), + )* + } + } + fn pre_cancel(task: &$combined_name) -> DispatchResult { + match task { + $( + $combined_name::$task(t) => <$hook>::pre_cancel(t), + )* + } + } + } + + $( + impl From<$vtask $(<$($generic),*>)?> for $combined_name { + fn from(t: $vtask $(<$($generic),*>)?) -> Self{ + $combined_name::$task(t) + } + } + )* + }; } From be4e52628d198b5661c6c959eb2f21730f354e44 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Thu, 21 Mar 2024 20:11:54 +0800 Subject: [PATCH 7/8] xtokens consume rate-limiter by CurrencyId --- xtokens/src/lib.rs | 41 +++++++++++++++++++++------------------- xtokens/src/mock/para.rs | 12 ++++-------- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/xtokens/src/lib.rs b/xtokens/src/lib.rs index 06571a7b3..565d94b2b 100644 --- a/xtokens/src/lib.rs +++ b/xtokens/src/lib.rs @@ -144,7 +144,8 @@ pub mod module { type CurrencyId: Parameter + Member + Clone; /// Convert `T::CurrencyId` to `Location`. - type CurrencyIdConvert: Convert>; + type CurrencyIdConvert: Convert> + + Convert>; /// Convert `T::AccountId` to `Location`. type AccountIdToLocation: Convert; @@ -468,25 +469,27 @@ pub mod module { for i in 0..asset_len { let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; - // per asset check - let amount = match asset.fun { - Fungibility::Fungible(amount) => amount, - Fungibility::NonFungible(_) => 1, - }; - - // try consume quota of the rate limiter. - // NOTE: use AssetId as the key, use AccountId as whitelist filter key. - match T::RateLimiter::try_consume(rate_limiter_id, asset.id.clone(), amount, Some(who)) { - Ok(_) => {} - Err(_e @ RateLimiterError::Deny) => { - return Err(Error::::RateLimiterDeny.into()); - } - Err(ref _e @ RateLimiterError::Delay { duration }) => { - if duration > need_delay.unwrap_or_default() { - need_delay = Some(duration); + if let Some(currency_id) = T::CurrencyIdConvert::convert(asset.id.0.clone()) { + // per asset check + let amount = match asset.fun { + Fungibility::Fungible(amount) => amount, + Fungibility::NonFungible(_) => 1, + }; + + // try consume quota of the rate limiter. + // NOTE: use CurrencyId as the key, use AccountId as whitelist filter key. + match T::RateLimiter::try_consume(rate_limiter_id, currency_id, amount, Some(who)) { + Ok(_) => {} + Err(_e @ RateLimiterError::Deny) => { + return Err(Error::::RateLimiterDeny.into()); } - } - }; + Err(ref _e @ RateLimiterError::Delay { duration }) => { + if duration > need_delay.unwrap_or_default() { + need_delay = Some(duration); + } + } + }; + } } Ok(need_delay) diff --git a/xtokens/src/mock/para.rs b/xtokens/src/mock/para.rs index 9c59a4a70..f4752635e 100644 --- a/xtokens/src/mock/para.rs +++ b/xtokens/src/mock/para.rs @@ -248,11 +248,9 @@ impl RateLimiter> for MockRateLimiter { value: u128, ) -> Result<(), RateLimiterError>> { let encoded_limit_key = limit_key.encode(); - let r_multi_location: Location = CurrencyIdConvert::convert(CurrencyId::R).unwrap(); - let r_asset_id = AssetId(r_multi_location); - let encoded_r_asset_id = r_asset_id.encode(); + let encoded_currency_id = CurrencyId::R.encode(); - if encoded_limit_key == encoded_r_asset_id { + if encoded_limit_key == encoded_currency_id { let accumulation = R_ACCUMULATION.with(|v| *v.borrow()); ensure!((accumulation + value) <= 2000, RateLimiterError::Deny); } @@ -262,11 +260,9 @@ impl RateLimiter> for MockRateLimiter { fn consume(_: Self::RateLimiterId, limit_key: impl Encode, value: u128) { let encoded_limit_key = limit_key.encode(); - let r_multi_location: Location = CurrencyIdConvert::convert(CurrencyId::R).unwrap(); - let r_asset_id = AssetId(r_multi_location); - let encoded_r_asset_id = r_asset_id.encode(); + let encoded_currency_id = CurrencyId::R.encode(); - if encoded_limit_key == encoded_r_asset_id { + if encoded_limit_key == encoded_currency_id { R_ACCUMULATION.with(|v| *v.borrow_mut() += value); } } From face78f5236ee63bb4e22306922c112e6388b61b Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Mon, 25 Mar 2024 09:59:28 +0800 Subject: [PATCH 8/8] add tests for XTokensTaskHooks --- delay-tasks/src/lib.rs | 13 ++-- delay-tasks/src/mock.rs | 106 +++++++++++++++++++++++++------ delay-tasks/src/tests.rs | 131 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 224 insertions(+), 26 deletions(-) diff --git a/delay-tasks/src/lib.rs b/delay-tasks/src/lib.rs index 97c758741..7990bf05b 100644 --- a/delay-tasks/src/lib.rs +++ b/delay-tasks/src/lib.rs @@ -13,6 +13,7 @@ use orml_traits::{ task::{DelayTaskHooks, DelayTasksManager, DispatchableTask}, MultiCurrency, NamedMultiReservableCurrency, }; +use orml_xtokens::XtokensTask; use parity_scale_codec::FullCodec; use scale_info::TypeInfo; use sp_runtime::{ @@ -271,7 +272,7 @@ pub mod module { } pub struct DelayedXtokensTaskHooks(PhantomData); - impl DelayTaskHooks> for DelayedXtokensTaskHooks + impl DelayTaskHooks> for DelayedXtokensTaskHooks where ::Currency: MultiCurrency< T::AccountId, @@ -279,9 +280,9 @@ pub mod module { Balance = ::Balance, >, { - fn pre_delay(task: &orml_xtokens::XtokensTask) -> DispatchResult { + fn pre_delay(task: &XtokensTask) -> DispatchResult { match task { - orml_xtokens::XtokensTask::::TransferAssets { who, assets, .. } => { + XtokensTask::::TransferAssets { who, assets, .. } => { let asset_len = assets.len(); for i in 0..asset_len { let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; @@ -303,9 +304,9 @@ pub mod module { Ok(()) } - fn pre_delayed_execute(task: &orml_xtokens::XtokensTask) -> DispatchResult { + fn pre_delayed_execute(task: &XtokensTask) -> DispatchResult { match task { - orml_xtokens::XtokensTask::::TransferAssets { who, assets, .. } => { + XtokensTask::::TransferAssets { who, assets, .. } => { let asset_len = assets.len(); for i in 0..asset_len { let asset = assets.get(i).ok_or(Error::::AssetIndexNonExistent)?; @@ -327,7 +328,7 @@ pub mod module { Ok(()) } - fn pre_cancel(task: &orml_xtokens::XtokensTask) -> DispatchResult { + fn pre_cancel(task: &XtokensTask) -> DispatchResult { Self::pre_delayed_execute(task) } } diff --git a/delay-tasks/src/mock.rs b/delay-tasks/src/mock.rs index 08b16531c..7edd1e1f2 100644 --- a/delay-tasks/src/mock.rs +++ b/delay-tasks/src/mock.rs @@ -5,10 +5,13 @@ use super::*; use frame_support::{ construct_runtime, derive_impl, parameter_types, - traits::{ConstU128, EqualPrivilegeOnly, Everything}, + traits::{EqualPrivilegeOnly, Everything}, }; use frame_system::EnsureRoot; -use orml_traits::{define_combined_task_and_bind_delay_hooks, parameter_type_with_key, task::TaskResult}; +use orml_traits::{ + define_combined_task_and_bind_delay_hooks, location::AbsoluteReserveProvider, parameter_type_with_key, + task::TaskResult, +}; use serde::{Deserialize, Serialize}; use sp_runtime::{traits::IdentityLookup, AccountId32, BuildStorage, DispatchError}; use sp_std::cell::RefCell; @@ -29,23 +32,6 @@ impl frame_system::Config for Runtime { type AccountId = AccountId; type Lookup = IdentityLookup; type Block = Block; - type AccountData = pallet_balances::AccountData; -} - -impl pallet_balances::Config for Runtime { - type MaxLocks = ConstU32<50>; - type Balance = Balance; - type RuntimeEvent = RuntimeEvent; - type DustRemoval = (); - type ExistentialDeposit = ConstU128<1>; - type AccountStore = System; - type WeightInfo = (); - type MaxReserves = ConstU32<50>; - type ReserveIdentifier = [u8; 8]; - type RuntimeHoldReason = RuntimeHoldReason; - type RuntimeFreezeReason = RuntimeFreezeReason; - type FreezeIdentifier = [u8; 8]; - type MaxFreezes = (); } impl pallet_preimage::Config for Runtime { @@ -181,6 +167,85 @@ impl orml_tokens::Config for Runtime { type DustRemovalWhitelist = Everything; } +parameter_types! { + pub SelfLocation: Location = Location::new(1, [Parachain(2000)]); +} + +pub struct AccountIdToLocation; +impl Convert for AccountIdToLocation { + fn convert(account: AccountId) -> Location { + [Junction::AccountId32 { + network: None, + id: account.into(), + }] + .into() + } +} + +parameter_type_with_key! { + pub ParachainMinFee: |location: Location| -> Option { + #[allow(clippy::match_ref_pats)] // false positive + match (location.parents, location.first_interior()) { + (1, Some(Parachain(3))) => Some(100), + _ => None, + } + }; +} + +pub enum Weightless {} +impl PreparedMessage for Weightless { + fn weight_of(&self) -> Weight { + unreachable!() + } +} + +pub struct MockExec; +impl ExecuteXcm for MockExec { + type Prepared = Weightless; + + fn prepare(_message: Xcm) -> Result> { + unreachable!() + } + + fn execute(_origin: impl Into, _pre: Weightless, _hash: &mut XcmHash, _weight_credit: Weight) -> Outcome { + unreachable!() + } + + fn charge_fees(_location: impl Into, _fees: Assets) -> XcmResult { + Err(XcmError::Unimplemented) + } +} + +parameter_types! { + pub UniversalLocation: InteriorLocation = Here; + pub const UnitWeightCost: Weight = Weight::from_parts(10, 10); + pub const BaseXcmWeight: Weight = Weight::from_parts(100_000_000, 100_000_000); + pub const MaxInstructions: u32 = 100; + pub const MaxAssetsIntoHolding: u32 = 64; + pub const MaxAssetsForTransfer: usize = 2; +} + +impl orml_xtokens::Config for Runtime { + type RuntimeEvent = RuntimeEvent; + type Balance = Balance; + type CurrencyId = CurrencyId; + type CurrencyIdConvert = CurrencyIdConvert; + type AccountIdToLocation = AccountIdToLocation; + type SelfLocation = SelfLocation; + type XcmExecutor = MockExec; + type Weigher = xcm_builder::FixedWeightBounds; + type BaseXcmWeight = BaseXcmWeight; + type UniversalLocation = UniversalLocation; + type MaxAssetsForTransfer = MaxAssetsForTransfer; + type MinXcmFee = ParachainMinFee; + type LocationsFilter = Everything; + type ReserveProvider = AbsoluteReserveProvider; + type RateLimiter = (); + type RateLimiterId = (); + type Task = MockTaskType; + type DelayTasks = DelayTasks; +} + thread_local! { pub static DISPATCH_SUCCEEDED: RefCell = RefCell::new(0); pub static DISPATCH_FAILED: RefCell = RefCell::new(0); @@ -325,6 +390,7 @@ define_combined_task_and_bind_delay_hooks! { FailPreDelay(FailPreDelayTask, FailPreDelayTaskHook), FailPreDelayedExecute(FailPreDelayedExecuteTask, FailPreDelayedExecuteTaskHook), FailPreCancel(FailPreCancelTask, FailPreCancelTaskHook), + Xtokens(XtokensTask, DelayedXtokensTaskHooks), } } @@ -356,7 +422,7 @@ construct_runtime!( Scheduler: pallet_scheduler, Preimage: pallet_preimage, Tokens: orml_tokens, - Balances: pallet_balances, + XTokens: orml_xtokens, } ); diff --git a/delay-tasks/src/tests.rs b/delay-tasks/src/tests.rs index 816e81e1e..0c720c497 100644 --- a/delay-tasks/src/tests.rs +++ b/delay-tasks/src/tests.rs @@ -301,3 +301,134 @@ fn do_delayed_execute_work() { assert_eq!(PRE_DELAYED_EXECUTE_SUCCEEDED.with(|v| *v.borrow()), 2); }); } + +#[test] +fn delayed_xtokens_task_hooks_work() { + ExtBuilder::default().build().execute_with(|| { + let assets: Assets = Assets::from(vec![ + (Location::parent(), 1000).into(), + ( + ( + Parent, + Parachain(1), + Junction::from(BoundedVec::try_from(b"A".to_vec()).unwrap()), + ), + 2000, + ) + .into(), + ( + ( + Parent, + Parachain(2), + Junction::from(BoundedVec::try_from(b"B".to_vec()).unwrap()), + ), + 3000, + ) + .into(), + ]); + let fee: Asset = (Location::parent(), 1000).into(); + let dest: Location = ( + Parent, + Parachain(2), + Junction::AccountId32 { + network: None, + id: BOB.into(), + }, + ) + .into(); + let task = XtokensTask::::TransferAssets { + who: ALICE, + assets, + fee, + dest, + dest_weight_limit: WeightLimit::Unlimited, + }; + + assert_ok!(Tokens::deposit(CurrencyId::R, &ALICE, 3000)); + assert_ok!(Tokens::deposit(CurrencyId::A, &ALICE, 3000)); + assert_ok!(Tokens::deposit(CurrencyId::B, &ALICE, 3000)); + assert_eq!(Tokens::free_balance(CurrencyId::R, &ALICE), 3000); + assert_eq!(Tokens::free_balance(CurrencyId::A, &ALICE), 3000); + assert_eq!(Tokens::free_balance(CurrencyId::B, &ALICE), 3000); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::R, &ALICE), + 0 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::A, &ALICE), + 0 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::B, &ALICE), + 0 + ); + + assert_ok!(DelayedXtokensTaskHooks::::pre_delay(&task)); + assert_eq!(Tokens::free_balance(CurrencyId::R, &ALICE), 2000); + assert_eq!(Tokens::free_balance(CurrencyId::A, &ALICE), 1000); + assert_eq!(Tokens::free_balance(CurrencyId::B, &ALICE), 0); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::R, &ALICE), + 1000 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::A, &ALICE), + 2000 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::B, &ALICE), + 3000 + ); + + assert_ok!(DelayedXtokensTaskHooks::::pre_delayed_execute(&task)); + assert_eq!(Tokens::free_balance(CurrencyId::R, &ALICE), 3000); + assert_eq!(Tokens::free_balance(CurrencyId::A, &ALICE), 3000); + assert_eq!(Tokens::free_balance(CurrencyId::B, &ALICE), 3000); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::R, &ALICE), + 0 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::A, &ALICE), + 0 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::B, &ALICE), + 0 + ); + + assert_ok!(DelayedXtokensTaskHooks::::pre_delay(&task)); + assert_eq!(Tokens::free_balance(CurrencyId::R, &ALICE), 2000); + assert_eq!(Tokens::free_balance(CurrencyId::A, &ALICE), 1000); + assert_eq!(Tokens::free_balance(CurrencyId::B, &ALICE), 0); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::R, &ALICE), + 1000 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::A, &ALICE), + 2000 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::B, &ALICE), + 3000 + ); + + assert_ok!(DelayedXtokensTaskHooks::::pre_cancel(&task)); + assert_eq!(Tokens::free_balance(CurrencyId::R, &ALICE), 3000); + assert_eq!(Tokens::free_balance(CurrencyId::A, &ALICE), 3000); + assert_eq!(Tokens::free_balance(CurrencyId::B, &ALICE), 3000); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::R, &ALICE), + 0 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::A, &ALICE), + 0 + ); + assert_eq!( + Tokens::reserved_balance_named(&ReserveId::get(), CurrencyId::B, &ALICE), + 0 + ); + }); +}