remove localAssets lazy migration (#2797)

RomarQ · web-flow · commit b064f1a25c78 · 2024-05-20T10:41:51.000+01:00
* remove localAssets lazy migration

* remove unused type
diff --git a/pallets/moonbeam-lazy-migrations/src/lib.rs b/pallets/moonbeam-lazy-migrations/src/lib.rs
@@ -37,25 +37,16 @@ pub use pallet::*;
 pub mod pallet {
 	use super::*;
 	use frame_support::pallet_prelude::*;
-	use frame_support::traits::ReservableCurrency;
 	use frame_system::pallet_prelude::*;
 	use sp_core::H160;
 
 	pub const ARRAY_LIMIT: u32 = 1000;
 	pub type GetArrayLimit = ConstU32<ARRAY_LIMIT>;
 
-	const INTERMEDIATES_NODES_SIZE: u64 = 4096;
-	const MAX_LOCAL_ASSETS_STORAGE_ENTRY_SIZE: u64 =
-		(/* biggest key on moonbeam */136) + (/* biggest value on moonbeam */142);
-
 	/// Pallet for multi block migrations
 	#[pallet::pallet]
 	pub struct Pallet<T>(PhantomData<T>);
 
-	#[pallet::storage]
-	/// If true, it means that LocalAssets storage has been removed.
-	pub(crate) type LocalAssetsMigrationCompleted<T: Config> = StorageValue<_, bool, ValueQuery>;
-
 	#[pallet::storage]
 	/// The total number of suicided contracts that were removed
 	pub(crate) type SuicidedContractsRemoved<T: Config> = StorageValue<_, u32, ValueQuery>;
@@ -68,16 +59,8 @@ pub mod pallet {
 
 	#[pallet::error]
 	pub enum Error<T> {
-		/// There are no more storage entries to be removed
-		AllStorageEntriesHaveBeenRemoved,
 		/// The limit cannot be zero
 		LimitCannotBeZero,
-		/// The maximum number of assets cannot be zero
-		MaxAssetsCannotBeZero,
-		/// The limit for unlocking funds is too high
-		UnlockLimitTooHigh,
-		/// There are no more VotingOf entries to be removed and democracy funds to be unlocked
-		AllDemocracyFundsUnlocked,
 		/// There must be at least one address
 		AddressesLengthCannotBeZero,
 		/// The contract is not corrupted (Still exist or properly suicided)
@@ -86,126 +69,6 @@ pub mod pallet {
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
-		// TODO(rodrigo): This extrinsic should be removed once LocalAssets pallet storage is removed
-		#[pallet::call_index(0)]
-		#[pallet::weight({
-			// "*limit" is used twice to account to the possibility that we may need to unreserve 
-			// deposits for every approval
-			let possible_iterations = max_assets.saturating_add(*limit).saturating_add(*limit);
-			let proof_size = INTERMEDIATES_NODES_SIZE + (MAX_LOCAL_ASSETS_STORAGE_ENTRY_SIZE
-				.saturating_mul(<u64>::from(possible_iterations)));
-
-			Weight::from_parts(0, proof_size)
-			.saturating_add(<T as frame_system::Config>::DbWeight::get()
-				.reads_writes((*max_assets + *limit + 1).into(), (*limit + 1).into()))
-		})]
-		pub fn clear_local_assets_storage(
-			origin: OriginFor<T>,
-			max_assets: u32,
-			limit: u32,
-		) -> DispatchResultWithPostInfo {
-			ensure_signed(origin)?;
-			ensure!(limit != 0, Error::<T>::LimitCannotBeZero);
-			ensure!(max_assets != 0, Error::<T>::MaxAssetsCannotBeZero);
-
-			ensure!(
-				!LocalAssetsMigrationCompleted::<T>::get(),
-				Error::<T>::AllStorageEntriesHaveBeenRemoved
-			);
-
-			let mut allowed_removals = limit;
-
-			const PALLET_PREFIX: &'static str = "LocalAssets";
-
-			struct LocalAssetsStorageAsset;
-			impl frame_support::traits::StorageInstance for LocalAssetsStorageAsset {
-				const STORAGE_PREFIX: &'static str = "Asset";
-				fn pallet_prefix() -> &'static str {
-					PALLET_PREFIX
-				}
-			}
-			struct LocalAssetsStorageApprovals;
-			impl frame_support::traits::StorageInstance for LocalAssetsStorageApprovals {
-				const STORAGE_PREFIX: &'static str = "Approvals";
-				fn pallet_prefix() -> &'static str {
-					PALLET_PREFIX
-				}
-			}
-
-			/// Data concerning an approval.
-			/// Duplicated the type from pallet_assets (The original type is private)
-			#[derive(
-				Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, Default, MaxEncodedLen, TypeInfo,
-			)]
-			pub struct Approval<Balance, DepositBalance> {
-				/// The amount of funds approved for the balance transfer from the owner to some delegated
-				/// target.
-				pub(super) amount: Balance,
-				/// The amount reserved on the owner's account to hold this item in storage.
-				pub(super) deposit: DepositBalance,
-			}
-
-			type AssetMap = frame_support::storage::types::StorageMap<
-				LocalAssetsStorageAsset,
-				Blake2_128Concat,
-				u128,
-				// It is fine to add a dummy `Value` type
-				// The value is not going to be decoded, since we only care about the keys)
-				(),
-			>;
-
-			for asset_id in AssetMap::iter_keys().take(max_assets as usize) {
-				let approvals_iter = frame_support::storage::types::StorageNMap::<
-					LocalAssetsStorageApprovals,
-					(
-						NMapKey<Blake2_128Concat, u128>,         // asset id
-						NMapKey<Blake2_128Concat, T::AccountId>, // owner
-						NMapKey<Blake2_128Concat, T::AccountId>, // delegate
-					),
-					Approval<T::Balance, T::Balance>,
-				>::drain_prefix((asset_id,));
-				for ((owner, _), approval) in approvals_iter {
-					allowed_removals = allowed_removals.saturating_sub(1);
-					// Unreserve deposit (most likely will be zero)
-					pallet_balances::Pallet::<T>::unreserve(&owner, approval.deposit);
-					// Check if the removal limit was reached
-					if allowed_removals < 1 {
-						break;
-					}
-				}
-				// Remove asset, since it does not contain more approvals
-				AssetMap::remove(asset_id);
-				allowed_removals = allowed_removals.saturating_sub(1);
-				// Check if the removal limit was reached
-				if allowed_removals < 1 {
-					break;
-				}
-			}
-
-			// Remove remaining storage
-			if allowed_removals > 0 {
-				let hashed_prefix = sp_io::hashing::twox_128(PALLET_PREFIX.as_bytes());
-
-				let keys_removed =
-					match sp_io::storage::clear_prefix(&hashed_prefix, Some(allowed_removals)) {
-						sp_io::KillStorageResult::AllRemoved(value) => {
-							LocalAssetsMigrationCompleted::<T>::set(true);
-							value
-						}
-						sp_io::KillStorageResult::SomeRemaining(value) => value,
-					};
-
-				allowed_removals = allowed_removals.saturating_sub(keys_removed);
-			}
-
-			log::info!(
-				"Removed {} storge keys 🧹",
-				limit.saturating_sub(allowed_removals)
-			);
-
-			Ok(Pays::No.into())
-		}
-
 		// TODO(rodrigo): This extrinsic should be removed once the storage of destroyed contracts
 		// has been removed
 		#[pallet::call_index(1)]
diff --git a/pallets/moonbeam-lazy-migrations/src/tests.rs b/pallets/moonbeam-lazy-migrations/src/tests.rs
@@ -20,7 +20,7 @@ use {
 		mock::{ExtBuilder, LazyMigrations, RuntimeOrigin, Test},
 		Error,
 	},
-	frame_support::{assert_noop, assert_ok},
+	frame_support::assert_noop,
 	rlp::RlpStream,
 	sp_core::{H160, H256},
 	sp_io::hashing::keccak_256,
@@ -263,64 +263,3 @@ fn test_clear_suicided_mixed_suicided_and_non_suicided() {
 		);
 	})
 }
-
-/// TODO(rodrigo): This test should be removed once LocalAssets pallet storage is removed
-#[test]
-fn test_call_clear_local_assets_storage() {
-	let mut context = ExtBuilder::default().build();
-
-	let pallet_prefix = sp_io::hashing::twox_128("LocalAssets".as_bytes());
-	let total_storage_entries: u8 = 5;
-
-	let gen_dummy_entry = |dummy: u8| -> [u8; 32] {
-		[pallet_prefix, sp_io::hashing::twox_128(&[dummy])]
-			.concat()
-			.try_into()
-			.unwrap()
-	};
-
-	context.execute_with(|| {
-		for i in 0u8..total_storage_entries {
-			let dummy_data = gen_dummy_entry(i);
-			sp_io::storage::set(&dummy_data, &dummy_data);
-			dbg!(sp_io::storage::exists(&dummy_data));
-		}
-	});
-
-	// Commit changes
-	let _ = context.commit_all();
-
-	// Next block
-	context.execute_with(|| {
-		// Check that the storage entries exist before attempting to remove it
-		for i in 0u8..total_storage_entries {
-			let dummy_data = gen_dummy_entry(i);
-			assert!(sp_io::storage::exists(&dummy_data));
-		}
-		// Clear all storage entries
-		assert_ok!(LazyMigrations::clear_local_assets_storage(
-			RuntimeOrigin::signed(AccountId32::from([0; 32])),
-			1,
-			total_storage_entries.into()
-		));
-		// Check that all storage entries got deleted
-		for i in 0u8..total_storage_entries {
-			let dummy_data = gen_dummy_entry(i);
-			assert!(!sp_io::storage::exists(&dummy_data));
-		}
-	});
-
-	// Commit changes
-	let _ = context.commit_all();
-
-	// Next block
-	context.execute_with(|| {
-		// No more storage entries to be removed (expect failure)
-		assert!(LazyMigrations::clear_local_assets_storage(
-			RuntimeOrigin::signed(AccountId32::from([0; 32])),
-			1,
-			1
-		)
-		.is_err())
-	});
-}
diff --git a/runtime/moonbase/src/asset_config.rs b/runtime/moonbase/src/asset_config.rs
@@ -179,9 +179,6 @@ pub type ForeignAssetModifierOrigin = EitherOfDiverse<
 	>,
 >;
 
-pub type LocalAssetModifierOrigin =
-	EitherOfDiverse<EnsureRoot<AccountId>, governance::custom_origins::GeneralAdmin>;
-
 impl pallet_asset_manager::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
 	type Balance = Balance;
diff --git a/runtime/moonbase/tests/xcm_mock/parachain.rs b/runtime/moonbase/tests/xcm_mock/parachain.rs
@@ -126,7 +126,6 @@ impl pallet_balances::Config for Runtime {
 }
 
 pub type ForeignAssetInstance = ();
-pub type LocalAssetInstance = pallet_assets::Instance1;
 
 // Required for runtime benchmarks
 pallet_assets::runtime_benchmarks_enabled! {
@@ -174,30 +173,6 @@ impl pallet_assets::Config<ForeignAssetInstance> for Runtime {
 	}
 }
 
-impl pallet_assets::Config<LocalAssetInstance> for Runtime {
-	type RuntimeEvent = RuntimeEvent;
-	type Balance = Balance;
-	type AssetId = AssetId;
-	type Currency = Balances;
-	type ForceOrigin = EnsureRoot<AccountId>;
-	type AssetDeposit = AssetDeposit;
-	type MetadataDepositBase = MetadataDepositBase;
-	type MetadataDepositPerByte = MetadataDepositPerByte;
-	type ApprovalDeposit = ApprovalDeposit;
-	type StringLimit = AssetsStringLimit;
-	type Freezer = ();
-	type Extra = ();
-	type AssetAccountDeposit = AssetAccountDeposit;
-	type WeightInfo = pallet_assets::weights::SubstrateWeight<Runtime>;
-	type RemoveItemsLimit = ConstU32<656>;
-	type AssetIdParameter = AssetId;
-	type CreateOrigin = AsEnsureOriginWithArg<EnsureNever<AccountId>>;
-	type CallbackHandle = ();
-	pallet_assets::runtime_benchmarks_enabled! {
-		type BenchmarkHelper = BenchmarkHelper;
-	}
-}
-
 /// Type for specifying how a `Location` can be converted into an `AccountId`. This is used
 /// when determining ownership of accounts for asset transacting and when attempting to use XCM
 /// `Transact` in order to determine the dispatch Origin.
@@ -1092,7 +1067,6 @@ construct_runtime!(
 		AssetManager: pallet_asset_manager,
 		XcmTransactor: pallet_xcm_transactor,
 		Treasury: pallet_treasury,
-		LocalAssets: pallet_assets::<Instance1>,
 		Proxy: pallet_proxy,
 
 		Timestamp: pallet_timestamp,
diff --git a/runtime/moonbeam/src/asset_config.rs b/runtime/moonbeam/src/asset_config.rs
@@ -177,9 +177,6 @@ pub type ForeignAssetModifierOrigin = EitherOfDiverse<
 	>,
 >;
 
-pub type LocalAssetModifierOrigin =
-	EitherOfDiverse<EnsureRoot<AccountId>, governance::custom_origins::GeneralAdmin>;
-
 impl pallet_asset_manager::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
 	type Balance = Balance;
diff --git a/runtime/moonriver/src/asset_config.rs b/runtime/moonriver/src/asset_config.rs
@@ -177,8 +177,6 @@ pub type ForeignAssetModifierOrigin = EitherOfDiverse<
 		governance::custom_origins::GeneralAdmin,
 	>,
 >;
-pub type LocalAssetModifierOrigin =
-	EitherOfDiverse<EnsureRoot<AccountId>, governance::custom_origins::GeneralAdmin>;
 
 impl pallet_asset_manager::Config for Runtime {
 	type RuntimeEvent = RuntimeEvent;
diff --git a/test/suites/dev/moonbase/test-pov/test-lazy-migrations-storage-removal.ts b/test/suites/dev/moonbase/test-pov/test-lazy-migrations-storage-removal.ts
diff --git a/test/suites/smoke/test-foreign-asset-consistency.ts b/test/suites/smoke/test-foreign-asset-consistency.ts
