Allow signed origins to send arbitrary XCMs from some system chains #407
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
Relaxing the `SendXcmOrigin` filter for some of the System chains: - Relay Kusama: already open to signed origins and pluralities - Relay Polkadot: relax to same level as Kusama ^ - Asset Hubs (P and K): open up to signed origins - People (P and K): allow signed origins - Encointer: allow signed origins - Collectives Polkadot: no change, only Fellows collective allowed - Bridge Hubs: no change, no arbitrary XCM sends allowed - Coretime Kusama: no change, no arbitrary XCM sends allowed We could allow it on all chains but on some of them there are no usecases afaict, so starting out conservative, opening it up only on chains where it's wanted.
Maybe from Coretime chain there would be users that want to |
|
@joepetrowski WDYT about current list? |
cc @seadanda |
|
The list looks reasonable to me. Will let @seadanda chime in. |
|
List looks fine to me too, I don't see any immediate usecase for arbitrary Transacts. |
|
No use cases from our side so far, but there could potentially be some, so I wouldn't be against it. |
|
Same opinion as @Szegoo here. |
|
@joepetrowski @seadanda @Szegoo @poppyseedDev what about a full(ish) review? |
joepetrowski
approved these changes
Aug 22, 2024
seadanda
approved these changes
Aug 23, 2024
|
@kianenigma @shawntabrizi @xlc need another review here please |
bkchr
approved these changes
Sep 3, 2024
…x-xcm-send-filter
|
/merge |
|
Enabled Available commands
For more information see the documentation |
fellowship-merge-bot
bot
merged commit fac5569
into
polkadot-fellows:main
47 checks passed
12 tasks
ntn-x2
added a commit
to KILTprotocol/kilt-node
that referenced
this pull request
Oct 8, 2024
Fixes KILTprotocol/ticket#3625. # Description * Removed the requirement that the pool must be enabled (i.e., not stopped) for the XCM `Matcher` implementation to work. The matcher should not really care about that. All other XCM components have not changed. * Add a new storage map to the switch pallet to track pending switches. The storage is updated either when calling `switch`, or when receiving a query response, which deletes entries from the map, as defined by the `OnResponse` implementation of the pallet itself. * Two more events for transfer finalisation and reversal have been added. * One error thrown when setting up a new pair with previous pending transfers still in storage is also added. * The pallet takes two more config items that aid in the error recovery process. * The XCM message sent to destination has changed. It now does the transfer as two distinct steps withdraw + deposit, where the second could fail. In either case, a report message is sent back to the source chain that will handle it accordingly. * I updated existing tests to check for the new storage map, and wrote tests for the `OnResponse` implementation. * I added a few support types and traits both in `runtime-common` and `kilt-support`. I will refactor the code in a different PR, where I will also tackle KILTprotocol/ticket#3628. ## How to test You can run a Chopsticks environment, set up a switch pair, and try to do a switch for an account that has enough KILTs but has no ED on AssetHub. The operation will eventually fail and the transfer will be reverted back on KILT. A revert event is generated. You can also test a correct transfer, which will behave as before, while removing the pending transfer storage entry and generating an event for the transfer finalization. ## TODO - [x] Add runtime logic - [x] Test happy case with Chopsticks - [x] Update unit tests to check for the new storage entries - [x] Test with upcoming runtime allowing signed origins to send XCM messages ([PR](polkadot-fellows/runtimes#407)) - [x] Update the hook trait to account for the new state a switch can be in - [x] Add unit tests for the new feature - [x] Update relevant calls, tests and benchmarks to clean up the additional storage - [x] Add event and tests when transfer is finally confirmed - [x] Test case when pair is paused with pending confirmations with Chopsticks - [x] Review XCM components to allow more things while the switch pair is paused (to accommodate the new revert capabilities) - [x] Update docs - [x] Revise log messages and log levels
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.
Relaxing the
SendXcmOriginfilter for some of the System chains:Relay Kusama: already open to signed origins and pluralities ✅
Relay Polkadot: relax to same level as Kusama ^ ✅
Asset Hubs (P and K): open up to signed origins ✅
People (P and K): allow signed origins ✅
Encointer: allow signed origins ✅
Collectives Polkadot: no change, only Fellows collective allowed ❌
Bridge Hubs (P and K): no change, no arbitrary XCM sends allowed ❌
Coretime (P and K): no change, no arbitrary XCM sends allowed ❌
We could allow it on all chains but on some of them there are no usecases afaict, so starting out conservative, opening it up only on chains where it's wanted.