Skip to content

Commit

Permalink
Modify the scheduleAs parameter to be an optional parameter (#499)
Browse files Browse the repository at this point in the history 
* Modify the scheduleAs parameter to be an optional parameter

* Remove unused code
  • Loading branch information
@imstar15
imstar15 authored Jan 6, 2024
1 parent 62b74f2 commit fa94802
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 145 deletions.
2 changes: 1 addition & 1 deletion pallets/automation-time/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ benchmarks! {
.saturating_mul(ED_MULTIPLIER.into())
.saturating_mul(DEPOSIT_MULTIPLIER.into());
let _ = T::MultiCurrency::deposit(currency_id.into(), &caller, foreign_currency_amount);
}: schedule_xcmp_task(RawOrigin::Signed(caller), schedule, Box::new(destination.into()), Box::new(schedule_fee.into()), Box::new(fee), call, Weight::from_parts(1_000, 0), Weight::from_parts(2_000, 0))
}: schedule_xcmp_task(RawOrigin::Signed(caller), schedule, Box::new(destination.into()), Box::new(schedule_fee.into()), Box::new(fee), call, Weight::from_parts(1_000, 0), Weight::from_parts(2_000, 0), InstructionSequence::PayThroughSovereignAccount, None)

schedule_auto_compound_delegated_stake_task_full {
let task_weight = <T as Config>::WeightInfo::run_auto_compound_delegated_stake_task().ref_time();
Expand Down
2 changes: 1 addition & 1 deletion pallets/automation-time/src/fees.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
use crate::{AccountOf, Action, ActionOf, Config, Error, MultiBalanceOf, Pallet};

use frame_support::traits::Get;
use frame_system::RawOrigin;
use orml_traits::MultiCurrency;
use pallet_xcmp_handler::{InstructionSequence, XcmpTransactor};
use sp_runtime::{
Expand Down Expand Up @@ -246,6 +245,7 @@ mod tests {
use codec::Encode;
use frame_benchmarking::frame_support::assert_err;
use frame_support::sp_runtime::AccountId32;
use frame_system::RawOrigin;

#[test]
fn pay_checked_fees_for_success() {
Expand Down
94 changes: 22 additions & 72 deletions pallets/automation-time/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ pub use types::*;

use codec::Decode;
use core::convert::TryInto;
use cumulus_primitives_core::ParaId;
use frame_support::{
dispatch::{GetDispatchInfo, PostDispatchInfo},
pallet_prelude::*,
Expand Down Expand Up @@ -377,6 +376,8 @@ pub mod pallet {
/// * `encoded_call`: Call that will be sent via XCMP to the parachain id provided.
/// * `encoded_call_weight`: Required weight at most the provided call will take.
/// * `overall_weight`: The overall weight in which fees will be paid for XCM instructions.
/// * `instruction_sequence`: The instruction sequence for the XCM call.
/// * `schedule_as`: The real task executor. If it is None, the caller will be the executor.
///
/// # Errors
/// * `InvalidTime`: Time in seconds must be a multiple of SlotSizeSeconds.
Expand All @@ -386,8 +387,11 @@ pub mod pallet {
/// * `TimeSlotFull`: Time slot is full. No more tasks can be scheduled for this time.
/// * `UnsupportedFeePayment`: Unsupported fee payment.
/// * `InvalidAssetLocation` Invalid asset location.
#[pallet::call_index(2)]
#[pallet::weight(<T as Config>::WeightInfo::schedule_xcmp_task_full(schedule.number_of_executions()))]
#[pallet::call_index(1)]
#[pallet::weight(
<T as Config>::WeightInfo::schedule_xcmp_task_full(schedule.number_of_executions())
.saturating_add(T::DbWeight::get().reads(if schedule_as.is_some() { 1 } else { 0 }))
)]
pub fn schedule_xcmp_task(
origin: OriginFor<T>,
schedule: ScheduleParam,
Expand All @@ -397,8 +401,16 @@ pub mod pallet {
encoded_call: Vec<u8>,
encoded_call_weight: Weight,
overall_weight: Weight,
instruction_sequence: InstructionSequence,
schedule_as: Option<T::AccountId>,
) -> DispatchResult {
let who = ensure_signed(origin)?;

// Make sure the owner is the proxy account of the user account.
if let Some(schedule_as_account) = schedule_as.clone() {
T::EnsureProxy::ensure_ok(schedule_as_account, who.clone())?;
}

let destination =
MultiLocation::try_from(*destination).map_err(|()| Error::<T>::BadVersion)?;
let schedule_fee =
Expand All @@ -418,8 +430,8 @@ pub mod pallet {
encoded_call,
encoded_call_weight,
overall_weight,
schedule_as: None,
instruction_sequence: InstructionSequence::PayThroughSovereignAccount,
schedule_as,
instruction_sequence,
};

let schedule = schedule.validated_into::<T>()?;
Expand All @@ -428,68 +440,6 @@ pub mod pallet {
Ok(())
}

/// Schedule a task through XCMP through proxy account to fire an XCMP message with a provided call.
///
/// Before the task can be scheduled the task must past validation checks.
/// * The transaction is signed
/// * The times are valid
/// * The given asset location is supported
///
/// # Parameters
/// * `schedule`: The triggering rules for recurring task or the list of unix standard times in seconds for when the task should run.
/// * `destination`: Destination the XCMP call will be sent to.
/// * `schedule_fee`: The payment asset location required for scheduling automation task.
/// * `execution_fee`: The fee will be paid for XCMP execution.
/// * `encoded_call`: Call that will be sent via XCMP to the parachain id provided.
/// * `encoded_call_weight`: Required weight at most the provided call will take.
/// * `overall_weight`: The overall weight in which fees will be paid for XCM instructions.
///
/// # Errors
/// * `InvalidTime`: Time in seconds must be a multiple of SlotSizeSeconds.
/// * `PastTime`: Time must be in the future.
/// * `DuplicateTask`: There can be no duplicate tasks.
/// * `TimeTooFarOut`: Execution time or frequency are past the max time horizon.
/// * `TimeSlotFull`: Time slot is full. No more tasks can be scheduled for this time.
/// * `Other("proxy error: expected `ProxyType::Any`")`: schedule_as must be a proxy account of type "any" for the caller.
#[pallet::call_index(3)]
#[pallet::weight(<T as Config>::WeightInfo::schedule_xcmp_task_full(schedule.number_of_executions()).saturating_add(T::DbWeight::get().reads(1)))]
pub fn schedule_xcmp_task_through_proxy(
origin: OriginFor<T>,
schedule: ScheduleParam,
destination: Box<VersionedMultiLocation>,
schedule_fee: Box<VersionedMultiLocation>,
execution_fee: Box<AssetPayment>,
encoded_call: Vec<u8>,
encoded_call_weight: Weight,
overall_weight: Weight,
schedule_as: T::AccountId,
) -> DispatchResult {
let who = ensure_signed(origin)?;

// Make sure the owner is the proxy account of the user account.
T::EnsureProxy::ensure_ok(schedule_as.clone(), who.clone())?;

let destination =
MultiLocation::try_from(*destination).map_err(|()| Error::<T>::BadVersion)?;
let schedule_fee =
MultiLocation::try_from(*schedule_fee).map_err(|()| Error::<T>::BadVersion)?;

let action = Action::XCMP {
destination,
schedule_fee,
execution_fee: *execution_fee,
encoded_call,
encoded_call_weight,
overall_weight,
schedule_as: Some(schedule_as),
instruction_sequence: InstructionSequence::PayThroughRemoteDerivativeAccount,
};
let schedule = schedule.validated_into::<T>()?;

Self::validate_and_schedule_task(action, who, schedule, vec![])?;
Ok(())
}

/// Schedule a task to increase delegation to a specified up to a minimum balance
/// Task will reschedule itself to run on a given frequency until a failure occurs
///
Expand All @@ -506,7 +456,7 @@ pub mod pallet {
/// * `TimeSlotFull`: Time slot is full. No more tasks can be scheduled for this time.
/// * `TimeTooFarOut`: Execution time or frequency are past the max time horizon.
/// * `InsufficientBalance`: Not enough funds to pay execution fee.
#[pallet::call_index(4)]
#[pallet::call_index(2)]
#[pallet::weight(<T as Config>::WeightInfo::schedule_auto_compound_delegated_stake_task_full())]
pub fn schedule_auto_compound_delegated_stake_task(
origin: OriginFor<T>,
Expand Down Expand Up @@ -547,7 +497,7 @@ pub mod pallet {
/// * `DuplicateTask`: There can be no duplicate tasks.
/// * `TimeSlotFull`: Time slot is full. No more tasks can be scheduled for this time.
/// * `TimeTooFarOut`: Execution time or frequency are past the max time horizon.
#[pallet::call_index(5)]
#[pallet::call_index(3)]
#[pallet::weight(<T as Config>::WeightInfo::schedule_dynamic_dispatch_task_full(schedule.number_of_executions()))]
pub fn schedule_dynamic_dispatch_task(
origin: OriginFor<T>,
Expand All @@ -573,7 +523,7 @@ pub mod pallet {
///
/// # Errors
/// * `TaskDoesNotExist`: The task does not exist.
#[pallet::call_index(6)]
#[pallet::call_index(4)]
#[pallet::weight(<T as Config>::WeightInfo::cancel_scheduled_task_full())]
pub fn cancel_task(origin: OriginFor<T>, task_id: TaskIdV2) -> DispatchResult {
let who = ensure_signed(origin)?;
Expand All @@ -593,7 +543,7 @@ pub mod pallet {
///
/// # Errors
/// * `TaskDoesNotExist`: The task does not exist.
#[pallet::call_index(7)]
#[pallet::call_index(5)]
#[pallet::weight(<T as Config>::WeightInfo::force_cancel_scheduled_task_full())]
pub fn force_cancel_task(
origin: OriginFor<T>,
Expand All @@ -618,7 +568,7 @@ pub mod pallet {
/// # Errors
/// * `TaskDoesNotExist`: The task does not exist.
/// * `TaskScheduleAsNotMatch`: The schedule_as account of the task does not match.
#[pallet::call_index(8)]
#[pallet::call_index(6)]
#[pallet::weight(<T as Config>::WeightInfo::cancel_task_with_schedule_as_full())]
pub fn cancel_task_with_schedule_as(
origin: OriginFor<T>,
Expand Down
37 changes: 25 additions & 12 deletions pallets/automation-time/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ fn will_remove_task_from_account_tasks_when_task_canceled_with_schedule_as() {
frame_system::Call::remark_with_event { remark: vec![0] }.into();

// Schedule task
assert_ok!(AutomationTime::schedule_xcmp_task_through_proxy(
assert_ok!(AutomationTime::schedule_xcmp_task(
RuntimeOrigin::signed(task_owner.clone()),
ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] },
Box::new(destination.into()),
Expand All @@ -531,7 +531,8 @@ fn will_remove_task_from_account_tasks_when_task_canceled_with_schedule_as() {
call.encode(),
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
schedule_as.clone(),
InstructionSequence::PayThroughRemoteDerivativeAccount,
Some(schedule_as.clone()),
));

// Check if the task's schedule_as is correct
Expand Down Expand Up @@ -571,7 +572,7 @@ fn cancel_task_with_incorrect_schedule_as_will_fail() {
frame_system::Call::remark_with_event { remark: vec![0] }.into();

// Schedule task
assert_ok!(AutomationTime::schedule_xcmp_task_through_proxy(
assert_ok!(AutomationTime::schedule_xcmp_task(
RuntimeOrigin::signed(task_owner.clone()),
ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] },
Box::new(destination.into()),
Expand All @@ -583,7 +584,8 @@ fn cancel_task_with_incorrect_schedule_as_will_fail() {
call.encode(),
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
schedule_as.clone(),
InstructionSequence::PayThroughRemoteDerivativeAccount,
Some(schedule_as.clone()),
));

// Check if the task's schedule_as is correct
Expand Down Expand Up @@ -653,7 +655,7 @@ fn cancel_with_schedule_as_and_incorrect_owner_will_fail() {
frame_system::Call::remark_with_event { remark: vec![0] }.into();

// Schedule task
assert_ok!(AutomationTime::schedule_xcmp_task_through_proxy(
assert_ok!(AutomationTime::schedule_xcmp_task(
RuntimeOrigin::signed(task_owner.clone()),
ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] },
Box::new(destination.into()),
Expand All @@ -665,7 +667,8 @@ fn cancel_with_schedule_as_and_incorrect_owner_will_fail() {
call.encode(),
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
schedule_as.clone(),
InstructionSequence::PayThroughRemoteDerivativeAccount,
Some(schedule_as.clone()),
));

// Check if the task's schedule_as is correct
Expand Down Expand Up @@ -1025,6 +1028,8 @@ fn schedule_xcmp_works() {
call,
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
InstructionSequence::PayThroughSovereignAccount,
None,
));
})
}
Expand All @@ -1047,6 +1052,8 @@ fn schedule_xcmp_works_with_multi_currency() {
call,
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
InstructionSequence::PayThroughSovereignAccount,
None,
));
})
}
Expand All @@ -1073,14 +1080,16 @@ fn schedule_xcmp_works_with_unsupported_currency_will_fail() {
call,
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
InstructionSequence::PayThroughSovereignAccount,
None,
),
Error::<Test>::UnsupportedFeePayment,
);
})
}

#[test]
fn schedule_xcmp_through_proxy_works() {
fn schedule_xcmp_with_schedule_as_works() {
new_test_ext(START_BLOCK_TIME).execute_with(|| {
let destination = MultiLocation::new(1, X1(Parachain(PARA_ID)));
let delegator_account = AccountId32::new(DELEGATOR_ACCOUNT);
Expand All @@ -1090,7 +1099,7 @@ fn schedule_xcmp_through_proxy_works() {
// Funds including XCM fees
get_xcmp_funds(proxy_account.clone());

assert_ok!(AutomationTime::schedule_xcmp_task_through_proxy(
assert_ok!(AutomationTime::schedule_xcmp_task(
RuntimeOrigin::signed(proxy_account.clone()),
ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] },
Box::new(destination.into()),
Expand All @@ -1099,7 +1108,8 @@ fn schedule_xcmp_through_proxy_works() {
call,
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
delegator_account.clone(),
InstructionSequence::PayThroughRemoteDerivativeAccount,
Some(delegator_account.clone()),
));

let tasks = AutomationTime::get_scheduled_tasks(SCHEDULED_TIME);
Expand All @@ -1124,7 +1134,7 @@ fn schedule_xcmp_through_proxy_works() {
}

#[test]
fn schedule_xcmp_through_proxy_same_as_delegator_account() {
fn schedule_xcmp_with_schedule_as_same_as_delegator_account() {
new_test_ext(START_BLOCK_TIME).execute_with(|| {
let delegator_account = AccountId32::new(ALICE);
let call: Vec<u8> = vec![2, 4, 5];
Expand All @@ -1134,7 +1144,7 @@ fn schedule_xcmp_through_proxy_same_as_delegator_account() {
get_xcmp_funds(delegator_account.clone());

assert_noop!(
AutomationTime::schedule_xcmp_task_through_proxy(
AutomationTime::schedule_xcmp_task(
RuntimeOrigin::signed(delegator_account.clone()),
ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] },
Box::new(destination.into()),
Expand All @@ -1143,7 +1153,8 @@ fn schedule_xcmp_through_proxy_same_as_delegator_account() {
call,
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
delegator_account,
InstructionSequence::PayThroughRemoteDerivativeAccount,
Some(delegator_account),
),
sp_runtime::DispatchError::Other("proxy error: expected `ProxyType::Any`"),
);
Expand Down Expand Up @@ -1173,6 +1184,8 @@ fn schedule_xcmp_fails_if_not_enough_funds() {
call,
Weight::from_parts(100_000, 0),
Weight::from_parts(200_000, 0),
InstructionSequence::PayThroughSovereignAccount,
None,
),
Error::<Test>::InsufficientBalance,
);
Expand Down
26 changes: 0 additions & 26 deletions pallets/xcmp-handler/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,32 +228,6 @@ pub mod pallet {
Ok(instructions)
}

fn get_local_xcm(
asset: MultiAsset,
destination: MultiLocation,
) -> Result<xcm::latest::Xcm<<T as pallet::Config>::RuntimeCall>, DispatchError> {
let reserve =
T::ReserveProvider::reserve(&asset).ok_or(Error::<T>::InvalidAssetLocation)?;
let local_xcm = if reserve == MultiLocation::here() {
Xcm(vec![
WithdrawAsset::<<T as pallet::Config>::RuntimeCall>(asset.into()),
DepositAsset::<<T as pallet::Config>::RuntimeCall> {
assets: Wild(All),
beneficiary: destination,
},
])
} else if reserve == destination {
Xcm(vec![
WithdrawAsset::<<T as pallet::Config>::RuntimeCall>(asset.clone().into()),
BurnAsset::<<T as pallet::Config>::RuntimeCall>(asset.into()),
])
} else {
return Err(Error::<T>::UnsupportedFeePayment.into())
};

Ok(local_xcm)
}

/// Construct the instructions for a transact xcm with our local currency.
///
/// Local instructions
Expand Down
Loading

0 comments on commit fa94802

Please sign in to comment.