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

Commit reveal Standard to FLOW #89

Open
Peixer opened this issue May 20, 2022 · 7 comments
Open

Commit reveal Standard to FLOW #89

Peixer opened this issue May 20, 2022 · 7 comments
Labels
Feature Feedback SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board

Comments

@Peixer
Copy link

Peixer commented May 20, 2022

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
    }
    
    ....
  }
@franklywatson franklywatson added the SC-Eng Issues that we want to see surfaced in SC-Eng ZH Board label May 26, 2022
@psiemens
Copy link
Contributor

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
Copy link
Author

Peixer commented May 30, 2022

@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
Copy link
Contributor

bjartek commented Jul 12, 2022

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
Copy link

bluesign commented Jul 13, 2022

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
Copy link
Contributor

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
Copy link
Member

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

@franklywatson
Copy link
Collaborator

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?

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

7 participants