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.

Sign up for GitHub

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: New instruction for the reservation of asset classes/IDs #35

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

RFC: New instruction for the reservation of asset classes/IDs #35

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
93bcae2
create asset instruction draft
vstam1 Jun 9, 2023
796cbf2
move proposal to right place
vstam1 Jun 9, 2023
9d9c4b9
remove file
vstam1 Jun 9, 2023
9ec98b2
add some more info
vstam1 Jun 12, 2023
49923ad
more info and example
vstam1 Jun 12, 2023
36b2591
change to InitializeAsset instruction
vstam1 Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions proposals/0000-create-asset-instruction.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ Replaces:

## Summary

The proposed change is to introduce a new Instruction called `CreateAsset`. The instruction allows foreign chains to create fully-backed derivatives. Currently these derivatives are created using the `Transact` instruction, or directly on the sovereign chain.
The proposed change is to introduce a new Instruction called `CreateAsset`. The instruction allows foreign chains to create fully-backed derivatives. Currently, these derivatives are created using the `Transact` instruction, or directly on the sovereign chain.


## Motivation

Currently the `Transact` instruction is used to create derivatives. The `Transact` instruction requires knowledge of the foreign system to determine which call to make. The call can differ per system, and in FRAME-based systems can depend on different instances of a pallet. The CreateAsset instruction abstracts the need to know this call away. Sending consensus systems can send this instruction to any chain without having to know the structure of the destination chain.
Currently, the `Transact` instruction is used to create derivatives. The `Transact` instruction requires knowledge of the foreign system to determine which call to make. The call can differ per system, and in FRAME-based systems, it can depend on different instances of a pallet. The CreateAsset instruction abstracts away the need to know this call. Sending consensus systems can send this instruction to any chain without having to know the structure of the destination chain.

## Specification

Expand All @@ -28,10 +28,23 @@ The instruction would have the following specification:
CreateAsset {asset: MultiAsset, owner: MultiLocation}
Copy link
Contributor

@gavofyork gavofyork Jun 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I would instead call this AcquireAssetId, since:

a) you're not actually creating any assets; and
b) you're appointing a controller for the given asset class/ID.

Alternatives are:

  • OccupyAssetId
  • SeizeAssetId
  • TakeAssetId
  • AppointAssetIdAdmin
  • ProcureAssetId
  • ObtainAssetId

My favourites are ReserveAssetId and HoldAssetId, however these clash with pre-existing terminology.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about CreateAssetId or CreateAssetType or CreateAssetClass?

Copy link
Contributor

@gavofyork gavofyork Jun 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think you're really creating anything so much as priming some namespace which is already yours to do so by virtue of the origin.

Perhaps InitializeAsset or SetupAsset..

```

The `CreateAsset` instruction allows for the creation of a single asset. This can be a fungible or non fungible asset.
The `CreateAsset` instruction allows for the creation of a single asset. This can be a fungible or non-fungible asset based on the specification of the `asset` field. The MultiAsset can describe a single asset. The `id` field is either an asset identity for a fungible asset or a class for a non-fungible asset. The fun field represents either the amount in the case of a fungible asset or the instance Id for a non-fungible asset.
Copy link
Contributor

Choose a reason for hiding this comment

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

either the amount in the case of a fungible asset

@vstam1
what amount? e.g. for pallet_assets::create we need min_balance, is this it?

just thinking, if it would be better like this, not sure:

CreateAsset {
    id: Concrete(MultiLocation {parents: 1, interior: X1(GeneralIndex(1))})
    fun: NonFungible(?),
    owner: MultiLocation {parents: 1, interior: Here},
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These instructions are not connected to FRAME-based systems, so this instruction should also be usable to create an ERC20 token, for example, where the amount can represent the total_supply.

Like you said, the amount could represent the min_balance for the assets_pallet.

I do not directly see the difference between the CreateAsset with the assets field or with the id and fun fields split as the MultiAsset has both of these fields. However, if we split the fields, we could set the fun to Option<Fungibility>.

Copy link
Contributor

Choose a reason for hiding this comment

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

thx, I know they are not connected and xcm should be disconnected from frame as much possible,

ideally, fields should be self explanatory and also to avoid confusion or misuse,
so e.g. MultiAsset.fun.amount if user thinks about this amount as "total_supply" and on target system, we use this with several "AssetTransactors" where one can interpret this as "total_supply" and other as "min_balance", it could lead to problems
so for this case, fields split, as you said with Option could work better:

CreateAsset {
    id: Concrete(MultiLocation {parents: 1, interior: X1(GeneralIndex(1))})
    total_supply: Option<Funbility>,
    min_balance: Option<Funbility>,
    owner: MultiLocation {parents: 1, interior: Here},
}

we can go also with some properties wrapper and so on :)

AssetProperties {
    total_supply: Option<Funbility>,
    min_balance: Option<Funbility>,
}

yeah, I understand that this should be pretty generic to cover all cases,
so I am interesting where this will go :)

Copy link
Contributor Author

@vstam1 vstam1 Jun 13, 2023 •
edited by gavofyork
Loading

Choose a reason for hiding this comment

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

Yeah you are definitely right that it could cause serious problems (imagining an asset that has its min balance set to value that should be total_supply haha). I would like to see more feedback on this approach. Especially the AssetProperties approach as it might also solve the Metadata related question.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of an AssetProperties, but I think we can't reuse Fungibility for those fields.
I think a possible approach would be to make the fungibility separation in the properties themselves.

enum AssetProperties {
  Fungible {
    total_supply: u128,
    min_balance: u128,
    // probably some other fields
  },
  NonFungible {
    collection_id: u64,
    // probably more fields
  },
}

If we add the largest amount of properties pertaining to that fungibility as possible, if some implementations don't handle some of them, like collections for example, they can just ignore that field.

Copy link
Contributor

@gavofyork gavofyork Jun 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We could reasonably have an AssetProperties field in there I guess. It'll be a bit weird through since we'll need to make all field Optional otherwise we'll force the sender to introduce garbage info for fields which don't make sense for their asset and hope that the receiver ignores them. But then not all receive will support all fields. I guess we can have a convention of strictness, whereby receivers require some fields to be Some (if their implementation really needs them) or None (if their implementation doesn't support them and they're actually important). Other fields it might be flexible on.

enum Instruction {
  /* snip */
  InitializeAsset {
    /// The (concrete) asset ID, relative to the receiver/executor. This is what can
    /// be used as an asset ID when minting or teleporting assets into the chain.
    asset_id: MultiLocation,
    /// The owner location, relative to the receiver/executor. Some implementations
    /// might support all kinds of locations, others might only support
    /// `{ parents: 0, interior: X1(AccountId32 { .. }) }` patterns.
    owner: MultiLocation,
    /// Relevant properties of the asset class/ID to be initialized. The
    /// implementation may require some fields to be `Some` and others `None`.
    properties: AssetProperties,
  }
}
enum AssetProperties {
  Fungible {
    total_supply: Option<u128>,
    min_balance: Option<u128>,
  },
  NonFungible,
}

What would the proposed collection_id be? We have a named owner already for the asset class/ID who can actually mint the (NF)Tokens. If collection_id is passed anywhere, I'd have expected it to be passed when minting, not initializing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea of calling the instruction InitializeAsset, as well as adding the AssetProperties field. Is there already a standard for NFT metadata?


## Security considerations
The `owner` field is a MultiLocation representation of the owner of the to-be-created asset. MultiLocations can represent all sorts of consensus systems, from a 32-byte representation of a user account using the `AccountId32` Junction to a governance instance using the `Plurality` Junction.

One of the most common use cases of the `CreateAsset` instruction is the creation of derivate assets. Take the following scenario, for example, where we have two chains (A and B), B is an interior location to A (relay chain <-> parachain relation), and chain A has a non-fungible asset with asset class 1. Now we create a derivative asset on chain B using the `CreateAsset` instruction with as owner Chain A. The instruction will look as follows:
```rust
CreateAsset {
asset: MultiAsset {
id: Concrete(MultiLocation {parents: 1, interior: X1(GeneralIndex(1))})
fun: NonFungible(?) // See open questions
},
owner: MultiLocation {parents: 1, interior: Here},
}
```

## Security considerations
The XCVM implementation has to check if the origin is allowed to create this asset.
Copy link
Contributor

Choose a reason for hiding this comment

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

In which case is the origin allowed to create the asset?

Copy link
Contributor

@gavofyork gavofyork Jun 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I suppose this depends entirely on the implementation. For what it's worth, the initial impl for AssetHub will allow any location to create its own asset and assets at any sublocations, which is a perfectly reasonable convention.


## Impact
The impact of this proposal is Low. It introduces a new instruction, so XCVM implementations have to be updated.
Expand All @@ -40,7 +53,8 @@ The impact of this proposal is Low. It introduces a new instruction, so XCVM imp
As previously discussed, it is possible to create assets using the `Transact` instruction.

## Questions and open Discussions (optional)
- How do we represent the `fun` field in the MultiAsset struct? In most of the FRAME-based systems we currently have, the creation of assets does not require an amount in the case of fungible assets or an instance id in the case of non-fungible assets.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would avoid it altogether by not using a MultiAsset but instead just the id portion of it.

This would require exposing the AssetId, which is fine. But I would also propose that XCMv4 remove the AssetId::Abstract variant and thus make the id just a straight MultiLocation. This would make the instruction contain the fields { asset_id: InteriorMultiLocation, owner: MultiLocation }, where asset_id is an interior location of Origin.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I think there's not actually any need to require that asset_id be an InteriorMultiLocation. Some chains might privilege certain origins to be able to create assets which are not interior to them. So this becomes: { asset_id: MultiLocation, owner: MultiLocation }.


- Currently the instruction allows for the creation of a single asset. Should the instruction allow for the creation of multiple assets?
- Currently, the instruction allows for the creation of a single asset. Should the instruction allow for the creation of multiple assets?

- How should we handle metadata?