cryptonetlab / retriev

Home of Retriev protocol (by CryptoNet + YOMI)
https://retriev.org
18 stars 6 forks source link

Add NFT improvement to Smart Contract #10

Closed 0xjona closed 2 years ago

0xjona commented 2 years ago

We want to add this new feature: the retrievability deals will appear as an NFT for both parties (client and provider) in their wallet.

irenegia commented 2 years ago

Step 0

irenegia commented 2 years ago

@nicola could you please write here any specif request for this new feature? Thx I remember we discussed

  1. Having a transferable NFT for the provider, since he can decide to sell the deal to another provider;
    • However, I do not like this feature since I believe it goes in the opposite direction of having the client that chooses a list of providers who are the only once allowed to accept the client's proposal.
  2. For the client a nontransferable is fine;

Anything else?

turinglabsorg commented 2 years ago

@nicola i started the integration of the ERC721 contract, so the dynamic will be:

1) Once a deal is created a new non-transfereable NFT is issued to client 2) When deal is accepted by a provider a transfereable NFT is issued to provider

What about metadata? What should we include inside and how we should compose the image? Can we start creating an API that serves all the metadata or do we want to think about a solution inside IPFS?

nicola commented 2 years ago

I was reflecting, having non-transferable NFTs to the user could be very painful - especially if they don't want them to show up.

My proposal is to only do (2). Instead of storing the data of the deal in another smart contract, they should be in the NFT (on-chain). In other words, the NFT is the deal. Regarding the preview, I don't think it should be the hash of the file.. we can make a simple nice image with the storage on-boarding logo (or an SVG that reads the smart contract to describe the deal - e.g. the ENS NFTs)

turinglabsorg commented 2 years ago

@nicola yes it can be done without problem, i've done something similar on polygon, will try to add the functions and understand if we reach the gas limit (we're very close). If we reach it we'll need to split the contract in 2 and it's a little bit longer task but it can be done.

0xjona commented 2 years ago

@nicola @irenegia so we agreed on what to implement! About the image, we can start the process for a basic brand-identity (we already have an issue about the name) with some icons to simplify the narrative of the whole protocol process. This could be one of the first issues assigned to the graphic designer (and me), with the aim to have a diagram made up of those icons (that we can use all over the websites and metadata ecc).

What do you think?

irenegia commented 2 years ago

@0xjona yes, sure, let's start work on the name and logo!! See #17, (but remember, let's keep this simple for now! )

nicola commented 2 years ago

I suggest we create another issue for the NFT preview design and we tag it as design work to be done. I am new to the tooling, but in the future I will create issues like this one.

turinglabsorg commented 2 years ago

I suggest we create another issue for the NFT preview design and we tag it as design work to be done. I am new to the tooling, but in the future I will create issues like this one.

Agree, let's create a new design-related issue, can you help with that @0xjona?

0xjona commented 2 years ago

thanks @nicola and @turinglabsorg! It's in today's agenda https://github.com/protocol/retriev/issues/24#issuecomment-1134851569

turinglabsorg commented 2 years ago

@0xjona @irenegia we still need to define how nfts should work, do you want to plan a meeting or can we do async?

turinglabsorg commented 2 years ago

@nicola @0xjona this is a preview of the nft collection: https://testnets.opensea.io/collection/retriev

0xjona commented 2 years ago

Things that need to be validated/answered

  1. Any comments/feedbacks on how the NFT shows in the collection? https://testnets.opensea.io/collection/retriev

  2. consider that making NFTs per deal costs us 30% more in GAS (+50k), and the price depends on the network (which considering ethereum is kinda unpredictable atm). Are we okay with thins? Note that in any case (with or without NFTs), this contract is too expensive to launch on Ethereum! Are we fine with this?

  3. Are NFTs going to be transferable?

    • [ ] Yes on both side
    • [ ] Yes, just the provider one (only if the deal is still open/running)
    • [ ] No comments? [ Comment: Aggregating data from the NFTs the provider is holding (like duration, payment and collateral) can help us define a score per each NFT hold and eventually define a ranking! This would be a good solutions for #2 , but we need no transferableNFTs for this ]

We need a closing feedback from @nicola about this issue ASAP. We’ll open a new one in the future related to the format of metadata (check opensea for now) and to the potentiality of using NFTs (e.g. snapshot voting power, DAO, token-gated chats ecc).

irenegia commented 2 years ago

update

We had a call on wed (8th june), here what we agreed (correct if I am wrong):

CC @0xjona

0xjona commented 2 years ago

Yes agreed!

@turinglabsorg you’ve coded a render for NFTs, does it help us design also the metadata?

turinglabsorg commented 2 years ago

@0xjona i think this issue can be closed, we've nft fully implemented in contract and live at: https://testnets.opensea.io/assets/rinkeby/0xbc331a7bea063dbce8b3d16f77850b617bc36cba/1

0xjona commented 2 years ago

Thanks! We've opened issue #46 to complete the process