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: Rename locking instructions #39

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vstam1
Copy link
Contributor

@vstam1 vstam1 commented Jul 18, 2023

Summary

This RFC proposes renaming several XCM instructions to align with changes in Substrate naming.
The following instructions and error are affected:

Instructions:
LockAsset -> FreezeAsset
UnlockAsset -> ThawAsset
NoteUnlockable -> NoteThawable
RequestUnlock -> RequestThaw

Error:
LockError -> FreezeError

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.

My main concern with this is that we want the wider ecosystem to adopt XCM, and the main terminology used is asset locking/unlocking. Imposing a new term for this could make adoption harder.

proposals/0000-rename-locking-instructions.md Outdated Show resolved Hide resolved

## Impact

This change will impact all XCM code that uses the lock instructions, constituting a breaking change. All code should be updated to use these new names. Failure to update the code could result in errors.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep in mind this will probably go to a new XCM version, which means the migration is pretty straightforward.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is a breaking change in the API level, but not on the transaction layer, since the codec indices for these instructions did not change, nor did the instructions parameters change.

proposals/0000-rename-locking-instructions.md Outdated Show resolved Hide resolved
proposals/0000-rename-locking-instructions.md Outdated Show resolved Hide resolved
@vstam1
Copy link
Contributor Author

vstam1 commented Jul 24, 2023

My main concern with this is that we want the wider ecosystem to adopt XCM, and the main terminology used is asset locking/unlocking. Imposing a new term for this could make adoption harder.

I do also think that keeping the old terminology is confusing once we updated from locking -> freezing in FRAME.

@xlc
Copy link
Contributor

xlc commented Aug 2, 2023

The XCM spec shouldn't care about FRAME. We may want to do the rename but the reason shouldn't be because FRAME did it. If anything, it is the FRAME should honor XCM's naming convention.

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.

4 participants