Add lint rule to not use Promise.race (#5103)

config/eslint-config-ironfish/index.js

@@ -68,6 +68,7 @@ module.exports = {
   rules: {
     'ironfish/no-vague-imports': 'error',
     'ironfish/no-buffer-cmp': 'error',
+    'ironfish/no-promise-race': 'error',
 
     // Catches expressions that aren't assigned
     '@typescript-eslint/no-unused-expressions': [

config/eslint-plugin-ironfish/index.js

@@ -42,4 +42,19 @@ module.exports.rules = {
       };
     },
   },
+  "no-promise-race": {
+    create(context) {
+      return {
+        MemberExpression: function (node) {
+          if (node.object.name === 'Promise' && node.property.name === 'race') {
+            context.report({
+              node,
+              message:
+                "Promise.race leaks memory. You can work around it by using PromiseUtils.split to pass resolve/reject to other Promises. See https://github.com/nodejs/node/issues/17469#issuecomment-685216777 for more details.",
+            });
+          }
+        },
+      };
+    },
+  },
 };

ironfish/src/utils/asyncQueue.test.ts

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 import { AsyncQueue } from './asyncQueue'
+import { PromiseUtils } from './promise'
 
 describe('AsyncQueue', () => {
   it('yields items in the same order as they were added', async () => {
@@ -81,7 +82,11 @@ describe('AsyncQueue', () => {
         () => 'otherPromise',
       )
 
-      const resolved = await Promise.race([pushPromise, otherPromise])
+      const [promise, res, rej] = PromiseUtils.split()
+      pushPromise.then(res, rej)
+      otherPromise.then(res, rej)
+
+      const resolved = await promise
       expect(resolved).toBe('otherPromise')
 
       // After popping an element, the promise returned by push should resolve
@@ -101,7 +106,11 @@ describe('AsyncQueue', () => {
         () => 'otherPromise',
       )
 
-      const resolved = await Promise.race([popPromise, otherPromise])
+      const [promise, res, rej] = PromiseUtils.split()
+      popPromise.then(res, rej)
+      otherPromise.then(res, rej)
+
+      const resolved = await promise
       expect(resolved).toBe('otherPromise')
 
       // After pushing a new element, the promise returned by pop should

ironfish/src/wallet/scanner/noteDecryptor.ts

@@ -6,6 +6,7 @@ import { Config } from '../../fileStores'
 import { BlockHeader } from '../../primitives'
 import { Transaction } from '../../primitives/transaction'
 import { AsyncQueue } from '../../utils/asyncQueue'
+import { PromiseUtils } from '../../utils/promise'
 import { WorkerPool } from '../../workerPool'
 import { Job } from '../../workerPool/job'
 import {
@@ -23,6 +24,14 @@ export type DecryptNotesFromTransactionsCallback = (
   transactions: Array<{ transaction: Transaction; decryptedNotes: Array<DecryptedNote> }>,
 ) => Promise<void>
 
+type DecryptQueueValue = {
+  job: Job
+  accounts: ReadonlyArray<Account>
+  blockHeader: BlockHeader
+  transactions: ReadonlyArray<Transaction>
+  callback: DecryptNotesFromTransactionsCallback
+}
+
 export class BackgroundNoteDecryptor {
   private isStarted = false
 
@@ -34,13 +43,7 @@ export class BackgroundNoteDecryptor {
 
   private readonly workerPool: WorkerPool
   private readonly options: DecryptNotesOptions
-  private readonly decryptQueue: AsyncQueue<{
-    job: Job
-    accounts: ReadonlyArray<Account>
-    blockHeader: BlockHeader
-    transactions: ReadonlyArray<Transaction>
-    callback: DecryptNotesFromTransactionsCallback
-  }>
+  private readonly decryptQueue: AsyncQueue<DecryptQueueValue>
 
   constructor(workerPool: WorkerPool, config: Config, options: DecryptNotesOptions) {
     this.workerPool = workerPool
@@ -81,13 +84,30 @@ export class BackgroundNoteDecryptor {
   }
 
   private async decryptLoop(): Promise<void> {
+    let resolve: (value: DecryptQueueValue | void) => unknown
+    let reject: (reason?: unknown) => void
+
+    this.onStopped.then(
+      (value) => resolve?.(value),
+      (err) => reject?.(err),
+    )
+
     while (this.isStarted) {
       if (this.decryptQueue.isEmpty() && this.triggerFlushed) {
         this.triggerFlushed()
         this.triggerFlushed = null
       }
 
-      const item = await Promise.race([this.decryptQueue.pop(), this.onStopped])
+      const [promise, resolveNew, rejectNew] = PromiseUtils.split<DecryptQueueValue | void>()
+      resolve = resolveNew
+      reject = rejectNew
+
+      this.decryptQueue.pop().then(
+        (value) => resolve(value),
+        (err) => reject(err),
+      )
+
+      const item = await promise
       if (!item) {
         break
       }