Polkadot-1.7.2: restricted transfer location changes

libs/types/src/locations.rs

@@ -14,45 +14,19 @@ use cfg_primitives::AccountId;
 use frame_support::RuntimeDebugNoBound;
 use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
 use scale_info::TypeInfo;
-use sp_core::{crypto::AccountId32, H256};
-use sp_runtime::traits::{BlakeTwo256, Hash};
-use staging_xcm::VersionedLocation;
+// Please note that if this version change,
+// a migration could be required in those places where
+// RestrictedTransferLocation is stored
+use staging_xcm::v4::Location;
 
 use crate::domain_address::DomainAddress;
 /// Location types for destinations that can receive restricted transfers
 #[derive(Clone, RuntimeDebugNoBound, Encode, Decode, Eq, PartialEq, MaxEncodedLen, TypeInfo)]
 pub enum RestrictedTransferLocation {
 	/// Local chain account sending destination.
 	Local(AccountId),
-	/// XCM MultiLocation sending destinations.
-	/// Using hash value here as Multilocation is large -- v1 is 512 bytes, but
-	/// next largest is only 40 bytes other values aren't hashed as we have
-	/// blake2 hashing on storage map keys, and we don't want the extra overhead
-	XCM(H256),
+	/// XCM Location sending destinations.
+	XCM(Location),
 	/// DomainAddress sending location from a liquidity pools' instance
 	Address(DomainAddress),
 }
-
-impl From<AccountId32> for RestrictedTransferLocation {
-	fn from(value: AccountId32) -> Self {
-		Self::Local(value)
-	}
-}
-
-impl From<VersionedLocation> for RestrictedTransferLocation {
-	fn from(vml: VersionedLocation) -> Self {
-		// using hash here as multilocation is significantly larger than any other enum
-		// type here -- 592 bytes, vs 40 bytes for domain address (next largest)
-		Self::XCM(BlakeTwo256::hash(&vml.encode()))
-
-		// TODO-1.7: I'm afraid of locations translated from v3 to v4 will
-		// generate a different hash here. How this affect our current chain
-		// state?
-	}
-}
-
-impl From<DomainAddress> for RestrictedTransferLocation {
-	fn from(da: DomainAddress) -> Self {
-		Self::Address(da)
-	}
-}

pallets/transfer-allowlist/src/lib.rs

@@ -109,15 +109,7 @@ pub mod pallet {
 		type Deposit: Get<DepositBalanceOf<Self>>;
 
 		/// Type containing the locations a transfer can be sent to.
-		type Location: Member
-			+ Debug
-			+ Eq
-			+ PartialEq
-			+ TypeInfo
-			+ Encode
-			+ EncodeLike
-			+ Decode
-			+ MaxEncodedLen;
+		type Location: Member + TypeInfo + Encode + EncodeLike + Decode + MaxEncodedLen;
 
 		/// Type for pallet weights
 		type WeightInfo: WeightInfo;

runtime/common/src/migrations/mod.rs

@@ -17,6 +17,7 @@ pub mod increase_storage_version;
 pub mod loans;
 pub mod nuke;
 pub mod precompile_account_codes;
+pub mod restricted_location;
 
 pub mod utils {
 	use frame_support::storage::unhashed;

runtime/common/src/migrations/restricted_location.rs

@@ -0,0 +1,126 @@
+use cfg_primitives::AccountId;
+use cfg_types::{locations::RestrictedTransferLocation, tokens::CurrencyId};
+use frame_support::{
+	traits::{Get, OnRuntimeUpgrade},
+	weights::Weight,
+};
+use pallet_transfer_allowlist::AccountCurrencyTransferAllowance;
+use parity_scale_codec::Encode;
+use sp_core::H256;
+use sp_runtime::traits::{BlakeTwo256, Hash};
+use staging_xcm::v4;
+
+mod old {
+	use cfg_primitives::AccountId;
+	use cfg_types::{domain_address::DomainAddress, tokens::CurrencyId};
+	use frame_support::{pallet_prelude::*, storage_alias};
+	use frame_system::pallet_prelude::*;
+	use pallet_transfer_allowlist::AllowanceDetails;
+	use sp_core::H256;
+	use staging_xcm::v3;
+
+	#[derive(
+		Clone, RuntimeDebugNoBound, Encode, Decode, Eq, PartialEq, MaxEncodedLen, TypeInfo,
+	)]
+	pub enum RestrictedTransferLocation {
+		Local(AccountId),
+		XCM(H256),
+		Address(DomainAddress),
+	}
+
+	#[storage_alias]
+	pub type AccountCurrencyTransferAllowance<T: pallet_transfer_allowlist::Config> = StorageNMap<
+		pallet_transfer_allowlist::Pallet<T>,
+		(
+			NMapKey<Twox64Concat, AccountId>,
+			NMapKey<Twox64Concat, CurrencyId>,
+			NMapKey<Blake2_128Concat, RestrictedTransferLocation>,
+		),
+		AllowanceDetails<BlockNumberFor<T>>,
+		OptionQuery,
+	>;
+
+	pub fn create_apps_location_v3(account_id: &AccountId) -> v3::Location {
+		// Ref: https://github.com/centrifuge/apps/blob/b59bdd34561a4ccd90e0d803c14a3729fc2f3a6d/centrifuge-app/src/utils/usePermissions.tsx#L386
+		//v3::Location::new(1, v3::Junctions::X2((), ()))
+		todo!()
+	}
+}
+
+const LOG_PREFIX: &str = "MigrateRestrictedTransferLocation:";
+
+pub struct MigrateRestrictedTransferLocation<T>(sp_std::marker::PhantomData<T>);
+impl<T> OnRuntimeUpgrade for MigrateRestrictedTransferLocation<T>
+where
+	T: pallet_transfer_allowlist::Config<
+		AccountId = AccountId,
+		CurrencyId = CurrencyId,
+		Location = RestrictedTransferLocation,
+	>,
+{
+	fn on_runtime_upgrade() -> Weight {
+		log::info!("{LOG_PREFIX} Check keys to migrate...");
+
+		let mut weight = Weight::zero();
+
+		let key_translations = old::AccountCurrencyTransferAllowance::<T>::iter_keys()
+			.filter_map(|(account_id, currency_id, old_restricted_location)| {
+				weight.saturating_accrue(T::DbWeight::get().reads(1));
+				match old_restricted_location {
+					old::RestrictedTransferLocation::XCM(hash) => {
+						migrate_location_key(&account_id, hash).map(|new_restricted_location| {
+							(
+								(account_id.clone(), currency_id, old_restricted_location),
+								(account_id, currency_id, new_restricted_location),
+							)
+						})
+					}
+					_ => None,
+				}
+			})
+			.collect::<Vec<_>>();
+
+		for (old_key, new_key) in key_translations {
+			log::info!("{LOG_PREFIX} Remove {old_key:?} and add {new_key:?}");
+
+			let value = old::AccountCurrencyTransferAllowance::<T>::get(&old_key);
+			old::AccountCurrencyTransferAllowance::<T>::remove(old_key);
+			AccountCurrencyTransferAllowance::<T>::set(new_key, value);
+
+			weight.saturating_accrue(T::DbWeight::get().writes(2));
+		}
+
+		weight
+	}
+
+	#[cfg(feature = "try-runtime")]
+	fn pre_upgrade() -> Result<sp_std::vec::Vec<u8>, sp_runtime::TryRuntimeError> {
+		todo!()
+	}
+
+	#[cfg(feature = "try-runtime")]
+	fn post_upgrade(pre_state: sp_std::vec::Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
+		todo!()
+	}
+}
+
+fn migrate_location_key(account_id: &AccountId, hash: H256) -> Option<RestrictedTransferLocation> {
+	let old_location = old::create_apps_location_v3(account_id);
+	if BlakeTwo256::hash(&old_location.encode()) == hash {
+		match v4::Location::try_from(old_location) {
+			Ok(location) => {
+				log::info!("{LOG_PREFIX} Hash: '{hash}' migrated!");
+				let new_restricted_location = RestrictedTransferLocation::XCM(location);
+
+				Some(new_restricted_location)
+			}
+			Err(_) => {
+				log::error!("{LOG_PREFIX} Non isometric location v3 -> v4");
+				None
+			}
+		}
+	} else {
+		log::error!("{LOG_PREFIX} Hash can not be recovered");
+		None
+	}
+}

runtime/common/src/transfer_filter.rs

@@ -22,9 +22,8 @@ use pallet_restricted_tokens::TransferDetails;
 use pallet_restricted_xtokens::TransferEffects;
 use parity_scale_codec::{Decode, Encode};
 use scale_info::TypeInfo;
-use sp_core::Hasher;
 use sp_runtime::{
-	traits::{BlakeTwo256, Convert, DispatchInfoOf, SignedExtension, StaticLookup},
+	traits::{Convert, DispatchInfoOf, SignedExtension, StaticLookup},
 	transaction_validity::{InvalidTransaction, TransactionValidityError},
 	DispatchError, DispatchResult, TokenError,
 };
@@ -49,12 +48,12 @@ impl<
 			amalgamate_allowance(
 				T::allowance(
 					sender.clone(),
-					RestrictedTransferLocation::XCM(BlakeTwo256::hash(&destination.encode())),
+					RestrictedTransferLocation::XCM(destination.clone()),
 					FilterCurrency::Specific(currency),
 				),
 				T::allowance(
 					sender,
-					RestrictedTransferLocation::XCM(BlakeTwo256::hash(&destination.encode())),
+					RestrictedTransferLocation::XCM(destination),
 					FilterCurrency::All,
 				),
 			)