fix migration and tests

[wangjj9219]

The key's concat type of the map storage assetRegistry.locationToAssetId is Twox64Concat:

open-runtime-module-library/asset-registry/src/lib.rs

Line 100 in a730f79

pub type LocationToAssetId<T: Config> = StorageMap<_, Twox64Concat, MultiLocation, T::AssetId, OptionQuery>;

In the previous migration test cases, using Blake2_128Concat as the concat type for the old key:

open-runtime-module-library/asset-registry/src/migrations.rs

Line 30 in a730f79

storage_key_iter::<xcm::v2::MultiLocation, T::AssetId, Blake2_128Concat>(module_prefix, storage_prefix)

open-runtime-module-library/asset-registry/src/tests.rs

Lines 613 to 619 in a730f79

	// Store raw xcm::v2 data
	put_storage_value(
	module_prefix,
	storage_prefix,
	&Blake2_128Concat::hash(&old_key),
	asset_id,
	);

This seems masked some issues:
there are many Multilocation with GeneralKey on Karura, some data lost during the migration tests on forked Karura.

After debugging, it was found that atually if the Iterator here is not drained first, the insertion in the following for loop maybe cause skip some data:

open-runtime-module-library/asset-registry/src/migrations.rs

Lines 29 to 37 in a730f79

	let old_data =
	storage_key_iter::<xcm::v2::MultiLocation, T::AssetId, Blake2_128Concat>(module_prefix, storage_prefix)
	.drain();
	for (old_key, value) in old_data {
	weight.saturating_accrue(T::DbWeight::get().writes(1));
	let new_key: MultiLocation = old_key.try_into().expect("Stored xcm::v2::MultiLocation");
	LocationToAssetId::<T>::insert(new_key, value);
	}

[codecov-commenter]

Codecov Report

Merging #893 (8e90d6a) into master (6df51b3) will increase coverage by 0.05%.
The diff coverage is 93.33%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #893      +/-   ##
==========================================
+ Coverage   78.79%   78.84%   +0.05%     
==========================================
  Files         106      106              
  Lines       10838    10879      +41     
==========================================
+ Hits         8540     8578      +38     
- Misses       2298     2301       +3     

Impacted Files	Coverage Δ
asset-registry/src/migrations.rs	85.71% <0.00%> (ø)
asset-registry/src/tests.rs	97.95% <95.34%> (-0.10%)	⬇️
unknown-tokens/src/migrations.rs	88.63% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[wangjj9219]

@tgmichel Hi, buddy, you should include this fix.

[xlc]

We should prefer use translate if possible.

Also a sidenote, read everything in memory is actually ok because the 5MB PoV limit will kick in way before the actual memory limit.

[tgmichel]

you should include this fix

Thank you @wangjj9219

prefer use translate if possible

translate can be used to migrate values, not keys unfortunately (afaik)

[xlc]

Right. It is not possible to do in place translate of the key while keeping the same storage name.