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

Add commons crate for BridgeHub runtimes [do not merge] #22

Closed
wants to merge 1 commit into from

Conversation

vgeddes
Copy link

@vgeddes vgeddes commented Nov 8, 2023

This PR adds a crate bridge-hub-common with common utilities for all BridgeHub runtimes (similar to the purpose of the asset-hub-common crate).

It is not meant to hold runtime config or utilities that are specific to either Snowbridge or ParityBridge. There are other crates for that. Rather, it is meant to handle cross-cutting concerns that affect all bridges on BridgeHub.

The crate initially provides specialized config for the MessageQueue pallet. See paritytech#2146 for background and discussion.

Changes:

  • Introduce a AggregateMessageOrigin that identifies message queues belonging to either XCMP or to Snowbridge
  • Introduce BridgeHubMessageRouter, a message processor that routes messages either to XCMP or to Snowbridge.

@vgeddes vgeddes changed the title Add commons crate for BridgeHub runtimes Add commons crate for BridgeHub runtimes [do not merge] Nov 8, 2023
@yrong
Copy link

yrong commented Nov 14, 2023

@vgeddes Seems now we have 2 different kinds of MessageOrigin.

AggregateMessageOrigin in

pub enum AggregateMessageOrigin {

and the one in this PR.

Consider the change you made in Snowfork/snowbridge#1005 I would suggest without adding a new kind of AggregateMessageOrigin just add a general enum item into the previous cumulus primitive. Something like this:

#[derive(Encode, Decode, MaxEncodedLen, Clone, Eq, PartialEq, TypeInfo, Debug)]
pub enum AggregateMessageOrigin {
	/// The message came from the para-chain itself.
	Here,
	/// The message came from the relay-chain.
	///
	/// This is used by the DMP queue.
	Parent,
	/// The message came from a sibling para-chain.
	///
	/// This is used by the HRMP queue.
	Sibling(ParaId),
        /// For other customize origin like snowbridge
        GeneralKey([u8; 32])
}

@claravanstaden Seems you've merged the change here into #18 ? Then the integration tests there will not get compiled

 cargo check -p bridge-hub-rococo-integration-tests
...
error[E0277]: can't compare `bridge_hub_common::message_queue::AggregateMessageOrigin` with `CumulusAggregateMessageOrigin`
   --> cumulus/parachains/integration-tests/emulated/chains/parachains/bridges/bridge-hub-rococo/src/lib.rs:28:1
    |
28  | / decl_test_parachains! {
29  | |     pub struct BridgeHubRococo {
30  | |         genesis = genesis::genesis(),
31  | |         on_init = {
...   |
44  | |     },
45  | | }
    | |_^ no implementation for `bridge_hub_common::message_queue::AggregateMessageOrigin == CumulusAggregateMessageOrigin`

because the xcm-emulator in

type MessageProcessor: ProcessMessage<Origin = CumulusAggregateMessageOrigin> + ServiceQueues;
accepts only the former.

And my suggestion above(i.e. stick to one AggregateMessageOrigin) will also resolve the compile issue here.

@claravanstaden claravanstaden changed the base branch from master to snowbridge November 15, 2023 12:20
@claravanstaden claravanstaden changed the base branch from snowbridge to master November 15, 2023 12:21
@claravanstaden claravanstaden changed the base branch from master to snowbridge November 23, 2023 08:09
@claravanstaden claravanstaden changed the base branch from snowbridge to master November 23, 2023 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants