feat: add metaMask SDK connector #5028

EdouardBougon · 2024-10-23T20:46:05Z

MetaMask SDK integration

This PR adds the connection to MetaMask using the MetaMask SDK.

The SDK must be used for both connecting via the extension and on mobile.

vercel · 2024-10-23T20:46:09Z

@EdouardBougon is attempting to deploy a commit to the cow Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2024-10-23T20:46:20Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

socket-security · 2024-10-23T20:47:12Z

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

vercel · 2024-10-24T12:10:14Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
cowfi	✅ Ready (Inspect)	Visit Preview	Dec 16, 2024 11:45am
sdk-tools	✅ Ready (Inspect)	Visit Preview	Dec 16, 2024 11:45am
swap-dev	✅ Ready (Inspect)	Visit Preview	Dec 16, 2024 11:45am

elena-zh · 2024-10-24T17:26:00Z

Hey @EdouardBougon , thank you for your contribution!

I've reviewed the PR and have some issues to report:

Different networks when connect to MM:

open cowswap
connect to MM on Sepolia (for example) --> the app sends a request to change a network to Ethereum.
Expected: it should not force a user to connect to Ethereum. Sepolia details should be displayed on CoW Swap

When another wallet extensions are enabled in a browser, they are duplicating in a connection modal
Mobile: in 3 cases out of 4, when I connected to MM, I saw 'could not load balances' error message --> balances were not loaded
When I finally saw balances, I tried placing a trade, but a request was not sent to the wallet. Tested on iOS 17.6.1, Chrome+MM (WC)
(reproduced only once): I sent a request to change a network in MM ( iOS 17.6.1, Safari+MM (WC)), accepted in MM app, and the app started to redirect me to MM again and again until I closed it. See the video:

IMG_7190.MP4

Could you please take a look at these issues?

alfetopito

Hey there, thanks a lot for your contribution!

There are a few issues we have noticed, could you please take a look at them?

One question, I see the SDK is bringing a lot of dependencies.
What's the bundle size it's adding?

libs/wallet/src/web3-react/connection/metaMaskSdk.tsx

apps/cowswap-frontend/src/modules/wallet/containers/ConnectWalletOptions.tsx

baptiste-marchand · 2024-11-05T12:49:56Z

I have read the CLA Document and I hereby sign the CLA

baptiste-marchand · 2024-11-05T13:04:12Z

Hi @elena-zh and @alfetopito,

Thank you for your feedback! I'm taking over Edouard's work.

I took care of fixing all the identified issues. The 4th one never happened to me, so please let me know if you're still facing it with this version.

baptiste · 2024-11-05T19:18:40Z

I will not sign the CLA

alfetopito

It's working great!

Can I just ask that the original author (@EdouardBougon) sign the CLA too?

EdouardBougon · 2024-11-06T09:38:50Z

I have read the CLA Document and I hereby sign the CLA

elena-zh

Great job!
Thank you for addressing all these!

There is 1 nitpick related to networks change using WC, but this is happening on Prod now. so is not related to the current implementation.

anxolin

I think the app was already detecting Metmask as per https://eips.ethereum.org/EIPS/eip-6963

My concern with the PR is that its not working great when you use other injected wallet that is not Metamask.

Also Metamask icon shows first instead of the ones of the available wallets.

Look what we show in production:

I am a Rabby wallet user (sorry, I switched some time ago), and this version show the Metamask icon first. We should give more priority to the wallets the user has. If they have Metamask, it will show up.

There's a second issue, if I click in "Metamask" it doesn't do anything, it remains connecting forever (not realising I don't have it installed)

Another issue has been raised by Leandro above. He is concerned with the bundle size. I think you lazyload, but still would be good to have the dependencies in check, plus having more dependencies adds vulnerabilities (see what happened with Ledger library some months ago). If they are not necessary and we can use EIP standards instead, I would prefer to do so. Ultimately every wallet wants to push for their SDK into the Dapps, but this is not sustainable

elena-zh · 2024-11-08T12:14:48Z

Hey @baptiste-marchand , thank you for the fix!
One nitpick related to the uninstalled case: when I close 'connect' modal that appears when MM is not installed, connection modals remains in the 'connecting' state. It would be nice to show a connection error instead (like it works for WC option:

baptiste-marchand · 2024-11-12T16:16:40Z

There's a second issue, if I click in "Metamask" it doesn't do anything, it remains connecting forever (not realising I don't have it installed)

@anxolin I managed to reproduce it only by disabling the MetaMask extension after having clicked on connect wallet on Cowswap. Is that also the situation you found yourself in, or it was in a more normal situation?

baptiste-marchand · 2024-11-12T16:18:15Z

About your questions related to the MetaMask SDK, a member from the team should reach out to you on telegram

baptiste-marchand · 2024-11-12T17:16:33Z

One nitpick related to the uninstalled case: when I close 'connect' modal that appears when MM is not installed, connection modals remains in the 'connecting' state. It would be nice to show a connection error instead (like it works for WC option:

@elena-zh I'm not sure I can fix this one. This is related to how web3-react and MetaMask SDK are integrated together. From what I understood WC is throwing an error when the modal is closed, while MetaMask doesn't.

baptiste-marchand · 2024-11-14T12:57:22Z

Hi @anxolin! I wanted to give you a quick update: our team is actively working on reducing the MetaMask SDK bundle size and removing some dependencies. We will provide you with further updates next week.

…to metamask-sdk # Conflicts: # yarn.lock

socket-security · 2024-12-16T11:22:53Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher

🚮 Removed packages: npm/@babel/[email protected], npm/@babel/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]

View full report↗︎

elena-zh · 2024-12-17T10:57:54Z

Hey @EdouardBougon , @baptiste-marchand ,
we were about to include this PR into the release, but, unfortunately, we had to revert these changes in #5125.
Main problems:

Impossible to sign orders in a mobile device (using WC connector): the app hangs in this state (see the image) and there is no request in MM
MM is not installed on a desktop version, I connected to it using a native connector: impossible to sign transactions using it
MM is not installed on a desktop version, I connected to it using regular WC option> each time I pick a token, and try signing an order, this modal pops up

Please feel free to open a new PR with the fixes.
Thank you

vercel bot deployed to Preview – swap-dev October 24, 2024 12:26 View deployment

alfetopito reviewed Oct 25, 2024

View reviewed changes

libs/wallet/src/web3-react/connection/metaMaskSdk.tsx Outdated Show resolved Hide resolved

apps/cowswap-frontend/src/modules/wallet/containers/ConnectWalletOptions.tsx Show resolved Hide resolved

github-actions bot added a commit that referenced this pull request Nov 5, 2024

@baptiste-marchand has signed the CLA from Pull Request #5028

1a7766b

baptiste-marchand force-pushed the metamask-sdk branch 3 times, most recently from e8f2a49 to 22ae7f3 Compare November 5, 2024 13:24

vercel bot deployed to Preview – swap-dev November 5, 2024 13:42 View deployment

alfetopito approved these changes Nov 6, 2024

View reviewed changes

github-actions bot added a commit that referenced this pull request Nov 6, 2024

@EdouardBougon has signed the CLA from Pull Request #5028

924349f

elena-zh approved these changes Nov 6, 2024

View reviewed changes

anxolin reviewed Nov 7, 2024

View reviewed changes

vercel bot deployed to Preview – swap-dev November 8, 2024 11:44 View deployment

baptiste-marchand force-pushed the metamask-sdk branch from 4218bdf to ca769a8 Compare November 27, 2024 15:57

vercel bot deployed to Preview – swap-dev December 5, 2024 08:18 View deployment

EdouardBougon and others added 2 commits December 11, 2024 17:35

feat: add metaMask SDK connector

6b4880c

fix: display wallet connectors once

cad1117

baptiste-marchand and others added 4 commits December 11, 2024 17:35

fix: load balances and switch network

c82365a

fix: display installed wallets first when MetaMask isn't installed

6bcb0a6

fix: babel syntax error

b6a8229

feat: add metaMask SDK connector 0.31.2

e7d2eda

EdouardBougon force-pushed the metamask-sdk branch from ca769a8 to e7d2eda Compare December 12, 2024 11:11

vercel bot had a problem deploying to Preview – sdk-tools December 12, 2024 11:24 Failure

vercel bot deployed to Preview – cowfi December 12, 2024 11:29 View deployment

vercel bot deployed to Preview – swap-dev December 12, 2024 11:29 View deployment

shoom3301 added 2 commits December 16, 2024 16:18

Merge branch 'develop' of https://github.com/EdouardBougon/cowswap in…

79fd27c

…to metamask-sdk # Conflicts: # yarn.lock

chore: update yarnlock

cd08d73

vercel bot deployed to Preview – sdk-tools December 16, 2024 11:27 View deployment

chore: fix wallets ordering

284988c

vercel bot deployed to Preview – sdk-tools December 16, 2024 11:32 View deployment

chore: remove excessive condition

8cd824f

vercel bot deployed to Preview – sdk-tools December 16, 2024 11:34 View deployment

shoom3301 approved these changes Dec 16, 2024

View reviewed changes

vercel bot deployed to Preview – swap-dev December 16, 2024 11:45 View deployment

shoom3301 merged commit fe8f376 into cowprotocol:develop Dec 16, 2024
5 of 9 checks passed

github-actions bot locked and limited conversation to collaborators Dec 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add metaMask SDK connector #5028

feat: add metaMask SDK connector #5028

EdouardBougon commented Oct 23, 2024

vercel bot commented Oct 23, 2024

github-actions bot commented Oct 23, 2024 •

edited

Loading

socket-security bot commented Oct 23, 2024 •

edited

Loading

vercel bot commented Oct 24, 2024 •

edited

Loading

elena-zh commented Oct 24, 2024

alfetopito left a comment

baptiste-marchand commented Nov 5, 2024

baptiste-marchand commented Nov 5, 2024

baptiste commented Nov 5, 2024

alfetopito left a comment

EdouardBougon commented Nov 6, 2024

elena-zh left a comment

anxolin left a comment

elena-zh commented Nov 8, 2024

baptiste-marchand commented Nov 12, 2024 •

edited

Loading

baptiste-marchand commented Nov 12, 2024

baptiste-marchand commented Nov 12, 2024

baptiste-marchand commented Nov 14, 2024

socket-security bot commented Dec 16, 2024 •

edited

Loading

elena-zh commented Dec 17, 2024 •

edited

Loading

feat: add metaMask SDK connector #5028

feat: add metaMask SDK connector #5028

Conversation

EdouardBougon commented Oct 23, 2024

MetaMask SDK integration

vercel bot commented Oct 23, 2024

github-actions bot commented Oct 23, 2024 • edited Loading

socket-security bot commented Oct 23, 2024 • edited Loading

vercel bot commented Oct 24, 2024 • edited Loading

elena-zh commented Oct 24, 2024

alfetopito left a comment

Choose a reason for hiding this comment

baptiste-marchand commented Nov 5, 2024

baptiste-marchand commented Nov 5, 2024

baptiste commented Nov 5, 2024

alfetopito left a comment

Choose a reason for hiding this comment

EdouardBougon commented Nov 6, 2024

elena-zh left a comment

Choose a reason for hiding this comment

anxolin left a comment

Choose a reason for hiding this comment

elena-zh commented Nov 8, 2024

baptiste-marchand commented Nov 12, 2024 • edited Loading

baptiste-marchand commented Nov 12, 2024

baptiste-marchand commented Nov 12, 2024

baptiste-marchand commented Nov 14, 2024

socket-security bot commented Dec 16, 2024 • edited Loading

elena-zh commented Dec 17, 2024 • edited Loading

github-actions bot commented Oct 23, 2024 •

edited

Loading

socket-security bot commented Oct 23, 2024 •

edited

Loading

vercel bot commented Oct 24, 2024 •

edited

Loading

baptiste-marchand commented Nov 12, 2024 •

edited

Loading

socket-security bot commented Dec 16, 2024 •

edited

Loading

elena-zh commented Dec 17, 2024 •

edited

Loading