Skip to content

Commit

Permalink
feat(orderbook): automatically remove dust orders
Browse files Browse the repository at this point in the history 
This removes dust orders from the order book as they arise due to orders
being partially removed or matched. Dust orders are defined as orders
where either side of the trade has less than 100 satoshis of volume.

This logic exists in three places. During matching, we check whether the
remainder of any maker order is beneath the dust limit. During partial
order removal, we check whether the remainder is beneath the dust limit
and, if so, remove the entire order instead. Lastly, any remaining order
after matching is checked against the dust limit as well.

Closes #1798.

Follow-up to #1785.
  • Loading branch information
@sangaman
sangaman committed Aug 20, 2020
1 parent de691b2 commit 0d8435f
Show file tree
Hide file tree
Showing 9 changed files with 295 additions and 125 deletions.
48 changes: 38 additions & 10 deletions lib/orderbook/OrderBook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class OrderBook extends EventEmitter {
currencies.forEach(currency => this.currencyInstances.set(currency.id, currency));
pairs.forEach((pair) => {
this.pairInstances.set(pair.id, pair);
this.tradingPairs.set(pair.id, new TradingPair(this.logger, pair.id, this.nomatching));
this.addTradingPair(pair.id);
});

this.pool.updatePairs(this.pairIds);
Expand Down Expand Up @@ -286,12 +286,27 @@ class OrderBook extends EventEmitter {

const pairInstance = await this.repository.addPair(pair);
this.pairInstances.set(pairInstance.id, pairInstance);
this.tradingPairs.set(pairInstance.id, new TradingPair(this.logger, pairInstance.id, this.nomatching));
this.addTradingPair(pairInstance.id);

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

private addTradingPair = (pairId: string) => {
const tp = new TradingPair(this.logger, pairId, this.nomatching);
this.tradingPairs.set(pairId, tp);
tp.on('peerOrder.dust', (order) => {
this.removePeerOrder(order.id, order.pairId);
});
tp.on('ownOrder.dust', (order) => {
this.removeOwnOrder({
orderId: order.id,
pairId: order.pairId,
quantityToRemove: order.quantity,
});
});
}

public addCurrency = async (currency: CurrencyCreationAttributes) => {
if (this.currencyInstances.has(currency.id)) {
throw errors.CURRENCY_ALREADY_EXISTS(currency.id);
Expand Down Expand Up @@ -342,9 +357,13 @@ class OrderBook extends EventEmitter {
}): Promise<PlaceOrderResult> => {
const stampedOrder = this.stampOwnOrder(order, replaceOrderId);

if (order.quantity * order.price < 1) {
if (order.quantity < TradingPair.QUANTITY_DUST_LIMIT) {
const baseCurrency = order.pairId.split('/')[0];
throw errors.MIN_QUANTITY_VIOLATED(TradingPair.QUANTITY_DUST_LIMIT, baseCurrency);
}
if (order.quantity * order.price < TradingPair.QUANTITY_DUST_LIMIT) {
const quoteCurrency = order.pairId.split('/')[1];
throw errors.MIN_QUANTITY_VIOLATED(1, quoteCurrency);
throw errors.MIN_QUANTITY_VIOLATED(TradingPair.QUANTITY_DUST_LIMIT, quoteCurrency);
}

if (this.nomatching) {
Expand Down Expand Up @@ -376,6 +395,11 @@ class OrderBook extends EventEmitter {
throw errors.MARKET_ORDERS_NOT_ALLOWED();
}

if (order.quantity < TradingPair.QUANTITY_DUST_LIMIT) {
const baseCurrency = order.pairId.split('/')[0];
throw errors.MIN_QUANTITY_VIOLATED(TradingPair.QUANTITY_DUST_LIMIT, baseCurrency);
}

const stampedOrder = this.stampOwnOrder({ ...order, price: order.isBuy ? Number.POSITIVE_INFINITY : 0 });
const addResult = await this.placeOrder({
onUpdate,
Expand Down Expand Up @@ -609,8 +633,14 @@ class OrderBook extends EventEmitter {
// instead we preserve the remainder and return it to the parent caller, which will sum
// up any remaining orders and add them to the order book as a single order once
// matching is complete
this.addOwnOrder(remainingOrder, replacedOrderIdentifier?.id);
onUpdate && onUpdate({ type: PlaceOrderEventType.RemainingOrder, order: remainingOrder });
if (remainingOrder.quantity < TradingPair.QUANTITY_DUST_LIMIT ||
remainingOrder.quantity * remainingOrder.price < TradingPair.QUANTITY_DUST_LIMIT) {
remainingOrder = undefined;
this.logger.verbose(`remainder for order ${order.id} does not meet dust limit and will be discarded`);
} else {
this.addOwnOrder(remainingOrder, replacedOrderIdentifier?.id);
onUpdate && onUpdate({ type: PlaceOrderEventType.RemainingOrder, order: remainingOrder });
}
}
} else if (replacedOrderIdentifier) {
// we tried to replace an order but the replacement order was fully matched, so simply remove the original order
Expand All @@ -637,8 +667,7 @@ class OrderBook extends EventEmitter {
}

/**
* Executes a swap between maker and taker orders. Emits the `peerOrder.filled` event if the swap
* succeeds and `peerOrder.invalidation` if the swap fails.
* Executes a swap between maker and taker orders. Emits the `peerOrder.filled` event if the swap succeeds.
* @returns A promise that resolves to a [[SwapSuccess]] once the swap is completed, throws a [[SwapFailureReason]] if it fails
*/
public executeSwap = async (maker: PeerOrder, taker: OwnOrder): Promise<SwapSuccess> => {
Expand All @@ -661,7 +690,6 @@ class OrderBook extends EventEmitter {
return swapResult;
} catch (err) {
const failureReason: number = err;
this.emit('peerOrder.invalidation', maker);
this.logger.error(`swap between orders ${maker.id} & ${taker.id} failed due to ${SwapFailureReason[failureReason]}`);
throw failureReason;
}
Expand Down Expand Up @@ -735,7 +763,7 @@ class OrderBook extends EventEmitter {
}

// TODO: penalize peers for sending ordes too small to swap?
if (order.quantity * order.price < 1) {
if (order.quantity * order.price < TradingPair.QUANTITY_DUST_LIMIT) {
this.logger.warn('incoming peer order is too small to swap');
return false;
}
Expand Down
97 changes: 71 additions & 26 deletions lib/orderbook/TradingPair.ts
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import assert from 'assert';
import { EventEmitter } from 'events';
import FastPriorityQueue from 'fastpriorityqueue';
import { OrderingDirection } from '../constants/enums';
import Logger from '../Logger';
import errors from './errors';
import { isOwnOrder, MatchingResult, Order, OrderMatch, OwnOrder, PeerOrder } from './types';
import { isOwnOrder, MatchingResult, Order, OrderMatch, OrderPortion, OwnOrder, PeerOrder } from './types';

/** A map between orders and their order ids. */
type OrderMap<T extends Order> = Map<string, T>;
Expand All @@ -23,19 +24,36 @@ type OrderSidesQueues = {
sellQueue: FastPriorityQueue<Order>,
};

interface TradingPair {
/** Adds a listener to be called when all or part of a remote order was removed due to not meeting dust minimum. */
on(event: 'peerOrder.dust', listener: (order: OrderPortion) => void): this;
/** Adds a listener to be called when all or part of a local order was removed due to not meeting dust minimum. */
on(event: 'ownOrder.dust', listener: (order: OrderPortion) => void): this;

/** Notifies listeners that a remote order was removed due to not meeting dust minimum. */
emit(event: 'peerOrder.dust', order: OrderPortion): boolean;
/** Notifies listeners that a local order was removed due to not meeting dust minimum. */
emit(event: 'ownOrder.dust', order: OrderPortion): boolean;
}

/**
* Represents a single trading pair in the order book. Responsible for managing all active orders
* and for matching orders according to their price and quantity.
*/
class TradingPair {
class TradingPair extends EventEmitter {
/** A pair of priority queues for the buy and sell sides of this trading pair */
public queues?: OrderSidesQueues;
/** A pair of maps between active own orders ids and orders for the buy and sell sides of this trading pair. */
public ownOrders: OrderSidesMaps<OwnOrder>;
/** A map between peerPubKey and a pair of maps between active peer orders ids and orders for the buy and sell sides of this trading pair. */
public peersOrders: Map<string, OrderSidesMaps<PeerOrder>>;

/** The minimum quantity for both sides of a trade that is considered swappable and not dust. */
public static QUANTITY_DUST_LIMIT = 100;

constructor(private logger: Logger, public pairId: string, private nomatching = false) {
super();

if (!nomatching) {
this.queues = {
buyQueue: TradingPair.createPriorityQueue(OrderingDirection.Desc),
Expand Down Expand Up @@ -224,29 +242,36 @@ class TradingPair {
}

if (quantityToRemove && quantityToRemove < order.quantity) {
// if quantityToRemove is below the order quantity, reduce the order quantity
if (isOwnOrder(order)) {
assert(quantityToRemove <= order.quantity - order.hold, 'cannot remove more than available quantity after holds');
}
order.quantity = order.quantity - quantityToRemove;
this.logger.trace(`order quantity reduced by ${quantityToRemove}: ${orderId}`);
return { order: { ...order, quantity: quantityToRemove } as T, fullyRemoved: false } ;
} else {
// otherwise, remove the order entirely
if (isOwnOrder(order)) {
assert(order.hold === 0, 'cannot remove an order with a hold');
const remainingQuantity = order.quantity - quantityToRemove;
if (remainingQuantity < TradingPair.QUANTITY_DUST_LIMIT ||
(remainingQuantity * order.price) < TradingPair.QUANTITY_DUST_LIMIT) {
// the remaining quantity doesn't meet the dust limit, so we remove the entire order
this.logger.trace(`removing entire order ${orderId} because remaining quantity does not meet dust limit`);
} else {
// if quantityToRemove is below the order quantity but above dust limit, reduce the order quantity
if (isOwnOrder(order)) {
assert(quantityToRemove <= order.quantity - order.hold, 'cannot remove more than available quantity after holds');
}
order.quantity = order.quantity - quantityToRemove;
this.logger.trace(`order quantity reduced by ${quantityToRemove}: ${orderId}`);
return { order: { ...order, quantity: quantityToRemove } as T, fullyRemoved: false } ;
}
const map = order.isBuy ? maps.buyMap : maps.sellMap;
map.delete(order.id);
}

if (!this.nomatching) {
const queue = order.isBuy ? this.queues!.buyQueue : this.queues!.sellQueue;
queue.remove(order);
}
// otherwise, remove the order entirely
if (isOwnOrder(order)) {
assert(order.hold === 0, 'cannot remove an order with a hold');
}
const map = order.isBuy ? maps.buyMap : maps.sellMap;
map.delete(order.id);

this.logger.trace(`order removed: ${orderId}`);
return { order: order as T, fullyRemoved: true };
if (!this.nomatching) {
const queue = order.isBuy ? this.queues!.buyQueue : this.queues!.sellQueue;
queue.remove(order);
}

this.logger.trace(`order removed: ${orderId}`);
return { order: order as T, fullyRemoved: true };
}

private getOrderMap = (order: Order): OrderMap<Order> | undefined => {
Expand Down Expand Up @@ -365,11 +390,17 @@ class TradingPair {
: makerOrder;

const matchingQuantity = getMatchingQuantity(remainingOrder, makerAvailableQuantityOrder);
if (matchingQuantity <= 0) {
// there's no match with the best available maker order, so end the matching routine
break;
} else if (makerOrder.quantity * makerOrder.price < 1) {
// there's a match but it doesn't meet the 1 satoshi minimum on both sides of the trade
if (matchingQuantity * makerOrder.price < TradingPair.QUANTITY_DUST_LIMIT) {
// there's no match with the best available maker order OR there's a match
// but it doesn't meet the dust minimum on both sides of the trade
if (isOwnOrder(makerOrder) && makerOrder.hold > 0) {
// part of this order is on hold, so put it aside and try to match the next order
assert(queue.poll() === makerOrder);
queueRemovedOrdersWithHold.push(makerOrder);
} else {
// there's no hold, so end the matching routine
break;
}
break;
} else {
/** Whether the maker order is fully matched and should be removed from the queue. */
Expand Down Expand Up @@ -409,6 +440,20 @@ class TradingPair {

assert(queue.poll() === makerOrder);
queueRemovedOrdersWithHold.push(makerOrder);
} else {
// we must make sure that we don't leave an order that is too small to swap in the order book
const makerLeftoverAvailableQuantity = isOwnOrder(makerOrder)
? makerOrder.quantity - makerOrder.hold
: makerOrder.quantity;

if (makerLeftoverAvailableQuantity < TradingPair.QUANTITY_DUST_LIMIT ||
(makerLeftoverAvailableQuantity * makerOrder.price < TradingPair.QUANTITY_DUST_LIMIT)) {
if (isOwnOrder(makerOrder)) {
this.emit('ownOrder.dust', { ...makerOrder, quantity: makerLeftoverAvailableQuantity });
} else {
this.emit('peerOrder.dust', makerOrder);
}
}
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions lib/service/Service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const argChecks = {
HAS_PAIR_ID: ({ pairId }: { pairId: string }) => { if (pairId === '') throw errors.INVALID_ARGUMENT('pairId must be specified'); },
HAS_RHASH: ({ rHash }: { rHash: string }) => { if (rHash === '') throw errors.INVALID_ARGUMENT('rHash must be specified'); },
POSITIVE_AMOUNT: ({ amount }: { amount: number }) => { if (amount <= 0) throw errors.INVALID_ARGUMENT('amount must be greater than 0'); },
POSITIVE_QUANTITY: ({ quantity }: { quantity: number }) => { if (quantity <= 0) throw errors.INVALID_ARGUMENT('quantity must be greater than 0'); },
PRICE_NON_NEGATIVE: ({ price }: { price: number }) => { if (price < 0) throw errors.INVALID_ARGUMENT('price cannot be negative'); },
PRICE_MAX_DECIMAL_PLACES: ({ price }: { price: number }) => {
if (checkDecimalPlaces(price)) throw errors.INVALID_ARGUMENT('price cannot have more than 12 decimal places');
Expand Down Expand Up @@ -615,7 +614,6 @@ class Service {
callback?: (e: ServicePlaceOrderEvent) => void,
) => {
argChecks.PRICE_NON_NEGATIVE(args);
argChecks.POSITIVE_QUANTITY(args);
argChecks.PRICE_MAX_DECIMAL_PLACES(args);
argChecks.HAS_PAIR_ID(args);
const { pairId, price, quantity, orderId, side, replaceOrderId, immediateOrCancel } = args;
Expand Down
26 changes: 13 additions & 13 deletions test/integration/OrderBook.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ describe('OrderBook', () => {
});

it('should not add a new own order with a duplicated localId', async () => {
const order: orders.OwnOrder = createOwnOrder(0.01, 1000, false);
const order: orders.OwnOrder = createOwnOrder(0.01, 100000, false);

await expect(orderBook.placeLimitOrder({ order })).to.be.fulfilled;

Expand Down Expand Up @@ -221,71 +221,71 @@ describe('nomatching OrderBook', () => {
});

it('should accept but not match limit orders', async () => {
const buyOrder = createOwnOrder(0.01, 1000, true);
const buyOrder = createOwnOrder(0.01, 100000, true);
const buyOrderResult = await orderBook.placeLimitOrder({ order: buyOrder });
expect(buyOrderResult.remainingOrder!.localId).to.be.equal(buyOrder.localId);
expect(buyOrderResult.remainingOrder!.quantity).to.be.equal(buyOrder.quantity);

const sellOrder = createOwnOrder(0.01, 1000, false);
const sellOrder = createOwnOrder(0.01, 100000, false);
const sellOrderResult = await orderBook.placeLimitOrder({ order: sellOrder });
expect(sellOrderResult.remainingOrder!.localId).to.be.equal(sellOrder.localId);
expect(sellOrderResult.remainingOrder!.quantity).to.be.equal(sellOrder.quantity);
});

it('should not place the same order twice', async () => {
const order = createOwnOrder(0.01, 1000, true);
const order = createOwnOrder(0.01, 100000, true);
await expect(orderBook.placeLimitOrder({ order })).to.be.fulfilled;
await expect(orderBook.placeLimitOrder({ order })).to.be.rejected;
});

it('should not remove the same order twice', async () => {
const order = createOwnOrder(0.01, 1000, true);
const order = createOwnOrder(0.01, 100000, true);
await expect(orderBook.placeLimitOrder({ order })).to.be.fulfilled;
expect(() => orderBook.removeOwnOrderByLocalId(order.localId)).to.not.throw();
expect(() => orderBook.removeOwnOrderByLocalId(order.localId)).to.throw();
});

it('should allow own order partial removal, but should not find the order localId after it was fully removed', async () => {
const order = createOwnOrder(0.01, 1000, true);
const order = createOwnOrder(0.01, 100000, true);
const { remainingOrder } = await orderBook.placeLimitOrder({ order });

orderBook['removeOwnOrder']({
orderId: remainingOrder!.id,
pairId: order.pairId,
quantityToRemove: remainingOrder!.quantity - 100,
quantityToRemove: remainingOrder!.quantity - 10000,
});
orderBook['removeOwnOrder']({
orderId: remainingOrder!.id,
pairId: order.pairId,
quantityToRemove: 100,
quantityToRemove: 10000,
});

expect(() => orderBook['removeOwnOrder']({
orderId: remainingOrder!.id,
pairId: order.pairId,
quantityToRemove: 100,
quantityToRemove: 10000,
})).to.throw;
});

it('should allow own order partial removal, but should not find the order id after it was fully removed', async () => {
const order = createOwnOrder(0.01, 1000, true);
const order = createOwnOrder(0.01, 100000, true);
const { remainingOrder } = await orderBook.placeLimitOrder({ order });

orderBook['removeOwnOrder']({
orderId: remainingOrder!.id,
pairId: order.pairId,
quantityToRemove: remainingOrder!.quantity - 100,
quantityToRemove: remainingOrder!.quantity - 10000,
});
orderBook['removeOwnOrder']({
orderId: remainingOrder!.id,
pairId: order.pairId,
quantityToRemove: 100,
quantityToRemove: 10000,
});

expect(() => orderBook['removeOwnOrder']({
orderId: remainingOrder!.id,
pairId: order.pairId,
quantityToRemove: 100,
quantityToRemove: 10000,
})).to.throw;
});

Expand Down
4 changes: 2 additions & 2 deletions test/integration/Service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ describe('API Service', () => {
const placeOrderArgs = {
pairId,
orderId: '1',
price: 100,
quantity: 1,
price: 1,
quantity: 100,
side: OrderSide.Buy,
immediateOrCancel: false,
replaceOrderId: '',
Expand Down
Loading

0 comments on commit 0d8435f

Please sign in to comment.