iroha [suggestion] Eliminate `AssetValueType` inconsistencies

Feature request

There are several value types for assets exist:

"AssetValue": {
  "Enum": [
    {
      "tag": "Quantity",
      "discriminant": 0,
      "type": "u32"
    },
    {
      "tag": "BigQuantity",
      "discriminant": 1,
      "type": "u128"
    },
    {
      "tag": "Fixed",
      "discriminant": 2,
      "type": "Fixed"
    },
    {
      "tag": "Store",
      "discriminant": 3,
      "type": "Metadata"
    }
  ]
}

Basically just Numeric and Store. Although:

One can Transfer, Mint and Burn assets of type Numeric, but not assets of type Store
One can SetKetValue and RemoveKeyValue in relation to a Store asset, but not Numeric one
Once/Infinitely mintability makes little sense since it does not affect Store assets at all
A Store asset cannot be destroyed, there will be empty metadata container in the end

Motivation

Current situation hardly can show Store and Numeric assets being just a type of the same entity. They feel totally different entities.

I'd support either enabling the same set of instructions to be applicable to any asset with meaningful outcome or make Store asset something different than an asset. In the latter case an adjustment to enable deleting that somehow would be still worth considering.

Who can help?

No response

Nov 27 '23 13:11 Mingela

@mversic @Erigara I'd like to work on this and the related tickets: #3924 and #4101.

Also, while working on examples/docs, I struggled to motivate the distinction between registering and minting assets, since minting/setting a key-value pair on a non-existing asset will create it anyway, making Register somewhat redundant and possibly footgun-prone in the permissions department (e.g. having a permission to mint but not register will effectively let me register). I think it makes sense to generally reconsider the data model and make it more explicit and DRY.

IMO, Register should only give the account an ability to hold assets of a certain definition. Mint, Burn, etc. should only increase or decrease the value, returning an error if there is no "wallet" to hold the asset.

Jun 25 '24 06:06 nxsaken

@mversic @Erigara I'd like to work on this and the related tickets: #3924 and #4101.

Also, while working on examples/docs, I struggled to motivate the distinction between registering and minting assets, since minting/setting a key-value pair on a non-existing asset will create it anyway, making Register somewhat redundant and possibly footgun-prone in the permissions department (e.g. having a permission to mint but not register will effectively let me register). I think it makes sense to generally reconsider the data model and make it more explicit and DRY.

IMO, Register should only give the account an ability to hold assets of a certain definition. Mint, Burn, etc. should only increase or decrease the value, returning an error if there is no "wallet" to hold the asset.

Either that or remove Register<Asset> since it's anyway already possible to register it via minting it. And what about Transfer<Asset>. Should asset first be registered? IMO it doesn't make sense to first register asset in the case of transfer

Jun 25 '24 06:06 mversic

@mversic I think both are valid depending on our goals. If we want to provide explicit instructions, we should disallow implicit Registers via mints and transfers and ask users to Register an ability to hold an asset explicitly. If we don't want the notion of first being able to hold an asset to acquire it, then we should remove Register and Unregister – but I like that less because having none of the asset would be the same as having zero of it. Some users may want to use the none case to mean that the user does not have a respective wallet yet, and I think we should provide that flexibility.

Jun 25 '24 06:06 nxsaken

Imo it kinda strange to have Register and Unregister for assets, so implicit balance update through Mint, Burn, Transfer should be enough.

Jun 28 '24 08:06 Erigara

@Erigara my reasoning is that even though I have an overall user account in FooBank (equivalent to Iroha Account) and, say, KZT (Iroha Asset Definition) and USD money accounts (Iroha Asset), I can't just accept transfers or do cash-ins in EUR without opening a EUR account first. It can be meaningful to have None of the Asset be different than Some(0) or Some(<EmptyMap>). Maybe it's fine to conflate them.

Jun 28 '24 09:06 nxsaken

@Erigara my reasoning is that even though I have an overall user account in FooBank (equivalent to Iroha Account) and, say, KZT (Iroha Asset Definition) and USD money accounts (Iroha Asset), I can't just accept transfers or do cash-ins in EUR without opening a EUR account first.

on the other hand I can have a multi-currency account in my bank and be able to receive any payment

Jun 28 '24 09:06 mversic

@mversic but you have to ask for it, it doesn't come with the user account. And multi-currency accounts are in reality just multiple accounts abstracted away into a single one (though depends on your country probably)

Jun 28 '24 09:06 nxsaken

Imo it kinda strange to have Register and Unregister for assets, so implicit balance update through Mint, Burn, Transfer should be enough.

I agree, in my opinion Register/Unregister seem semantically dubious and just add more friction for the user

can we also get some insight how other blockchains do it? some input from @Mingela and @takemiyamakoto would be highly appreciated

Jun 28 '24 10:06 mversic

Continuing a discussion on NFTs and store assets raised in the documentation repo (https://github.com/hyperledger/iroha-2-docs/pull/502#discussion_r1664115460) here. Basically, we should also think about introducing a proper non-fungible asset type and/or make store assets more consistent within the same definition.

cc: @s8sato

Jul 05 '24 07:07 nxsaken

Re: https://github.com/hyperledger/iroha/issues/4672#issuecomment-2210565693

Do we need a fungible Store asset type? It would also allow creating multiple instances based on the prototype, with the prototype dictating which keys are allowed and which are required. (Probably not, the instances would not be identical even if they had similar keys; fungibility implies interchangeability)

Jul 05 '24 10:07 nxsaken

I propose the following names:

AssetDefinition/AssetDefinitionId (numeric) -> Fungible/FungibleId
Asset/AssetId (numeric) -> Wallet/WalletId
AssetDefinition/AssetDefinitionId (store) -> NonFungible/NonFungibleId
Asset (store) -> remove, non-fungible assets are unique

Names would be shorter, and there would no longer be confusion between assets and asset definitions.

Jul 15 '24 06:07 nxsaken

Let me comment on specific statements,

Agree on removal of Register, Unregister for assets.
As for the permission to limit ownership, for example in Iroha1 there are can_transfer and can_receive permissions to implement that, we might consider similar structure if there is such a desire.

AssetDefinition/AssetDefinitionId (numeric) -> Fungible/FungibleId

Asset/AssetId (numeric) -> Wallet/WalletId

AssetDefinition/AssetDefinitionId (store) -> NonFungible/NonFungibleId

Does not provide enough clarity how to handle NonFungible transfers and getting one's balance requests given WalletId comprises an AccountId. What about removing Asset/Wallet at all replacing that with a query, e.g. GetBalanceOf<>For<>(AssetDefinitionId, AccountId)? Which, in turn, returns Value (Numeric/Store). And leave the Walletish abstraction just up to the WSV internals. Isn't an Account a collection of <AssetDefinition, Value> in that regard?

Jul 15 '24 07:07 Mingela

how to handle NonFungible transfers

NonFungible is simply an AssetDefinition+Asset for store assets, and ownership of those can be transferred between accounts. It doesn't make sense to have AssetId for store assets because they are not interchangeable. NonFungible should be both a definition and the value. This diagram reflects the idea: https://github.com/hyperledger/iroha/issues/4672#issuecomment-2210565693.

getting one's balance requests given WalletId comprises an AccountId

WalletId is just a more explicit name for AssetId, no change implementation-wise. Eventually Asset/Wallet should disappear along with other similar structs, when #3921 is completed. I think removing it should be out of this issue's scope because it requires a deeper ISI refactor.

Separating FungibleId (asset#domain) and NonFungibleId (unique_hash$domain or unique_hash) would simplify the data model, e.g. remove a lot of runtime checks. Alternatively we would need an extra mechanism that maps a universal asset id to a fungible or non-fungible id. I think the former is better, since fungible and non-fungible assets are so different conceptually.

Jul 15 '24 07:07 nxsaken

I think removing it should be out of this issue's scope because it requires a deeper ISI refactor.

Alright, just make sure the further step is implemented within the same release so that there is no excessive overhead propagated to SDKs 😃

Jul 15 '24 07:07 Mingela