chore: add more vigilant checking to resolution (#101)

* chore: add more vigilant checking to resolution

* chore: update blocking for getNotificationHistory

* chore: run prettier

* chore: remove unused import

* chore: add UT

* chore: run prettier

* chore: update

* fix: bug where correct topic is ignored

* chore: use response topic instead of subscription topic

* chore: bump patch

Author: devceline

package-lock.json

@@ -1,7 +1,7 @@
 {
   "name": "@walletconnect/notify-client",
   "description": "WalletConnect Notify Client",
-  "version": "1.1.1",
+  "version": "1.1.2",
   "author": "WalletConnect, Inc. <walletconnect.com>",
   "homepage": "https://github.com/walletconnect/notify-client-js/",
   "license": "Apache-2.0",

packages/notify-client/package.json

@@ -34,7 +34,12 @@ import {
   NOTIFY_AUTHORIZATION_STATEMENT_ALL_DOMAINS,
   NOTIFY_AUTHORIZATION_STATEMENT_THIS_DOMAIN,
 } from "../constants";
-import { INotifyEngine, JsonRpcTypes, NotifyClientTypes } from "../types";
+import {
+  INotifyEngine,
+  JsonRpcTypes,
+  NotifyClientTypes,
+  NotifyEngineTypes,
+} from "../types";
 import { getCaip10FromDidPkh } from "../utils/address";
 import { getDappUrl } from "../utils/formats";
 
@@ -287,13 +292,22 @@ export class NotifyEngine extends INotifyEngine {
     );
 
     return new Promise<boolean>((resolve) => {
-      this.client.once("notify_subscription", (args) => {
+      const listener = (
+        args: NotifyClientTypes.EventArguments["notify_subscription"]
+      ) => {
+        if (args.topic !== responseTopic) {
+          return;
+        }
+        this.client.off("notify_subscription", listener);
         if (args.params.error) {
           resolve(false);
         } else {
           resolve(true);
         }
-      });
+      };
+
+      this.client.on("notify_subscription", listener);
+
       // SPEC: Wallet sends wc_notifySubscribe request (type 1 envelope) on subscribe topic with subscriptionAuth
       this.sendRequest<"wc_notifySubscribe">(
         subscribeTopic,
@@ -350,13 +364,22 @@ export class NotifyEngine extends INotifyEngine {
     );
 
     return new Promise<boolean>((resolve, reject) => {
-      this.client.once("notify_update", (args) => {
+      const listener = (
+        args: NotifyClientTypes.EventArguments["notify_update"]
+      ) => {
+        if (args.topic !== topic) {
+          return;
+        }
+        this.client.off("notify_update", listener);
+
         if (args.params.error) {
           reject(args.params.error);
         } else {
           resolve(true);
         }
-      });
+      };
+
+      this.client.on("notify_update", listener);
 
       this.sendRequest(topic, "wc_notifyUpdate", {
         updateAuth,
@@ -420,13 +443,23 @@ export class NotifyEngine extends INotifyEngine {
       );
 
       return new Promise((resolve, reject) => {
-        this.once("notify_get_notifications_response", (args) => {
+        const listener = (
+          args: NotifyEngineTypes.EventArguments["notify_get_notifications_response"]
+        ) => {
+          if (args.topic !== topic) {
+            return;
+          }
+
+          this.off("notify_get_notifications_response", listener);
+
           if (args.error === null) {
             resolve(args);
           } else {
             reject(new Error(args.error));
           }
-        });
+        };
+
+        this.on("notify_get_notifications_response", listener);
 
         // Add timeout to prevent memory leaks with unresolving promises
         setTimeout(() => {
@@ -490,13 +523,21 @@ export class NotifyEngine extends INotifyEngine {
     });
 
     return new Promise<void>((resolve, reject) => {
-      this.client.once("notify_delete", (args) => {
+      const listener = (
+        args: NotifyClientTypes.EventArguments["notify_delete"]
+      ) => {
+        if (args.topic !== topic) {
+          return;
+        }
+        this.client.off("notify_delete", listener);
         if (args.params.error) {
           reject(args.params.error);
         } else {
           resolve();
         }
-      });
+      };
+
+      this.client.on("notify_delete", listener);
 
       this.sendRequest(topic, "wc_notifyDelete", { deleteAuth }).then(() => {
         this.client.logger.info(
@@ -941,6 +982,7 @@ export class NotifyEngine extends INotifyEngine {
           }));
 
         this.emit("notify_get_notifications_response", {
+          topic,
           hasMore: claims.mre ?? false,
           hasMoreUnread: claims.mur ?? false,
           error: null,
@@ -954,6 +996,7 @@ export class NotifyEngine extends INotifyEngine {
         );
 
         this.emit("notify_get_notifications_response", {
+          topic,
           error: payload.error.message,
         });
       }

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

@@ -25,11 +25,12 @@ export declare namespace NotifyEngineTypes {
     publishedAt: number;
   }
 
-  type EventResponseOrError<T> =
+  type EventResponseOrError<T> = { topic: string } & (
     | (T & { error: null })
     | {
         error: string;
-      };
+      }
+  );
 
   type Event = "notify_get_notifications_response";
 

packages/notify-client/src/types/engine.ts

@@ -13,7 +13,7 @@ import {
   NotifyClientTypes,
 } from "../src/";
 import { waitForEvent } from "./helpers/async";
-import { testDappMetadata } from "./helpers/mocks";
+import { gmHackersMetadata, testDappMetadata } from "./helpers/mocks";
 import { createNotifySubscription, sendNotifyMessage } from "./helpers/notify";
 import { disconnectSocket } from "./helpers/ws";
 import axios from "axios";
@@ -844,6 +844,112 @@ describe("Notify", () => {
       });
     });
 
+    describe("Blocking functions", () => {
+      it("Subscribe only resolves once a subscription succeeded and is stored", async () => {
+        const preparedRegistration = await wallet.prepareRegistration({
+          account,
+          domain: testDappMetadata.appDomain,
+          allApps: true,
+        });
+
+        await wallet.register({
+          registerParams: preparedRegistration.registerParams,
+          signature: await onSign(preparedRegistration.message),
+        });
+
+        await wallet.subscribe({
+          appDomain: testDappMetadata.appDomain,
+          account,
+        });
+
+        expect(wallet.subscriptions.length).toEqual(1);
+        expect(wallet.subscriptions.getAll()[0].metadata.appDomain).toEqual(
+          testDappMetadata.appDomain
+        );
+      });
+
+      it.skipIf(!hasTestProjectSecret)(
+        "Only resolves when topic accurate response is issued",
+        async () => {
+          const preparedRegistration = await wallet.prepareRegistration({
+            account,
+            domain: testDappMetadata.appDomain,
+            allApps: true,
+          });
+
+          await wallet.register({
+            registerParams: preparedRegistration.registerParams,
+            signature: await onSign(preparedRegistration.message),
+          });
+
+          await wallet.subscribe({
+            appDomain: testDappMetadata.appDomain,
+            account,
+          });
+
+          expect(wallet.subscriptions.length).toEqual(1);
+          expect(wallet.subscriptions.getAll()[0].metadata.appDomain).toEqual(
+            testDappMetadata.appDomain
+          );
+
+          const app1Topic = wallet.subscriptions.getAll()[0].topic;
+
+          let gotMessage = false;
+
+          // send messages to app1
+          await sendNotifyMessage(account, "Test1");
+
+          console.log(">>>>>>>>>> sent message");
+
+          wallet.on("notify_message", () => {
+            gotMessage = true;
+          });
+
+          await waitForEvent(() => gotMessage);
+
+          console.log(">>>>>>>>>> got message");
+
+          const notifs1 = wallet.getNotificationHistory({
+            topic: app1Topic,
+            limit: 5,
+          });
+
+          // close transport to prevent getting a real response from the relay
+          await wallet.core.relayer.transportClose();
+
+          const emptyNotif = {
+            body: "",
+            id: "",
+            sentAt: Date.now(),
+            title: "",
+            type: "",
+            url: "",
+          };
+
+          wallet.engine["emit"]("notify_get_notifications_response", {
+            topic: "wrong_topic",
+            error: null,
+            hasMore: false,
+            hasMoreUnread: false,
+            notifications: [],
+          });
+
+          wallet.engine["emit"]("notify_get_notifications_response", {
+            topic: app1Topic,
+            error: null,
+            hasMore: false,
+            hasMoreUnread: false,
+            notifications: [emptyNotif, emptyNotif],
+          });
+
+          expect(notifs1).resolves.toSatisfy((resolved: any) => {
+            console.log("Resolved is!", resolved);
+            return resolved.notifications.length === 2;
+          });
+        }
+      );
+    });
+
     describe.skipIf(!hasTestProjectSecret)("Message Deduping", () => {
       it("dedups messages based on notify message id", async () => {
         await createNotifySubscription(wallet, account, onSign);