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

feat: add support for cross-chain switch reversal #744

Merged
merged 32 commits into from
Oct 8, 2024
Merged

Conversation

ntn-x2
Copy link
Member

@ntn-x2 ntn-x2 commented Oct 2, 2024

Fixes https://github.com/KILTprotocol/ticket/issues/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 https://github.com/KILTprotocol/ticket/issues/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

  • Add runtime logic
  • Test happy case with Chopsticks
  • Update unit tests to check for the new storage entries
  • Test with upcoming runtime allowing signed origins to send XCM messages (PR)
  • Update the hook trait to account for the new state a switch can be in
  • Add unit tests for the new feature
  • Update relevant calls, tests and benchmarks to clean up the additional storage
  • Add event and tests when transfer is finally confirmed
  • Test case when pair is paused with pending confirmations with Chopsticks
  • Review XCM components to allow more things while the switch pair is paused (to accommodate the new revert capabilities)
  • Update docs
  • Revise log messages and log levels

@ntn-x2 ntn-x2 self-assigned this Oct 2, 2024
@ntn-x2 ntn-x2 force-pushed the aa/transfer-revert branch 2 times, most recently from d6289fa to e9a4c1a Compare October 3, 2024 10:02
@ntn-x2 ntn-x2 marked this pull request as ready for review October 8, 2024 09:01
@ntn-x2 ntn-x2 requested a review from Ad96el October 8, 2024 09:01
Copy link
Member

@Ad96el Ad96el left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments alongside the offline discussion about the query Id

pallets/pallet-asset-switch/src/lib.rs Show resolved Hide resolved
pallets/pallet-asset-switch/src/mock.rs Outdated Show resolved Hide resolved
@ntn-x2 ntn-x2 force-pushed the aa/transfer-revert branch from eaf2922 to cb42bbb Compare October 8, 2024 15:35
@ntn-x2 ntn-x2 merged commit f2c90ce into develop Oct 8, 2024
2 checks passed
@ntn-x2 ntn-x2 deleted the aa/transfer-revert branch October 8, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants