fix: load native signatures and preserve request confirmation

.circleci/config.yml

@@ -86,6 +86,9 @@ jobs:
       - run:
           name: Yarn install
           command: yarn install --frozen-lockfile --cache-folder ~/.cache/yarn
+      - run:
+          name: Verify native signature module
+          command: node -e 'require("secp256k1/bindings")'
       - save_cache:
           name: Save Yarn Package Cache
           key: yarn-packages-{{ checksum "yarn.lock" }}

package.json

@@ -64,6 +64,7 @@
     "underscore": "^1.12.1",
     "crypto-js": "^4.2.0",
     "minimist": "^0.2.4",
+    "node-gyp-build": "4.8.4",
     "semver": "^7.5.4",
     "json-schema": "^0.4.0",
     "json5": "^2.2.1"

packages/request-client.js/src/api/request.ts

@@ -34,6 +34,8 @@ export default class Request {
   private paymentNetwork: PaymentTypes.IPaymentNetwork | null = null;
   private contentDataExtension: ContentDataExtension | null;
   private emitter: EventEmitter;
+  private confirmationPromise: Promise<Types.IRequestDataWithEvents>;
+  private refreshPromise: Promise<unknown> = Promise.resolve();
   private logger: SimpleLogger = new SimpleLogger(LogTypes.LogLevel.WARN);
 
   /**
@@ -119,6 +121,14 @@ export default class Request {
     this.contentDataExtension = options?.contentDataExtension || null;
     this.paymentNetwork = options?.paymentNetwork || null;
     this.emitter = new EventEmitter();
+    // Subscribe before refresh can finish creation, so late callers cannot miss its result.
+    this.confirmationPromise = new Promise((resolve, reject) => {
+      this.on('confirmed', resolve);
+      this.on('error', reject);
+    });
+    // Keep failures available to waitForConfirmation without an unhandled rejection
+    // when callers use the event API instead.
+    this.confirmationPromise.catch(() => undefined);
     this.skipPaymentDetection = options?.skipPaymentDetection || false;
     this.disableEvents = options?.disableEvents || false;
     this.currencyManager = currencyManager;
@@ -162,10 +172,7 @@ export default class Request {
    * @returns the request data
    */
   public waitForConfirmation(): Promise<Types.IRequestDataWithEvents> {
-    return new Promise((resolve, reject): any => {
-      this.on('confirmed', resolve);
-      this.on('error', reject);
-    });
+    return this.confirmationPromise;
   }
 
   /**
@@ -748,7 +755,17 @@ export default class Request {
    * @param requestAndMeta return from getRequestFromId to avoid asking twice
    * @returns Refreshed request data
    */
-  public async refresh(
+  public refresh(
+    requestAndMeta?: RequestLogicTypes.IReturnGetRequestFromId,
+  ): Promise<Types.IRequestDataWithEvents> {
+    // Finish the initial refresh before applying a newer confirmation refresh.
+    const refresh = this.refreshPromise.then(() => this.refreshData(requestAndMeta));
+    // A failed read must not prevent a later refresh from retrying.
+    this.refreshPromise = refresh.catch(() => undefined);
+    return refresh;
+  }
+
+  private async refreshData(
     requestAndMeta?: RequestLogicTypes.IReturnGetRequestFromId,
   ): Promise<Types.IRequestDataWithEvents> {
     if (this.confirmationErrorOccurredAtCreation) {

packages/request-client.js/test/api/request.test.ts

@@ -9,6 +9,7 @@ import {
 import { EventEmitter } from 'events';
 
 import Request from '../../src/api/request';
+import { IRequestDataWithEvents } from '../../src/types';
 
 const mockRequestLogic: RequestLogicTypes.IRequestLogic = {
   async createRequest(): Promise<any> {
@@ -153,6 +154,136 @@ describe('api/request', () => {
     expect(handleError).toHaveBeenCalled();
   });
 
+  describe('waitForConfirmation', () => {
+    afterEach(() => {
+      jest.restoreAllMocks();
+    });
+
+    it('waits for refreshed confirmation data and supports repeated waits', async () => {
+      const emitter = new EventEmitter();
+      const request = new Request('1', mockRequestLogic, currencyManager, {
+        requestLogicCreateResult: emitter as any,
+      });
+      const data = request.getData();
+      let finishRefresh!: (value: IRequestDataWithEvents) => void;
+      jest.spyOn(request, 'refresh').mockReturnValue(
+        new Promise((resolve) => {
+          finishRefresh = resolve;
+        }),
+      );
+      const confirmed = jest.fn();
+      const pending = request.waitForConfirmation().then(confirmed);
+      emitter.emit('confirmed');
+      await Promise.resolve();
+      expect(confirmed).not.toHaveBeenCalled();
+
+      finishRefresh(data);
+      await pending;
+      expect(confirmed).toHaveBeenCalledWith(data);
+      await expect(request.waitForConfirmation()).resolves.toBe(data);
+    });
+
+    it('returns confirmation that arrived before the caller started waiting', async () => {
+      const emitter = new EventEmitter();
+      const request = new Request('1', mockRequestLogic, currencyManager, {
+        requestLogicCreateResult: emitter as any,
+      });
+      const data = request.getData();
+      jest.spyOn(request, 'refresh').mockResolvedValue(data);
+      const notified = new Promise<void>((resolve) => request.on('confirmed', () => resolve()));
+
+      emitter.emit('confirmed');
+      await notified;
+
+      await expect(request.waitForConfirmation()).resolves.toBe(data);
+    });
+
+    it('rejects a late wait when creation failed', async () => {
+      const emitter = new EventEmitter();
+      const request = new Request('1', mockRequestLogic, currencyManager, {
+        requestLogicCreateResult: emitter as any,
+      });
+      const error = new Error('creation failed');
+      const notified = jest.fn();
+      request.on('error', notified);
+
+      emitter.emit('error', error);
+      await new Promise<void>((resolve) => setImmediate(resolve));
+
+      expect(notified).toHaveBeenCalledWith(error);
+      await expect(request.waitForConfirmation()).rejects.toBe(error);
+    });
+
+    it('rejects when refreshing the confirmation fails', async () => {
+      const emitter = new EventEmitter();
+      const request = new Request('1', mockRequestLogic, currencyManager, {
+        requestLogicCreateResult: emitter as any,
+      });
+      const error = new Error('refresh failed');
+      jest.spyOn(request, 'refresh').mockRejectedValue(error);
+      const pending = expect(request.waitForConfirmation()).rejects.toBe(error);
+
+      emitter.emit('confirmed');
+
+      await pending;
+    });
+
+    it('keeps confirmed data when the initial refresh is still running', async () => {
+      const fixture = await mockRequestLogic.getRequestFromId('1');
+      const confirmed = {
+        ...fixture,
+        result: {
+          request: { ...fixture.result.request!, state: RequestLogicTypes.STATE.CREATED },
+          pending: null,
+        },
+      };
+      let finishInitial!: (value: RequestLogicTypes.IReturnGetRequestFromId) => void;
+      jest
+        .spyOn(mockRequestLogic, 'getRequestFromId')
+        .mockImplementationOnce(() => new Promise((resolve) => (finishInitial = resolve)))
+        .mockResolvedValue(confirmed);
+      const emitter = new EventEmitter();
+      const request = new Request('1', mockRequestLogic, currencyManager, {
+        requestLogicCreateResult: emitter as any,
+      });
+      const events: string[] = [];
+      request.on('confirmed', () => events.push('confirmed'));
+      const initial = request.refresh().then(() => events.push('initial'));
+
+      emitter.emit('confirmed');
+      await new Promise<void>((resolve) => setImmediate(resolve));
+      finishInitial({
+        ...fixture,
+        result: { request: null, pending: confirmed.result.request },
+      });
+      await initial;
+      await expect(request.waitForConfirmation()).resolves.toMatchObject({
+        state: RequestLogicTypes.STATE.CREATED,
+        pending: null,
+      });
+      expect(request.getData()).toMatchObject({
+        state: RequestLogicTypes.STATE.CREATED,
+        pending: null,
+      });
+      expect(events).toEqual(['initial', 'confirmed']);
+    });
+
+    it('can refresh again after an earlier refresh failed', async () => {
+      const fixture = await mockRequestLogic.getRequestFromId('1');
+      const error = new Error('initial refresh failed');
+      jest
+        .spyOn(mockRequestLogic, 'getRequestFromId')
+        .mockRejectedValueOnce(error)
+        .mockResolvedValue(fixture);
+      const request = new Request('1', mockRequestLogic, currencyManager);
+      const initial = expect(request.refresh()).rejects.toBe(error);
+      const next = request.refresh();
+
+      await initial;
+      await expect(next).resolves.toMatchObject({ requestId: '1' });
+    });
+  });
+
   describe('accept', () => {
     it('calls request-logic', async () => {
       const spy = jest.spyOn(mockRequestLogic, 'acceptRequest');

packages/request-client.js/test/index.test.ts

@@ -827,9 +827,10 @@ describe('request-client.js', () => {
         [idRaw1.encryptionParams],
       );
 
+      await request.waitForConfirmation();
       const requestFromId = await requestNetwork.fromRequestId(request.requestId);
 
-      expect(requestFromId).toMatchObject(request);
+      expect(requestFromId.getData()).toMatchObject(request.getData());
 
       const requestData = requestFromId.getData();
       expect(requestData.meta).not.toBeNull();
@@ -1008,7 +1009,7 @@ describe('request-client.js', () => {
 
       const fetchedRequest = await requestNetwork.fromRequestId(request.requestId);
 
-      expect(fetchedRequest).toMatchObject(request);
+      expect(fetchedRequest.getData()).toMatchObject(request.getData());
 
       const requestData = fetchedRequest.getData();
       expect(requestData.meta).not.toBeNull();
@@ -1038,7 +1039,7 @@ describe('request-client.js', () => {
       );
 
       const fetchedRequest = await requestNetwork.fromRequestId(request.requestId);
-      expect(fetchedRequest).toMatchObject(request);
+      expect(fetchedRequest.getData()).toMatchObject(request.getData());
 
       const requestData = fetchedRequest.getData();
       expect(requestData.meta).not.toBeNull();
@@ -1081,6 +1082,7 @@ describe('request-client.js', () => {
         [idRaw1.encryptionParams],
       );
 
+      await request.waitForConfirmation();
       const fetchedRequest = await requestNetwork.fromRequestId(request.requestId);
       const requestData = fetchedRequest.getData();
       expect(requestData).toMatchObject(request.getData());
@@ -1134,6 +1136,7 @@ describe('request-client.js', () => {
         },
         [idRaw1.encryptionParams, idRaw2.encryptionParams],
       );
+      await request.waitForConfirmation();
       const fetchedRequest = await payeeRequestNetwork.fromRequestId(request.requestId);
       const requestData = fetchedRequest.getData();
       expect(requestData).toMatchObject(request.getData());

packages/utils/test/retry.test.ts

@@ -10,6 +10,7 @@ class TestClass {
 describe('Retry', () => {
   afterEach(() => {
     jest.clearAllMocks();
+    jest.useRealTimers();
   });
   it('retries a sync function', () => {
     const spy = jest.fn();
@@ -78,7 +79,7 @@ describe('Retry', () => {
     await expect(retry(test.method, { context: test })()).resolves.toBe('private');
   });
 
-  it('waits for the delay before retring', async () => {
+  it('waits for the configured delay before retrying', async () => {
     jest.useFakeTimers();
     jest.setSystemTime(0);
 
@@ -92,9 +93,9 @@ describe('Retry', () => {
     const retryDelay = 1000;
 
     const promise = retry(throwOnce, { retryDelay })();
-    jest.advanceTimersByTime(500);
+    await jest.advanceTimersByTimeAsync(retryDelay - 1);
     expect(throwOnce).toHaveBeenCalledTimes(1);
-    jest.advanceTimersByTime(500);
+    await jest.advanceTimersByTimeAsync(1);
     const callTime = await promise;
 
     expect(callTime).toBe(retryDelay);
@@ -103,26 +104,6 @@ describe('Retry', () => {
   });
 
   /* eslint-disable no-magic-numbers */
-  it('waits for the delay before retring (without fake timer)', async () => {
-    let retrying = false;
-    const spy = jest.fn(() => Date.now());
-
-    function throwOnce(): Promise<any> {
-      if (retrying) {
-        return Promise.resolve(spy());
-      }
-      retrying = true;
-      throw new Error(`thew`);
-    }
-
-    const now = Date.now();
-
-    setTimeout(() => expect(spy).not.toHaveBeenCalled(), 200);
-    const after = await retry(throwOnce, { retryDelay: 500 })();
-
-    expect(after).toBeGreaterThanOrEqual(now + 500);
-  });
-
   it('delay increases exponentially if using exponentialBackoff', async () => {
     jest.useFakeTimers();
     jest.setSystemTime(0);

yarn.lock

@@ -19226,12 +19226,7 @@ node-forge@^1.3.1:
   resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-1.3.2.tgz#d0d2659a26eef778bf84d73e7f55c08144ee7750"
   integrity sha512-6xKiQ+cph9KImrRh0VsjH2d8/GXA4FIMlgU4B757iI1ApvcyA9VlouP0yZJha01V+huImO+kKMU7ih+2+E14fw==
 
-node-gyp-build@^4.2.0:
-  version "4.6.0"
-  resolved "https://registry.npmjs.org/node-gyp-build/-/node-gyp-build-4.6.0.tgz"
-  integrity sha512-NTZVKn9IylLwUzaKjkas1e4u2DLNcV4rdYagA4PWdPwW87Bi7z+BznyKSRwS/761tV/lzCGXplWsiaMjLqP2zQ==
-
-node-gyp-build@^4.3.0:
+node-gyp-build@4.8.4, node-gyp-build@^4.2.0, node-gyp-build@^4.3.0:
   version "4.8.4"
   resolved "https://registry.yarnpkg.com/node-gyp-build/-/node-gyp-build-4.8.4.tgz#8a70ee85464ae52327772a90d66c6077a900cfc8"
   integrity sha512-LA4ZjwlnUblHVgq0oBF3Jl/6h/Nvs5fzBLwdEF4nuxnFdsfajde4WfxtJr3CaiH+F6ewcIB/q4jQ4UzPyid+CQ==