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

Coherence in Asset registration #2339

Closed
appetrosyan opened this issue Jun 8, 2022 · 13 comments
Closed

Coherence in Asset registration #2339

appetrosyan opened this issue Jun 8, 2022 · 13 comments
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@appetrosyan
Copy link
Contributor

That was deliberate, actually, because I wanted to propose restricting registering an asset to a default value (either per value type or per asset definition). So I tried to leave Mints as pairs of Register+Mint here rather than replacing the Mint.

Originally posted by @QuentinI in #2333 (comment)

@appetrosyan
Copy link
Contributor Author

This seems to be the right approach. Registering an asset is not the same as minting an asset to a specific wallet.

It should also be noted that we no longer require for something to be registered in order for it to be Minted, which IMO is a bit of a security risk. This increases the surface area for mint instructions in the permission validators, it makes the process unnecessarily more complex.

@QuentinI
Copy link
Contributor

QuentinI commented Jun 8, 2022

It should also be noted that we no longer require for something to be registered in order for it to be Minted

After #2333 we will have to register assets before minting.

I'm still not sure registering assets to specific accounts makes sense, but assuming we want it, I do think it should be semantically different from minting and create asset with default value. Depending on how flexible we want it to be, it can either be a predefined value for each AssetValue variant or defined per-asset in AssetDefinition.

Relatedly, I only now noticed #2333 as is makes it possible to mint non-mintable assets twice, because you can first set arbitrary value for it when registering, and then mint it. If we decide to stick to register having arbitrary value, we could just prohibit minting non-mintable assets althogether, which will result in desired behaviour; but I'm not sure what we could do otherwise.

@appetrosyan
Copy link
Contributor Author

appetrosyan commented Jun 8, 2022

I have a few questions regarding this

  1. Do we want assets to be mintable directly to any wallet without registering.
  2. Do we want assets to be mintable when an asset is registered, or when an asset definition is registered.
  3. Do we want to register and mint the initial quantity in one instruction or one transaction
  4. Do we want mints to be variadic? Expression based? Explicitly targetted?

AFAIK, the previous consensus was that we want to mint directly to wallets, provided types and permissions are correct. We want to Register definitions and not assets (because that's tedious, and most people will do it in one tx). So it makes no sense to add an initial quantity to a registered asset definition: if you don't have it, you have ZERO, and you don't have ZERO, you just remove the asset from the dashmap.

Regarding the last point, you could argue either way. I think variadic expression based mints would be convenient. If done correctly they can take up less space, and in the average case a person who can mint an asset can mint precisely the right amounts to many wallets.

@appetrosyan
Copy link
Contributor Author

For the record I have not yet made a decision. I think we need to brainstorm a few ideas and maybe poll a few times.

@QuentinI
Copy link
Contributor

QuentinI commented Jun 8, 2022

Considering registration I would say you need to register asset definition, then you can mint without registering asset to account. That actually is how it is now.
#2333 adds support for registering assets to accounts and requires to do it before minting. I was under impression that's what you would like based on this comment.

From my perspective the whole idea of registering asset to account is redundant and I don't see why we would want it.

@mversic
Copy link
Contributor

mversic commented Jun 9, 2022

Relatedly, I only now noticed #2333 as is makes it possible to mint non-mintable assets twice, because you can first set arbitrary value for it when registering, and then mint it. If we decide to stick to register having arbitrary value, we could just prohibit minting non-mintable assets althogether, which will result in desired behaviour; but I'm not sure what we could do otherwise.

From my perspective the whole idea of registering asset to account is redundant and I don't see why we would want it.

  1. If you take NFTs in the picture it would make sense to allow for the initial value to be set when registering an asset to an account. We also wouldn't need Mintable::Once which, to my mind, just introduces confusion for the user. It seems to be that the only reason we've introduced Mintable::Once is because we didn't have a way to register an asset with an initial value

@SamHSmith
Copy link
Contributor

SamHSmith commented Jun 9, 2022

Maybe an asset registration always includes an implicit minting? So that you can register an asset with initial value and nonmintable, or zero initial value and it is mintable, or a combination. This is what you guys have already said so I guess I'm just joining in.

What is the difference between registering an asset and registering an asset definition? Is it like registering fiat currency vs registering the US dollar? I think we should probably avoid too many layers of indirection. Register asset definition -> register asset -> mint/send asset to people, feels slightly too abstract. And I'm not sure what is gained by having asset definitions, other than "These two assets behave the same". And isn't there a problem of duplicate asset definitions? People registering the same thing many times. Then don't we have to compare asset definitions for equivilance anyway?

Also how do domains relate to this question?

@mversic
Copy link
Contributor

mversic commented Jun 9, 2022

Maybe an asset registration always includes an implicit minting? So that you can register an asset with initial value and nonmintable, or zero initial value and it is mintable, or a combination. This is what you guys have already said so I guess I'm just joining in.

We haven't had the suggestion to do both

@appetrosyan
Copy link
Contributor Author

@mversic Indeed that is the case. I fought tooth and nail to keep the API the way it was and so Mintable::Once was necessary. I would encourage revisiting the assumptions and potentially changing the way we mint/register.

@mversic
Copy link
Contributor

mversic commented Jun 9, 2022

What is the difference between registering an asset and registering an asset definition? Is it like registering fiat currency vs registering the US dollar?

Registering an asset definition is more like introducing a support for a US dollar savings accounts and registering an asset is opening a savings account for a particular person

@mversic
Copy link
Contributor

mversic commented Jun 9, 2022

And isn't there a problem of duplicate asset definitions? People registering the same thing many times. Then don't we have to compare asset definitions for equivilance anyway?

The equivalence relation is nominal, not structural. So yes, users can define structurally equal asset definitions, but under different names. I find this a desired behavior

@s8sato
Copy link
Contributor

s8sato commented Jun 9, 2022

  1. Do we want to register and mint the initial quantity in one instruction or one transaction

Since both are already feasible, I think the requirement is to combine existing instructions to provide encapsulation of commonly used patterns.

  1. If I register an asset to an account without registering the definition of the asset to a domain, what does asset.id.definition_id.domain_id represent?

@mversic
Copy link
Contributor

mversic commented Jun 9, 2022

  1. If I register an asset to an account without registering the definition of the asset to a domain, what does asset.id.definition_id.domain_id represent?

That's not allowed and would produce FindError. Asset definition must first be registered and only then can a user assign asset to an account

@appetrosyan appetrosyan added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

5 participants