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

Add collectibles support to Status wallet 🦋 #10373

Closed
hesterbruikman opened this issue Apr 20, 2020 · 21 comments
Closed

Add collectibles support to Status wallet 🦋 #10373

hesterbruikman opened this issue Apr 20, 2020 · 21 comments
Projects

Comments

@hesterbruikman
Copy link
Contributor

Feature Issue

Different use cases for Keycard and the Core app at Devcon 6 have been proposed to the event organizers; Per invitation of the organizers who are looking to provide applications for dogfooding at the event.

Core to the proposal is the use of NFT's:

  • authenticate attendees,
  • offer them NFTs,
  • check if they own a given NFT, thus authorizing specific actions at participating booths and related events

Currently, NFT's/ERC721 are hardcoded and will need to be expanded. Either hard coded or dynamically.

User Story

  1. As a user I want to view NFT's that I have redeemed in my Status wallet
  2. As a user I want to show/share proof of ownership of NFT's that I own, to others as authentication
  3. (optional) As a user I want to send NFT's I have redeemed to others

Related use cases to distribute and redeem NFT's will be coordinated by the Keycard team (cc @guylouis). Use case 1 and 2 above are integral to how the Core app handles ERC721 tokens and whether/how the user is able to view, manage, share proof of ownership and send these.

Description

Implementation allows to view all NFT's distributed at Devcon through Keycard:

  • NFT's will be created by Status as well as by 3rd party dapp developers
  • NFT's details can be known ahead of the event as an NFT bucket address needs to be made available and the token needs to be authenticated

Acceptance Criteria

  • Users can view NFT's redeemed from the NFTBucket in their Status Wallet
  • Users can share proof that they own an NFT
  • Display of NFT's follows Figma design (please ping @hesterbruikman to check for updates in due time.

Notes

  • Check the support of ERC721 in other wallets
  • Can ERC721 tokens be send? Can they be send in chat? What would they look like in chat? (e.g. forwarding event tickets)
  • What is the estimated effort for adding hard coded NFT's?
  • What is the estimated effort and proposal for adding NFT's dynamically and/or a submission flow for NFT creators
@hesterbruikman
Copy link
Contributor Author

@guylouis @andremedeiros please check if this accurately captures the Devcon proposal and work required by Core to support the proposal

@flexsurfer
Copy link
Member

flexsurfer commented Apr 21, 2020

the big issue with NFTs they don't follow same standard, implementation of every token is different

Check the support of ERC721 in other wallets

other wallets use third party APIs https://docs.opensea.io/reference#api-overview

Can ERC721 tokens be send? Can they be send in chat?

yes, it can be sent, and in chat as well but every token must be implemented separately

What is the estimated effort for adding hard coded NFT's?

about 4h

What is the estimated effort and proposal for adding NFT's dynamically and/or a submission flow for NFT creators

this might be tricky to implement

@errorists
Copy link
Contributor

the complete new designs are here, please let me know if I'm missing something. Collectible display, info, sending flows, and yes I went ahead and added OpenSea integration

@rachelhamlin rachelhamlin added this to To Do in Core via automation Apr 21, 2020
@guylouis
Copy link
Contributor

guylouis commented Apr 21, 2020

100%

One precision though, on devcon has not accepted our proposal yet, but the idea is that
i. during Devcon
Users will be able to collect NFTS on their keycard with a tap, prove ownership by the keycard of a given NFT by a tap. Fo this the NFTS won't actually be the property of the user wallet by they will be the property of a smartcontract that we call a NFT Bucket. So during i. users wont't be able to see their NFTs in a wallet
**ii. after devcon or once user doesn't want to use his keycard anymore, he can redeem the NFT from the NFTBucket to his wallet, this is done by going to a dApp.

It's in the context of ii that, yes, Status wallet should be able to show erc721s.

Since the NFTbuckets will be implemented prior to the show (and some totally new NFTs might be created) we can consider that we can add support for the NFTs that will be created for the show beforehand. But it's not ideal if some dapps/nftbuckets are created last minute.

If a user redeems his NFT on his wallet and still wants to prove he owns this NFT, I am pretty curious about any ideas how this could work (this is user scenario 2) ? any ideas ?

cc @gravityblast @bitgamma

@andremedeiros
Copy link
Contributor

every token must be implemented separately

@flexsurfer help me understand, why is every token separate?

@flexsurfer
Copy link
Member

flexsurfer commented Apr 21, 2020

@andremedeiros because not all of NFTs uses ERC721, they have their own contracts

@hesterbruikman
Copy link
Contributor Author

There will be a process around this as tokens need to be authenticated and addresses for NFT buckets provided, so following ERC721 could be a requirement

@flexsurfer
Copy link
Member

flexsurfer commented Apr 21, 2020

but ERC721 will work only for sending, for showing them there is no standard and every NFT should be implemented separately, but to send them we need to show them first :) so ...

@hesterbruikman
Copy link
Contributor Author

Gotcha. So we'd also need a stickerpack like specification for image formats

@andremedeiros
Copy link
Contributor

for showing them there is no standard

Well that sounds fun.

@guylouis
Copy link
Contributor

guylouis commented Apr 22, 2020

I imagine we have two problems (well at least :))
i. use a list of erc721 contract adresses of the NFTs we want to scan
ii. showing the NFTs in the wallet interface

for i. I imagine we hardcode the list for now, right ? couldn't we have the list stored on ipfs and access it from status to make adding an ERC721 easier ?

for ii. a first step could be to support only erc-721 contracts that implement the optional erc721 metadata JSON schema (See spec here which includes name, description and image.

Also: am I right saying that integrating with opensea API would solve i and ii, but might not fit with our principles (centralized, paid solution) ?

Note: a good article about his issue
https://hackernoon.com/the-one-thing-missing-from-erc-721-standard-for-digital-collectibles-on-the-blockchain-9ee26e4a918c

@flexsurfer
Copy link
Member

for ii. a first step could be to support only erc-721 contracts that implement the optional erc721 metadata JSON schema (See spec here which includes name, description and image.

would be great if we could find at least one NFT which has optional erc721 metadata :)

@flexsurfer
Copy link
Member

usually, it looks like that

(def graphql-url "https://api.pixura.io/graphql")

(defn graphql-query [address]
  (str "{
         collectiblesByOwner: allErc721Tokens(condition: {owner: \"" address "\"}) {
           collectibles: nodes {
            tokenId,
            metadata: erc721MetadatumByTokenId {
              metadataUri,
              description,
              name,
              imageUri
            }}}}"))

(defmethod load-collectibles-fx superrare [_ _ _ address _]
  {:http-post {:url                   graphql-url
               :data                  (types/clj->json {:query (graphql-query (ethereum/naked-address address))})
               :opts                  {:headers {"Content-Type" "application/json"}}

@guylouis
Copy link
Contributor

ok, so is this the right conclusion below ?

  • we can only add erc-721 support one by one by hardcoding them
  • or use a service like opensea API (but we have an issue with principles here)
  • we should define a format of ERC721 metadata that we support in status. We need this at least for Devcon if our dogfooding proposal goes through, since users will collect NFTS on the show with their keycards, and we will want users to redeem the NFTs they have collected to their status wallet at some point (often, it will be after the show). We can encourage the projects that will create dedicated NFTs to respect our format.

@StatusSceptre StatusSceptre moved this from To Do to Backlog in Core Apr 22, 2020
@flexsurfer
Copy link
Member

flexsurfer commented Apr 23, 2020

we should define a format of ERC721 metadata that we support in status

my suggestion is to use ipfs

function tokenURI(uint256 _tokenId) external view returns (string); should return ipfs://hash

"ERC721 Metadata JSON Schema"

"image" should contain ipfs://hash

this is will be similar to how our stickers work , but in that case, such NTFs will be supported only by Status

another option would be to implement component for NFTs in status which will be like the special Dapp will be opened not a full screen and it will show only specific view, for example, one NFT or list of NFTs, in that case, we could use opensea API and support all NFTs

@errorists
Copy link
Contributor

another option would be to implement component for NFTs in status which will be like the special Dapp

@flexsurfer which special Dapp do you mean? do you mean showing the collectible in a webview?

will be opened not a full screen and it will show only specific view, for example, one NFT or list of NFTs

why not full screen? so if I get that right, we have a list of collectibles you own in your wallet, here we are limited to only showing which meta, the title ?
Then opening that would bring a detailed view where we fetch all remaining metadata from open sea?
sorry for the question marks everywhere, I’m just having trouble picturing it

@guylouis
Copy link
Contributor

guylouis commented Apr 23, 2020

@flexsurfer

"but in that case, such NTFs will be supported only by Status"

can you help me understand ? thanks

@flexsurfer
Copy link
Member

flexsurfer commented Apr 23, 2020

do you mean showing the collectible in a webview?

we already show them in webview :) it will be webview only technically for user it will be looking like native it only about implementation

why not full screen?

i mean it will be part of status UI, not in the browser, we won't open browser

@flexsurfer
Copy link
Member

can you help me understand ? thanks

because all NFTs are web2, open sea also web2, so they use uri just as in regular web, not sure if they will add support for ipfs protocol

@andremedeiros
Copy link
Contributor

I am 100% with @flexsurfer here, with a small correction.

We should support any URI that tokenURI returns, with maybe a privacy toggle for IPFS only. Anything that returns the JSON metadata format is supported tho.

@churik
Copy link
Member

churik commented Sep 22, 2021

@hesterbruikman
Does it still make sense after #12485 ?

@churik churik closed this as completed Nov 25, 2021
Core automation moved this from Backlog to Done Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Core
  
Done
Development

No branches or pull requests

6 participants