Add support for WalletConnect v2 #13

hlopes-ledger · 2023-03-01T15:34:54Z

Changes

add workspaces to root package.json so we can test demo app with local loader package without manual linking
moved checkSupport parameter types and their management to a supportOptions module, added parameters for WalletConnect v2, sets defaults for chain parameters, getSupportOptions is called by other modules
renamed ExtensionUnavailableModal to ExtensionInstallModal
renamed ConnectWithLedgerLiveModal to UseLedgerLiveModal
renamed the Ethereum provider to ExtensionEvm since it is used for Polygon and future EVM chains, moved chain compatibility checks there, renamed related constants and types
renamed Solana to ExtensionSolana since this is through the extension, and we might support Solana through WalletConnect as well in the future
renamed the original WalletConnect module to WalletConnectLegacy which now imports the dist file from @walletconnect/legacy-provider, updated the module declaration in src/@types/index.d.ts
created a new WalletConnectEvm module for the v2 provider which imports the dist file from @walletconnect/ethereum-provider v2, added a new module declaration in src/@types/index.d.ts
removed the TryConnectEthereum module - when the extension is supported but not installed we now default to the WalletConnect provider instead of the extension one, this will make it possible to use WC in that case if the user needs it
the modal module now has a showExtensionOrLLModal function with the logic to show the ExtensionInstallModal if both the user's platform and the required chains are supported, or the UseLedgerLiveModal one
the eth_requestAccounts request on both WalletConnect provider modules now call showExtensionOrLLModal
getProvider now returns the WalletConnectLegacy provider if the version parameter is 1 or the WalletConnectEvm one otherwise
store result of checkSupport in a module variable and make it available through getSupportResult, is used by showExtensionOrLLModal to decide which modal to show
made some changes to the rollup config to be able to bundle @walletconnect/ethereum-provider v2

WC v1 testing information

demo app, https://63f771085fca280e96307935--monumental-dolphin-d87e18.netlify.app
it initializes Connect Kit with chainId = 1
you should see [WalletConnectLegacy] getWalletConnectLegacyProvider on the console
on Safari iOS/macOS it should show the "Try Ledger Connect" modal
if Extension is enabled it should pop up
on other browsers it should show the "Do you have Ledger Live?" modal
switching chains on the wallet should be reflected on the app

WC v2 testing information

demo app, https://63ff7d8f84b8a9320aadac90--monumental-dolphin-d87e18.netlify.app/
it initializes Connect Kit with version = 2, projectId, chains = [1, 137], optionalChains = [5]
you should see [WalletConnectEvm] getWalletConnectProvider on the console
same as above for browser behaviour
switching chains must be done programatically on the app, the buttons should update app state

netlify · 2023-03-01T15:34:58Z

❌ Deploy Preview for legder-connect-kit failed.

Name	Link
🔨 Latest commit	`fbb2a81`
🔍 Latest deploy log	https://app.netlify.com/sites/legder-connect-kit/deploys/64944f5758aa1500074a0fc6

nenadV91 · 2023-03-06T12:29:07Z

Hello, just want to give my feedback.

When I click on "Open ledger live Desktop" button,

in the ledger-live I see this

I'm using
Macbook, os Ventura 13.1
Browser: Chrome

hlopes-ledger · 2023-03-06T14:00:09Z

Hi @nenadV91,

I forgot to mention that to test it with Ledger Live you need the new version of the WalletConnect Live App (not released yet). To confirm that v2 works without depending on LL you can scan the QR code (or copy and paste it from the browser console) with a third party mobile wallet that supports v2, like Trust or Zerion. I will make the encoded URI the ledgerlive:// deep link when the Live App is released.

Instructions for installing the new Live App are on the Ledger Slack (accessible to Ledger employees only): https://ledger.slack.com/archives/C04JHL7AQBF/p1677492460127719. It might be enough to check that the connection works but it is outdated and has some bugs that have already been fixed. I've been running the latest version directly from the repo.

Justkant

Some small stuff, if you want more infos or discuss some of my comments we can do that on a huddle too

packages/connect-kit/package.json

Justkant · 2023-03-15T09:47:17Z

packages/connect-kit/src/@types/index.d.ts

+declare module '@walletconnect/legacy-provider/dist/umd/index.min.js' {
+  import WalletConnectProvider from '@walletconnect/legacy-provider/dist/umd/index.min.js';
+  export default WalletConnectProvider
+};


Why do you need this file and types declarations ?

The WalletConnect packages are not browser safe, there is an open issue since 2020 and I've commented on it: WalletConnect/walletconnect-monorepo#341

The solution is to consume the pre bundled package instead of trying to bundle it together with your own lib: WalletConnect/walletconnect-monorepo#341

If importing the @walletconnect/ethereum-provider directly you will have all sorts of problems with nodejs dependencies, even after trying to polyfill them.

Added a comment on the file mentioning this.

Making it a peer dependency instead of bundling it might avoid this altogether.

Justkant · 2023-03-15T09:53:13Z

packages/connect-kit/src/components/UseLedgerLiveModal/UseLedgerLiveModal.tsx

+// placeholder functions until the component is initialized
+let setModalUri = (uri: string) => {};
+
+// called by the WalletConnect display_uri event handler to set a new URI
+export let setWalletConnectUri = (uri: string): void => {
+  log('setModalUri', uri);
+  setModalUri(uri);
+}


You might want to use a provider instead for managing a shared/global state

Justkant · 2023-03-15T09:59:18Z

packages/connect-kit/src/components/UseLedgerLiveModal/UseLedgerLiveModal.tsx

+  useEffect(() => {
+    previousUriRef.current = uri;
+  });


I think it's better to only update when needed

Suggested change

useEffect(() => {

previousUriRef.current = uri;

});

useEffect(() => {

previousUriRef.current = uri;

}, [uri]);

Justkant · 2023-03-15T09:59:47Z

packages/connect-kit/src/components/UseLedgerLiveModal/UseLedgerLiveModal.tsx

+  if (uri !== previousUri && uri !== wcUri) {
+    setWcUri(uri);
+  }


I think we should move this inside the useEffect

packages/connect-kit/src/components/UseLedgerLiveModal/UseLedgerLiveModal.tsx

packages/connect-kit/src/providers/ExtensionEvm.ts

packages/connect-kit/src/providers/ExtensionSolana.ts

Justkant · 2023-03-15T13:40:29Z

packages/connect-kit/src/providers/WalletConnectEvm.ts

+      return new Promise(async (resolve, reject) => {
+        try {
+          if (!provider?.session?.connected) {
+            showExtensionOrLLModal('', reject),
+
+            // connect initializes the session and waits for connection
+            await provider.connect();
+          } else {
+            log('reusing existing session');
+          }
+
+          // call the original provider request
+          return resolve(await baseRequest({ method, params }));
+        } catch(err) {
+          logError('error', err);
+          return reject(err);
+        }
+      });


You might want to change the way this Promise is done as it looks like an anti-pattern with the async/await: https://stackoverflow.com/a/43050114

Also this talk (https://www.youtube.com/watch?v=XV-u_Ow47s0) is interesting

Fixed, I've replaced the async/await with .then

I understand the "anti-pattern" issue is due to the risk of not correctly handling the errors when using async within a promise constructor. The whole purpose of patching the provider in this way is to get the "user rejected request" error to the dApp UI when the modal is closed. So in this case reject is being passed as the onClose callback to the modal component and also in the catch block to hadle the error correctly.

Nice talk!

Justkant · 2023-03-15T13:48:24Z

packages/connect-kit/src/providers/WalletConnectLegacy.ts

+    if (method === 'eth_requestAccounts') {
+      log('calling patched', method, params);
+
+      return new Promise(async (resolve, reject) => {


Same comment as for WalletConnectEvm

- add workspaces to root `package.json` so we can test demo app with local loader package without manual linking - moved `checkSupport` parameter types and their management to a `supportOptions` module, added parameters for WalletConnect v2, sets defaults for chain parameters, `getSupportOptions` is called by other modules - renamed `ExtensionUnavailableModal` to `ExtensionInstallModal` - renamed `ConnectWithLedgerLiveModal` to `UseLedgerLiveModal` - renamed the `Ethereum` provider to `ExtensionEvm` since it is used for Polygon and future EVM chains, moved chain compatibility checks there, renamed related constants and types - renamed `Solana` to `ExtensionSolana` since this is through the extension, and we might support Solana through WalletConnect as well in the future - renamed the original `WalletConnect` module to `WalletConnectLegacy` which now imports the dist file from `@walletconnect/legacy-provider`, updated the module declaration in `src/@types/index.d.ts` - created a new `WalletConnectEvm` module for the v2 provider which imports the dist file from `@walletconnect/ethereum-provider` v2, added a new module declaration in `src/@types/index.d.ts` - removed the `TryConnectEthereum` module - when the extension is supported but not installed we now default to the `WalletConnect` provider instead of the extension one, this will make it possible to use WC in that case if the user needs it - the modal module now has a `showExtensionOrLLModal` function with the logic to show the `ExtensionInstallModal` if both the user's platform and the required chains are supported, or the `UseLedgerLiveModal` one - the `eth_requestAccounts` request on both WalletConnect provider modules now call `showExtensionOrLLModal` - `getProvider` now returns the `WalletConnectLegacy` provider if the version parameter is 1 or the `WalletConnectEvm` one otherwise - store result of `checkSupport` in a module variable and make it available through `getSupportResult`, is used by `showExtensionOrLLModal` to decide which modal to show - made some changes to the rollup config to be able to bundle @walletconnect/ethereum-provider v2

Fixes LIVE-7738

- add await to calls to baseRequest inside eth_requestAccounts - remove event handlers before adding again

- update package versions - rename version parameter to walletConnectVersion - update connect-kit-loader's tsconfig outDir

hlopes-ledger closed this Mar 1, 2023

hlopes-ledger reopened this Mar 1, 2023

hlopes-ledger force-pushed the feat/lc-513-wallet-connect-v2-launch branch 3 times, most recently from 89e3d61 to 92527ae Compare March 1, 2023 16:55

hlopes-ledger requested a review from a team March 14, 2023 09:22

Justkant requested changes Mar 15, 2023

View reviewed changes

hlopes-ledger force-pushed the feat/lc-513-wallet-connect-v2-launch branch 2 times, most recently from 389b08f to ccff5af Compare March 15, 2023 21:25

Justkant approved these changes Mar 24, 2023

View reviewed changes

hlopes-ledger force-pushed the main branch 2 times, most recently from 2cfabdb to a431778 Compare April 28, 2023 11:46

hlopes-ledger added 15 commits June 16, 2023 09:10

Code review fixes

766cdfb

Update README and CHANGELOGs

8113f60

Remove unwanted call to eth_chainId

6a22c60

Change showExtensionOrLLModal params to be an object

943ab8b

add WC's EthereumProvider class

a8c84f0

Add _forceWcV1 and _forceWcV2 parameters

c99494b

Replace _forceWcV* params with local storage entries

ab49c6d

Add list of optional methods and events if not set

2213543

Fixes LIVE-7738

Restructure provider code, update WalletConnect versions

72fe4a7

Update README for release

18b139a

fix versions and CDN URL

7b3b21d

chore: release Connect Kit v1.1.0-beta.1

4ed5ba7

chore: release Connect Kit Loader v1.1.0-beta.1

2c6d2bd

Fix bundling

54e2c1b

hlopes-ledger added 5 commits June 19, 2023 17:47

Only update LL URI if modal is open

549b64a

- add await to calls to baseRequest inside eth_requestAccounts - remove event handlers before adding again

Update package versions for publishing

d9f7861

Update WalletConnect EthereumProvider class

24ac6f9

Revert some event handling changes

18898c0

Update example apps

431dfbe

hlopes-ledger force-pushed the feat/lc-513-wallet-connect-v2-launch branch from 60ebfac to 431dfbe Compare June 20, 2023 16:45

Remove references to metadata as it's not supported in LL

90f0bec

hlopes-ledger force-pushed the feat/lc-513-wallet-connect-v2-launch branch 3 times, most recently from 2595543 to f57c449 Compare June 21, 2023 15:10

Prepare beta 3 release

ee9cbcc

- update package versions - rename version parameter to walletConnectVersion - update connect-kit-loader's tsconfig outDir

hlopes-ledger force-pushed the feat/lc-513-wallet-connect-v2-launch branch from f57c449 to ee9cbcc Compare June 21, 2023 15:15

hlopes-ledger added 4 commits June 21, 2023 15:17

chore: release Connect Kit v1.1.0-beta.3

9c8f20b

chore: release Connect Kit Loader v1.1.0-beta.2

3f85e3d

Add development steps to README

d855237

Prepare 1.1.0 release

fbb2a81

hlopes-ledger merged commit 909c1ad into main Jun 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for WalletConnect v2 #13

Add support for WalletConnect v2 #13

hlopes-ledger commented Mar 1, 2023 •

edited

Loading

netlify bot commented Mar 1, 2023 •

edited

Loading

nenadV91 commented Mar 6, 2023

hlopes-ledger commented Mar 6, 2023 •

edited

Loading

Justkant left a comment

Justkant Mar 15, 2023

hlopes-ledger Mar 15, 2023 •

edited

Loading

hlopes-ledger Mar 15, 2023

hlopes-ledger Mar 16, 2023

Justkant Mar 15, 2023

Justkant Mar 15, 2023

Justkant Mar 15, 2023

Justkant Mar 15, 2023

hlopes-ledger Mar 15, 2023 •

edited

Loading

Justkant Mar 15, 2023

Add support for WalletConnect v2 #13

Add support for WalletConnect v2 #13

Conversation

hlopes-ledger commented Mar 1, 2023 • edited Loading

netlify bot commented Mar 1, 2023 • edited Loading

❌ Deploy Preview for legder-connect-kit failed.

nenadV91 commented Mar 6, 2023

hlopes-ledger commented Mar 6, 2023 • edited Loading

Justkant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlopes-ledger Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlopes-ledger Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlopes-ledger commented Mar 1, 2023 •

edited

Loading

netlify bot commented Mar 1, 2023 •

edited

Loading

hlopes-ledger commented Mar 6, 2023 •

edited

Loading

hlopes-ledger Mar 15, 2023 •

edited

Loading

hlopes-ledger Mar 15, 2023 •

edited

Loading