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<u8> for new task id, we use the original [u8, 32] of the Hash and convert it to a Vec<u8> This will keep our task id show up in the same way before and after update. * code format
AvaProtocol · Aug 9, 2023 · a3ffd03 · a3ffd03
1 parent 4885534
commit a3ffd03
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 57 deletions.
diff --git 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::<T>(caller.clone(), times, max_tasks_per_slot - 1);
+		let task_id = schedule_xcmp_tasks::<T>(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
@@ -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<T: Config> OnRuntimeUpgrade for UpdateTaskIDV2ForMissedQueueV2<T> {
 
 #[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<u8> = TEST_TASKID1.as_bytes().to_vec();
-			let task_id2: Vec<u8> = TEST_TASKID2.as_bytes().to_vec();
+			let task_id1: Vec<u8> =
+				hex::decode(TEST_TASKID1).expect("test task id hex code is malformed");
+			let task_id2: Vec<u8> =
+				hex::decode(TEST_TASKID2).expect("test task id hex code is malformed");
 
 			let old_taskqueuev2 = vec![
 				(account_id1.clone(), generate_old_task_id::<Test>(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<u8> = TEST_TASKID1.as_bytes().to_vec();
-			let task_id2: Vec<u8> = TEST_TASKID2.as_bytes().to_vec();
+			let task_id1: Vec<u8> = hex::decode(TEST_TASKID1).expect("TEST_TASKID1 is malformed");
+			let task_id2: Vec<u8> = hex::decode(TEST_TASKID2).expect("TEST_TASKID1 is malformed");
 
 			super::ScheduledTasksV3::<Test>::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<u8> = TEST_TASKID1.as_bytes().to_vec();
-			let task_id2: Vec<u8> = TEST_TASKID2.as_bytes().to_vec();
+			let task_id1: Vec<u8> =
+				hex::decode(TEST_TASKID1).expect("test task id hex code is malformed");
+			let task_id2: Vec<u8> =
+				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
@@ -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<T: Config> OnRuntimeUpgrade for UpdateXcmpTask<T> {
 		let mut tasks: Vec<(AccountOf<T>, TaskOf<T>)> = vec![];
 		AccountTasks::<T>::drain().for_each(|(account_id, _task_id, task)| {
 			let migrated_task: TaskOf<T> = task.into();
+
 			tasks.push((account_id, migrated_task));
 			migrated_tasks += 1;
 		});
@@ -131,7 +129,12 @@ impl<T: Config> OnRuntimeUpgrade for UpdateXcmpTask<T> {
 #[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::<Test>::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::<Test>::get(account_id.clone(), task_id1.clone()).unwrap(),
 				TaskOf::<Test> {
@@ -221,9 +224,7 @@ mod test {
 
 			UpdateXcmpTask::<Test>::on_runtime_upgrade();
 
-			assert_eq!(crate::AccountTasks::<Test>::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::<Test>::get(account_id.clone(), task_id1.clone()).unwrap(),
 				TaskOf::<Test> {
@@ -280,7 +281,7 @@ mod test {
 
 			assert_eq!(crate::AccountTasks::<Test>::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::<Test>::get(account_id.clone(), task_id1.clone()).unwrap(),
 				TaskOf::<Test> {

diff --git 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<T> = <T as frame_system::Config>::Hash;
@@ -127,8 +120,7 @@ pub mod deprecate {
 	}
 
 	pub fn old_taskid_to_idv2<T: frame_system::Config>(old_task_id: &OldTaskId<T>) -> Vec<u8> {
-		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<T: frame_system::Config>(

diff --git 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<TaskIdV2> {
 	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::<Vec<_>>()