Skip to content
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

Ensure xcm versions over bridge (on sending chains) #2481

Merged
merged 53 commits into from
Dec 12, 2023

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented Nov 24, 2023

Summary

This pull request proposes a solution for improved control of the versioned XCM flow over the bridge (across different consensus chains) and resolves the situation where the sending chain/consensus has already migrated to a higher XCM version than the receiving chain/consensus.

Problem/Motivation

The current flow over the bridge involves a transfer from AssetHubRococo (AHR) to BridgeHubRococo (BHR) to BridgeHubWestend (BHW) and finally to AssetHubWestend (AHW), beginning with a reserve-backed transfer on AHR.

In this process:

  1. AHR sends XCM ExportMessage through XcmpQueue, incorporating XCM version checks using the WrapVersion feature, influenced by pallet_xcm::SupportedVersion (managed by pallet_xcm::force_xcm_version or version discovery).

  2. BHR handles the ExportMessage instruction, utilizing the latest XCM version. The HaulBlobExporter converts the inner XCM to VersionedXcm::from, also using the latest XCM version.

However, challenges arise:

  • Incompatibility when BHW uses a different version than BHR. For instance, if BHR migrates to XCMv4 while BHW remains on XCMv3, BHR's VersionedXcm::from uses VersionedXcm::V4 variant, causing encoding issues for BHW.
      /// Just a simulation of possible error, which could happen on BHW
      /// (this code is based on actual master without XCMv4)
      let encoded = hex_literal::hex!("0400");
      println!("{:?}", VersionedXcm::<()>::decode(&mut &encoded[..]));
    
      Err(Error { cause: None, desc: "Could not decode `VersionedXcm`, variant doesn't exist" })
    
  • Similar compatibility issues exist between AHR and AHW.

Solution

This pull request introduces the following solutions:

  1. New trait CheckVersion - added to the xcm module and exposing pallet_xcm::SupportedVersion. This enhancement allows checking the actual XCM version for desired destinations outside of the pallet_xcm module.

  2. Version Check in HaulBlobExporter uses CheckVersion to check known/configured destination versions, ensuring compatibility. For example, in the scenario mentioned, BHR can store the version 3 for BHW. If BHR is on XCMv4, it will attempt to downgrade the message to version 3 instead of using the latest version 4.

  3. Version Check in pallet-xcm-bridge-hub-router - this check ensures compatibility with the real destination's XCM version, preventing the unnecessary sending of messages to the local bridge hub if versions are incompatible.

These additions aim to improve the control and compatibility of XCM flows over the bridge and addressing issues related to version mismatches.

Possible alternative solution

(More investigation is needed, and at the very least, it should extend to XCMv4/5. If this proves to be a viable option, I can open an RFC for XCM.).

Add the XcmVersion attribute to the ExportMessage so that the sending chain can determine, based on what is stored in pallet_xcm::SupportedVersion, the version the destination is using. This way, we may not need to handle the version in HaulBlobExporter.

ExportMessage {
	network: NetworkId,
	destination: InteriorMultiLocation,
	xcm: Xcm<()>
	destination_xcm_version: Version, // <- new attritbute
},
pub trait ExportXcm {
        fn validate(
		network: NetworkId,
		channel: u32,
		universal_source: &mut Option<InteriorMultiLocation>,
		destination: &mut Option<InteriorMultiLocation>,
		message: &mut Option<Xcm<()>>,
                destination_xcm_version: Version, , // <- new attritbute
	) -> SendResult<Self::Ticket>;

Future Directions

This PR does not fix version discovery over bridge, further investigation will be conducted here: #2417.

TODO

  • pallet_xcm mock for tests uses hard-coded XCM version 2 - change to 3 or lastest?
  • fix pallet-xcm-bridge-hub-router
  • fix HaulBlobExporter with version determination here
  • add unit-tests to the runtimes
  • run benchmarks for ExportMessage
  • extend local run scripts about force_xcm_version(dest, version)
  • when merged, prepare governance calls for Rococo/Westend
  • add PRDoc

Part of: paritytech/parity-bridges-common#2719

@bkontur bkontur added T9-cumulus This PR/Issue is related to cumulus. T15-bridges This PR/Issue is related to bridges. labels Nov 24, 2023
@bkontur
Copy link
Contributor Author

bkontur commented Nov 29, 2023

bot bench cumulus-bridge-hubs --runtime=bridge-hub-rococo --subcommand=xcm --pallet=pallet_xcm_benchmarks::generic
bot bench cumulus-bridge-hubs --runtime=bridge-hub-westend --subcommand=xcm --pallet=pallet_xcm_benchmarks::generic

@bkontur
Copy link
Contributor Author

bkontur commented Nov 29, 2023

bot bench cumulus-bridge-hubs --runtime=bridge-hub-rococo --subcommand=xcm --pallet=pallet_xcm_benchmarks::generic

@bkontur
Copy link
Contributor Author

bkontur commented Nov 29, 2023

bot bench cumulus-bridge-hubs --runtime=bridge-hub-rococo --subcommand=xcm --pallet=pallet_xcm_benchmarks::generic
bot bench cumulus-assets --runtime=asset-hub-rococo --pallet=pallet_xcm_bridge_hub_router
bot bench cumulus-assets --runtime=asset-hub-westend --pallet=pallet_xcm_bridge_hub_router

@bkontur
Copy link
Contributor Author

bkontur commented Nov 29, 2023

bot bench cumulus-bridge-hubs --runtime=bridge-hub-westend --subcommand=xcm --pallet=pallet_xcm_benchmarks::generic

@command-bot
Copy link

command-bot bot commented Dec 11, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4684673 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4684673/artifacts/download.

@bkontur
Copy link
Contributor Author

bkontur commented Dec 12, 2023

bot fmt

@command-bot
Copy link

command-bot bot commented Dec 12, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4692219 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 3-db233b21-f059-4337-a863-ddcb451c33bc to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Dec 12, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4692219 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4692219/artifacts/download.

@bkontur
Copy link
Contributor Author

bkontur commented Dec 12, 2023

bot fmt

@command-bot
Copy link

command-bot bot commented Dec 12, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4694083 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 4-57c739b8-4d84-4199-850a-ebd1c1bf9317 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Dec 12, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4694083 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4694083/artifacts/download.

@bkontur bkontur enabled auto-merge (squash) December 12, 2023 14:03
@bkontur bkontur merged commit 575b8f8 into master Dec 12, 2023
115 checks passed
@bkontur bkontur deleted the bko-bridge-xcm-version branch December 12, 2023 15:04
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-kusama-bridge/2971/8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T9-cumulus This PR/Issue is related to cumulus. T15-bridges This PR/Issue is related to bridges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants