From 0fe2d66d85c2fcc2d559740662977750ddae21e1 Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Thu, 10 Sep 2020 01:55:41 -0400 Subject: [PATCH] fix: don't activate unsupported pairs with peers This ensures that, even when not running in strict mode, we only activate trading pairs with peers when we support both currencies of the pair and the trading pair itself locally. Previously we would activate every trading pair, like BTC/LTC when we don't have a swap client for LTC, resulting in us requesting and receiving order updates for trading pairs that are irrelevent to us. --- lib/orderbook/OrderBook.ts | 9 ++++++++- lib/swaps/SwapClientManager.ts | 9 +++++++++ test/jest/Orderbook.spec.ts | 6 ++---- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/orderbook/OrderBook.ts b/lib/orderbook/OrderBook.ts index d22e343f3..6e500f12e 100644 --- a/lib/orderbook/OrderBook.ts +++ b/lib/orderbook/OrderBook.ts @@ -994,6 +994,11 @@ class OrderBook extends EventEmitter { if (peer.isPairActive(pairId)) { return false; // don't verify a pair that is already active } + if (!this.tradingPairs.has(pairId)) { + // if we don't support the trading pair locally, then we don't want to verify or activate it + return false; + } + const [baseCurrency, quoteCurrency] = pairId.split('/'); const peerCurrenciesEnabled = !peer.disabledCurrencies.has(baseCurrency) && !peer.disabledCurrencies.has(quoteCurrency); @@ -1050,7 +1055,9 @@ class OrderBook extends EventEmitter { // activate verified currencies currenciesToVerify.forEach((swappable, currency) => { - if (swappable || !this.strict) { // always activate currencies if not in strict mode + // in strict mode, we only activate "swappable" currencies where a route to peer is possible or a sanity swap has completed + // in non-strict mode, we activate any currency which we also support locally + if (swappable || (!this.strict && this.swaps.swapClientManager.has(currency))) { peer.activateCurrency(currency); } }); diff --git a/lib/swaps/SwapClientManager.ts b/lib/swaps/SwapClientManager.ts index d5c1b20b2..e705c24c7 100644 --- a/lib/swaps/SwapClientManager.ts +++ b/lib/swaps/SwapClientManager.ts @@ -247,6 +247,15 @@ class SwapClientManager extends EventEmitter { return this.swapClients.get(currency); } + /** + * Returns whether the swap client manager has a client for a given currency. + * @param currency the currency that the swap client is linked to. + * @returns `true` if a swap client exists, false otherwise. + */ + public has = (currency: string): boolean => { + return this.swapClients.has(currency); + } + /** Gets the type of swap client for a given currency. */ public getType = (currency: string) => { return this.swapClients.get(currency)?.type; diff --git a/test/jest/Orderbook.spec.ts b/test/jest/Orderbook.spec.ts index e535cc2b3..51b395615 100644 --- a/test/jest/Orderbook.spec.ts +++ b/test/jest/Orderbook.spec.ts @@ -175,13 +175,11 @@ describe('OrderBook', () => { test('nosanityswaps enabled adds pairs and requests orders', async () => { orderbook['nosanityswaps'] = true; await orderbook['verifyPeerPairs'](peer); - expect(mockActivateCurrency).toHaveBeenCalledTimes(3); + expect(mockActivateCurrency).toHaveBeenCalledTimes(2); expect(mockActivateCurrency).toHaveBeenCalledWith('BTC'); expect(mockActivateCurrency).toHaveBeenCalledWith('LTC'); - expect(mockActivateCurrency).toHaveBeenCalledWith('WETH'); - expect(mockActivatePair).toHaveBeenCalledTimes(2); + expect(mockActivatePair).toHaveBeenCalledTimes(1); expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[0]); - expect(mockActivatePair).toHaveBeenCalledWith(advertisedPairs[1]); }); test('isPeerCurrencySupported returns true for a known currency with matching identifiers', async () => {