XRPLF / xrpl-dev-portal

Source code for xrpl.org including developer documentation
https://xrpl.org
Other
520 stars 1.01k forks source link

Documentation missing regardin the `asfAuthorizedNFTokenMinter` flag and the `NFTokenMinter` property #1627

Closed nixer89 closed 1 year ago

nixer89 commented 1 year ago

With an AccountSet transaction you can authorize another account to be able to mint NFT "in your name". To do so you have to set the NFTokenMinter property and enable the AccountSet-Flag asfAuthorizedNFTokenMinter.

But this flag is not documented as AccountRoot flag:

https://xrpl.org/accountroot.html#accountroot-flags

So it is currently not possible to check if the Flag asfAuthorizedNFTokenMinter is enabled on an account or not.

nixer89 commented 1 year ago

ok, I just learned (by studying the code) that if the NFTokenMinter wants to be set, you also have to set the flag asfAuthorizedNFTokenMinter in the same transaction.

But to remove the NFTokenMinter, you "simply" have to clear the flag asfAuthorizedNFTokenMinter.

So there is never an AccountRoot flag for asfAuthorizedNFTokenMinter.

But I think the documentation is missing some info about setting and removing the NFTokenMinter. Especially that you have to set the flag AND the NFTokenMinter in the same transaction. And Clearing the Flag, actually removes the AccountRoot Property NFTokenMinter.

scottschurr commented 1 year ago

There appears to be some confusion here.

  1. The asfAuthorizedNFTokenMinter flag is used only in the AccountSet transaction. That flag indicates whether the transaction is intending to affect the presence or value of the NFTokenMinter field on an account root. Specifically, there is no corresponding flag on the AccountRoot.

  2. If you want to know whether a specific AccountRoot supports an NFTokenMinter, then look for the presence of the optional NFTokenMinter field in the AccountRoot object. If the NFTokenMinter field is absent from the AccountRoot, then that AccountRoot does not have an authorized NFTokenMinter. If the NFTokenMinter field is present, then the AccountRoot does have an authorized NFTokenMinter, and the value of the field tells you who that authorized NFTokenMinter is.

Does that clarify what's going on?

nixer89 commented 1 year ago

Hi @scottschurr ! Yes, I figured this out by checking the code.

I was indeed confused at first due to some missing documentation regarding this "special" case. I since changed the title of the issue to "missing documentation" :)

Your explanation very much clarifies the topic regarding the AccountRoot and NFTokenMinter properties.

Is there a reason we have the asfAuthorizedNFTokenMinter for double checking when setting the NFTokenMinter ? This is much different from other AccountSet fields like the Domain, EmailHash, MessageKey or even when setting a RegularKey (through the SetRegularKey transaction)

scottschurr commented 1 year ago

@nixer89, yes there is a reason. The availability of the flag makes it possible to remove the NFTokenMinter field. There is no such think as an "empty" AccountID (unlike a Domain or EmailHash). We need a way to indicate that the NFTokenMinter field should be removed. Clearing the flag provides that indication. For symmetry we therefore require setting the flag when the NFTokenMinter field is added.

nixer89 commented 1 year ago

okay I see, thanks Scott. This is surely a bit confusing. I think it makes sense to add a hint about this in the docs.

Since it is very different from other AccountSet string properties (Domain) and very different from setting another AccountID property to the AccountRoot object (RegularKey)

DennisDawson commented 1 year ago

Adding a new topic, PR https://github.com/XRPLF/xrpl-dev-portal/pull/1663, to provide a description for how to assign and unassign an authorized minter, including the clarification from above.