Skip to content

Commit

Permalink
Merge pull request #1821 from ExchangeUnion/dust-order-removal
Browse files Browse the repository at this point in the history 
feat(orderbook): automatically remove dust orders
  • Loading branch information
@sangaman
sangaman authored Aug 25, 2020
2 parents a2b766e + 0d8435f commit aa41615
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 @@ -612,8 +636,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 @@ -640,8 +670,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 @@ -664,7 +693,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 @@ -738,7 +766,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 @@ -610,7 +609,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 aa41615

Please sign in to comment.