onflow / flow-nft

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

Commit reveal Standard to FLOW #89

Open Peixer opened 2 years ago

Peixer commented 2 years ago

Issue To Be Solved

A generic proposal to apply commit reveal to NFT metadata. This approach is used for blind sale where we want to hide the metadata information and only then reveal it, using provenance hash to ensure that there was no manipulation in the metadata.

(Optional): Suggest A Solution

My suggestion is to create an interface with public access and apply it to the Collection, that way it would be possible for the administrator to access the NFT within the collection of a third party. Only the admin can use the interface because it requires Admin resource reference.

The interface receiving NFT Id, admin resource reference and the original metadata with all information

  pub resource interface IRevealNFT {
    pub fun revealNFT(id: UInt64, admin: &Admin, metadata: {String: String})
  }

The standard collection applying the IRevealNFT to call reveal function to a specific NFT

  pub resource Collection: NonFungibleToken.Provider, NonFungibleToken.Receiver, NonFungibleToken.CollectionPublic, MetadataViews.ResolverCollection, IRevealNFT { 

    ....

    pub fun revealNFT(id: UInt64, admin: &Admin, metadata: {String: String}) {
      if self.ownedNFTs[id] != nil {
        let nft = &self.ownedNFTs[id] as auth &NonFungibleToken.NFT
        let storeFrontNFT = nft as! &NFT
        storeFrontNFT.reveal(metadata: metadata)
      } else {
        panic("can't find nft id")
      }
    }

   ....

  }

The HashMetadata struct to save the hash from a specific range of NFTs, considering that we can have multiples hash from one collection/drop

  pub struct HashMetadata {
    pub let hash: String
    pub let start: UInt64
    pub let end: UInt64

    init(hash: String, start: UInt64, end: UInt64) {
      self.hash = hash
      self.start = start
      self.end = end
    }
  }

The NFT resource with reveal function where it update the metadata field - just add here to exemplify, it is not part of suggestion

  pub resource NFT: NonFungibleToken.INFT, MetadataViews.Resolver  {
    pub let id: UInt64
    access(contract) var metadata: {String: String}
    pub let hashMetadata: HashMetadata

    ....

    access(account) fun reveal(metadata: {String: String}) {
      self.metadata = metadata
    }

    ....
  }
psiemens commented 2 years ago

I've also been thinking about how to reveal on-chain metadata. I really like your proposal. I think the provenance hash is even an improvement upon existing reveal mechanisms, some of which only rely on the NFT ID to map to an off-chain file that may or may not be content-addressable.

Is the IRevealNFT interface necessary? I see it as more of a nice-to-have. I think an admin can borrow an NFT through the normal CollectionPublic capability and then call the reveal function, which could be contract-restricted. I'm just not sure it makes sense to require the user to link an additional capability that should only ever be used once.

Peixer commented 2 years ago

@psiemens Agreed, I don't think we must have the IRevealNFT interface, it's just a fancy way to be more explicit. Talking about to add on CollectionPublic, since this function will be only available to admin I don't like the idea to add on public access.

bjartek commented 2 years ago

I think it is a nice concept however would it be possible to have the metadata be Traits? Then you can send in other values then strings.

So lets say I have an nft where i want to reveal the play_id and the serial of that play. They are both uint64.

bluesign commented 2 years ago

I agree with @psiemens, it would be nice to have this on CollectionPublic.

Roughly I think below can work.

pub resource interface CollectionPublic{
    access(contract) fun reveal( id: UInt64, metadata: {String: String})
}

pub resource Admin {
    pub fun reveal(user: Address, id: UInt64, metadata: {String: String}){
        getAccount(user).getCapability(/public/PublicMyCollectionPath)
        .borrow<&MyCollection{CollectionPublic}>()!
        .reveal(id: id, metadata:metadata)
    }
}

I don't think we can make a standard here to be honest, but this can be a best practice. Depending on the metadata, people can change the signature of reveal function.

satyamakgec commented 2 years ago

I don't think we should only restrict the revealing permission to ADMIN itself, It can be done by anyone given that provided metadata hash is same as the hash provided during the mint. The reason of doing so is that there are some cases where NFTs are minted from a contract and NFT owner would not want to give the Admin resource to the minter contract or whatnot so minter contract can easily facilitate the reveal process if reveal is public to use.

joshuahannan commented 1 year ago

Is this correctly assigned to @satyamakgec or should it be assigned to @sisyphusSmiling ? Just want to make sure we are tracking this properly

franklywatson commented 1 year ago

It's probably nobody assigned to this TBH. The Commit-reveal being discussed isn't about the standard, but specifically for random. But maybe we can consider a standard after that?