-
Notifications
You must be signed in to change notification settings - Fork 141
[AHM] Disable indirect XCMs from RC to AH and vice versa during migration #774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
080251e
778614b
93ab9df
aef0086
a8acf16
7112444
ffb8938
e4c42ef
9f3c7a4
a2b9c62
a76e7ee
77c1968
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,224 @@ | ||
| // Copyright (C) Parity Technologies (UK) Ltd. | ||
| // This file is part of Polkadot. | ||
|
|
||
| // Polkadot is free software: you can redistribute it and/or modify | ||
| // it under the terms of the GNU General Public License as published by | ||
| // the Free Software Foundation, either version 3 of the License, or | ||
| // (at your option) any later version. | ||
|
|
||
| // Polkadot is distributed in the hope that it will be useful, | ||
| // but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| // GNU General Public License for more details. | ||
|
|
||
| // You should have received a copy of the GNU General Public License | ||
| // along with Polkadot. If not, see <http://www.gnu.org/licenses/>. | ||
|
|
||
| use crate::porting_prelude::*; | ||
| use asset_hub_polkadot_runtime::{ | ||
| xcm_config::XcmRouter as AhXcmRouter, BuildStorage, ParachainSystem as AhParachainSystem, | ||
| }; | ||
| use codec::Encode; | ||
| use cumulus_primitives_core::send_xcm; | ||
| use pallet_ah_migrator::{ | ||
| AhMigrationStage as AhMigrationStageStorage, MigrationStage as AhMigrationStage, | ||
| }; | ||
| use pallet_rc_migrator::{ | ||
| MigrationStage as RcMigrationStage, RcMigrationStage as RcMigrationStageStorage, | ||
| }; | ||
| use polkadot_runtime::xcm_config::XcmRouter as RcXcmRouter; | ||
| use sp_runtime::traits::Dispatchable; | ||
| use xcm::prelude::*; | ||
|
|
||
| #[test] | ||
| fn test_send_to_rc_from_ah() { | ||
| let mut t: sp_io::TestExternalities = frame_system::GenesisConfig::<AhRuntime>::default() | ||
| .build_storage() | ||
| .unwrap() | ||
| .into(); | ||
|
|
||
| // our universal xcm message to send to the RC | ||
| let xcm_message = Xcm(vec![ | ||
| Instruction::UnpaidExecution { weight_limit: WeightLimit::Unlimited, check_origin: None }, | ||
| Instruction::Transact { | ||
| origin_kind: OriginKind::Xcm, | ||
| require_weight_at_most: Weight::from_parts(1_000_000_000, 10_000), | ||
| call: AhRuntimeCall::System(frame_system::Call::remark { remark: vec![1] }) | ||
| .encode() | ||
| .into(), | ||
| }, | ||
| ]); | ||
|
|
||
| // prepare the AH to send XCM messages to RC and Collectives. | ||
| t.execute_with(|| { | ||
| let now = 1; | ||
| frame_system::Pallet::<AhRuntime>::reset_events(); | ||
| frame_system::Pallet::<AhRuntime>::set_block_number(now); | ||
|
|
||
| // setup default XCM version | ||
| let result = | ||
| AhRuntimeCall::PolkadotXcm(pallet_xcm::Call::<AhRuntime>::force_default_xcm_version { | ||
| maybe_xcm_version: Some(xcm::prelude::XCM_VERSION), | ||
| }) | ||
| .dispatch(AhRuntimeOrigin::root()); | ||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
|
|
||
| // open the channel between AH and Collectives (1001) | ||
| AhParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests(1001.into()); | ||
| }); | ||
|
|
||
| // sending XCM messages via main `XcmRouter` from AH to RC and AH to Collectives succeeds | ||
| // while migration is pending. | ||
| t.execute_with(|| { | ||
| let now = 2; | ||
| frame_system::Pallet::<AhRuntime>::reset_events(); | ||
| frame_system::Pallet::<AhRuntime>::set_block_number(now); | ||
|
|
||
| AhMigrationStageStorage::<AhRuntime>::put(AhMigrationStage::Pending); | ||
|
|
||
| let dest = Location::parent(); | ||
| let result = send_xcm::<AhXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok()); | ||
|
|
||
| let dest = Location::new(1, Parachain(1001)); | ||
| let result = send_xcm::<AhXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
| }); | ||
|
|
||
| // sending XCM messages via main `XcmRouter` fails from AH to RC but succeeds from AH to | ||
| // Collectives while migration is ongoing. | ||
| t.execute_with(|| { | ||
| let now = 2; | ||
| frame_system::Pallet::<AhRuntime>::reset_events(); | ||
| frame_system::Pallet::<AhRuntime>::set_block_number(now); | ||
|
|
||
| AhMigrationStageStorage::<AhRuntime>::put(AhMigrationStage::DataMigrationOngoing); | ||
|
|
||
| let dest = Location::parent(); | ||
| let err = send_xcm::<AhXcmRouter>(dest, xcm_message.clone()).unwrap_err(); | ||
|
|
||
| assert_eq!(err, SendError::Unroutable); | ||
|
|
||
| let dest = Location::new(1, Parachain(1001)); | ||
| let result = send_xcm::<AhXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
| }); | ||
|
|
||
| // sending XCM messages via main `XcmRouter` from AH to RC and AH to Collectives succeeds | ||
| // while migration is done. | ||
| t.execute_with(|| { | ||
| let now = 2; | ||
| frame_system::Pallet::<AhRuntime>::reset_events(); | ||
| frame_system::Pallet::<AhRuntime>::set_block_number(now); | ||
|
|
||
| AhMigrationStageStorage::<AhRuntime>::put(AhMigrationStage::MigrationDone); | ||
|
|
||
| let dest = Location::parent(); | ||
| let result = send_xcm::<AhXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
|
|
||
| let dest = Location::new(1, Parachain(1001)); | ||
| let result = send_xcm::<AhXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
| }); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_send_to_ah_from_rc() { | ||
| let mut t: sp_io::TestExternalities = frame_system::GenesisConfig::<RcRuntime>::default() | ||
| .build_storage() | ||
| .unwrap() | ||
| .into(); | ||
|
|
||
| // our universal xcm message to send to the RC | ||
| let xcm_message = Xcm(vec![ | ||
| Instruction::UnpaidExecution { weight_limit: WeightLimit::Unlimited, check_origin: None }, | ||
| Instruction::Transact { | ||
| origin_kind: OriginKind::Xcm, | ||
| require_weight_at_most: Weight::from_parts(1_000_000_000, 10_000), | ||
| call: RcRuntimeCall::System(frame_system::Call::remark { remark: vec![1] }) | ||
| .encode() | ||
| .into(), | ||
| }, | ||
| ]); | ||
|
|
||
| // prepare the RC to send XCM messages to AH and Collectives. | ||
| t.execute_with(|| { | ||
| let now = 1; | ||
| frame_system::Pallet::<RcRuntime>::reset_events(); | ||
| frame_system::Pallet::<RcRuntime>::set_block_number(now); | ||
|
|
||
| // setup default XCM version | ||
| let result = | ||
| RcRuntimeCall::XcmPallet(pallet_xcm::Call::<RcRuntime>::force_default_xcm_version { | ||
| maybe_xcm_version: Some(xcm::prelude::XCM_VERSION), | ||
| }) | ||
| .dispatch(RcRuntimeOrigin::root()); | ||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
| }); | ||
|
|
||
| // sending XCM messages via main `XcmRouter` from RC to AH and RC to Collectives succeeds | ||
| // while migration is pending. | ||
| t.execute_with(|| { | ||
| let now = 2; | ||
| frame_system::Pallet::<RcRuntime>::reset_events(); | ||
| frame_system::Pallet::<RcRuntime>::set_block_number(now); | ||
|
|
||
| RcMigrationStageStorage::<RcRuntime>::put(RcMigrationStage::Pending); | ||
|
|
||
| let dest = Location::new(0, Parachain(1000)); | ||
| let result = send_xcm::<RcXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
|
|
||
| let dest = Location::new(0, Parachain(1001)); | ||
| let result = send_xcm::<RcXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
| }); | ||
|
|
||
| // sending XCM messages via main `XcmRouter` fails from RC to AH but succeeds from RC to | ||
| // Collectives while migration is ongoing. | ||
| t.execute_with(|| { | ||
| let now = 2; | ||
| frame_system::Pallet::<RcRuntime>::reset_events(); | ||
| frame_system::Pallet::<RcRuntime>::set_block_number(now); | ||
|
|
||
| RcMigrationStageStorage::<RcRuntime>::put(RcMigrationStage::AccountsMigrationInit); | ||
|
|
||
| let dest = Location::new(0, Parachain(1000)); | ||
| let err = send_xcm::<RcXcmRouter>(dest, xcm_message.clone()).unwrap_err(); | ||
|
|
||
| assert_eq!(err, SendError::Unroutable); | ||
|
|
||
| let dest = Location::new(0, Parachain(1001)); | ||
| let result = send_xcm::<RcXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
| }); | ||
|
|
||
| // sending XCM messages via main `XcmRouter` from RC to AH and RC to Collectives succeeds | ||
| // while migration is done. | ||
| t.execute_with(|| { | ||
| let now = 2; | ||
| frame_system::Pallet::<RcRuntime>::reset_events(); | ||
| frame_system::Pallet::<RcRuntime>::set_block_number(now); | ||
|
|
||
| RcMigrationStageStorage::<RcRuntime>::put(RcMigrationStage::MigrationDone); | ||
|
|
||
| let dest = Location::new(0, Parachain(1000)); | ||
| let result = send_xcm::<RcXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
|
|
||
| let dest = Location::new(0, Parachain(1001)); | ||
| let result = send_xcm::<RcXcmRouter>(dest, xcm_message.clone()); | ||
|
|
||
| assert!(result.is_ok(), "fails with error: {:?}", result.err()); | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,7 +137,7 @@ pub fn call_allowed_status(call: &<Runtime as frame_system::Config>::RuntimeCall | |
| Coretime(coretime::Call::<Runtime>::request_revenue_at { .. }) => (OFF, ON), | ||
| Coretime(..) => (ON, ON), | ||
| StateTrieMigration(..) => (OFF, OFF), // Deprecated | ||
| XcmPallet(..) => (OFF, ON), /* TODO allow para origins and root to call this during the migration, see https://github.com/polkadot-fellows/runtimes/pull/559#discussion_r1928789463 */ | ||
| XcmPallet(..) => (OFF, ON), | ||
|
||
| MessageQueue(..) => (ON, ON), // TODO think about this | ||
| AssetRate(..) => (OFF, OFF), | ||
| Beefy(..) => (OFF, ON), /* TODO @claravanstaden @bkontur */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1558,7 +1558,7 @@ impl pallet_rc_migrator::Config for Runtime { | |
| >; | ||
| type Currency = Balances; | ||
| type CheckingAccount = xcm_config::CheckAccount; | ||
| type SendXcm = xcm_config::XcmRouter; | ||
| type SendXcm = xcm_config::XcmRouterWithoutException; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need a special one here on the relay? I thought we want to use the one that disables during migration.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bc its the only one that can send to AH and we need it for the migrator pallet to send data. the rest cannot. |
||
| type MaxRcWeight = RcMigratorMaxWeight; | ||
| type MaxAhWeight = AhMigratorMaxWeight; | ||
| type AhExistentialDeposit = AhExistentialDeposit; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1159,7 +1159,7 @@ impl pallet_ah_migrator::Config for Runtime { | |||||||||
| type RcPalletsOrigin = ah_migration::RcPalletsOrigin; | ||||||||||
| type RcToAhPalletsOrigin = ah_migration::RcToAhPalletsOrigin; | ||||||||||
| type Preimage = Preimage; | ||||||||||
| type SendXcm = xcm_config::XcmRouter; | ||||||||||
| type SendXcm = xcm_config::LocalXcmRouterWithoutException; | ||||||||||
|
||||||||||
| type SendXcm = xcm_config::LocalXcmRouterWithoutException; | |
| type SendXcm = WithUniqueTopic< | |
| cumulus_primitives_utility::ParentAsUmp<ParachainSystem, PolkadotXcm, PriceForParentDelivery>, | |
| >; |
But the other thing - the XcmRouter was previously wrapped with WithUniqueTopic, but whatever we are adding here isn't. That means the SetTopic feature for tracking XCM messages won't apply probably.
Maybe we don’t need to track XCM messages at all. I need to go through the entire flow starting from the initial migration message (first-tracking-id). Most likely, the up-and-down migration messages won’t be tracked under the same first-tracking-id due to the query/response behavior (also need to check).
Anyway, just wanted to raise the point: without WithUniqueTopic, we most probably lose the ability to track messages using the SetTopic feature from the AssetHub migrator to the RC (maybe not that big issue also).
cc: @raymondkfcheung maybe, it could be worth to double-check all the XCM routing set it up for this migration, when everyhting will be ready (all similar PRs are merged to this dev-ahm branch).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I usually try to introduce as little entropy as possible. The LocalXcmRouterWithoutException type is exactly the same as the one currently used (prior to this PR); only the name has been changed. This means I do not introduce any behaviour change to the migrator pallets with this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means I do not introduce any behaviour change to the migrator pallets with this PR.
Well, using LocalXcmRouterWithoutException instead of XcmRouter avoids using SetTopic with WithUniqueTopic - which may not be relevant behaviour change for migration. As I said before, just raising the point, we can go with or without WithUniqueTopic probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now. I was trying to avoid touching the ToKusamaXcmRouter and SovereignPaidRemoteExporter<...> routers, this sounds good to you? I am adding now the SetTopic for the migrator router.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have to whitelist query responses for our flow control system
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
querierhere from the perspective of thedestination