Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A new Metadata view to interop with NFT collections that store NFT in a offchain json file #129

Open
bjartek opened this issue Sep 20, 2022 · 33 comments
Labels
Feature Feedback SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board

Comments

@bjartek
Copy link
Contributor

bjartek commented Sep 20, 2022

DimentionX and Rarible both store nfts in an offchain json file that follows the openSea standard. I think this should be formalized into a Struct.

I vaguely remeber that we have talked about this before @psiemens

@austinkline
Copy link
Contributor

That was part of the the NFTView is for, would the goal be to translate the format from the OS struct to the NFTView?

@spacepluk
Copy link

I'm very happy to see this here :D

@psiemens
Copy link
Contributor

psiemens commented Sep 20, 2022

When we talked about this before, the idea was to have a simple view that returns a URL pointing to a JSON file.

I think it'd basically just be a port of the ERC 721 / 1155 tokenURI function: https://docs.opensea.io/docs/metadata-standards#implementing-token-uri

As @bjartek mentioned, there are some existing projects that store metadata this way. I know that many of us are champions of on-chain data (myself included), but I still think its valid for off-chain metadata like this to be given a home inside a view.

@joshuahannan
Copy link
Member

Anybody have any ideas for the implementation for this?

@bjartek
Copy link
Contributor Author

bjartek commented Oct 13, 2022

A naive implementation would just be to have a struct that has a File, but then you would lose mediaType information, does anybody use anything but json for this?

Another option is to use Medias so that you can set the mediaType to what you want like application/json+opensea?

On ETH/opensea this is called tokenURI but I am not sure if that name is appropriate here. What about OffchainMetadataURI?

so what about

pub stuct OffchainMetadataURI{
   pub let media:Media
   
   init(_ media:Media) {
    self.media=media
  }

  pub fun uri() : String {
    return self.media.uri()
  }
}

@austinkline
Copy link
Contributor

Anybody have any ideas for the implementation for this?

I would strongly suggest that we lay out an actual data format that is expected. This problem to me is framed with the following question:

"What is the purpose of exposing off-chain data, and who is expected to use it?"

My answer to that question is that exposing data this way is almost exclusively for third-party platforms. Products which store data off-chain such as Solarpups already know how their data is formatted for their own system to work. Guiding products to a specific unified (but flexible) format will be extremely important.

I think we have two options.

  1. As @spacepluk has asked for in the past, we endorse vending all contents of the NFTView struct from this external URL
  2. We encourage only a subset such as Traits and Display.

My preference here is that if we are going to start encouraging off-chain paths for data like this, we should just let all data be specified in this way. I definitely prefer data be on-chain as much as it can be, but I don't see a compelling reason to be restrictive in what is actually "allowed".

I think the struct that @bjartek has laid out above is fine for the view, but I would stress that the resulting format should be consistent, and we should definitely discuss what that should be at length as well

@joshuahannan joshuahannan added the SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board label Oct 18, 2022
@psiemens
Copy link
Contributor

psiemens commented Oct 18, 2022

While I think the best solution for Flow NFTs is to put the metadata on chain, I still think there should be way for projects using the OpenSea JSON format to make use of metadata views.

My preference here is that if we are going to start encouraging off-chain paths for data like this, we should just let all data be specified in this way.

@austinkline When you say all data, do you just mean that projects shouldn't use a mixture of off-chain JSON files and on-chain metadata?

I think the struct that @bjartek has laid out above is fine for the view, but I would stress that the resulting format should be consistent, and we should definitely discuss what that should be at length as well

Unless I'm mistaken, I think all projects that use JSON files follow the format in the OpenSea docs, which is an extension of the ERC721 Metadata JSON Schema.

I was thinking the view should just very clearly indicate that it expects the data to be in that format:

/// ERC721TokenURIView returns a JSON file that conforms to the ERC721 Metadata JSON Schema.
///
/// Note: the majority of Flow NFTs store metadata directly on chain rather than in an external file.
/// Please consider implementing the NFTView, which is designed to have feature parity 
/// with the ERC721 metadata format. 
///
pub struct ERC721TokenURIView {
  
  /// A JSON file that conforms to the ERC721 Metadata JSON Schema.
  ///
  /// Ref: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
  ///
  pub let file: File
   
  init(_ file: File) {
    self.file = file
  }
}

@bjartek
Copy link
Contributor Author

bjartek commented Nov 2, 2022

This suggestion works for me @psiemens it is very specific and easy to document!

@austinkline
Copy link
Contributor

While I think the best solution for Flow NFTs is to put the metadata on chain, I still think there should be way for projects using the OpenSea JSON format to make use of metadata views.

My preference here is that if we are going to start encouraging off-chain paths for data like this, we should just let all data be specified in this way.

@austinkline When you say all data, do you just mean that projects shouldn't use a mixture of off-chain JSON files and on-chain metadata?

I think the struct that @bjartek has laid out above is fine for the view, but I would stress that the resulting format should be consistent, and we should definitely discuss what that should be at length as well

Unless I'm mistaken, I think all projects that use JSON files follow the format in the OpenSea docs, which is an extension of the ERC721 Metadata JSON Schema.

I was thinking the view should just very clearly indicate that it expects the data to be in that format:

/// ERC721TokenURIView returns a JSON file that conforms to the ERC721 Metadata JSON Schema.
///
/// Note: the majority of Flow NFTs store metadata directly on chain rather than in an external file.
/// Please consider implementing the NFTView, which is designed to have feature parity 
/// with the ERC721 metadata format. 
///
pub struct ERC721TokenURIView {
  
  /// A JSON file that conforms to the ERC721 Metadata JSON Schema.
  ///
  /// Ref: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
  ///
  pub let file: File
   
  init(_ file: File) {
    self.file = file
  }
}

Two things:

  1. My only hesitation is we would have two formats basically going on now since our onchain metadata differs slightly from the opensea doc. If we're okay as a community living with that just given the ubiquitousness of the Opensea standard, I can live with that. I did want to make sure we call that out and willing make that trade-off though. As an example, we have a rarity field built into our traits, the OS doc does not.

Is it worth considering where those two things deviate or is the consensus that this isn't a big deal and we can live with it?

  1. Should this offchain data also specify things like NFTCollectionData and NFTCollectionDisplay views? Or are they expected to vend some things on-chain and some things off-chain?

@bluesign
Copy link

bluesign commented Nov 25, 2022

I still think there should be way for projects using the OpenSea JSON format to make use of metadata views.

Why should we endorse someone's mistake in the standard? They put metadata to an url with json, because it was not feasible to otherwise. But on Flow it is feasible.

I don't see any flow activity on OS ( despite announcement ages ago ) , is this a requirement from them ?

I believe this will kill metadata standard, if OS forces people to add this, and some projects started to support only this format. Then wallets will begin to support this json stuff and there would be no motivation left to keep metadata on chain. ( which is a big hassle compared to json file )

I am all in support for making this a struct covering JSON data , but external JSON is really a bad idea.

@franklywatson
Copy link
Collaborator

The requirement isn't coming from OpenSea no. I also agree with @bluesign, this will undermine our efforts to encourage the right on-chain practices.

@austinkline
Copy link
Contributor

@bluesign @franklywatson the unfortunate thing is that whether we like it or not, applications will define their own off-chain formats when it suits their needs (Solarpups and DimensionX are both organized this way?). So if we want platforms to be able to show data coming from all sources (on or off-chain) either it gets defined here, or platforms like Flowty will just do it themselves.

I agree that it puts metadata standards at risk from being on-chain, that's why I would advise against the opensea format and instead just make it a json representation of the NftView. OR we could also state that only a subset of views can be off-chain such as traits, the rest would need to be on-chain

@bluesign
Copy link

bluesign commented Dec 5, 2022

I think this is very tricky situation from business sense.

But I see ( I think everyone agrees ) off-chain metadata is direct competitor to on-chain data. As it is easier to keep and maintain off-chain to the dapp. But for other participants, user and other dapps and wallets it is for sure worse for interoperability.

Ecosystem should push new participants to the standard, but here we have new participants pushing ecosystem to their standard. ( which maybe good in the short run, but in the long run dangerous )

Also I don't believe platforms should support those dapps till they have onchain metadata, but I understand that platforms may need them more than they need the platforms.

@austinkline
Copy link
Contributor

I think this is very tricky situation from business sense.

But I see ( I think everyone agrees ) off-chain metadata is direct competitor to on-chain data. As it is easier to keep and maintain off-chain to the dapp. But for other participants, user and other dapps and wallets it is for sure worse for interoperability.

I honestly just read this as a lack of incentive for creators to do the "right" thing. If there were better tools that onchain were as easy as off, I don't think we'd have a hard time selling it.

Ecosystem should push new participants to the standard, but here we have new participants pushing ecosystem to their standard. ( which maybe good in the short run, but in the long run dangerous )

I guess the question here is about whether it's realistic for all use cases to be put into this bucket. @spacepluk had brought this up a while ago that on-chain traits and updates and things are just too complex for them to want to do, offchain made more sense to them. As much as we want to keep things in the "right" place, we run the risk of scaring off builders if we don't provide guidance and ways to make this all happen.

I don't think this is them pushing the flow standard at all, quite the opposite. Folks are asking for a standard for the way they need to vend data for their use case, isn't that a little different?

Also I don't believe platforms should support those dapps till they have onchain metadata, but I understand that platforms may need them more than they need the platforms.

Yeah, generally speaking I don't think this is a thing that will happen long-term. Either a standard format gets defined in some way here, or platforms will just have to sort it out themselves where it will eventually converge.

Curious what others think though, it just seems unlikely that a platform which wants to support everyone out there would put a line in the sand and alienate some kinds of creators.

@joshuahannan
Copy link
Member

Anyone have any thoughts for how we could move forward here? I'm not an app developer, so I don't have a good idea for what would be useful for this

@austinkline
Copy link
Contributor

austinkline commented Jan 18, 2023

My view-point remains the same as it was before. App developers will need this, so whether we think it's "right" or not is inconsequential. We should define the standard for a view that points to an api and just returns the NFTView imo

The alternative is that we do nothing, individual platforms define their own, and they could be in conflict

@bjartek
Copy link
Contributor Author

bjartek commented Jan 19, 2023

The cases i have seen here is that people need a view to expose Traits from an external url.

I belive this is the case for both dimmention x and solarpups. Are there sny others that need this?

The metadata standard requires Display so that should be resolved normally imho.

@spacepluk
Copy link

Thanks for surfacing our use-case @austinkline 🙏

FWIW in our case (DimensionX) putting the metadata on chain doesn't add any value to the game and it makes things a lot more complicated for us. The source of truth will always be the game's database and it's a lot easier to expose a view of that data (using whatever format) in an external url that can query the database and be up-to-date at all times.

Of course we could keep a copy of all that data on chain but it will always be lagging the actual in-game data, and we don't have any practical reason to do it now. Also, for us to be able to update said metadata on chain I think it would have to be stored in the contract's account which kind of defeats one of the perceived advantages of on-chain data (data ownership).

Until now nobody has complained about our metadata not being on-chain, so we gather that most people just don't care about it.

That's my 2cents. I'm sure there are other use-cases where on-chain metadata isn't practical or desirable so it would be nice to have official support for it.

@austinkline
Copy link
Contributor

austinkline commented Jan 19, 2023

The cases i have seen here is that people need a view to expose Traits from an external url.

I belive this is the case for both dimmention x and solarpups. Are there sny others that need this?

The metadata standard requires Display so that should be resolved normally imho.

Why limit things, though? If we're already pulling data off-chain, isn't it likely that we will find ourselves being asked to support more than just traits at some point in the future anyway? We'd just say that on-chain takes precedent. I don't really see the value in going half-way here and requiring management of things in two places. It's almost guaranteed that collections will just start pushing the envelope anyway.

To re-iterate, this kind of thing is going to happen regardless of our thoughts on whether it is the "right" thing to do, my opinion is that it is not practical to try and insist that people adhere to something when we know it is unlikely to happen forever. What we instead should be doing is pushing platforms to make features driven by on-chain data, thereby incentivizing products like DimensionX to bring their data on-chain to justify the added complexity.

@bjartek
Copy link
Contributor Author

bjartek commented Jan 19, 2023

@spacepluk can i complain now :p I really want it on chain but using this solution would work aswell.

The use cases I have seen for this is solarpups/rarible and @spacepluk/dimensionX. I wonder what format @spacepluk has in his api and what he is willing to change?

I know that rarible is hosting it as the opensea/erc-721 format.

I do not know about solarpups, but I can ask them.

The way I see it is that the immediate need here is to host something that is in a known format. So the erc721/opensea format makes sense for me. I am not even sure how easy it to return something that serialize into the NFTView since it contains views that are not trivial. NFTCollectionData and Royalty.

@austinkline
Copy link
Contributor

I think we can toss out CollectionData and Royalties because they are meant to be on-chain, are there others that fit the same kind of idea?

My issue with using the OS format is that it is way different than the NFTView we have (editions, traits with rarity on them, how external urls work) that they really aren't that similar. I'd rather we optimize for platforms native to flow, not for EVM products which aren't here but I understand where that might not be the preference for folks

@bjartek
Copy link
Contributor Author

bjartek commented Jan 19, 2023

@austinkline then we are on the same page.

@spacepluk
Copy link

The use cases I have seen for this is solarpups/rarible and @spacepluk/dimensionX. I wonder what format @spacepluk has in his api and what he is willing to change?

We just use a super simple adhoc format that we agreed with Gaia. This is working fine right now and changing it would require work on both sides... so I think it's unlikely to happen unless there's a good reason for it...

That said, we're happy to adhere to any standard you come up with when we have the opportunity. A json representation of the NFTView would work just fine for us.

And I'm just speculating here but maybe using opensea/erc-721 will increase the chances of getting flow collections into opensea?

@bjartek
Copy link
Contributor Author

bjartek commented Jan 20, 2023

And I'm just speculating here but maybe using opensea/erc-721 will increase the chances of getting flow collections into opensea?

This is a good point

@austinkline
Copy link
Contributor

austinkline commented Jan 21, 2023

I would urge everyone here to define a standard that is best for flow and those who build on it. OpenSea is originally an EVM-based product and as such, metadata standards will be the least of their concerns should they decide to come to Flow (trust me, I used to work there.) When OpenSea went to Solana, they worked with the community to define a standard because there wasn't one at the time, that is not the case here

OpenSea's standard and the FlowNFT are similar but there is a reason they aren't the same, we even discussed that way back when the FlowNFT view was being made. That is where the displayType field on traits comes from, in-fact!

Key differences:

  • How external url works (it is its own view here)
  • id and uuid (they are different)
  • Traits have a rarity nested inside of them
  • Folks have asked about adding editions to the NFTView, where would that even fit in OpenSea's standard?

Consider a future where we use oracles to bring in data that exists off-chain for certain interactions. It is much better that we keep our on and off-chain standards aligned for collections who have them both, and for consumers to not have two data types. It my (strong) opinion that we should not use OpenSea's standards and stick to our own.

Example off-chain FlowNFT:

{
 "id": 1234,
  "uuid": 7776123213,
  "display": {
    "name": "FooNFT #123",
    "description": "lorem ipsum",
    "thumbnail": "https://example.com/images/123.png"
  },
  "externalURL": {
    "url": "https://example.com/nft/123"
  },
  "traits": {
    "traits": [...]
  },
  "editions": {
    "infoList": [...]
  },
  "collectionDisplay": {
    "name": "Foo NFT Collection",
    "description": "whatever you want here",
    "externalURL": "https://exmaple.com",
    "squareImage": "https://example.com/square.png",
    "bannerImage": "https://example.com/banner.png",
    "socials": {
      "twitter": "...",
      "discord": "..."
    }
  }
}

Opensea reference:

https://docs.opensea.io/docs/metadata-standards

{
  "description": "Friendly OpenSea Creature that enjoys long swims in the ocean.", // display.description
  "external_url": "https://openseacreatures.io/3",
  "image": "https://storage.googleapis.com/opensea-prod.appspot.com/puffs/3.png",  // display.thumbnail
  "name": "Dave Starbelly", // display.name
  "attributes": [ ... ] // similar to traits
}

@bluesign
Copy link

bluesign commented Jan 21, 2023

I think I agree with @bjartek's comment about traits, @austinkline's comment about json standard.

If this is unavoidable ( I agree in some cases it can be, gaming is one of them ) Limiting this to traits ( which are the dynamic part of the metadata ) can benefit a lot:

  • First of all, NFT will have some on-chain metadata presence
  • Many of utility dapps ( even market places ) can work by missing some traits / all traits
  • This will allow having some minimal basic data, and updating rest from off chain. So even wallet doesn't support off chain metadata, you can still see your NFTs nicely
  • Btw we don't need to limit to traits ( agree on @austinkline there )

I suggest;

  • adding a field to NFTView: externalMetadata which will be a .json file.
  • letting it to define any metadata ( and override ) except display.
  • json format can be like @austinkline suggested in previous message

This will allow at least having basic (display) metadata on-chain. Rest can override from external off-chain sources. I think keeping display on chain is important in general.

@joshuahannan
Copy link
Member

These sound like good suggestions. How would be the best way to represent json on chain? Would we just encode it in a string or dictionary or something?

@spacepluk
Copy link

If I understand correctly we don't need to have the json on chain, just a url to the json that mimics NFTView.

I'm not sure if it's already the case in @austinkline's example but I think it would be good if the json format matches the output of fcl.decode (from fcl-js). Then you don't need to write special code to handle this data depending on where it came from.

@alilloig
Copy link
Member

Anybody could help me understand what will be the advantages of encoding a json file on-chain rather than just keeping a link to the off-chain file?

@austinkline
Copy link
Contributor

@spacepluk is right, we wouldn't be bringing it on-chain. The metadata view should be a url to json data that we can pull, and its format should match whatever we settle on here, basically the equivalent of the tokenURI method used a lot in the EVM world.

It sounds like we have consensus to use the format the NFTView would return if you were to return it from flow itself in a script, with some stipulations that they don't all make sense to return. With that in mind, the views that should not be stored off-chain are:

  • display
  • collectionData
  • royalties

@bjartek in case I've missed something or you have other thoughts

@alilloig
Copy link
Member

So, if I'm understanding right, resources implementing this new view would use it to provide anyone querying them the same info as if they were queried for the NFTView? So this will also need to define a json object format that matches NFTView - display - collectionData - royalties?

Do you have an idea of how we could keep encouraging people to keep metadata on chain (data that smart contracts could ever care of) instead of just using the json format?

@bluesign
Copy link

bluesign commented Jan 23, 2023

I think at least keeping display on chain is required. Otherwise showing nfts will be like 10s of different json calls from everywhere. But I still think this will kill on chain metadata a bit. Actually limiting to traits like @bjartek said is the best middle ground imo.

@bjartek
Copy link
Contributor Author

bjartek commented Feb 8, 2023

Can we add a contentType to the view and just leave it up to the implementer on what kind of data they want to return? Some people want ERC721 OpenSea things, others want a more complete view. If we give them the option to say what they want then it is easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Feedback SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board
Projects
None yet
Development

No branches or pull requests

8 participants