feat: disable raiden currencies w/out direct chan

This commit attempts to ensure that we have a direct channel with peers
for raiden currencies before exchanging orders with that peer for any
pairs involving that currency.

When a peer advertises a trading pair, either via initial handshake or
on a node state update, we check that we have a direct channel for any
raiden currencies before activating advertised pairs. If the trading
pair is not activated, then we do not send or accept orders for that
pair with the peer.

An existing timer with an interval of one minute is used for each peer
to periodically check if inactive currencies and pairs can be activated
by performing the same checks described above, including the direct
channel check for raiden currencies.

Closes #1027.

lib/orderbook/OrderBook.ts

@@ -129,6 +129,7 @@ class OrderBook extends EventEmitter {
     const { minQuantity } = this.thresholds;
     return order.quantity >= minQuantity;
   }
+
   /**
    * Checks that a currency advertised by a peer is known to us, has a swap client identifier,
    * and that their token identifier matches ours.
@@ -566,7 +567,8 @@ class OrderBook extends EventEmitter {
 
     this.emit('ownOrder.added', order);
 
-    this.broadcastOrder(order);
+    const outgoingOrder = OrderBook.createOutgoingOrder(order);
+    this.pool.broadcastOrder(outgoingOrder);
     return true;
   }
 
@@ -784,52 +786,62 @@ class OrderBook extends EventEmitter {
       return !peer.disabledCurrencies.has(baseCurrency) && !peer.disabledCurrencies.has(quoteCurrency);
     });
 
-    if (this.nosanityswaps) {
-      // we have disabled sanity checks, so assume all pairs should be activated
-      pairIdsToVerify.forEach(peer.activatePair);
-      return;
-    }
-
     // identify the unique currencies we need to verify for specified trading pairs
-    const currenciesToVerify = new Set<string>();
+    /** A map between currencies we are verifying and whether the currency is swappable. */
+    const currenciesToVerify = new Map<string, boolean>();
     pairIdsToVerify.forEach((pairId) => {
       const [baseCurrency, quoteCurrency] = pairId.split('/');
-      if (!peer.verifiedCurrencies.has(baseCurrency)) {
-        currenciesToVerify.add(baseCurrency);
+      if (!peer.isCurrencyActive(baseCurrency)) {
+        currenciesToVerify.set(baseCurrency, true);
       }
-      if (!peer.verifiedCurrencies.has(quoteCurrency)) {
-        currenciesToVerify.add(quoteCurrency);
+      if (!peer.isCurrencyActive(quoteCurrency)) {
+        currenciesToVerify.set(quoteCurrency, true);
       }
     });
 
-    const sanitySwapPromises: Promise<void>[] = [];
+    currenciesToVerify.forEach((_, currency) => {
+      if (!this.swaps.swapClientManager.hasRouteToPeer(currency, peer)) {
+        currenciesToVerify.set(currency, false);
+      }
+    });
 
-    // Set a time limit for all sanity swaps to complete.
-    const sanitySwapTimeout = setTimeoutPromise(OrderBook.MAX_SANITY_SWAP_TIME, false);
+    if (!this.nosanityswaps) {
+      const sanitySwapPromises: Promise<void>[] = [];
+
+      // Set a time limit for all sanity swaps to complete.
+      const sanitySwapTimeout = setTimeoutPromise(OrderBook.MAX_SANITY_SWAP_TIME, false);
+
+      currenciesToVerify.forEach((swappable, currency) => {
+        if (swappable && this.currencyInstances.has(currency)) {
+          // perform sanity swaps for each of the currencies that we support
+          const sanitySwapPromise = new Promise<void>(async (resolve) => {
+            // success resolves to true if the sanity swap succeeds before the timeout
+            const success = await Promise.race([this.swaps.executeSanitySwap(currency, peer), sanitySwapTimeout]);
+            if (!success) {
+              currenciesToVerify.set(currency, false);
+            }
+            resolve();
+          });
+          sanitySwapPromises.push(sanitySwapPromise);
+        }
+      });
 
-    currenciesToVerify.forEach((currency) => {
-      if (this.currencyInstances.has(currency)) {
-        // perform sanity swaps for each of the currencies that we support
-        const sanitySwapPromise = new Promise<void>(async (resolve) => {
-          // success resolves to true if the sanity swap succeeds before the timeout
-          const success = await Promise.race([this.swaps.executeSanitySwap(currency, peer), sanitySwapTimeout]);
-          if (success) {
-            peer.verifiedCurrencies.add(currency);
-          }
-          resolve();
-        });
-        sanitySwapPromises.push(sanitySwapPromise);
+      // wait for all sanity swaps to finish or timeout
+      await Promise.all(sanitySwapPromises);
+    }
+
+    // activate verified currencies
+    currenciesToVerify.forEach((swappable, currency) => {
+      if (swappable) {
+        peer.activateCurrency(currency);
       }
     });
 
-    // wait for all sanity swaps to finish or timeout
-    await Promise.all(sanitySwapPromises);
-
-    // activate pairs that have had both currencies verified
+    // activate pairs that have both currencies active
     const activationPromises: Promise<void>[] = [];
-    pairIdsToVerify.forEach(async (pairId) => {
+    pairIdsToVerify.forEach((pairId) => {
       const [baseCurrency, quoteCurrency] = pairId.split('/');
-      if (peer.verifiedCurrencies.has(baseCurrency) && peer.verifiedCurrencies.has(quoteCurrency)) {
+      if (peer.isCurrencyActive(baseCurrency) && peer.isCurrencyActive(quoteCurrency)) {
         activationPromises.push(peer.activatePair(pairId));
       }
     });
@@ -854,16 +866,6 @@ class OrderBook extends EventEmitter {
     await peer.sendOrders(outgoingOrders, reqId);
   }
 
-  /**
-   * Create an outgoing order and broadcast it to all peers.
-   */
-  private broadcastOrder = (order: OwnOrder) => {
-    if (this.swaps && this.swaps.isPairSupported(order.pairId)) {
-      const outgoingOrder = OrderBook.createOutgoingOrder(order);
-      this.pool.broadcastOrder(outgoingOrder);
-    }
-  }
-
   public stampOwnOrder = (order: OwnLimitOrder): OwnOrder  => {
     const id = uuidv1();
     // verify localId isn't duplicated. use global id if blank

lib/p2p/Peer.ts

@@ -63,17 +63,17 @@ class Peer extends EventEmitter {
   /** Timer to periodically call getNodes #402 */
   public discoverTimer?: NodeJS.Timer;
   /** Currencies that we have verified we can swap with this peer. */
-  public verifiedCurrencies = new Set<string>();
-  /**
-   * Currencies that we cannot swap because we are missing a swap client identifier or because the
-   * peer's token identifier for this currency does not match ours - for example this may happen
-   * because a peer is using a different raiden token contract address for a currency than we are.
-   */
   public disabledCurrencies = new Set<string>();
   /** Interval to check required responses from peer. */
   public static readonly STALL_INTERVAL = 5000;
   /** Trading pairs advertised by this peer which we have verified that we can swap. */
   private activePairs = new Set<string>();
+  private activeCurrencies = new Set<string>();
+  /**
+   * Currencies that we cannot swap because we are missing a swap client identifier or because the
+   * peer's token identifier for this currency does not match ours - for example this may happen
+   * because a peer is using a different raiden token contract address for a currency than we are.
+   */
   /** Whether we have received and authenticated a [[SessionInitPacket]] from the peer. */
   private opened = false;
   private opening = false;
@@ -384,17 +384,25 @@ class Peer extends EventEmitter {
     await this.sendPacket(packet);
   }
 
-  public disableCurrency = (currency: string) => {
-    if (!this.disabledCurrencies.has(currency)) {
-      this.disabledCurrencies.add(currency);
-      this.verifiedCurrencies.delete(currency);
+  public deactivateCurrency = (currency: string) => {
+    if (this.activeCurrencies.has(currency)) {
+      this.activeCurrencies.delete(currency);
       this.activePairs.forEach((activePairId) => {
         const [baseCurrency, quoteCurrency] = activePairId.split('/');
         if (baseCurrency === currency || quoteCurrency === currency) {
           this.deactivatePair(activePairId);
         }
       });
-      this.logger.debug(`disabled ${currency} for peer ${this.label}`);
+      this.logger.debug(`deactivated ${currency} for peer ${this.label}`);
+    }
+  }
+
+  public activateCurrency = (currency: string) => this.activeCurrencies.add(currency);
+
+  public disableCurrency = (currency: string) => {
+    if (!this.disabledCurrencies.has(currency)) {
+      this.disabledCurrencies.add(currency);
+      this.deactivateCurrency(currency);
     }
   }
 
@@ -431,6 +439,8 @@ class Peer extends EventEmitter {
 
   public isPairActive = (pairId: string) => this.activePairs.has(pairId);
 
+  public isCurrencyActive = (currency: string) => this.activeCurrencies.has(currency);
+
   /**
    * Gets lnd client's listening uris for the provided currency.
    * @param currency

lib/swaps/SwapClientManager.ts

@@ -2,6 +2,7 @@ import { EventEmitter } from 'events';
 import Config from '../Config';
 import { SwapClientType } from '../constants/enums';
 import { Models } from '../db/DB';
+import lndErrors from '../lndclient/errors';
 import LndClient from '../lndclient/LndClient';
 import { LndInfo } from '../lndclient/types';
 import { Loggers } from '../Logger';
@@ -12,7 +13,6 @@ import seedutil from '../utils/seedutil';
 import { UnitConverter } from '../utils/UnitConverter';
 import errors from './errors';
 import SwapClient, { ClientStatus } from './SwapClient';
-import lndErrors from '../lndclient/errors';
 
 export function isRaidenClient(swapClient: SwapClient): swapClient is RaidenClient {
   return (swapClient.type === SwapClientType.Raiden);
@@ -398,6 +398,27 @@ class SwapClientManager extends EventEmitter {
     await swapClient.openChannel({ peerIdentifier, units, currency });
   }
 
+  public hasRouteToPeer = (currency: string, peer: Peer) => {
+    const swapClient = this.get(currency);
+    if (!swapClient) {
+      return false;
+    }
+    let hasRoute = false;
+    if (isRaidenClient(swapClient)) {
+      // temporary check to ensure we have a direct channel
+      if (peer.raidenAddress && swapClient.getRoute(1, peer.raidenAddress, currency)) {
+        // a route found means we have a direct channel
+        hasRoute = true;
+      }
+    } else if (isLndClient(swapClient)) {
+      // lnd doesn't currently have a way to see if any route exists, regardless of balance
+      // for example, if we have a direct channel to peer but no balance in the channel and
+      // no other routes, QueryRoutes will return nothing as of lnd v0.8.1
+      hasRoute = true;
+    }
+    return hasRoute;
+  }
+
   private bind = () => {
     for (const [currency, swapClient] of this.swapClients.entries()) {
       if (isLndClient(swapClient)) {

lib/swaps/Swaps.ts

@@ -1079,7 +1079,7 @@ class Swaps extends EventEmitter {
         // something is wrong with swaps for this currency with this peer
         if (failedCurrency) {
           try {
-            this.pool.getPeer(deal.peerPubKey).disableCurrency(failedCurrency);
+            this.pool.getPeer(deal.peerPubKey).deactivateCurrency(failedCurrency);
           } catch (err) {
             this.logger.debug(`could not disable currency ${failedCurrency} for peer ${deal.peerPubKey}`);
           }

test/jest/Orderbook.spec.ts

@@ -44,6 +44,7 @@ jest.mock('../../lib/db/DB', () => {
 });
 const advertisedPairs = ['LTC/BTC', 'WETH/BTC'];
 const mockActivatePair = jest.fn();
+const mockActivateCurrency = jest.fn();
 const mockGetIdentifier = jest.fn(() => 'pubkeyoraddress');
 const tokenIdentifiers: any = {
   BTC: 'bitcoin-regtest',
@@ -52,11 +53,17 @@ const tokenIdentifiers: any = {
 const mockPeerGetTokenIdentifer = jest.fn((currency: string) => tokenIdentifiers[currency]);
 jest.mock('../../lib/p2p/Peer', () => {
   return jest.fn().mockImplementation(() => {
+    let currencyActive = false;
     return {
       advertisedPairs,
       activatePair: mockActivatePair,
+      activateCurrency: (currency: string) => {
+        mockActivateCurrency(currency);
+        currencyActive = true;
+      },
       disabledCurrencies: new Map(),
       isPairActive: () => false,
+      isCurrencyActive: () => currencyActive,
       getTokenIdentifier: mockPeerGetTokenIdentifer,
       getIdentifier: mockGetIdentifier,
     };
@@ -73,7 +80,14 @@ jest.mock('../../lib/p2p/Pool', () => {
   });
 });
 jest.mock('../../lib/swaps/Swaps');
-jest.mock('../../lib/swaps/SwapClientManager');
+jest.mock('../../lib/swaps/SwapClientManager', () => {
+  return jest.fn().mockImplementation(() => {
+    return {
+      // temporary mock while raiden direct channel checks are required
+      hasRouteToPeer: jest.fn().mockReturnValue(true),
+    };
+  });
+});
 jest.mock('../../lib/Logger');
 jest.mock('../../lib/nodekey/NodeKey');
 const mockedNodeKey = <jest.Mock<NodeKey>><any>NodeKey;
@@ -144,9 +158,13 @@ describe('OrderBook', () => {
   test('nosanityswaps enabled adds pairs and requests orders', async () => {
     orderbook['nosanityswaps'] = true;
     await orderbook['verifyPeerPairs'](peer);
+    expect(mockActivateCurrency).toHaveBeenCalledTimes(3);
+    expect(mockActivateCurrency).toHaveBeenCalledWith('BTC');
+    expect(mockActivateCurrency).toHaveBeenCalledWith('LTC');
+    expect(mockActivateCurrency).toHaveBeenCalledWith('WETH');
     expect(mockActivatePair).toHaveBeenCalledTimes(2);
-    expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[0], expect.any(Number), expect.anything());
-    expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[1], expect.any(Number), expect.anything());
+    expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[0]);
+    expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[1]);
   });
 
   test('isPeerCurrencySupported returns true for a known currency with matching identifiers', async () => {