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

RFC: Custom Asset Claimer #37

Merged

Conversation

mrshiposha
Copy link
Contributor

@mrshiposha mrshiposha commented Jun 21, 2023

Summary

The proposed change provides a way to set a claimer to potentially dropped assets.
Currently, determining a claimer of dropped assets is implementation-specific, and there is no way to set a custom claimer.
The ability to set a custom claimer origin makes it easier to rescue the dropped assets, especially in the case of a reserve-based transfer.

Copy link
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

Awesome work specifying this new instruction! Left some comments but overall the addition seems solid. Thank you for contributing 😁

proposals/0000-dropped-assets-claimers.md Outdated Show resolved Hide resolved
proposals/0000-dropped-assets-claimers.md Outdated Show resolved Hide resolved
proposals/0000-dropped-assets-claimers.md Outdated Show resolved Hide resolved
proposals/0000-dropped-assets-claimers.md Outdated Show resolved Hide resolved
proposals/0000-dropped-assets-claimers.md Outdated Show resolved Hide resolved
proposals/0000-dropped-assets-claimers.md Outdated Show resolved Hide resolved
proposals/0000-dropped-assets-claimers.md Outdated Show resolved Hide resolved
mrshiposha and others added 4 commits June 21, 2023 17:18
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/xcm-rfc-dropped-assets-claimers/3176/1

@gavofyork
Copy link
Contributor

I suspect that being able to split up assets arbitrarily and allow for numerous claimants is probably going to be difficult to guarantee economic security. I think something like SetAssetTrapClaimer(MultiLocation) could work ok. Would this be sufficient to deliver the main benefits?

@mrshiposha
Copy link
Contributor Author

mrshiposha commented Jul 6, 2023

I agree with Gavin's comment above. Indeed, having just one claimer for all the dropped assets should be enough to rescue them conveniently. Also, this instruction will be much easier to implement.

If there is a need for complex claiming logic, a smart contract could be set as a claimer.

The RFC is modified accordingly.

@mrshiposha mrshiposha changed the title RFC: Dropped Assets Claimers RFC: Custom Asset Claimer Jul 6, 2023
@mrshiposha
Copy link
Contributor Author

@franciscoaguirre, the RFC is modified. Could you take a look, please?

Copy link
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

I like the simplification 🚀

@franciscoaguirre
Copy link
Contributor

Starting the FCP (Final Comment Period), it will last 10 calendar days. After that, this RFC will be either accepted, rejected or explicitly postponed for a future time.

@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/final-comment-period-for-two-xcm-rfcs/3516/1

@franciscoaguirre
Copy link
Contributor

franciscoaguirre commented Aug 3, 2023

The FCP for this RFC is over. The RFC has been accepted!

The next steps would be to:

  • Change the proposal name to 0037, since that's the PR number
  • Change the proposal number to 37
  • Change the proposal version to 4, since it will be included in XCMv4
    After that we can merge it. Once it's merged you should open a new PR that modifies README.md with the new instruction.

How does that sound @mrshiposha ?

@mrshiposha
Copy link
Contributor Author

@franciscoaguirre sounds great!

@mrshiposha
Copy link
Contributor Author

@franciscoaguirre I updated the RFC info:

  • The RFC file is now named 0037-custom-asset-claimer.md
  • The Number is now 37
  • The Status is now Accepted
  • The Version is now: 4

@franciscoaguirre franciscoaguirre merged commit b52ab1d into polkadot-fellows:master Aug 4, 2023
@franciscoaguirre
Copy link
Contributor

@mrshiposha
Perfect. Could you create a new PR to master modifying the README?

@mrshiposha
Copy link
Contributor Author

@franciscoaguirre Sure! Here it is: #41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants