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

Make pallets XCM compatible + less configuration overhead #2457

Open
3 of 5 tasks
svyatonik opened this issue Nov 28, 2022 · 4 comments
Open
3 of 5 tasks

Make pallets XCM compatible + less configuration overhead #2457

svyatonik opened this issue Nov 28, 2022 · 4 comments
Assignees

Comments

@svyatonik
Copy link
Contributor

svyatonik commented Nov 28, 2022

We've removed a lot of code (#1638) because the architecture has been changed significantly. And now we have some options to make pallets configuration easier. That's a task for the future (after initial deployment), but I'm gonna leave some ideas here:

  • now we have 3 types with almost the same meaning: XCM MultiLocation, ChainId and LaneId. The idea is that we'll have two bridge hubs, which will be used to build bridges between different parachains. Every bridge now (not at the code, but in plans) uses types in following matter: every parachain (bridge hubs, Statemin?, ...) all have XCM "addresses" (MultiLocations), bridge hubs also have ChainIds and every bridge will be assigned a single lane (e.g. Statimine <> Statemint will be using lane 0x00000000). We may remove ChainId and LaneId types and instead use pair of MultiLocations (or whatever is used to identify chains in global consensus);
  • we may remove configurable proof types from the messaging pallet and move storage proof verification code to the messaging pallet;
  • we may move most of bridge-runtime-common code to the messaging pallet and then we'll be able to replace most of configuration options and glue code by adding single type BridgedChain = SomeBridgedChain;. This would also allow us to implement things like Signed extension to refund relayer at the target chain #1657 easier;

Things to do after refactoring:

  • review and remove/add conditions to integrity tests;
  • (optionally?) maybe remove VerificationError and move all variants to the pallet errors?
@svyatonik
Copy link
Contributor Author

Also see this comment: #1657 (comment)

@svyatonik
Copy link
Contributor Author

svyatonik commented May 31, 2023

Future pallet_bridge_messages::Config (may be changed):

	#[pallet::config]
	pub trait Config<I: 'static = ()>: frame_system::Config {
		// General types

		/// The overarching event type.
		type RuntimeEvent: From<Event<Self, I>>
			+ IsType<<Self as frame_system::Config>::RuntimeEvent>;
		/// Benchmarks results from runtime we're plugged into.
		type WeightInfo: WeightInfoExt;

		/// This chain type.
		type ThisChain: ChainWithMessages;
		/// Bridged chain type.
		type BridgedChain: ChainWithMessages;
		/// Bridged chain headers provider.
		type BridgedHeaderChain: HeaderChain<Self::BridgedChain>;

		/// Get all active outbound lanes that the message pallet is serving.
		type ActiveOutboundLanes: Get<&'static [LaneId]>;

		/// Payload type of outbound messages. This payload is dispatched on the bridged chain.
		type OutboundPayload: Parameter + Size;
		/// Payload type of inbound messages. This payload is dispatched on this chain.
		type InboundPayload: Decode;

		/// Handler for relayer payments that happen during message delivery transaction.
		type DeliveryPayments: DeliveryPayments<Self::AccountId>;
		/// Handler for relayer payments that happen during message delivery confirmation
		/// transaction.
		type DeliveryConfirmationPayments: DeliveryConfirmationPayments<Self::AccountId>;

		/// Message dispatch handler.
		type MessageDispatch: MessageDispatch<DispatchPayload = Self::InboundPayload>;
	}
}

@svyatonik
Copy link
Contributor Author

The main part of this issue is fixed. I've decided to leave OutboundPayload + InboundPayload + MessageDispatch abstractions because at some point we may (?) start accepting messages, which are sent to bridge hub itself => we'll probably need to decode the encoded XCM BridgeMessage at bridge hub.

Regarding the first point (merging ChainId, LaneId and XCM locations) - let's think of that in context of #2451. I'm not sure right now what's the best way to implement that

@svyatonik
Copy link
Contributor Author

Working on #2213, I think the best way would be to:

  1. to avoid referencing XCM primitives from basic bridge pallets. Right now it'll be complicated, because we'll have a temptation to use some stuff from Cumulus, but we can't. So let's not use XCM stuff directly from the basic bridge pallets;
  2. instead, let's add some abstractions to those pallets:
    2.1) let's remove LaneId([u8; 4]) and make code generic over lane key. We'll use ordered (MultiLocation, MultiLocation) for that. See details in the XCM bridge hub pallet (will be deployed at bridge hubs) #2213
    2.2) let's try to get rid of ChainId and either make code generic over this type. We'll use XCM NetworkId for that. Alternative may be using (don't know how, though) the Vec<u8> as chain ID.

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

No branches or pull requests

2 participants