Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

XCM: Allow reclaim of assets dropped from holding #3727

Merged
merged 16 commits into from
Aug 28, 2021
Merged

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented Aug 26, 2021

Introduces a mechanism for systematically allowing the rescue of lost contents of the Holding Register.

Fixes #3694

Trapping assets

XCM Executor's config now includes AssetTrap type which is a hook to which the contents of the Holding Register is passed at the close of execution. Like OnResponse, a (recommended) implementation is provided by the XCM pallet.

This will record a hash of the assets left in the Holding Register at the finish of execution together with the origin. A later claim may be made from the origin in order to resurrect the assets. This is done through the single-function trait ClaimAssets which verifies that the assets to be claimed match a hash placed by the same origin.

If the version with which the MultiAssets have been stored is not the most recent version, then the version index must be included within the claim ticket parameter, a MultiLocation as a GeneralIndex.

ClaimAsset

A new instruction is provided, ClaimAsset, which gains access to any ClaimAssets implementations exposed to the XCM Executor. If the pallet_xcm::Pallet implementation is exposed and the origin has unclaimed assets trapped, then those assets may be provided (exactly, as a hash of them is used to validate the claim) together with a ticket reflecting the version of XCM at the time of the trap (or Here if unchanged) and they will be resurrected into the Holding Register.

Additional

Also introduces the Trap instruction and error type to force an error to be thrown, and be able to recognise it. It comes with a u64 parameter to allow some degree of information to be conveyed with it (without bloating the stack footprint of XcmError).

Migration

xcm_executor::Config

Two new type definitions are required, and they'll generally want to point at the XCM Pallet concrete type if there is one, e.g.:

impl xcm_executor::Config for ExecuteXcm {
	/* snip */
	type AssetTrap = XcmPallet;
	type AssetClaims = XcmPallet;
}

Note that destinations which use v1 or v0 for message transport will not be able to use these features (even if the destination supports v2).

TODO

  • ShouldTrap filter type to allow runtimes to dictate which assets/origins should be trapped.
  • Unit test.

@gavofyork gavofyork added A3-in_progress Pull request is in progress. No review needed at this stage. B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Aug 26, 2021
@gavofyork gavofyork added this to the v0.9.11 milestone Aug 26, 2021
@gavofyork gavofyork added this to In Progress in Cross Chain Message Transport via automation Aug 26, 2021
@gavofyork gavofyork added this to In progress in Runtime via automation Aug 26, 2021
@gavofyork gavofyork added this to In progress in XCM via automation Aug 26, 2021
@gavofyork gavofyork marked this pull request as draft August 26, 2021 10:47
@gavofyork gavofyork mentioned this pull request Aug 26, 2021
47 tasks
@gavofyork gavofyork added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Aug 27, 2021
@gavofyork gavofyork marked this pull request as ready for review August 27, 2021 21:18
@gavofyork gavofyork changed the title XCM: Introduce AssetTrap XCM: Allow reclaim of assets dropped from holding Aug 27, 2021
xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
ClearError,

/// Create some assets which are being held on behalf of the origin.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean "claim" instead of "create" here?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this context they're interchangeable - the Assets will be "created" in the sense that the MultiAsset instance(s) will be created.

Runtime automation moved this from In progress to Needs Audit Aug 27, 2021
Copy link
Contributor

@KiChjang KiChjang left a comment

Choose a reason for hiding this comment

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

Nothing really stands out as concerning, aside from some minor documentation stuff.

@gavofyork gavofyork merged commit 8ea7669 into master Aug 28, 2021
@gavofyork gavofyork deleted the gav-asset-trap branch August 28, 2021 00:09
Cross Chain Message Transport automation moved this from In Progress to Done Aug 28, 2021
Runtime automation moved this from Needs Audit to Done Aug 28, 2021
XCM automation moved this from In progress to Done Aug 28, 2021
@shawntabrizi shawntabrizi moved this from Done to Archive in Runtime Sep 2, 2021
@shawntabrizi shawntabrizi moved this from Done to Archive in XCM Sep 2, 2021
ordian added a commit that referenced this pull request Sep 2, 2021
* master:
  dependabot: ignore yet another git dep (#3759)
  Bump serde_json from 1.0.66 to 1.0.67 (#3767)
  Bump syn from 1.0.74 to 1.0.75 (#3710)
  Companion for substrate #9371 (#3487)
  Fixes/improvements for disputes (#3753)
  chore: test helper arbitrary ordering for 2 (#3762)
  disputes: fix relay chain selection sanity check (#3750)
  technical committee is using the weight of council, but should have its own generated weight instead (#3511)
  new proxy for auctions, crowdloans, registrar, slots (#3683)
  Bump libc from 0.2.100 to 0.2.101 (#3726)
  Removed unneeded deps (#3658)
  Bump serde from 1.0.127 to 1.0.130 (#3739)
  Companion for Generate storage info for pallet authority_discovery #9428 (#3517)
  Return a Result in invert_location (#3730)
  XCM: Allow reclaim of assets dropped from holding (#3727)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited.
Development

Successfully merging this pull request may close these issues.

XCM: Asset trap to avoid accidental burning of Holding Register contents
2 participants