From 0d8435f38f5facb673978cc0e09d553e478a7696 Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Tue, 18 Aug 2020 14:20:21 -0400 Subject: [PATCH] feat(orderbook): automatically remove dust orders 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. --- lib/orderbook/OrderBook.ts | 48 ++++++++--- lib/orderbook/TradingPair.ts | 97 +++++++++++++++------ lib/service/Service.ts | 2 - test/integration/OrderBook.spec.ts | 26 +++--- test/integration/Service.spec.ts | 4 +- test/jest/Orderbook.spec.ts | 27 ++++++ test/simulation/tests-instability.go | 16 ++-- test/simulation/tests-integration.go | 76 ++++++++++++++-- test/unit/TradingPair.spec.ts | 124 +++++++++++++++------------ 9 files changed, 295 insertions(+), 125 deletions(-) diff --git a/lib/orderbook/OrderBook.ts b/lib/orderbook/OrderBook.ts index b75ee4fd5..744be8db6 100644 --- a/lib/orderbook/OrderBook.ts +++ b/lib/orderbook/OrderBook.ts @@ -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); @@ -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); @@ -342,9 +357,13 @@ class OrderBook extends EventEmitter { }): Promise => { 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) { @@ -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, @@ -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 @@ -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 => { @@ -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; } @@ -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; } diff --git a/lib/orderbook/TradingPair.ts b/lib/orderbook/TradingPair.ts index 9b8e516db..35d9b9d62 100644 --- a/lib/orderbook/TradingPair.ts +++ b/lib/orderbook/TradingPair.ts @@ -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 = Map; @@ -23,11 +24,23 @@ type OrderSidesQueues = { sellQueue: FastPriorityQueue, }; +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. */ @@ -35,7 +48,12 @@ class TradingPair { /** 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>; + /** 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), @@ -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 | undefined => { @@ -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. */ @@ -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); + } + } } } } diff --git a/lib/service/Service.ts b/lib/service/Service.ts index 8575fac4b..b851db7a4 100644 --- a/lib/service/Service.ts +++ b/lib/service/Service.ts @@ -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'); @@ -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; diff --git a/test/integration/OrderBook.spec.ts b/test/integration/OrderBook.spec.ts index d76ee7374..c092e279a 100644 --- a/test/integration/OrderBook.spec.ts +++ b/test/integration/OrderBook.spec.ts @@ -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; @@ -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; }); diff --git a/test/integration/Service.spec.ts b/test/integration/Service.spec.ts index 3b3478ad5..f8bd9c6de 100644 --- a/test/integration/Service.spec.ts +++ b/test/integration/Service.spec.ts @@ -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: '', diff --git a/test/jest/Orderbook.spec.ts b/test/jest/Orderbook.spec.ts index c2822566d..1ad1fefc3 100644 --- a/test/jest/Orderbook.spec.ts +++ b/test/jest/Orderbook.spec.ts @@ -310,4 +310,31 @@ describe('OrderBook', () => { replaceOrderId: oldOrder.remainingOrder!.id, }); }); + + test('removeOwnOrder removes entire order if dust would have remained', async (done) => { + const quantity = 10000; + const order: OwnLimitOrder = { + quantity, + pairId, + localId, + price: 0.01, + isBuy: false, + }; + const { remainingOrder } = await orderbook.placeLimitOrder({ order }); + expect(remainingOrder!.quantity).toEqual(quantity); + + orderbook.on('ownOrder.removed', (orderPortion) => { + expect(orderPortion.quantity).toEqual(quantity); + expect(orderPortion.id).toEqual(remainingOrder!.id); + expect(orderPortion.pairId).toEqual(pairId); + done(); + }); + + const removedOrder = orderbook['removeOwnOrder']({ + pairId, + orderId: remainingOrder!.id, + quantityToRemove: quantity - 1, + }); + expect(removedOrder.quantity).toEqual(quantity); + }); }); diff --git a/test/simulation/tests-instability.go b/test/simulation/tests-instability.go index 9c026eec2..e2e8d4f4f 100644 --- a/test/simulation/tests-instability.go +++ b/test/simulation/tests-instability.go @@ -136,7 +136,7 @@ func testMakerCrashedDuringSwapConnextIn(net *xudtest.NetworkHarness, ht *harnes ht.act.connect(net.Alice, net.Bob) ht.act.verifyConnectivity(net.Alice, net.Bob) - err = openETHChannel(ht.ctx, net.Bob, 400, 0) + err = openETHChannel(ht.ctx, net.Bob, 40000, 0) ht.assert.NoError(err) // Save the initial balances. @@ -148,7 +148,7 @@ func testMakerCrashedDuringSwapConnextIn(net *xudtest.NetworkHarness, ht *harnes aliceOrderReq := &xudrpc.PlaceOrderRequest{ OrderId: "maker_order_id", Price: 40, - Quantity: 1, + Quantity: 100, PairId: "BTC/ETH", Side: xudrpc.OrderSide_SELL, } @@ -265,7 +265,7 @@ func testMakerConnextClientCrashedBeforeSettlement(net *xudtest.NetworkHarness, ht.act.connect(net.Alice, net.Bob) ht.act.verifyConnectivity(net.Alice, net.Bob) - err = openETHChannel(ht.ctx, net.Bob, 400, 0) + err = openETHChannel(ht.ctx, net.Bob, 40000, 0) ht.assert.NoError(err) // Save the initial balances. @@ -281,7 +281,7 @@ func testMakerConnextClientCrashedBeforeSettlement(net *xudtest.NetworkHarness, aliceOrderReq := &xudrpc.PlaceOrderRequest{ OrderId: "maker_order_id", Price: 40, - Quantity: 1, + Quantity: 100, PairId: "BTC/ETH", Side: xudrpc.OrderSide_SELL, } @@ -409,7 +409,7 @@ func testMakerCrashedAfterSendDelayedSettlementConnextOut(net *xudtest.NetworkHa ht.act.connect(net.Alice, net.Bob) ht.act.verifyConnectivity(net.Alice, net.Bob) - err = openETHChannel(ht.ctx, net.Alice, 400, 0) + err = openETHChannel(ht.ctx, net.Alice, 40000, 0) ht.assert.NoError(err) // Save the initial balances. @@ -425,7 +425,7 @@ func testMakerCrashedAfterSendDelayedSettlementConnextOut(net *xudtest.NetworkHa aliceOrderReq := &xudrpc.PlaceOrderRequest{ OrderId: "maker_order_id", Price: 40, - Quantity: 1, + Quantity: 100, PairId: "BTC/ETH", Side: xudrpc.OrderSide_BUY, } @@ -497,7 +497,7 @@ func testMakerCrashedAfterSendDelayedSettlementConnextIn(net *xudtest.NetworkHar ht.act.connect(net.Alice, net.Bob) ht.act.verifyConnectivity(net.Alice, net.Bob) - err = openETHChannel(ht.ctx, net.Bob, 400, 0) + err = openETHChannel(ht.ctx, net.Bob, 40000, 0) ht.assert.NoError(err) // Save the initial balances. @@ -513,7 +513,7 @@ func testMakerCrashedAfterSendDelayedSettlementConnextIn(net *xudtest.NetworkHar aliceOrderReq := &xudrpc.PlaceOrderRequest{ OrderId: "maker_order_id", Price: 40, - Quantity: 1, + Quantity: 100, PairId: "BTC/ETH", Side: xudrpc.OrderSide_SELL, } diff --git a/test/simulation/tests-integration.go b/test/simulation/tests-integration.go index 4a66b4dc7..1be0db9e1 100644 --- a/test/simulation/tests-integration.go +++ b/test/simulation/tests-integration.go @@ -23,6 +23,10 @@ var integrationTestCases = []*testCase{ name: "order matching and swap connext", test: testOrderMatchingAndSwapConnext, }, + { + name: "dust order discarded", + test: testDustOrderDiscarded, + }, { name: "order replacement", test: testOrderReplacement, @@ -256,6 +260,60 @@ func testOrderMatchingAndSwap(net *xudtest.NetworkHarness, ht *harnessTest) { ht.act.disconnect(net.Alice, net.Bob) } +func testDustOrderDiscarded(net *xudtest.NetworkHarness, ht *harnessTest) { + // Connect Alice to Bob. + ht.act.connect(net.Alice, net.Bob) + ht.act.verifyConnectivity(net.Alice, net.Bob) + + // Place an order on Alice. + req := &xudrpc.PlaceOrderRequest{ + OrderId: "maker_order_id", + Price: 0.02, + Quantity: 10000, + PairId: "LTC/BTC", + Side: xudrpc.OrderSide_BUY, + } + ht.act.placeOrderAndBroadcast(net.Alice, net.Bob, req) + + // Place a matching order on Bob. + req = &xudrpc.PlaceOrderRequest{ + OrderId: "taker_order_id", + Price: req.Price, + Quantity: 10099, + PairId: req.PairId, + Side: xudrpc.OrderSide_SELL, + } + + aliceOrderChan := subscribeOrders(ht.ctx, net.Alice) + res, err := net.Bob.Client.PlaceOrderSync(ht.ctx, req) + + // verify that there is no remaining order + ht.assert.NoError(err) + ht.assert.Len(res.InternalMatches, 0) + ht.assert.Len(res.SwapFailures, 0) + ht.assert.Len(res.SwapSuccesses, 1) + ht.assert.Nil(res.RemainingOrder) + + e := <-aliceOrderChan + ht.assert.NoError(e.err) + ht.assert.NotNil(e.orderUpdate) + orderRemoval := e.orderUpdate.GetOrderRemoval() + ht.assert.NotNil(orderRemoval) + ht.assert.Equal(req.PairId, orderRemoval.PairId) + ht.assert.True(orderRemoval.IsOwnOrder) + + // verify that the order books are empty + srcNodeCount, destNodeCount, err := getOrdersCount(ht.ctx, net.Alice, net.Bob) + ht.assert.NoError(err) + ht.assert.Equal(0, int(srcNodeCount.Own)) + ht.assert.Equal(0, int(srcNodeCount.Peer)) + ht.assert.Equal(0, int(destNodeCount.Own)) + ht.assert.Equal(0, int(destNodeCount.Peer)) + + // Cleanup. + ht.act.disconnect(net.Alice, net.Bob) +} + func testOrderReplacement(net *xudtest.NetworkHarness, ht *harnessTest) { // Connect Alice to Bob. ht.act.connect(net.Alice, net.Bob) @@ -402,20 +460,20 @@ func testOrderMatchingAndSwapConnext(net *xudtest.NetworkHarness, ht *harnessTes ht.assert.Equal(uint64(0), resBal.Balances["ETH"].ChannelBalance) // Open channel from Alice. - err = openETHChannel(ht.ctx, net.Alice, 400, 0) + err = openETHChannel(ht.ctx, net.Alice, 40000, 0) ht.assert.NoError(err) // Verify Alice ETH balance. resBal, err = net.Alice.Client.GetBalance(ht.ctx, &xudrpc.GetBalanceRequest{Currency: "ETH"}) ht.assert.Equal(uint64(199997900), resBal.Balances["ETH"].TotalBalance) - ht.assert.Equal(resBal.Balances["ETH"].TotalBalance-400, resBal.Balances["ETH"].WalletBalance) - ht.assert.Equal(uint64(400), resBal.Balances["ETH"].ChannelBalance) + ht.assert.Equal(resBal.Balances["ETH"].TotalBalance-40000, resBal.Balances["ETH"].WalletBalance) + ht.assert.Equal(uint64(40000), resBal.Balances["ETH"].ChannelBalance) // Place an order on Alice. req := &xudrpc.PlaceOrderRequest{ OrderId: "maker_order_id", Price: 40, - Quantity: 1, + Quantity: 100, PairId: "BTC/ETH", Side: xudrpc.OrderSide_BUY, } @@ -435,15 +493,15 @@ func testOrderMatchingAndSwapConnext(net *xudtest.NetworkHarness, ht *harnessTes // Verify Alice ETH balance. resBal, err = net.Alice.Client.GetBalance(ht.ctx, &xudrpc.GetBalanceRequest{Currency: "ETH"}) - ht.assert.Equal(uint64(199997860), resBal.Balances["ETH"].TotalBalance) - ht.assert.Equal(resBal.Balances["ETH"].TotalBalance-360, resBal.Balances["ETH"].WalletBalance) - ht.assert.Equal(uint64(360), resBal.Balances["ETH"].ChannelBalance) + ht.assert.Equal(uint64(199993900), resBal.Balances["ETH"].TotalBalance) + ht.assert.Equal(resBal.Balances["ETH"].TotalBalance-36000, resBal.Balances["ETH"].WalletBalance) + ht.assert.Equal(uint64(36000), resBal.Balances["ETH"].ChannelBalance) // Verify Bob ETH balance. resBal, err = net.Bob.Client.GetBalance(ht.ctx, &xudrpc.GetBalanceRequest{Currency: "ETH"}) - ht.assert.Equal(uint64(40), resBal.Balances["ETH"].TotalBalance) + ht.assert.Equal(uint64(4000), resBal.Balances["ETH"].TotalBalance) ht.assert.Equal(uint64(0), resBal.Balances["ETH"].WalletBalance) - ht.assert.Equal(uint64(40), resBal.Balances["ETH"].ChannelBalance) + ht.assert.Equal(uint64(4000), resBal.Balances["ETH"].ChannelBalance) // Cleanup. ht.act.disconnect(net.Alice, net.Bob) diff --git a/test/unit/TradingPair.spec.ts b/test/unit/TradingPair.spec.ts index b5694b654..9cad9b199 100644 --- a/test/unit/TradingPair.spec.ts +++ b/test/unit/TradingPair.spec.ts @@ -134,97 +134,111 @@ describe('TradingPair.match', () => { beforeEach(init); it('should fully match with two maker orders', () => { - tp.addPeerOrder(createPeerOrder(5, 5, false)); - tp.addPeerOrder(createPeerOrder(5, 5, false)); - const { remainingOrder } = tp.match(createOwnOrder(5, 10, true)); + tp.addPeerOrder(createPeerOrder(5, 5000, false)); + tp.addPeerOrder(createPeerOrder(5, 5000, false)); + const { remainingOrder } = tp.match(createOwnOrder(5, 10000, true)); expect(remainingOrder).to.be.undefined; }); it('should match with own order if equivalent peer order exists', () => { - const peerOrder = createPeerOrder(5, 5, false); - const ownOrder = createOwnOrder(5, 5, false); + const peerOrder = createPeerOrder(5, 5000, false); + const ownOrder = createOwnOrder(5, 5000, false); tp.addPeerOrder(peerOrder); tp.addOwnOrder(ownOrder); - const { matches, remainingOrder } = tp.match(createOwnOrder(5, 5, true)); + const { matches, remainingOrder } = tp.match(createOwnOrder(5, 5000, true)); expect(remainingOrder).to.be.undefined; expect(matches[0].maker).to.not.equal(peerOrder); expect(matches[0].maker).to.equal(ownOrder); }); it('should split taker order when makers are insufficient', () => { - tp.addPeerOrder(createPeerOrder(5, 40, false)); - tp.addPeerOrder(createPeerOrder(5, 50, false)); - const { remainingOrder } = tp.match(createOwnOrder(5, 100, true)); + tp.addPeerOrder(createPeerOrder(5, 4000, false)); + tp.addPeerOrder(createPeerOrder(5, 5000, false)); + const { remainingOrder } = tp.match(createOwnOrder(5, 10000, true)); expect(remainingOrder).to.not.be.undefined; - expect(remainingOrder!.quantity).to.equal(10); + expect(remainingOrder!.quantity).to.equal(1000); }); it('should split one maker order when taker is insufficient', () => { - tp.addPeerOrder(createPeerOrder(5, 5, false)); - tp.addPeerOrder(createPeerOrder(5, 6, false)); - const { matches, remainingOrder } = tp.match(createOwnOrder(5, 10, true)); + tp.addPeerOrder(createPeerOrder(5, 5000, false)); + tp.addPeerOrder(createPeerOrder(5, 6000, false)); + const { matches, remainingOrder } = tp.match(createOwnOrder(5, 10000, true)); expect(remainingOrder).to.be.undefined; matches.forEach((match) => { - expect(match.maker.quantity).to.equal(5); + expect(match.maker.quantity).to.equal(5000); }); const peekResult = tp.queues!.sellQueue.peek(); expect(peekResult).to.not.be.undefined; - expect(peekResult!.quantity).to.equal(1); + expect(peekResult!.quantity).to.equal(1000); + }); + + it('should mark split maker order as dust', (done) => { + tp.on('peerOrder.dust', (orderPortion) => { + expect(orderPortion.quantity).to.equal(1); + done(); + }); + + tp.addPeerOrder(createPeerOrder(5, 5000, false)); + const { matches, remainingOrder } = tp.match(createOwnOrder(5, 4999, true)); + expect(remainingOrder).to.be.undefined; + matches.forEach((match) => { + expect(match.maker.quantity).to.equal(4999); + }); }); it('should not match maker own order hold quantity', () => { - const ownOrder = createOwnOrder(5, 5, false); + const ownOrder = createOwnOrder(5, 5000, false); tp.addOwnOrder(ownOrder); - tp.addOrderHold(ownOrder.id, 5); + tp.addOrderHold(ownOrder.id, 5000); - const { matches, remainingOrder } = tp.match(createOwnOrder(5, 5, true)); + const { matches, remainingOrder } = tp.match(createOwnOrder(5, 5000, true)); expect(remainingOrder).to.not.be.undefined; - expect(remainingOrder!.quantity).to.equal(5); + expect(remainingOrder!.quantity).to.equal(5000); expect(matches.length).to.be.equal(0); }); it('should not match maker own order hold quantity, and should split the taker order', () => { - const ownOrder = createOwnOrder(5, 5, false); + const ownOrder = createOwnOrder(5, 5000, false); tp.addOwnOrder(ownOrder); - tp.addOrderHold(ownOrder.id, 3); + tp.addOrderHold(ownOrder.id, 3000); - const { matches, remainingOrder } = tp.match(createOwnOrder(5, 5, true)); + const { matches, remainingOrder } = tp.match(createOwnOrder(5, 5000, true)); expect(remainingOrder).to.not.be.undefined; - expect(remainingOrder!.quantity).to.equal(3); + expect(remainingOrder!.quantity).to.equal(3000); expect(matches.length).to.be.equal(1); - expect(matches[0].maker.quantity).to.be.equal(2); - expect(matches[0].taker.quantity).to.be.equal(2); + expect(matches[0].maker.quantity).to.be.equal(2000); + expect(matches[0].taker.quantity).to.be.equal(2000); }); it('should not match maker own order hold quantity, and should fully match the taker order', () => { - const ownOrder = createOwnOrder(5, 5, false); + const ownOrder = createOwnOrder(5, 5000, false); tp.addOwnOrder(ownOrder); - tp.addOrderHold(ownOrder.id, 3); + tp.addOrderHold(ownOrder.id, 3000); - const { matches, remainingOrder } = tp.match(createOwnOrder(5, 2, true)); + const { matches, remainingOrder } = tp.match(createOwnOrder(5, 2000, true)); expect(remainingOrder).to.be.undefined; expect(matches.length).to.be.equal(1); - expect(matches[0].maker.quantity).to.be.equal(2); - expect(matches[0].taker.quantity).to.be.equal(2); + expect(matches[0].maker.quantity).to.be.equal(2000); + expect(matches[0].taker.quantity).to.be.equal(2000); }); it('should not match maker own order hold quantity, but keep the order for the next match', () => { - const ownOrder = createOwnOrder(5, 5, false); + const ownOrder = createOwnOrder(5, 5000, false); tp.addOwnOrder(ownOrder); - tp.addOrderHold(ownOrder.id, 5); + tp.addOrderHold(ownOrder.id, 5000); - let mr = tp.match(createOwnOrder(5, 5, true)); + let mr = tp.match(createOwnOrder(5, 5000, true)); expect(mr.remainingOrder).to.not.be.undefined; - expect(mr.remainingOrder!.quantity).to.equal(5); + expect(mr.remainingOrder!.quantity).to.equal(5000); expect(mr.matches.length).to.be.equal(0); - tp.removeOrderHold(ownOrder.id, 5); + tp.removeOrderHold(ownOrder.id, 5000); - mr = tp.match(createOwnOrder(5, 5, true)); + mr = tp.match(createOwnOrder(5, 5000, true)); expect(mr.remainingOrder).to.be.undefined; expect(mr.matches.length).to.be.equal(1); - expect(mr.matches[0].maker.quantity).to.be.equal(5); - expect(mr.matches[0].taker.quantity).to.be.equal(5); + expect(mr.matches[0].maker.quantity).to.be.equal(5000); + expect(mr.matches[0].taker.quantity).to.be.equal(5000); }); }); @@ -232,7 +246,7 @@ describe('TradingPair.removeOwnOrder', () => { beforeEach(init); it('should add a new ownOrder and then remove it', async () => { - const ownOrder = createOwnOrder(5, 5, false); + const ownOrder = createOwnOrder(5, 5000, false); tp.addOwnOrder(ownOrder); tp.removeOwnOrder(ownOrder.id); isEmpty(tp); @@ -246,11 +260,11 @@ describe('TradingPair.removePeerOrders', () => { const firstPeerPubKey = '026a848ebd1792001ff10c6e212f6077aec5669af3de890e1ae196b4e9730d75b9'; const secondPeerPubKey = '029a96c975d301c1c8787fcb4647b5be65a3b8d8a70153ff72e3eac73759e5e345'; - const firstHostOrders = [createPeerOrder(0.01, 500, false, ms(), firstPeerPubKey), - createPeerOrder(0.01, 500, false, ms(), firstPeerPubKey)]; + const firstHostOrders = [createPeerOrder(0.01, 50000, false, ms(), firstPeerPubKey), + createPeerOrder(0.01, 50000, false, ms(), firstPeerPubKey)]; tp.addPeerOrder(firstHostOrders[0]); tp.addPeerOrder(firstHostOrders[1]); - tp.addPeerOrder(createPeerOrder(0.01, 500, false, ms(), secondPeerPubKey)); + tp.addPeerOrder(createPeerOrder(0.01, 50000, false, ms(), secondPeerPubKey)); expect(tp.getPeersOrders().sellArray.length).to.equal(3); const removedOrders = tp.removePeerOrders(firstPeerPubKey); @@ -259,14 +273,14 @@ describe('TradingPair.removePeerOrders', () => { expect(tp.queues!.sellQueue.size).to.equal(1); expect(tp.getPeersOrders().sellArray.length).to.equal(1); - const matchingResult = tp.match(createOwnOrder(0.01, 1500, true)); + const matchingResult = tp.match(createOwnOrder(0.01, 150000, true)); expect(matchingResult.remainingOrder).to.not.be.undefined; - expect(matchingResult.remainingOrder!.quantity).to.equal(1000); + expect(matchingResult.remainingOrder!.quantity).to.equal(100000); }); it('should add a new peerOrder and then remove it partially', () => { - const quantity = 5; - const quantityToRemove = 3; + const quantity = 5000; + const quantityToRemove = 3000; const order = createPeerOrder(5, quantity, false); tp.addPeerOrder(order); @@ -319,41 +333,41 @@ describe('TradingPair queues and maps integrity', () => { }); it('queue and map should have the same order instance after a partial peer order removal', () => { - const peerOrder = createPeerOrder(0.01, 1000, false, ms()); + const peerOrder = createPeerOrder(0.01, 100000, false, ms()); tp.addPeerOrder(peerOrder); expect(tp.getPeersOrders().sellArray.length).to.equal(1); - const removeResult = tp.removePeerOrder(peerOrder.id, peerOrder.peerPubKey, 300); - expect(removeResult.order.quantity).to.equal(300); + const removeResult = tp.removePeerOrder(peerOrder.id, peerOrder.peerPubKey, 30000); + expect(removeResult.order.quantity).to.equal(30000); expect(tp.getPeersOrders().sellArray.length).to.equal(1); const listRemainingOrder = tp.getPeerOrder(peerOrder.id, peerOrder.peerPubKey); const queueRemainingOrder = tp.queues!.sellQueue.peek(); - expect(listRemainingOrder && listRemainingOrder.quantity).to.equal(700); + expect(listRemainingOrder && listRemainingOrder.quantity).to.equal(70000); expect(listRemainingOrder).to.equal(queueRemainingOrder); }); it('queue and map should have the same order instance after a partial match / maker order split', () => { - const peerOrder = createPeerOrder(0.01, 1000, false, ms()); + const peerOrder = createPeerOrder(0.01, 100000, false, ms()); tp.addPeerOrder(peerOrder); expect(tp.getPeersOrders().sellArray.length).to.equal(1); - const ownOrder = createOwnOrder(0.01, 300, true); + const ownOrder = createOwnOrder(0.01, 30000, true); const matchingResult = tp.match(ownOrder); expect(matchingResult.remainingOrder).to.be.undefined; const listRemainingOrder = tp.getPeerOrder(peerOrder.id, peerOrder.peerPubKey); const queueRemainingOrder = tp.queues!.sellQueue.peek(); - expect(listRemainingOrder && listRemainingOrder.quantity).to.equal(700); + expect(listRemainingOrder && listRemainingOrder.quantity).to.equal(70000); expect(listRemainingOrder).to.equal(queueRemainingOrder); }); it('queue and map should both have the maker order removed after a full match', () => { - const peerOrder = createPeerOrder(0.01, 1000, false, ms()); + const peerOrder = createPeerOrder(0.01, 100000, false, ms()); tp.addPeerOrder(peerOrder); expect(tp.getPeersOrders().sellArray.length).to.equal(1); - const ownOrder = createOwnOrder(0.01, 1000, true); + const ownOrder = createOwnOrder(0.01, 100000, true); const matchingResult = tp.match(ownOrder); expect(matchingResult.remainingOrder).to.be.undefined;