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

Replacing Rainbowkit with Web3Modal #1465

Merged
merged 23 commits into from
Mar 21, 2024

Conversation

mudrila
Copy link
Contributor

@mudrila mudrila commented Mar 16, 2024

This PR contains following changes:

  • Replace RainbowKit with Web3Modal
  • Add HTTPS support for local dev server in order to overcome possible issues for local development
  • Implement trick to open web3 modal from 2nd attempt if 1st attempt fails (happens frequently both on local and deployed site)

Closes #1450

Moving further, we'll have to update web3Modal version to v4.x+ together with other web3 libraries (viem, wagmi, etc)

Testing

  1. Connect your wallet through different wallet providers (MetaMask, CoinBase, Injected Wallet, WalletConnect)
  2. Try to create any type of Safe (better to test with Multisig and Azorius ERC-20/ERC-721). Make sure safe is properly created
  3. Try to create proposal for that Safe(s). Make sure proposal properly created
  4. Try to vote on that proposal. Make sure vote appears right after you voted, breakdown updated properly, etc.
  5. Try to execute that proposal

@mudrila mudrila added enhancement New feature or request maintenance Keep the lights on labels Mar 16, 2024
@mudrila mudrila self-assigned this Mar 16, 2024
Copy link

netlify bot commented Mar 16, 2024

Deploy Preview for fractal-dev ready!

Name Link
🔨 Latest commit f73c306
🔍 Latest deploy log https://app.netlify.com/sites/fractal-dev/deploys/65fc682349d1c40008842f1b
😎 Deploy Preview https://deploy-preview-1465.app.dev.fractalframework.xyz
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mudrila mudrila marked this pull request as ready for review March 16, 2024 11:57
} catch (e) {
console.error(e);
try {
open();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem of not opening Web3Modal from first attempt happens quite frequently. However, I haven't ever noticed that it doesn't work from 2nd attempt, and this fix works smoothly for me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So prior to the NextJS removal update. I could not get useWeb3Modal to work properly within the header, while still calling createWeb3Modal else where in the app. The Header component seem to be getting mounted before the providers were getting set and that function is called. Since it seems to be 'working' now but flaky I wonder if there is something in how we mount our header components that still wonky

Copy link
Contributor

@Da-Colon Da-Colon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loading a DAO up (Azorius) Seems to be really slow. looking at the network tab, all the requests are going through walletconnect provider.

image

Can also you talk a little bit on why you needed to add mkcert plugin and HTTPS support?

} catch (e) {
console.error(e);
try {
open();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So prior to the NextJS removal update. I could not get useWeb3Modal to work properly within the header, while still calling createWeb3Modal else where in the app. The Header component seem to be getting mounted before the providers were getting set and that function is called. Since it seems to be 'working' now but flaky I wonder if there is something in how we mount our header components that still wonky

src/providers/NetworkConfig/testWallet.ts Outdated Show resolved Hide resolved
@mudrila
Copy link
Contributor Author

mudrila commented Mar 18, 2024

Can also you talk a little bit on why you needed to add mkcert plugin and HTTPS support?

@Da-Colon I was experiencing issues while trying to connect wallet locally and found some GH discussion that was mentioning using HTTPS local dev server to overcome this problem. That, eventually, worked.

@mudrila
Copy link
Contributor Author

mudrila commented Mar 18, 2024

So prior to the NextJS removal update. I could not get useWeb3Modal to work properly within the header, while still calling createWeb3Modal else where in the app. The Header component seem to be getting mounted before the providers were getting set and that function is called. Since it seems to be 'working' now but flaky I wonder if there is something in how we mount our header components that still wonky

No, that seems like somewhat known issue with web3modal v3 (the one we are using) and should be fixed with migration to v4

@mudrila
Copy link
Contributor Author

mudrila commented Mar 18, 2024

Loading a DAO up (Azorius) Seems to be really slow. looking at the network tab, all the requests are going through walletconnect provider.

Damn, that's right. Our custom RPC node is not picked up. Which in a nutshell should be good, but certainly not at this stage 😄
I've tried to play with injecting publicClient but wasn't able to make it work. I'll try to do this in scope of #1444 and hopefully this will work there

src/components/ui/utils/ConnectWalletToast.tsx Outdated Show resolved Hide resolved
src/providers/NetworkConfig/web3-modal.config.ts Outdated Show resolved Hide resolved
src/providers/NetworkConfig/web3-modal.config.ts Outdated Show resolved Hide resolved
src/providers/NetworkConfig/web3-modal.config.ts Outdated Show resolved Hide resolved
vite.config.mts Outdated Show resolved Hide resolved
@mudrila mudrila requested a review from adamgall March 18, 2024 20:30
@adamgall adamgall marked this pull request as ready for review March 20, 2024 15:52
Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of comments, but lookin good!

src/hooks/utils/useDisplayName.ts Show resolved Hide resolved
src/hooks/DAO/loaders/useLoadDAONode.ts Show resolved Hide resolved
Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shipit

edit: not yet

@tomstuart123
Copy link

**1) Coinbase Wallet + Brave **
Azorious

  • Try to create Azorious Safe = SUCCESS
  • Delegate vote = SUCCESS
  • Try to create proposal for that Safe(s). = SUCCESS
  • Try to vote on that proposal. Make sure vote appears right after you voted, breakdown updated properly, etc. = SUCCESS
  • Try to execute that proposal = SUCCESS

Multisig

  • Try to create Azorious Safe = SUCCESS
  • Try to create proposal for that Safe(s). = SUCCESS
  • Try to vote on that proposal. Make sure vote appears right after you voted, breakdown updated properly, etc. = SUCCESS
  • Try to execute that proposal = SUCCESS

**2) SafeUI +WC + Chrome **
Azorious

  • create azoriuos safe - SUCCESS - no issue
  • Delegate vote = PARTIAL SUCCESS - issue - delegate vote toaster didn't remove by itself
  • Try to create proposal for that Safe(s). = PARTIAL SUCCESS - issue - 'creating proposal' toaster didn't remove- Try to vote on that proposal. = PARTIAL SUCCESS - issue - 'casting vote' toaster didn't remove by itself
  • Try to execute that proposal = PARTIAL SUCCESS - issue - 'casting vote' toaster didn't remove by itself

Multisig

Screenshot 2024-03-21 at 12 09 44

here's the safe with the issues - https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0xa719FcC542a59A6C52b4C482e562334bF1aF9828

**3) Metamask + Chrome **
Azorious

  • Try to create Azorious Safe =SUCCESS
  • Delegate vote (azorius) = SUCCESS
  • Try to create proposal for that Safe(s). = SUCCESSS
  • Try to vote on that proposal. Make sure vote appears right after you voted, breakdown updated properly, etc = SUCCESS
  • Try to execute that proposal = SUCCESS

Multisig

  • Try to create Azorious Safe = SUCCESS
  • Try to create proposal for that Safe(s). = SUCCESS
  • Try to vote on that proposal. Make sure vote appears right after you voted, breakdown updated properly, etc. = SUCCESS
  • Try to execute that proposal = SUCCESS

Note links to test safes
--> https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0xa719FcC542a59A6C52b4C482e562334bF1aF9828
--> https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0x7DA5B360cf079b8c9A78F954cc7F01f94c21DA97
--> https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0x5D1b267727eb4b84b5Ee9A1b481Ae0A0293C5e78
--> https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0x652A8159eCc60F63589FbE35FBDa0E77679BA70E
--> https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0x21d14603f1c55a5be95d49A47CF44707090332de
--? https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0x652A8159eCc60F63589FbE35FBDa0E77679BA70E
--> https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0x5De7c6164710DF979E73a7463E068f34589864dA
--> https://deploy-preview-1465.app.dev.fractalframework.xyz/daos/0x7DA5B360cf079b8c9A78F954cc7F01f94c21DA97

Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, found another thing that should be fixed up as part of the different behaviors of RainbowKit and Web3Modal.

src/pages/DAOController.tsx Outdated Show resolved Hide resolved
@mudrila mudrila requested a review from adamgall March 21, 2024 15:31
@adamgall adamgall merged commit 7fed0c1 into develop Mar 21, 2024
6 of 7 checks passed
@adamgall adamgall deleted the refactor/#1450-replace-rainbowkit-with-web3-modal branch March 21, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request maintenance Keep the lights on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace RainbowKit with Web3Modal
4 participants