From d15fe711a38d53ed12f3221865524d801119f08b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vinh=20Qu=E1=BB=91c=20Nguy=E1=BB=85n?= Date: Wed, 26 Jul 2023 20:21:15 -0700 Subject: [PATCH] Auto generated task id and remove provided_id (#384) --- .../rpc/runtime-api/src/lib.rs | 3 +- pallets/automation-time/rpc/src/lib.rs | 33 +- pallets/automation-time/src/benchmarking.rs | 148 ++--- pallets/automation-time/src/lib.rs | 288 +++++---- pallets/automation-time/src/migrations/mod.rs | 2 + .../src/migrations/update_task_idv2.rs | 387 ++++++++++++ .../src/migrations/update_xcmp_task.rs | 81 +-- .../automation-time/src/migrations/utils.rs | 142 +++++ pallets/automation-time/src/mock.rs | 91 +-- pallets/automation-time/src/tests.rs | 596 +++++++++++------- pallets/automation-time/src/types.rs | 50 +- runtime/neumann/src/lib.rs | 19 +- runtime/oak/src/lib.rs | 19 +- runtime/turing/src/lib.rs | 31 +- 14 files changed, 1257 insertions(+), 633 deletions(-) create mode 100644 pallets/automation-time/src/migrations/update_task_idv2.rs create mode 100644 pallets/automation-time/src/migrations/utils.rs diff --git a/pallets/automation-time/rpc/runtime-api/src/lib.rs b/pallets/automation-time/rpc/runtime-api/src/lib.rs index acd8e9a2d..cc95c0ce5 100644 --- a/pallets/automation-time/rpc/runtime-api/src/lib.rs +++ b/pallets/automation-time/rpc/runtime-api/src/lib.rs @@ -55,13 +55,12 @@ sp_api::decl_runtime_apis! { Hash: Codec, Balance: Codec, { - fn generate_task_id(account_id: AccountId, provided_id: Vec) -> Hash; fn query_fee_details(uxt: Block::Extrinsic) -> Result, Vec>; fn get_time_automation_fees(action: AutomationAction, executions: u32) -> Balance; fn calculate_optimal_autostaking( principal: i128, collator: AccountId ) -> Result>; - fn get_auto_compound_delegated_stake_task_ids(account_id: AccountId) -> Vec; + fn get_auto_compound_delegated_stake_task_ids(account_id: AccountId) -> Vec>; } } diff --git a/pallets/automation-time/rpc/src/lib.rs b/pallets/automation-time/rpc/src/lib.rs index abc4e9e97..6c9a752c8 100644 --- a/pallets/automation-time/rpc/src/lib.rs +++ b/pallets/automation-time/rpc/src/lib.rs @@ -36,15 +36,6 @@ use std::sync::Arc; /// An RPC endpoint to provide information about tasks. #[rpc(client, server)] pub trait AutomationTimeApi { - /// Generates the task_id given the account_id and provided_id. - #[method(name = "automationTime_generateTaskId")] - fn generate_task_id( - &self, - account: AccountId, - provided_id: String, - at: Option, - ) -> RpcResult; - #[method(name = "automationTime_queryFeeDetails")] fn query_fee_details( &self, @@ -72,7 +63,7 @@ pub trait AutomationTimeApi { fn get_auto_compound_delegated_stake_task_ids( &self, account: AccountId, - ) -> RpcResult>; + ) -> RpcResult>>; } /// An implementation of Automation-specific RPC methods on full client. @@ -115,26 +106,6 @@ where AccountId: Codec, Hash: Codec, { - fn generate_task_id( - &self, - account: AccountId, - provided_id: String, - at: Option, - ) -> RpcResult { - let api = self.client.runtime_api(); - let at = BlockId::hash(at.unwrap_or_else(|| self.client.info().best_hash)); - - let runtime_api_result = - api.generate_task_id(&at, account, provided_id.as_bytes().to_vec()); - runtime_api_result.map_err(|e| { - JsonRpseeError::Call(CallError::Custom(ErrorObject::owned( - Error::RuntimeError.into(), - "Unable to generate task_id", - Some(format!("{:?}", e)), - ))) - }) - } - fn query_fee_details( &self, encoded_xt: Bytes, @@ -233,7 +204,7 @@ where fn get_auto_compound_delegated_stake_task_ids( &self, account: AccountId, - ) -> RpcResult> { + ) -> RpcResult>> { let api = self.client.runtime_api(); let at = BlockId::hash(self.client.info().best_hash); diff --git a/pallets/automation-time/src/benchmarking.rs b/pallets/automation-time/src/benchmarking.rs index 46e04fb55..bc4a46c95 100644 --- a/pallets/automation-time/src/benchmarking.rs +++ b/pallets/automation-time/src/benchmarking.rs @@ -17,6 +17,7 @@ #![cfg(feature = "runtime-benchmarks")] +//mod mock; use super::*; use frame_benchmarking::{account, benchmarks}; use frame_system::RawOrigin; @@ -26,7 +27,7 @@ use sp_runtime::traits::{AccountIdConversion, Saturating}; use sp_std::cmp; use xcm::latest::prelude::*; -use crate::{MissedTaskV2Of, Pallet as AutomationTime, TaskOf}; +use crate::{Config, MissedTaskV2Of, Pallet as AutomationTime, TaskOf}; const SEED: u32 = 0; // existential deposit multiplier @@ -46,22 +47,22 @@ fn schedule_notify_tasks(owner: T::AccountId, times: Vec, count: ); let time_moment: u32 = times[0].saturated_into(); Timestamp::::set_timestamp(time_moment.into()); - let mut provided_id = vec![0u8]; + let mut task_id = vec![0u8]; for _ in 0..count { - provided_id = increment_provided_id(provided_id); + task_id = increment_task_id(task_id); let task = TaskOf::::create_event_task::( owner.clone(), - provided_id.clone(), + task_id.clone(), times.clone(), vec![4, 5, 6], ) .unwrap(); - let task_id = AutomationTime::::schedule_task(&task, provided_id.clone()).unwrap(); + let task_id = AutomationTime::::schedule_task(&task).unwrap(); >::insert(owner.clone(), task_id, task); } - provided_id + task_id } fn schedule_xcmp_tasks(owner: T::AccountId, times: Vec, count: u32) -> Vec { @@ -73,13 +74,13 @@ fn schedule_xcmp_tasks(owner: T::AccountId, times: Vec, count: u let para_id: u32 = 2001; let time_moment: u32 = times[0].saturated_into(); Timestamp::::set_timestamp(time_moment.into()); - let mut provided_id = vec![0u8]; + let mut task_id = vec![0u8]; for _ in 0..count { - provided_id = increment_provided_id(provided_id); + task_id = increment_task_id(task_id); let task = TaskOf::::create_xcmp_task::( owner.clone(), - provided_id.clone(), + task_id.clone(), times.clone(), MultiLocation::new(1, X1(Parachain(para_id))), MultiLocation::default(), @@ -93,53 +94,55 @@ fn schedule_xcmp_tasks(owner: T::AccountId, times: Vec, count: u InstructionSequence::PayThroughSovereignAccount, ) .unwrap(); - let task_id = AutomationTime::::schedule_task(&task, provided_id.clone()).unwrap(); + let task_id = AutomationTime::::schedule_task(&task).unwrap(); >::insert(owner.clone(), task_id, task); } - provided_id + task_id } fn schedule_auto_compound_delegated_stake_tasks( owner: T::AccountId, time: u64, count: u32, -) -> Vec<(T::Hash, TaskOf)> { +) -> Vec<(TaskIdV2, TaskOf)> { let time_moment: u32 = time.saturated_into(); Timestamp::::set_timestamp(time_moment.into()); let mut tasks = Vec::with_capacity(count.try_into().unwrap()); + let mut task_id = vec![0u8]; for i in 0..count { let collator: T::AccountId = account("collator", 0, i); - let provided_id = AutomationTime::::generate_auto_compound_delegated_stake_provided_id( - &owner, &collator, - ); let frequency = 3600; + task_id = increment_task_id(task_id); let account_minimum = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into()); let task = TaskOf::::create_auto_compound_delegated_stake_task::( owner.clone(), - provided_id.clone(), + task_id.clone(), time, frequency, collator, account_minimum, ) .unwrap(); - let task_id = AutomationTime::::schedule_task(&task, provided_id).unwrap(); - >::insert(owner.clone(), task_id, task); - tasks.push((task_id, Pallet::::get_account_task(owner.clone(), task_id).unwrap())); + AutomationTime::::schedule_task(&task).unwrap(); + >::insert(owner.clone(), task_id.clone(), task); + tasks.push(( + task_id.clone(), + Pallet::::get_account_task(owner.clone(), task_id.clone()).unwrap(), + )); } tasks } -fn increment_provided_id(mut provided_id: Vec) -> Vec { - let last = provided_id.last_mut().unwrap(); +fn increment_task_id(mut task_id: Vec) -> Vec { + let last = task_id.last_mut().unwrap(); if let Some(next) = last.checked_add(1) { *last = next; } else { - provided_id.push(0u8); + task_id.push(0u8); } - provided_id + task_id } benchmarks! { @@ -170,14 +173,13 @@ benchmarks! { let fee = AssetPayment { asset_location: MultiLocation::new(0, Here).into(), amount: 100u128 }; - let mut provided_id = schedule_xcmp_tasks::(caller.clone(), times, max_tasks_per_slot - 1); - provided_id = increment_provided_id(provided_id); + let mut task_id = schedule_xcmp_tasks::(caller.clone(), times, max_tasks_per_slot - 1); let foreign_currency_amount = T::MultiCurrency::minimum_balance(currency_id.into()) .saturating_add(1u32.into()) .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), provided_id, schedule, Box::new(destination.into()), Box::new(schedule_fee.into()), Box::new(fee), call, Weight::from_ref_time(1_000), Weight::from_ref_time(2_000)) + }: schedule_xcmp_task(RawOrigin::Signed(caller), schedule, Box::new(destination.into()), Box::new(schedule_fee.into()), Box::new(fee), call, Weight::from_ref_time(1_000), Weight::from_ref_time(2_000)) schedule_auto_compound_delegated_stake_task_full { let task_weight = ::WeightInfo::run_auto_compound_delegated_stake_task().ref_time(); @@ -210,12 +212,17 @@ benchmarks! { let account_min = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into()); T::Currency::deposit_creating(&caller, account_min.saturating_mul(DEPOSIT_MULTIPLIER.into())); - - let provided_id = vec![1, 2, 3]; - let task_id = Pallet::::generate_task_id(caller.clone(), provided_id.clone()); - }: schedule_dynamic_dispatch_task(RawOrigin::Signed(caller.clone()), provided_id, schedule, Box::new(call)) + }: schedule_dynamic_dispatch_task(RawOrigin::Signed(caller.clone()), schedule, Box::new(call)) verify { - assert_last_event::(Event::TaskScheduled { who: caller, task_id: task_id, schedule_as: None }.into()) + { + let length = frame_system::Pallet::::events().len() as u8; + // Our task id will be in this format: 1-0-[event-index] + // 1 -> block number(we run in first block) + // 0 -> extrinsic index + // Because there is the event itself and we count from 0, so the event index of final + // one will be : total - 2 + (ascii of 0) + assert_last_event::(Event::TaskScheduled { who: caller, schedule_as: None, task_id: vec![49, 45, 48, 45, 48 + length-2], }.into()) + } } schedule_dynamic_dispatch_task_full { @@ -234,14 +241,12 @@ benchmarks! { let account_min = T::Currency::minimum_balance().saturating_mul(ED_MULTIPLIER.into()); T::Currency::deposit_creating(&caller, account_min.saturating_mul(DEPOSIT_MULTIPLIER.into())); - let provided_id = vec![1, 2, 3]; - let task_id = Pallet::::generate_task_id(caller.clone(), provided_id.clone()); - // Almost fill up all time slots schedule_notify_tasks::(caller.clone(), times, T::MaxTasksPerSlot::get() - 1); - }: schedule_dynamic_dispatch_task(RawOrigin::Signed(caller.clone()), provided_id, schedule, Box::new(call)) + }: schedule_dynamic_dispatch_task(RawOrigin::Signed(caller.clone()), schedule, Box::new(call)) verify { - assert_last_event::(Event::TaskScheduled { who: caller, task_id: task_id, schedule_as: None }.into()) + let length = frame_system::Pallet::::events().len() as u8; + assert_last_event::(Event::TaskScheduled { who: caller, schedule_as: None, task_id: vec![49, 45, 48, 45, 48 + length-2], }.into()) } cancel_scheduled_task_full { @@ -253,16 +258,14 @@ benchmarks! { times.push(hour); } - let provided_id = schedule_notify_tasks::(caller.clone(), times, T::MaxTasksPerSlot::get()); - let task_id = Pallet::::generate_task_id(caller.clone(), provided_id); + let task_id = schedule_notify_tasks::(caller.clone(), times, T::MaxTasksPerSlot::get()); }: cancel_task(RawOrigin::Signed(caller), task_id) force_cancel_scheduled_task { let caller: T::AccountId = account("caller", 0, SEED); let time: u64 = 10800; - let provided_id = schedule_notify_tasks::(caller.clone(), vec![time], 1); - let task_id = Pallet::::generate_task_id(caller.clone(), provided_id); + let task_id = schedule_notify_tasks::(caller.clone(), vec![time], 1); }: force_cancel_task(RawOrigin::Root, caller, task_id) force_cancel_scheduled_task_full { @@ -274,8 +277,7 @@ benchmarks! { times.push(hour); } - let provided_id = schedule_notify_tasks::(caller.clone(), times, T::MaxTasksPerSlot::get()); - let task_id = Pallet::::generate_task_id(caller.clone(), provided_id); + let task_id = schedule_notify_tasks::(caller.clone(), times, T::MaxTasksPerSlot::get()); }: force_cancel_task(RawOrigin::Root, caller, task_id) run_notify_task { @@ -301,8 +303,7 @@ benchmarks! { let fee = AssetPayment { asset_location: MultiLocation::new(1, X1(Parachain(para_id))).into(), amount: 1000u128 }; - let provided_id = schedule_xcmp_tasks::(caller.clone(), vec![time], 1); - let task_id = Pallet::::generate_task_id(caller.clone(), provided_id); + let task_id = schedule_xcmp_tasks::(caller.clone(), vec![time], 1); }: { AutomationTime::::run_xcmp_task(destination, caller, fee, call, Weight::from_ref_time(100_000), Weight::from_ref_time(200_000), task_id.clone(), InstructionSequence::PayThroughSovereignAccount) } run_auto_compound_delegated_stake_task { @@ -315,25 +316,26 @@ benchmarks! { T::DelegatorActions::setup_delegator(&collator, &delegator)?; let (task_id, task) = schedule_auto_compound_delegated_stake_tasks::(delegator.clone(), 3600, 1).pop().unwrap(); - }: { AutomationTime::::run_auto_compound_delegated_stake_task(delegator, collator, account_minimum, task_id, &task) } + }: { AutomationTime::::run_auto_compound_delegated_stake_task(delegator, collator, account_minimum, task_id.clone(), &task) } run_dynamic_dispatch_action { let caller: T::AccountId = account("caller", 0, SEED); - let task_id = AutomationTime::::generate_task_id(caller.clone(), vec![1]); + let task_id = vec![49, 45, 48, 45, 52]; let call: ::Call = frame_system::Call::remark { remark: vec![] }.into(); let encoded_call = call.encode(); - }: { AutomationTime::::run_dynamic_dispatch_action(caller.clone(), encoded_call, task_id) } + }: { AutomationTime::::run_dynamic_dispatch_action(caller.clone(), encoded_call, task_id.clone()) } verify { - assert_last_event::(Event::DynamicDispatchResult{ who: caller, task_id, result: Ok(()) }.into()) + assert_last_event::(Event::DynamicDispatchResult{ who: caller, task_id: task_id.clone(), result: Ok(()) }.into()) } run_dynamic_dispatch_action_fail_decode { let caller: T::AccountId = account("caller", 0, SEED); - let task_id = AutomationTime::::generate_task_id(caller.clone(), vec![1]); + let task_id = vec![49, 45, 48, 45, 52]; + let bad_encoded_call: Vec = vec![1]; - }: { AutomationTime::::run_dynamic_dispatch_action(caller.clone(), bad_encoded_call, task_id) } + }: { AutomationTime::::run_dynamic_dispatch_action(caller.clone(), bad_encoded_call, task_id.clone()) } verify { - assert_last_event::(Event::CallCannotBeDecoded{ who: caller, task_id }.into()) + assert_last_event::(Event::CallCannotBeDecoded{ who: caller, task_id: task_id.clone() }.into()) } /* @@ -352,11 +354,11 @@ benchmarks! { let time: u32 = 10800; for i in 0..v { - let provided_id: Vec = vec![i.saturated_into::()]; - let task = TaskOf::::create_event_task::(caller.clone(), provided_id.clone(), vec![time.into()], vec![4, 5, 6]).unwrap(); - let task_id = AutomationTime::::schedule_task(&task, provided_id).unwrap(); - let missed_task = MissedTaskV2Of::::new(caller.clone(), task_id, time.into()); - >::insert(caller.clone(), task_id, task); + let task_id: Vec = vec![i.saturated_into::()]; + let task = TaskOf::::create_event_task::(caller.clone(), task_id.clone(), vec![time.into()], vec![4, 5, 6]).unwrap(); + AutomationTime::::schedule_task(&task).unwrap(); + let missed_task = MissedTaskV2Of::::new(caller.clone(), task_id.clone(), time.into()); + >::insert(caller.clone(), task_id.clone(), task); missed_tasks.push(missed_task) } }: { AutomationTime::::run_missed_tasks(missed_tasks, weight_left) } @@ -370,9 +372,8 @@ benchmarks! { let weight_left = Weight::from_ref_time(500_000_000_000); for i in 0..v { - let provided_id: Vec = vec![i.saturated_into::()]; - let task_id = AutomationTime::::generate_task_id(caller.clone(), provided_id); - let missed_task = MissedTaskV2Of::::new(caller.clone(), task_id, time.into()); + let task_id: Vec = vec![i.saturated_into::()]; + let missed_task = MissedTaskV2Of::::new(caller.clone(), task_id.clone(), time.into()); missed_tasks.push(missed_task) } }: { AutomationTime::::run_missed_tasks(missed_tasks, weight_left) } @@ -393,11 +394,11 @@ benchmarks! { let execution_time = 10800; for i in 0..v { - let provided_id: Vec = vec![i.saturated_into::()]; - let task = TaskOf::::create_event_task::(caller.clone(), provided_id.clone(), vec![execution_time], vec![65, 65.saturating_add(i as u8)]).unwrap(); - let task_id = AutomationTime::::schedule_task(&task, provided_id).unwrap(); - >::insert(caller.clone(), task_id, task); - task_ids.push((caller.clone(), task_id)) + let task_id: Vec = vec![i.saturated_into::()]; + let task = TaskOf::::create_event_task::(caller.clone(), task_id.clone(), vec![execution_time], vec![65, 65.saturating_add(i as u8)]).unwrap(); + let task_id = AutomationTime::::schedule_task(&task).unwrap(); + >::insert(caller.clone(), task_id.clone(), task); + task_ids.push((caller.clone(), task_id.clone())) } }: { AutomationTime::::run_tasks(task_ids, weight_left) } @@ -409,8 +410,7 @@ benchmarks! { let weight_left = Weight::from_ref_time(500_000_000_000); for i in 0..v { - let provided_id: Vec = vec![i.saturated_into::()]; - let task_id = AutomationTime::::generate_task_id(caller.clone(), provided_id); + let task_id: Vec = vec![i.saturated_into::()]; task_ids.push((caller.clone(), task_id)) } }: { AutomationTime::::run_tasks(task_ids, weight_left) } @@ -441,9 +441,9 @@ benchmarks! { for i in 0..v { for j in 0..1 { let time = time.saturating_add(3600); - let provided_id: Vec = vec![i.saturated_into::(), j.saturated_into::()]; - let task = TaskOf::::create_event_task::(caller.clone(), provided_id.clone(), vec![time.into()], vec![4, 5, 6]).unwrap(); - let task_id = AutomationTime::::schedule_task(&task, provided_id).unwrap(); + let task_id: Vec = vec![i.saturated_into::(), j.saturated_into::()]; + let task = TaskOf::::create_event_task::(caller.clone(), task_id.clone(), vec![time.into()], vec![4, 5, 6]).unwrap(); + AutomationTime::::schedule_task(&task).unwrap(); >::insert(caller.clone(), task_id, task); } } @@ -453,14 +453,14 @@ benchmarks! { let caller: T::AccountId = account("callerName", 0, SEED); let last_time_slot: u64 = 7200; let current_time = 10800; - let mut provided_id = vec![0u8]; + let mut task_id = vec![0u8]; Timestamp::::set_timestamp(current_time.saturated_into::().into()); for i in 0..T::MaxTasksPerSlot::get() { - provided_id = increment_provided_id(provided_id); - let task = TaskOf::::create_event_task::(caller.clone(), provided_id.clone(), vec![current_time.into()], vec![4, 5, 6]).unwrap(); - let task_id = AutomationTime::::schedule_task(&task, provided_id.clone()).unwrap(); - >::insert(caller.clone(), task_id, task); + task_id = increment_task_id(task_id); + let task = TaskOf::::create_event_task::(caller.clone(), task_id.clone(), vec![current_time.into()], vec![4, 5, 6]).unwrap(); + AutomationTime::::schedule_task(&task).unwrap(); + >::insert(caller.clone(), task_id.clone(), task); } }: { AutomationTime::::update_scheduled_task_queue(current_time, last_time_slot) } diff --git a/pallets/automation-time/src/lib.rs b/pallets/automation-time/src/lib.rs index 8aa2d4ab9..6451895ea 100644 --- a/pallets/automation-time/src/lib.rs +++ b/pallets/automation-time/src/lib.rs @@ -70,7 +70,8 @@ use pallet_timestamp::{self as timestamp}; pub use pallet_xcmp_handler::InstructionSequence; use pallet_xcmp_handler::XcmpTransactor; use primitives::EnsureProxy; -use scale_info::TypeInfo; +//use scale_info::TypeInfo; +use scale_info::{prelude::format, TypeInfo}; use sp_runtime::{ traits::{CheckedConversion, Convert, Dispatchable, SaturatedConversion, Saturating}, ArithmeticError, DispatchError, Perbill, @@ -79,6 +80,8 @@ use sp_std::{boxed::Box, vec, vec::Vec}; pub use weights::WeightInfo; use xcm::{latest::prelude::*, VersionedMultiLocation}; +use scale_info::prelude::string::String; + #[frame_support::pallet] pub mod pallet { use super::*; @@ -89,12 +92,14 @@ pub mod pallet { pub type MultiBalanceOf = <::MultiCurrency as MultiCurrency< ::AccountId, >>::Balance; - pub type TaskId = ::Hash; - pub type AccountTaskId = (AccountOf, TaskId); + + pub type TaskIdV2 = Vec; + + pub type AccountTaskId = (AccountOf, TaskIdV2); pub type ActionOf = Action, BalanceOf>; pub type TaskOf = Task, BalanceOf>; - pub type MissedTaskV2Of = MissedTaskV2, TaskId>; - pub type ScheduledTasksOf = ScheduledTasks, TaskId>; + pub type MissedTaskV2Of = MissedTaskV2, TaskIdV2>; + pub type ScheduledTasksOf = ScheduledTasks, TaskIdV2>; pub type MultiCurrencyId = <::MultiCurrency as MultiCurrency< ::AccountId, >>::CurrencyId; @@ -203,7 +208,7 @@ pub mod pallet { #[pallet::storage] #[pallet::getter(fn get_account_task)] pub type AccountTasks = - StorageDoubleMap<_, Twox64Concat, AccountOf, Twox64Concat, TaskId, TaskOf>; + StorageDoubleMap<_, Twox64Concat, AccountOf, Twox64Concat, TaskIdV2, TaskOf>; #[pallet::storage] #[pallet::getter(fn get_task_queue)] @@ -235,8 +240,6 @@ pub mod pallet { TimeTooFarOut, /// The message cannot be empty. EmptyMessage, - /// The provided_id cannot be empty - EmptyProvidedId, /// There can be no duplicate tasks. DuplicateTask, /// Time slot is full. No more tasks can be scheduled for this time. @@ -272,13 +275,13 @@ pub mod pallet { /// Schedule task success. TaskScheduled { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, schedule_as: Option>, }, /// Cancelled a task. TaskCancelled { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, }, /// Notify event for the task. Notify { @@ -287,75 +290,75 @@ pub mod pallet { /// A Task was not found. TaskNotFound { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, }, /// Successfully transferred funds SuccessfullyTransferredFunds { - task_id: TaskId, + task_id: TaskIdV2, }, /// Successfully sent XCMP XcmpTaskSucceeded { - task_id: T::Hash, + task_id: TaskIdV2, destination: MultiLocation, }, /// Failed to send XCMP XcmpTaskFailed { - task_id: T::Hash, + task_id: TaskIdV2, destination: MultiLocation, error: DispatchError, }, /// Transfer Failed TransferFailed { - task_id: TaskId, + task_id: TaskIdV2, error: DispatchError, }, SuccesfullyAutoCompoundedDelegatorStake { - task_id: TaskId, + task_id: TaskIdV2, amount: BalanceOf, }, AutoCompoundDelegatorStakeFailed { - task_id: TaskId, + task_id: TaskIdV2, error_message: Vec, error: DispatchErrorWithPostInfo, }, /// The task could not be run at the scheduled time. TaskMissed { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, execution_time: UnixTime, }, /// The result of the DynamicDispatch action. DynamicDispatchResult { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, result: DispatchResult, }, /// The call for the DynamicDispatch action can no longer be decoded. CallCannotBeDecoded { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, }, /// A recurring task was rescheduled TaskRescheduled { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, schedule_as: Option>, }, /// A recurring task was not rescheduled TaskNotRescheduled { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, error: DispatchError, }, /// A recurring task attempted but failed to be rescheduled TaskFailedToReschedule { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, error: DispatchError, }, TaskCompleted { who: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, }, } @@ -380,12 +383,10 @@ pub mod pallet { /// /// 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 /// * The given asset location is supported /// /// # Parameters - /// * `provided_id`: An id provided by the user. This id must be unique for the user. /// * `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. @@ -405,7 +406,6 @@ pub mod pallet { #[pallet::weight(::WeightInfo::schedule_xcmp_task_full(schedule.number_of_executions()))] pub fn schedule_xcmp_task( origin: OriginFor, - provided_id: Vec, schedule: ScheduleParam, destination: Box, schedule_fee: Box, @@ -432,7 +432,7 @@ pub mod pallet { let schedule = schedule.validated_into::()?; - Self::validate_and_schedule_task(action, who, provided_id, schedule)?; + Self::validate_and_schedule_task(action, who, schedule)?; Ok(().into()) } @@ -440,12 +440,10 @@ pub mod pallet { /// /// 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 /// * The given asset location is supported /// /// # Parameters - /// * `provided_id`: An id provided by the user. This id must be unique for the user. /// * `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. @@ -466,7 +464,6 @@ pub mod pallet { #[pallet::weight(::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, - provided_id: Vec, schedule: ScheduleParam, destination: Box, schedule_fee: Box, @@ -498,7 +495,7 @@ pub mod pallet { }; let schedule = schedule.validated_into::()?; - Self::validate_and_schedule_task(action, who, provided_id, schedule)?; + Self::validate_and_schedule_task(action, who, schedule)?; Ok(().into()) } @@ -528,8 +525,6 @@ pub mod pallet { account_minimum: BalanceOf, ) -> DispatchResult { let who = ensure_signed(origin)?; - let provided_id: Vec = - Self::generate_auto_compound_delegated_stake_provided_id(&who, &collator_id); let action = Action::AutoCompoundDelegatedStake { delegator: who.clone(), @@ -537,7 +532,7 @@ pub mod pallet { account_minimum, }; let schedule = Schedule::new_recurring_schedule::(execution_time, frequency)?; - Self::validate_and_schedule_task(action, who, provided_id, schedule)?; + Self::validate_and_schedule_task(action, who, schedule)?; Ok(().into()) } @@ -545,7 +540,6 @@ pub mod pallet { /// ** This is currently limited to calls from the System and Balances pallets. /// /// # 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. /// * `call`: The call that will be dispatched. /// @@ -559,7 +553,6 @@ pub mod pallet { #[pallet::weight(::WeightInfo::schedule_dynamic_dispatch_task_full(schedule.number_of_executions()))] pub fn schedule_dynamic_dispatch_task( origin: OriginFor, - provided_id: Vec, schedule: ScheduleParam, call: Box<::Call>, ) -> DispatchResult { @@ -569,7 +562,7 @@ pub mod pallet { let action = Action::DynamicDispatch { encoded_call }; let schedule = schedule.validated_into::()?; - Self::validate_and_schedule_task(action, who, provided_id, schedule)?; + Self::validate_and_schedule_task(action, who, schedule)?; Ok(().into()) } @@ -584,12 +577,12 @@ pub mod pallet { /// * `TaskDoesNotExist`: The task does not exist. #[pallet::call_index(6)] #[pallet::weight(::WeightInfo::cancel_scheduled_task_full())] - pub fn cancel_task(origin: OriginFor, task_id: TaskId) -> DispatchResult { + pub fn cancel_task(origin: OriginFor, task_id: TaskIdV2) -> DispatchResult { let who = ensure_signed(origin)?; - AccountTasks::::get(who, task_id) + AccountTasks::::get(who, task_id.clone()) .ok_or(Error::::TaskDoesNotExist) - .map(|task| Self::remove_task(task_id, task))?; + .map(|task| Self::remove_task(task_id.clone(), task))?; Ok(().into()) } @@ -607,13 +600,13 @@ pub mod pallet { pub fn force_cancel_task( origin: OriginFor, owner_id: AccountOf, - task_id: TaskId, + task_id: TaskIdV2, ) -> DispatchResult { ensure_root(origin)?; - AccountTasks::::get(owner_id, task_id) + AccountTasks::::get(owner_id, task_id.clone()) .ok_or(Error::::TaskDoesNotExist) - .map(|task| Self::remove_task(task_id, task))?; + .map(|task| Self::remove_task(task_id.clone(), task))?; Ok(().into()) } @@ -912,7 +905,12 @@ pub mod pallet { let (task_action_weight, dispatch_error) = match task.action.clone() { Action::Notify { message } => Self::run_notify_task(message), Action::NativeTransfer { sender, recipient, amount } => - Self::run_native_transfer_task(sender, recipient, amount, *task_id), + Self::run_native_transfer_task( + sender, + recipient, + amount, + task_id.clone(), + ), Action::XCMP { destination, execution_fee, @@ -929,7 +927,7 @@ pub mod pallet { encoded_call, encoded_call_weight, overall_weight, - *task_id, + task_id.clone(), instruction_sequence, ), Action::AutoCompoundDelegatedStake { @@ -940,17 +938,17 @@ pub mod pallet { delegator, collator, account_minimum, - *task_id, + task_id.clone(), &task, ), Action::DynamicDispatch { encoded_call } => Self::run_dynamic_dispatch_action( task.owner_id.clone(), encoded_call, - *task_id, + task_id.clone(), ), }; - Self::handle_task_post_processing(*task_id, task, dispatch_error); + Self::handle_task_post_processing(task_id.clone(), task, dispatch_error); task_action_weight .saturating_add(T::DbWeight::get().writes(1u64)) .saturating_add(T::DbWeight::get().reads(1u64)) @@ -984,26 +982,27 @@ pub mod pallet { for missed_task in missed_tasks.iter() { consumed_task_index += 1; - let action_weight = - match AccountTasks::::get(missed_task.owner_id.clone(), missed_task.task_id) - { - None => { - Self::deposit_event(Event::TaskNotFound { - who: missed_task.owner_id.clone(), - task_id: missed_task.task_id.clone(), - }); - ::WeightInfo::run_missed_tasks_many_missing(1) - }, - Some(task) => { - Self::deposit_event(Event::TaskMissed { - who: task.owner_id.clone(), - task_id: missed_task.task_id.clone(), - execution_time: missed_task.execution_time, - }); - Self::handle_task_post_processing(missed_task.task_id, task, None); - ::WeightInfo::run_missed_tasks_many_found(1) - }, - }; + let action_weight = match AccountTasks::::get( + missed_task.owner_id.clone(), + missed_task.task_id.clone(), + ) { + None => { + Self::deposit_event(Event::TaskNotFound { + who: missed_task.owner_id.clone(), + task_id: missed_task.task_id.clone(), + }); + ::WeightInfo::run_missed_tasks_many_missing(1) + }, + Some(task) => { + Self::deposit_event(Event::TaskMissed { + who: task.owner_id.clone(), + task_id: missed_task.task_id.clone(), + execution_time: missed_task.execution_time, + }); + Self::handle_task_post_processing(missed_task.task_id.clone(), task, None); + ::WeightInfo::run_missed_tasks_many_found(1) + }, + }; weight_left = weight_left.saturating_sub(action_weight); @@ -1032,7 +1031,7 @@ pub mod pallet { sender: AccountOf, recipient: AccountOf, amount: BalanceOf, - task_id: T::Hash, + task_id: TaskIdV2, ) -> (Weight, Option) { match T::Currency::transfer( &sender, @@ -1058,7 +1057,7 @@ pub mod pallet { encoded_call: Vec, encoded_call_weight: Weight, overall_weight: Weight, - task_id: TaskId, + task_id: TaskIdV2, flow: InstructionSequence, ) -> (Weight, Option) { let fee_asset_location = MultiLocation::try_from(fee.asset_location); @@ -1096,7 +1095,7 @@ pub mod pallet { delegator: AccountOf, collator: AccountOf, account_minimum: BalanceOf, - task_id: TaskId, + task_id: TaskIdV2, task: &TaskOf, ) -> (Weight, Option) { // TODO: Handle edge case where user has enough funds to run task but not reschedule @@ -1132,7 +1131,7 @@ pub mod pallet { pub fn run_dynamic_dispatch_action( caller: AccountOf, encoded_call: Vec, - task_id: TaskId, + task_id: TaskIdV2, ) -> (Weight, Option) { match ::Call::decode(&mut &*encoded_call) { Ok(scheduled_call) => { @@ -1181,15 +1180,15 @@ pub mod pallet { /// Decrements task executions left. /// If task is complete then removes task. If task not complete update task map. /// A task has been completed if executions left equals 0. - fn decrement_task_and_remove_if_complete(task_id: TaskId, mut task: TaskOf) { + fn decrement_task_and_remove_if_complete(task_id: TaskIdV2, mut task: TaskOf) { match task.schedule { Schedule::Fixed { ref mut executions_left, .. } => { *executions_left = executions_left.saturating_sub(1); if *executions_left <= 0 { - AccountTasks::::remove(task.owner_id.clone(), task_id); + AccountTasks::::remove(task.owner_id.clone(), task_id.clone()); Self::deposit_event(Event::TaskCompleted { who: task.owner_id.clone(), - task_id, + task_id: task_id.clone(), }); } else { AccountTasks::::insert(task.owner_id.clone(), task_id, task); @@ -1200,7 +1199,7 @@ pub mod pallet { } /// Removes the task of the provided task_id and all scheduled tasks, including those in the task queue. - fn remove_task(task_id: TaskId, task: TaskOf) { + fn remove_task(task_id: TaskIdV2, task: TaskOf) { let mut found_task: bool = false; let mut execution_times = task.execution_times(); Self::clean_execution_times_vector(&mut execution_times); @@ -1289,23 +1288,26 @@ pub mod pallet { } if !found_task { - Self::deposit_event(Event::TaskNotFound { who: task.owner_id.clone(), task_id }); + Self::deposit_event(Event::TaskNotFound { + who: task.owner_id.clone(), + task_id: task_id.clone(), + }); } - AccountTasks::::remove(task.owner_id.clone(), task_id); - Self::deposit_event(Event::TaskCancelled { who: task.owner_id, task_id }); + AccountTasks::::remove(task.owner_id.clone(), task_id.clone()); + Self::deposit_event(Event::TaskCancelled { + who: task.owner_id, + task_id: task_id.clone(), + }); } /// Schedule task and return it's task_id. - pub fn schedule_task( - task: &TaskOf, - provided_id: Vec, - ) -> Result, Error> { + pub fn schedule_task(task: &TaskOf) -> Result> { let owner_id = task.owner_id.clone(); - let task_id = Self::generate_task_id(owner_id.clone(), provided_id.clone()); + let execution_times = task.execution_times(); - if AccountTasks::::contains_key(owner_id.clone(), task_id) { + if AccountTasks::::contains_key(owner_id.clone(), task.task_id.clone()) { Err(Error::::DuplicateTask)?; } @@ -1313,27 +1315,30 @@ pub mod pallet { #[cfg(feature = "dev-queue")] if execution_times == vec![0] { let mut task_queue = Self::get_task_queue(); - task_queue.push((owner_id, task_id)); + task_queue.push((owner_id, task.task_id.clone())); TaskQueueV2::::put(task_queue); - return Ok(task_id) + return Ok(task.task_id.clone()) } - Self::insert_scheduled_tasks(task_id, task, execution_times) + Self::insert_scheduled_tasks(task, execution_times) } /// Insert the account/task id into scheduled tasks /// With transaction will protect against a partial success where N of M execution times might be full, /// rolling back any successful insertions into the schedule task table. fn insert_scheduled_tasks( - task_id: TaskId, task: &TaskOf, execution_times: Vec, - ) -> Result, Error> { - with_transaction(|| -> storage::TransactionOutcome, DispatchError>> { + ) -> Result> { + let task_id = task.task_id.clone(); + + with_transaction(|| -> storage::TransactionOutcome> { for time in execution_times.iter() { let mut scheduled_tasks = Self::get_scheduled_tasks(*time).unwrap_or_default(); - if let Err(_) = scheduled_tasks.try_push::>(task_id, task) { + if let Err(_) = + scheduled_tasks.try_push::>(task_id.clone(), task) + { return Rollback(Err(DispatchError::Other("time slot full"))) } >::insert(*time, scheduled_tasks); @@ -1344,19 +1349,13 @@ pub mod pallet { .map_err(|_| Error::::TimeSlotFull) } - /// TODO ENG-538: Refactor validate_and_schedule_task function /// Validate and schedule task. /// This will also charge the execution fee. pub fn validate_and_schedule_task( action: ActionOf, owner_id: AccountOf, - provided_id: Vec, schedule: Schedule, ) -> DispatchResult { - if provided_id.len() == 0 { - Err(Error::::EmptyProvidedId)? - } - match action.clone() { Action::XCMP { execution_fee, instruction_sequence, .. } => { let asset_location = MultiLocation::try_from(execution_fee.asset_location) @@ -1380,13 +1379,17 @@ pub mod pallet { let executions = schedule.known_executions_left(); - let task = - TaskOf::::new(owner_id.clone(), provided_id.clone(), schedule, action.clone()); + let task = TaskOf::::new( + owner_id.clone(), + Self::generate_task_idv2(), + schedule, + action.clone(), + ); let task_id = T::FeeHandler::pay_checked_fees_for(&owner_id, &action, executions, || { - let task_id = Self::schedule_task(&task, provided_id)?; - AccountTasks::::insert(owner_id.clone(), task_id, task); + let task_id = Self::schedule_task(&task)?; + AccountTasks::::insert(owner_id.clone(), task_id.clone(), task); Ok(task_id) })?; @@ -1396,25 +1399,24 @@ pub mod pallet { }; Self::deposit_event(Event::::TaskScheduled { who: owner_id, task_id, schedule_as }); + Ok(()) } - fn reschedule_or_remove_task( - task_id: TaskId, - mut task: TaskOf, - dispatch_error: Option, - ) { + fn reschedule_or_remove_task(mut task: TaskOf, dispatch_error: Option) { + let task_id = task.task_id.clone(); + if let Some(err) = dispatch_error { Self::deposit_event(Event::::TaskNotRescheduled { who: task.owner_id.clone(), - task_id, + task_id: task_id.clone(), error: err, }); - AccountTasks::::remove(task.owner_id.clone(), task_id); + AccountTasks::::remove(task.owner_id.clone(), task_id.clone()); } else { let owner_id = task.owner_id.clone(); let action = task.action.clone(); - match Self::reschedule_existing_task(task_id, &mut task) { + match Self::reschedule_existing_task(&mut task) { Ok(_) => { let schedule_as = match action { Action::XCMP { schedule_as, .. } => schedule_as, @@ -1422,23 +1424,25 @@ pub mod pallet { }; Self::deposit_event(Event::::TaskRescheduled { who: owner_id, - task_id, + task_id: task_id.clone(), schedule_as, }); }, Err(err) => { Self::deposit_event(Event::::TaskFailedToReschedule { who: task.owner_id.clone(), - task_id, + task_id: task_id.clone(), error: err, }); - AccountTasks::::remove(task.owner_id.clone(), task_id); + AccountTasks::::remove(task.owner_id.clone(), task_id.clone()); }, }; } } - fn reschedule_existing_task(task_id: TaskId, task: &mut TaskOf) -> DispatchResult { + fn reschedule_existing_task(task: &mut TaskOf) -> DispatchResult { + let task_id = task.task_id.clone(); + match task.schedule { Schedule::Recurring { ref mut next_execution_time, frequency } => { let new_execution_time = next_execution_time @@ -1448,12 +1452,12 @@ pub mod pallet { // TODO: should execution fee depend on whether task is recurring? T::FeeHandler::pay_checked_fees_for(&task.owner_id, &task.action, 1, || { - Self::insert_scheduled_tasks(task_id, task, vec![new_execution_time]) + Self::insert_scheduled_tasks(task, vec![new_execution_time]) .map_err(|e| e.into()) })?; let owner_id = task.owner_id.clone(); - AccountTasks::::insert(owner_id.clone(), task_id, task.clone()); + AccountTasks::::insert(owner_id.clone(), task_id.clone(), task.clone()); let schedule_as = match task.action.clone() { Action::XCMP { schedule_as, .. } => schedule_as.clone(), @@ -1462,7 +1466,7 @@ pub mod pallet { Self::deposit_event(Event::::TaskScheduled { who: owner_id, - task_id, + task_id: task_id.clone(), schedule_as, }); }, @@ -1472,30 +1476,52 @@ pub mod pallet { } fn handle_task_post_processing( - task_id: TaskId, + task_id: TaskIdV2, task: TaskOf, error: Option, ) { match task.schedule { Schedule::Fixed { .. } => Self::decrement_task_and_remove_if_complete(task_id, task), - Schedule::Recurring { .. } => Self::reschedule_or_remove_task(task_id, task, error), + Schedule::Recurring { .. } => Self::reschedule_or_remove_task(task, error), } } - pub fn generate_task_id(owner_id: AccountOf, provided_id: Vec) -> TaskId { - let task_hash_input = TaskHashInput::new(owner_id, provided_id); - T::Hashing::hash_of(&task_hash_input) + pub fn generate_task_idv2() -> TaskIdV2 { + let current_block_number = + match TryInto::::try_into(>::block_number()).ok() { + Some(i) => i, + None => 0, + }; + + let tx_id = match >::extrinsic_index() { + Some(i) => i, + None => 0, + }; + + let evt_index = >::event_count(); + + format!("{:}-{:}-{:}", current_block_number, tx_id, evt_index) + .as_bytes() + .to_vec() } - pub fn generate_auto_compound_delegated_stake_provided_id( - delegator: &AccountOf, - collator: &AccountOf, - ) -> Vec { - let mut provided_id = "AutoCompoundDelegatedStake".as_bytes().to_vec(); - provided_id.extend(delegator.encode()); - provided_id.extend(collator.encode()); - provided_id + // Find auto compount tasks of a paritcular account by iterate over AccountTasks + // StorageDoubleMap using the first key prefix which is account_id. + pub fn get_auto_compound_delegated_stake_task_ids( + account_id: AccountOf, + ) -> Vec { + AccountTasks::::iter_prefix_values(account_id.clone()) + .filter(|task| { + match task.action { + // We don't care about the inner content, we just want to pick out the + // AutoCompoundDelegatedStake + Action::AutoCompoundDelegatedStake { .. } => true, + _ => false, + } + }) + .map(|task| task.task_id) + .collect() } /// Calculates the execution fee for a given action based on weight and num of executions diff --git a/pallets/automation-time/src/migrations/mod.rs b/pallets/automation-time/src/migrations/mod.rs index 1a3fff88f..2db2f6a4a 100644 --- a/pallets/automation-time/src/migrations/mod.rs +++ b/pallets/automation-time/src/migrations/mod.rs @@ -1,3 +1,5 @@ // Migrations +pub mod update_task_idv2; pub mod update_xcmp_task; +pub mod utils; diff --git a/pallets/automation-time/src/migrations/update_task_idv2.rs b/pallets/automation-time/src/migrations/update_task_idv2.rs new file mode 100644 index 000000000..a61e33975 --- /dev/null +++ b/pallets/automation-time/src/migrations/update_task_idv2.rs @@ -0,0 +1,387 @@ +use core::marker::PhantomData; + +use crate::{ + AccountTaskId, Action, Config, MissedTaskV2, MissedTaskV2Of, ScheduledTasksOf, TaskIdV2, + UnixTime, +}; +use codec::{Decode, Encode}; +use cumulus_primitives_core::ParaId; +use frame_support::{ + dispatch::EncodeLike, + storage::types::ValueQuery, + traits::{Get, OnRuntimeUpgrade}, + weights::{RuntimeDbWeight, Weight}, + Twox64Concat, +}; +use scale_info::{prelude::format, TypeInfo}; + +use sp_runtime::traits::Convert; +use sp_std::{vec, vec::Vec}; +use xcm::{latest::prelude::*, VersionedMultiLocation}; + +use crate::migrations::utils::{ + deprecate::{generate_old_task_id, old_taskid_to_idv2}, + OldAccountTaskId, OldMissedTaskV2Of, OldScheduledTasksOf, TEST_TASKID1, TEST_TASKID2, +}; + +// This is old TaskQueueV2 with old Task +#[frame_support::storage_alias] +pub type TaskQueueV2 = + StorageValue>, ValueQuery>; + +pub struct UpdateTaskIDV2ForTaskQueueV2(PhantomData); +impl OnRuntimeUpgrade for UpdateTaskIDV2ForTaskQueueV2 { + fn on_runtime_upgrade() -> Weight { + log::info!(target: "automation-time", "TaskID migration"); + + let mut storage_ops: u64 = 0; + + let old_task_queue = TaskQueueV2::::get(); + let migrated_tasks = old_task_queue.len(); + + let new_task_queue: Vec> = old_task_queue + .iter() + .map(|(a, b)| ((*a).clone(), old_taskid_to_idv2::(b))) + .collect::>>(); + + storage_ops += old_task_queue.len() as u64; + crate::TaskQueueV2::::put(new_task_queue); + + log::info!( + target: "automation-time", + "migration: Update TaskQueueV2 succesful! Migrated {} tasks.", + migrated_tasks + ); + + let db_weight: RuntimeDbWeight = T::DbWeight::get(); + db_weight.reads_writes(storage_ops + 1, storage_ops + 1) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + let prev_count = crate::TaskQueueV2::::get().len() as u32; + Ok(prev_count.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { + let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) + .expect("the state parameter should be something that was generated by pre_upgrade"); + let post_count = crate::TaskQueueV2::::get().len() as u32; + assert!(post_count == prev_count); + + Ok(()) + } +} + +// This is old ScheduledTasksV3 with old Taskid using T::Hash +#[frame_support::storage_alias] +pub type ScheduledTasksV3 = + StorageMap>; +pub struct UpdateTaskIDV2ForScheduledTasksV3(PhantomData); +impl OnRuntimeUpgrade for UpdateTaskIDV2ForScheduledTasksV3 { + fn on_runtime_upgrade() -> Weight { + log::info!(target: "automation-time", "TaskID migration"); + + let mut migrated_tasks = 0; + let mut storage_ops: u64 = 0; + ScheduledTasksV3::::iter().for_each(|(time_slot, old_scheduled_tasks)| { + storage_ops += 1; + let migrated_scheduled_tasks: Vec<(T::AccountId, TaskIdV2)> = old_scheduled_tasks + .tasks + .into_iter() + .map(|(account_id, old_task_id)| { + migrated_tasks += 1; + storage_ops += 1; + (account_id.clone(), old_taskid_to_idv2::(&(old_task_id.clone()))) + }) + .collect::>(); + + storage_ops += 1; + crate::ScheduledTasksV3::::insert( + time_slot, + ScheduledTasksOf:: { + tasks: migrated_scheduled_tasks, + weight: old_scheduled_tasks.weight, + }, + ); + }); + + log::info!( + target: "automation-time", + "migration: Update ScheduledTasksV3 succesful! Migrated {} tasks.", + migrated_tasks + ); + + let db_weight: RuntimeDbWeight = T::DbWeight::get(); + db_weight.reads_writes(storage_ops + 1, storage_ops + 1) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + let prev_count = crate::ScheduledTasksV3::::iter().count() as u32; + Ok(prev_count.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { + let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) + .expect("the state parameter should be something that was generated by pre_upgrade"); + let post_count = crate::ScheduledTasksV3::::iter().count() as u32; + assert!(post_count == prev_count); + + Ok(()) + } +} + +// This is old MissedQueueV2 with old Task +#[frame_support::storage_alias] +pub type MissedQueueV2 = + StorageValue>, ValueQuery>; + +pub struct UpdateTaskIDV2ForMissedQueueV2(PhantomData); +impl OnRuntimeUpgrade for UpdateTaskIDV2ForMissedQueueV2 { + fn on_runtime_upgrade() -> Weight { + log::info!(target: "automation-time", "TaskID migration"); + + let old_missed_queue = MissedQueueV2::::get(); + let migrated_tasks: u64 = old_missed_queue.len() as u64; + + let migrated_missed_queuee: Vec> = old_missed_queue + .into_iter() + .map(|old_missed_task| { + let a: MissedTaskV2Of = MissedTaskV2Of:: { + owner_id: old_missed_task.owner_id.clone(), + execution_time: old_missed_task.execution_time, + task_id: old_taskid_to_idv2::(&old_missed_task.task_id.clone()), + }; + + a + }) + .collect::>>(); + + crate::MissedQueueV2::::put(migrated_missed_queuee); + + log::info!( + target: "automation-time", + "migration: Update MissedQueueV2 succesful! Migrated {} tasks.", + migrated_tasks + ); + + let db_weight: RuntimeDbWeight = ::DbWeight::get(); + db_weight.reads_writes(migrated_tasks + 1, migrated_tasks + 1) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + let prev_count = crate::MissedQueueV2::::get().len() as u32; + Ok(prev_count.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev_count: Vec) -> Result<(), &'static str> { + let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) + .expect("the state parameter should be something that was generated by pre_upgrade"); + let post_count = crate::MissedQueueV2::::get().len() as u32; + assert!(post_count == prev_count); + + Ok(()) + } +} + +#[cfg(test)] +mod test { + use super::{ + generate_old_task_id, old_taskid_to_idv2, MissedTaskV2, ParaId, + UpdateTaskIDV2ForMissedQueueV2, UpdateTaskIDV2ForScheduledTasksV3, + UpdateTaskIDV2ForTaskQueueV2, TEST_TASKID1, TEST_TASKID2, + }; + use crate::{ + migrations::utils::{OldMissedTaskV2, OldScheduledTasksOf, OldTask}, + mock::*, + }; + use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; + use sp_runtime::AccountId32; + use xcm::latest::prelude::*; + + #[test] + fn on_runtime_upgrade() { + new_test_ext(0).execute_with(|| { + let account_id1 = AccountId32::new(ALICE); + let account_id2 = AccountId32::new(BOB); + + let task_id1: Vec = TEST_TASKID1.as_bytes().to_vec(); + let task_id2: Vec = TEST_TASKID2.as_bytes().to_vec(); + + let old_taskqueuev2 = vec![ + (account_id1.clone(), generate_old_task_id::(account_id1.clone(), vec![1])), + (account_id2.clone(), generate_old_task_id::(account_id2.clone(), vec![2])), + ]; + + super::TaskQueueV2::::put(old_taskqueuev2); + UpdateTaskIDV2ForTaskQueueV2::::on_runtime_upgrade(); + + let queue = crate::TaskQueueV2::::get(); + let (task_owner, task_id) = + queue.first().expect("migration task failed to copy task over"); + + assert_eq!( + *task_owner, + account_id1.clone(), + "migration failed to convert OldTaskId -> TaskIDV2 for TaskQueueV2" + ); + assert_eq!(*task_id, task_id1); + + let (task_owner, task_id) = + queue.last().expect("migration task failed to copy task over"); + assert_eq!( + *task_owner, + account_id2.clone(), + "migration failed to convert OldTaskId -> TaskIDV2 for TaskQueueV2" + ); + assert_eq!(*task_id, task_id2); + }) + } + + #[test] + fn on_runtime_upgrade_for_schedule_taskv3() { + new_test_ext(0).execute_with(|| { + let task_owner1 = AccountId32::new(ALICE); + let task_owner2 = AccountId32::new(BOB); + + // These are H256/BlakeTwo256 hex generate from our old task id generation from hashing + let task_id1: Vec = TEST_TASKID1.as_bytes().to_vec(); + let task_id2: Vec = TEST_TASKID2.as_bytes().to_vec(); + + super::ScheduledTasksV3::::insert( + 3600, + OldScheduledTasksOf:: { + tasks: vec![ + ( + task_owner1.clone(), + generate_old_task_id::(task_owner1.clone(), vec![1]), + ), + ( + task_owner2.clone(), + generate_old_task_id::(task_owner2.clone(), vec![2]), + ), + ], + // just a random number to compare later on + weight: 1_789_657, + }, + ); + + super::ScheduledTasksV3::::insert( + 7200, + OldScheduledTasksOf:: { + tasks: vec![ + ( + task_owner2.clone(), + generate_old_task_id::(task_owner2.clone(), vec![33]), + ), + ( + task_owner1.clone(), + generate_old_task_id::(task_owner1.clone(), vec![32]), + ), + ], + // just a random number to compare later on + weight: 1_967_672, + }, + ); + UpdateTaskIDV2ForScheduledTasksV3::::on_runtime_upgrade(); + + // ensure all the slots are converted properly + let scheduled_tasks = crate::ScheduledTasksV3::::get(3600) + .expect("ScheduledTasksV3 failed to migrate"); + assert_eq!( + scheduled_tasks.weight, 1_789_657, + "migration failed to convert old ScheduledTasksV3 to new TaskID format" + ); + assert_eq!( + scheduled_tasks.tasks, + vec![ + (task_owner1.clone(), task_id1.clone()), + (task_owner2.clone(), task_id2.clone()), + ], + "migration failed to convert old ScheduledTasksV3 to new TaskID format" + ); + + let scheduled_tasks = crate::ScheduledTasksV3::::get(7200) + .expect("ScheduledTasksV3 failed to migrate"); + assert_eq!( + scheduled_tasks.weight, 1_967_672, + "migration failed to convert old ScheduledTasksV3 to new TaskID format" + ); + assert_eq!( + scheduled_tasks.tasks, + vec![ + // (task owner, vec![33]) hash -> "0x7191f3d83bcbeb221f7b00f501e9a8da3ba3c2d15a672eb694ee7e09dbaddd1e" + ( + task_owner2.clone(), + "0x7191f3d83bcbeb221f7b00f501e9a8da3ba3c2d15a672eb694ee7e09dbaddd1e" + .as_bytes() + .to_vec(), + ), + // (task owner1, vec![32]) hash -> "0xe94040ca5d09f0e1023aecf5abc3afac0b9e66bc3b1209183b3a009f4c073c2b" + ( + task_owner1.clone(), + "0xe94040ca5d09f0e1023aecf5abc3afac0b9e66bc3b1209183b3a009f4c073c2b" + .as_bytes() + .to_vec(), + ), + ], + "migration failed to convert old ScheduledTasksV3 to new TaskID format" + ); + }) + } + + #[test] + fn on_runtime_upgrade_for_missed_queue_v2() { + new_test_ext(0).execute_with(|| { + let account_id1 = AccountId32::new(ALICE); + let account_id2 = AccountId32::new(BOB); + + // These are H256/BlakeTwo256 hex generate from our old task id generation from hashing + let task_id1: Vec = TEST_TASKID1.as_bytes().to_vec(); + let task_id2: Vec = TEST_TASKID2.as_bytes().to_vec(); + + let old_missed_queueu_v2 = vec![ + OldMissedTaskV2 { + owner_id: account_id1.clone(), + task_id: generate_old_task_id::(account_id1.clone(), vec![1]), + execution_time: 3600, + }, + OldMissedTaskV2 { + owner_id: account_id2.clone(), + task_id: generate_old_task_id::(account_id2.clone(), vec![2]), + execution_time: 7200, + }, + ]; + + super::MissedQueueV2::::put(old_missed_queueu_v2); + UpdateTaskIDV2ForMissedQueueV2::::on_runtime_upgrade(); + + let queue = crate::MissedQueueV2::::get(); + + assert_eq!( + queue.first().expect("migration task failed to copy missed queuev2 over"), + &MissedTaskV2 { + owner_id: account_id1.clone(), + task_id: task_id1, + execution_time: 3600, + }, + "migration failed to convert old MissedTaskV2 -> new MissedTaskV2 for MissedQueueV2" + ); + + assert_eq!( + queue.last().expect("migration task failed to copy missed queuev2 over"), + &MissedTaskV2 { + owner_id: account_id2.clone(), + task_id: task_id2, + execution_time: 7200, + }, + "migration failed to convert old MissedTaskV2 -> new MissedTaskV2 for MissedQueueV2" + ); + }) + } +} diff --git a/pallets/automation-time/src/migrations/update_xcmp_task.rs b/pallets/automation-time/src/migrations/update_xcmp_task.rs index d5b1bddac..7f8da9d59 100644 --- a/pallets/automation-time/src/migrations/update_xcmp_task.rs +++ b/pallets/automation-time/src/migrations/update_xcmp_task.rs @@ -2,68 +2,24 @@ use core::marker::PhantomData; use crate::{ weights::WeightInfo, AccountOf, ActionOf, AssetPayment, BalanceOf, Config, InstructionSequence, - Schedule, TaskId, TaskOf, + Schedule, TaskOf, }; use codec::{Decode, Encode}; use cumulus_primitives_core::ParaId; use frame_support::{traits::OnRuntimeUpgrade, weights::Weight, Twox64Concat}; use scale_info::TypeInfo; use sp_runtime::traits::Convert; -use sp_std::vec::Vec; +use sp_std::{vec, vec::Vec}; use xcm::{latest::prelude::*, VersionedMultiLocation}; +use crate::migrations::utils::{ + deprecate::{generate_old_task_id, old_taskid_to_idv2}, + OldAccountTaskId, OldAction, OldTask, OldTaskId, TEST_TASKID1, +}; + const EXECUTION_FEE_AMOUNT: u128 = 3_000_000_000; const INSTRUCTION_WEIGHT_REF_TIME: u64 = 150_000_000; -#[derive(Debug, Encode, Decode, TypeInfo)] -#[scale_info(skip_type_params(MaxExecutionTimes))] -pub struct OldTask { - pub owner_id: T::AccountId, - pub provided_id: Vec, - pub schedule: Schedule, - pub action: OldAction, -} - -impl From> for TaskOf { - fn from(task: OldTask) -> Self { - TaskOf:: { - owner_id: task.owner_id, - provided_id: task.provided_id, - schedule: task.schedule, - action: task.action.into(), - } - } -} - -/// The enum that stores all action specific data. -#[derive(Clone, Debug, Eq, PartialEq, Encode, Decode, TypeInfo)] -pub enum OldAction { - Notify { - message: Vec, - }, - NativeTransfer { - sender: T::AccountId, - recipient: T::AccountId, - amount: BalanceOf, - }, - XCMP { - para_id: ParaId, - currency_id: T::CurrencyId, - xcm_asset_location: VersionedMultiLocation, - encoded_call: Vec, - encoded_call_weight: Weight, - schedule_as: Option, - }, - AutoCompoundDelegatedStake { - delegator: T::AccountId, - collator: T::AccountId, - account_minimum: BalanceOf, - }, - DynamicDispatch { - encoded_call: Vec, - }, -} - impl From> for ActionOf { fn from(action: OldAction) -> Self { match action { @@ -109,7 +65,7 @@ pub type AccountTasks = StorageDoubleMap< Twox64Concat, AccountOf, Twox64Concat, - TaskId, + OldTaskId, OldTask, >; @@ -121,7 +77,11 @@ impl OnRuntimeUpgrade for UpdateXcmpTask { let mut migrated_tasks = 0u32; AccountTasks::::iter().for_each(|(account_id, task_id, task)| { let migrated_task: TaskOf = task.into(); - crate::AccountTasks::::insert(account_id, task_id, migrated_task); + crate::AccountTasks::::insert( + account_id, + migrated_task.task_id.clone(), + migrated_task, + ); migrated_tasks += 1; }); @@ -155,8 +115,8 @@ impl OnRuntimeUpgrade for UpdateXcmpTask { #[cfg(test)] mod test { use super::{ - OldAction, OldTask, ParaId, UpdateXcmpTask, EXECUTION_FEE_AMOUNT, - INSTRUCTION_WEIGHT_REF_TIME, + generate_old_task_id, OldAction, OldTask, ParaId, UpdateXcmpTask, EXECUTION_FEE_AMOUNT, + INSTRUCTION_WEIGHT_REF_TIME, TEST_TASKID1, }; use crate::{mock::*, ActionOf, AssetPayment, InstructionSequence, Pallet, Schedule, TaskOf}; use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; @@ -169,7 +129,6 @@ mod test { let para_id: ParaId = 1000.into(); let account_id = AccountId32::new(ALICE); let schedule_as = Some(AccountId32::new(BOB)); - let task_id = Pallet::::generate_task_id(account_id.clone(), vec![1]); let encoded_call_weight = Weight::from_ref_time(10); let task = OldTask:: { @@ -189,16 +148,20 @@ mod test { }, }; - super::AccountTasks::::insert(account_id.clone(), task_id, task); + let old_task_id = + generate_old_task_id::(account_id.clone(), task.provided_id.clone()); + super::AccountTasks::::insert(account_id.clone(), old_task_id.clone(), task); UpdateXcmpTask::::on_runtime_upgrade(); assert_eq!(crate::AccountTasks::::iter().count(), 1); + + let task_id1 = TEST_TASKID1.as_bytes().to_vec(); assert_eq!( - crate::AccountTasks::::get(account_id.clone(), task_id).unwrap(), + crate::AccountTasks::::get(account_id.clone(), task_id1.clone()).unwrap(), TaskOf:: { owner_id: account_id.clone(), - provided_id: vec![1], + task_id: task_id1, schedule: Schedule::Fixed { execution_times: vec![0, 1].try_into().unwrap(), executions_left: 2 diff --git a/pallets/automation-time/src/migrations/utils.rs b/pallets/automation-time/src/migrations/utils.rs new file mode 100644 index 000000000..04b18073d --- /dev/null +++ b/pallets/automation-time/src/migrations/utils.rs @@ -0,0 +1,142 @@ +// Holding the old data structure so we can share them betwen multiple migrations +use crate::{ + weights::WeightInfo, AccountOf, AccountTaskId, Action, ActionOf, AssetPayment, BalanceOf, + Config, InstructionSequence, MissedTaskV2, MissedTaskV2Of, Schedule, TaskIdV2, TaskOf, + UnixTime, +}; +use frame_system::pallet_prelude::*; + +use codec::{Decode, Encode}; +use cumulus_primitives_core::ParaId; +use frame_support::{ + dispatch::EncodeLike, storage::types::ValueQuery, traits::OnRuntimeUpgrade, weights::Weight, + Twox64Concat, +}; + +use scale_info::{prelude::format, TypeInfo}; +use sp_runtime::traits::Convert; +use sp_std::{vec, vec::Vec}; +use xcm::{latest::prelude::*, VersionedMultiLocation}; + +// These are H256/BlakeTwo256 hex generate from our old task id generation from hashing +// These cons are used for our unit test +// (Account, ProvidedID) +pub const TEST_TASKID1: &str = "0xd1263842e34adeb00be1146b30bc6527951f0a0f5d5a3f7a758537735b8bcb04"; +pub const TEST_TASKID2: &str = "0xf76acf0b1d8ef503450a4d5c1a451f1921a906e8711f551c2945e09fb44de5ff"; + +// Old format +pub type OldTaskId = ::Hash; +pub type OldAccountTaskId = (AccountOf, OldTaskId); + +#[derive(Debug, Encode, Decode, TypeInfo)] +#[scale_info(skip_type_params(MaxExecutionTimes))] +pub struct OldTask { + pub owner_id: T::AccountId, + pub provided_id: Vec, + pub schedule: Schedule, + pub action: OldAction, +} + +impl From> for TaskOf { + fn from(task: OldTask) -> Self { + // Previously, our task id is a blake256 hash of account_id + provided_id + // Now, our task id is generate use a different formula without relying on `provided_id` + // for existing task in our storage, the type of the TaskID was T::Hash. + // When emitting on the event, it can be re-present as hex string of the hash. + // so in new task, we simply take that hash and convert it to the hex string, and convert + // to a Vec which is our new taskidv2 format. + let old_task_id = + deprecate::generate_old_task_id::(task.owner_id.clone(), task.provided_id.clone()); + + TaskOf:: { + owner_id: task.owner_id, + task_id: deprecate::old_taskid_to_idv2::(&old_task_id), + schedule: task.schedule, + action: task.action.into(), + } + } +} + +/// The enum that stores all action specific data. +#[derive(Clone, Debug, Eq, PartialEq, Encode, Decode, TypeInfo)] +pub enum OldAction { + Notify { + message: Vec, + }, + NativeTransfer { + sender: T::AccountId, + recipient: T::AccountId, + amount: BalanceOf, + }, + XCMP { + para_id: ParaId, + currency_id: T::CurrencyId, + xcm_asset_location: VersionedMultiLocation, + encoded_call: Vec, + encoded_call_weight: Weight, + schedule_as: Option, + }, + AutoCompoundDelegatedStake { + delegator: T::AccountId, + collator: T::AccountId, + account_minimum: BalanceOf, + }, + DynamicDispatch { + encoded_call: Vec, + }, +} + +pub type OldScheduledTasksOf = OldScheduledTasks, OldTaskId>; + +#[derive(Debug, Decode, Eq, Encode, PartialEq, TypeInfo)] +#[scale_info(skip_type_params(T))] +pub struct OldScheduledTasks { + pub tasks: Vec<(AccountId, OldTaskId)>, + pub weight: u128, +} + +impl Default for OldScheduledTasks { + fn default() -> Self { + Self { tasks: vec![], weight: 0 } + } +} + +#[derive(Debug, Eq, PartialEq, Encode, Decode, TypeInfo)] +pub struct OldMissedTaskV2 { + pub owner_id: AccountId, + pub task_id: TaskId, + pub execution_time: UnixTime, +} + +pub type OldMissedTaskV2Of = OldMissedTaskV2, OldTaskId>; + +pub mod deprecate { + use super::*; + + use sp_runtime::traits::Hash; + + #[derive(Debug, Encode, Decode, TypeInfo)] + pub struct TaskHashInput { + owner_id: AccountId, + provided_id: Vec, + } + + impl TaskHashInput { + pub fn new(owner_id: AccountId, provided_id: Vec) -> Self { + Self { owner_id, provided_id } + } + } + + pub fn old_taskid_to_idv2(old_task_id: &OldTaskId) -> Vec { + let task_id = format!("{:?}", old_task_id); + return task_id.as_bytes().to_vec() + } + + pub fn generate_old_task_id( + owner_id: AccountOf, + provided_id: Vec, + ) -> OldTaskId { + let task_hash_input = TaskHashInput::new(owner_id, provided_id); + T::Hashing::hash_of(&task_hash_input) + } +} diff --git a/pallets/automation-time/src/mock.rs b/pallets/automation-time/src/mock.rs index 441d8d7b4..cae32a6b2 100644 --- a/pallets/automation-time/src/mock.rs +++ b/pallets/automation-time/src/mock.rs @@ -17,6 +17,8 @@ use super::*; use crate as pallet_automation_time; +use crate::TaskIdV2; + use frame_benchmarking::frame_support::assert_ok; use frame_support::{ construct_runtime, parameter_types, traits::Everything, weights::Weight, PalletId, @@ -30,7 +32,7 @@ use sp_runtime::{ traits::{AccountIdConversion, BlakeTwo256, CheckedSub, Convert, IdentityLookup}, AccountId32, DispatchError, Perbill, }; -use sp_std::marker::PhantomData; +use sp_std::{marker::PhantomData, vec::Vec}; use xcm::latest::prelude::*; type UncheckedExtrinsic = system::mocking::MockUncheckedExtrinsic; @@ -115,6 +117,7 @@ impl system::Config for Test { type Lookup = IdentityLookup; type Header = Header; type RuntimeEvent = RuntimeEvent; + //type RuntimeEvent = From> + IsType<::RuntimeEvent>; type BlockHashCount = BlockHashCount; type Version = (); type PalletInfo = PalletInfo; @@ -467,8 +470,8 @@ impl pallet_automation_time::Config for Test { // Build genesis storage according to the mock runtime. pub fn new_test_ext(state_block_time: u64) -> sp_io::TestExternalities { - let t = system::GenesisConfig::default().build_storage::().unwrap(); - let mut ext = sp_io::TestExternalities::new(t); + let genesis_storage = system::GenesisConfig::default().build_storage::().unwrap(); + let mut ext = sp_io::TestExternalities::new(genesis_storage); ext.execute_with(|| System::set_block_number(1)); ext.execute_with(|| Timestamp::set_timestamp(state_block_time)); ext @@ -477,25 +480,19 @@ pub fn new_test_ext(state_block_time: u64) -> sp_io::TestExternalities { // A function to support test scheduleing a Fixed schedule // We don't focus on making sure the execution run properly. We just focus on // making sure a task is scheduled into the queue -pub fn schedule_task( - owner: [u8; 32], - provided_id: Vec, - scheduled_times: Vec, - message: Vec, -) -> sp_core::H256 { +pub fn schedule_task(owner: [u8; 32], scheduled_times: Vec, message: Vec) -> TaskIdV2 { let account_id = AccountId32::new(owner); - let task_hash_input = TaskHashInput::new(account_id.clone(), provided_id.clone()); let call: RuntimeCall = frame_system::Call::remark_with_event { remark: message }.into(); assert_ok!(fund_account_dynamic_dispatch(&account_id, scheduled_times.len(), call.encode())); assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(account_id.clone()), - provided_id, ScheduleParam::Fixed { execution_times: scheduled_times }, Box::new(call), )); - BlakeTwo256::hash_of(&task_hash_input) + + last_task_id() } // A function to support test scheduling a Recurring schedule @@ -503,70 +500,67 @@ pub fn schedule_task( // making sure a task is scheduled into the queue pub fn schedule_recurring_task( owner: [u8; 32], - provided_id: Vec, next_execution_time: UnixTime, frequency: Seconds, message: Vec, -) -> sp_core::H256 { +) -> TaskIdV2 { let account_id = AccountId32::new(owner); - let task_hash_input = TaskHashInput::new(account_id.clone(), provided_id.clone()); let call: RuntimeCall = frame_system::Call::remark_with_event { remark: message }.into(); assert_ok!(fund_account_dynamic_dispatch(&account_id, 1, call.encode())); assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(account_id.clone()), - provided_id, ScheduleParam::Recurring { next_execution_time, frequency }, Box::new(call), )); - BlakeTwo256::hash_of(&task_hash_input) + last_task_id() } pub fn add_task_to_task_queue( owner: [u8; 32], - provided_id: Vec, + task_id: TaskIdV2, scheduled_times: Vec, action: ActionOf, -) -> sp_core::H256 { +) -> TaskIdV2 { let schedule = Schedule::new_fixed_schedule::(scheduled_times).unwrap(); - add_to_task_queue(owner, provided_id, schedule, action) + add_to_task_queue(owner, task_id, schedule, action) } pub fn add_recurring_task_to_task_queue( owner: [u8; 32], - provided_id: Vec, + task_id: TaskIdV2, scheduled_time: u64, frequency: u64, action: ActionOf, -) -> sp_core::H256 { +) -> TaskIdV2 { let schedule = Schedule::new_recurring_schedule::(scheduled_time, frequency).unwrap(); - add_to_task_queue(owner, provided_id, schedule, action) + add_to_task_queue(owner, task_id, schedule, action) } pub fn add_to_task_queue( owner: [u8; 32], - provided_id: Vec, + task_id: TaskIdV2, schedule: Schedule, action: ActionOf, -) -> sp_core::H256 { - let task_id = create_task(owner, provided_id, schedule, action); +) -> TaskIdV2 { + let task_id = create_task(owner, task_id, schedule, action); let mut task_queue = AutomationTime::get_task_queue(); - task_queue.push((AccountId32::new(owner), task_id)); + task_queue.push((AccountId32::new(owner), task_id.clone())); TaskQueueV2::::put(task_queue); task_id } pub fn add_task_to_missed_queue( owner: [u8; 32], - provided_id: Vec, + task_id: TaskIdV2, scheduled_times: Vec, action: ActionOf, -) -> sp_core::H256 { +) -> TaskIdV2 { let schedule = Schedule::new_fixed_schedule::(scheduled_times.clone()).unwrap(); - let task_id = create_task(owner, provided_id, schedule, action); + let task_id = create_task(owner, task_id.clone(), schedule, action); let missed_task = - MissedTaskV2Of::::new(AccountId32::new(owner), task_id, scheduled_times[0]); + MissedTaskV2Of::::new(AccountId32::new(owner), task_id.clone(), scheduled_times[0]); let mut missed_queue = AutomationTime::get_missed_queue(); missed_queue.push(missed_task); MissedQueueV2::::put(missed_queue); @@ -575,14 +569,12 @@ pub fn add_task_to_missed_queue( pub fn create_task( owner: [u8; 32], - provided_id: Vec, + task_id: TaskIdV2, schedule: Schedule, action: ActionOf, -) -> sp_core::H256 { - let task_hash_input = TaskHashInput::new(AccountId32::new(owner), provided_id.clone()); - let task_id = BlakeTwo256::hash_of(&task_hash_input); - let task = TaskOf::::new(owner.into(), provided_id, schedule, action); - AccountTasks::::insert(AccountId::new(owner), task_id, task); +) -> TaskIdV2 { + let task = TaskOf::::new(owner.into(), task_id.clone(), schedule, action); + AccountTasks::::insert(AccountId::new(owner), task_id.clone(), task); task_id } @@ -598,6 +590,31 @@ pub fn last_event() -> RuntimeEvent { events().pop().unwrap() } +// A utility test function to simplify the process of getting a task id that we just scheduled in the +// test by looking at the last id and pluck it +pub fn last_task_id() -> TaskIdV2 { + get_task_ids_from_events() + .last() + .expect("Unable to find a task_id from the existing TaskScheduled events") + .clone() +} + +// A utility test function to pluck out the task id from events, useful when dealing with multiple +// task scheduling +pub fn get_task_ids_from_events() -> Vec { + System::events() + .into_iter() + .filter_map(|e| match e.event { + RuntimeEvent::AutomationTime(crate::Event::TaskScheduled { + who, + schedule_as, + task_id, + }) => Some(task_id), + _ => None, + }) + .collect::>() +} + pub fn get_funds(account: AccountId) { let double_action_weight = MockWeight::::run_native_transfer_task() * 2; let action_fee = ExecutionWeightFee::get() * u128::from(double_action_weight.ref_time()); diff --git a/pallets/automation-time/src/tests.rs b/pallets/automation-time/src/tests.rs index 96683e0fc..de3bf2937 100644 --- a/pallets/automation-time/src/tests.rs +++ b/pallets/automation-time/src/tests.rs @@ -17,9 +17,9 @@ use crate::{ mock::*, AccountTasks, Action, ActionOf, AssetPayment, Config, Error, InstructionSequence, - LastTimeSlot, MissedTaskV2Of, ScheduleParam, ScheduledTasksOf, TaskHashInput, TaskOf, - TaskQueueV2, WeightInfo, + LastTimeSlot, MissedTaskV2Of, ScheduleParam, ScheduledTasksOf, TaskOf, TaskQueueV2, WeightInfo, }; + use codec::Encode; use frame_support::{ assert_noop, assert_ok, @@ -43,6 +43,10 @@ pub const START_BLOCK_TIME: u64 = 33198768000 * 1_000; pub const SCHEDULED_TIME: u64 = START_BLOCK_TIME / 1_000 + 7200; const LAST_BLOCK_TIME: u64 = START_BLOCK_TIME / 1_000; +// This is 1-0-3: {1: block idx}-{0: first extrinsic in block}-{3: the event index} +const FIRST_TASK_ID: [u8; 5] = [49, 45, 48, 45, 51]; +const SECOND_TASK_ID: [u8; 5] = [49, 45, 48, 45, 54]; + const EXPECT_CALCULATE_SCHEDULE_FEE_AMOUNT: &str = "Calculate schedule fee amount should work"; const DEFAULT_SCHEDULE_FEE_LOCATION: MultiLocation = MOONBASE_ASSET_LOCATION; @@ -121,6 +125,20 @@ fn calculate_expected_xcmp_action_schedule_fee( expected_schedule_fee_amount } +// Helper function to asset event easiser +/// Assert the given `event` exists. +#[cfg(any(feature = "std", feature = "runtime-benchmarks", test))] +pub fn assert_has_event(event: RuntimeEvent) { + let evts = System::events().into_iter().map(|evt| evt.event).collect::>(); + assert!(evts.iter().any(|record| record == &event)) +} + +/// Assert the last event equal to the given `event`. +#[cfg(any(feature = "std", feature = "runtime-benchmarks", test))] +pub fn assert_last_event(event: RuntimeEvent) { + assert_eq!(events().last().expect("events expected"), &event); +} + // when schedule with a Fixed Time schedule and passing an epoch that isn't the // beginning of hour, raise an error // the smallest granularity unit we allow is hour @@ -133,7 +151,6 @@ fn schedule_invalid_time_fixed_schedule() { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], // Simulate epoch of 1 extra second at the beginning of this hour ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME + 1] }, Box::new(call) @@ -162,7 +179,6 @@ fn schedule_invalid_time_recurring_schedule() { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], ScheduleParam::Recurring { next_execution_time: *next_run, frequency: *frequency @@ -183,7 +199,6 @@ fn schedule_past_time() { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(frame_system::Call::remark { remark: vec![12] }.into()) ), @@ -193,7 +208,6 @@ fn schedule_past_time() { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME - 3600] }, Box::new(frame_system::Call::remark { remark: vec![12] }.into()) ), @@ -215,7 +229,6 @@ fn schedule_past_time_recurring() { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], ScheduleParam::Recurring { next_execution_time: *next_run, frequency: *frequency @@ -263,7 +276,6 @@ fn schedule_too_far_out() { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], task_far_schedule.clone(), Box::new(frame_system::Call::remark { remark: vec![12] }.into()) ), @@ -273,28 +285,12 @@ fn schedule_too_far_out() { }) } -#[test] -fn schedule_no_provided_id() { - new_test_ext(START_BLOCK_TIME).execute_with(|| { - assert_noop!( - AutomationTime::schedule_dynamic_dispatch_task( - RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![], - ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, - Box::new(frame_system::Call::remark { remark: vec![12] }.into()) - ), - Error::::EmptyProvidedId, - ); - }) -} - #[test] fn schedule_not_enough_for_fees() { new_test_ext(START_BLOCK_TIME).execute_with(|| { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![60], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(frame_system::Call::remark { remark: vec![12] }.into()) ), @@ -308,8 +304,7 @@ fn schedule_not_enough_for_fees() { fn schedule_transfer_with_dynamic_dispatch() { new_test_ext(START_BLOCK_TIME).execute_with(|| { 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_id = FIRST_TASK_ID.to_vec().clone(); fund_account(&account_id, 900_000_000, 2, Some(0)); @@ -318,7 +313,6 @@ fn schedule_transfer_with_dynamic_dispatch() { 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), )); @@ -350,7 +344,7 @@ fn schedule_transfer_with_dynamic_dispatch() { }), RuntimeEvent::AutomationTime(crate::Event::DynamicDispatchResult { who: account_id.clone(), - task_id, + task_id: task_id.clone(), result: Ok(()), }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { @@ -369,7 +363,7 @@ fn will_emit_task_completed_event_when_task_completed() { let frequency = 3_600; 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_id = FIRST_TASK_ID.to_vec().clone(); fund_account(&account_id, 900_000_000, 2, Some(0)); @@ -380,7 +374,6 @@ fn will_emit_task_completed_event_when_task_completed() { let next_execution_time = SCHEDULED_TIME + frequency; assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(account_id.clone()), - vec![1, 2], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME, next_execution_time] }, Box::new(call), )); @@ -415,12 +408,13 @@ fn will_emit_task_completed_event_when_task_completed() { } // The TaskCompleted event will not be emitted when the task is canceled. +#[test] fn will_not_emit_task_completed_event_when_task_canceled() { new_test_ext(START_BLOCK_TIME).execute_with(|| { let frequency = 3_600; 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_id = FIRST_TASK_ID.to_vec(); fund_account(&account_id, 900_000_000, 2, Some(0)); @@ -431,7 +425,6 @@ fn will_not_emit_task_completed_event_when_task_canceled() { let next_execution_time = SCHEDULED_TIME + frequency; assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(account_id.clone()), - vec![1, 2], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME, next_execution_time] }, Box::new(call), )); @@ -477,7 +470,7 @@ fn will_emit_task_completed_event_when_task_failed() { let frequency = 3_600; 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_id = FIRST_TASK_ID.to_vec(); fund_account(&account_id, 900_000_000, 2, Some(0)); let current_funds = Balances::free_balance(AccountId32::new(ALICE)); @@ -491,7 +484,6 @@ fn will_emit_task_completed_event_when_task_failed() { let next_execution_time = SCHEDULED_TIME + frequency; assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(account_id.clone()), - vec![1, 2], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME, next_execution_time] }, Box::new(call), )); @@ -781,7 +773,6 @@ fn schedule_xcmp_works() { assert_ok!(AutomationTime::schedule_xcmp_task( RuntimeOrigin::signed(alice.clone()), - vec![50], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(destination.into()), Box::new(NATIVE_LOCATION.into()), @@ -799,7 +790,6 @@ fn schedule_xcmp_works() { #[test] fn schedule_xcmp_through_proxy_works() { new_test_ext(START_BLOCK_TIME).execute_with(|| { - let provided_id = vec![50]; let destination = MultiLocation::new(1, X1(Parachain(PARA_ID.into()))); let delegator_account = AccountId32::new(DELEGATOR_ACCOUNT); let proxy_account = AccountId32::new(PROXY_ACCOUNT); @@ -810,7 +800,6 @@ fn schedule_xcmp_through_proxy_works() { assert_ok!(AutomationTime::schedule_xcmp_task_through_proxy( RuntimeOrigin::signed(proxy_account.clone()), - provided_id, ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(destination.clone().into()), Box::new(MultiLocation::default().into()), @@ -848,7 +837,6 @@ fn schedule_xcmp_through_proxy_works() { #[test] fn schedule_xcmp_through_proxy_same_as_delegator_account() { new_test_ext(START_BLOCK_TIME).execute_with(|| { - let provided_id = vec![50]; let delegator_account = AccountId32::new(ALICE); let call: Vec = vec![2, 4, 5]; let destination = MultiLocation::new(1, X1(Parachain(PARA_ID.into()))); @@ -859,7 +847,6 @@ fn schedule_xcmp_through_proxy_same_as_delegator_account() { assert_noop!( AutomationTime::schedule_xcmp_task_through_proxy( RuntimeOrigin::signed(delegator_account.clone()), - provided_id, ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(destination.clone().into()), Box::new(MultiLocation::default().into()), @@ -887,7 +874,6 @@ fn schedule_xcmp_fails_if_not_enough_funds() { assert_noop!( AutomationTime::schedule_xcmp_task( RuntimeOrigin::signed(alice.clone()), - vec![50], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(destination.into()), Box::new(NATIVE_LOCATION.into()), @@ -925,7 +911,7 @@ fn schedule_auto_compound_delegated_stake() { AutomationTime::get_account_task(account_task_id.0.clone(), account_task_id.1), TaskOf::::create_auto_compound_delegated_stake_task::( alice.clone(), - AutomationTime::generate_auto_compound_delegated_stake_provided_id(&alice, &bob), + FIRST_TASK_ID.to_vec().clone(), SCHEDULED_TIME, 3_600, bob, @@ -991,48 +977,69 @@ fn schedule_auto_compound_with_high_frequency() { }) } -// task id is a hash of account and provider_id. task id needs to be unique. -// the same provider_id cannot be submitted twice per account. -// verify that we're returning DuplicateTask when the same account submit duplicate -// provider id. #[test] -fn schedule_duplicates_errors() { +fn get_auto_compound_delegated_stake_task_ids_return_only_auto_compount_task_id() { + let owner = AccountId32::new(ALICE); + let delegator = AccountId32::new(BOB); + new_test_ext(START_BLOCK_TIME).execute_with(|| { - let call: RuntimeCall = frame_system::Call::remark { remark: vec![2, 4, 5] }.into(); - assert_ok!(fund_account_dynamic_dispatch( - &AccountId32::new(ALICE), - // only schedule one time in the schedule param below - 1, - call.encode() + get_funds(owner.clone()); + assert_ok!(AutomationTime::schedule_auto_compound_delegated_stake_task( + RuntimeOrigin::signed(owner.clone()), + SCHEDULED_TIME, + 3_600, + delegator.clone(), + 1_000_000_000, )); + let account_task_id = last_task_id(); + + fund_account(&owner, 900_000_000, 1, Some(0)); + let call: RuntimeCall = frame_system::Call::remark { remark: vec![2, 4, 5] }.into(); assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( - RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], + RuntimeOrigin::signed(owner.clone()), ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, - Box::new(frame_system::Call::remark { remark: vec![2, 4, 5] }.into()) - ),); - - // refund the test account again with same amount - assert_ok!(fund_account_dynamic_dispatch( - &AccountId32::new(ALICE), - // only schedule one time in the schedule param below - 1, - call.encode() + Box::new(call), )); - assert_noop!( - AutomationTime::schedule_dynamic_dispatch_task( - RuntimeOrigin::signed(AccountId32::new(ALICE)), - // repeat same id as above - vec![50], - ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, - Box::new(frame_system::Call::remark { remark: vec![2, 4, 5] }.into()) - ), - Error::::DuplicateTask, + let non_autocompound_task_id = last_task_id(); + + assert_ne!( + account_task_id, non_autocompound_task_id, + "autocompound task id and a normal task id should be different" + ); + + // Now when we get auto compount test, we should get only once + assert_eq!( + AutomationTime::get_auto_compound_delegated_stake_task_ids(owner), + vec![account_task_id], ); }) } +// test that we cannot schedule another task with the same id +// Because the ID is auto-generated now so to test this scenerio, we use the +// normal schedule call to schedule a task and call into a low level API +// to schedule a new task with that same last generated task id to observe +// the error +#[test] +fn schedule_duplicates_errors() { + new_test_ext(START_BLOCK_TIME).execute_with(|| { + let owner = AccountId32::new(ALICE); + get_funds(owner.clone()); + let task_id = schedule_task(ALICE, vec![SCHEDULED_TIME], vec![2, 4]); + + let task2 = TaskOf::::create_event_task::( + owner, + task_id, + vec![SCHEDULED_TIME], + vec![10, 12], + ) + .unwrap(); + + assert_noop!(AutomationTime::schedule_task(&task2), Error::::DuplicateTask,); + }) +} + // there is an upper limit of how many time slot in Fixed Scheduled, when // passing a large enough array we return TooManyExecutionsTimes error #[test] @@ -1048,7 +1055,6 @@ fn schedule_max_execution_times_errors() { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], ScheduleParam::Fixed { execution_times: vec![ SCHEDULED_TIME, @@ -1074,7 +1080,6 @@ fn schedule_execution_times_removes_dupes() { get_funds(owner.clone()); let task_id1 = schedule_task( ALICE, - vec![50], vec![ SCHEDULED_TIME, SCHEDULED_TIME, @@ -1084,6 +1089,7 @@ fn schedule_execution_times_removes_dupes() { ], vec![2, 4], ); + match AutomationTime::get_account_task(owner, task_id1) { None => { panic!("A task should exist if it was scheduled") @@ -1091,7 +1097,7 @@ fn schedule_execution_times_removes_dupes() { Some(task) => { let expected_task = TaskOf::::create_event_task::( AccountId32::new(ALICE), - vec![50], + vec![49, 45, 48, 45, 52], vec![SCHEDULED_TIME, SCHEDULED_TIME + 10800], vec![2, 4], ) @@ -1119,7 +1125,6 @@ fn schedule_time_slot_full() { assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![50], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(call1) )); @@ -1128,7 +1133,6 @@ fn schedule_time_slot_full() { assert_ok!(fund_account_dynamic_dispatch(&AccountId32::new(ALICE), 1, call2.encode())); assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![60], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(call2) )); @@ -1138,7 +1142,6 @@ fn schedule_time_slot_full() { assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![70], ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(call3) ), @@ -1167,18 +1170,17 @@ fn schedule_time_slot_full_rolls_back() { let call1: RuntimeCall = frame_system::Call::remark { remark: vec![2, 4, 5] }.into(); assert_ok!(fund_account_dynamic_dispatch(&AccountId32::new(ALICE), 1, call1.encode())); - let task_id1 = schedule_task(ALICE, vec![40], vec![SCHEDULED_TIME + 7200], vec![2, 4, 5]); + let task_id1 = schedule_task(ALICE, vec![SCHEDULED_TIME + 7200], vec![2, 4, 5]); let call2: RuntimeCall = frame_system::Call::remark { remark: vec![2, 4] }.into(); assert_ok!(fund_account_dynamic_dispatch(&AccountId32::new(ALICE), 1, call1.encode())); - let task_id2 = schedule_task(ALICE, vec![50], vec![SCHEDULED_TIME + 7200], vec![2, 4]); + let task_id2 = schedule_task(ALICE, vec![SCHEDULED_TIME + 7200], vec![2, 4]); let call: RuntimeCall = frame_system::Call::remark { remark: vec![2] }.into(); assert_ok!(fund_account_dynamic_dispatch(&AccountId32::new(ALICE), 1, call.encode())); assert_noop!( AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - vec![119], ScheduleParam::Fixed { execution_times: vec![ SCHEDULED_TIME, @@ -1203,31 +1205,163 @@ fn schedule_time_slot_full_rolls_back() { }, Some(ScheduledTasksOf:: { tasks: account_task_ids, .. }) => { assert_eq!(account_task_ids.len(), 2); - assert_eq!(account_task_ids[0].1, task_id1); - assert_eq!(account_task_ids[1].1, task_id2); + assert_eq!(account_task_ids[0].1, task_id1.clone()); + assert_eq!(account_task_ids[1].1, task_id2.clone()); }, } }) } +// verify that task scheduled in different block has the right id +#[test] +fn taskid_changed_per_block() { + new_test_ext(START_BLOCK_TIME).execute_with(|| { + let owner = AccountId32::new(ALICE); + let task_id1 = schedule_task( + ALICE, + vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], + vec![2, 4, 5], + ); + + System::set_block_number(20); + let task_id2 = schedule_task( + ALICE, + vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], + vec![2, 4, 5], + ); + LastTimeSlot::::put((SCHEDULED_TIME - 14400, SCHEDULED_TIME - 14400)); + + assert_eq!(task_id1, FIRST_TASK_ID.to_vec().clone()); + assert_eq!(task_id2, vec![50, 48, 45, 48, 45, 54]); + }) +} + +// verify that task scheduled in same block with different extrinsic index has different tx id +#[test] +fn taskid_adjusted_on_extrinsicid_on_same_block() { + new_test_ext(START_BLOCK_TIME).execute_with(|| { + let first_caller = AccountId32::new(ALICE); + let task_id1 = schedule_task( + ALICE, + vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], + vec![2, 4, 5], + ); + + // Set to a high and more than one digit extrinsic index to test task_id also match + System::set_extrinsic_index(234); + + let second_caller = AccountId32::new(BOB); + let task_id2 = schedule_task( + BOB, + vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], + vec![2, 4, 5], + ); + LastTimeSlot::::put((SCHEDULED_TIME - 14400, SCHEDULED_TIME - 14400)); + + assert_eq!(task_id1, FIRST_TASK_ID.to_vec().clone()); + assert_eq!(task_id2, vec![49, 45, 50, 51, 52, 45, 56]); + + assert_has_event(RuntimeEvent::AutomationTime(crate::Event::TaskScheduled { + who: first_caller, + task_id: FIRST_TASK_ID.to_vec().clone(), + schedule_as: None, + })); + + assert_has_event(RuntimeEvent::AutomationTime(crate::Event::TaskScheduled { + who: second_caller, + task_id: vec![49, 45, 50, 51, 52, 45, 56], + schedule_as: None, + })); + }) +} + +// verify that task scheduled in same block with different extrinsic index has different tx id +#[test] +fn taskid_adjusted_on_eventindex_on_same_block_from_same_caller() { + new_test_ext(START_BLOCK_TIME).execute_with(|| { + let caller = AccountId32::new(ALICE); + + let task_id1 = schedule_task( + ALICE, + vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], + vec![2, 4, 5], + ); + + // Set to a high and more than one digit extrinsic index to test task_id also match + System::set_extrinsic_index(234); + + let task_id2 = schedule_task( + ALICE, + vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], + vec![2, 4, 5], + ); + + // 1-0-3 + assert_eq!(task_id1, "1-0-3".as_bytes().to_vec()); + // 1-234-6 + assert_eq!(task_id2, "1-234-6".as_bytes().to_vec()); + + assert_has_event(RuntimeEvent::AutomationTime(crate::Event::TaskScheduled { + who: caller.clone(), + task_id: "1-0-3".as_bytes().to_vec(), + schedule_as: None, + })); + + assert_has_event(RuntimeEvent::AutomationTime(crate::Event::TaskScheduled { + who: caller.clone(), + task_id: "1-234-6".as_bytes().to_vec(), + schedule_as: None, + })); + }) +} + +// verify that task scheduled in same block with same extrinsic index has different event id +#[test] +fn taskid_on_same_extrinsid_have_unique_event_index() { + new_test_ext(START_BLOCK_TIME).execute_with(|| { + let owner = AccountId32::new(ALICE); + let task_id1 = schedule_task( + ALICE, + vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], + vec![2, 4, 5], + ); + + let task_id2 = schedule_task( + ALICE, + vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], + vec![2, 4, 5], + ); + LastTimeSlot::::put((SCHEDULED_TIME - 14400, SCHEDULED_TIME - 14400)); + + assert_eq!(task_id1, FIRST_TASK_ID.to_vec().clone()); + assert_eq!(task_id2, SECOND_TASK_ID.to_vec().clone()); + + assert_has_event(RuntimeEvent::AutomationTime(crate::Event::TaskScheduled { + who: owner, + task_id: FIRST_TASK_ID.to_vec().clone(), + schedule_as: None, + })); + }) +} + // verify that the owner of a task can cancel a Fixed schedule task by its id. // In this test we focus on confirmation of canceling the task that has a single // execution times #[test] fn cancel_works_for_fixed_scheduled() { new_test_ext(START_BLOCK_TIME).execute_with(|| { - let task_id1 = schedule_task(ALICE, vec![40], vec![SCHEDULED_TIME], vec![2, 4, 5]); - let task_id2 = schedule_task(ALICE, vec![50], vec![SCHEDULED_TIME], vec![2, 4]); + let task_id1 = schedule_task(ALICE, vec![SCHEDULED_TIME], vec![2, 4, 5]); + let task_id2 = schedule_task(ALICE, vec![SCHEDULED_TIME], vec![2, 4]); LastTimeSlot::::put((SCHEDULED_TIME - 14400, SCHEDULED_TIME - 14400)); System::reset_events(); assert_ok!(AutomationTime::cancel_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - task_id1, + task_id1.clone(), )); assert_ok!(AutomationTime::cancel_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - task_id2, + task_id2.clone(), )); if let Some(_) = AutomationTime::get_scheduled_tasks(SCHEDULED_TIME) { @@ -1238,11 +1372,11 @@ fn cancel_works_for_fixed_scheduled() { [ RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: AccountId32::new(ALICE), - task_id: task_id1 + task_id: task_id1.clone() }), RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: AccountId32::new(ALICE), - task_id: task_id2 + task_id: task_id2.clone(), }), ] ); @@ -1258,16 +1392,18 @@ fn cancel_works_for_multiple_executions_scheduled() { let owner = AccountId32::new(ALICE); let task_id1 = schedule_task( ALICE, - vec![40], vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], vec![2, 4, 5], ); LastTimeSlot::::put((SCHEDULED_TIME - 14400, SCHEDULED_TIME - 14400)); System::reset_events(); - assert_ok!(AutomationTime::cancel_task(RuntimeOrigin::signed(owner.clone()), task_id1,)); + assert_ok!(AutomationTime::cancel_task( + RuntimeOrigin::signed(owner.clone()), + task_id1.clone(), + )); - assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id1), None); + assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id1.clone()), None); if let Some(_) = AutomationTime::get_scheduled_tasks(SCHEDULED_TIME) { panic!("Tasks scheduled for the time it should have been deleted") } @@ -1281,7 +1417,7 @@ fn cancel_works_for_multiple_executions_scheduled() { events(), [RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: owner, - task_id: task_id1 + task_id: task_id1.clone(), })] ); }) @@ -1291,20 +1427,19 @@ fn cancel_works_for_multiple_executions_scheduled() { #[test] fn cancel_works_for_recurring_scheduled() { new_test_ext(START_BLOCK_TIME).execute_with(|| { - let task_id1 = - schedule_recurring_task(ALICE, vec![40], SCHEDULED_TIME, 3600, vec![2, 4, 5]); - let task_id2 = schedule_recurring_task(ALICE, vec![50], SCHEDULED_TIME, 3600, vec![2, 4]); + let task_id1 = schedule_recurring_task(ALICE, SCHEDULED_TIME, 3600, vec![2, 4, 5]); + let task_id2 = schedule_recurring_task(ALICE, SCHEDULED_TIME, 3600, vec![2, 4]); LastTimeSlot::::put((SCHEDULED_TIME - 14400, SCHEDULED_TIME - 14400)); System::reset_events(); assert_ok!(AutomationTime::cancel_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - task_id1, + task_id1.clone(), )); assert_ok!(AutomationTime::cancel_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - task_id2, + task_id2.clone(), )); if let Some(_) = AutomationTime::get_scheduled_tasks(SCHEDULED_TIME) { @@ -1315,11 +1450,11 @@ fn cancel_works_for_recurring_scheduled() { [ RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: AccountId32::new(ALICE), - task_id: task_id1 + task_id: task_id1.clone() }), RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: AccountId32::new(ALICE), - task_id: task_id2 + task_id: task_id2.clone() }), ] ); @@ -1333,13 +1468,12 @@ fn cancel_works_for_recurring_scheduled() { fn cancel_works_for_an_executed_task() { new_test_ext(START_BLOCK_TIME).execute_with(|| { let owner = AccountId32::new(ALICE); - let task_id1 = - schedule_task(ALICE, vec![50], vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600], vec![50]); + let task_id1 = schedule_task(ALICE, vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600], vec![50]); Timestamp::set_timestamp(SCHEDULED_TIME * 1_000); LastTimeSlot::::put((SCHEDULED_TIME - 3600, SCHEDULED_TIME - 3600)); System::reset_events(); - match AutomationTime::get_account_task(owner.clone(), task_id1) { + match AutomationTime::get_account_task(owner.clone(), task_id1.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -1354,7 +1488,7 @@ fn cancel_works_for_an_executed_task() { }, Some(ScheduledTasksOf:: { tasks: task_ids, .. }) => { assert_eq!(task_ids.len(), 1); - assert_eq!(task_ids[0].1, task_id1); + assert_eq!(task_ids[0].1, task_id1.clone()); }, } match AutomationTime::get_scheduled_tasks(SCHEDULED_TIME + 3600) { @@ -1363,7 +1497,7 @@ fn cancel_works_for_an_executed_task() { }, Some(ScheduledTasksOf:: { tasks: task_ids, .. }) => { assert_eq!(task_ids.len(), 1); - assert_eq!(task_ids[0].1, task_id1); + assert_eq!(task_ids[0].1, task_id1.clone()); }, } @@ -1380,12 +1514,12 @@ fn cancel_works_for_an_executed_task() { }), RuntimeEvent::AutomationTime(crate::Event::DynamicDispatchResult { who: owner.clone(), - task_id: task_id1, + task_id: task_id1.clone(), result: Ok(()), }), ] ); - match AutomationTime::get_account_task(owner.clone(), task_id1) { + match AutomationTime::get_account_task(owner.clone(), task_id1.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -1401,24 +1535,24 @@ fn cancel_works_for_an_executed_task() { }, Some(ScheduledTasksOf:: { tasks: task_ids, .. }) => { assert_eq!(task_ids.len(), 1); - assert_eq!(task_ids[0].1, task_id1); + assert_eq!(task_ids[0].1, task_id1.clone()); }, } assert_ok!(AutomationTime::cancel_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - task_id1 + task_id1.clone() )); assert_eq!(AutomationTime::get_scheduled_tasks(SCHEDULED_TIME), None); assert_eq!(AutomationTime::get_scheduled_tasks(SCHEDULED_TIME + 3600), None); - assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id1), None); + assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id1.clone()), None); assert_eq!( events(), [RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: owner, - task_id: task_id1, + task_id: task_id1.clone(), })] ); }) @@ -1437,19 +1571,19 @@ fn cancel_works_for_tasks_in_queue() { ); LastTimeSlot::::put((SCHEDULED_TIME, SCHEDULED_TIME)); - assert_eq!(task_id, AutomationTime::get_task_queue()[0].1); + assert_eq!(task_id.clone(), AutomationTime::get_task_queue()[0].1); assert_eq!(1, AutomationTime::get_task_queue().len()); assert_ok!(AutomationTime::cancel_task( RuntimeOrigin::signed(AccountId32::new(ALICE)), - task_id, + task_id.clone(), )); assert_eq!( events(), [RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: AccountId32::new(ALICE), - task_id + task_id: task_id.clone() }),] ); assert_eq!(0, AutomationTime::get_task_queue().len()); @@ -1467,7 +1601,8 @@ fn cancel_task_must_exist() { vec![2, 4, 5], ) .unwrap(); - let task_id = BlakeTwo256::hash_of(&task); + //let task_id = BlakeTwo256::hash_of(&task); + let task_id = vec![49, 45, 48, 45, 52]; assert_noop!( AutomationTime::cancel_task(RuntimeOrigin::signed(AccountId32::new(ALICE)), task_id), @@ -1492,24 +1627,33 @@ fn cancel_task_not_found() { vec![2, 4, 5], ) .unwrap(); - let task_id = BlakeTwo256::hash_of(&task); - AccountTasks::::insert(owner.clone(), task_id, task); + let task_id = vec![49, 45, 48, 45, 49]; + AccountTasks::::insert(owner.clone(), task_id.clone(), task); - assert_ok!(AutomationTime::cancel_task(RuntimeOrigin::signed(owner.clone()), task_id,)); + assert_ok!(AutomationTime::cancel_task( + RuntimeOrigin::signed(owner.clone()), + task_id.clone(), + )); assert_eq!( events(), [ RuntimeEvent::AutomationTime(crate::Event::TaskNotFound { who: owner.clone(), - task_id + task_id: task_id.clone(), }), - RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: owner, task_id }) + RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { + who: owner, + task_id: task_id.clone() + }) ] ); // now ensure the task id is also removed from AccountTasks assert_noop!( - AutomationTime::cancel_task(RuntimeOrigin::signed(AccountId32::new(ALICE)), task_id), + AutomationTime::cancel_task( + RuntimeOrigin::signed(AccountId32::new(ALICE)), + task_id.clone() + ), Error::::TaskDoesNotExist, ); }) @@ -1522,42 +1666,48 @@ fn cancel_task_fail_non_owner() { let owner = AccountId32::new(ALICE); let task_id1 = schedule_task( ALICE, - vec![40], vec![SCHEDULED_TIME, SCHEDULED_TIME + 3600, SCHEDULED_TIME + 7200], vec![2, 4, 5], ); LastTimeSlot::::put((SCHEDULED_TIME - 14400, SCHEDULED_TIME - 14400)); + System::reset_events(); // BOB cannot cancel because he isn't the task owner assert_noop!( - AutomationTime::cancel_task(RuntimeOrigin::signed(AccountId32::new(BOB)), task_id1), + AutomationTime::cancel_task( + RuntimeOrigin::signed(AccountId32::new(BOB)), + task_id1.clone() + ), Error::::TaskDoesNotExist, ); // But Alice can cancel as expected - assert_ok!(AutomationTime::cancel_task(RuntimeOrigin::signed(owner.clone()), task_id1,)); + assert_ok!(AutomationTime::cancel_task( + RuntimeOrigin::signed(owner.clone()), + task_id1.clone(), + )); }) } // verifying that root/sudo can force_cancel anybody's tasks -// #[test] +#[test] fn force_cancel_task_works() { new_test_ext(START_BLOCK_TIME).execute_with(|| { - let task_id = schedule_task(ALICE, vec![40], vec![SCHEDULED_TIME], vec![2, 4, 5]); + let task_id = schedule_task(ALICE, vec![SCHEDULED_TIME], vec![2, 4, 5]); LastTimeSlot::::put((SCHEDULED_TIME - 14400, SCHEDULED_TIME - 14400)); System::reset_events(); assert_ok!(AutomationTime::force_cancel_task( RawOrigin::Root.into(), AccountId32::new(ALICE), - task_id + task_id.clone() )); assert_eq!( events(), [RuntimeEvent::AutomationTime(crate::Event::TaskCancelled { who: AccountId32::new(ALICE), - task_id + task_id: task_id.clone() }),] ); }) @@ -1574,9 +1724,6 @@ mod extrinsics { new_test_ext(START_BLOCK_TIME).execute_with(|| { let account_id = AccountId32::new(ALICE); let execution_times = vec![SCHEDULED_TIME]; - let provided_id = vec![0]; - let task_id = - AutomationTime::generate_task_id(account_id.clone(), provided_id.clone()); let call: RuntimeCall = frame_system::Call::remark { remark: vec![] }.into(); assert_ok!(fund_account_dynamic_dispatch( &account_id, @@ -1586,7 +1733,6 @@ mod extrinsics { assert_ok!(AutomationTime::schedule_dynamic_dispatch_task( RuntimeOrigin::signed(account_id.clone()), - provided_id, ScheduleParam::Fixed { execution_times: vec![SCHEDULED_TIME] }, Box::new(call) )); @@ -1594,7 +1740,7 @@ mod extrinsics { last_event(), RuntimeEvent::AutomationTime(crate::Event::TaskScheduled { who: account_id, - task_id, + task_id: FIRST_TASK_ID.to_vec().clone(), schedule_as: None, }) ); @@ -1611,13 +1757,13 @@ mod run_dynamic_dispatch_action { fn cannot_decode() { new_test_ext(START_BLOCK_TIME).execute_with(|| { let account_id = AccountId32::new(ALICE); - let task_id = AutomationTime::generate_task_id(account_id.clone(), vec![1]); + let task_id = vec![49, 45, 48, 45, 52]; let bad_encoded_call: Vec = vec![1]; let (weight, _) = AutomationTime::run_dynamic_dispatch_action( account_id.clone(), bad_encoded_call, - task_id, + task_id.clone(), ); assert_eq!( @@ -1628,7 +1774,7 @@ mod run_dynamic_dispatch_action { events(), [RuntimeEvent::AutomationTime(crate::Event::CallCannotBeDecoded { who: account_id, - task_id, + task_id: task_id.clone(), }),] ); }) @@ -1638,17 +1784,21 @@ mod run_dynamic_dispatch_action { fn call_errors() { new_test_ext(START_BLOCK_TIME).execute_with(|| { let account_id = AccountId32::new(ALICE); - let task_id = AutomationTime::generate_task_id(account_id.clone(), vec![1]); + let task_id = vec![49, 45, 48, 45, 52]; let call: RuntimeCall = frame_system::Call::set_code { code: vec![] }.into(); let encoded_call = call.encode(); - AutomationTime::run_dynamic_dispatch_action(account_id.clone(), encoded_call, task_id); + AutomationTime::run_dynamic_dispatch_action( + account_id.clone(), + encoded_call, + task_id.clone(), + ); assert_eq!( events(), [RuntimeEvent::AutomationTime(crate::Event::DynamicDispatchResult { who: account_id, - task_id, + task_id: task_id.clone(), result: Err(DispatchError::BadOrigin), }),] ); @@ -1659,17 +1809,21 @@ mod run_dynamic_dispatch_action { fn call_filtered() { new_test_ext(START_BLOCK_TIME).execute_with(|| { let account_id = AccountId32::new(ALICE); - let task_id = AutomationTime::generate_task_id(account_id.clone(), vec![1]); + let task_id = FIRST_TASK_ID.to_vec(); let call: RuntimeCall = pallet_timestamp::Call::set { now: 100 }.into(); let encoded_call = call.encode(); - AutomationTime::run_dynamic_dispatch_action(account_id.clone(), encoded_call, task_id); + AutomationTime::run_dynamic_dispatch_action( + account_id.clone(), + encoded_call, + task_id.clone(), + ); assert_eq!( events(), [RuntimeEvent::AutomationTime(crate::Event::DynamicDispatchResult { who: account_id, - task_id, + task_id: task_id.clone(), result: Err(DispatchError::from(frame_system::Error::::CallFiltered)), }),] ); @@ -1680,17 +1834,21 @@ mod run_dynamic_dispatch_action { fn call_works() { new_test_ext(START_BLOCK_TIME).execute_with(|| { let account_id = AccountId32::new(ALICE); - let task_id = AutomationTime::generate_task_id(account_id.clone(), vec![1]); + let task_id = FIRST_TASK_ID.to_vec(); let call: RuntimeCall = frame_system::Call::remark { remark: vec![] }.into(); let encoded_call = call.encode(); - AutomationTime::run_dynamic_dispatch_action(account_id.clone(), encoded_call, task_id); + AutomationTime::run_dynamic_dispatch_action( + account_id.clone(), + encoded_call, + task_id.clone(), + ); assert_eq!( events(), [RuntimeEvent::AutomationTime(crate::Event::DynamicDispatchResult { who: account_id, - task_id, + task_id: task_id.clone(), result: Ok(()), }),] ); @@ -1750,7 +1908,7 @@ fn trigger_tasks_updates_queues() { SCHEDULED_TIME - 3600, ); assert_eq!(AutomationTime::get_missed_queue().len(), 0); - let scheduled_task_id = schedule_task(ALICE, vec![50], vec![SCHEDULED_TIME], vec![50]); + let scheduled_task_id = schedule_task(ALICE, vec![SCHEDULED_TIME], vec![50]); Timestamp::set_timestamp(SCHEDULED_TIME * 1_000); LastTimeSlot::::put((SCHEDULED_TIME - 3600, SCHEDULED_TIME - 3600)); System::reset_events(); @@ -1786,15 +1944,15 @@ fn trigger_tasks_handles_missed_slots() { assert_eq!(AutomationTime::get_missed_queue().len(), 0); - let missed_task_id = schedule_task(ALICE, vec![50], vec![SCHEDULED_TIME - 3600], vec![50]); + let missed_task_id = schedule_task(ALICE, vec![SCHEDULED_TIME - 3600], vec![50]); let missed_task = MissedTaskV2Of::::new( AccountId32::new(ALICE), missed_task_id, SCHEDULED_TIME - 3600, ); - let task_will_be_run_id = schedule_task(ALICE, vec![59], vec![SCHEDULED_TIME], vec![50]); - let scheduled_task_id = schedule_task(ALICE, vec![60], vec![SCHEDULED_TIME], vec![50]); + let task_will_be_run_id = schedule_task(ALICE, vec![SCHEDULED_TIME], vec![50]); + let scheduled_task_id = schedule_task(ALICE, vec![SCHEDULED_TIME], vec![50]); Timestamp::set_timestamp(SCHEDULED_TIME * 1_000); LastTimeSlot::::put((SCHEDULED_TIME - 7200, SCHEDULED_TIME - 7200)); @@ -1819,12 +1977,12 @@ fn trigger_tasks_handles_missed_slots() { }), RuntimeEvent::AutomationTime(crate::Event::DynamicDispatchResult { who: AccountId32::new(ALICE), - task_id: task_will_be_run_id, + task_id: task_will_be_run_id.clone(), result: Ok(()), }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: AccountId32::new(ALICE), - task_id: task_will_be_run_id, + task_id: task_will_be_run_id.clone(), }), ], ); @@ -1859,19 +2017,14 @@ fn trigger_tasks_limits_missed_slots() { assert_eq!(AutomationTime::get_missed_queue().len(), 0); Timestamp::set_timestamp((SCHEDULED_TIME - 25200) * 1_000); - let missing_task_id1 = - schedule_task(ALICE, vec![50], vec![SCHEDULED_TIME - 3600], vec![50]); + let missing_task_id1 = schedule_task(ALICE, vec![SCHEDULED_TIME - 3600], vec![50]); - let missing_task_id2 = - schedule_task(ALICE, vec![60], vec![SCHEDULED_TIME - 7200], vec![50]); - let missing_task_id3 = - schedule_task(ALICE, vec![70], vec![SCHEDULED_TIME - 10800], vec![50]); - let missing_task_id4 = - schedule_task(ALICE, vec![80], vec![SCHEDULED_TIME - 14400], vec![50]); - let missing_task_id5 = - schedule_task(ALICE, vec![90], vec![SCHEDULED_TIME - 18000], vec![50]); + let missing_task_id2 = schedule_task(ALICE, vec![SCHEDULED_TIME - 7200], vec![50]); + let missing_task_id3 = schedule_task(ALICE, vec![SCHEDULED_TIME - 10800], vec![50]); + let missing_task_id4 = schedule_task(ALICE, vec![SCHEDULED_TIME - 14400], vec![50]); + let missing_task_id5 = schedule_task(ALICE, vec![SCHEDULED_TIME - 18000], vec![50]); - let task_id = schedule_task(ALICE, vec![100], vec![SCHEDULED_TIME], vec![32]); + let task_id = schedule_task(ALICE, vec![SCHEDULED_TIME], vec![32]); Timestamp::set_timestamp(SCHEDULED_TIME * 1_000); LastTimeSlot::::put((SCHEDULED_TIME - 25200, SCHEDULED_TIME - 25200)); @@ -1897,7 +2050,7 @@ fn trigger_tasks_limits_missed_slots() { }), RuntimeEvent::AutomationTime(crate::Event::DynamicDispatchResult { who: owner.clone(), - task_id, + task_id: task_id.clone(), result: Ok(()), }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { @@ -1906,7 +2059,7 @@ fn trigger_tasks_limits_missed_slots() { }), RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: owner.clone(), - task_id: missing_task_id0, + task_id: missing_task_id0.clone(), execution_time: SCHEDULED_TIME - 25200, }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { @@ -1915,7 +2068,7 @@ fn trigger_tasks_limits_missed_slots() { }), RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: owner.clone(), - task_id: missing_task_id5, + task_id: missing_task_id5.clone(), execution_time: SCHEDULED_TIME - 18000, }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { @@ -1924,7 +2077,7 @@ fn trigger_tasks_limits_missed_slots() { }), RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: owner.clone(), - task_id: missing_task_id4, + task_id: missing_task_id4.clone(), execution_time: SCHEDULED_TIME - 14400, }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { @@ -1933,7 +2086,7 @@ fn trigger_tasks_limits_missed_slots() { }), RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: owner.clone(), - task_id: missing_task_id3, + task_id: missing_task_id3.clone(), execution_time: SCHEDULED_TIME - 10800, }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { @@ -1995,12 +2148,12 @@ fn trigger_tasks_completes_all_tasks() { RuntimeEvent::AutomationTime(crate::Event::Notify { message: message_one.clone() }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: AccountId32::new(ALICE), - task_id: task_id1, + task_id: task_id1.clone(), }), RuntimeEvent::AutomationTime(crate::Event::Notify { message: message_two.clone() }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: AccountId32::new(ALICE), - task_id: task_id2, + task_id: task_id2.clone(), }), ] ); @@ -2014,10 +2167,9 @@ fn trigger_tasks_completes_all_tasks() { fn trigger_tasks_handles_nonexisting_tasks() { new_test_ext(START_BLOCK_TIME).execute_with(|| { let owner = AccountId32::new(ALICE); - let task_hash_input = TaskHashInput::new(owner.clone(), vec![20]); - let bad_task_id = BlakeTwo256::hash_of(&task_hash_input); + let bad_task_id = vec![1, 2, 3]; let mut task_queue = AutomationTime::get_task_queue(); - task_queue.push((owner.clone(), bad_task_id)); + task_queue.push((owner.clone(), bad_task_id.clone())); TaskQueueV2::::put(task_queue); LastTimeSlot::::put((LAST_BLOCK_TIME, LAST_BLOCK_TIME)); @@ -2027,7 +2179,7 @@ fn trigger_tasks_handles_nonexisting_tasks() { events(), [RuntimeEvent::AutomationTime(crate::Event::TaskNotFound { who: owner, - task_id: bad_task_id + task_id: bad_task_id.clone() }),] ); assert_eq!(0, AutomationTime::get_task_queue().len()); @@ -2061,7 +2213,7 @@ fn trigger_tasks_completes_some_tasks() { RuntimeEvent::AutomationTime(crate::Event::Notify { message: message_one.clone() }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: AccountId32::new(ALICE), - task_id: task_id1, + task_id: task_id1.clone(), }), ] ); @@ -2098,28 +2250,34 @@ fn trigger_tasks_completes_all_missed_tasks() { [ RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: owner.clone(), - task_id: task_id1, + task_id: task_id1.clone(), execution_time: SCHEDULED_TIME }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: task_id1, + task_id: task_id1.clone(), }), RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: owner.clone(), - task_id: task_id2, + task_id: task_id2.clone(), execution_time: SCHEDULED_TIME }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner, - task_id: task_id2, + task_id: task_id2.clone(), }), ] ); assert_eq!(AutomationTime::get_missed_queue().len(), 0); - assert_eq!(AutomationTime::get_account_task(AccountId32::new(ALICE), task_id1), None); - assert_eq!(AutomationTime::get_account_task(AccountId32::new(ALICE), task_id2), None); + assert_eq!( + AutomationTime::get_account_task(AccountId32::new(ALICE), task_id1.clone()), + None + ); + assert_eq!( + AutomationTime::get_account_task(AccountId32::new(ALICE), task_id2.clone()), + None + ); }) } @@ -2140,7 +2298,7 @@ fn missed_tasks_updates_executions_left() { Action::Notify { message: vec![40] }, ); - match AutomationTime::get_account_task(owner.clone(), task_id1) { + match AutomationTime::get_account_task(owner.clone(), task_id1.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -2148,7 +2306,7 @@ fn missed_tasks_updates_executions_left() { assert_eq!(task.schedule.known_executions_left(), 2); }, } - match AutomationTime::get_account_task(owner.clone(), task_id2) { + match AutomationTime::get_account_task(owner.clone(), task_id2.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -2165,19 +2323,19 @@ fn missed_tasks_updates_executions_left() { [ RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: AccountId32::new(ALICE), - task_id: task_id1, + task_id: task_id1.clone(), execution_time: SCHEDULED_TIME }), RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: AccountId32::new(ALICE), - task_id: task_id2, + task_id: task_id2.clone(), execution_time: SCHEDULED_TIME }), ] ); assert_eq!(AutomationTime::get_missed_queue().len(), 0); - match AutomationTime::get_account_task(owner.clone(), task_id1) { + match AutomationTime::get_account_task(owner.clone(), task_id1.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -2185,7 +2343,7 @@ fn missed_tasks_updates_executions_left() { assert_eq!(task.schedule.known_executions_left(), 1); }, } - match AutomationTime::get_account_task(owner.clone(), task_id2) { + match AutomationTime::get_account_task(owner.clone(), task_id2.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -2209,12 +2367,12 @@ fn missed_tasks_removes_completed_tasks() { ); let mut task_queue = AutomationTime::get_task_queue(); - task_queue.push((owner.clone(), task_id01)); + task_queue.push((owner.clone(), task_id01.clone())); TaskQueueV2::::put(task_queue); assert_eq!(AutomationTime::get_missed_queue().len(), 1); assert_eq!(AutomationTime::get_task_queue().len(), 1); - match AutomationTime::get_account_task(owner.clone(), task_id01) { + match AutomationTime::get_account_task(owner.clone(), task_id01.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -2235,16 +2393,16 @@ fn missed_tasks_removes_completed_tasks() { RuntimeEvent::AutomationTime(crate::Event::Notify { message: message_one }), RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: AccountId32::new(ALICE), - task_id: task_id01, + task_id: task_id01.clone(), execution_time: SCHEDULED_TIME }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: AccountId32::new(ALICE), - task_id: task_id01, + task_id: task_id01.clone(), }), ] ); - assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id01), None); + assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id01.clone()), None); }) } @@ -2322,7 +2480,7 @@ fn trigger_tasks_completes_some_xcmp_tasks() { [ RuntimeEvent::AutomationTime(crate::Event::XcmpTaskSucceeded { destination, - task_id, + task_id: task_id.clone(), }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: AccountId32::new(ALICE), @@ -2419,9 +2577,9 @@ fn auto_compound_delegated_stake_reschedules_and_reruns() { .expect("Task should have been rescheduled") .tasks .into_iter() - .find(|t| *t == (AccountId32::new(ALICE), task_id)) + .find(|t| *t == (AccountId32::new(ALICE), task_id.clone())) .expect("Task should have been rescheduled"); - let task = AutomationTime::get_account_task(AccountId32::new(ALICE), task_id) + let task = AutomationTime::get_account_task(AccountId32::new(ALICE), task_id.clone()) .expect("Task should not have been removed from task map"); assert_eq!(task.schedule.known_executions_left(), 1); assert_eq!(task.execution_times(), vec![next_scheduled_time]); @@ -2516,10 +2674,12 @@ fn auto_compound_delegated_stake_does_not_reschedule_on_failure() { .expect("AutoCompound failure event should have been emitted"); assert!(AutomationTime::get_scheduled_tasks(SCHEDULED_TIME + frequency) .filter(|scheduled| { - scheduled.tasks.iter().any(|t| *t == (AccountId32::new(ALICE), task_id)) + scheduled.tasks.iter().any(|t| *t == (AccountId32::new(ALICE), task_id.clone())) }) .is_none()); - assert!(AutomationTime::get_account_task(AccountId32::new(ALICE), task_id).is_none()); + assert!( + AutomationTime::get_account_task(AccountId32::new(ALICE), task_id.clone()).is_none() + ); }) } @@ -2537,7 +2697,7 @@ fn trigger_tasks_updates_executions_left() { Action::Notify { message: message_one.clone() }, ); - match AutomationTime::get_account_task(owner.clone(), task_id01) { + match AutomationTime::get_account_task(owner.clone(), task_id01.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -2555,7 +2715,7 @@ fn trigger_tasks_updates_executions_left() { events(), [RuntimeEvent::AutomationTime(crate::Event::Notify { message: message_one.clone() }),] ); - match AutomationTime::get_account_task(owner.clone(), task_id01) { + match AutomationTime::get_account_task(owner.clone(), task_id01.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -2578,7 +2738,7 @@ fn trigger_tasks_removes_completed_tasks() { Action::Notify { message: message_one.clone() }, ); - match AutomationTime::get_account_task(owner.clone(), task_id01) { + match AutomationTime::get_account_task(owner.clone(), task_id01.clone()) { None => { panic!("A task should exist if it was scheduled") }, @@ -2598,7 +2758,7 @@ fn trigger_tasks_removes_completed_tasks() { RuntimeEvent::AutomationTime(crate::Event::Notify { message: message_one.clone() }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: task_id01, + task_id: task_id01.clone(), }), ] ); @@ -2639,18 +2799,18 @@ fn on_init_runs_tasks() { RuntimeEvent::AutomationTime(crate::Event::Notify { message: message_one.clone() }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: task_id1, + task_id: task_id1.clone(), }), RuntimeEvent::AutomationTime(crate::Event::Notify { message: message_two.clone() }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: task_id2, + task_id: task_id2.clone(), }), ] ); - assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id1), None); - assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id2), None); - assert_ne!(AutomationTime::get_account_task(owner.clone(), task_id3), None); + assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id1.clone()), None); + assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id2.clone()), None); + assert_ne!(AutomationTime::get_account_task(owner.clone(), task_id3.clone()), None); assert_eq!(AutomationTime::get_task_queue().len(), 1); assert_eq!(AutomationTime::get_missed_queue().len(), 0); @@ -2661,16 +2821,16 @@ fn on_init_runs_tasks() { [ RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: AccountId32::new(ALICE), - task_id: task_id3, + task_id: task_id3.clone(), execution_time: LAST_BLOCK_TIME }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: task_id3, + task_id: task_id3.clone(), }), ], ); - assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id3), None); + assert_eq!(AutomationTime::get_account_task(owner.clone(), task_id3.clone()), None); assert_eq!(AutomationTime::get_task_queue().len(), 0); assert_eq!(AutomationTime::get_missed_queue().len(), 0); }) @@ -2692,7 +2852,7 @@ fn on_init_check_task_queue() { vec![SCHEDULED_TIME], Action::Notify { message: vec![i] }, ); - tasks.push(task_id); + tasks.push(task_id.clone()); } Timestamp::set_timestamp(START_BLOCK_TIME + (10 * 1000)); AutomationTime::on_initialize(1); @@ -2705,12 +2865,12 @@ fn on_init_check_task_queue() { RuntimeEvent::AutomationTime(crate::Event::Notify { message: vec![0] }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: tasks[0], + task_id: tasks[0].clone(), }), RuntimeEvent::AutomationTime(crate::Event::Notify { message: vec![1] }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: tasks[1], + task_id: tasks[1].clone(), }), ], ); @@ -2725,12 +2885,12 @@ fn on_init_check_task_queue() { RuntimeEvent::AutomationTime(crate::Event::Notify { message: vec![2] }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: tasks[2], + task_id: tasks[2].clone(), }), RuntimeEvent::AutomationTime(crate::Event::Notify { message: vec![3] }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner.clone(), - task_id: tasks[3], + task_id: tasks[3].clone(), }), ], ); @@ -2744,12 +2904,12 @@ fn on_init_check_task_queue() { [ RuntimeEvent::AutomationTime(crate::Event::TaskMissed { who: AccountId32::new(ALICE), - task_id: tasks[4], + task_id: tasks[4].clone(), execution_time: LAST_BLOCK_TIME }), RuntimeEvent::AutomationTime(crate::Event::TaskCompleted { who: owner, - task_id: tasks[4], + task_id: tasks[4].clone(), }), ], ); @@ -2791,9 +2951,9 @@ fn on_init_shutdown() { Timestamp::set_timestamp(START_BLOCK_TIME + (3600 * 1_000)); AutomationTime::on_initialize(2); assert_eq!(events(), [],); - assert_ne!(AutomationTime::get_account_task(owner.clone(), task_id1), None); - assert_ne!(AutomationTime::get_account_task(owner.clone(), task_id2), None); - assert_ne!(AutomationTime::get_account_task(owner.clone(), task_id3), None); + assert_ne!(AutomationTime::get_account_task(owner.clone(), task_id1.clone()), None); + assert_ne!(AutomationTime::get_account_task(owner.clone(), task_id2.clone()), None); + assert_ne!(AutomationTime::get_account_task(owner.clone(), task_id3.clone()), None); assert_eq!(AutomationTime::get_task_queue().len(), 3); assert_eq!(AutomationTime::get_missed_queue().len(), 0); }) diff --git a/pallets/automation-time/src/types.rs b/pallets/automation-time/src/types.rs index 9dda6067e..166d9dbcf 100644 --- a/pallets/automation-time/src/types.rs +++ b/pallets/automation-time/src/types.rs @@ -210,7 +210,7 @@ impl Schedule { #[scale_info(skip_type_params(MaxExecutionTimes))] pub struct Task { pub owner_id: AccountId, - pub provided_id: Vec, + pub task_id: Vec, pub schedule: Schedule, pub action: Action, } @@ -218,7 +218,7 @@ pub struct Task { impl PartialEq for Task { fn eq(&self, other: &Self) -> bool { self.owner_id == other.owner_id && - self.provided_id == other.provided_id && + self.task_id == other.task_id && self.action == other.action && self.schedule == other.schedule } @@ -229,16 +229,16 @@ impl Eq for Task {} impl Task { pub fn new( owner_id: AccountId, - provided_id: Vec, + task_id: Vec, schedule: Schedule, action: Action, ) -> Self { - Self { owner_id, provided_id, schedule, action } + Self { owner_id, task_id, schedule, action } } pub fn create_event_task( owner_id: AccountId, - provided_id: Vec, + task_id: Vec, execution_times: Vec, message: Vec, ) -> Result { @@ -246,12 +246,12 @@ impl Task { frame_system::Call::remark_with_event { remark: message }.into(); let action = Action::DynamicDispatch { encoded_call: call.encode() }; let schedule = Schedule::new_fixed_schedule::(execution_times)?; - Ok(Self::new(owner_id, provided_id, schedule, action)) + Ok(Self::new(owner_id, task_id, schedule, action)) } pub fn create_xcmp_task( owner_id: AccountId, - provided_id: Vec, + task_id: Vec, execution_times: Vec, destination: MultiLocation, schedule_fee: MultiLocation, @@ -274,12 +274,12 @@ impl Task { instruction_sequence, }; let schedule = Schedule::new_fixed_schedule::(execution_times)?; - Ok(Self::new(owner_id, provided_id, schedule, action)) + Ok(Self::new(owner_id, task_id, schedule, action)) } pub fn create_auto_compound_delegated_stake_task( owner_id: AccountId, - provided_id: Vec, + task_id: Vec, next_execution_time: UnixTime, frequency: Seconds, collator_id: AccountId, @@ -291,7 +291,7 @@ impl Task { account_minimum, }; let schedule = Schedule::new_recurring_schedule::(next_execution_time, frequency)?; - Ok(Self::new(owner_id, provided_id, schedule, action)) + Ok(Self::new(owner_id, task_id, schedule, action)) } pub fn execution_times(&self) -> Vec { @@ -317,18 +317,6 @@ impl MissedTaskV2 { } } -#[derive(Debug, Encode, Decode, TypeInfo)] -pub struct TaskHashInput { - owner_id: AccountId, - provided_id: Vec, -} - -impl TaskHashInput { - pub fn new(owner_id: AccountId, provided_id: Vec) -> Self { - Self { owner_id, provided_id } - } -} - #[derive(Debug, Decode, Eq, Encode, PartialEq, TypeInfo)] #[scale_info(skip_type_params(T))] pub struct ScheduledTasks { @@ -378,7 +366,7 @@ mod tests { mod scheduled_tasks { use super::*; - use crate::{AccountTaskId, BalanceOf, ScheduledTasksOf, TaskId, TaskOf}; + use crate::{AccountTaskId, BalanceOf, ScheduledTasksOf, TaskOf}; use sp_runtime::AccountId32; #[test] @@ -391,10 +379,10 @@ mod tests { vec![0], ) .unwrap(); - let task_id = TaskId::::default(); + let task_id = vec![48, 45, 48, 45, 48]; assert_err!( ScheduledTasksOf:: { tasks: vec![], weight: MaxWeightPerSlot::get() } - .try_push::>(task_id, &task), + .try_push::>(task_id.clone(), &task), Error::::TimeSlotFull ); }) @@ -404,7 +392,8 @@ mod tests { fn try_push_errors_when_slot_is_full_by_task_count() { new_test_ext(START_BLOCK_TIME).execute_with(|| { let alice = AccountId32::new(ALICE); - let id = (alice.clone(), TaskId::::default()); + let id = (alice.clone(), vec![49, 45, 48, 45, 42]); + let task = TaskOf::::create_event_task::( alice.clone(), vec![0], @@ -419,7 +408,7 @@ mod tests { tasks }, ); - let task_id = TaskId::::default(); + let task_id = vec![48, 45, 48, 45, 48]; assert_err!( ScheduledTasksOf:: { tasks, weight: 0 } .try_push::>(task_id, &task), @@ -446,10 +435,13 @@ mod tests { vec![0], ) .unwrap(); - let task_id = TaskId::::default(); + // When we schedule a test on the first block, on the first extrinsics and no event + // at all this is the first task id we generate + // {block-num}-{extrinsics-idx}-{evt-idx} + let task_id = "0-1-0".as_bytes().to_vec(); let mut scheduled_tasks = ScheduledTasksOf::::default(); scheduled_tasks - .try_push::>(task_id, &task) + .try_push::>(task_id.clone(), &task) .expect("slot is not full"); assert_eq!(scheduled_tasks.tasks, vec![(task.owner_id, task_id)]); diff --git a/runtime/neumann/src/lib.rs b/runtime/neumann/src/lib.rs index 57ff379d9..7250131a4 100644 --- a/runtime/neumann/src/lib.rs +++ b/runtime/neumann/src/lib.rs @@ -1135,10 +1135,6 @@ impl_runtime_apis! { } impl pallet_automation_time_rpc_runtime_api::AutomationTimeApi for Runtime { - fn generate_task_id(account_id: AccountId, provided_id: Vec) -> Hash { - AutomationTime::generate_task_id(account_id, provided_id) - } - fn query_fee_details( uxt: ::Extrinsic, ) -> Result, Vec> { @@ -1223,19 +1219,8 @@ impl_runtime_apis! { Ok(AutostakingResult{period: res.0, apy: res.1}) } - fn get_auto_compound_delegated_stake_task_ids(account_id: AccountId) -> Vec { - ParachainStaking::delegator_state(account_id.clone()) - .map_or(vec![], |s| s.delegations.0).into_iter() - .map(|d| d.owner) - .map(|collator_id| { - AutomationTime::generate_auto_compound_delegated_stake_provided_id(&account_id, &collator_id) - }) - .map(|provided_id| { - AutomationTime::generate_task_id(account_id.clone(), provided_id) - }) - .filter(|task_id| { - AutomationTime::get_account_task(account_id.clone(), task_id).is_some() - }).collect() + fn get_auto_compound_delegated_stake_task_ids(account_id: AccountId) -> Vec> { + AutomationTime::get_auto_compound_delegated_stake_task_ids(account_id) } } diff --git a/runtime/oak/src/lib.rs b/runtime/oak/src/lib.rs index 1cb5451c7..e422924a2 100644 --- a/runtime/oak/src/lib.rs +++ b/runtime/oak/src/lib.rs @@ -1147,10 +1147,6 @@ impl_runtime_apis! { } impl pallet_automation_time_rpc_runtime_api::AutomationTimeApi for Runtime { - fn generate_task_id(account_id: AccountId, provided_id: Vec) -> Hash { - AutomationTime::generate_task_id(account_id, provided_id) - } - fn query_fee_details( uxt: ::Extrinsic, ) -> Result, Vec> { @@ -1235,19 +1231,8 @@ impl_runtime_apis! { Ok(AutostakingResult{period: res.0, apy: res.1}) } - fn get_auto_compound_delegated_stake_task_ids(account_id: AccountId) -> Vec { - ParachainStaking::delegator_state(account_id.clone()) - .map_or(vec![], |s| s.delegations.0).into_iter() - .map(|d| d.owner) - .map(|collator_id| { - AutomationTime::generate_auto_compound_delegated_stake_provided_id(&account_id, &collator_id) - }) - .map(|provided_id| { - AutomationTime::generate_task_id(account_id.clone(), provided_id) - }) - .filter(|task_id| { - AutomationTime::get_account_task(account_id.clone(), task_id).is_some() - }).collect() + fn get_auto_compound_delegated_stake_task_ids(account_id: AccountId) -> Vec> { + AutomationTime::get_auto_compound_delegated_stake_task_ids(account_id) } } diff --git a/runtime/turing/src/lib.rs b/runtime/turing/src/lib.rs index 621ae63f7..bae47d29a 100644 --- a/runtime/turing/src/lib.rs +++ b/runtime/turing/src/lib.rs @@ -146,7 +146,17 @@ pub type Executive = frame_executive::Executive< // All migrations executed on runtime upgrade as a nested tuple of types implementing // `OnRuntimeUpgrade`. -type Migrations = (pallet_automation_time::migrations::update_xcmp_task::UpdateXcmpTask,); +type Migrations = ( + // First we upgrade storage from the old task id -> the new task id + pallet_automation_time::migrations::update_task_idv2::UpdateTaskIDV2ForTaskQueueV2, + pallet_automation_time::migrations::update_task_idv2::UpdateTaskIDV2ForMissedQueueV2, + pallet_automation_time::migrations::update_task_idv2::UpdateTaskIDV2ForScheduledTasksV3< + Runtime, + >, + // Then we add the extra info in new XCMP, we also update the new task id for AccountTasks in + // this migration + pallet_automation_time::migrations::update_xcmp_task::UpdateXcmpTask, +); /// Opaque types. These are used by the CLI to instantiate machinery that don't need to know /// the specifics of the runtime. They can then be made to be agnostic over specific formats @@ -1159,10 +1169,6 @@ impl_runtime_apis! { } impl pallet_automation_time_rpc_runtime_api::AutomationTimeApi for Runtime { - fn generate_task_id(account_id: AccountId, provided_id: Vec) -> Hash { - AutomationTime::generate_task_id(account_id, provided_id) - } - fn query_fee_details( uxt: ::Extrinsic, ) -> Result, Vec> { @@ -1249,19 +1255,8 @@ impl_runtime_apis! { Ok(AutostakingResult{period: res.0, apy: res.1}) } - fn get_auto_compound_delegated_stake_task_ids(account_id: AccountId) -> Vec { - ParachainStaking::delegator_state(account_id.clone()) - .map_or(vec![], |s| s.delegations.0).into_iter() - .map(|d| d.owner) - .map(|collator_id| { - AutomationTime::generate_auto_compound_delegated_stake_provided_id(&account_id, &collator_id) - }) - .map(|provided_id| { - AutomationTime::generate_task_id(account_id.clone(), provided_id) - }) - .filter(|task_id| { - AutomationTime::get_account_task(account_id.clone(), task_id).is_some() - }).collect() + fn get_auto_compound_delegated_stake_task_ids(account_id: AccountId) -> Vec> { + AutomationTime::get_auto_compound_delegated_stake_task_ids(account_id) } }