Giveth / giveth-dapps-v2

This project is the aggregation of GIVeconomy and Giveth.io DApps in a single repo
https://staging.giveth.io
GNU General Public License v3.0
64 stars 34 forks source link

Connecting to the Dapp with a Solana Wallet #3400

Closed divine-comedian closed 10 months ago

divine-comedian commented 1 year ago

figma

https://www.figma.com/file/vMiyVd1Ly5LjgSyZrChVV8/Giveth-users?type=design&node-id=1101%3A3779&mode=design&t=6gBBVPBLcG3Z1cC7-1


Users will now have another option to sign in to the Giveth Dapp using a Solana wallet provider

This should be separate from the flow for EVM (Ethereum based) wallets - the user needs to pick either Solana or Ethereum and proceed to separate options

We should consider #3370 when making any changes

divine-comedian commented 1 year ago

Re: @jainkrati - we should decide which package we want to use for connecting to the dapp.

jainkrati commented 1 year ago

https://solana-labs.github.io/solana-web3.js/

divine-comedian commented 1 year ago

Some links from PM Design Sync: Need to harmonize this with new designs in here #3370 How do we make this compatible with future non-EVM chains? How do we make this compatible with AA in the far future?

https://docs.coin98.com/developer-guide/coin98-adapter

https://github.com/WalletConnect/web3modal

https://www.dynamic.xyz/features/crypto-native-login

aminlatifi commented 1 year ago

@jainkrati @divine-comedian I looked at the dynamic wallet, it has a multi-network wallet with spectacular features. It has practical features (like multi-wallet support, email verification, etc.) and totally customizable. The only drawbacks I found so far

Big bundle size Probably not open-source Limited features on the free plan I mailed the support to get confirmation whether it's open-source or not, and asked whether they may give us discounts?

BTW, I think it's worth looking at this 10 minutes demo. https://youtu.be/LKP-aJOTxfw

MoeNick commented 1 year ago

Yeah the best UX is to customize the UI of web3modal, do you think it's possible?

divine-comedian commented 1 year ago

OMG I'm so sorry - I accidentally edited Amin's comment with my response instead of responding in a new comment.

@aminlatifi - Does it work with our current implementation of wagmi & viem?

Devs worked very hard and long to make the dapp compatible with wagmi & wallet connect

In general, given the delivery constraints I would recommend keeping our scope small and trying to work with what we've already integrated

I think what came out of yesterday's design call is the question is if we can customize the UI of web3modal? and if so how customizable is it?

jainkrati commented 1 year ago

I would suggest using the free and open source solana wallet adapter @aminlatifi . @MoeNick I could not find solana support for wagmi in the documentation. So for now, with minimum efforts we can just have a login with solana button which triggers solana wallet adapter and a variable in the frontend which maintains walletDetails { blockchainType: , address: }

aminlatifi commented 12 months ago

OMG I'm so sorry - I accidentally edited Amin's comment with my response instead of responding in a new comment.

@aminlatifi - Does it work with our current implementation of wagmi & viem?

Devs worked very hard and long to make the dapp compatible with wagmi & wallet connect

In general, given the delivery constraints I would recommend keeping our scope small and trying to work with what we've already integrated

I think what came out of yesterday's design call is the question is if we can customize the UI of web3modal? and if so how customizable is it?

Wagmi and viem are built for evm EVM-compatible chains and support that protocol. I don't think we can't get Solana support out of it, or at least we need more research. At this stage, I am adding new dependencies/libraries to support connecting to solana wallet (#3447). BTW, I think we finally have lots of edge cases to deal with and the integration would be challenging.

aminlatifi commented 12 months ago

I had connected solana wallet, but it's incomplete till we can successfully sign in. Sign in process includes:

I think we would be able to finish it till the end of Dec 6.

aminlatifi commented 12 months ago
image

I could connect front end to the authentication server (https://github.com/Giveth/SiweAuthMicroservice/pull/22), committed to #3447. The procedure wasn't complete since I didn't run a local instance of the notification service connected to my authentication service. So I want to leave it in this state for others to develop, or for myself to continue next week.

For easier development, I suggest we review and merge Giveth/SiweAuthMicroservice#22 if it's okay, that will make development easier as staging services will be connected to a Solana supporting authentication service and it won't be necessary to bring up all services locally.

aminlatifi commented 12 months ago
image

I could connect front end to the authentication server (Giveth/SiweAuthMicroservice#22), committed to #3447. The procedure wasn't complete since I didn't run a local instance of the notification service connected to my authentication service. So I want to leave it in this state for others to develop, or for myself to continue next week.

For easier development, I suggest we review and merge Giveth/SiweAuthMicroservice#22 if it's okay, that will make development easier as staging services will be connected to a Solana supporting authentication service and it won't be necessary to bring up all services locally.

@jainkrati

aminlatifi commented 11 months ago

@MohammadPCh where this feature is enabled to be tested? @maryjaf

maryjaf commented 11 months ago

I think it's better for the first phase of testing, this changes be deployed on a feature branch

I've been some testing and there are some problems 1- I don't understand why by refreshing page the wallet address be changed

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/bb130824-12c4-48cf-9094-8ad21d02ddaf

2- I couldn't complete the flow of creating user profile

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/47d8ce48-ffa5-4684-bb10-8921465cc178

aminlatifi commented 11 months ago

@maryjaf The burner wallet is unsafe disposal wallet, and its key changes by refreshing the page. https://github.com/solana-labs/wallet-adapter/blob/55ff7e0a7c68a3ed12771f0feb06331a36b9509c/packages/wallets/unsafe-burner/src/adapter.ts#L27-L30 It's good you mentioned it, we must enable it only in test env. I will do it.

aminlatifi commented 11 months ago

@maryjaf The second issue is solved, the backend changes was merged to the develop instead of staging! Besides, I made this to only enable burner wallet in test mode https://github.com/Giveth/giveth-dapps-v2/pull/3498

maryjaf commented 11 months ago

@maryjaf The second issue is solved, the backend changes was merged to the develop instead of staging! Besides, I made this to only enable burner wallet in test mode #3498

Thanks @aminlatifi the problem has been resolved and the profile be created now but there is a sth wrong in this flow, after completing the flow the user should refresh the page that changes be applied I tested on this branch:https://giveth-dapps-v2-git-feat-solanaconnect-givethio.vercel.app/account

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/6701e84d-a88d-4f8a-8ac6-ba3682cf73ef

aminlatifi commented 11 months ago

@maryjaf The second issue is solved, the backend changes was merged to the develop instead of staging! Besides, I made this to only enable burner wallet in test mode #3498

Thanks @aminlatifi the problem has been resolved and the profile be created now but there is a sth wrong in this flow, after completing the flow the user should refresh the page that changes be applied I tested on this branch:https://giveth-dapps-v2-git-feat-solanaconnect-givethio.vercel.app/account

Screen.Recording.2023-12-18.at.4.35.51.PM.mov

Must be fixed by https://github.com/Giveth/giveth-dapps-v2/pull/3502

maryjaf commented 11 months ago

2- I couldn't complete the flow of creating user profile

Thanks @aminlatifi # 2 has been resolved totally

maryjaf commented 11 months ago

I have a question 3- when I select solana what is the expected behavior by tapping on switch network ? @aminlatifi @divine-comedian Now the below pic is shown solana--> tap on switch network

image.png image.png

Ethereum --> switch network

image.png
aminlatifi commented 11 months ago

I have a question when I select solana what is the expected behavior by tapping on switch network ? @aminlatifi @divine-comedian Now the below pic is shown solana--> tap on switch network image.png image.png

Ethereum --> switch network image.png

I think we must hide it!

maryjaf commented 11 months ago

4- What should be shown on GIVeconomy when the network is set on Solana ?

image

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/f65d02e2-369c-465a-a06e-dc3eaac7338a

maryjaf commented 11 months ago

5- Gitcoin passport should be disabled when the user connected with solana?

image.png image.png
aminlatifi commented 11 months ago

4- What should be shown on GIVeconomy when the network is set on Solana ?

image

Screen.Recording.2023-12-25.at.11.58.24.AM.mov

Right now, it has a behavior like when no wallet was connected in the past.

MoeNick commented 11 months ago

@aminlatifi Please let me know if you are blocked by applying @maryjaf comments. Is there any scenario that is not clear?

maryjaf commented 11 months ago

I have a question when I select solana what is the expected behavior by tapping on switch network ? @aminlatifi @divine-comedian Now the below pic is shown solana--> tap on switch network image.png image.png Ethereum --> switch network image.png

I think we must hide it!

Is this final that switch network should be hide in all pages when a user sign in with solana address? should we create separate issue for this ? @aminlatifi @MoeNick

imagine this scenario one project has two recipient address on solana and polygon the project be added to a round that it is supported by polygon only my user login with solana address and select this project in round to donate in this case if" switch network" is shown by tapping on nothing happens and if switch network" isn't shown i think a design should be required for this scenario ? am I right? @MoeNick

maryjaf commented 11 months ago

6- could you please take a look @aminlatifi @alireza-sharifpour when I change my wallet on EVM, by switching user, my account view for example would be changed but this scenario on solana/phantom wallet happens by refreshing page

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/69518900-99c8-4f34-9c1a-65360b0c6852

MoeNick commented 11 months ago

I have a question when I select solana what is the expected behavior by tapping on switch network ? @aminlatifi @divine-comedian Now the below pic is shown solana--> tap on switch network image.png image.png Ethereum --> switch network image.png

I think we must hide it!

Is this final that switch network should be hide in all pages when a user sign in with solana address? should we create separate issue for this ? @aminlatifi @MoeNick

imagine this scenario one project has two recipient address on solana and polygon the project be added to a round that it is supported by polygon only my user login with solana address and select this project in round to donate in this case if" switch network" is shown by tapping on nothing happens and if switch network" isn't shown i think a design should be required for this scenario ? am I right? @MoeNick

We should hide the switch network from the header menu, in this case, I explained to Alireza that switching from/to EVM chains means logging out and showing this splash page again to the user.

maryjaf commented 10 months ago

7- Bug in showing user when user switches between solana and ethereum in below video, I sign in with etherum and my user is shown correctly by tapping on "signout" and then tap on "sign in with solana" and select phantom, my profile pic and name related to ethereum account will be shown

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/10300f0e-1853-451b-8500-df0b932e0a52

maryjaf commented 10 months ago

I have a question 3- when I select solana what is the expected behavior by tapping on switch network ? @aminlatifi @divine-comedian Now the below pic is shown solana--> tap on switch network image.png image.png

Ethereum --> switch network image.png

I've checked on stg but there is no change for this flow, please take a look @alireza-sharifpour I am not sure if this flow is also a part of the last PR changes or not

Should a separate issue be registered for this? @MoeNick

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/2bdc94d1-ae26-4a27-86c9-088006405cc7

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/8a3aeeb8-1f0e-4ab6-94d9-aaf0cfabe54b

maryjaf commented 10 months ago

6- could you please take a look @aminlatifi @alireza-sharifpour when I change my wallet on EVM, by switching user, my account view for example would be changed but this scenario on solana/phantom wallet happens by refreshing page

Screen.Recording.2024-01-04.at.3.27.22.PM.mov

Do we have any plan for fixing this or this behavior is accepted? @aminlatifi @alireza-sharifpour

Phantom-solana: need to refresh

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/83073166-ce1e-40c9-b91a-0027fa5a5063

metamask-eth : witouth refreshing

https://github.com/Giveth/giveth-dapps-v2/assets/111529185/88b2ce7a-5e2d-49b6-b38f-4c01cb856906

maryjaf commented 10 months ago

Do we have any plan for fixing this or this behavior is accepted? @aminlatifi @alireza-sharifpour

Phantom-solana: need to refresh

Item 6 Has been resolved for me, mybe it's about my connection

aminlatifi commented 10 months ago

6- could you please take a look @aminlatifi @alireza-sharifpour when I change my wallet on EVM, by switching user, my account view for example would be changed but this scenario on solana/phantom wallet happens by refreshing page Screen.Recording.2024-01-04.at.3.27.22.PM.mov

Do we have any plan for fixing this or this behavior is accepted? @aminlatifi @alireza-sharifpour

Phantom-solana: need to refresh

Screen.Recording.2024-01-11.at.6.35.31.PM.mov metamask-eth : witouth refreshing

Screen.Recording.2024-01-11.at.6.37.06.PM.mov

@alireza-sharifpour have you pushed fixes to theses?

maryjaf commented 10 months ago

6- could you please take a look @aminlatifi @alireza-sharifpour when I change my wallet on EVM, by switching user, my account view for example would be changed but this scenario on solana/phantom wallet happens by refreshing page

Today I've checked more this problem when I complete sign flow and get authorization, by switching wallet project view(owner view / other) be changed But otherwise, the behavior of the system is similar to the video I attached above @aminlatifi @alireza-sharifpour

aminlatifi commented 10 months ago

6- could you please take a look @aminlatifi @alireza-sharifpour when I change my wallet on EVM, by switching user, my account view for example would be changed but this scenario on solana/phantom wallet happens by refreshing page Screen.Recording.2024-01-04.at.3.27.22.PM.mov

Do we have any plan for fixing this or this behavior is accepted? @aminlatifi @alireza-sharifpour

Phantom-solana: need to refresh

Screen.Recording.2024-01-11.at.6.35.31.PM.mov metamask-eth : witouth refreshing

Screen.Recording.2024-01-11.at.6.37.06.PM.mov

The issue is on the Solana wallet library, the hook doesn't update publicKey (user address) value on switching to another Solana account. To be sure, the issue exists here as well https://soldons.vercel.app/

aminlatifi commented 10 months ago

6- could you please take a look @aminlatifi @alireza-sharifpour when I change my wallet on EVM, by switching user, my account view for example would be changed but this scenario on solana/phantom wallet happens by refreshing page Screen.Recording.2024-01-04.at.3.27.22.PM.mov

Do we have any plan for fixing this or this behavior is accepted? @aminlatifi @alireza-sharifpour Phantom-solana: need to refresh Screen.Recording.2024-01-11.at.6.35.31.PM.mov metamask-eth : witouth refreshing Screen.Recording.2024-01-11.at.6.37.06.PM.mov

The issue is on the Solana wallet library, the hook doesn't update publicKey (user address) value on switching to another Solana account. To be sure, the issue exists here as well https://soldons.vercel.app/

I reported the issue on the library repo, https://github.com/solana-labs/wallet-adapter/issues/883 @jainkrati

aminlatifi commented 10 months ago

@MoeNick @maryjaf As I commented above, the issue is with the library provided by Solana. Do you think it's a serious issue? asking since we can wait till library providers fix this. Otherwise, fixing this can be challenging.

divine-comedian commented 10 months ago

@aminlatifi - Do you want to get in touch with our Solana Foundation contact? Maybe she can expedite the bug fixes w/ wallet adaptor team.

alireza-sharifpour commented 10 months ago

I have a question 3- when I select solana what is the expected behavior by tapping on switch network ? @aminlatifi @divine-comedian Now the below pic is shown solana--> tap on switch network image.png image.png Ethereum --> switch network image.png

I've checked on stg but there is no change for this flow, please take a look @alireza-sharifpour I am not sure if this flow is also a part of the last PR changes or not

Should a separate issue be registered for this? @MoeNick

Screen.Recording.2024-01-11.at.5.55.11.PM.mov Screen.Recording.2024-01-11.at.5.57.52.PM.mov

Thanks @maryjaf for all testing, The issue related to switching from solana to EVM wallets should be resolved now, after discussing with @MoeNick, we decided to change the flow and show the WelcomModal to user when they want to switch from Solana wallets inside the user menu. It's merged and ready for test.

maryjaf commented 10 months ago

I have a question 3- when I select solana what is the expected behavior by tapping on switch network ? @aminlatifi @divine-comedian Now the below pic is shown solana--> tap on switch network image.png image.png Ethereum --> switch network image.png

I've checked on stg but there is no change for this flow, please take a look @alireza-sharifpour I am not sure if this flow is also a part of the last PR changes or not Should a separate issue be registered for this? @MoeNick Screen.Recording.2024-01-11.at.5.55.11.PM.mov Screen.Recording.2024-01-11.at.5.57.52.PM.mov

Thanks @maryjaf for all testing, The issue related to switching from solana to EVM wallets should be resolved now, after discussing with @MoeNick, we decided to change the flow and show the WelcomModal to user when they want to switch from Solana wallets inside the user menu. It's merged and ready for test.

Thanks @alireza-sharifpour this issue has been resolved now when my network is set on eth, by tapping on "switch network" I see the previous modal that i could select all other eth chains and if my network is set on solana by tapping on switch network I see the below page

image
maryjaf commented 10 months ago

Is it possible that this change affects on signing with mobile and previous changes related to wagmi & web3 modal ? I couldn't sign in with eth-metamask on STG env on mobile @alireza-sharifpour @MohammadPCh but I could sign in on prod

maryjaf commented 10 months ago

Is it possible that this change affects on signing with mobile and previous changes related to wagmi & web3 modal ? I couldn't sign in with eth-metamask on STG env on mobile @alireza-sharifpour @MohammadPCh but I could sign in on prod

it seems it was my problem, now I could sign in with metamask on mobile on stg without any problem

maryjaf commented 10 months ago

@MoeNick @maryjaf As I commented above, the issue is with the library provided by Solana. Do you think it's a serious issue? asking since we can wait till library providers fix this. Otherwise, fixing this can be challenging.

Do you agree to make this problem a separate issue? And in this case, the current issue for connecting to the dApp, will not be blocked by this problem @aminlatifi

aminlatifi commented 10 months ago

@MoeNick @maryjaf As I commented above, the issue is with the library provided by Solana. Do you think it's a serious issue? asking since we can wait till library providers fix this. Otherwise, fixing this can be challenging.

Do you agree to make this problem a separate issue? And in this case, the current issue for connecting to the dApp, will not be blocked by this problem @aminlatifi

Yeah, good idea. BTW, @alireza-sharifpour has prepared a PR to solve this issue with Phantom wallet.

maryjaf commented 10 months ago

Thanks guys All problems of this issue have been resolved