-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 Polimec HRMP info #5503
Merged
Merged
Add Polimec HRMP info #5503
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d20a1de
added info provided by Polimec
filippoweb3 d22f7ec
Merge branch 'master' into update-hrmp-info
filippoweb3 9893188
added useful links
filippoweb3 ee4e85b
Update docs/general/thousand-validators.md
DrW3RK cd53c99
Update docs/learn/learn-guides-accounts-multisig.md
DrW3RK 6e6e7b3
Update docs/learn/learn-guides-accounts-multisig.md
DrW3RK c0d1f74
Add Joe's feedback
DrW3RK d253724
hrmp channel management
joepetrowski addfbe7
remove second person
joepetrowski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this document gives concrete instructions on how to open HRMP channels between chains, this section falls short explaining what specific logic needs to be added to xcm-executor pallet to handle high-level HRMP instructions. Perhaps, a link to the forked code base and an actual project example that successfully opened channels without relying on governance would help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This suggestion came from my comments. We're open-sourcing the code at Polimec next Monday and could include it here. However, the codebase is still in development.
The wording is very good @ filippoweb3. It should be noted that the initial opening would probably not be done by a notification from the relay, since someone needs to send the
hrmpInitOpenChannel
extrinsic to the relay first for this to happen.At Polimec we expect the other parachain to first initiate the opening from whatever means they want (probably sudo), and from this notification start our logic to check if we should accept and initiate the other direction channel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! Yeah, just want to ensure there are concrete instructions on this page and avoid ambiguity for parachain teams. Sorry for the mix-up with Referenda 340. We will link to your implementation once it is available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JuaniRios thanks, we will leave this open until we can link to the instructions you will opensource :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@filippoweb3 @DrW3RK Sorry for the delay. We're open sourcing sometime this week, prob tomorrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@filippoweb3 our legal team is now reviewing the licences. ETA around Wednesday next week
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our codebase is now open source!
Here are some interesting links you might want to use:
Let me know if you have any questions :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DrW3RK I added those links above, lmk if its ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @JuaniRios for contributing!