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

Disable creation of local XC-20s & remove tests #2508

Merged
merged 4 commits into from
Oct 10, 2023

Conversation

fgamundi
Copy link
Contributor

@fgamundi fgamundi commented Oct 2, 2023

What does it do?

Substrate local XC-20 assets have been deprecated in favor of EVM ERC20 XCM tokens. This is the first of multiple PRs to deprecate localAssets in Moonbeam.
Sets the ForceOrigin of LocalAsset config to Never and remove related tests.

What important points reviewers should know?

Even though local assets are meant to be registered through assetManager.registerLocalAsset, setting the localAssets config origin to never is enough to make that extrinsic fail, effectually disabling the creation of new local XC20s, as shown with a dev env:
image

Is there something left for follow-up PRs?

Existing local XC20s in Moonbeam are not actively used. After the assets has been destroyed (through governance), follow-up PRs should be made to remove the localAssets pallet from the runtime

@fgamundi fgamundi added A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit breaking Needs to be mentioned in breaking changes labels Oct 2, 2023
@fgamundi fgamundi self-assigned this Oct 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

Coverage generated "Mon Oct 2 23:29:56 UTC 2023":
https://d3ifz9vhxc2wtb.cloudfront.net/pulls/2508/html/index.html

Master coverage: 87.39%
Pull coverage:

@fgamundi fgamundi changed the title Disable creation of local XC-20s Disable creation of local XC-20s & remove tests Oct 3, 2023
@fgamundi
Copy link
Contributor Author

fgamundi commented Oct 3, 2023

@librelois @Agusrodri thanks for the review!
I've removed all tests related to local assets, could you please rereview? The typescript tests that shows as modified really are that the file was replaced with another and Git takes it as a modification (like test-hrmp-...-5 was removed and test-hrmp-...-6 was renamed to 5)

@Agusrodri
Copy link
Contributor

@fgamundi the changes applied to tests look good 👍

@fgamundi fgamundi merged commit 69d8dcb into master Oct 10, 2023
29 of 30 checks passed
@fgamundi fgamundi deleted the fg-disable-localasset-create branch October 10, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants