feat: add metaMask SDK connector

[EdouardBougon]

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]

@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]

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

[socket-security]

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert	Package	Note	Source	CI
Protestware or potentially unwanted behavior	npm/[email protected]	Note: The script attempts to run a local post-install script, which could potentially contain malicious code. The error handling suggests that it is designed to fail silently, which is a common tactic in malicious scripts.	yarn.lock	⚠︎
Protestware or potentially unwanted behavior	npm/[email protected]	Note: This package prints a protestware console message on install regarding Ukraine for users with Russian language locale	yarn.lock	⚠︎
Critical CVE	npm/[email protected]	CVE: GHSA-h755-8qp9-cq85 protobufjs Prototype Pollution vulnerability (CRITICAL) Affected versions: >= 6.10.0, < 6.11.4 Patched version: 6.11.4	yarn.lock	⚠︎

View full report↗︎

Next steps

What is protestware?

This package is a joke, parody, or includes undocumented or hidden behavior unrelated to its primary function.

Consider that consuming this package may come along with functionality unrelated to its primary purpose.

What is a critical CVE?

Contains a Critical Common Vulnerability and Exposure (CVE).

Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

• @SocketSecurity ignore npm/[email protected]
• @SocketSecurity ignore npm/[email protected]

[vercel]

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

Name	Status	Preview	Updated (UTC)
cowfi	✅ Ready (Inspect)	Visit Preview	Dec 12, 2024 11:29am
sdk-tools	❌ Failed (Inspect)		Dec 12, 2024 11:29am
swap-dev	✅ Ready (Inspect)	Visit Preview	Dec 12, 2024 11:29am

[elena-zh]

Hey @EdouardBougon , thank you for your contribution!

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

1. 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
  

2. When another wallet extensions are enabled in a browser, they are duplicating in a connection modal
   
3. Mobile: in 3 cases out of 4, when I connected to MM, I saw 'could not load balances' error message --> balances were not loaded
   
4. 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)
5. (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?

[baptiste-marchand]

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

[baptiste-marchand]

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]

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]

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]

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]

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]

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

[baptiste-marchand]

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]

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.

[socket-security]

Report too large to display inline

View full report↗︎