Merge PR #868 from 'nodech/wallet-deadlock-fix'

handshake-org · Oct 27, 2023 · bc3c172 · bc3c172
2 parents bb7da60 + 29625eb
commit bc3c172
Show file tree

Hide file tree

Showing 4 changed files with 720 additions and 698 deletions.
diff --git a/lib/wallet/walletdb.js b/lib/wallet/walletdb.js
@@ -143,6 +143,13 @@ class WalletDB extends EventEmitter {
     });
 
     this.client.bind('block connect', async (entry, txs) => {
+      // If we are rescanning or doing initial sync we ignore
+      // block connect events. This avoids deadlocks when using
+      // nodeclient, but also skips unnecessary addBlock calls
+      // that would just repeat after the txLock is unlocked.
+      if (this.rescanning)
+        return;
+
       try {
         await this.addBlock(entry, txs);
       } catch (e) {
@@ -385,13 +392,15 @@ class WalletDB extends EventEmitter {
 
   async syncNode() {
     const unlock = await this.txLock.lock();
+    this.rescanning = true;
     try {
       this.logger.info('Resyncing from server...');
       await this.syncInitState();
       await this.syncFilter();
       await this.syncChain();
       await this.resend();
     } finally {
+      this.rescanning = false;
       unlock();
     }
   }
@@ -457,6 +466,7 @@ class WalletDB extends EventEmitter {
 
   /**
    * Connect and sync with the chain server.
+   * Part of syncNode.
    * @private
    * @returns {Promise}
    */
@@ -477,11 +487,13 @@ class WalletDB extends EventEmitter {
       height -= 1;
     }
 
+    // syncNode sets the rescanning to true.
     return this.scan(height);
   }
 
   /**
    * Rescan blockchain from a given height.
+   * Needs this.rescanning = true to be set from the caller.
    * @private
    * @param {Number?} height
    * @returns {Promise}
@@ -501,12 +513,7 @@ class WalletDB extends EventEmitter {
 
     const tip = await this.getTip();
 
-    try {
-      this.rescanning = true;
-      await this.client.rescan(tip.hash);
-    } finally {
-      this.rescanning = false;
-    }
+    return this.client.rescan(tip.hash);
   }
 
   /**
@@ -600,6 +607,7 @@ class WalletDB extends EventEmitter {
 
   async rescan(height) {
     const unlock = await this.txLock.lock();
+
     try {
       return await this._rescan(height);
     } finally {
@@ -615,7 +623,13 @@ class WalletDB extends EventEmitter {
    */
 
   async _rescan(height) {
-    return this.scan(height);
+    this.rescanning = true;
+
+    try {
+      return await this.scan(height);
+    } finally {
+      this.rescanning = false;
+    }
   }
 
   /**
@@ -2237,6 +2251,7 @@ class WalletDB extends EventEmitter {
 
   async addBlock(entry, txs) {
     const unlock = await this.txLock.lock();
+
     try {
       return await this._addBlock(entry, txs);
     } finally {
@@ -2274,7 +2289,7 @@ class WalletDB extends EventEmitter {
       // processed (in the case of a crash).
       this.logger.warning('Already saw WalletDB block (%d).', tip.height);
     } else if (tip.height !== this.state.height + 1) {
-      await this.scan(this.state.height);
+      await this._rescan(this.state.height);
       return 0;
     }
 

diff --git a/test/wallet-balance-test.js b/test/wallet-balance-test.js
@@ -438,7 +438,7 @@ describe('Wallet Balance', function() {
         await discoverFn(wallet, ahead, opts);
 
       // Final look at full picture.
-      await wdb.scan(chain.tip.height - 1);
+      await wdb.rescan(chain.tip.height - 1);
       await checks.blockConfirmCheck(wallet, ahead, opts);
 
       if (discoverAt === BEFORE_BLOCK_UNCONFIRM)
@@ -449,7 +449,7 @@ describe('Wallet Balance', function() {
       await checks.blockUnconfirmCheck(wallet, ahead, opts);
 
       // Clean up wallet.
-      await wdb.scan(chain.tip.height - 1);
+      await wdb.rescan(chain.tip.height - 1);
       await checks.blockFinalConfirmCheck(wallet, ahead, opts);
     };
   };