fix: problematic session completion

packages/sign-client/src/constants/client.ts

@@ -26,6 +26,7 @@ export const SIGN_CLIENT_EVENTS: Record<SignClientTypes.Event, SignClientTypes.E
   proposal_expire: "proposal_expire",
   session_authenticate: "session_authenticate",
   session_request_expire: "session_request_expire",
+  session_connect: "session_connect",
 };
 
 export const SIGN_CLIENT_STORAGE_OPTIONS = {

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

@@ -147,6 +147,15 @@
 
   private recentlyDeletedLimit = 200;
   private relayMessageCache: RelayerTypes.MessageEvent[] = [];
+  private pendingSessions: Map<
+    number,
+    {
+      sessionTopic: string;
+      pairingTopic: string;
+      proposalId: number;
+      publicKey: string;
+    }
+  > = new Map();
 
   constructor(client: IEngine["client"]) {
     super(client);
@@ -222,45 +231,45 @@
       expiryTimestamp,
       pairingTopic: topic,
       ...(sessionProperties && { sessionProperties }),
+      id: payloadId(),
     };
+    const sessionConnectTarget = engineEvent("session_connect", proposal.id);
+
     const {
       reject,
       resolve,
       done: approval,
     } = createDelayedPromise<SessionTypes.Struct>(expiry, PROPOSAL_EXPIRY_MESSAGE);
-    this.events.once<"session_connect">(
-      engineEvent("session_connect"),
-      async ({ error, session }) => {
-        if (error) reject(error);
-        else if (session) {
-          session.self.publicKey = publicKey;
-          const completeSession = {
-            ...session,
-            pairingTopic: proposal.pairingTopic,
-            requiredNamespaces: proposal.requiredNamespaces,
-            optionalNamespaces: proposal.optionalNamespaces,
-            transportType: TRANSPORT_TYPES.relay,
-          };
-          await this.client.session.set(session.topic, completeSession);
-          await this.setExpiry(session.topic, session.expiry);
-          if (topic) {
-            await this.client.core.pairing.updateMetadata({
-              topic,
-              metadata: session.peer.metadata,
-            });
-          }
-          this.cleanupDuplicatePairings(completeSession);
-          resolve(completeSession);
-        }
-      },
-    );
-    const id = await this.sendRequest({
+
+    const proposalExpireHandler = ({ id }: { id: number }) => {
+      if (id === proposal.id) {
+        this.client.events.off("proposal_expire", proposalExpireHandler);
+        this.pendingSessions.delete(proposal.id);
+        // emit the event to trigger reject, this approach automatically cleans up the .once listener below
+        this.events.emit(sessionConnectTarget, {
+          error: { message: PROPOSAL_EXPIRY_MESSAGE, code: 0 },
+        });
+      }
+    };
+
+    this.client.events.on("proposal_expire", proposalExpireHandler);
+    this.events.once<"session_connect">(sessionConnectTarget, ({ error, session }) => {
+      this.client.events.off("proposal_expire", proposalExpireHandler);
+      if (error) reject(error);
+      else if (session) {
+        resolve(session);
+      }
+    });
+
+    await this.sendRequest({
       topic,
       method: "wc_sessionPropose",
       params: proposal,
       throwOnFailedPublish: true,
+      clientRpcId: proposal.id,
     });
-    await this.setProposal(id, { id, ...proposal });
+
+    await this.setProposal(proposal.id, proposal);
     return { uri, approval };
   };
 
@@ -863,51 +872,45 @@
         metadata: this.client.metadata,
       },
       expiryTimestamp: calcExpiry(ENGINE_RPC_OPTS.wc_sessionPropose.req.ttl),
+      id: payloadId(),
     };
 
     const { done, resolve, reject } = createDelayedPromise(authRequestExpiry, "Request expired");
 
+    const authenticateId = payloadId();
+    const sessionConnectEventTarget = engineEvent("session_connect", proposal.id);
+    const authenticateEventTarget = engineEvent("session_request", authenticateId);
+
     // handle fallback session proposal response
     const onSessionConnect = async ({ error, session }: any) => {
       // cleanup listener for authenticate response
-      this.events.off(engineEvent("session_request", id), onAuthenticate);
+      this.events.off(authenticateEventTarget, onAuthenticate);
       if (error) reject(error);
       else if (session) {
-        session.self.publicKey = publicKey;
-        await this.client.session.set(session.topic, session);
-        await this.setExpiry(session.topic, session.expiry);
-        if (pairingTopic) {
-          await this.client.core.pairing.updateMetadata({
-            topic: pairingTopic,
-            metadata: session.peer.metadata,
-          });
-        }
-        const sessionObject = this.client.session.get(session.topic);
-        await this.deleteProposal(fallbackId);
         resolve({
-          session: sessionObject,
+          session,
         });
       }
     };
     // handle session authenticate response
     const onAuthenticate = async (payload: any) => {
       // delete this auth request on response
       // we're using payload from the wallet to establish the session so we don't need to keep this around
-      await this.deletePendingAuthRequest(id, { message: "fulfilled", code: 0 });
+      await this.deletePendingAuthRequest(authenticateId, { message: "fulfilled", code: 0 });
       if (payload.error) {
         // wallets that do not support wc_sessionAuthenticate will return an error
         // we should not reject the promise in this case as the fallback session proposal will be used
         const error = getSdkError("WC_METHOD_UNSUPPORTED", "wc_sessionAuthenticate");
         if (payload.error.code === error.code) return;
 
         // cleanup listener for fallback response
-        this.events.off(engineEvent("session_connect"), onSessionConnect);
+        this.events.off(sessionConnectEventTarget, onSessionConnect);
         return reject(payload.error.message);
       }
       // delete fallback proposal on successful authenticate as the proposal will not be responded to
-      await this.deleteProposal(fallbackId);
+      await this.deleteProposal(proposal.id);
       // cleanup listener for fallback response
-      this.events.off(engineEvent("session_connect"), onSessionConnect);
+      this.events.off(sessionConnectEventTarget, onSessionConnect);
 
       const {
         cacaos,
@@ -1005,17 +1008,14 @@
       });
     };
 
-    // set the ids for both requests
-    const id = payloadId();
-    const fallbackId = payloadId();
     // subscribe to response events
-    this.events.once<"session_connect">(engineEvent("session_connect"), onSessionConnect);
-    this.events.once(engineEvent("session_request", id), onAuthenticate);
+    this.events.once<"session_connect">(sessionConnectEventTarget, onSessionConnect);
+    this.events.once(authenticateEventTarget, onAuthenticate);
 
     let linkModeURL;
     try {
       if (isLinkMode) {
-        const payload = formatJsonRpcRequest("wc_sessionAuthenticate", request, id);
+        const payload = formatJsonRpcRequest("wc_sessionAuthenticate", request, authenticateId);
         this.client.core.history.set(pairingTopic, payload);
         const message = await this.client.core.crypto.encode("", payload, {
           type: TYPE_2,
@@ -1031,27 +1031,27 @@
             params: request,
             expiry: params.expiry,
             throwOnFailedPublish: true,
-            clientRpcId: id,
+            clientRpcId: authenticateId,
           }),
           this.sendRequest({
             topic: pairingTopic,
             method: "wc_sessionPropose",
             params: proposal,
             expiry: ENGINE_RPC_OPTS.wc_sessionPropose.req.ttl,
             throwOnFailedPublish: true,
-            clientRpcId: fallbackId,
+            clientRpcId: proposal.id,
           }),
         ]);
       }
     } catch (error) {
       // cleanup listeners on failed publish
-      this.events.off(engineEvent("session_connect"), onSessionConnect);
-      this.events.off(engineEvent("session_request", id), onAuthenticate);
+      this.events.off(sessionConnectEventTarget, onSessionConnect);
+      this.events.off(authenticateEventTarget, onAuthenticate);
       throw error;
     }
 
-    await this.setProposal(fallbackId, { id: fallbackId, ...proposal });
-    await this.setAuthRequest(id, {
+    await this.setProposal(proposal.id, proposal);
+    await this.setAuthRequest(authenticateId, {
       request: {
         ...request,
         verifyContext: {} as any,
@@ -1884,11 +1884,13 @@
         selfPublicKey,
         peerPublicKey,
       );
-      this.client.logger.trace({
-        type: "method",
-        method: "onSessionProposeResponse",
+      this.pendingSessions.set(id, {
         sessionTopic,
+        pairingTopic: topic,
+        proposalId: id,
+        publicKey: selfPublicKey,
       });
+
       const subscriptionId = await this.client.core.relayer.subscribe(sessionTopic, {
         transportType,
       });
@@ -1900,12 +1902,12 @@
       await this.client.core.pairing.activate({ topic });
     } else if (isJsonRpcError(payload)) {
       await this.client.proposal.delete(id, getSdkError("USER_DISCONNECTED"));
-      const target = engineEvent("session_connect");
+      const target = engineEvent("session_connect", id);
       const listeners = this.events.listenerCount(target);
       if (listeners === 0) {
         throw new Error(`emitting ${target} without any listeners, 954`);
       }
-      this.events.emit(engineEvent("session_connect"), { error: payload.error });
+      this.events.emit(target, { error: payload.error });
     }
   };
 
@@ -1918,18 +1920,28 @@
       this.isValidSessionSettleRequest(params);
       const { relay, controller, expiry, namespaces, sessionProperties, sessionConfig } =
         payload.params;
-      const session = {
+      const pendingSession = [...this.pendingSessions.values()].find(
+        (s) => s.sessionTopic === topic,
+      );
+
+      if (!pendingSession) {
+        return this.client.logger.error(`Pending session not found for topic ${topic}`);
+      }
+
+      const proposal = this.client.proposal.get(pendingSession.proposalId);
+
+      const session: SessionTypes.Struct = {
         topic,
         relay,
         expiry,
         namespaces,
         acknowledged: true,
-        pairingTopic: "", // pairingTopic will be set in the `session_connect` handler
-        requiredNamespaces: {},
-        optionalNamespaces: {},
+        pairingTopic: pendingSession.pairingTopic,
+        requiredNamespaces: proposal.requiredNamespaces,
+        optionalNamespaces: proposal.optionalNamespaces,
         controller: controller.publicKey,
         self: {
-          publicKey: "",
+          publicKey: pendingSession.publicKey,
           metadata: this.client.metadata,
         },
         peer: {
@@ -1940,12 +1952,22 @@
         ...(sessionConfig && { sessionConfig }),
         transportType: TRANSPORT_TYPES.relay,
       };
-      const target = engineEvent("session_connect");
-      const listeners = this.events.listenerCount(target);
-      if (listeners === 0) {
-        throw new Error(`emitting ${target} without any listeners 997`);
-      }
-      this.events.emit(engineEvent("session_connect"), { session });
+
+      await this.client.session.set(session.topic, session);
+      await this.setExpiry(session.topic, session.expiry);
+
+      await this.client.core.pairing.updateMetadata({
+        topic: pendingSession.pairingTopic,
+        metadata: session.peer.metadata,
+      });
+
+      this.client.events.emit("session_connect", { session });
+      this.events.emit(engineEvent("session_connect", pendingSession.proposalId), { session });
+
+      this.pendingSessions.delete(pendingSession.proposalId);
+      this.deleteProposal(pendingSession.proposalId, false);
+      this.cleanupDuplicatePairings(session);
+
       await this.sendResult<"wc_sessionSettle">({
         id: payload.id,
         topic,
@@ -2116,7 +2138,7 @@
     }, 500);
   };
 
   private onSessionDeleteRequest: EnginePrivate["onSessionDeleteRequest"] = async (
     topic,
     payload,
   ) => {

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

@@ -94,6 +94,53 @@ describe("Sign Client Integration", () => {
       expect(clients.B.metadata.redirect?.universal).to.exist;
       await deleteClients(clients);
     });
+    it("should connect with out of order URIs", async () => {
+      const clients = await initTwoClients();
+      // load three proposals
+      const { uri: uriOne, approval: approvalOne } = await clients.A.connect(TEST_CONNECT_PARAMS);
+      const { uri: uriTwo, approval: approvalTwo } = await clients.A.connect(TEST_CONNECT_PARAMS);
+      const { uri: uriThree, approval: approvalThree } = await clients.A.connect(
+        TEST_CONNECT_PARAMS,
+      );
+
+      if (!uriOne || !uriTwo || !uriThree) throw new Error("URI is undefined");
+
+      const wallet = clients.B;
+
+      const onSessionPropose = async (params) => {
+        await wallet.approve({ id: params.id, namespaces: TEST_NAMESPACES });
+      };
+      wallet.on("session_proposal", onSessionPropose);
+
+      // approve the oldest
+      const sessionOne = await Promise.all([approvalOne(), wallet.pair({ uri: uriOne })]).then(
+        (result) => result[0],
+      );
+      // approve the newest
+      const sessionThree = await Promise.all([
+        approvalThree(),
+        wallet.pair({ uri: uriThree }),
+      ]).then((result) => result[0]);
+      // approve the middle
+      const sessionTwo = await Promise.all([approvalTwo(), wallet.pair({ uri: uriTwo })]).then(
+        (result) => result[0],
+      );
+
+      wallet.off("session_proposal", onSessionPropose);
+
+      expect(sessionOne).to.exist;
+      expect(sessionTwo).to.exist;
+      expect(sessionThree).to.exist;
+      expect(sessionOne.topic).to.not.eq(sessionTwo.topic);
+      expect(sessionOne.topic).to.not.eq(sessionThree.topic);
+      expect(sessionTwo.topic).to.not.eq(sessionThree.topic);
+      expect(wallet.session.getAll().length).to.eq([sessionOne, sessionTwo, sessionThree].length);
+      expect(wallet.session.get(sessionOne.topic)).to.exist;
+      expect(wallet.session.get(sessionTwo.topic)).to.exist;
+      expect(wallet.session.get(sessionThree.topic)).to.exist;
+
+      await deleteClients(clients);
+    });
     it("connect (with old pairing)", async () => {
       const {
         clients,