Permisionless way to create HRMP channels between system parachains and other parachains about rfcs

[joepetrowski]

Why do we need a whole new pallet for this? Couldn't we add one new call, similar to establish_system_channel, that a parachain would call?

So:

pub fn establish_channel_with_system(
	origin: OriginFor<T>,
	interlocutor: ParaId,
) -> DispatchResult {
	let origin = ensure_parachain(<T as Config>::RuntimeOrigin::from(origin))?;
	ensure!(interlocutor.is_system(), Error::<T>::ChannelCreationNotAuthorized);
	// ...
	// establish bidirectional channel
	// ...
}

[joepetrowski]

But like you said in that issue, that just requires implementation of the Hrmp* instructions.

[acatangiu]

We had to fork the executor to have an implementation of the HRMP instructions at Polimec.

All we did is create a config type on the executor so we can pass whatever logic we want to handle the HRMP instructions in the construct_runtime.

@joepetrowski, why can't the original xcm executor have something like this instead of returning an error? This way we don't have to keep an up-to-date fork of the executor for only two lines of code. It's generic enough to not cause any complications, and if any parachain wants to avoid using those instructions, they can simply pass an empty implementation.

Why not try to upstream it then?

I believe it was always the plan to have an implementation for that instruction, but there's always more things to do than time to do them. I would welcome a PR that gets it done 🚀

[joepetrowski]

@joepetrowski, why can't the original xcm executor have something like this instead of returning an error? This way we don't have to keep an up-to-date fork of the executor for only two lines of code. It's generic enough to not cause any complications, and if any parachain wants to avoid using those instructions, they can simply pass an empty implementation.

Yeah this is what I'm arguing for in this thread...

[joepetrowski]

Maybe we should open these channels automatically between all chains and the system chains. Maybe at the beginning using some low capacity, which could be increased by providing some deposit.

Yeah, so we could make a call like this that would let any para open a channel with any system chain, but with a default low-capacity configuration. Then if a chain wants a higher capacity channel, they can use the force_open_hrmp_channel call that goes through governance.

[bkontur]

We had to fork the executor to have an implementation of the HRMP instructions at Polimec.

All we did is create a config type on the executor so we can pass whatever logic we want to handle the HRMP instructions in the construct_runtime.

@joepetrowski, why can't the original xcm executor have something like this instead of returning an error? This way we don't have to keep an up-to-date fork of the executor for only two lines of code. It's generic enough to not cause any complications, and if any parachain wants to avoid using those instructions, they can simply pass an empty implementation.

I initiated this issue paritytech/polkadot-sdk#3692 for different reasons just a few hours ago, and @acatangiu directed me to this RFC issue. Now, I also see @xlc's comment paritytech/polkadot-sdk#922 (comment). It seems like everything revolves around the same issue - adding a callback handler for HrmpNewChannelOpenRequest / HrmpChannelAccepted / HrmpChannelClosing XCM instructions to the XcmExecutor.

@JuaniRios, as Adrian mentioned, you could open a PR to the polkadot-sdk repo, and I could assist with its completion. This way, you can eliminate your fork. I checked your use-case, and it appears to be different from mine and @xlc's regarding the VersionDiscoveryQueue. However, with tuple implementation for HrmpHandler, we can support both and more use cases.

[xlc]

Such pallet could also fix paritytech/polkadot-sdk#922, which is the root cause of polkadot-fellows/runtimes#190

[xlc]

so it can be reused by non system parachains and I want to fix paritytech/polkadot-sdk#922 at the same time

[xlc]

yeah but the implementation have to be lived somewhere? also for security reasons we may want to config things like max channel count or deposit requirements etc. what is your suggestion?

[joepetrowski]

yeah but the implementation have to be lived somewhere?

The same place that implements the other instructions... It's just an implementation of the ExecuteXcm trait.

also for security reasons we may want to config things like max channel count or deposit requirements

We already have a Configuration pallet that contains these.

[xlc]

I don’t really get what are you suggesting. Yeah maybe not a new pallet but where do you suggest to put those code?
And maybe it doesn’t need to be a new pallet but why it can’t be a new pallet?

[joepetrowski]

Where it already is.

[xlc]

maybe we need to touch this file but I still think we need a new pallet. eg to allow configure the parameters via governance proposal

and forcing more non optional parameters to already super complicated xcm configurations just sounds bad

anyway, those are technical details and not exactly something we need to discuss before a draft is created.

my high level questions are:

should we do this? sounds like a yes from Joe
do we need to have some limits and what are the limits for the system parachains?
what are the hrmp channel parameters that should be used or do we want to not hardcoded it?
any security risks or other concerns?
to confirm we need a rfc for this

[bkchr]

We should develop a pallet for system parachains to allow other parachains to permisionlessly create bidirectional HRMP channels with it

With on-demand parachains this isn't such a great idea. I mean with the current implementation of XCMP it isn't a problem. However, if we ever go to offchain-xcmp, this will lead to problems.

[xlc]

that’s exactly the reason why I am asking do we need to have some limits and what are the limits for the system parachains?
permisionless does not mean no requirement at all. we can still ask for deposit, time delay, etc.

I think we absolutely need the system parachain to be able to communicate with every long live parachain / whatever coretime software that wants to purchase coretime directly via xcm. Otherwise we have a problem to fix and in that case we identified a problem and we need to then find out a solution to it.

I don’t exactly know all the details and limitations about offchain-xcmp but if it’s feature set does not cover everything we have currently, we need either alter the design to make it more capable, or in the case that’s not feasible, keep the hrmp mechanism.

[JuaniRios]

We had to fork the executor to have an implementation of the HRMP instructions at Polimec.

All we did is create a config type on the executor so we can pass whatever logic we want to handle the HRMP instructions in the construct_runtime.

@joepetrowski, why can't the original xcm executor have something like this instead of returning an error? This way we don't have to keep an up-to-date fork of the executor for only two lines of code. It's generic enough to not cause any complications, and if any parachain wants to avoid using those instructions, they can simply pass an empty implementation.

[bkchr]

After writing this comment, I thought more about this. Actually my comment isn't that correct. As long as the system chains are building every 24 hours a block, it should be quite easy for them to get hold of the messages from someone else. So, we can probably ignore what I said.

This brings me back to some other thought I had, based on this comment from you:

are created to offer functionalities to relaychain and other parachains.

Maybe we should open these channels automatically between all chains and the system chains. Maybe at the beginning using some low capacity, which could be increased by providing some deposit.

[xlc]

ok. that will work.
next question: do we need rfc for this? I should have everything I needed to start implement this.

[xlc]

paritytech/polkadot-sdk#3721 allow any parachain to have HRMP channel with a system parachain