Call for Participation: NFT Token Standard Working Group [Status Updated]

icrc30_get_token_approvals

There is a discrepency between this title and the function in the example for the end point…I think “token” is missing.

icrc30_get_approvals : (token_ids : vec nat, prev : opt TokenApproval; take : opt nat)
    -> (vec TokenApproval) query;
2 Likes

Good point, it probably makes lots of sense to move the memo to the top level and also have it mandatory in that case in order to be able to differentiate between the transfer-initiated and user-initiated revocation of approvals. A main use case of this is also the implicit revocation of approvals when a transfer happens (I originally read your comment to be related to this).

Doing so also would be in line with the established best practices as also confirmed by Mario. If it were not mandatory, we might not be able to differentiate what happened based on the ledger history.

We might even think of mandating that the memo must contain the height of the block that contains the associated transfer that has lead to this revocation.

What do others in the WG think about this?

1 Like

Thinking further about this, I wonder whether we need to include the metadata in full. In ICRC-1 the governing principle is that the ledger should be reconstructible from the blocks. Applying this principle here would mean to have the full metadata in the block log to be able to reconstruct the ledger from the block log. Externally-referenced things we don’t care about as they are not part of the ledger. The draft has been modified to represent this proposal.

In terms of size this should not be an issue as there are multiple archive canisters keeping the block log, while there is (currently) one canister that keeps all token state.

What do people think about this?

Denial of service protection is really hard, thus we have the problem you mention. If you use the Account instead of the principal, you can do unlimited DoS with a single principal and its many subaccounts as you mention above, so we could equally skip the mechanism altogether in that case.

The original idea was to use the principal in order to limit what a single principal can do in terms of DoS. For a single principal one can build more effective DoS protection, e.g., by requiring a principal to own at least one token in order to be able to make collection-level approvals.

Also this DoS protection is optional, so you need not implement it in your ledger.

Seems this is something to come back to in the next meeting. Doing an actual implementation really brings lots of good insights! :slight_smile:

You meant to use (1) icrc7_tokens_of to get the tokens of an account and (2) icrc7_token_metadata as a batch call on the response list of tokens received in 1, correct? tokens only gives you a list of token ids, not the corresponding metadata.

1 Like

I think that we don’t need to log post-transfer revocations if the standard says that you “MUST” do this as it can be reproduced by simply looking for transfers, but I’m open to other interpretations. It could be VERY chatty.

2 Likes

Thinking more about it, I must agree. Initially I thought that making it explicit in the log makes the logic easier to determine active approvals, but the additional wasted space is probably not worth the simplifications of the code.

The next WG meeting is taking place tomorrow, December 5, 2023.

We will go over the issues that have been raised on the forum on the recent version.

Relevant drafts:

Has @bob11 posted his thoughts? Between Bioniq and Toniq, seems like he is leading NFT efforts on the IC…

1 Like

While Toniq hasn’t really been participating much in the working group, you all have created a fantastic standard. So excellent job everyone!

A few personal opinions that don’t actually impact the standard very much:

  1. I don’t love the Approve/transferFrom model (I think there are better ways). There are massive problems with this approach that we’ve seen from ETH NFT land, and I’m sad we’re building it in as a standard, but I accept that people want it for consistency across chains.

  2. I also don’t love the final result of the single textual representation of ICRC-1 accounts. I understand why it was done the way it was done, but the result is kind of ugly, with inconsistent length, and not very user friendly. I’m not saying to change it, but I wish it was closer to Ethereum where all addresses are the same length. Feels nice to have that address consistency.

Otherwise, I think this is a great standard created by some of the brightest developers on ICP. So good work everyone.

3 Likes

Proposal for agenda for the call on Dec 5:

  • Has the WG feedback from the recent call been implemented according to WG’s liking?
    • In particular also splitting off ICRC-30 as separate document
  • Block log schema
    • Draft to be validated by WG
    • Do we want to mandate what a memo should contain in case of a ledger-initiated revocation of an approval? Do we want to mandate to have the memo a level up in this case?
  • Do we need to store the complete NFT metadata in the mint block?
    • If we want to have ledgers reproducible from the log, we need this; Anything else?
      • Having this property makes the ledger much stronger
  • Storing implicit approval revocation on transfer in the block log?
    • Rather no, because it can be inferred from the transfer and existing approvals on the token; Is the logic for this simple enough?

Next steps

  • Do people need more time for the reference implementations?
  • Addressing any remaining items
  • Formatting fixes
  • WG Voting
    • Handle ICRC-7 and ICRC-30 in tandem as bundle or separately in voting?
  • NNS voting

I am still tweaking the block log schema for NFTs and wondered whether in each block we should strive to express the absolute minimum information: E.g., for a transfer_from, we could omit the from from the block log as it is implied by the current owner. Modeling it is redundant, and redundancy is not necessary here and just wastes space. What do you think?

@mariop, @skilesare, @benji, @sea-snake, @levifeldman, @kayicp

I think shortening the names is fine 30xferfr or something like that 30app, 30appc, 30rvk,30rvkc. It is better for ledger size. Text bytes are big.

1 Like

Though a ledger could internally reduce storage size by creating a mapping for these text values so they’re only stored once and each transaction is stored with a reference to a key in the map.

I do agree on the minimum information, the history as a whole should contain all information needed but it’s ok to not have contextual/historical information within the history items itself. For easy accessing information you’d want something like an index canister anyway.

2 Likes

As long as someone reading the log will be able to know what operation it is, then it works.

Update to this: The group agreed in the meeting to have some redundancy in the block log where it helps to simplify the implementation. We changed the block scheme (back) to contain this redundancy.

Do we need an extensions mechanism in ICRC-30 or would the extensions go into the base that ICRC-30 extends, i.e., IRCR-7? Likely, it is the latter, but would like to get some some inputs on this one. Relates to the comment at the very end of ICRC-30.

Another open point in ICRC-30:

“Collection-level approvals can be successfully created independently of currently owning tokens of the collection at approval time.”

If we want to allow DoS mitigations here, this needs to be relaxed so that collection-level approvals should only be possible in case someone holds at least one token. We could also leave the aspect unspecified whether you need to hold tokens and leave it to an implementation. Spec-wise, it would be cleaner to not require a user to hold tokens to make approvals.

Besides this and the above question, I think the standards are finished.

Looking forward to some input:
@skilesare, @benji, @sea-snake, @levifeldman, @kayicp

In my implementation, I’ve made this a configuration flag. To me it makes sense to require someone to have a token in almost all instances, but I’m sure someone could come up with a use case where pre-approving is important.

1 Like

i have been thinking about this. not only for approval but for any update calls that has anything to do with the caller’s token, actually. i mean, since the execution of update calls is ~500k cycles.