Skip to content

Commit

Permalink
Remove schedule native transfer (#377)
Browse files Browse the repository at this point in the history
  • Loading branch information
@v9n
v9n authored Jul 7, 2023
1 parent 7f6a8b9 commit 7b3d4e4
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 290 deletions.
77 changes: 2 additions & 75 deletions pallets/automation-time/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,30 +64,6 @@ fn schedule_notify_tasks<T: Config>(owner: T::AccountId, times: Vec<u64>, count:
provided_id
}

fn schedule_transfer_tasks<T: Config>(owner: T::AccountId, time: u64, count: u32) -> Vec<u8> {
let time_moment: u32 = time.saturated_into();
Timestamp::<T>::set_timestamp(time_moment.into());
let recipient: T::AccountId = account("recipient", 0, SEED);
let amount: BalanceOf<T> = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into());
let mut provided_id = vec![0u8];

for _ in 0..count {
provided_id = increment_provided_id(provided_id);
let task = TaskOf::<T>::create_native_transfer_task::<T>(
owner.clone(),
provided_id.clone(),
vec![time],
recipient.clone(),
amount.clone(),
)
.unwrap();
let task_id = AutomationTime::<T>::schedule_task(&task, provided_id.clone()).unwrap();
<AccountTasks<T>>::insert(owner.clone(), task_id, task);
}

provided_id
}

fn schedule_xcmp_tasks<T: Config>(owner: T::AccountId, times: Vec<u64>, count: u32) -> Vec<u8> {
let transfer_amount = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into());
T::Currency::deposit_creating(
Expand Down Expand Up @@ -195,34 +171,6 @@ benchmarks! {
let _ = T::MultiCurrency::deposit(currency_id.into(), &caller, foreign_currency_amount);
}: schedule_xcmp_task(RawOrigin::Signed(caller), provided_id, schedule, para_id.into(), currency_id, asset_location.into(), call, Weight::from_ref_time(1_000))

schedule_native_transfer_task_empty{
let caller: T::AccountId = account("caller", 0, SEED);
let recipient: T::AccountId = account("to", 0, SEED);
let time: u64 = 7200;
let transfer_amount = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into());
let time_moment: u32 = time.try_into().unwrap();
Timestamp::<T>::set_timestamp(time_moment.into());
T::Currency::deposit_creating(&caller, transfer_amount.clone().saturating_mul(DEPOSIT_MULTIPLIER.into()));
}: schedule_native_transfer_task(RawOrigin::Signed(caller), vec![10], vec![time], recipient, transfer_amount)

schedule_native_transfer_task_full{
let v in 1 .. T::MaxExecutionTimes::get();

let caller: T::AccountId = account("caller", 0, SEED);
let recipient: T::AccountId = account("to", 0, SEED);
let transfer_amount = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into());
T::Currency::deposit_creating(&caller, transfer_amount.clone().saturating_mul(DEPOSIT_MULTIPLIER.into()));

let mut times: Vec<u64> = vec![];
for i in 0..v {
let hour: u64 = (3600 * (i + 1)).try_into().unwrap();
times.push(hour);
}

let mut provided_id = schedule_notify_tasks::<T>(caller.clone(), times.clone(), T::MaxTasksPerSlot::get() - 1);
provided_id = increment_provided_id(provided_id);
}: schedule_native_transfer_task(RawOrigin::Signed(caller), provided_id, times, recipient, transfer_amount)

schedule_auto_compound_delegated_stake_task_full {
let task_weight = <T as Config>::WeightInfo::run_auto_compound_delegated_stake_task().ref_time();
let max_tasks_per_slot_by_weight: u32 = (T::MaxWeightPerSlot::get() / task_weight as u128).try_into().unwrap();
Expand Down Expand Up @@ -329,22 +277,6 @@ benchmarks! {
assert_last_event::<T>(Event::Notify{ message }.into())
}

run_native_transfer_task {
let starting_multiplier: u32 = 20;
let amount_starting: BalanceOf<T> = T::Currency::minimum_balance().saturating_mul(starting_multiplier.into());
let caller: T::AccountId = account("caller", 0, SEED);
T::Currency::deposit_creating(&caller, amount_starting.clone().saturating_mul(DEPOSIT_MULTIPLIER.into()));
let time: u64 = 10800;
let recipient: T::AccountId = account("recipient", 0, SEED);
let amount_transferred: BalanceOf<T> = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into());

let provided_id = schedule_transfer_tasks::<T>(caller.clone(), time, 1);
let task_id = Pallet::<T>::generate_task_id(caller.clone(), provided_id);
}: { AutomationTime::<T>::run_native_transfer_task(caller, recipient, amount_transferred, task_id) }
verify {
assert_last_event::<T>(Event::SuccessfullyTransferredFunds{ task_id }.into())
}

run_xcmp_task {
let caller: T::AccountId = account("caller", 0, SEED);
let currency_id: T::CurrencyId = T::GetNativeCurrencyId::get();
Expand Down Expand Up @@ -451,16 +383,11 @@ benchmarks! {
let weight_left = Weight::from_ref_time(500_000_000_000);
let mut task_ids = vec![];
let caller: T::AccountId = account("caller", 0, SEED);
let time = 10800;
let recipient: T::AccountId = account("to", 0, SEED);
let starting_multiplier: u32 = 20;
let transfer_amount = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into());
let starting_amount = T::Currency::minimum_balance().saturating_mul(starting_multiplier.into());
T::Currency::deposit_creating(&caller, starting_amount.clone().saturating_mul(DEPOSIT_MULTIPLIER.into()));
let execution_time = 10800;

for i in 0..v {
let provided_id: Vec<u8> = vec![i.saturated_into::<u8>()];
let task = TaskOf::<T>::create_native_transfer_task::<T>(caller.clone(), provided_id.clone(), vec![time], recipient.clone(), transfer_amount.clone()).unwrap();
let task = TaskOf::<T>::create_event_task::<T>(caller.clone(), provided_id.clone(), vec![execution_time], vec![65, 65.saturating_add(i as u8)]).unwrap();
let task_id = AutomationTime::<T>::schedule_task(&task, provided_id).unwrap();
<AccountTasks<T>>::insert(caller.clone(), task_id, task);
task_ids.push((caller.clone(), task_id))
Expand Down
50 changes: 1 addition & 49 deletions pallets/automation-time/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,55 +366,6 @@ pub mod pallet {

#[pallet::call]
impl<T: Config> Pallet<T> {
/// Schedule a task to transfer native token balance from sender to recipient.
///
/// Before the task can be scheduled the task must past validation checks.
/// * The transaction is signed
/// * The provided_id's length > 0
/// * The times are valid
/// * Larger transfer amount than the acceptable minimum
/// * Transfer to account other than to self
///
/// # Parameters
/// * `provided_id`: An id provided by the user. This id must be unique for the user.
/// * `execution_times`: The list of unix standard times in seconds for when the task should run.
/// * `recipient_id`: Account ID of the recipient.
/// * `amount`: Amount of balance to transfer.
///
/// # Errors
/// * `InvalidTime`: Time must end in a whole hour.
/// * `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.
/// * `InvalidAmount`: Amount has to be larger than 0.1 OAK.
/// * `TransferToSelf`: Sender cannot transfer money to self.
#[pallet::call_index(1)]
#[pallet::weight(<T as Config>::WeightInfo::schedule_native_transfer_task_full(execution_times.len().try_into().unwrap()))]
pub fn schedule_native_transfer_task(
origin: OriginFor<T>,
provided_id: Vec<u8>,
execution_times: Vec<UnixTime>,
recipient_id: AccountOf<T>,
#[pallet::compact] amount: BalanceOf<T>,
) -> DispatchResult {
let who = ensure_signed(origin)?;

// check for greater than existential deposit
if amount < T::Currency::minimum_balance() {
Err(<Error<T>>::InvalidAmount)?
}
// check not sent to self
if who == recipient_id {
Err(<Error<T>>::TransferToSelf)?
}
let action =
Action::NativeTransfer { sender: who.clone(), recipient: recipient_id, amount };
let schedule = Schedule::new_fixed_schedule::<T>(execution_times)?;
Self::validate_and_schedule_task(action, who, provided_id, schedule)?;
Ok(().into())
}

/// Schedule a task through XCMP to fire an XCMP message with a provided call.
///
/// Before the task can be scheduled the task must past validation checks.
Expand Down Expand Up @@ -1130,6 +1081,7 @@ pub mod pallet {
);

let call_weight = scheduled_call.get_dispatch_info().weight;

let (maybe_actual_call_weight, result) =
match scheduled_call.dispatch(dispatch_origin) {
Ok(post_info) => (post_info.actual_weight, Ok(())),
Expand Down
50 changes: 43 additions & 7 deletions pallets/automation-time/src/mock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,25 +197,60 @@ parameter_types! {
pub const MaxWeightPercentage: Perbill = Perbill::from_percent(10);
pub const UpdateQueueRatio: Perbill = Perbill::from_percent(50);
pub const ExecutionWeightFee: Balance = 12;
pub const MaxWeightPerSlot: u128 = 70_000_000;

// When unit testing dynamic dispatch, we use the real weight value of the extrinsics call
// This is an external lib that we don't own so we try to not mock, follow the rule don't mock
// what you don't own
// One of test we do is Balances::transfer call, which has its weight define here:
// https://github.com/paritytech/substrate/blob/polkadot-v0.9.38/frame/balances/src/weights.rs#L61-L73
// When logging the final calculated amount, its value is 73_314_000.
//
// in our unit test, we test a few transfers with dynamic dispatch. On top
// of that, there is also weight of our call such as fetching the tasks,
// move from schedule slot to tasks queue,.. so the weight of a schedule
// transfer with dynamic dispatch is even higher.
//
// and because we test run a few of them so I set it to ~10x value of 73_314_000
pub const MaxWeightPerSlot: u128 = 700_000_000;
pub const XmpFee: u128 = 1_000_000;
pub const GetNativeCurrencyId: CurrencyId = NATIVE;
}

pub struct MockWeight<T>(PhantomData<T>);
impl<Test: frame_system::Config> pallet_automation_time::WeightInfo for MockWeight<Test> {
fn schedule_notify_task_empty() -> Weight {
pub struct MockPalletBalanceWeight<T>(PhantomData<T>);
impl<Test: frame_system::Config> pallet_balances::WeightInfo for MockPalletBalanceWeight<Test> {
fn transfer() -> Weight {
Weight::from_ref_time(100_000)
}

fn transfer_keep_alive() -> Weight {
Weight::zero()
}
fn schedule_notify_task_full(_v: u32) -> Weight {
fn set_balance_creating() -> Weight {
Weight::zero()
}
fn set_balance_killing() -> Weight {
Weight::zero()
}
fn force_transfer() -> Weight {
Weight::zero()
}
fn schedule_native_transfer_task_empty() -> Weight {
fn transfer_all() -> Weight {
Weight::zero()
}
fn schedule_native_transfer_task_full(_v: u32) -> Weight {
fn force_unreserve() -> Weight {
Weight::zero()
}
}

pub struct MockWeight<T>(PhantomData<T>);
impl<Test: frame_system::Config> pallet_automation_time::WeightInfo for MockWeight<Test> {
fn schedule_notify_task_empty() -> Weight {
Weight::zero()
}
fn schedule_notify_task_full(_v: u32) -> Weight {
Weight::zero()
}

fn schedule_auto_compound_delegated_stake_task_full() -> Weight {
Weight::zero()
}
Expand Down Expand Up @@ -326,6 +361,7 @@ impl Contains<RuntimeCall> for ScheduleAllowList {
fn contains(c: &RuntimeCall) -> bool {
match c {
RuntimeCall::System(_) => true,
RuntimeCall::Balances(_) => true,
_ => false,
}
}
Expand Down
114 changes: 50 additions & 64 deletions pallets/automation-time/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ use codec::Encode;
use core::convert::TryInto;
use frame_support::{assert_noop, assert_ok, traits::OnInitialize, weights::Weight};
use frame_system::{self, RawOrigin};
use pallet_balances;

use pallet_valve::Shutdown;
use sp_runtime::{
traits::{BlakeTwo256, Hash},
Expand Down Expand Up @@ -217,75 +219,59 @@ fn schedule_not_enough_for_fees() {
})
}

// test schedule transfer with dynamic dispatch.
#[test]
fn schedule_native_transfer_invalid_amount() {
fn schedule_transfer_with_dynamic_dispatch() {
new_test_ext(START_BLOCK_TIME).execute_with(|| {
assert_noop!(
AutomationTime::schedule_native_transfer_task(
RuntimeOrigin::signed(AccountId32::new(ALICE)),
vec![50],
vec![SCHEDULED_TIME],
AccountId32::new(BOB),
0,
),
Error::<Test>::InvalidAmount,
);
})
}
let account_id = AccountId32::new(ALICE);
let provided_id = vec![1, 2];
let task_id = AutomationTime::generate_task_id(account_id.clone(), provided_id.clone());
let task_hash_input = TaskHashInput::new(account_id.clone(), vec![1, 2]);

#[test]
fn schedule_native_transfer_cannot_transfer_to_self() {
new_test_ext(START_BLOCK_TIME).execute_with(|| {
assert_noop!(
AutomationTime::schedule_native_transfer_task(
RuntimeOrigin::signed(AccountId32::new(ALICE)),
vec![50],
vec![SCHEDULED_TIME],
AccountId32::new(ALICE),
1,
),
Error::<Test>::TransferToSelf,
);
})
}
fund_account(&account_id, 900_000_000, 2, Some(0));

#[test]
fn schedule_native_transfer_works() {
new_test_ext(START_BLOCK_TIME).execute_with(|| {
get_funds(AccountId32::new(ALICE));
assert_ok!(AutomationTime::schedule_native_transfer_task(
RuntimeOrigin::signed(AccountId32::new(ALICE)),
vec![50],
vec![SCHEDULED_TIME],
AccountId32::new(BOB),
1,
let call: <Test as frame_system::Config>::RuntimeCall =
pallet_balances::Call::transfer { dest: AccountId32::new(BOB), value: 127 }.into();

assert_ok!(AutomationTime::schedule_dynamic_dispatch_task(
RuntimeOrigin::signed(account_id.clone()),
vec![1, 2],
ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] },
Box::new(call),
));
match AutomationTime::get_scheduled_tasks(SCHEDULED_TIME) {
None => {
panic!("A task should be scheduled")
},
Some(ScheduledTasksOf::<Test> { tasks: account_task_ids, .. }) =>
match AutomationTime::get_account_task(
account_task_ids[0].0.clone(),
account_task_ids[0].1,
) {
None => {
panic!("A task should exist if it was scheduled")
},
Some(task) => {
let expected_task = TaskOf::<Test>::create_native_transfer_task::<Test>(
AccountId32::new(ALICE),
vec![50],
vec![SCHEDULED_TIME],
AccountId32::new(BOB),
1,
)
.unwrap();

assert_eq!(task, expected_task);
},
},
}

Timestamp::set_timestamp(SCHEDULED_TIME * 1_000);
LastTimeSlot::<Test>::put((LAST_BLOCK_TIME, LAST_BLOCK_TIME));
System::reset_events();

AutomationTime::trigger_tasks(Weight::from_ref_time(900_000_000));
let my_events = events();

let recipient = AccountId32::new(BOB);
assert_eq!(Balances::free_balance(recipient.clone()), 127);

assert_eq!(
my_events,
[
RuntimeEvent::System(frame_system::Event::NewAccount {
account: recipient.clone()
}),
RuntimeEvent::Balances(pallet_balances::pallet::Event::Endowed {
account: recipient.clone(),
free_balance: 127,
}),
RuntimeEvent::Balances(pallet_balances::pallet::Event::Transfer {
from: account_id.clone(),
to: recipient.clone(),
amount: 127,
}),
RuntimeEvent::AutomationTime(crate::Event::DynamicDispatchResult {
who: account_id.clone(),
task_id,
result: Ok(()),
}),
]
);
})
}

Expand Down
Loading

0 comments on commit 7b3d4e4

Please sign in to comment.