add contract_metadata_update event

[lachlanglen]

Added contract_metadata_update event to Standards -> Tokens -> NonFungibleToken as per the following discussion: https://gov.near.org/t/extending-nep-171-events-standard-to-include-contract-metadata-update-events/30384

Implementation of this new event enables 3rd party applications to listen for updates to NFT contract metadata, e.g. a new dedicated IPFS gateway (base_uri) and provide a consistent and higher quality experience for their users.

[nearbuild]

@frol asked nft builder group to review, not sure if there are any contentions, wanted to move this up the NEP process

[frol]

@lachlanglen Thanks for submitting this NEP extension proposal!

As a moderator, I reviewed this PR and move it to the subject matter experts review stage.

[frol]

I see that there were already several LGTMs from @MinorityProgrammers, @mattlockyer, and @nearbuild on NEAR Gov Forum, so I take them as SME reviews.

As a working group member, I will review this proposal later today.

[frol]

The above spec uses NFTContractMetadata type and this means that all the required fields should be present, yet this example implies that only modified fields become part of the emitted event. What is the actual path you propose to go and why?

[lachlanglen]

I think it probably makes more sense to actually not include the contract_metadata at all in the event, primarily due to the fact that icon would be included which could easily put the log over the 16KB limit. I'll update to reflect this.

[frol]

@lachlanglen This is a very good point that I also was thinking about. I still think it is worth including the list of fields that were updated. What do you think?

[lachlanglen]

@frol I hear your point that the listener can choose to act only if the updated fields are relevant to them, but since they need to request the full metadata object anyway via nft_metadata, I'm not sure how much benefit this provides. Additionally, it introduces a requirement for the contract to keep track of which fields were updated, rather than simply being able to accept an NFTContractMetadata object in an update method and set the whole thing. Wdyt?

[frol]

@lachlanglen Are you familiar with the near-sdk-rs implementation of NFT events? There are two reasons for my question:

1. We will need someone to contribute the implementation to near-sdk-rs (Developer Governance strives for community contributions all across the ecosystem)
2. I wonder how to integrate this event in such a way, that we can ensure that near-sdk-rs users are safeguarded and don't need to keep in mind that they need to emit the event (e.g. mint and burn events are emitted in the internal near-sdk-rs implementation)

[lachlanglen]

Sure, I'm happy to contribute the near-sdk-rs implementation.

I guess we should add the update method then? thoughts on the naming? e.g. contract_metadata_update or set_contract_metadata (possibly more accurate if the whole object is being received and set rather than individual elements)

fn set_contract_metadata(&mut self, metadata: NFTContractMetadata);

[lachlanglen]

Actually I guess something like set_nft_metadata or nft_metadata_set would be more consistent with current methods... though I don't like convention of calling the contract's metadata nft_metadata as I find it misleading and confusing.

[frol]

@lachlanglen Naming is hard, so I would like someone to find the best UX as part of this contribution. Currently, examples do not give any examples of updating the metadata, but implementors can alter the metadata directly, which should be redesigned, I believe.

https://github.com/near/near-sdk-rs/blob/master/examples/non-fungible-token/nft/src/lib.rs#L33

[lachlanglen]

@frol implementors can alter the metadata directly - what do you mean by this?

[frol]

@lachlanglen As NFT contract author I can just implement a new method like:

fn lets_update_metadata_and_do_something_else(&mut self) {
    self.metadata.set(NftMetadata { ... });
}

Given that metadata is defined as:

pub struct Contract {
    tokens: NonFungibleToken,
    metadata: LazyOption<NFTContractMetadata>,
}

The interface won't guard me against doing this, and this way I won't emit the metadata update event unless I know that I should emit it. This makes the interface fragile, and most of the implementations won't emit the metadata update events. Ideally, we should suggest using such an interface that would make it impossible to avoid emitting the event. (please, see this talk)

[lachlanglen]

Thanks for the feedback @frol, made changes as per your notes.

[frol]

As a working group member, I lean towards approving this proposal given there are no breaking changes and this could be quite beneficial for indexers.

@near/wg-contract-standards I invite the rest of the working group members to review this NEP extension.

[nearbuild]

When will this go to vote?

[ori-near]

When will this go to vote?

We are waiting for the working group members to add their voting indication, which should hopefully happen this week. From there, we will schedule the next call.

[mfornet]

I'm leaning toward approving this proposal.

[abacabadabacaba]

I'm leaning towards approving this proposal.

[ori-near]

As the moderator, I would like to thank the author @lachlanglen for submitting this NEP extension, and for the @near/wg-contract-standards working group members for your review. Based on the voting indications above, it seems like this proposal is close to a decision. We are therefore scheduling the third Contract Standards Working group meeting next week, where this NEP can enter the final voting stage. We encourage anyone who is interested to join the meeting.

Meeting Info
📅 Date: Friday, Jan 13, 4pm UTC
✏️ Register

[ori-near]

Thank you to everyone who attended the third Contract Standards Working Group meeting today! The working group members reviewed the NEP and reached the following consensus:

Status: Approved

• @frol add contract_metadata_update event #423 (comment)
• @abacabadabacaba add contract_metadata_update event #423 (comment)
• @mfornet add contract_metadata_update event #423 (comment)

Meeting Recording:
https://youtu.be/pBLN9UyE6AA

@lachlanglen Thank you for authoring this NEP!

Next Steps:

• @lachlanglen To embed the benefits & concerns into the NEP document
• @lachlanglen To add a changelog at the end of the NEP document linking PR/NEP add contract_metadata_update event #423 for future reference
• @near/nep-moderators to approve and merge the NEP
• @near/nep-moderators To post on the NEAR Gigs Board a call for champions who are interested in implementing this NEP in the tooling

Implementation Status:
Needs support in SDK and API packages

Community contributions to the SDKs are welcome!

[frol]

@lachlanglen I went ahead with #351 and added the "Decision Context" section with Benefits and Concerns there. Would you be able to follow the pattern and update this PR accordingly, so we can merge it?