onflow / flow-nft

The non-fungible token standard on the Flow blockchain
https://onflow.org
The Unlicense
465 stars 169 forks source link

Add EVMBridgedMetadata & URI to MetadataViews #203

Closed sisyphusSmiling closed 5 months ago

sisyphusSmiling commented 6 months ago

Related: #129

Description

With EVM on Flow coming in the near future, we'll need to reconcile differences between EVM & Cadence NFT metadata handling expectations and standards.

Motivated by ongoing work while iterating on the VM bridge, this PR puts forth two metadata views for consideration.

The first, URI, is a generic struct intended to store a string pointer to some metadata. In many cases, this may just be a JSON blob at a URL or an IPFS pointer. In other cases, this may actually be NFT metadata as a JSON blob itself as in URL-encoded URIs. In the case of ERC721s bridged from EVM, we likely won't have enough context to determine the type of URI this is, thus the generic nature of this view.

Here is the URI interface:

pub struct URI: MetadataViews.File {
    /// The base URI prefix, if any. Not needed for all URIs, but helpful
    /// for some use cases For example, updating a whole NFT collection's
    /// image host easily
    pub let baseURI: String?
    /// The URI string value
    /// NOTE: this is set on init as a concatenation of the baseURI and the
    /// value if baseURI != nil
    access(self) let value: String

    pub view fun uri(): String

}

The second, EVMBridgedMetadata, contains three fields

pub struct EVMBridgedMetadata {
    pub let name: String
    pub let symbol: String

    pub let uri: {MetadataViews.File}
}

The fields name and symbol are included in several ERC standards, notably ERC721 and ERC20. The last field, uri, allows Cadence projects to define how they would like their token or contract's metadata to be transmitted to an EVM counterpart. In the case of Cadence-native projects, this field could be any File implementing struct, while in the case of bridged assets (e.g. ERC721) the uri value would likely just be a URI struct.

Without getting into the specifics of bridging assets which is out of scope for this PR, it would be very helpful to get feedback on the views as their presented, if we feel they're worth adding in the MetadataViews contract, and if the community feels anything is missing from these very barebones implementations.


For contributor use:

sisyphusSmiling commented 6 months ago

Relevant discussion in discord

sisyphusSmiling commented 6 months ago

Updating to note I added a baseURI: String? field to URI based on feedback implementing the view in bridge contracts.

@joshuahannan I think deploying the update before Crescendo makes the most sense to allow projects to adopt the view as they see fit before bridging is enabled. There was constructive discussion in the linked discord thread, but I'd hoped for more community feedback before merging this view. I'll raise this PR again in discord to ask folks to share feedback.

sisyphusSmiling commented 5 months ago

Yeah, we'd want to include this before 1.0. I can follow up with a PR including an implementation.

joshuahannan commented 5 months ago

oh yeah, and make sure you run make test to run the tests and generate the assets