Merge pull request #715 from ExchangeUnion/remove-order-hold

 feat(rpc/orderbook): handle remove order with hold

lib/grpc/GrpcService.ts

@@ -204,8 +204,9 @@ class GrpcService {
    */
   public removeOrder: grpc.handleUnaryCall<xudrpc.RemoveOrderRequest, xudrpc.RemoveOrderResponse> = async (call, callback) => {
     try {
-      await this.service.removeOrder(call.request.toObject());
+      const quantityOnHold = this.service.removeOrder(call.request.toObject());
       const response = new xudrpc.RemoveOrderResponse();
+      response.setQuantityOnHold(quantityOnHold);
       callback(null, response);
     } catch (err) {
       callback(this.getGrpcError(err), null);

lib/orderbook/OrderBook.ts

@@ -16,7 +16,7 @@ import { CurrencyInstance, PairInstance, CurrencyFactory } from '../types/db';
 import { Pair, OrderIdentifier, OwnOrder, OrderPortion, OwnLimitOrder, PeerOrder } from '../types/orders';
 import { PlaceOrderEvent, PlaceOrderEventCase, PlaceOrderResult } from '../types/orderBook';
 import { SwapRequestPacket, SwapFailedPacket } from '../p2p/packets';
-import { SwapResult } from 'lib/swaps/types';
+import { SwapResult, SwapDeal } from 'lib/swaps/types';
 
 interface OrderBook {
   /** Adds a listener to be called when a remote order was added. */
@@ -431,14 +431,53 @@ class OrderBook extends EventEmitter {
   /**
    * Removes an order from the order book by its local id. Throws an error if the specified pairId
    * is not supported or if the order to cancel could not be found.
+   * @returns any quantity of the order that was on hold and could not be immediately removed.
    */
   public removeOwnOrderByLocalId = (localId: string) => {
-    const order = this.localIdMap.get(localId);
-    if (!order) {
+    const orderIdentifier = this.localIdMap.get(localId);
+    if (!orderIdentifier) {
       throw errors.LOCAL_ID_DOES_NOT_EXIST(localId);
     }
 
-    this.removeOwnOrder(order.id, order.pairId);
+    const order = this.getOwnOrder(orderIdentifier.id, orderIdentifier.pairId);
+    if (order.hold) {
+      let remainingHold = order.hold;
+      // we can't remove the entire order as some of it is on hold, start by removing any available portion
+      this.logger.debug(`can't remove local order ${localId} yet because it has a hold of ${order.hold}`);
+      const availableQuantity = order.quantity - order.hold;
+      if (availableQuantity) {
+        this.removeOwnOrder(orderIdentifier.id, orderIdentifier.pairId, availableQuantity);
+      }
+
+      const cleanup = (quantity: number) => {
+        remainingHold -= quantity;
+        this.logger.debug(`removed hold of ${quantity} on local order ${localId}, ${remainingHold} remaining`);
+        if (remainingHold === 0) {
+          // we can stop listening for swaps once all holds are cleared
+          this.swaps!.removeListener('swap.failed', failedHandler);
+          this.swaps!.removeListener('swap.paid', paidHandler);
+        }
+      };
+
+      const failedHandler = (deal: SwapDeal) => {
+        if (deal.orderId === orderIdentifier.id) {
+          // remove the portion that failed now that it's not on hold
+          this.removeOwnOrder(orderIdentifier.id, orderIdentifier.pairId, deal.quantity!);
+          cleanup(deal.quantity!);
+        }
+      };
+
+      const paidHandler = (result: SwapResult) => {
+        if (result.orderId === orderIdentifier.id) {
+          cleanup(result.quantity);
+        }
+      };
+
+      this.swaps!.on('swap.failed', failedHandler);
+      this.swaps!.on('swap.paid', paidHandler);
+    }
+    this.removeOwnOrder(orderIdentifier.id, orderIdentifier.pairId);
+    return order.hold;
   }
 
   private addOrderHold = (orderId: string, pairId: string, holdAmount: number) => {

lib/orderbook/TradingPair.ts

@@ -180,20 +180,27 @@ class TradingPair {
 
   private removeOrder = <T extends Order>(orderId: string, maps: OrderSidesMaps<Order>, quantityToRemove?: number):
     { order: T, fullyRemoved: boolean } => {
+    assert(quantityToRemove === undefined || quantityToRemove > 0, 'quantityToRemove cannot be 0 or negative');
     const order = maps.buy.get(orderId) || maps.sell.get(orderId);
     if (!order) {
       throw errors.ORDER_NOT_FOUND(orderId);
     }
 
     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.debug(`order quantity reduced by ${quantityToRemove}: ${orderId}`);
       return { order: { ...order, quantity: quantityToRemove } as T, fullyRemoved: false } ;
     } else {
       // otherwise, remove the order entirely
-      const list = order.isBuy ? maps.buy : maps.sell;
-      list.delete(order.id);
+      if (isOwnOrder(order)) {
+        assert(order.hold === 0, 'cannot remove an order with a hold');
+      }
+      const map = order.isBuy ? maps.buy : maps.sell;
+      map.delete(order.id);
 
       if (!this.nomatching) {
         const queue = order.isBuy ? this.queues!.buy : this.queues!.sell;

lib/service/Service.ts

@@ -113,11 +113,11 @@ class Service extends EventEmitter {
   /*
    * Remove placed order from the orderbook.
    */
-  public removeOrder = async (args: { orderId: string }) => {
+  public removeOrder = (args: { orderId: string }) => {
     const { orderId } = args;
     argChecks.HAS_ORDER_ID(args);
 
-    this.orderBook.removeOwnOrderByLocalId(orderId);
+    return this.orderBook.removeOwnOrderByLocalId(orderId);
   }
 
   /** Gets the total lightning network channel balance for a given currency. */

proto/xudrpc.proto

@@ -45,7 +45,8 @@ service Xud {
 
   /* Removes an order from the order book by its local id. This should be called when an order is
    * canceled or filled outside of xud. Removed orders become immediately unavailable for swaps,
-   * and peers are notified that the order is no longer valid. */
+   * and peers are notified that the order is no longer valid. Any portion of the order that is 
+   * on hold due to ongoing swaps will not be removed until after the swap attempts complete. */
   rpc RemoveOrder(RemoveOrderRequest) returns (RemoveOrderResponse) {
     option (google.api.http) = {
       post: "/v1/removeorder"
@@ -256,7 +257,11 @@ message RemoveOrderRequest {
   // The local id of the order to remove.
   string order_id = 1 [json_name = "order_id"];
 }
-message RemoveOrderResponse {}
+message RemoveOrderResponse {
+  // Any portion of the order that was on hold due to ongoing swaps at the time of the request
+  // and could not be removed until after the swaps finish.
+  double quantity_on_hold = 1 [json_name = "hold"];
+}
 
 message ChannelBalance {
   // Sum of channels balances denominated in satoshis or equivalent.

test/integration/Service.spec.ts

@@ -9,6 +9,7 @@ chai.use(chaiAsPromised);
 describe('API Service', () => {
   let xud: Xud;
   let service: Service;
+  let orderId: string | undefined;
 
   const pairId = 'LTC/BTC';
   const placeOrderArgs = {
@@ -68,7 +69,10 @@ describe('API Service', () => {
   });
 
   it('should place an order', async () => {
-    await expect(service.placeOrder(placeOrderArgs)).to.be.fulfilled;
+    const result = await service.placeOrder(placeOrderArgs);
+    expect(result.remainingOrder).to.not.be.undefined;
+    expect(result.remainingOrder!.pairId).to.equal(pairId);
+    orderId = result.remainingOrder!.id;
   });
 
   it('should get orders', async () => {
@@ -88,13 +92,17 @@ describe('API Service', () => {
     expect(order.quantity).to.equal(placeOrderArgs.quantity);
     expect(order.pairId).to.equal(placeOrderArgs.pairId);
     expect(order.isBuy).to.equal(placeOrderArgs.side === OrderSide.Buy);
+    expect(order.id).to.equal(orderId);
   });
 
-  it('should remove an order', async () => {
+  it('should remove an order', () => {
+    const tp = xud['orderBook'].tradingPairs.get('LTC/BTC')!;
+    expect(tp.ownOrders.buy.has(orderId!)).to.be.true;
     const args = {
       orderId: '1',
     };
-    await expect(service.removeOrder(args)).to.be.fulfilled;
+    service.removeOrder(args);
+    expect(tp.ownOrders.buy.has(orderId!)).to.be.false;
   });
 
   it('should fail adding a currency with a ticker that is not 2 to 5 characters long', async () => {