diff --git a/lib/orderbook/OrderBook.ts b/lib/orderbook/OrderBook.ts index 15cb56e90..b75ee4fd5 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,9 +333,14 @@ class OrderBook extends EventEmitter { return pair.destroy(); } - public placeLimitOrder = async (order: OwnLimitOrder, immediateOrCancel = false, - onUpdate?: (e: PlaceOrderEvent) => void): Promise => { - const stampedOrder = this.stampOwnOrder(order); + public placeLimitOrder = async ({ order, immediateOrCancel = false, replaceOrderId, onUpdate }: + { + order: OwnLimitOrder, + immediateOrCancel?: boolean, + replaceOrderId?: string, + onUpdate?: (e: PlaceOrderEvent) => void, + }): Promise => { + const stampedOrder = this.stampOwnOrder(order, replaceOrderId); if (order.quantity * order.price < 1) { const quoteCurrency = order.pairId.split('/')[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) { @@ -967,19 +1037,19 @@ class OrderBook extends EventEmitter { await peer.sendOrders(outgoingOrders, reqId); } - public stampOwnOrder = (order: OwnLimitOrder): OwnOrder => { + public stampOwnOrder = (order: OwnLimitOrder, replaceOrderId?: string): OwnOrder => { const id = uuidv1(); // verify localId isn't duplicated. use global id if blank if (order.localId === '') { order.localId = id; - } else if (this.localIdMap.has(order.localId)) { + } else if (this.localIdMap.has(order.localId) && order.localId !== replaceOrderId) { throw errors.DUPLICATE_ORDER(order.localId); } 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..8575fac4b 100644 --- a/lib/service/Service.ts +++ b/lib/service/Service.ts @@ -611,25 +611,21 @@ class Service { */ public placeOrder = async ( args: { pairId: string, price: number, quantity: number, orderId: string, side: number, - replaceOrderId?: string, immediateOrCancel: boolean }, + 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, price, quantity, isBuy: side === OrderSide.Buy, - localId: orderId, + localId: orderId || replaceOrderId, }; /** Modified callback that converts Order to ServiceOrder before passing to callback. */ @@ -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, + replaceOrderId, + onUpdate: serviceCallback, + }; + 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..d76ee7374 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', () => { @@ -304,6 +328,19 @@ describe('nomatching OrderBook', () => { expect(() => orderBook['stampOwnOrder'](ownOrderWithLocalId)) .to.throw(`order with local id ${ownOrderWithLocalId.localId} already exists`); }); + + it('does not throw an error when replacing an existing localId', async () => { + const ownOrderWithLocalId = { + ...ownOrder(), + localId: uuidv1(), + }; + orderBook['localIdMap'].set(ownOrderWithLocalId.localId, { + id: ownOrderWithLocalId.localId, + pairId: ownOrderWithLocalId.pairId, + }); + const stampedOrder = orderBook['stampOwnOrder'](ownOrderWithLocalId, ownOrderWithLocalId.localId); + expect(stampedOrder.localId).to.equal(ownOrderWithLocalId.localId); + }); }); after(async () => { diff --git a/test/integration/Service.spec.ts b/test/integration/Service.spec.ts index 77e997258..3b3478ad5 100644 --- a/test/integration/Service.spec.ts +++ b/test/integration/Service.spec.ts @@ -21,6 +21,7 @@ describe('API Service', () => { quantity: 1, side: OrderSide.Buy, immediateOrCancel: false, + replaceOrderId: '', }; before(async () => { diff --git a/test/jest/Orderbook.spec.ts b/test/jest/Orderbook.spec.ts index 90a84e8e0..c2822566d 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, + 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(orderbook.getOwnOrders(pairId).sellArray[0].localId).toEqual(localId); + + 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..4a66b4dc7 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,86 @@ 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) + order := res.RemainingOrder + 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(order.IsOwnOrder) + ht.assert.Equal(replacedOrderID, order.LocalId) + + // 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(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.removeOrderAndInvalidate(net.Alice, net.Bob, order) + 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 })));