search

manifoldxyz / creator-core-solidity

136 stars 58 forks source link

Support EIP-4906 Metadata Update event for ERC721 #19

Open ssa3512 opened 1 year ago

ssa3512 commented 1 year ago

EIP-4906 allows for the following events to be fired to signal to consumers that the JSON metadata for a token or set of tokens has changed. 

event MetadataUpdate(uint256 _tokenId);
event BatchMetadataUpdate(uint256 _fromTokenId, uint256 _toTokenId);

It would be great if the creator core interfaces could support this to be triggered either by an extension or an admin. For instance, when the claim is updated for LazyMintable, the extension could call back into the creator contract to trigger a refresh of all tokens in the contract. Additionally, custom extensions or admins who are controlling metadata with other off-chain methods could manually trigger this when metadata is changed.

This functionality is natively supported by OpenSea to trigger a refresh of the metadata for the specified token ids.

wwhchung commented 1 year ago

It’s not as straightforward as emitting the event, due to the extensions framework (as you pointed out).

  1. The extension itself can’t emit the event (the spec assumes the root contract emits the event)
  2. Getting the full set of impacted tokens may be gas inefficient, and the tokens may not be sequential.

Looking into how the interface could be changed to support this (Eg providing a method on the creator contract which is callable by any extension which causes the creator contract to emit an event). This is the suggestion you’ve mentioned.

If we did that, we would want to consider the implications of what is/isn’t allowed (Eg if we only allow events to be emitted for tokens created by that extension, it is extremely gas inefficient).

So it seems like the only viable way is to allow any registered extension to call the method with any range of tokens. Thinking through if this would cause any downstream issues. At the moment, it would seem that a malicious actor could trigger unnecessary load on the indexers.

0xGh commented 1 month ago

So it seems like the only viable way is to allow any registered extension to call the method with any range of tokens.

This is totally fine and it is then on the dev to decide when / how in the extension.