feat: add NFT mint/transfer/burn event standard

[ghost]

No description provided.

[frol]

Hey, I feel that we should define our standards to be as slim as possible. This standard extension still makes sense, but from what I have heard, there are too many ways of how tokens can be minted/burned, so it feels that those cannot be limited to just certain method names (it can be implemented that way, but it might just feel unnatural.

As per my #254 (comment), I feel there are just 3 high-level scenarios that need to be standardized:

1. Observability of minting, transfer, burning events
2. Interface to transfer assets
3. Interface to get status, i.e. balance (for FT) and token-by-owner and owner-by-token (for NFT)

Batch operations are great, but I would keep them an extension. I find this current proposal to be quite complicated in terms of the interface, and at the same time, it might limit some other use-cases. Batch interfaces are not required by any of the listed scenarios, and while it might address my concerns about observability, I have heard that it is not realistic to expect people to create workarounds with cross-contract calls just to make sure to have a call with nft_batch_* as a method name. I feel that log-events is the way to go, and we can easily document that there can be more than a single mint/transfer/burn events in a single execution and that will completely solve (1) with just a minor tweak to the existing implementations and it does not interfere with this batch extension.

Thus, I suggest we discuss the observability in #254 and consider if these interfaces still need to be defined.

[zcstarr]

just commenting here to ask if approval makes sense as an event @frol @evergreen-trading-systems, then downstream subscribers like app market places know when approval changes. They could just subscribe to the events when the approval happens.

Otherwise they'd need to poll the chain. I think revocation isn't as important because if you're going to make tx assuming revocation, can just do an is_approved check before hand, no polling required.

The erc-721 event is :

    /// @dev This emits when the approved address for an NFT is changed or
    ///  reaffirmed. The zero address indicates there is no approved address.
    ///  When a Transfer event emits, this also indicates that the approved
    ///  address for that NFT (if any) is reset to none.
event Approval(address indexed _owner, address indexed _approved, uint256 indexed _tokenId);

I think we'd need approval_id, approved_account , owner, token_id . Although approval_id is not strictly necessary. I see this more as an ability for listening systems to be able to respond to approvals. What do you all think? I think for #245 we're looking at the same thing.

[ghost]

@zcstarr Maybe no This whole need for event logs started because different people had different ways of doing the same thing and the wallet only displayed certain formats. The 3 main things every NFT marketplace is doing and already being logged are

• minting
• burning
• transfer

but only in a specific format so not every marketplace are seeing their transactions on the wallet.

That's why approval doesn't make sense first but after. I wish there was more than 1 wallet app today. When integration with metamask happens, issue like this won't move slow.

[zcstarr]

@zcstarr Maybe no This whole need for event logs started because different people had different ways of doing the same thing and the wallet only displayed certain formats. The 3 main things every NFT marketplace is doing and already being logged are

My thinking on approval is that with the approval, people can write dapps and server side dapps that can respond to to approval changes. I guess my fear is, if we don't cover the basics of events, that major implementers won't implement them and the ecosystem will have some people that emit the event and some that don't so it becomes and unreliable event.

With regards to events in general, Implementers are always going to do things differently and in unexpected places. Events are the proper tool to share knowledge about contract state changes in a uniform way, while allowing developers the freedom to write their contracts in the ways that make sense for their use case.

[ghost]

I agree with you @zcstarr . Anything that is useful to index should have an event. Currently, the only implementors active in this conversation are the wallet developers. They are concerned with the 3 events like I said. Others may concerned with others.

I see the need for 2 prs

1. Standard structure that every event must adhere to e.g. <event-type> <event-log-data>
   a. EVENT_JSON {...}
   b. EVENT_BINARY ...
2. Types of events - nft_mint, nft_burn, nft_transfer, nft_approval etc

This separates the concern of events only being useful to wallet indexers and starts to be useful for all kinds of indexers. I will make the pr.

[mikedotexe]

Thanks for the patience, will rally other folks now.

[telezhnaya]

It's better to write one sentence per line
https://asciidoctor.org/docs/asciidoc-recommended-practices/#one-sentence-per-line

near/core-contracts#144 (comment)

[ghost]

Even though I like the ascii docs I don't agree here because in markdown a line break continues a paragraph which helps word wrapping. Asciidocs are not a standard that governs markdown files. To comply I will update.

If this is the standard of NEPs can we make it official.

[ghost]

Added authorised_id

@mikedotexe I must have forgotten.

Approval events and others will be separate prs.

This is ready to merge.

[frol]

I believe this document is good enough to be merged. It addresses the observability problem for Indexers to pick up the information about common NFT operations.

[telezhnaya]

It's important to mention here that string representation of json should not contain line breaks (Ideally: get rid of all space symbols unless they are in string fields).

When Indexer parses these lines, for each line, it just cuts off "EVENT_JSON:", trims everything else, and passes the result into json parsing library.

If you pass

EVENT_JSON: {
"my_valid_json": true
}

It will give an error because there are 3 lines instead of 1, where the first line contains invalid json, second and third lines do not start from "EVENT_JSON:".

We also need to decide, are we OK with other messages here (without "EVENT_JSON:" at the beginning). We need to say explicitly in the NEP, whether we ignore or restrict them

@mikedotexe @frol

[telezhnaya]

Are we sure that we don't want to put everything into json with the structure like

{
"type": "event_json",
"data": [...] // here could be all events
}

?

It simplifies everything, we don't need to overcomplicate this doc, Indexer does not need the logic with cutting/trimming.

[ghost]

@mikedotexe

[frol]

@telezhnaya Individual calls to near_sdk::env::log are recorded as individual strings in the logs array of the execution outcome, so we don't care about new lines inside. We only want to specify that each event should be a separate call to the log host function.

Proof: https://explorer.testnet.near.org/transactions/326HmzvgRYND7skVCdTpCnHMji6qiU8JFhQMNuvS2b74

Contract:

use near_sdk::borsh::{self, BorshDeserialize, BorshSerialize};
use near_sdk::{near_bindgen, setup_alloc};

setup_alloc!();

#[near_bindgen]
#[derive(BorshDeserialize, BorshSerialize, Default)]
pub struct Test {}

#[near_bindgen]
impl Test {
    pub fn test_log(&mut self) {
        near_sdk::env::log(r#"EVENT_JSON:{
            "standard": "nep",
            "version": "1.0.0",
            "event": "mint",
            "data": [
                {"owner_id": "frol.near", "token_ids": ["house"]}
            ]
        }"#.as_bytes());
        near_sdk::env::log("EVENT_JSON:{\"standard\": \"nep\", \n \"version\": \"1.0.0\",    \n   \"event\": \"burn\", \"data\": [{\"owner_id\": \"frol.near\", \"token_ids\": [\"house\"]}]}".as_bytes());
    }
}

[frol]

@evergreen-trading-systems Please, review my latest commit (I wanted to make a proposed change, but it was really hard to read, so I decided to commit it (if that is too much, we can always revert the commit)

[ghost]

Thanks @frol your changes made this document much more concise.

1 question. Is it true you can have a valid event log without data field i.e. EVENT_JSON:{"standard": "nepXXX", "version": "1.0.0", "event": "xyz_is_triggered"}

[frol]

Is it true you can have a valid event log without data field

I don't have any strong opinion about it, but I feel that it is fine to allow it being optional (hence data?: unknown) on the events standard and then force it to be required case-by-case. What do you think?

[ghost]

Cool makes sense. There is no strong need for it to be now so agree with case-by-case.

[telezhnaya]

Thank you @evergreen-trading-systems and @frol, great job!

[telezhnaya]

I like this, sounds much clearer!

[mikedotexe]

Great work everyone