fix(lndclient): unknown service lnrpc.Lightning

This fixes an issue where attempting a `GetInfo` call to lnd while lnd is locked results in an `UNIMPLEMENTED` grpc error that persists even after lnd has been unlocked and the `GetInfo` call becomes available. We close grpc clients after connection verification attempts so that we have a fresh start for future attempts Fixes #1039.
ExchangeUnion · Jun 19, 2019 · 9cb0d55 · 9cb0d55
1 parent d11bee6
commit 9cb0d55
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 33 deletions.
diff --git a/lib/Xud.ts b/lib/Xud.ts
@@ -38,7 +38,7 @@ class Xud extends EventEmitter {
   private grpcAPIProxy?: GrpcWebProxyServer;
   private swaps!: Swaps;
   private shuttingDown = false;
-  private swapClientManager!: SwapClientManager;
+  private swapClientManager?: SwapClientManager;
 
   /**
    * Create an Exchange Union daemon.
@@ -184,10 +184,12 @@ class Xud extends EventEmitter {
     this.shuttingDown = true;
     this.logger.info('XUD is shutting down');
 
-    this.swapClientManager.close();
     // TODO: ensure we are not in the middle of executing any trades
     const closePromises: Promise<void>[] = [];
 
+    if (this.swapClientManager) {
+      closePromises.push(this.swapClientManager.close());
+    }
     if (this.httpServer) {
       closePromises.push(this.httpServer.close());
     }

diff --git a/lib/lndclient/LndClient.ts b/lib/lndclient/LndClient.ts
@@ -36,9 +36,9 @@ interface LndClient {
 class LndClient extends SwapClient {
   public readonly type = SwapClientType.Lnd;
   public readonly cltvDelta: number;
-  private lightning!: LightningClient | LightningMethodIndex;
-  private walletUnlocker!: WalletUnlockerClient | InvoicesMethodIndex;
-  private invoices!: InvoicesClient | InvoicesMethodIndex;
+  private lightning?: LightningClient | LightningMethodIndex;
+  private walletUnlocker?: WalletUnlockerClient | InvoicesMethodIndex;
+  private invoices?: InvoicesClient | InvoicesMethodIndex;
   private meta!: grpc.Metadata;
   private uri!: string;
   private credentials!: ChannelCredentials;
@@ -155,7 +155,7 @@ class LndClient extends SwapClient {
     if (this.isDisabled()) {
       error = errors.LND_IS_DISABLED.message;
     } else if (!this.isConnected()) {
-      error = errors.LND_IS_UNAVAILABLE.message;
+      error = errors.LND_IS_UNAVAILABLE(this.status).message;
     } else {
       try {
         const lnd = await this.getInfo();
@@ -189,11 +189,10 @@ class LndClient extends SwapClient {
     if (this.isDisabled()) {
       throw(errors.LND_IS_DISABLED);
     }
+
     if (!this.isConnected()) {
       this.logger.info(`trying to verify connection to lnd at ${this.uri}`);
       this.lightning = new LightningClient(this.uri, this.credentials);
-      this.invoices = new InvoicesClient(this.uri, this.credentials);
-      this.walletUnlocker = new WalletUnlockerClient(this.uri, this.credentials);
 
       try {
         const getInfoResponse = await this.getInfo();
@@ -208,6 +207,15 @@ class LndClient extends SwapClient {
             this.identityPubKey = newPubKey;
           }
           this.emit('connectionVerified', newPubKey);
+
+          this.invoices = new InvoicesClient(this.uri, this.credentials);
+
+          if (this.walletUnlocker) {
+            // WalletUnlocker service is disabled when the main Lightning service is available
+            this.walletUnlocker.close();
+            this.walletUnlocker = undefined;
+          }
+
           this.subscribeChannels();
         } else {
           await this.setStatus(ClientStatus.OutOfSync);
@@ -216,11 +224,14 @@ class LndClient extends SwapClient {
       } catch (err) {
         if (err.code === grpc.status.UNIMPLEMENTED) {
           // if GetInfo is unimplemented, it means this lnd instance is online but locked
+          this.walletUnlocker = new WalletUnlockerClient(this.uri, this.credentials);
+          this.lightning.close();
+          this.lightning = undefined;
           await this.setStatus(ClientStatus.WaitingUnlock);
         } else {
           this.logger.error(`could not verify connection to lnd at ${this.uri}, error: ${JSON.stringify(err)},
             retrying in ${LndClient.RECONNECT_TIMER} ms`);
-          await this.setStatus(ClientStatus.Disconnected);
+          await this.disconnect();
         }
       }
     }
@@ -489,6 +500,9 @@ class LndClient extends SwapClient {
   }
 
   private subscribeSingleInvoice = (rHash: string) => {
+    if (!this.invoices) {
+      throw errors.LND_IS_UNAVAILABLE(this.status);
+    }
     const paymentHash = new lndrpc.PaymentHash();
     // TODO: use RHashStr when bug fixed in lnd - https://github.com/lightningnetwork/lnd/pull/3019
     paymentHash.setRHash(hexToUint8Array(rHash));
@@ -511,6 +525,9 @@ class LndClient extends SwapClient {
    * Subscribes to channel events.
    */
   private subscribeChannels = (): void => {
+    if (!this.lightning) {
+      throw errors.LND_IS_UNAVAILABLE(this.status);
+    }
     if (this.channelSubscription) {
       this.channelSubscription.cancel();
     }
@@ -519,19 +536,16 @@ class LndClient extends SwapClient {
     .on('error', async (error) => {
       this.channelSubscription = undefined;
       this.logger.error(`lnd has been disconnected, error: ${error}`);
-      await this.setStatus(ClientStatus.Disconnected);
+      await this.disconnect();
     });
   }
 
   /**
    * Attempts to close an open channel.
    */
   public closeChannel = (fundingTxId: string, outputIndex: number, force: boolean): void => {
-    if (this.isDisabled()) {
-      throw(errors.LND_IS_DISABLED);
-    }
-    if (this.isDisconnected()) {
-      throw(errors.LND_IS_UNAVAILABLE);
+    if (!this.lightning) {
+      throw(errors.LND_IS_UNAVAILABLE(this.status));
     }
     const request = new lndrpc.CloseChannelRequest();
     const channelPoint = new lndrpc.ChannelPoint();
@@ -555,11 +569,21 @@ class LndClient extends SwapClient {
       });
   }
 
-  /** Lnd client specific cleanup. */
-  protected closeSpecific = () => {
+  /** Lnd specific procedure to disconnect from the server. */
+  protected disconnect = async () => {
     if (this.channelSubscription) {
       this.channelSubscription.cancel();
+      this.channelSubscription = undefined;
+    }
+    if (this.lightning) {
+      this.lightning.close();
+      this.lightning = undefined;
+    }
+    if (this.invoices) {
+      this.invoices.close();
+      this.invoices = undefined;
     }
+    await this.setStatus(ClientStatus.Disconnected);
   }
 }
 

diff --git a/lib/lndclient/errors.ts b/lib/lndclient/errors.ts
@@ -1,4 +1,5 @@
 import errorCodesPrefix from '../constants/errorCodesPrefix';
+import { ClientStatus } from '../swaps/SwapClient';
 
 const codesPrefix = errorCodesPrefix.LND;
 const errorCodes = {
@@ -11,10 +12,10 @@ const errors = {
     message: 'lnd is disabled',
     code: errorCodes.LND_IS_DISABLED,
   },
-  LND_IS_UNAVAILABLE: {
-    message: 'lnd is not available',
+  LND_IS_UNAVAILABLE: (status: ClientStatus) => ({
+    message: `lnd is ${ClientStatus[status]}`,
     code: errorCodes.LND_IS_UNAVAILABLE,
-  },
+  }),
 };
 
 export { errorCodes };

diff --git a/lib/raidenclient/RaidenClient.ts b/lib/raidenclient/RaidenClient.ts
@@ -82,7 +82,7 @@ class RaidenClient extends SwapClient {
         `could not verify connection to raiden at ${this.host}:${this.port}, retrying in ${RaidenClient.RECONNECT_TIMER} ms`,
         err,
       );
-      await this.setStatus(ClientStatus.Disconnected);
+      await this.disconnect();
     }
   }
 
@@ -351,7 +351,9 @@ class RaidenClient extends SwapClient {
   }
 
   /** Raiden client specific cleanup. */
-  protected closeSpecific() {}
+  protected disconnect = async () => {
+    await this.setStatus(ClientStatus.Disconnected);
+  }
 }
 
 export default RaidenClient;
diff --git a/lib/swaps/SwapClient.ts b/lib/swaps/SwapClient.ts
@@ -138,18 +138,21 @@ abstract class SwapClient extends EventEmitter {
   public isWaitingUnlock(): boolean {
     return this.status === ClientStatus.WaitingUnlock;
   }
+  public isNotInitialized(): boolean {
+    return this.status === ClientStatus.NotInitialized;
+  }
   /** Ends all connections, subscriptions, and timers for for this client. */
-  public close() {
+  public async close() {
+    await this.disconnect();
     if (this.reconnectionTimer) {
       clearTimeout(this.reconnectionTimer);
     }
     if (this.updateCapacityTimer) {
       clearInterval(this.updateCapacityTimer);
     }
-    this.closeSpecific();
     this.removeAllListeners();
   }
-  protected abstract closeSpecific(): void;
+  protected abstract async disconnect(): Promise<void>;
 }
 
 export default SwapClient;

diff --git a/lib/swaps/SwapClientManager.ts b/lib/swaps/SwapClientManager.ts
@@ -113,7 +113,7 @@ class SwapClientManager extends EventEmitter {
         try {
           return swapClient.genSeed();
         } catch (err) {
-          swapClient.logger.debug('could not generate seed');
+          swapClient.logger.error('could not generate seed', err);
         }
       }
     }
@@ -248,19 +248,21 @@ class SwapClientManager extends EventEmitter {
    * Closes all swap client instances gracefully.
    * @returns Nothing upon success, throws otherwise.
    */
-  public close = (): void => {
+  public close = async (): Promise<void> => {
+    const closePromises: Promise<void>[] = [];
     let raidenClosed = false;
     for (const swapClient of this.swapClients.values()) {
-      swapClient.close();
+      closePromises.push(swapClient.close());
       if (isRaidenClient(swapClient)) {
         raidenClosed = true;
       }
     }
     // we make sure to close raiden client because it
     // might not be associated with any currency
     if (!this.raidenClient.isDisabled() && !raidenClosed) {
-      this.raidenClient.close();
+      closePromises.push(this.raidenClient.close());
     }
+    await Promise.all(closePromises);
   }
 
   private bind = () => {

diff --git a/test/jest/SwapClientManager.spec.ts b/test/jest/SwapClientManager.spec.ts
@@ -133,7 +133,7 @@ describe('Swaps.SwapClientManager', () => {
     expect(onListenerMock).toHaveBeenCalledTimes(4);
     expect(swapClientManager.get('BTC')).not.toBeUndefined();
     expect(swapClientManager.get('LTC')).not.toBeUndefined();
-    swapClientManager.close();
+    await swapClientManager.close();
     expect(closeMock).toHaveBeenCalledTimes(2);
   });
 
@@ -146,7 +146,7 @@ describe('Swaps.SwapClientManager', () => {
     expect(swapClientManager['swapClients'].size).toEqual(1);
     expect(onListenerMock).toHaveBeenCalledTimes(2);
     expect(swapClientManager.get('BTC')).not.toBeUndefined();
-    swapClientManager.close();
+    await swapClientManager.close();
     expect(closeMock).toHaveBeenCalledTimes(1);
   });
 
@@ -160,7 +160,7 @@ describe('Swaps.SwapClientManager', () => {
     expect(onListenerMock).toHaveBeenCalledTimes(0);
     expect(swapClientManager.get('BTC')).toBeUndefined();
     expect(swapClientManager.get('WETH')).toBeUndefined();
-    swapClientManager.close();
+    await swapClientManager.close();
     expect(closeMock).toHaveBeenCalledTimes(0);
   });
 
@@ -170,7 +170,7 @@ describe('Swaps.SwapClientManager', () => {
     swapClientManager = new SwapClientManager(config, loggers);
     await swapClientManager.init(db.models);
     expect(swapClientManager['swapClients'].size).toEqual(3);
-    swapClientManager.close();
+    await swapClientManager.close();
     expect(closeMock).toHaveBeenCalledTimes(3);
   });