Refactor XCM tests#3685
Open
manuelmauro wants to merge 71 commits intomasterfrom
Open
Conversation
manuelmauro
added
B0-silent
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces xcm-simulator with xcm-emulator in dev-dependencies, deletes the legacy xcm_mock test harness, and adds extensive new XCM unit tests and xcm-emulator integration tests/utilities across moonbase/moonbeam/moonriver; also updates Cargo.toml entries and adds Changes
Sequence Diagram(s)sequenceDiagram
participant Para as Parachain
participant Relay as Relay (Westend)
participant Moon as Moonbase/Moonbeam
participant Asset as AssetHub
rect rgba(200,200,255,0.5)
Para->>Relay: UMP/DMP: transfer_assets / limited_teleport_assets
Relay->>Asset: process reserve/teleport to AssetHub
Relay->>Moon: update relay indices / HRMP registration
end
rect rgba(200,255,200,0.5)
Para->>Moon: XCMP/HRMP: WithdrawAsset -> BuyExecution -> DepositAsset
Moon->>Para: QueryResponse / version discovery
Moon->>Relay: optional UMP for relay‑mediated flows
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
- Remove incorrect doc comment on register_dot_asset - Use 'should' phrasing in expect() per Rust guidelines - Switch remark to remark_with_event and verify Remarked event on relay - Encode calls via RuntimeCall instead of manual pallet-index assembly - Assert OpenChannelRequested/Accepted events and channel storage in HRMP test - Clean up unused PalletInfoT import
Keep the pre-existing xcm_tests.rs and xcm_mock/ from master as a separate test binary so the old suite runs in parallel with the new xcm_config_tests, xcm_emulator_tests, and xcm_integration_tests. This allows splitting the refactoring across multiple PRs.
Same as the moonbeam restore — keep the pre-existing xcm_tests.rs and xcm_mock/ from master for moonbase (47 tests) and moonriver (47 tests) so they run alongside the new test suites.
New transact tests (emulator_transact_tests.rs): - transact_through_sovereign: fee_payer=None, custom_fee_weight, refund - transact_through_derivative: basic, custom_fee_weight, refund - transact_through_signed: basic, custom_fee_weight, refund - hrmp_close_via_xcm_transactor: close a previously opened channel New transfer tests (emulator_transfer_tests.rs): - evm_account_receives_foreign_asset: EVM account with GLMR receives DOT - foreign_assets_survive_native_balance_drain: DOT survives GLMR drain Also: - Extract relay_remark_call(), assert_relay_remark_executed(), relay_derived_account() and setup_derivative() helpers to reduce duplication - Add ConvertLocation import for relay account derivation - Use real Westend LocationConverter for signed origin account derivation (Account32Hash != HashedDescription<DescribeFamily>)
Two new tests covering HRMP-based transact from Moonbeam to sibling: - transact_through_signed_para_to_para: basic remark execution - transact_through_signed_para_to_para_refund: with fee refund Introduces setup_para_to_para_signed() helper that: - Opens bidirectional HRMP channels - Registers DOT on the sibling chain - Derives the signed-origin account on sibling (HashedDescription) - Funds the derived account with DOT via relay DMP
Three new tests covering EthereumXcm operations via signed XCM transact: - transact_through_signed_para_to_para_ethereum: EVM value transfer via EthereumXcm::transact (MoonbeamCall dispatcher re-routes Signed origin to XcmEthereumTransaction origin) - transact_through_signed_para_to_para_ethereum_no_proxy_fails: proxy transact without proxy setup — verifies no balance change - transact_through_signed_para_to_para_ethereum_proxy_succeeds: proxy transact with ALITH delegating to derived account Introduces setup_para_to_para_ethereum() and ethereum_xcm_transfer_call() helpers.
Three new tests covering Moonbeam's native token (GLMR) transfers to sibling: - transfer_glmr_from_moonbeam_to_sibling: reserve-backed GLMR transfer - transfer_glmr_roundtrip_moonbeam_sibling: GLMR → sibling → back, verifies only fees are lost in the roundtrip - transfer_glmr_to_sibling_with_trader_fees: verifies sibling's XCM weight trader deducts GLMR fees and treasury collects them Introduces register_glmr_on_sibling() and setup_glmr_para_to_para() helpers.
Documents test inventory, emulator network topology, technical discoveries, deferred work, and known issues for future reference when splitting changes across PRs.
- Fix barrier_passes_unpaid_with_weight_credit by supplying weight credit via new execute_xcm_with_credit() helper - Add 5 barrier tests: AccountKey20 paid execution, unpaid rejection, known/unknown query response - Add 2 reserve tests: DOT from relay, teleport always rejected - Add 2 transactor tests: foreign asset withdraw, ERC20 bridge asset - Remove unused ONE_DOT constant from traders_test (dead code warning) - Add xcm_config_tests/COVERAGE.md documenting all 48 tests - Apply all fixes to moonbeam, moonbase, and moonriver runtimes
Add 7 new emulator tests covering previously-deferred scenarios: Transfer tests (emulator_transfer_tests.rs, 11→16): - transfer_glmr_across_three_chains: GLMR traverses A→B→A→C via 3rd parachain (ParaCPara, para 2006) - transfer_dot_to_sibling_via_remote_reserve: DOT via RemoteReserve through relay - transfer_dot_roundtrip_via_remote_reserve: DOT outbound + return - transfer_glmr_self_reserve_to_sibling: GLMR self-reserve transfer - receive_sibling_native_asset: sibling sends its native to Moonbeam Versioning tests (new emulator_versioning_tests.rs, 2 tests): - xcm_version_discovery_with_relay: SafeXcmVersion + relay awareness - xcm_version_discovery_with_sibling: SafeXcmVersion + sibling awareness Infrastructure: - Add ParaCPara (para 2006) to emulator network - Register para 2006 in relay genesis (head, lifecycle, sovereign) - Add para_c_execute_with() helper - Update COVERAGE.md (28 → 35 tests)
…-tests # Conflicts: # Cargo.lock
With PR #3692 removing `runtime-benchmarks` from the CI test command, the `force-xcm-processor` workaround is no longer needed. Tests run without benchmarks, so both Westend and Moonbeam use the real `MessageProcessor` by default. Removed: - `[patch]` section from root Cargo.toml (~200 local path overrides) - `force-xcm-processor` feature from runtime/moonbeam/Cargo.toml - `force-xcm-processor` cfg guards from runtime/moonbeam/src/xcm_config.rs All polkadot-sdk crates now resolve from the upstream moonbeam-polkadot-stable2506 branch.
Add the real asset-hub-westend-runtime as a parachain (para 1000) in the xcm-emulator network, alongside the existing Moonbeam instances. New tests (emulator_asset_hub_tests.rs, 3 tests): - transfer_dot_from_relay_to_asset_hub: relay teleports DOT to real AH - relay_funds_both_asset_hub_and_moonbeam: DOT on both chains from relay - transfer_trust_backed_asset_from_asset_hub_to_moonbeam: USDT (id=1984) from Asset Hub to Moonbeam as an EVM foreign asset Infrastructure: - Add asset-hub-westend-runtime to workspace and moonbeam-runtime deps - Add AssetHubPara declaration with genesis (para 1000) - Register Asset Hub on relay (head, lifecycle, sovereign, endowment) - Add asset_hub_execute_with() helper - Update COVERAGE.md (35 → 38 tests) Note: Direct DOT transfers between Asset Hub and Moonbeam are not supported — DOT uses teleport (relay↔AH) vs reserve (relay↔Moonbeam), a hybrid that pallet_xcm cannot route in a single call. Trust-backed assets (USDT, etc.) work because Asset Hub is their reserve.
ParaCPara (para 2006) was an identical Moonbeam clone used only for the transfer_glmr_across_three_chains test, whose individual legs were already covered by transfer_glmr_from_moonbeam_to_sibling and transfer_glmr_roundtrip_moonbeam_sibling. Asset Hub (para 1000) replaces it as the third chain in the network, providing genuine cross-runtime coverage instead of a duplicate. Removed: - ParaCPara declaration, genesis, relay registration, PARA_C_ID - CHARLETH test account (unused after ParaC removal) - transfer_glmr_across_three_chains test (redundant) - setup_three_chains / register_glmr_foreign_asset helpers Network: 4 chains → 3 chains (Asset Hub + Moonbeam + Sibling) Tests: 38 → 37
Port the full xcm_emulator_tests suite to moonriver and moonbase runtimes, both using westend-runtime as the relay (XCM executor behavior is identical across relay chains; Kusama runtime is not available in the polkadot-sdk fork). Each runtime now has 37 emulator tests covering: - 17 transact tests (sovereign, derivative, signed, HRMP) - 15 transfer tests (DOT, native token, remote reserve, roundtrip) - 3 Asset Hub tests (DOT teleport, USDT trust-backed asset) - 2 versioning tests (relay + sibling discovery) Runtime-specific adaptations: - moonriver: MOVR currency, PalletInstance(10) - moonbase: UNIT currency, PalletInstance(3) All 3 runtimes × 3 suites pass: 48 config + 37 emulator + 32 integration
Remove the legacy XCM test infrastructure (23,525 lines) across all three runtimes. This code used xcm-simulator with hand-rolled mock runtimes and is fully replaced by: - xcm_config_tests (48 tests): unit tests against real XcmConfig - xcm_integration_tests (32 tests): xcm-simulator with real runtime - xcm_emulator_tests (37 tests): xcm-emulator with real relay + parachains Removed per runtime: - tests/xcm_mock/ (mod.rs, parachain.rs, relay_chain.rs, statemint_like.rs) - tests/xcm_tests.rs (~5,400 lines)
…ests
Remove dead code across all 3 runtimes:
- relay_execute_and_dispatch()
- {moonbeam,moonriver,moonbase}_execute_and_dispatch()
- para_ids::ASSET_HUB constant
The relay chain is Westend (12 decimals), so 1_000_000_000_000_000 planck equals 1,000 WND, not 100,000 DOT. Update the inline comment in all three relay.rs files.
…foAccess Replace hardcoded pallet indices in set_westend_relay_indices() with values derived from the Westend runtime using PalletInfoAccess::index(). Assert the derived values match expectations so tests fail fast with a clear message if Westend reorders its pallets after a relay upgrade.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
runtime/moonbase/tests/xcm_emulator/transfers.rs (1)
649-663:⚠️ Potential issue | 🟡 MinorUpdate the copied docs to match
UNITandPalletInstance(3).This helper registers
UNIT_ASSET_IDatPalletInstance(3), but the surrounding docs still mention GLMR andindex 10. The code looks right; the stale comments are now misleading.Suggested fix
-/// Register Moonbase's native GLMR as a foreign asset on the sibling and +/// Register Moonbase's native token (UNIT) as a foreign asset on the sibling and /// configure the XCM weight trader price. fn register_unit_on_sibling() { sibling_execute_with(|| { // From the sibling's perspective, Moonbase's native token lives at: - // ../Parachain(2004)/PalletInstance(3) (pallet_balances = index 10) + // ../Parachain(2004)/PalletInstance(3) (pallet_balances = index 3) let glmr_location = xcm::latest::Location::new(1, [Parachain(MOONBASE_PARA_ID), PalletInstance(3u8)]); @@ - 18, // GLMR has 18 decimals + 18, // UNIT has 18 decimals🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/moonbase/tests/xcm_emulator/transfers.rs` around lines 649 - 663, The block comment above register_unit_on_sibling is stale and mentions GLMR and index 10; update it to correctly describe that this helper registers UNIT_ASSET_ID as the sibling's foreign asset at PalletInstance(3) (pallet index 3) with symbol "UNIT" and 18 decimals so the comment matches the implementation using UNIT_ASSET_ID, PalletInstance(3), and b"UNIT".runtime/moonbase/tests/xcm_emulator/transact.rs (1)
730-771:⚠️ Potential issue | 🟠 MajorTrack the derivative account in the refund assertion.
This test funds the derivative sub-account, but the assertion reads the sovereign balance. If the derivative pays and the sovereign stays flat,
fee_spentbecomes0and the refund check passes without validating anything.Suggested fix
- let sovereign_before = WestendRelay::<PolkadotMoonbeamNet>::execute_with(|| { - let sovereign = WestendRelay::<PolkadotMoonbeamNet>::sovereign_account_id_of( - Location::new(0, [Parachain(MOONBASE_PARA_ID)]), - ); - <westend_runtime::Balances as Inspect<_>>::balance(&sovereign) - }); + let derivative = WestendRelay::<PolkadotMoonbeamNet>::execute_with(|| { + let sovereign = WestendRelay::<PolkadotMoonbeamNet>::sovereign_account_id_of( + Location::new(0, [Parachain(MOONBASE_PARA_ID)]), + ); + pallet_utility::Pallet::<westend_runtime::Runtime>::derivative_account_id( + sovereign, + 0u16, + ) + }); + let derivative_before = WestendRelay::<PolkadotMoonbeamNet>::execute_with(|| { + <westend_runtime::Balances as Inspect<_>>::balance(&derivative) + }); @@ - let sovereign_after = WestendRelay::<PolkadotMoonbeamNet>::execute_with(|| { - let sovereign = WestendRelay::<PolkadotMoonbeamNet>::sovereign_account_id_of( - Location::new(0, [Parachain(MOONBASE_PARA_ID)]), - ); - <westend_runtime::Balances as Inspect<_>>::balance(&sovereign) - }); - let fee_spent = sovereign_before.saturating_sub(sovereign_after); + let derivative_after = WestendRelay::<PolkadotMoonbeamNet>::execute_with(|| { + <westend_runtime::Balances as Inspect<_>>::balance(&derivative) + }); + let fee_spent = derivative_before.saturating_sub(derivative_after); assert!( - fee_spent < ONE_DOT * 20, - "With refund, sovereign should spend less than the full fee: spent={fee_spent}" + fee_spent > 0 && fee_spent < ONE_DOT * 20, + "With refund, derivative should spend some DOT, but less than the full fee: spent={fee_spent}" );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/moonbase/tests/xcm_emulator/transact.rs` around lines 730 - 771, The refund assertion only checks the sovereign balance but the call uses transact_through_derivative, so you must also track the derivative sub-account balance to ensure the fee was actually charged/refunded; before the call capture both balances (use WestendRelay::<PolkadotMoonbeamNet>::sovereign_account_id_of to compute the sovereign and the derivative account id according to the same derivative scheme used by moonbase_runtime::XcmTransactor::transact_through_derivative or any helper like derivative_account_id if available), then after the call read both balances and compute fee_spent as the total decrease across sovereign and derivative (or verify derivative decreased less than the overpaid amount and sovereign increased by the refund), and update the assert to compare that combined fee_spent < ONE_DOT * 20.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@runtime/moonbase/tests/xcm_emulator/transfers.rs`:
- Around line 624-640: The test currently burns via <moonbase_runtime::Balances
as Mutate<_>>::burn_from(...) but never verifies the native free balance was
actually drained; add an assertion using
moonbase_runtime::Balances::free_balance(moonbase_runtime::AccountId::from(test_account))
(or equivalent API) immediately after the burn to ensure the native balance is
zero (or less than/equal to the expected remaining amount) before checking the
foreign asset via moonbase_runtime::EvmForeignAssets::balance(DOT_ASSET_ID,
...); reference the existing symbols Mutate::burn_from, Balances::free_balance,
test_account and EvmForeignAssets::balance to locate where to insert the check.
In `@runtime/moonbeam/tests/xcm_emulator/transfers.rs`:
- Around line 624-640: The test currently calls <moonbeam_runtime::Balances as
Mutate<_>>::burn_from(...) with Precision::BestEffort which may be ignored, so
update the test to explicitly prove the native GLMR balance is zero before
checking the foreign asset: after calling burn_from (or replace it with a
deterministic call like <moonbeam_runtime::Balances as
Mutate<_>>::burn_to_zero(...) or perform a forced transfer/slash via
<moonbeam_runtime::Balances as Mutate<_>>::reduce_balance(...) if available),
fetch the native balance with
<moonbeam_runtime::Balances::free_balance>(moonbeam_runtime::AccountId::from(test_account))
and assert it equals zero (or assert it decreased by the expected amount), then
proceed to call <moonbeam_runtime::EvmForeignAssets::balance>(DOT_ASSET_ID,
AccountId::from(test_account)) and assert the foreign asset remains >
U256::zero().
---
Duplicate comments:
In `@runtime/moonbase/tests/xcm_emulator/transact.rs`:
- Around line 730-771: The refund assertion only checks the sovereign balance
but the call uses transact_through_derivative, so you must also track the
derivative sub-account balance to ensure the fee was actually charged/refunded;
before the call capture both balances (use
WestendRelay::<PolkadotMoonbeamNet>::sovereign_account_id_of to compute the
sovereign and the derivative account id according to the same derivative scheme
used by moonbase_runtime::XcmTransactor::transact_through_derivative or any
helper like derivative_account_id if available), then after the call read both
balances and compute fee_spent as the total decrease across sovereign and
derivative (or verify derivative decreased less than the overpaid amount and
sovereign increased by the refund), and update the assert to compare that
combined fee_spent < ONE_DOT * 20.
In `@runtime/moonbase/tests/xcm_emulator/transfers.rs`:
- Around line 649-663: The block comment above register_unit_on_sibling is stale
and mentions GLMR and index 10; update it to correctly describe that this helper
registers UNIT_ASSET_ID as the sibling's foreign asset at PalletInstance(3)
(pallet index 3) with symbol "UNIT" and 18 decimals so the comment matches the
implementation using UNIT_ASSET_ID, PalletInstance(3), and b"UNIT".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d771d86e-084f-4e3c-9aa3-1e3c2eb91faa
📒 Files selected for processing (5)
runtime/moonbase/tests/xcm_emulator/transact.rsruntime/moonbase/tests/xcm_emulator/transfers.rsruntime/moonbeam/tests/xcm_emulator/transfers.rsruntime/moonriver/tests/xcm_emulator/transact.rsruntime/moonriver/tests/xcm_emulator/transfers.rs
…n test The foreign_assets_survive_native_balance_drain test uses BestEffort precision for burn_from but never verified the drain succeeded. Add an assert_eq on the remaining native balance so the test precondition is explicitly validated before checking foreign asset accessibility.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
runtime/moonbeam/tests/xcm_emulator/network.rs (1)
263-298: Guard the inner Westend call indices too.The pallet indices are now runtime-derived, but
bond,rebond,as_derivative,init_open_channel, etc. are still magic numbers. A Westend upgrade that preserves pallet positions but reorders calls insideStaking,Utility, orHrmpwill bypass these asserts and break the transact tests much later. Consider deriving or at least asserting those inner call indices from the runtime as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/moonbeam/tests/xcm_emulator/network.rs` around lines 263 - 298, The current RelayChainIndices uses hardcoded inner call indices (bond, rebond, as_derivative, init_open_channel, accept_open_channel, close_channel, cancel_open_request) which can drift even if pallet indices stay the same; update the test to derive or assert these inner call indices from the Westend runtime instead of magic numbers: query the Westend runtime Call/metadata for the Staking, Utility and Hrmp call discriminants (e.g. use the runtime Call enums or metadata API for westend_runtime::Staking, westend_runtime::Utility and westend_runtime::Hrmp) and assign/compare those values into RelayChainIndices (or add explicit assert_eq! checks) for the symbols bond, rebond, as_derivative, init_open_channel, accept_open_channel, close_channel and cancel_open_request so the test fails immediately if any inner call index changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@runtime/moonbase/tests/xcm_emulator/network.rs`:
- Line 335: The inline comment for the endowment variable (let endowment: u128 =
1_000_000_000_000_000;) incorrectly states "100k WND" — update the comment to
reflect the correct amount given Westend's 12 decimals ("1,000 WND") so the
comment matches the numeric value; locate the endowment declaration (symbol:
endowment) in network.rs and mirror the fix already made in relay.rs to keep
consistency.
In `@runtime/moonbeam/tests/xcm_emulator/network.rs`:
- Around line 332-356: The comment for the endowment in asset_hub_genesis is
wrong: the literal let endowment: u128 = 1_000_000_000_000_000 is 1_000 WND (12
decimals), not 100k WND — either update the comment to "1_000 WND" or change the
literal to 100_000_000_000_000_000 (100_000 * 10^12) to represent 100k WND; edit
the endowment variable in asset_hub_genesis accordingly so the comment and value
match.
---
Nitpick comments:
In `@runtime/moonbeam/tests/xcm_emulator/network.rs`:
- Around line 263-298: The current RelayChainIndices uses hardcoded inner call
indices (bond, rebond, as_derivative, init_open_channel, accept_open_channel,
close_channel, cancel_open_request) which can drift even if pallet indices stay
the same; update the test to derive or assert these inner call indices from the
Westend runtime instead of magic numbers: query the Westend runtime
Call/metadata for the Staking, Utility and Hrmp call discriminants (e.g. use the
runtime Call enums or metadata API for westend_runtime::Staking,
westend_runtime::Utility and westend_runtime::Hrmp) and assign/compare those
values into RelayChainIndices (or add explicit assert_eq! checks) for the
symbols bond, rebond, as_derivative, init_open_channel, accept_open_channel,
close_channel and cancel_open_request so the test fails immediately if any inner
call index changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7299d4c-b6f7-47b6-8f97-43ce98edadc6
📒 Files selected for processing (6)
runtime/moonbase/tests/xcm_emulator/network.rsruntime/moonbase/tests/xcm_emulator/relay.rsruntime/moonbeam/tests/xcm_emulator/network.rsruntime/moonbeam/tests/xcm_emulator/relay.rsruntime/moonriver/tests/xcm_emulator/network.rsruntime/moonriver/tests/xcm_emulator/relay.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- runtime/moonriver/tests/xcm_emulator/relay.rs
- runtime/moonriver/tests/xcm_emulator/network.rs
The comment said 'pallet_balances = index 10' but the code uses PalletInstance(3u8), matching Moonbase's construct_runtime ordering. Update the comment to say 'index 3'.
The comment said '100k WND' but 1_000_000_000_000_000 planck with 12 decimals is 1,000 WND. Update the comment in all three network.rs files.
Remove stale bp_moonriver::GlobalConsensusLocation reference from the moonbase reserves doc (moonbase has no cross-chain bridge entry). Fix moonriver doc to say bp_moonbeam::GlobalConsensusLocation / Assets from Moonbeam, matching the actual Reserves type.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
runtime/moonriver/tests/xcm_emulator/network.rs (1)
276-298:⚠️ Potential issue | 🟠 MajorThe remaining hardcoded call indices can still silently misencode relay calls.
The pallet indices are now guarded, but the Staking/Utility/HRMP call slots below are still raw literals. If Westend reorders any of those calls, this helper will keep compiling and
pallet_xcm_transactorwill encode the wrong relay dispatch. Please derive or assert these call indices too, for example from SCALE-encodedwestend_runtime::RuntimeCallvariants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/moonriver/tests/xcm_emulator/network.rs` around lines 276 - 298, The hardcoded numeric call indices in RelayChainIndices (fields bond, bond_extra, unbond, withdraw_unbonded, validate, nominate, chill, set_payee, set_controller, rebond, as_derivative, init_open_channel, accept_open_channel, close_channel, cancel_open_request) can silently misencode relay dispatches; replace the literals with values derived or asserted from the actual westend runtime call enum (e.g. inspect SCALE-encoded westend_runtime::RuntimeCall variants or use runtime metadata to get the variant index) and add an assert or error if the derived index does not match expectations so the test fails rather than misencoding. Ensure you locate and update the RelayChainIndices construction in network.rs and derive each of the listed fields from westend_runtime::RuntimeCall (or equivalent lookup) instead of using raw u8 literals.
🧹 Nitpick comments (1)
runtime/moonriver/tests/xcm_emulator/network.rs (1)
157-166: Clean up the remaining Moonbeam/Polkadot copy-paste names.
PolkadotMoonbeamNet, theregister_dot_assethelper/docstring, and theMoonbeam genesis helperbanner all describe a different setup than this module. Renaming them to Moonriver/relay-neutral terms will make future failures and helper reuse much easier to reason about.Also applies to: 234-237, 328-330
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runtime/moonriver/tests/xcm_emulator/network.rs` around lines 157 - 166, The file still uses Polkadot/Moonbeam-specific names; rename the struct PolkadotMoonbeamNet to a Moonriver/relay-neutral name (e.g., MoonriverNet), and update any helper names and docstrings such as register_dot_asset and the "Moonbeam genesis helper" banner to reflect Moonriver/relay-neutral terminology; ensure all references to the old symbols (PolkadotMoonbeamNet, register_dot_asset, and the Moonbeam genesis helper text) are updated consistently across the file (including the other occurrences around the helper and banner blocks) so identifiers, comments, and docs match the actual Moonriver/relay-neutral setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@runtime/moonriver/tests/xcm_emulator/network.rs`:
- Around line 276-298: The hardcoded numeric call indices in RelayChainIndices
(fields bond, bond_extra, unbond, withdraw_unbonded, validate, nominate, chill,
set_payee, set_controller, rebond, as_derivative, init_open_channel,
accept_open_channel, close_channel, cancel_open_request) can silently misencode
relay dispatches; replace the literals with values derived or asserted from the
actual westend runtime call enum (e.g. inspect SCALE-encoded
westend_runtime::RuntimeCall variants or use runtime metadata to get the variant
index) and add an assert or error if the derived index does not match
expectations so the test fails rather than misencoding. Ensure you locate and
update the RelayChainIndices construction in network.rs and derive each of the
listed fields from westend_runtime::RuntimeCall (or equivalent lookup) instead
of using raw u8 literals.
---
Nitpick comments:
In `@runtime/moonriver/tests/xcm_emulator/network.rs`:
- Around line 157-166: The file still uses Polkadot/Moonbeam-specific names;
rename the struct PolkadotMoonbeamNet to a Moonriver/relay-neutral name (e.g.,
MoonriverNet), and update any helper names and docstrings such as
register_dot_asset and the "Moonbeam genesis helper" banner to reflect
Moonriver/relay-neutral terminology; ensure all references to the old symbols
(PolkadotMoonbeamNet, register_dot_asset, and the Moonbeam genesis helper text)
are updated consistently across the file (including the other occurrences around
the helper and banner blocks) so identifiers, comments, and docs match the
actual Moonriver/relay-neutral setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b1159eb7-ea1d-455e-87c0-38bfad636501
📒 Files selected for processing (6)
runtime/moonbase/tests/xcm_emulator/network.rsruntime/moonbase/tests/xcm_emulator/transfers.rsruntime/moonbeam/tests/xcm_emulator/network.rsruntime/moonbeam/tests/xcm_emulator/transfers.rsruntime/moonriver/tests/xcm_emulator/network.rsruntime/moonriver/tests/xcm_emulator/transfers.rs
…-later license - Pin alloy-consensus, alloy-eips, alloy-serde, alloy-tx-macros to v1.6.3 in Cargo.lock (v1.8.3 requires rustc 1.91, project uses 1.88) - Add LGPL-3.0-or-later to allowed licenses (compatible with GPL-3.0, needed for transitive deps rug and gmp-mpfr-sys from revm-precompile)
The polkadot-sdk stable2512 upgrade removed XcmPaymentApiV1. Use XcmPaymentApiV2 which adds a required asset_id parameter to query_delivery_fees.
Bump polkadot-sdk to the new cherry-pick (04250209) that makes xcm-emulator's block producer overridable, and plug in a NimbusId pre-runtime digest + 6000ms slot for each Moonbeam runtime so decl_test_parachains! no longer requires pallet_aura::Config. Also bump the Westend ParachainHost api_version from 13 to 15 so dmq_contents resolves against the trait the runtime actually implements.
Replace deprecated `pallet_utility::Pallet::<T>::derivative_account_id` with the freestanding `pallet_utility::derivative_account_id` function, and drop the unused `xcm_emulator::Parachain` trait import from network modules. Rust tests now compile cleanly under `-D warnings`.
Point the xcm-emulator overridable block producer row at paritytech/polkadot-sdk#11791 and move its status from "Needs PR upstream" to "Upstream PR not merged".
Three fixes are required for the runtime-level xcm-emulator tests to execute a block on every parachain in the network: - Populate a single `pallet_aura` authority in the Asset Hub genesis. Without it `FindAuthor::find_author` panics on `*slot % authorities_len()` the first time Aura's `on_initialize` runs. - Register the NimbusId emitted by each Moonbeam `BlockProducer` in `pallet_author_mapping`'s genesis so `pallet_author_inherent` can resolve a block author (previously panicked with "No Account Mapped to this NimbusId"). - Endow Asset Hub's XCM checking account with WND and, on Moonbeam, bump ALITH/BALTATHAR's balance to 1 000 000 GLMR. Relay→AH teleports of the native token withdraw from the checking account, and the `pallet_author_mapping` genesis registration reserves `100 GLMR * SUPPLY_FACTOR` (10 000 GLMR on Moonbeam) which would otherwise leave ALITH with zero free balance. All three `xcm_emulator` test suites (37 tests each) now pass.
paritytech/polkadot-sdk#11791 was merged upstream on 2026-04-17. Update the status column from "Upstream PR not merged" to "PR Upstream Merged".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor: Replace XCM simulator tests with xcm-config unit tests and xcm-emulator integration tests
Summary
Replaces the legacy XCM test suite (
xcm_tests.rs+xcm_mock/) across all three runtimes with a two-layer testing architecture that is more modular, more thorough, and testsagainst real runtime configurations instead of hand-rolled mocks.
Motivation
The previous XCM tests relied on
xcm-simulatorwith manually maintained mock runtimes (parachain.rs,relay_chain.rs,statemint_like.rs) that duplicated large amounts ofconfiguration and drifted from the actual runtime over time. Several tests contained subtle bugs — no-op assertions, hardcoded values instead of config-derived ones, and tests that
didn't actually exercise what their names claimed. The monolithic ~5,500-line
xcm_tests.rsfiles were difficult to navigate and extend.What changed
Deleted (−23,529 lines):
runtime/{moonbeam,moonbase,moonriver}/tests/xcm_tests.rs— monolithic test filesruntime/{moonbeam,moonbase,moonriver}/tests/xcm_mock/— hand-rolled mock runtimes (parachain, relay chain, statemint-like)Added (+16,360 lines across 42 new files):
tests/xcm_config/tests/xcm_emulator/xcm-emulatorwith real Westend relay + Asset Hub Westend + Moonbeam parachainxcm_config/tests cover (per runtime):barriers.rs— which XCM messages are allowed/denied executionreserves.rs— reserve asset filtering and self-reserve recognitiontraders.rs— XCM fee charging logictransactors.rs— asset deposit/withdrawal mechanicslocation.rs— location-to-account conversions (relay, sibling, ethereum-style)origin.rs— XCM origin conversionweigher.rs— XCM message weight calculationxcm_emulator/tests cover (per runtime):transfers.rs— relay↔para, para↔para, and Asset Hub transfers (DMP/UMP/XCMP)transact.rs— signed transact,EthereumXcmtransact, sovereign transact, HRMP channel managementasset_hub.rs— multi-hop and remote-reserve transfers through Asset Hubrelay.rs— relay chain interactionsversioning.rs— XCM version negotiation and safe-subscriptionTest coverage
Dependency changes
xcm-simulator→xcm-emulator(dev dependency for all three runtimes)westend-runtime,asset-hub-westend-runtimeas dev dependencies (real relay/parachain runtimes for the emulator network)pallet-delegated-staking,parachains-commonas dev dependencies for moonbeam runtime