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 HRMP notification handlers to the xcm-executor #3696

Merged
merged 19 commits into from
Mar 19, 2024

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented Mar 14, 2024

Currently the xcm-executor returns an Unimplemented error if it receives any HRMP-related instruction.
What I propose here, which is what we are currently doing in our forked executor at polimec, is to introduce a trait implemented by the executor which will handle those instructions.

This way, if parachains want to keep the default behavior, they just use () and it will return unimplemented, but they can also implement their own logic to establish HRMP channels with other chains in an automated fashion, without requiring to go through governance.

Our implementation is mentioned in the polkadot HRMP docs, and it was suggested to us to submit a PR to add these changes to polkadot-sdk.

Polkadot address: 15fj1UhQp8Xes7y7LSmDYTy349mXvUwrbNmLaP5tQKBxsQY1

@JuaniRios JuaniRios requested review from athei and a team as code owners March 14, 2024 12:15
@cla-bot-2021
Copy link

cla-bot-2021 bot commented Mar 14, 2024

User @JuaniRios, please sign the CLA here.

@acatangiu
Copy link
Contributor

cc @bkontur

@JuaniRios JuaniRios changed the title Add HRMP messages implementation on the executor Add HRMP messages implementation on the xcm-executor Mar 14, 2024
@JuaniRios
Copy link
Contributor Author

Original disussion: polkadot-fellows/RFCs#82

@bkontur bkontur mentioned this pull request Mar 14, 2024
2 tasks
@JuaniRios JuaniRios requested a review from bkontur March 14, 2024 16:22
@bkontur
Copy link
Contributor

bkontur commented Mar 15, 2024

bot fmt

@command-bot
Copy link

command-bot bot commented Mar 15, 2024

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5539729 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-102eedb3-984d-4d73-8591-b0c7869e26d6 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Mar 15, 2024

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

@bkontur
Copy link
Contributor

bkontur commented Mar 15, 2024

@JuaniRios also please fix all compilation issues e.g.https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5539475 and we should be good to go

@JuaniRios JuaniRios requested a review from bkontur March 15, 2024 10:34
@bkontur bkontur added the T6-XCM This PR/Issue is related to XCM. label Mar 18, 2024
@bkontur bkontur enabled auto-merge March 18, 2024 15:59
@bkontur bkontur added this pull request to the merge queue Mar 19, 2024
Merged via the queue into paritytech:master with commit 8b3bf39 Mar 19, 2024
132 of 137 checks passed
ordian pushed a commit that referenced this pull request Mar 19, 2024
Currently the xcm-executor returns an `Unimplemented` error if it
receives any HRMP-related instruction.
What I propose here, which is what we are currently doing in our forked
executor at polimec, is to introduce a trait implemented by the executor
which will handle those instructions.

This way, if parachains want to keep the default behavior, they just use
`()` and it will return unimplemented, but they can also implement their
own logic to establish HRMP channels with other chains in an automated
fashion, without requiring to go through governance.

Our implementation is mentioned in the [polkadot HRMP
docs](https://arc.net/l/quote/hduiivbu), and it was suggested to us to
submit a PR to add these changes to polkadot-sdk.

---------

Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: command-bot <>
ordian added a commit that referenced this pull request Mar 19, 2024
* master:
  Make `availability-recovery-regression-bench` a benchmark (#3741)
  Add HRMP notification handlers to the xcm-executor (#3696)
  Bump the known_good_semver group with 2 updates (#3726)
  removed `pallet::getter` usage from cumulus pallets (#3471)
@franciscoaguirre
Copy link
Contributor

This is a breaking change until we have default configs

@acatangiu
Copy link
Contributor

This is a breaking change until we have default configs

yes, but the "soft" breaking type - only breaking at runtime config level and with () default config options available
this kind of "breaking change" is ubiquitous in polkadot-sdk releases

@bkontur
Copy link
Contributor

bkontur commented Mar 20, 2024

@JuaniRios Could you update the description with your "polkadot address: XYZ"?

@JuaniRios
Copy link
Contributor Author

@JuaniRios Could you update the description with your "polkadot address: XYZ"?

Added at the bottom of the PR description :)

@bkchr
Copy link
Member

bkchr commented Mar 20, 2024

/tip small

Copy link

@bkchr A referendum for a small (20 DOT) tip was successfully submitted for @JuaniRios (15fj1UhQp8Xes7y7LSmDYTy349mXvUwrbNmLaP5tQKBxsQY1 on polkadot).

Referendum number: 589.
tip

Copy link

The referendum has appeared on Polkassembly.

dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Mar 24, 2024
Currently the xcm-executor returns an `Unimplemented` error if it
receives any HRMP-related instruction.
What I propose here, which is what we are currently doing in our forked
executor at polimec, is to introduce a trait implemented by the executor
which will handle those instructions.

This way, if parachains want to keep the default behavior, they just use
`()` and it will return unimplemented, but they can also implement their
own logic to establish HRMP channels with other chains in an automated
fashion, without requiring to go through governance.

Our implementation is mentioned in the [polkadot HRMP
docs](https://arc.net/l/quote/hduiivbu), and it was suggested to us to
submit a PR to add these changes to polkadot-sdk.

---------

Co-authored-by: Branislav Kontur <[email protected]>
Co-authored-by: command-bot <>
@Polkadot-Forum
Copy link

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

https://forum.polkadot.network/t/parity-tech-update-for-march/7226/1

@Morganamilo Morganamilo mentioned this pull request Apr 4, 2024
12 tasks
github-merge-queue bot pushed a commit that referenced this pull request Apr 24, 2024
… from the relaychain (#4156)

This PR:
- introduces `AllowHrmpNotificationsFromRelayChain` barrier for allowing
HRMP notifications just from the relay chain (to fulfill safety
assumptions -
[see](https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/src/v4/mod.rs#L532))
- sets it up for all testnet SP parachains

Continuation of: #3696
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants