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

Dynamically assign an ERC721 symbol for bridged NFTs #29

Closed
sisyphusSmiling opened this issue Apr 12, 2024 · 2 comments · Fixed by #52
Closed

Dynamically assign an ERC721 symbol for bridged NFTs #29

sisyphusSmiling opened this issue Apr 12, 2024 · 2 comments · Fixed by #52

Comments

@sisyphusSmiling
Copy link
Contributor

sisyphusSmiling commented Apr 12, 2024

Issue To Be Solved

As suggested by @aishairzay in #28 comment, the bridge should enable projects to specify their own ERC721 symbols. However, there isn't a standard metadata view that includes a symbol field. One such view was proposed for review, but it can't be counted on that this view will be immediately or even widely implemented if accepted.

Suggest A Solution

  • Require projects implement the proposed view to specify their symbol. This would be least preferable as it would prevent bridging NFTs that don't implement this view.
  • Derive a symbol based on the fields that are accessible. This could be done on the first n sanitized values of the NFTCollectionDisplay.name value so ExampleNFT Collection where n == 6 would be Examp. This is also not ideal, but at least provides some notion of an identifier. This could be taken further, removing vowels, lower case all characters, etc.
  • Assign a random symbol defined from a random string derived from Cadence's runtime randomness at the time of bridging. Again, not ideal, but it preserves the permissionless nature. This could be done with a simple util method like
  • Assign the same symbol for all bridged NFTs unless otherwise defined in EVMBridgedMetadata. This is certainly not ideal as I'd imagine project want to avoid having the same symbol as others.
access(all) fun getRandSymbol(): String {
  let r = revertibleRandom().toBigEndianBytes()
  let shift: UInt8 = 65
  return String.fromUTF8([
    shift + (r[0] % 26), shift + (r[1] % 26), shift + (r[2] % 26), shift + (r[3] % 26),
    shift + (r[4] % 26), shift + (r[5] % 26), shift + (r[0] % 26), shift + (r[6] % 26)
  ])!
}
@aishairzay
Copy link
Collaborator

Since the symbol isn't as important for non-fungible as it is in fungible tokens in all existing products I know of (since they usually just use the full name of the NFT collection in most display references), I am okay with either option 2 or 3 you put up there. I think i'd go with #2 (take first n characters).

We should definitely still support and document an optional way to set the symbol yourself in a view though.

@sisyphusSmiling
Copy link
Contributor Author

Completed in #52 according to @aishairzay suggestion - slicing the first (up to) 4 alphanumeric characters from the defining contract name. We may revisit this approach in the future when we have some user feedback on which to better inform the decision path forward. For now, the implementation unblocks NFT bridging for those without a symbol defined.

In addition, EVMBridgedMetadata was merged in onflow/flow-nft#203 and the MetadataViews contract has been updated to include it on Testnet. Next steps include folding that view into NFT documentation and guides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants