Skip to content

Commit

Permalink
refactor(orderbook): make pool non-nullable
Browse files Browse the repository at this point in the history
This refactors the `OrderBook.pool` member field to be non-nullable.
Previously, it was nullable to accommodate automated tests. However,
this is not necessary by stubbing/mocking the `pool` field within tests,
which is done as part of this commit.
  • Loading branch information
sangaman committed May 31, 2019
1 parent e9e6c1e commit 5d3d14d
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 47 deletions.
66 changes: 28 additions & 38 deletions lib/orderbook/OrderBook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class OrderBook extends EventEmitter {
}

constructor(private logger: Logger, models: Models, public nomatching = false,
private pool?: Pool, private swaps?: Swaps, private nosanitychecks = false) {
private pool: Pool, private swaps?: Swaps, private nosanitychecks = false) {
super();

this.repository = new OrderBookRepository(models);
Expand All @@ -100,30 +100,28 @@ class OrderBook extends EventEmitter {
}

private bindPool = () => {
if (this.pool) {
this.pool.on('packet.order', this.addPeerOrder);
this.pool.on('packet.orderInvalidation', this.handleOrderInvalidation);
this.pool.on('packet.getOrders', this.sendOrders);
this.pool.on('packet.swapRequest', this.handleSwapRequest);
this.pool.on('peer.close', this.removePeerOrders);
this.pool.on('peer.pairDropped', this.removePeerPair);
this.pool.on('peer.pairsAdvertised', this.verifyPeerPairs);
this.pool.on('peer.nodeStateUpdate', (peer) => {
// remove any trading pairs for which we no longer have both swap client identifiers
peer.forEachActivePair((activePairId) => {
const [baseCurrency, quoteCurrency] = activePairId.split('/');
const isCurrencySupported = (currency: string) => {
const currencyAttributes = this.getCurrencyAttributes(currency);
return currencyAttributes && peer.getIdentifier(currencyAttributes.swapClient, currency);
};

if (!isCurrencySupported(baseCurrency) || !isCurrencySupported(quoteCurrency)) {
// this peer's node state no longer supports at least one of the currencies for this trading pair
peer.deactivatePair(activePairId);
}
});
this.pool.on('packet.order', this.addPeerOrder);
this.pool.on('packet.orderInvalidation', this.handleOrderInvalidation);
this.pool.on('packet.getOrders', this.sendOrders);
this.pool.on('packet.swapRequest', this.handleSwapRequest);
this.pool.on('peer.close', this.removePeerOrders);
this.pool.on('peer.pairDropped', this.removePeerPair);
this.pool.on('peer.pairsAdvertised', this.verifyPeerPairs);
this.pool.on('peer.nodeStateUpdate', (peer) => {
// remove any trading pairs for which we no longer have both swap client identifiers
peer.forEachActivePair((activePairId) => {
const [baseCurrency, quoteCurrency] = activePairId.split('/');
const isCurrencySupported = (currency: string) => {
const currencyAttributes = this.getCurrencyAttributes(currency);
return currencyAttributes && peer.getIdentifier(currencyAttributes.swapClient, currency);
};

if (!isCurrencySupported(baseCurrency) || !isCurrencySupported(quoteCurrency)) {
// this peer's node state no longer supports at least one of the currencies for this trading pair
peer.deactivatePair(activePairId);
}
});
}
});
}

private bindSwaps = () => {
Expand Down Expand Up @@ -232,9 +230,7 @@ class OrderBook extends EventEmitter {
this.pairInstances.set(pairInstance.id, pairInstance);
this.tradingPairs.set(pairInstance.id, new TradingPair(this.logger, pairInstance.id, this.nomatching));

if (this.pool) {
this.pool.updatePairs(this.pairIds);
}
this.pool.updatePairs(this.pairIds);
return pairInstance;
}

Expand Down Expand Up @@ -275,9 +271,7 @@ class OrderBook extends EventEmitter {
this.pairInstances.delete(pairId);
this.tradingPairs.delete(pairId);

if (this.pool) {
this.pool.updatePairs(this.pairIds);
}
this.pool.updatePairs(this.pairIds);
return pair.destroy();
}

Expand Down Expand Up @@ -641,9 +635,7 @@ class OrderBook extends EventEmitter {
this.localIdMap.delete(removeResult.order.localId);
}

if (this.pool) {
this.pool.broadcastOrderInvalidation(removeResult.order, takerPubKey);
}
this.pool.broadcastOrderInvalidation(removeResult.order, takerPubKey);
return removeResult.order;
} catch (err) {
if (quantityToRemove !== undefined) {
Expand Down Expand Up @@ -771,11 +763,9 @@ class OrderBook extends EventEmitter {
* Create an outgoing order and broadcast it to all peers.
*/
private broadcastOrder = (order: OwnOrder) => {
if (this.pool) {
if (this.swaps && this.swaps.isPairSupported(order.pairId)) {
const outgoingOrder = OrderBook.createOutgoingOrder(order);
this.pool.broadcastOrder(outgoingOrder);
}
if (this.swaps && this.swaps.isPairSupported(order.pairId)) {
const outgoingOrder = OrderBook.createOutgoingOrder(order);
this.pool.broadcastOrder(outgoingOrder);
}
}

Expand Down
18 changes: 9 additions & 9 deletions test/integration/OrderBook.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import Logger, { Level } from '../../lib/Logger';
import * as orders from '../../lib/orderbook/types';
import { SwapClientType } from '../../lib/constants/enums';
import { createOwnOrder } from '../utils';
import sinon, { SinonSandbox } from 'sinon';
import sinon from 'sinon';

const PAIR_ID = 'LTC/BTC';
const currencies = PAIR_ID.split('/');
Expand All @@ -30,24 +30,24 @@ const initValues = async (db: DB) => {
]);
};

const sandbox = sinon.createSandbox();
const pool = sandbox.createStubInstance(Pool) as any;
pool.broadcastOrder = () => {};
pool.broadcastOrderInvalidation = () => {};
pool.updatePairs = () => {};

describe('OrderBook', () => {
let db: DB;
let pool: Pool;
let swaps: Swaps;
let orderBook: OrderBook;
let sandbox: SinonSandbox;

before(async () => {
db = new DB(loggers.db);
await db.init();

await initValues(db);

sandbox = sinon.createSandbox();
pool = sandbox.createStubInstance(Pool) as any;
pool.broadcastOrder = () => {};
pool.broadcastOrderInvalidation = () => {};
swaps = sandbox.createStubInstance(Swaps) as any;
swaps = sandbox.createStubInstance(Swaps) as any;;
swaps.isPairSupported = () => true;
const lndBTC = sandbox.createStubInstance(LndClient) as any;
const lndLTC = sandbox.createStubInstance(LndClient) as any;
Expand Down Expand Up @@ -162,7 +162,7 @@ describe('nomatching OrderBook', () => {
});

beforeEach(async () => {
orderBook = new OrderBook(loggers.orderbook, db.models, true);
orderBook = new OrderBook(loggers.orderbook, db.models, true, pool);
await orderBook.init();
});

Expand Down

0 comments on commit 5d3d14d

Please sign in to comment.