numbersprotocol / capture-lite

A photo-sharing app with only verifiable photos and videos.
https://numbersprotocol.github.io/
GNU General Public License v3.0
25 stars 6 forks source link

Show custody wallet balance instead of Capture wallet balance #1210

Closed sync-by-unito[bot] closed 2 years ago

sync-by-unito[bot] commented 2 years ago

As a user, I want to see my custody wallet balance instead of Capture wallet if my main wallet is the custody wallet.

┆Issue is synchronized with this Asana task by Unito ┆Created By: Tammy Yang

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

Depend on backend release

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

James Chien What's difference between asset wallet and managed wallet? Seems like getManagedWallet$ ( https://github.com/numbersprotocol/capture-lite/blob/3ac96f0f5d1068de52c34a33162d2a8f61811022/src/app/shared/dia-backend/wallet/dia-backend-wallet.service.ts#L59-L68 ) is not being used anywhere? Search here ( https://github.com/numbersprotocol/capture-lite/search?q=getManagedWallet%24 ).

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

To show custody wallet balance instead of Capture wallet balance, I may need to update the api endpoint url here ( https://github.com/numbersprotocol/capture-lite/blob/3ac96f0f5d1068de52c34a33162d2a8f61811022/src/app/shared/dia-backend/wallet/dia-backend-wallet.service.ts#L38 ) right?

sync-by-unito[bot] commented 2 years ago

➤ James Chien commented:

Ray Hung Asset wallet

Managed wallet

The current problem is App retrieves asset wallet's NUM balance, but after backend migrates the NUM payment to managed wallet, App should get managed wallet's NUM balance instead.

Here's what I have in mind. Since we will not perform the NUM wallet migration at backend immediately, it's best that App doesn't need to know about the migration at all. I'll add an endpoint /wallets/num-wallet/ for App to use. Change the data source for displaying NUM balance to this endpoint and that should be all.

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

Update: /wallets/num-wallet/ ( https://dia-backend-dev.numbersprotocol.io/api/v3/redoc/#operation/wallets_num_wallet ) is working. Frontend app is getting data from the endpoint.

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

James Chien I saw that Backend (Seal API)-Storage-backend rc13.1-0.33.1-postman has passed QA testing. Let me know when I should go ahead and submit a PR for frontend app changes to call the new api/v3/wallets/num-wallet/ endpoint

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

Just opened one here: https://github.com/numbersprotocol/capture-lite/pull/1269 ( https://github.com/numbersprotocol/capture-lite/pull/1269 )

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

James Chien Another question, since now we will be displaying balance from managed wallet, should we update wallet address to managed wallet address? Now we're still displaying asset wallet address

sync-by-unito[bot] commented 2 years ago

➤ James Chien commented:

Asset wallet's primary purpose is for signing Capture proofs and we'd like the users to own this wallet, so we wouldn't want to replace it no matter NUM balance comes from it or not. For displaying custody wallet's address, there's a task for it Show NUM balance from custody wallet and allow users to see custody-wallet transaction history ( https://app.asana.com/0/1201016280880500/1201812866141486/f )

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

I see. Not sure if all users are aware that there are two wallets (asset/managed). Do you think we can update "Wallet Address" to "Asset Wallet Address" or "Capture Wallet Address" to be more explicit?

sync-by-unito[bot] commented 2 years ago

➤ James Chien commented:

A good point, but I'm not sure what is the text we want to show to users

Tammy Yang Ray has some UI suggestions^^

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

Update: feat(dia-backend-wallet.service.ts): show custody wallet balance #1269 ( https://github.com/numbersprotocol/capture-lite/pull/1269 ) has been merged

sync-by-unito[bot] commented 2 years ago

➤ Ray Hung commented:

Tammy YangDaYuan Some UI ideas:

sync-by-unito[bot] commented 2 years ago

➤ Tammy Yang commented:

Ray Hung good idea

I think in the future, we can call custody wallet “asset wallet” and Capture wallet “integrity signature key” or “integrity key” or “signature” because we will only use it for “signature” instead of using it as a wallet.

Bofu Chen what's your suggestion of the name of the future capture wallet?

sync-by-unito[bot] commented 2 years ago

➤ Bofu Chen commented:

My proposal is "Capture Web3 Wallet" (or "Web3 Wallet" in short) because

  1. Web3 indicates/implies that the user is fully responsible for it.
  2. Integrity signature is one of the "web3" actions.
    1. The other actions include withdrawing from the Asset Wallet.

I use "Web3 Wallet" instead of "Capture Wallet" because the user might think "Capture Wallet" as the wallet with balance which is the Asset Wallet actually.