Prep for Shiden AssetHub migration #1533

PierreOssun · 2025-08-29T14:54:25Z

Pull Request Summary

Ship 2, 3 and 4 described in this issue: #1532 :

Remove Kusama as KSM reserve

EDIT: Not doing (3) this as transfer_assets is a blacklisted call:
3. Deactivate calls that can use Kusama as reserve: Throw error on transfer_assets and (limited)reserve_transfer_assets by using latest version on pallet_xcm (updated to latest commit of stable2412)_

For XTokens deactivate calls that use Kusama as reserve: configure ReserveProvider to return None during migration, and AssetHub (Para 1000) after migration

Note:
Because of this issue in yamux, I am using a patched version

EDIT: Aslo updated e2e-tests PR

ashutoshvarma

Did we test it with chopsticks?

Cargo.toml

ashutoshvarma · 2025-09-02T01:09:55Z

bin/collator/src/parachain/service.rs

-    let default_backend = parachain_config.chain_spec.default_network_backend();
-    // If the network backend is unspecified, use the default for the given chain.
-    let network_backend = parachain_config
-        .network
-        .network_backend
-        .unwrap_or(default_backend);
+    let network_backend = parachain_config.network.network_backend;


I didn't check the stable2412 patch releases diff, but is there any other noteworthy changes other than this?

ashutoshvarma · 2025-09-02T01:51:19Z

primitives/src/xcm/mod.rs

+        if reserve_location.parents == 1
+            && !matches!(reserve_location.first_interior(), Some(Parachain(_)))
+        {
+            // DOT/KSM token is not allowed to be migrated to sibling parachain as it will use relay as reserve
+            // update this to `Some(Location::new(1, [Parachain(ASSET_HUB_PARA_ID)]))` when migration is done
+            return None;


I maybe misunderstanding something, the "if" condition check for a parachain reserve not relay, why?

I guess you miss the ! in !matches!(reserve_location.first_interior(), Some(Parachain(_)))
It checks that it is the relay token (parent:1 interior: None) and not some Parachain token

Dinonard

Not sure I understand about the pallet calls that will be disabled though.

Checking the docs provided by Parity, it relates to reserve transfer which is only used for transferring tokens for which Astar/Shiden are reserve.

Also, about the solution/approach - perhaps a more sound one would be to disable relay chain as valid reserve via a dedicated call, i.e. by changing some value? My only concern is the requirement for timing the runtime upgrade.

Dinonard · 2025-09-02T09:59:52Z

primitives/src/xcm/mod.rs

+/// addition will convert self absolute location to relative location.
+/// This struct will ensure that during (and only during) asset migration, no DOT/KSM token will get stuck
+/// on relay chain.
+pub struct AbsoluteAndRelativeReserveProviderAssetHubMigration<AbsoluteLocation>(


Is this only for migration though?

Seems like something that will be used going forward, and the other one will be deprecated.

This definition comment and the comment in lines below:

// DOT/KSM token is not allowed to be migrated to sibling parachain as it will use relay as reserve
// update this to Some(Location::new(1, [Parachain(ASSET_HUB_PARA_ID)])) when migration is done

Are indeed opposite 👍

So I'll suffix it with Shiden
So it will be None during migration and Para 1000 after migration

Then for Astar I'll use AbsoluteAndRelativeReserveProviderAstrar and use it for None

When both Shiden and Astar are migrated I will use AbsoluteAndRelativeReserveProvider back that send Para 1000 for KSM/DOT, and remove AbsoluteAndRelativeReserveProviderSiden and AbsoluteAndRelativeReserveProviderAstar temp structs

PierreOssun · 2025-09-02T12:37:32Z

@Dinonard

Checking the docs provided by Parity, it relates to reserve transfer which is only used for transferring tokens for which Astar/Shiden are reserve.

Yes the docs are not clear enough agree. But this message is more detailed. The issue is DOT/KSM transfers between 2 Paras. Also check this test

Also, about the solution/approach - perhaps a more sound one would be to disable relay chain as valid reserve via a dedicated call, i.e. by changing some value?

For xTokens, assetLocation cannot be None (for during the migration) as changing it to Para 1000 will (mistakenly) send the token via UMP/DMP (same issue as above). So this is None only during the migration

Also during migration it will still be possible to use XCM using transfer_assets_using_type_and_then so not ever "possibility" is blocked.

PierreOssun · 2025-09-02T13:35:50Z

@ashutoshvarma I haven't specifically tested with chopsticks. However I've tested it with e2e-tests

Dinonard · 2025-09-02T14:21:06Z

@PierreOssun

Yes the docs are not clear enough agree. But this message is more detailed. The issue is DOT/KSM transfers between 2 Paras. Also check this test

I see, but it's for the transfer_asset call which we don't even support.
Try copying & running that test in our XCM simulator context, I believe the call won't even work since it's blacklisted.

We should only need to block xtokens.

For xTokens, assetLocation cannot be None (for during the migration) as changing it to Para 1000 will (mistakenly) send the token via UMP/DMP (same issue as above). So this is None only during the migration

Also during migration it will still be possible to use XCM using transfer_assets_using_type_and_then so not ever "possibility" is blocked.

I see, and your idea is fine.
But the execution of this would require 2 runtime upgrades, right?

Dinonard

Looking at the docs for migration, I think I can phrase my concern now properly. :)

Do we really need to apply these changes to the runtime?

Asking this since I thought the flow would be something like:

apply runtime-1700
AH migration happens
disable relay chain as reserve (in some future runtime upgrade)

The rest should be handled offchain, by the UI & apps.
I might be missing something, but don't see the need to handle this via 2 separate runtime upgrades.

PierreOssun · 2025-09-02T15:30:28Z

@Dinonard

I see, but it's for the transfer_asset call which we don't even support.

I haven't considered it might be unsuported. This simplify a lot of things. I'll revert the pallet_xcm uplift 👍

But it remains for Xtokens though.

I also planned to have the flow as you described. It's because of the bug/issue that we need to have this intermediary runtime upgrade.

github-actions · 2025-09-02T16:13:23Z

Package	Line Rate	Branch Rate	Health
pallets/dapp-staking/src	80%	0%	✔
pallets/static-price-provider/src	91%	0%	✔
pallets/unified-accounts/src	79%	0%	➖
chain-extensions/types/assets/src	0%	0%	❌
chain-extensions/unified-accounts/src	0%	0%	❌
pallets/collective-proxy/src	94%	0%	✔
precompiles/xcm/src	69%	0%	➖
precompiles/dapp-staking/src	89%	0%	✔
precompiles/dispatch-lockdrop/src	83%	0%	✔
pallets/democracy-mbm/src	30%	0%	❌
precompiles/unified-accounts/src	100%	0%	✔
pallets/inflation/src	84%	0%	✔
pallets/xc-asset-config/src	62%	0%	➖
pallets/ethereum-checked/src	76%	0%	➖
pallets/collator-selection/src	89%	0%	✔
precompiles/assets-erc20/src	77%	0%	➖
primitives/src/xcm	57%	0%	➖
primitives/src	52%	0%	➖
pallets/dapp-staking/rpc/runtime-api/src	0%	0%	❌
pallets/astar-xcm-benchmarks/src	86%	0%	✔
pallets/dapp-staking/src/benchmarking	95%	0%	✔
chain-extensions/pallet-assets/src	54%	0%	➖
pallets/dapp-staking/src/test	0%	0%	❌
pallets/astar-xcm-benchmarks/src/fungible	100%	0%	✔
pallets/price-aggregator/src	75%	0%	➖
precompiles/dapp-staking/src/test	0%	0%	❌
precompiles/sr25519/src	56%	0%	➖
pallets/vesting-mbm/src	87%	0%	✔
pallets/astar-xcm-benchmarks/src/generic	100%	0%	✔
precompiles/substrate-ecdsa/src	67%	0%	➖
chain-extensions/types/unified-accounts/src	0%	0%	❌
pallets/dynamic-evm-base-fee/src	82%	0%	✔
Summary	74% (3788 / 5129)	0% (0 / 0)	➖

Minimum allowed line rate is 50%

Dinonard · 2025-09-03T06:42:11Z

I also planned to have the flow as you described. It's because of the bug/issue that we need to have this intermediary runtime upgrade.

I understand your point, but still, even if this happens, it's because someone executed this call.
Personally I'm more in favor of not handling this in the runtime, but won't push this anymore.

Instead, can we adjust your solution so that the AbsoluteAndRelativeReserveProvider is implemented in such a way that we can use governance call to trigger the migration mode, and to notify that it's been finished?

With that approach, both Shiden and Astar runtime code updates can be handled immediately.
Chain upgrade can be done before the migration starts (e.g. even 2 weeks before), and we can use governance call to notify the reserve provider that migration is ongoing and later on that it is finished. No need for consecutive runtime upgrades using that approach.

EDIT: This can also be tested using the integration testing.

Dinonard

Can you test this using integration test?

Also, since this new read has been introduced, we should account for the weight somewhere. Of course, it's not the end of the world if we ignore it since it's just one read and we'll remove it later, but in that case please add a comment somewhere that it's ignored on purpose.

Dinonard · 2025-09-04T06:05:34Z

primitives/src/xcm/mod.rs

+pub struct ReserveAssetFilter<T>(PhantomData<T>);
+impl<T> ContainsPair<Asset, Location> for ReserveAssetFilter<T>
+where
+    T: pallet_xc_asset_config::Config,


I understand this is temporary, but we should avoid making the entire pallet a dependency here. Instead, it could be handled by a simpler trait, like Get<MigrationStep> which the pallet can then implement.

Dinonard · 2025-09-04T06:07:26Z

primitives/src/xcm/mod.rs

+            && !matches!(reserve_location.first_interior(), Some(Parachain(_)));
+
+        // KSM/DOT token is not allowed to be transferred to sibling parachain as it will use relay as reserve during migration
+        if is_relay_token && asset_hub_migration_step == MigrationStep::Ongoing {


Just a suggestion, but you'd probably get better readability if you handle migration steps using match.

PierreOssun added 2 commits August 29, 2025 16:43

Prep for Shiden AssetHub migration

dad9dee

revert

f72bc13

PierreOssun added the shiden related to shiden runtime label Aug 29, 2025

PierreOssun mentioned this pull request Aug 29, 2025

[Do not merge] Shiden AssetHub migration AstarNetwork/e2e-tests#147

Open

PierreOssun added the runtime This PR/Issue is related to the topic “runtime”. label Aug 29, 2025

PierreOssun added 2 commits September 1, 2025 14:00

revert rust version bump & use patched yamux dep

ee3f34f

taplo

e90a90a

PierreOssun requested review from ashutoshvarma, Dinonard, ipapandinas and ermalkaleci September 1, 2025 12:02

ashutoshvarma reviewed Sep 2, 2025

View reviewed changes

PR comments

649e18f

Dinonard reviewed Sep 2, 2025

View reviewed changes

PR comments

0fa3965

Dinonard reviewed Sep 2, 2025

View reviewed changes

revert pallet_xcm uplift

be42abd

PierreOssun added 2 commits September 3, 2025 17:19

PR comment: use AssetHubMigrationStep in filters

0cc6f74

fix test build

fb860b7

Dinonard reviewed Sep 4, 2025

View reviewed changes

PR comments

9b13ec1

Prep for Shiden AssetHub migration #1533

Are you sure you want to change the base?

Prep for Shiden AssetHub migration #1533

Conversation

PierreOssun commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Summary

Uh oh!

ashutoshvarma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ashutoshvarma Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

ashutoshvarma Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PierreOssun Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

Dinonard left a comment

Choose a reason for hiding this comment

Uh oh!

Dinonard Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

PierreOssun Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

PierreOssun commented Sep 2, 2025

Uh oh!

PierreOssun commented Sep 2, 2025

Uh oh!

Dinonard commented Sep 2, 2025

Uh oh!

Dinonard left a comment

Choose a reason for hiding this comment

Uh oh!

PierreOssun commented Sep 2, 2025

Uh oh!

github-actions bot commented Sep 2, 2025

Uh oh!

Dinonard commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dinonard left a comment

Choose a reason for hiding this comment

Uh oh!

Dinonard Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Dinonard Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PierreOssun commented Aug 29, 2025 •

edited

Loading

ashutoshvarma Sep 2, 2025 •

edited

Loading

Dinonard commented Sep 3, 2025 •

edited

Loading