Merge pull request #3574 from WalletConnect/feat/pairing-idempotency

Feat/pairing idempotency

packages/core/src/constants/pairing.ts

@@ -51,3 +51,10 @@ export const PAIRING_RPC_OPTS: Record<
     },
   },
 };
+
+export const PAIRING_EVENTS = {
+  create: "pairing_create",
+  expire: "pairing_expire",
+  delete: "pairing_delete",
+  ping: "pairing_ping",
+};

packages/core/src/controllers/pairing.ts

@@ -44,6 +44,7 @@ import {
   PAIRING_RPC_OPTS,
   RELAYER_EVENTS,
   EXPIRER_EVENTS,
+  PAIRING_EVENTS,
 } from "../constants";
 import { Store } from "../controllers/store";
 
@@ -110,26 +111,32 @@ export class Pairing implements IPairing {
     this.isInitialized();
     this.isValidPair(params);
     const { topic, symKey, relay } = parseUri(params.uri);
-
+    let existingPairing;
     if (this.pairings.keys.includes(topic)) {
-      throw new Error(`Pairing already exists: ${topic}`);
+      existingPairing = this.pairings.get(topic);
+      if (existingPairing.active) {
+        throw new Error(
+          `Pairing already exists: ${topic}. Please try again with a new connection URI.`,
+        );
+      }
     }
 
-    if (this.core.crypto.hasKeys(topic)) {
-      throw new Error(`Keychain already exists: ${topic}`);
+    // avoid overwriting keychain pairing already exists
+    if (!this.core.crypto.keychain.has(topic)) {
+      await this.core.crypto.setSymKey(symKey, topic);
+      await this.core.relayer.subscribe(topic, { relay });
     }
 
     const expiry = calcExpiry(FIVE_MINUTES);
     const pairing = { topic, relay, expiry, active: false };
     await this.pairings.set(topic, pairing);
-    await this.core.crypto.setSymKey(symKey, topic);
-    await this.core.relayer.subscribe(topic, { relay });
     this.core.expirer.set(topic, expiry);
 
     if (params.activatePairing) {
       await this.activate({ topic });
     }
 
+    this.events.emit(PAIRING_EVENTS.create, pairing);
     return pairing;
   };
 
@@ -298,7 +305,7 @@ export class Pairing implements IPairing {
     try {
       this.isValidPing({ topic });
       await this.sendResult<"wc_pairingPing">(id, topic, true);
-      this.events.emit("pairing_ping", { id, topic });
+      this.events.emit(PAIRING_EVENTS.ping, { id, topic });
     } catch (err: any) {
       await this.sendError(id, topic, err);
       this.logger.error(err);
@@ -326,7 +333,7 @@ export class Pairing implements IPairing {
     try {
       this.isValidDisconnect({ topic });
       await this.deletePairing(topic);
-      this.events.emit("pairing_delete", { id, topic });
+      this.events.emit(PAIRING_EVENTS.delete, { id, topic });
     } catch (err: any) {
       await this.sendError(id, topic, err);
       this.logger.error(err);
@@ -362,12 +369,10 @@ export class Pairing implements IPairing {
   private registerExpirerEvents() {
     this.core.expirer.on(EXPIRER_EVENTS.expired, async (event: ExpirerTypes.Expiration) => {
       const { topic } = parseExpirerTarget(event.target);
-      if (topic) {
-        if (this.pairings.keys.includes(topic)) {
-          await this.deletePairing(topic, true);
-          this.events.emit("pairing_expire", { topic });
-        }
-      }
+      if (!topic) return;
+      if (!this.pairings.keys.includes(topic)) return;
+      await this.deletePairing(topic, true);
+      this.events.emit(PAIRING_EVENTS.expire, { topic });
     });
   }
 

packages/core/test/pairing.spec.ts

@@ -64,19 +64,20 @@ describe("Pairing", () => {
 
     it("throws when pairing is attempted on topic that already exists", async () => {
       const { topic, uri } = await coreA.pairing.create();
+      coreA.pairing.pairings.get(topic).active = true;
       await expect(coreA.pairing.pair({ uri })).rejects.toThrowError(
         `Pairing already exists: ${topic}`,
       );
     });
 
-    it("throws when keychain already exists", async () => {
-      const maliciousTopic = generateRandomBytes32();
+    it("should not override existing keychain values", async () => {
+      const keychainTopic = generateRandomBytes32();
+      const keychainValue = generateRandomBytes32();
       let { topic, uri } = await coreA.pairing.create();
-      coreA.crypto.keychain.set(maliciousTopic, maliciousTopic);
-      uri = uri.replace(topic, maliciousTopic);
-      await expect(coreA.pairing.pair({ uri })).rejects.toThrowError(
-        `Keychain already exists: ${maliciousTopic}`,
-      );
+      coreA.crypto.keychain.set(keychainTopic, keychainValue);
+      uri = uri.replace(topic, keychainTopic);
+      await coreA.pairing.pair({ uri });
+      expect(coreA.crypto.keychain.get(keychainTopic)).toBe(keychainValue);
     });
   });
 

packages/sign-client/src/controllers/engine.ts

@@ -1,5 +1,6 @@
 import {
   EXPIRER_EVENTS,
+  PAIRING_EVENTS,
   RELAYER_DEFAULT_PROTOCOL,
   RELAYER_EVENTS,
   VERIFY_SERVER,
@@ -30,6 +31,7 @@ import {
   ProposalTypes,
   RelayerTypes,
   SessionTypes,
+  PairingTypes,
 } from "@walletconnect/types";
 import {
   calcExpiry,
@@ -114,6 +116,7 @@ export class Engine extends IEngine {
       await this.cleanup();
       this.registerRelayerEvents();
       this.registerExpirerEvents();
+      this.registerPairingEvents();
       this.client.core.pairing.register({ methods: Object.keys(ENGINE_RPC_OPTS) });
       this.initialized = true;
 
@@ -1077,6 +1080,38 @@ export class Engine extends IEngine {
     });
   }
 
+  // ---------- Pairing Events ---------------------------------------- //
+  private registerPairingEvents() {
+    this.client.core.pairing.events.on(PAIRING_EVENTS.create, (pairing: PairingTypes.Struct) =>
+      this.onPairingCreated(pairing),
+    );
+  }
+
+  /**
+   * when a pairing is created, we check if there is a pending proposal for it.
+   * if there is, we send it to onSessionProposeRequest to be processed as if it was received from the relay.
+   * It allows QR/URI to be scanned multiple times without having to create new pairing.
+   */
+  private onPairingCreated = (pairing: PairingTypes.Struct) => {
+    if (pairing.active) return;
+    const proposals = this.client.proposal.getAll();
+    const proposal = proposals.find((p) => p.pairingTopic === pairing.topic);
+    if (!proposal) return;
+    this.onSessionProposeRequest(
+      pairing.topic,
+      formatJsonRpcRequest(
+        "wc_sessionPropose",
+        {
+          requiredNamespaces: proposal.requiredNamespaces,
+          optionalNamespaces: proposal.optionalNamespaces,
+          relays: proposal.relays,
+          proposer: proposal.proposer,
+        },
+        proposal.id,
+      ),
+    );
+  };
+
   // ---------- Validation Helpers ------------------------------------ //
   private isValidPairingTopic(topic: any) {
     if (!isValidString(topic, false)) {

packages/sign-client/test/sdk/client.spec.ts

@@ -5,7 +5,7 @@ import {
   JsonRpcError,
 } from "@walletconnect/jsonrpc-utils";
 import { RelayerTypes } from "@walletconnect/types";
-import { getSdkError } from "@walletconnect/utils";
+import { getSdkError, parseUri } from "@walletconnect/utils";
 import { expect, describe, it, vi, beforeEach, afterEach } from "vitest";
 import SignClient, { WALLETCONNECT_DEEPLINK_CHOICE } from "../../src";
 
@@ -23,6 +23,7 @@ import {
   TEST_REQUIRED_NAMESPACES_V2,
   TEST_NAMESPACES_V2,
   initTwoPairedClients,
+  TEST_CONNECT_PARAMS,
 } from "../shared";
 
 describe("Sign Client Integration", () => {
@@ -118,6 +119,55 @@ describe("Sign Client Integration", () => {
       expect(clients.A.pairing.getAll().length).to.eq(1);
       await deleteClients(clients);
     });
+    it("should emit session_proposal on every pair attempt with same URI as long as the proposal has not yet been approved or rejected", async () => {
+      const dapp = await SignClient.init({ ...TEST_SIGN_CLIENT_OPTIONS, name: "dapp" });
+      const wallet = await SignClient.init({ ...TEST_SIGN_CLIENT_OPTIONS, name: "wallet" });
+      const { uri, approval } = await dapp.connect(TEST_CONNECT_PARAMS);
+      if (!uri) throw new Error("URI is undefined");
+      expect(uri).to.exist;
+      const parsedUri = parseUri(uri);
+
+      // 1. attempt to pair
+      // 2. receive the session_proposal event
+      // 3. avoid approving or rejecting the proposal - simulates accidental closing of the app/modal etc
+      await Promise.all([
+        new Promise<void>((resolve) => {
+          wallet.once("session_proposal", (params) => {
+            expect(params).to.exist;
+            expect(params.params.pairingTopic).to.eq(parsedUri.topic);
+            resolve();
+          });
+        }),
+        wallet.pair({ uri }),
+      ]);
+      // 4. attempt to pair again with the same URI
+      // 5. receive the session_proposal event again
+      // 6. approve the proposal
+      await Promise.all([
+        new Promise<void>((resolve) => {
+          wallet.once("session_proposal", async (params) => {
+            expect(params).to.exist;
+            expect(params.params.pairingTopic).to.eq(parsedUri.topic);
+            await wallet.approve({ id: params.id, namespaces: TEST_NAMESPACES });
+            resolve();
+          });
+        }),
+        new Promise<void>(async (resolve) => {
+          const session = await approval();
+          expect(session).to.exist;
+          expect(session.topic).to.exist;
+          expect(session.pairingTopic).to.eq(parsedUri.topic);
+          resolve();
+        }),
+        wallet.pair({ uri }),
+      ]);
+
+      // 7. attempt to pair again with the same URI
+      // 8. should receive an error the pairing already exists
+      await expect(wallet.pair({ uri })).rejects.toThrowError();
+
+      await deleteClients({ A: dapp, B: wallet });
+    });
   });
 
   describe("disconnect", () => {