Skip to content

Commit

Permalink
fix: don't activate unsupported pairs with peers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sangaman committed Sep 11, 2020
1 parent 62b9312 commit 0fe2d66
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
9 changes: 8 additions & 1 deletion lib/orderbook/OrderBook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
});
Expand Down
9 changes: 9 additions & 0 deletions lib/swaps/SwapClientManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 2 additions & 4 deletions test/jest/Orderbook.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down

0 comments on commit 0fe2d66

Please sign in to comment.