From a3ffd039d1a77c1ecb62f3860b2a63ba7e97aa8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vinh=20Qu=E1=BB=91c=20Nguy=E1=BB=85n?= Date: Tue, 8 Aug 2023 17:54:56 -0700 Subject: [PATCH] use Vector of old task id hash for new task id (#412) * use Vector of old task id hash for new task id instead of converting the hash to its hex format and use the ascii string of the hex as Vec for new task id, we use the original [u8, 32] of the Hash and convert it to a Vec This will keep our task id show up in the same way before and after update. * code format --- pallets/automation-time/src/benchmarking.rs | 2 +- .../src/migrations/update_task_idv2.rs | 55 ++++++++++--------- .../src/migrations/update_xcmp_task.rs | 23 ++++---- .../automation-time/src/migrations/utils.rs | 20 ++----- pallets/automation-time/src/mock.rs | 7 +-- 5 files changed, 50 insertions(+), 57 deletions(-) diff --git a/pallets/automation-time/src/benchmarking.rs b/pallets/automation-time/src/benchmarking.rs index 35067287..baf6284a 100644 --- a/pallets/automation-time/src/benchmarking.rs +++ b/pallets/automation-time/src/benchmarking.rs @@ -176,7 +176,7 @@ benchmarks! { let fee = AssetPayment { asset_location: MultiLocation::new(0, Here).into(), amount: 100u128 }; - let mut task_id = schedule_xcmp_tasks::(caller.clone(), times, max_tasks_per_slot - 1); + let 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()) diff --git a/pallets/automation-time/src/migrations/update_task_idv2.rs b/pallets/automation-time/src/migrations/update_task_idv2.rs index b192146c..809ede13 100644 --- a/pallets/automation-time/src/migrations/update_task_idv2.rs +++ b/pallets/automation-time/src/migrations/update_task_idv2.rs @@ -1,25 +1,18 @@ use core::marker::PhantomData; -use crate::{ - AccountTaskId, Config, MissedTaskV2, MissedTaskV2Of, ScheduledTasksOf, TaskIdV2, UnixTime, -}; +use crate::{AccountTaskId, Config, MissedTaskV2Of, ScheduledTasksOf, TaskIdV2, UnixTime}; use codec::{Decode, Encode}; 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 sp_std::vec::Vec; use crate::migrations::utils::{ - deprecate::{generate_old_task_id, old_taskid_to_idv2}, - OldAccountTaskId, OldMissedTaskV2Of, OldScheduledTasksOf, TEST_TASKID1, TEST_TASKID2, + deprecate::old_taskid_to_idv2, OldAccountTaskId, OldMissedTaskV2Of, OldScheduledTasksOf, }; // This is old TaskQueueV2 with old Task @@ -189,14 +182,18 @@ impl OnRuntimeUpgrade for UpdateTaskIDV2ForMissedQueueV2 { #[cfg(test)] mod test { + use super::{ - generate_old_task_id, MissedTaskV2, UpdateTaskIDV2ForMissedQueueV2, - UpdateTaskIDV2ForScheduledTasksV3, UpdateTaskIDV2ForTaskQueueV2, TEST_TASKID1, - TEST_TASKID2, + UpdateTaskIDV2ForMissedQueueV2, UpdateTaskIDV2ForScheduledTasksV3, + UpdateTaskIDV2ForTaskQueueV2, }; use crate::{ - migrations::utils::{OldMissedTaskV2, OldScheduledTasksOf}, + migrations::utils::{ + deprecate::generate_old_task_id, OldMissedTaskV2, OldScheduledTasksOf, TEST_TASKID1, + TEST_TASKID2, + }, mock::*, + MissedTaskV2, }; use frame_support::traits::OnRuntimeUpgrade; use sp_runtime::AccountId32; @@ -207,8 +204,10 @@ mod test { 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 task_id1: Vec = + hex::decode(TEST_TASKID1).expect("test task id hex code is malformed"); + let task_id2: Vec = + hex::decode(TEST_TASKID2).expect("test task id hex code is malformed"); let old_taskqueuev2 = vec![ (account_id1.clone(), generate_old_task_id::(account_id1.clone(), vec![1])), @@ -247,8 +246,8 @@ mod test { 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(); + let task_id1: Vec = hex::decode(TEST_TASKID1).expect("TEST_TASKID1 is malformed"); + let task_id2: Vec = hex::decode(TEST_TASKID2).expect("TEST_TASKID1 is malformed"); super::ScheduledTasksV3::::insert( 3600, @@ -315,16 +314,18 @@ mod test { // (task owner, vec![33]) hash -> "0x7191f3d83bcbeb221f7b00f501e9a8da3ba3c2d15a672eb694ee7e09dbaddd1e" ( task_owner2.clone(), - "0x7191f3d83bcbeb221f7b00f501e9a8da3ba3c2d15a672eb694ee7e09dbaddd1e" - .as_bytes() - .to_vec(), + hex::decode( + "7191f3d83bcbeb221f7b00f501e9a8da3ba3c2d15a672eb694ee7e09dbaddd1e" + ) + .expect("invalid hex code") ), // (task owner1, vec![32]) hash -> "0xe94040ca5d09f0e1023aecf5abc3afac0b9e66bc3b1209183b3a009f4c073c2b" ( task_owner1.clone(), - "0xe94040ca5d09f0e1023aecf5abc3afac0b9e66bc3b1209183b3a009f4c073c2b" - .as_bytes() - .to_vec(), + hex::decode( + "e94040ca5d09f0e1023aecf5abc3afac0b9e66bc3b1209183b3a009f4c073c2b" + ) + .expect("invalid hex code"), ), ], "migration failed to convert old ScheduledTasksV3 to new TaskID format" @@ -339,8 +340,10 @@ mod test { 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 task_id1: Vec = + hex::decode(TEST_TASKID1).expect("test task id hex code is malformed"); + let task_id2: Vec = + hex::decode(TEST_TASKID2).expect("test task id hex code is malformed"); let old_missed_queueu_v2 = vec![ OldMissedTaskV2 { diff --git a/pallets/automation-time/src/migrations/update_xcmp_task.rs b/pallets/automation-time/src/migrations/update_xcmp_task.rs index a2bb36e6..43f0bca4 100644 --- a/pallets/automation-time/src/migrations/update_xcmp_task.rs +++ b/pallets/automation-time/src/migrations/update_xcmp_task.rs @@ -7,14 +7,11 @@ use frame_support::{ Twox64Concat, }; use sp_runtime::traits::Convert; -use sp_std::{boxed::Box, vec, vec::Vec}; +use sp_std::{vec, vec::Vec}; use xcm::latest::prelude::*; -use crate::migrations::utils::{ - deprecate::generate_old_task_id, OldAccountTaskId, OldAction, OldTask, OldTaskId, TEST_TASKID1, -}; +use crate::migrations::utils::{OldAction, OldTask, OldTaskId}; -use frame_support::{dispatch::GetDispatchInfo, pallet_prelude::DispatchError}; use primitives::TransferCallCreator; #[cfg(feature = "try-runtime")] @@ -93,6 +90,7 @@ impl OnRuntimeUpgrade for UpdateXcmpTask { let mut tasks: Vec<(AccountOf, TaskOf)> = vec![]; AccountTasks::::drain().for_each(|(account_id, _task_id, task)| { let migrated_task: TaskOf = task.into(); + tasks.push((account_id, migrated_task)); migrated_tasks += 1; }); @@ -131,7 +129,12 @@ impl OnRuntimeUpgrade for UpdateXcmpTask { #[cfg(test)] mod test { use super::*; - use crate::{mock::*, ActionOf, AssetPayment, InstructionSequence, Schedule, TaskOf}; + use crate::{ + migrations::utils::{deprecate::generate_old_task_id, TEST_TASKID1}, + mock::*, + ActionOf, AssetPayment, InstructionSequence, Schedule, TaskOf, + }; + use cumulus_primitives_core::ParaId; use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; use sp_runtime::AccountId32; @@ -169,7 +172,7 @@ mod test { assert_eq!(crate::AccountTasks::::iter().count(), 1); - let task_id1 = TEST_TASKID1.as_bytes().to_vec(); + let task_id1 = hex::decode(TEST_TASKID1).expect("malform hex code for test task id"); assert_eq!( crate::AccountTasks::::get(account_id.clone(), task_id1.clone()).unwrap(), TaskOf:: { @@ -221,9 +224,7 @@ mod test { UpdateXcmpTask::::on_runtime_upgrade(); - assert_eq!(crate::AccountTasks::::iter().count(), 1); - - let task_id1 = TEST_TASKID1.as_bytes().to_vec(); + let task_id1 = hex::decode(TEST_TASKID1).expect("malform hex code for test task id"); assert_eq!( crate::AccountTasks::::get(account_id.clone(), task_id1.clone()).unwrap(), TaskOf:: { @@ -280,7 +281,7 @@ mod test { assert_eq!(crate::AccountTasks::::iter().count(), 1); - let task_id1 = TEST_TASKID1.as_bytes().to_vec(); + let task_id1 = hex::decode(TEST_TASKID1).expect("malform hex code for test task id"); assert_eq!( crate::AccountTasks::::get(account_id.clone(), task_id1.clone()).unwrap(), TaskOf:: { diff --git a/pallets/automation-time/src/migrations/utils.rs b/pallets/automation-time/src/migrations/utils.rs index 59fdb0a3..cd46bfb4 100644 --- a/pallets/automation-time/src/migrations/utils.rs +++ b/pallets/automation-time/src/migrations/utils.rs @@ -1,26 +1,19 @@ // 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 crate::{AccountOf, BalanceOf, Config, Schedule, TaskOf, UnixTime}; use codec::{Decode, Encode}; use cumulus_primitives_core::ParaId; -use frame_support::{ - dispatch::EncodeLike, storage::types::ValueQuery, traits::OnRuntimeUpgrade, weights::Weight, - Twox64Concat, -}; +use frame_support::weights::Weight; -use scale_info::{prelude::format, TypeInfo}; +use scale_info::TypeInfo; use sp_std::{vec, vec::Vec}; use xcm::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"; +pub const TEST_TASKID1: &str = "d1263842e34adeb00be1146b30bc6527951f0a0f5d5a3f7a758537735b8bcb04"; +pub const TEST_TASKID2: &str = "f76acf0b1d8ef503450a4d5c1a451f1921a906e8711f551c2945e09fb44de5ff"; // Old format pub type OldTaskId = ::Hash; @@ -127,8 +120,7 @@ pub mod deprecate { } 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() + old_task_id.as_ref().into() } pub fn generate_old_task_id( diff --git a/pallets/automation-time/src/mock.rs b/pallets/automation-time/src/mock.rs index 27d67c69..11cbe8b5 100644 --- a/pallets/automation-time/src/mock.rs +++ b/pallets/automation-time/src/mock.rs @@ -684,11 +684,8 @@ 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), + RuntimeEvent::AutomationTime(crate::Event::TaskScheduled { task_id, .. }) => + Some(task_id), _ => None, }) .collect::>()