From 8fb68a46a43187812132b056fbd8f85cb30ad8ef Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Wed, 12 Aug 2020 11:22:35 -0400 Subject: [PATCH] feat(p2p): replace order in single packet This modifies the procedure for replacing an order in the order book. Previously, it would go like this: 1. Remove old order & send order invalidation to peers. 2. Place new order and wait for matching to complete (which may take some time if it involves swap attempts). 3. Send new order to peers. Now it goes: 1. Put the old order on hold, preventing further swaps/matching. 2. Place the new order and wait for matching to complete. 3. Remove the old order from the order book. 4. Send a single packet to peers with new order info and old order id. 5. Peers that receive this packet will remove the old order and add the new one sequentially. Closes #1805. --- lib/orderbook/OrderBook.ts | 106 ++++++++++++++++++++++----- lib/orderbook/TradingPair.ts | 35 ++++++--- lib/orderbook/types.ts | 9 ++- lib/p2p/Pool.ts | 27 ++++--- lib/p2p/packets/types/OrderPacket.ts | 4 + lib/proto/xudp2p_pb.d.ts | 4 + lib/proto/xudp2p_pb.js | 29 +++++++- lib/service/Service.ts | 16 ++-- proto/xudp2p.proto | 1 + test/integration/OrderBook.spec.ts | 76 ++++++++++++------- test/jest/Orderbook.spec.ts | 64 ++++++++++++++-- test/simulation/tests-integration.go | 82 +++++++++++++++++++++ test/unit/Parser.spec.ts | 3 +- 13 files changed, 375 insertions(+), 81 deletions(-) diff --git a/lib/orderbook/OrderBook.ts b/lib/orderbook/OrderBook.ts index 15cb56e90..077bb0bab 100644 --- a/lib/orderbook/OrderBook.ts +++ b/lib/orderbook/OrderBook.ts @@ -17,7 +17,8 @@ import errors, { errorCodes } from './errors'; import OrderBookRepository from './OrderBookRepository'; import TradingPair from './TradingPair'; import { - IncomingOrder, isOwnOrder, Order, OrderBookThresholds, OrderIdentifier, OrderPortion, OutgoingOrder, OwnLimitOrder, OwnMarketOrder, + IncomingOrder, isOwnOrder, Order, OrderBookThresholds, OrderIdentifier, + OrderInvalidation, OrderPortion, OutgoingOrder, OwnLimitOrder, OwnMarketOrder, OwnOrder, Pair, PeerOrder, PlaceOrderEvent, PlaceOrderEventType, PlaceOrderResult, } from './types'; @@ -122,9 +123,9 @@ class OrderBook extends EventEmitter { this.bindSwaps(); } - private static createOutgoingOrder = (order: OwnOrder): OutgoingOrder => { + private static createOutgoingOrder = (order: OwnOrder, replaceOrderId?: string): OutgoingOrder => { const { createdAt, localId, initialQuantity, hold, ...outgoingOrder } = order; - return outgoingOrder; + return replaceOrderId ? { ...outgoingOrder, replaceOrderId } : outgoingOrder; } private checkThresholdCompliance = (order: OwnOrder | IncomingOrder) => { @@ -180,7 +181,12 @@ class OrderBook extends EventEmitter { // we must remove the amount that was put on hold while the swap was pending for the remaining order this.removeOrderHold(orderId, pairId, quantity); - const ownOrder = this.removeOwnOrder(orderId, pairId, quantity, peerPubKey); + const ownOrder = this.removeOwnOrder({ + orderId, + pairId, + takerPubKey: peerPubKey, + quantityToRemove: quantity, + }); this.emit('ownOrder.swapped', { pairId, quantity, id: orderId }); await this.persistTrade({ quantity: swapSuccess.quantity, @@ -327,8 +333,13 @@ class OrderBook extends EventEmitter { return pair.destroy(); } - public placeLimitOrder = async (order: OwnLimitOrder, immediateOrCancel = false, - onUpdate?: (e: PlaceOrderEvent) => void): Promise => { + public placeLimitOrder = async ({ order, immediateOrCancel = false, replaceOrderId, onUpdate }: + { + order: OwnLimitOrder, + immediateOrCancel?: boolean, + replaceOrderId?: string, + onUpdate?: (e: PlaceOrderEvent) => void, + }): Promise => { const stampedOrder = this.stampOwnOrder(order); if (order.quantity * order.price < 1) { @@ -350,13 +361,17 @@ class OrderBook extends EventEmitter { return this.placeOrder({ onUpdate, + replaceOrderId, order: stampedOrder, discardRemaining: immediateOrCancel, maxTime: Date.now() + OrderBook.MAX_PLACEORDER_ITERATIONS_TIME, }); } - public placeMarketOrder = async (order: OwnMarketOrder, onUpdate?: (e: PlaceOrderEvent) => void): Promise => { + public placeMarketOrder = async ({ order, onUpdate }: { + order: OwnMarketOrder, + onUpdate?: (e: PlaceOrderEvent) => void, + }): Promise => { if (this.nomatching) { throw errors.MARKET_ORDERS_NOT_ALLOWED(); } @@ -389,12 +404,14 @@ class OrderBook extends EventEmitter { retry = false, onUpdate, maxTime, + replaceOrderId, }: { order: OwnOrder, discardRemaining?: boolean, retry?: boolean, onUpdate?: (e: PlaceOrderEvent) => void, maxTime?: number, + replaceOrderId?: string, }): Promise => { // Check if order complies to thresholds if (this.thresholds.minQuantity > 0) { @@ -403,6 +420,19 @@ class OrderBook extends EventEmitter { } } + assert(!(replaceOrderId && discardRemaining), 'can not replace order and discard remaining order'); + + let replacedOrderIdentifier: OrderIdentifier | undefined; + if (replaceOrderId) { + // put the order we are replacing on hold while we place the new order + replacedOrderIdentifier = this.localIdMap.get(replaceOrderId); + if (!replacedOrderIdentifier) { + throw errors.ORDER_NOT_FOUND(replaceOrderId); + } + assert(replacedOrderIdentifier.pairId === order.pairId); + this.addOrderHold(replacedOrderIdentifier.id, replacedOrderIdentifier.pairId); + } + // this method can be called recursively on swap failures retries. // if max time exceeded, don't try to match if (maxTime && Date.now() > maxTime) { @@ -567,6 +597,9 @@ class OrderBook extends EventEmitter { // failed swaps will be added to the remaining order which may be added to the order book. await Promise.all(matchPromises); + if (replacedOrderIdentifier) { + this.removeOrderHold(replacedOrderIdentifier.id, replacedOrderIdentifier.pairId); + } if (remainingOrder) { if (discardRemaining) { this.logger.verbose(`no more matches found for order ${order.id}, remaining order will be discarded`); @@ -576,9 +609,15 @@ 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); + 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 + this.removeOwnOrder({ + orderId: replacedOrderIdentifier.id, + pairId: replacedOrderIdentifier.pairId, + }); } failedMakerOrders.forEach((peerOrder) => { @@ -630,10 +669,20 @@ class OrderBook extends EventEmitter { /** * Adds an own order to the order book and broadcasts it to peers. + * Optionally removes/replaces an existing order. * @returns false if it's a duplicated order or with an invalid pair id, otherwise true */ - private addOwnOrder = (order: OwnOrder): boolean => { + private addOwnOrder = (order: OwnOrder, replaceOrderId?: string): boolean => { const tp = this.getTradingPair(order.pairId); + + if (replaceOrderId) { + this.removeOwnOrder({ + orderId: replaceOrderId, + pairId: order.pairId, + noBroadcast: true, + }); + } + const result = tp.addOwnOrder(order); assert(result, 'own order id is duplicated'); @@ -641,7 +690,7 @@ class OrderBook extends EventEmitter { this.emit('ownOrder.added', order); - const outgoingOrder = OrderBook.createOutgoingOrder(order); + const outgoingOrder = OrderBook.createOutgoingOrder(order, replaceOrderId); this.pool.broadcastOrder(outgoingOrder); return true; } @@ -742,7 +791,11 @@ class OrderBook extends EventEmitter { const removableQuantity = order.quantity - order.hold; if (remainingQuantityToRemove <= removableQuantity) { - this.removeOwnOrder(order.id, order.pairId, remainingQuantityToRemove); + this.removeOwnOrder({ + orderId: order.id, + pairId: order.pairId, + quantityToRemove: remainingQuantityToRemove, + }); remainingQuantityToRemove = 0; } else { // we can't immediately remove the entire quantity because of a hold on the order. @@ -750,14 +803,22 @@ class OrderBook extends EventEmitter { throw errors.QUANTITY_ON_HOLD(localId, order.hold); } - this.removeOwnOrder(order.id, order.pairId, removableQuantity); + this.removeOwnOrder({ + orderId: order.id, + pairId: order.pairId, + quantityToRemove: removableQuantity, + }); remainingQuantityToRemove -= removableQuantity; const failedHandler = (deal: SwapDeal) => { if (deal.orderId === order.id) { // remove the portion that failed now that it's not on hold const quantityToRemove = Math.min(deal.quantity!, remainingQuantityToRemove); - this.removeOwnOrder(order.id, order.pairId, quantityToRemove); + this.removeOwnOrder({ + quantityToRemove, + orderId: order.id, + pairId: order.pairId, + }); cleanup(quantityToRemove); } }; @@ -786,12 +847,12 @@ class OrderBook extends EventEmitter { return remainingQuantityToRemove; } - private addOrderHold = (orderId: string, pairId: string, holdAmount: number) => { + private addOrderHold = (orderId: string, pairId: string, holdAmount?: number) => { const tp = this.getTradingPair(pairId); tp.addOrderHold(orderId, holdAmount); } - private removeOrderHold = (orderId: string, pairId: string, holdAmount: number) => { + private removeOrderHold = (orderId: string, pairId: string, holdAmount?: number) => { const tp = this.getTradingPair(pairId); tp.removeOrderHold(orderId, holdAmount); } @@ -802,7 +863,14 @@ class OrderBook extends EventEmitter { * @param takerPubKey the node pub key of the taker who filled this order, if applicable * @returns the removed portion of the order */ - private removeOwnOrder = (orderId: string, pairId: string, quantityToRemove?: number, takerPubKey?: string) => { + private removeOwnOrder = ({ orderId, pairId, quantityToRemove, takerPubKey, noBroadcast }: + { + orderId: string, + pairId: string, + quantityToRemove?: number, + takerPubKey?: string, + noBroadcast?: boolean, + }) => { const tp = this.getTradingPair(pairId); try { const removeResult = tp.removeOwnOrder(orderId, quantityToRemove); @@ -811,7 +879,9 @@ class OrderBook extends EventEmitter { this.localIdMap.delete(removeResult.order.localId); } - this.pool.broadcastOrderInvalidation(removeResult.order, takerPubKey); + if (!noBroadcast) { + this.pool.broadcastOrderInvalidation(removeResult.order, takerPubKey); + } return removeResult.order; } catch (err) { if (quantityToRemove !== undefined) { @@ -979,7 +1049,7 @@ class OrderBook extends EventEmitter { return { ...order, id, initialQuantity: order.quantity, hold: 0, createdAt: ms() }; } - private handleOrderInvalidation = (oi: OrderPortion, peerPubKey: string) => { + private handleOrderInvalidation = (oi: OrderInvalidation, peerPubKey: string) => { try { const removeResult = this.removePeerOrder(oi.id, oi.pairId, peerPubKey, oi.quantity); this.emit('peerOrder.invalidation', removeResult.order); diff --git a/lib/orderbook/TradingPair.ts b/lib/orderbook/TradingPair.ts index d2582cec4..9b8e516db 100644 --- a/lib/orderbook/TradingPair.ts +++ b/lib/orderbook/TradingPair.ts @@ -308,20 +308,35 @@ class TradingPair { return maps.buyMap.get(orderId) || maps.sellMap.get(orderId); } - public addOrderHold = (orderId: string, holdAmount: number) => { + public addOrderHold = (orderId: string, holdAmount?: number) => { const order = this.getOwnOrder(orderId); - assert(holdAmount > 0); - assert(order.hold + holdAmount <= order.quantity, 'the amount of an order on hold cannot exceed the available quantity'); - order.hold += holdAmount; - this.logger.debug(`added hold of ${holdAmount} on order ${orderId}`); + if (holdAmount === undefined) { + if (order.hold > 0) { + // we can't put an entire order on hold if part of it is already on hold + throw errors.QUANTITY_ON_HOLD(order.localId, order.hold); + } + order.hold = order.quantity; + this.logger.debug(`placed entire order ${orderId} on hold`); + } else { + assert(holdAmount > 0); + assert(order.hold + holdAmount <= order.quantity, 'the amount of an order on hold cannot exceed the available quantity'); + order.hold += holdAmount; + this.logger.debug(`added hold of ${holdAmount} on order ${orderId}`); + } } - public removeOrderHold = (orderId: string, holdAmount: number) => { + public removeOrderHold = (orderId: string, holdAmount?: number) => { const order = this.getOwnOrder(orderId); - assert(holdAmount > 0); - assert(order.hold >= holdAmount, 'cannot remove more than is currently on hold for an order'); - order.hold -= holdAmount; - this.logger.debug(`removed hold of ${holdAmount} on order ${orderId}`); + if (holdAmount === undefined) { + assert(order.hold > 0); + order.hold = 0; + this.logger.debug(`removed entire hold on order ${orderId}`); + } else { + assert(holdAmount > 0); + assert(order.hold >= holdAmount, 'cannot remove more than is currently on hold for an order'); + order.hold -= holdAmount; + this.logger.debug(`removed hold of ${holdAmount} on order ${orderId}`); + } } /** diff --git a/lib/orderbook/types.ts b/lib/orderbook/types.ts index bf1eb80ba..1daab9440 100644 --- a/lib/orderbook/types.ts +++ b/lib/orderbook/types.ts @@ -99,7 +99,9 @@ export type PeerOrder = LimitOrder & Stamp & Remote; export type Order = OwnOrder | PeerOrder; /** An outgoing local order which only includes fields that are relevant to peers. */ -export type OutgoingOrder = Pick>; +export type OutgoingOrder = Pick> & { + replaceOrderId?: string; +}; /** An incoming peer order which only includes fields that are relevant to us. */ export type IncomingOrder = OutgoingOrder & Remote; @@ -110,6 +112,11 @@ export type OrderPortion = OrderIdentifier & { localId?: string; }; +export type OrderInvalidation = OrderIdentifier & { + /** The quantity of the order being removed, or the entire order if quantity is undefined */ + quantity?: number; +}; + export type Currency = { /** The ticker symbol for this currency such as BTC, LTC, ETH, etc... */ id: string; diff --git a/lib/p2p/Pool.ts b/lib/p2p/Pool.ts index ad43e592a..e047e8fc9 100644 --- a/lib/p2p/Pool.ts +++ b/lib/p2p/Pool.ts @@ -6,7 +6,7 @@ import { DisconnectionReason, ReputationEvent, SwapFailureReason, XuNetwork } fr import { Models } from '../db/DB'; import Logger from '../Logger'; import NodeKey from '../nodekey/NodeKey'; -import { IncomingOrder, OrderPortion, OutgoingOrder } from '../orderbook/types'; +import { IncomingOrder, OrderInvalidation, OrderPortion, OutgoingOrder } from '../orderbook/types'; import addressUtils from '../utils/addressUtils'; import { pubKeyToAlias } from '../utils/aliasUtils'; import { getExternalIp } from '../utils/utils'; @@ -29,7 +29,7 @@ type NodeReputationInfo = { interface Pool { on(event: 'packet.order', listener: (order: IncomingOrder) => void): this; on(event: 'packet.getOrders', listener: (peer: Peer, reqId: string, pairIds: string[]) => void): this; - on(event: 'packet.orderInvalidation', listener: (orderInvalidation: OrderPortion, peer: string) => void): this; + on(event: 'packet.orderInvalidation', listener: (orderInvalidation: OrderInvalidation, peer: string) => void): this; on(event: 'peer.active', listener: (peerPubKey: string) => void): this; on(event: 'peer.close', listener: (peerPubKey?: string) => void): this; /** Adds a listener to be called when a peer's advertised but inactive pairs should be verified. */ @@ -43,7 +43,7 @@ interface Pool { on(event: 'packet.swapFailed', listener: (packet: packets.SwapFailedPacket) => void): this; emit(event: 'packet.order', order: IncomingOrder): boolean; emit(event: 'packet.getOrders', peer: Peer, reqId: string, pairIds: string[]): boolean; - emit(event: 'packet.orderInvalidation', orderInvalidation: OrderPortion, peer: string): boolean; + emit(event: 'packet.orderInvalidation', orderInvalidation: OrderInvalidation, peer: string): boolean; emit(event: 'peer.active', peerPubKey: string): boolean; emit(event: 'peer.close', peerPubKey?: string): boolean; /** Notifies listeners that a peer's advertised but inactive pairs should be verified. */ @@ -773,19 +773,28 @@ class Pool extends EventEmitter { case PacketType.Order: { const receivedOrder: OutgoingOrder = (packet as packets.OrderPacket).body!; this.logger.trace(`received order from ${peer.label}: ${JSON.stringify(receivedOrder)}`); - const incomingOrder: IncomingOrder = { ...receivedOrder, peerPubKey: peer.nodePubKey! }; + const { id, pairId } = receivedOrder; + + if (peer.isPairActive(pairId)) { + if (receivedOrder.replaceOrderId) { + const orderInvalidation: OrderInvalidation = { + pairId, + id: receivedOrder.replaceOrderId, + }; + this.emit('packet.orderInvalidation', orderInvalidation, peer.nodePubKey!); + } - if (peer.isPairActive(incomingOrder.pairId)) { + const incomingOrder: IncomingOrder = { ...receivedOrder, peerPubKey: peer.nodePubKey! }; this.emit('packet.order', incomingOrder); } else { - this.logger.debug(`received order ${incomingOrder.id} for deactivated trading pair`); + this.logger.debug(`received order ${id} for deactivated trading pair`); } break; } case PacketType.OrderInvalidation: { - const orderPortion = (packet as packets.OrderInvalidationPacket).body!; - this.logger.trace(`received order invalidation from ${peer.label}: ${JSON.stringify(orderPortion)}`); - this.emit('packet.orderInvalidation', orderPortion, peer.nodePubKey as string); + const orderInvalidation = (packet as packets.OrderInvalidationPacket).body!; + this.logger.trace(`received order invalidation from ${peer.label}: ${JSON.stringify(orderInvalidation)}`); + this.emit('packet.orderInvalidation', orderInvalidation, peer.nodePubKey as string); break; } case PacketType.GetOrders: { diff --git a/lib/p2p/packets/types/OrderPacket.ts b/lib/p2p/packets/types/OrderPacket.ts index 8cbefeeee..c7bef0dd5 100644 --- a/lib/p2p/packets/types/OrderPacket.ts +++ b/lib/p2p/packets/types/OrderPacket.ts @@ -42,6 +42,7 @@ class OrderPacket extends Packet { price: obj.order!.price, quantity: obj.order!.quantity, isBuy: obj.order!.isBuy, + replaceOrderId: obj.order!.replaceOrderId || undefined, }, }); } @@ -53,6 +54,9 @@ class OrderPacket extends Packet { pbOrder.setPrice(this.body!.price); pbOrder.setQuantity(this.body!.quantity); pbOrder.setIsBuy(this.body!.isBuy); + if (this.body?.replaceOrderId) { + pbOrder.setReplaceOrderId(this.body.replaceOrderId); + } const msg = new pb.OrderPacket(); msg.setId(this.header.id); diff --git a/lib/proto/xudp2p_pb.d.ts b/lib/proto/xudp2p_pb.d.ts index 6cf6668a5..c70339f0b 100644 --- a/lib/proto/xudp2p_pb.d.ts +++ b/lib/proto/xudp2p_pb.d.ts @@ -46,6 +46,9 @@ export class Order extends jspb.Message { getIsBuy(): boolean; setIsBuy(value: boolean): void; + getReplaceOrderId(): string; + setReplaceOrderId(value: string): void; + serializeBinary(): Uint8Array; toObject(includeInstance?: boolean): Order.AsObject; @@ -64,6 +67,7 @@ export namespace Order { price: number, quantity: number, isBuy: boolean, + replaceOrderId: string, } } diff --git a/lib/proto/xudp2p_pb.js b/lib/proto/xudp2p_pb.js index b204570a2..409cf4eb2 100644 --- a/lib/proto/xudp2p_pb.js +++ b/lib/proto/xudp2p_pb.js @@ -253,7 +253,8 @@ proto.xudp2p.Order.toObject = function(includeInstance, msg) { pairId: jspb.Message.getFieldWithDefault(msg, 2, ""), price: +jspb.Message.getFieldWithDefault(msg, 3, 0.0), quantity: jspb.Message.getFieldWithDefault(msg, 4, 0), - isBuy: jspb.Message.getFieldWithDefault(msg, 5, false) + isBuy: jspb.Message.getFieldWithDefault(msg, 5, false), + replaceOrderId: jspb.Message.getFieldWithDefault(msg, 6, "") }; if (includeInstance) { @@ -310,6 +311,10 @@ proto.xudp2p.Order.deserializeBinaryFromReader = function(msg, reader) { var value = /** @type {boolean} */ (reader.readBool()); msg.setIsBuy(value); break; + case 6: + var value = /** @type {string} */ (reader.readString()); + msg.setReplaceOrderId(value); + break; default: reader.skipField(); break; @@ -374,6 +379,13 @@ proto.xudp2p.Order.serializeBinaryToWriter = function(message, writer) { f ); } + f = message.getReplaceOrderId(); + if (f.length > 0) { + writer.writeString( + 6, + f + ); + } }; @@ -454,6 +466,21 @@ proto.xudp2p.Order.prototype.setIsBuy = function(value) { }; +/** + * optional string replace_order_id = 6; + * @return {string} + */ +proto.xudp2p.Order.prototype.getReplaceOrderId = function() { + return /** @type {string} */ (jspb.Message.getFieldWithDefault(this, 6, "")); +}; + + +/** @param {string} value */ +proto.xudp2p.Order.prototype.setReplaceOrderId = function(value) { + jspb.Message.setProto3StringField(this, 6, value); +}; + + /** * Generated by JsPbCodeGenerator. diff --git a/lib/service/Service.ts b/lib/service/Service.ts index 9cd533c13..22b6c4712 100644 --- a/lib/service/Service.ts +++ b/lib/service/Service.ts @@ -614,15 +614,11 @@ class Service { replaceOrderId?: string, immediateOrCancel: boolean }, callback?: (e: ServicePlaceOrderEvent) => void, ) => { - const { pairId, price, quantity, orderId, side, replaceOrderId, immediateOrCancel } = args; argChecks.PRICE_NON_NEGATIVE(args); argChecks.POSITIVE_QUANTITY(args); argChecks.PRICE_MAX_DECIMAL_PLACES(args); argChecks.HAS_PAIR_ID(args); - - if (replaceOrderId) { - this.orderBook.removeOwnOrderByLocalId(replaceOrderId, false); - } + const { pairId, price, quantity, orderId, side, replaceOrderId, immediateOrCancel } = args; const order: OwnMarketOrder | OwnLimitOrder = { pairId, @@ -643,8 +639,14 @@ class Service { }); } : undefined; - return price > 0 ? await this.orderBook.placeLimitOrder(order, immediateOrCancel, serviceCallback) : - await this.orderBook.placeMarketOrder(order, serviceCallback); + const placeOrderRequest = { + order, + immediateOrCancel, + serviceCallback, + replaceOrderId, + }; + return price > 0 ? await this.orderBook.placeLimitOrder(placeOrderRequest) : + await this.orderBook.placeMarketOrder(placeOrderRequest); } /** Removes a currency. */ diff --git a/proto/xudp2p.proto b/proto/xudp2p.proto index 66177fb83..23a715ae4 100644 --- a/proto/xudp2p.proto +++ b/proto/xudp2p.proto @@ -13,6 +13,7 @@ message Order { double price = 3; uint64 quantity = 4; bool is_buy = 5; + string replace_order_id = 6; } message Node { diff --git a/test/integration/OrderBook.spec.ts b/test/integration/OrderBook.spec.ts index 4b667dbf6..825c14bdc 100644 --- a/test/integration/OrderBook.spec.ts +++ b/test/integration/OrderBook.spec.ts @@ -109,15 +109,15 @@ describe('OrderBook', () => { it('should append two new ownOrder', async () => { const order = { pairId: PAIR_ID, quantity: 500, price: 55, isBuy: true, hold: 0 }; - const { remainingOrder } = await orderBook.placeLimitOrder({ localId: uuidv1(), ...order }); + const { remainingOrder } = await orderBook.placeLimitOrder({ order: { localId: uuidv1(), ...order } }); expect(remainingOrder).to.not.be.undefined; expect(orderBook.getOwnOrder(remainingOrder!.id, PAIR_ID)).to.not.be.undefined; - await orderBook.placeLimitOrder({ localId: uuidv1(), ...order }); + await orderBook.placeLimitOrder({ order: { localId: uuidv1(), ...order } }); }); it('should fully match new ownOrder and remove matches', async () => { const order = { pairId: 'LTC/BTC', localId: uuidv1(), quantity: 600, price: 55, isBuy: false, hold: 0 }; - const matches = await orderBook.placeLimitOrder(order); + const matches = await orderBook.placeLimitOrder({ order }); expect(matches.remainingOrder).to.be.undefined; const firstMatch = matches.internalMatches[0]; @@ -134,7 +134,7 @@ describe('OrderBook', () => { it('should partially match new market order and discard remaining order', async () => { const order = { pairId: 'LTC/BTC', localId: uuidv1(), quantity: 1000, isBuy: false, hold: 0 }; - const result = await orderBook.placeMarketOrder(order); + const result = await orderBook.placeMarketOrder({ order }); const match = result.internalMatches[0]; expect(result.remainingOrder).to.be.undefined; expect(getOwnOrder(match)).to.be.undefined; @@ -142,39 +142,39 @@ describe('OrderBook', () => { it('should create, partially match, and remove an order', async () => { const order: orders.OwnOrder = createOwnOrder(10, 1000, true); - await orderBook.placeLimitOrder(order); + await orderBook.placeLimitOrder({ order }); const takerOrder: orders.OwnMarketOrder = { pairId: 'LTC/BTC', localId: uuidv1(), quantity: 500, isBuy: false }; - await orderBook.placeMarketOrder(takerOrder); + await orderBook.placeMarketOrder({ order: takerOrder }); expect(() => orderBook.removeOwnOrderByLocalId(order.localId)).to.not.throw(); }); it('should not add a new own order with a duplicated localId', async () => { const order: orders.OwnOrder = createOwnOrder(0.01, 1000, false); - await expect(orderBook.placeLimitOrder(order)).to.be.fulfilled; + await expect(orderBook.placeLimitOrder({ order })).to.be.fulfilled; - await expect(orderBook.placeLimitOrder(order)).to.be.rejected; + await expect(orderBook.placeLimitOrder({ order })).to.be.rejected; expect(() => orderBook.removeOwnOrderByLocalId(order.localId)).to.not.throw(); expect(() => orderBook.removeOwnOrderByLocalId(order.localId)).to.throw(); - await expect(orderBook.placeLimitOrder(order)).to.be.fulfilled; + await expect(orderBook.placeLimitOrder({ order })).to.be.fulfilled; - await expect(orderBook.placeLimitOrder(order)).to.be.rejected; + await expect(orderBook.placeLimitOrder({ order })).to.be.rejected; }); it('should place order with quantity higher than min quantity', async () => { orderBook['thresholds'] = { minQuantity : 10000 }; const order: orders.OwnOrder = createOwnOrder(0.01, 1000000, false); - await expect(orderBook.placeLimitOrder(order)).to.be.fulfilled; + await expect(orderBook.placeLimitOrder({ order })).to.be.fulfilled; }); it('should throw error if the order quantity exceeds min quantity', async () => { const order: orders.OwnOrder = createOwnOrder(0.01, 100, false); - await expect(orderBook.placeLimitOrder(order)).to.be.rejected; + await expect(orderBook.placeLimitOrder({ order })).to.be.rejected; }); after(async () => { @@ -217,52 +217,76 @@ describe('nomatching OrderBook', () => { it('should should not accept market orders', () => { const order = createOwnOrder(0.01, 1000, true); - expect(orderBook.placeMarketOrder(order)).to.be.rejected; + expect(orderBook.placeMarketOrder({ order })).to.be.rejected; }); it('should accept but not match limit orders', async () => { const buyOrder = createOwnOrder(0.01, 1000, true); - const buyOrderResult = await orderBook.placeLimitOrder(buyOrder); + 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 sellOrderResult = await orderBook.placeLimitOrder(sellOrder); + 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); - await expect(orderBook.placeLimitOrder(order)).to.be.fulfilled; - await expect(orderBook.placeLimitOrder(order)).to.be.rejected; + 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); - await expect(orderBook.placeLimitOrder(order)).to.be.fulfilled; + 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 { remainingOrder } = await orderBook.placeLimitOrder(order); + const { remainingOrder } = await orderBook.placeLimitOrder({ order }); - orderBook['removeOwnOrder'](remainingOrder!.id, order.pairId, remainingOrder!.quantity - 100); - orderBook['removeOwnOrder'](remainingOrder!.id, order.pairId, 100); + orderBook['removeOwnOrder']({ + orderId: remainingOrder!.id, + pairId: order.pairId, + quantityToRemove: remainingOrder!.quantity - 100, + }); + orderBook['removeOwnOrder']({ + orderId: remainingOrder!.id, + pairId: order.pairId, + quantityToRemove: 100, + }); - expect(() => orderBook['removeOwnOrder'](remainingOrder!.id, order.pairId, 100)).to.throw; + expect(() => orderBook['removeOwnOrder']({ + orderId: remainingOrder!.id, + pairId: order.pairId, + quantityToRemove: 100, + })).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 { remainingOrder } = await orderBook.placeLimitOrder(order); + const { remainingOrder } = await orderBook.placeLimitOrder({ order }); - orderBook['removeOwnOrder'](remainingOrder!.id, order.pairId, remainingOrder!.quantity - 100); - orderBook['removeOwnOrder'](remainingOrder!.id, order.pairId, 100); + orderBook['removeOwnOrder']({ + orderId: remainingOrder!.id, + pairId: order.pairId, + quantityToRemove: remainingOrder!.quantity - 100, + }); + orderBook['removeOwnOrder']({ + orderId: remainingOrder!.id, + pairId: order.pairId, + quantityToRemove: 100, + }); - expect(() => orderBook['removeOwnOrder'](remainingOrder!.id, order.pairId, 100)).to.throw; + expect(() => orderBook['removeOwnOrder']({ + orderId: remainingOrder!.id, + pairId: order.pairId, + quantityToRemove: 100, + })).to.throw; }); describe('stampOwnOrder', () => { diff --git a/test/jest/Orderbook.spec.ts b/test/jest/Orderbook.spec.ts index 90a84e8e0..b54fff276 100644 --- a/test/jest/Orderbook.spec.ts +++ b/test/jest/Orderbook.spec.ts @@ -76,6 +76,7 @@ jest.mock('../../lib/p2p/Pool', () => { on: jest.fn(), getNetwork: () => XuNetwork.MainNet, getTokenIdentifier: (currency: string) => tokenIdentifiers[currency] as string, + broadcastOrderInvalidation: jest.fn(), }; }); }); @@ -85,6 +86,7 @@ jest.mock('../../lib/swaps/SwapClientManager', () => { return { canRouteToPeer: jest.fn().mockReturnValue(true), isConnected: jest.fn().mockReturnValue(true), + get: jest.fn().mockReturnValue({ maximumOutboundCapacity: () => Number.MAX_SAFE_INTEGER }), }; }); }); @@ -227,7 +229,7 @@ describe('OrderBook', () => { swaps.swapClientManager.get = jest.fn().mockReturnValue({ totalOutboundAmount: () => 1, }); - await expect(orderbook.placeLimitOrder(order)) + await expect(orderbook.placeLimitOrder({ order })) .rejects.toMatchSnapshot(); }); @@ -240,10 +242,7 @@ describe('OrderBook', () => { price: 0.01, isBuy: false, }; - swaps.swapClientManager.get = jest.fn().mockReturnValue({ - maximumOutboundCapacity: () => quantity, - }); - await orderbook.placeLimitOrder(order); + await orderbook.placeLimitOrder({ order }); expect(orderbook.getOwnOrderByLocalId(localId)).toHaveProperty('localId', localId); }); @@ -256,10 +255,59 @@ describe('OrderBook', () => { price: 0.01, isBuy: false, }; - swaps.swapClientManager.get = jest.fn().mockReturnValue({ - maximumOutboundCapacity: () => quantity, + await orderbook.placeLimitOrder({ + order, + immediateOrCancel: true, }); - await orderbook.placeLimitOrder(order, true); expect(() => orderbook.getOwnOrderByLocalId(localId)).toThrow(`order with local id ${localId} does not exist`); }); + + test('placeLimitOrder with replaceOrderId replaces an order ', async () => { + pool.broadcastOrderInvalidation = jest.fn(); + pool.broadcastOrder = jest.fn(); + const oldQuantity = 10000; + const oldPrice = 0.01; + const newQuantity = 20000; + const newPrice = 0.02; + const order: OwnLimitOrder = { + pairId, + localId, + quantity: oldQuantity, + price: oldPrice, + isBuy: false, + }; + + const oldOrder = await orderbook.placeLimitOrder({ + order, + }); + expect(orderbook.getOwnOrders(pairId).sellArray.length).toEqual(1); + expect(orderbook.getOwnOrders(pairId).sellArray[0].quantity).toEqual(oldQuantity); + expect(orderbook.getOwnOrders(pairId).sellArray[0].price).toEqual(oldPrice); + expect(orderbook.getOwnOrders(pairId).sellArray[0].localId).toEqual(localId); + + const newOrder = await orderbook.placeLimitOrder({ + order: { + ...order, + localId: '1', + price: newPrice, + quantity: newQuantity, + }, + replaceOrderId: localId, + }); + + expect(orderbook.getOwnOrders(pairId).sellArray.length).toEqual(1); + expect(orderbook.getOwnOrders(pairId).sellArray[0].quantity).toEqual(newQuantity); + expect(orderbook.getOwnOrders(pairId).sellArray[0].price).toEqual(newPrice); + + expect(pool.broadcastOrderInvalidation).toHaveBeenCalledTimes(0); + expect(pool.broadcastOrder).toHaveBeenCalledTimes(2); + expect(pool.broadcastOrder).toHaveBeenCalledWith({ + id: newOrder.remainingOrder!.id, + isBuy: false, + pairId: 'LTC/BTC', + price: 0.02, + quantity: 20000, + replaceOrderId: oldOrder.remainingOrder!.id, + }); + }); }); diff --git a/test/simulation/tests-integration.go b/test/simulation/tests-integration.go index 09919a937..d6d90bc06 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: "order replacement", + test: testOrderReplacement, + }, { name: "internal match and invalidation", test: testInternalMatchAndInvalidation, @@ -252,6 +256,84 @@ func testOrderMatchingAndSwap(net *xudtest.NetworkHarness, ht *harnessTest) { 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) + ht.act.verifyConnectivity(net.Alice, net.Bob) + + var originalQuantity uint64 = 1000000 + originalPrice := 0.02 + var newQuantity uint64 = 2000000 + newPrice := 0.03 + var replacedOrderID = "replaced_order_id" + + // Place an order on Alice. + req := &xudrpc.PlaceOrderRequest{ + OrderId: replacedOrderID, + Price: originalPrice, + Quantity: originalQuantity, + PairId: "LTC/BTC", + Side: xudrpc.OrderSide_BUY, + } + ht.act.placeOrderAndBroadcast(net.Alice, net.Bob, req) + + // Subscribe to orders on Bob + bobOrderChan := subscribeOrders(ht.ctx, net.Bob) + + // Replace the order on Alice + req = &xudrpc.PlaceOrderRequest{ + ReplaceOrderId: replacedOrderID, + Price: newPrice, + Quantity: newQuantity, + PairId: req.PairId, + Side: req.Side, + } + res, err := net.Alice.Client.PlaceOrderSync(ht.ctx, req) + ht.assert.NoError(err) + ht.assert.Len(res.InternalMatches, 0) + ht.assert.Len(res.SwapSuccesses, 0) + ht.assert.Len(res.SwapFailures, 0) + ht.assert.NotNil(res.RemainingOrder) + ht.assert.True(res.RemainingOrder.IsOwnOrder) + + // Retrieve and verify the removed order event on Bob. + e := <-bobOrderChan + ht.assert.NoError(e.err) + ht.assert.NotNil(e.orderUpdate) + orderRemoval := e.orderUpdate.GetOrderRemoval() + ht.assert.NotNil(orderRemoval) + ht.assert.Equal(originalQuantity, orderRemoval.Quantity) + ht.assert.Equal(replacedOrderID, orderRemoval.LocalId) + ht.assert.Equal(req.PairId, orderRemoval.PairId) + ht.assert.False(orderRemoval.IsOwnOrder) + + // Retrieve and verify the added order event on Bob. + e = <-bobOrderChan + ht.assert.NoError(e.err) + ht.assert.NotNil(e.orderUpdate) + peerOrder := e.orderUpdate.GetOrder() + ht.assert.NotNil(peerOrder) + + // Verify the peer order. + ht.assert.Equal(newPrice, peerOrder.Price) + ht.assert.Equal(req.PairId, peerOrder.PairId) + ht.assert.Equal(newQuantity, peerOrder.Quantity) + ht.assert.Equal(req.Side, peerOrder.Side) + ht.assert.False(peerOrder.IsOwnOrder) + ht.assert.Equal(net.Alice.PubKey(), peerOrder.NodeIdentifier.NodePubKey) + + // Verify that only the replaced order is in the order books + srcNodeCount, destNodeCount, err := getOrdersCount(ht.ctx, net.Alice, net.Bob) + ht.assert.NoError(err) + ht.assert.Equal(1, int(srcNodeCount.Own)) + ht.assert.Equal(0, int(srcNodeCount.Peer)) + ht.assert.Equal(0, int(destNodeCount.Own)) + ht.assert.Equal(1, int(destNodeCount.Peer)) + + // Cleanup. + ht.act.disconnect(net.Alice, net.Bob) +} + func waitConnextReady(node *xudtest.HarnessNode) error { isReady := func() bool { info, err := node.Client.GetInfo(context.Background(), &xudrpc.GetInfoRequest{}) diff --git a/test/unit/Parser.spec.ts b/test/unit/Parser.spec.ts index d04a26afc..af4ed31ab 100644 --- a/test/unit/Parser.spec.ts +++ b/test/unit/Parser.spec.ts @@ -272,7 +272,8 @@ describe('Parser', () => { isBuy: false, }; testValidPacket(new packets.OrderPacket(orderPacketBody)); - testValidPacket(new packets.OrderPacket(removeUndefinedProps({ ...orderPacketBody, isBuy: true }))); + testValidPacket(new packets.OrderPacket({ ...orderPacketBody, isBuy: true })); + testValidPacket(new packets.OrderPacket({ ...orderPacketBody, replaceOrderId: uuid() })); testInvalidPacket(new packets.OrderPacket(orderPacketBody, uuid())); testInvalidPacket(new packets.OrderPacket(removeUndefinedProps({ ...orderPacketBody, id: undefined }))); testInvalidPacket(new packets.OrderPacket(removeUndefinedProps({ ...orderPacketBody, pairId: undefined })));