hesterbruikman commented 4 years ago

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

As a user I want to view NFT's that I have redeemed in my Status wallet
As a user I want to show/share proof of ownership of NFT's that I own, to others as authentication
(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 commented 4 years ago

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

flexsurfer commented 4 years ago

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 commented 4 years ago

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

guylouis commented 4 years ago

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 commented 4 years ago

every token must be implemented separately

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

flexsurfer commented 4 years ago

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

hesterbruikman commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

andremedeiros commented 4 years ago

for showing them there is no standard

Well that sounds fun.

guylouis commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.

flexsurfer commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@flexsurfer

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

can you help me understand ? thanks

flexsurfer commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 2 years ago

@hesterbruikman Does it still make sense after https://github.com/status-im/status-react/pull/12485 ?

status-im / status-mobile

Add collectibles support to Status wallet 🦋 #10373

Feature Issue

User Story

Description

Acceptance Criteria

Notes