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

bin/collator/src/parachain/service.rs

primitives/src/xcm/mod.rs

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.

primitives/src/xcm/mod.rs

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.

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.

tests/xcm-simulator/src/tests/fungible_assets.rs

ipapandinas

The approach of deactivating Xtokens calls using KSM reserves LGTM. Thank you for the testing effort!

PierreOssun · 2025-09-05T13:10:27Z

/bench astar pallet_xc_asset_config

github-actions · 2025-09-05T13:56:30Z

Benchmarks job is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/17494132837.
Please wait for a while.
Branch: feat/shiden-ah-migration-prep
SHA: 2b3a8f5

github-actions · 2025-09-05T14:17:39Z

Benchmarks have been finished.
You can download artifacts if exists https://github.com/AstarNetwork/Astar/actions/runs/17494132837.

ipapandinas · 2025-09-05T15:10:27Z

pallets/xc-asset-config/src/weights.rs

+		//  Estimated: `0`
+		// Minimum execution time: 13_646_000 picoseconds.
+		Weight::from_parts(13_783_000, 0)
+			.saturating_add(Weight::from_parts(0, 0))


The proof size is 0 now with frame-omni-bencher v1, I am not sure if this is ok. I think it's because in the --json-input they are empty.

Dinonard · 2025-09-12T08:15:10Z

The benchmark issue has now been fixed in the latest master, rerunning them should give proper PoV results.

…en-ah-migration-prep

PierreOssun · 2025-09-23T13:01:52Z

/bench astar pallet_xc_asset_config

github-actions · 2025-09-23T14:17:41Z

Benchmarks job is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/17947016025.
Please wait for a while.
Branch: feat/shiden-ah-migration-prep
SHA: 2f70b5d

github-actions · 2025-09-23T14:39:17Z

Benchmarks have been finished.
You can download artifacts if exists https://github.com/AstarNetwork/Astar/actions/runs/17947016025.

github-actions · 2025-09-23T16:03:48Z

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

Minimum allowed line rate is 50%

jxs · 2025-10-01T15:50:18Z

Hi, yamux 0.13.7 has been published and fixes this.
Thanks!

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

Closed

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 Dinonard, ashutoshvarma, ermalkaleci and ipapandinas September 1, 2025 12:02

ashutoshvarma reviewed Sep 2, 2025

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

bin/collator/src/parachain/service.rs Outdated Show resolved Hide resolved

primitives/src/xcm/mod.rs Outdated Show resolved Hide resolved

PR comments

649e18f

Dinonard reviewed Sep 2, 2025

View reviewed changes

primitives/src/xcm/mod.rs Outdated Show resolved Hide resolved

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

PierreOssun added 4 commits September 4, 2025 11:26

PR comments

9b13ec1

fixed test build

92a9b60

taplo

b17027a

Added tests

41e25c4

Dinonard previously approved these changes Sep 5, 2025

View reviewed changes

tests/xcm-simulator/src/tests/fungible_assets.rs Show resolved Hide resolved

ipapandinas previously approved these changes Sep 5, 2025

View reviewed changes

fixed reserves

2b3a8f5

PierreOssun dismissed stale reviews from ipapandinas and Dinonard via 2b3a8f5 September 5, 2025 13:02

updated weights

bfa7314

Dinonard previously approved these changes Sep 5, 2025

View reviewed changes

ipapandinas reviewed Sep 5, 2025

View reviewed changes

fixed weights build

88efe6e

PierreOssun dismissed Dinonard’s stale review via 88efe6e September 5, 2025 16:03

ipapandinas previously approved these changes Sep 5, 2025

View reviewed changes

Merge branch 'master' of github.com:AstarNetwork/Astar into feat/shid…

2f70b5d

…en-ah-migration-prep

fixed weights

c52c4fb

PierreOssun dismissed ipapandinas’s stale review via c52c4fb September 23, 2025 15:36

ipapandinas approved these changes Sep 25, 2025

View reviewed changes

Dinonard approved these changes Sep 25, 2025

View reviewed changes

PierreOssun merged commit c729a21 into master Sep 26, 2025
8 checks passed

PierreOssun deleted the feat/shiden-ah-migration-prep branch September 26, 2025 14:58

Prep for Shiden AssetHub migration #1533

Prep for Shiden AssetHub migration #1533

Uh oh!

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!

Uh oh!

Uh oh!

Dinonard left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

ipapandinas left a comment • 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 commented Sep 5, 2025

Uh oh!

github-actions bot commented Sep 5, 2025

Uh oh!

github-actions bot commented Sep 5, 2025

Uh oh!

ipapandinas Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

Dinonard commented Sep 12, 2025

Uh oh!

PierreOssun commented Sep 23, 2025

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

Uh oh!

jxs commented Oct 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

PierreOssun commented Aug 29, 2025 •

edited

Loading

Dinonard commented Sep 3, 2025 •

edited

Loading

ipapandinas left a comment •

edited

Loading