XDM: Permissioned channels - 1 #2602
Merged
Conversation
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
This comment was marked as outdated.
This comment was marked as outdated.
vedhavyas
force-pushed
the
permissioned_channels
branch
from
ba39fcb
to
8dc7763
Compare
…sus and domain, and necessary traits traits are used in the coming commits
…n consensus chain for the given domain by domain owner
…st updates and a minor cleanup to MessengerApi
…owlist extrinsic during verification
… channel. Also fixes a small discrepency with inherent extrinsics order
vedhavyas
force-pushed
the
permissioned_channels
branch
from
8dc7763
to
be5cd00
Compare
NingLin-P
reviewed
Mar 13, 2024
Comment on lines
-126
to
+135
| // `domain-block-preprocessor::CreateInherentDataProvider` | ||
| // pallets order defined in `construct_runtime` macro for domains. | ||
| // currently this is the following order | ||
| // - timestamp extrinsic | ||
| // - executive set_code extrinsic | ||
| // - messenger update_domain_allowlist extrinsic | ||
| // - block_fees transaction_byte_fee_extrinsic | ||
| // since we use `push_front` the extrinsic should be pushed in reversed order | ||
| // TODO: this will not be valid once we have a different runtime. To achive consistency across | ||
| // domains, we should define a runtime api for each domain that should order the extrinsics | ||
| // like inherent are derived while domain block is being built |
NingLin-P
approved these changes
Mar 14, 2024
vedhavyas
added
need to audit
vanhauser-thc
added
audited
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR introduces permissioned channels as described in this spec here - Permissioned channels
This is an invasive PR that changes how channels are opened between Chains. Whenever user initiates the channel, the src_chain will reject if the dst_chain is not in the allowed channel list. Once the dst_chain is in the allowed list, user can initiate a channel. When the message is relayed to dst_chain, if the dst_chain's allow list does not have the src_chain, then the transaction is not included. This brings a scenario where relayer will continue to gossip the transaction and txn will be dropped from the pool until dst_chain allowlist is updated.
There is potential DDOS where user can initiate multiple channels to dst_chain and all of them ends up failing since dst_chain allowlist is not updated.
This scenario does not exist for now since I have not removed the Sudo user call for all the operations including domain owner which is also a sudo user.
I'll be giving a second PR that would handle the above issue by making the channel initiation economincally not viable. That will be done with following upcoming changes
Once the second PR is in, Sudo is only required to open channel from consensus chain and to update consensus chain's allow list.
For the domains, domain owner can update the Domain's allowlist through consensus chain and then anyone can procced to open the channel.
Code contributor checklist: