Support dedicated lanes for pallets

[svyatonik]

Initial concept of the lane was that single application will use single lane (now we have two generic lanes in test deployments, though). While working on #944 I've found that the current set of messages integration APIs doesn't allow us to have lanes dedicated to runtime pallets. So if we have pallet (in this context: application) that is going to use messages pallet to send messages, then we can't verify (in the LaneMessageVerifier) that the message is actually sent by the pallet (not by the regular user).

Why this is important? In #935 I've introduced at-source-chain-callback for processing messages dispatch result. This callback is something like fn on_messages_dispatched(lane: LaneId, dispatch_results: MessageDispatchResults) {}. If the token-swap pallet (from #944 that uses this callback) would use existing generic lanes (0x00000000 or 0x00000001), then implementation of this callback won't be trivial - it'll need to check if every dispatched message is actually token-swap pallet message (=> extra storage read for every message) before doing something useful. If we'll have lane, dedicated to this pallet, then the implementation won't need to do this check (because it'll has if lane != T::OutboundLaneId::get() { return }) && will only deal with its own messages.

I've started this back in the April && just opening this PR for visibility now. If you guys think it makes sense to support such 'dedicated' lanes, then I'll continue to work on this after #935 is accepted. If this is not required - I'll just close the PR. Probably after review of #935 , something will change - I just didn't want to lose this code.

The alternative solutions I've considered:

1. use something like enum SendOrigin { External(AccountId), Root, Internal } in messages integration APIs. But then I've found that pallets may define their own origins && I've decided to use it - it seems to be the proper way;
2. give special meaning to frame_system::RawOrigin::None - so it'll mean that the message is sent by the runtime itself. But this is super-confusing.

[tomusdrw]

I like the idea of passing origin to is_message_accepted, but I'd drop the custom trait for that - is it necessary? Or why is it useful? It adds extra generic parameters, hence I'd prefer to avoid it.

[svyatonik]

I like the idea of passing origin to is_message_accepted, but I'd drop the custom trait for that - is it necessary? Or why is it useful? It adds extra generic parameters, hence I'd prefer to avoid it.

My understanding is that any pallet may declare its own origin (using #[pallet::origin]). All origins (including the one from frame_system) are then merged into final runtime-specific Origin. So when origin comes to message lane verifier/message payment components, they'll need to know at least two things:

1. who has crafted the message - e.g. we may use something like *lane_id == *b"swap" && matches!(origin, crate::Origin::TokenSwap(_)) to only accept messages from token swap pallet to swap lane;
2. whether the message is paid (and by whom) or not. This way we may have system-level pallets that may send messages for free (and altruistic relayers who are serving such lanes) and general/pallet-specific lanes, which require messages to be paid.

The trait is a way to keep using existing generic code (instant payments), which was previously using frame_system::Origin with this new concept. So in the end, it needs to convert generic runtime origin into account id to know who's paying for the message. Without trait, you'll need to do that on runtime level - i.e. every runtime will need its own implementation of the MessageDeliveryAndDispatchPayment, which will extract account id and then call InstantCurrencyPayments using extracted account.

The simpler option would be to use struct MessageSenderOrigin { sender_id: [u8; 4], paid_by: Option<AccountId> } (where e.g. sender_id=00000000 would mean that the message is sent by the regular user, sender_id=*b"swap" will meanthe token swap pallet, etc) everywhere instead of runtime-level origin, but I was assuming that it'll be better to use existing origin concept.

[tomusdrw]

My understanding is that any pallet may declare its own origin (using #[pallet::origin]). All origins (including the one from frame_system) are then merged into final runtime-specific Origin. So when origin comes to message lane verifier/message payment components, they'll need to know at least two things:

I see it totally makes sense, I didn't connect the dots. Looks good to me!

[svyatonik]

This field isn't used now anywhere deployments, but if this pallet will ever end up in some real-chain && we'll want to filter it by the token-swap originator, then we may use this field in the filter. Could remove on demand

[tomusdrw]

@svyatonik shall we merge?

[svyatonik]

@svyatonik shall we merge?

I've been asking this on Riot couple of weeks ago :) It won't be audited, and imo it deserves audit. So probably not now? Or we'll have some dedicated release branch without this PR merged in?

[tomusdrw]

I've been asking this on Riot couple of weeks ago :) It won't be audited, and imo it deserves audit. So probably not now? Or we'll have some dedicated release branch without this PR merged in?

Sorry, must have missed it or forgot. Sounds good, let's wait and branch out when we are ready to go live and after that merge to master.

[tomusdrw]

We've branched out polkadot-staging, so I guess this is good to go now?

[svyatonik]

We've branched out polkadot-staging, so I guess this is good to go now?

Let's not merge any needsaudit PRs related to existing pallets until we'll have a strict audit solution+policy. Like before opening polkadot-staging -> polkadot repo PR we'll need to update Substrate+Polkadot+Cumulus refs. Where shall we do that? If in master + then simply make a master -> polkadot-staging PR, then either difference(master, polkadot-staging) shall not have any needsaudit PRs, or we shall make reaudit. If we'll only update refs in polkadot-staging branch, then master and polkadot-staging will diverge and we'll have to do the same work twice. So I suggest to not to merge this PR now.