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

333 extend the orml tokens pallet so users are able to mint and burn custom assets #359

Conversation

gianfra-t
Copy link
Contributor

@gianfra-t gianfra-t commented Nov 29, 2023

Closes #333.

Note: This is still a very basic implementation. More functionality needs to be defined first.

General Overview

This pallet allows any user to "create" or "take" a CurrencyId:Token(u64), and define the controlling accounts.

Once taken, the owner can execute extrinsics mint and burn which calls orml_currencies functions deposit and withdraw respectively (this in turn will interact with orml_tokens as defined in the issue)

The StorageMap will hold the information about which CurrencyId:Token(u64) has already been taken, and the accounts that are able to mint, burn and transfer ownership.

Root Capabilities

Root can only transfer the ownership of the currency, which essentially allows to perform any operation.

@gianfra-t
Copy link
Contributor Author

gianfra-t commented Nov 29, 2023

We should define some more things before moving forward. Some of them I can think of right now are:

  • Do we need just the basic mint and burn functionality or we also need freezing assets, force transfer, etc.
  • Do we want to allow for the destruction of a currency as it is done in assets_pallet? If so, how do we deal with token holders?

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Looks good overall. We should probably add benchmarks but in my opinion we don't need to support freezing or anything fancy just yet.

pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/Cargo.toml Outdated Show resolved Hide resolved
@ebma
Copy link
Member

ebma commented Dec 7, 2023

Is this still a draft? Or is it ready for review now @gianfra-t?

@gianfra-t gianfra-t marked this pull request as ready for review December 7, 2023 14:41
@gianfra-t
Copy link
Contributor Author

I believe it's ready. It does not have much functionality though but we decided to make destruction a different ticket as I recall.

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Let's also require the owner of a currency to reserve a deposit when owning it, see here. This deposit cannot be unreserved until we add the 'destroy' feature in the future but it is very important because otherwise users could just claim ownership of all assets, squatting on them and distracting the usefulness of the feature overall. Note that the deposit reserve needs to be 'transferred' to the new owner when transferring ownership, see here, ie the new owner should then have to reserve some balance and it should be freed from the original owner.

pallets/orml-tokens-extension/Cargo.toml Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
pallets/orml-tokens-extension/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

@gianfra-t I added changes that remove the extra config type, please have a look. I did not re-generate the default weights yet, we should probably do that again now that we require the user to deposit some amount when claiming ownership.

pallets/orml-currencies-allowance-extension/src/mock.rs Outdated Show resolved Hide resolved
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Great 👍 ready for merge IMO

@gianfra-t gianfra-t merged commit 04a4d27 into main Dec 11, 2023
2 checks passed
@gianfra-t gianfra-t deleted the 333-extend-the-orml-tokens-pallet-so-users-are-able-to-mint-and-burn-custom-assets branch December 11, 2023 13:47
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.

Extend the orml-tokens pallet so users are able to mint and burn custom assets
2 participants