kodadot / nft-gallery

Generative Art Marketplace
https://kodadot.xyz
MIT License
611 stars 351 forks source link

Overwrite metadata attributes with on-chain attributes #6785

Open vikiival opened 10 months ago

vikiival commented 10 months ago

Context

With stickv4 we are able to show on-chain attributes. However current solutuion is based on the concat strategy. ([...onChainAttr, ...metadataAttr])

Solution

The proposed solution is to do a merge (concat + and overwrite those which are in metadata attrs)

Canary With on-chain
image image

I don't know what you think but imo the attributes from set_attribute extrinsics should overwrite those from the metadata in case of duplicates. aka

//pseudocode
const nftProperties = {
  ...attributesFromMetadata,
  ...attributesFromExtrinsics,
}

Originally posted by @niklasp in https://github.com/kodadot/nft-gallery/issues/6774#issuecomment-1683860996

niklasp commented 10 months ago

That means in practice its currently the other way around? E.g. when a NFT has two images defined, one in metadata and one in on chain attributes, the "first" one is picked from the metadata?

Also for other random attributes with the same name, both will appear in the Properties tab of the UI?

vikiival commented 10 months ago

That means in practice its currently the other way around? E.g. when a NFT has two images defined, one in metadata and one in on chain attributes, the "first" one is picked from the metadata?

current solutuion is based on the concat strategy

[🩵, 💚] + [🩵, 💙] = [🩵, 💚, 🩵, 💙]

niklasp commented 10 months ago

yeah i know concat but what does the ui display in above case when e.g. 🩵 is the NFT description

vikiival commented 10 months ago

display in above case when e.g. 🩵 is the NFT description

Currently it is stored under attributes.

I see your point now

If you want to override nft metadata we can either proceed with KodaDot conditional package (if-that). Or please open an issue in stick repo how it should work.

How we handle metadata

Currently metadata ipfs string is a foreign key to the MetadataEntity which is loaded from IPFS. Mutating the ME directly would result into undefined behaviour as ME could be shared with many other NFTs. What my brain says rn:

Is to store overrides on the NFT level as an additional JSON entity.

niklasp commented 10 months ago

@vikiival i tested the following setup

  1. created a NFT on asset hub with nft.mint
  2. then called nft.setMetadata on that NFT (https://kodadot.xyz/ahk/gallery/101-88888888)
  3. the metadata attributes all showed in the "Properties Tab"
  4. Then i did a nft.setAttribute on that nft with key: something value:223
  5. now only that single attribute shows in the "Properties" Tab

=> metadata attributes are not merged but overriden completely by the attributes from setAttribute extrinsics, as far as i can tell (also true for canary. and beta.)

So in heart notation, where [💙] is onchain attr

[🩵, 💚,] + [💙] = [💙]

roiLeo commented 6 months ago