NFT original metadata #552
Conversation
pro-wh
force-pushed
the
pro-wh/feature/holders18
branch
from
b08a215 to
2b5716d
Compare
| @@ -118,6 +118,7 @@ func (p *processor) ProcessItem(ctx context.Context, batch *storage.QueryBatch, | |||
| staleNFT.DownloadRound, | |||
| nftData.MetadataURI, | |||
| nftData.MetadataAccessed, | |||
| nftData.Metadata, | |||
There was a problem hiding this comment.
apparently this makes it so that the string is the JSON serialized value. I have no idea how, hypothetically, to tell the db that "the JSON value was a string, here's that string."
| @@ -1603,6 +1603,7 @@ func (c *StorageClient) RuntimeEVMNFTs(ctx context.Context, limit *uint64, offse | |||
| &ownerAddrData, | |||
| &nft.MetadataUri, | |||
| &metadataAccessedN, | |||
| &nft.Metadata, | |||
There was a problem hiding this comment.
and then this does unmarshal the JSON from the db into Metadata interface{} as like maps and stuff 🤷 and our API layer later re-marshals it 🔁
| metadata: | ||
| description: | | ||
| A metadata document for this NFT instance. | ||
| Currently only ERC-721 is supported, where the document is an Asset Metadata from the ERC721 Metadata JSON Schema. |
There was a problem hiding this comment.
https://eips.ethereum.org/EIPS/eip-721 their schema is here. if higher effort, we could import that and say it could be that schema
| BEGIN; | ||
|
|
||
| ALTER TABLE chain.evm_nfts | ||
| ADD COLUMN metadata JSONB; |
There was a problem hiding this comment.
using a json type to be nice to people who want to do things in the db. but beware there are no indices. downside is that both go and postgresql will parse the JSON code. if they ever have any difference in opinion on what's valid JSON, then someone could break our indexer.
|
omg regression tests are gonna need to be updated huh |
oh shoot what |
|
oh it's trying to give uninitialized empty string when there's no metadata available, which isn't a valid json value |
|
ok I'm gonna make the EVMNFTData struct fields nil-able. sql NULL in the metadata column will mean we didn't get the document for whatever reason. json null will theoretically mean the metadata document was null, although that doesn't fit the schema and would probably fail to demarshal first. empty strings in the NFT table (in name etc fields) will be from legacy code, although we haven't enabled the analyzer in our deployments yet, so we shouldn't see any (other than from contracts that actually give empty string) |
| @@ -4,6 +4,21 @@ | |||
| "description": "Swarm Teddies are the first Teddy Bear-like collection in Oasis Network, coming from space, these awesome creatures are unique, exclusive and hand designed with dedication and love.Born from an original concept, Swarm Teddies consists of 1000 cute bear NFTs that quickly adapted to their favorite themes, these creatures are not here to take part, they are here to take over.", | |||
| "id": "177", | |||
| "image": "ipfs://QmctsL3PKCbu3QGNmVfu9opWC3ujFecKR9u2DjuAhWSqkD/Casual-102.png", | |||
| "metadata": { | |||
| @@ -221,12 +755,9 @@ | |||
| } | |||
| }, | |||
| { | |||
| "description": "", | |||
There was a problem hiding this comment.
these fields are removed by following the logic posted in the PR:
- we can't get metadata from this URL (unresolvable hostname)
- so the name+description+image are left as nil in the EVMNFTData struct during analysis
- which go in as NULL in the database
- which I suppose we are leaving them as field-absent in our API output 🤷
pro-wh
force-pushed
the
pro-wh/feature/holders18
branch
from
4df8c1f to
d5069d4
Compare
this leads to some data duplication. but eh, in the hypothetical situation where there's more than one token standard,
name,description, andimagewill have different routines for how to populate them, andmetadatawill be whatever appropriate metadata that standard provides, e.g. it might be arranged differently, or name etc. will be from an on-chain call instead, or whatever.