Merge pull request #1080 from ExchangeUnion/swaps/persist-active-deals

feat(swap): persist active swap deals to db

lib/constants/enums.ts

@@ -55,19 +55,23 @@ export enum SwapRole {
 }
 
 export enum SwapPhase {
-  /** The swap has been created locally. */
+  /** The swap deal has been created locally. */
   SwapCreated = 0,
-  /** The swap has been sent to a peer to request approval. */
+  /** We've made a request to a peer to accept this swap. */
   SwapRequested = 1,
   /** The terms of the swap have been agreed to, and we will attempt to execute it. */
-  SwapAgreed = 2,
+  SwapAccepted = 2,
   /**
-   * We have commanded swap client to send payment according to the agreed terms. The payment (and swap)
-   * could still fail due to no route with sufficient capacity, lack of cooperation from the
-   * receiver or any intermediary node along the route, or an unexpected error from swap client.
+   * We have made a request to the swap client to send payment according to the agreed terms.
+   * The payment (and swap) could still fail due to no route with sufficient capacity, lack of
+   * cooperation from the receiver or any intermediary node along the route, or an unexpected
+   * error from the swap client.
    */
   SendingPayment = 3,
-  /** We have received the agreed amount of the swap, and the preimage is now known to both sides. */
+  /**
+   * We have received the agreed amount of the swap and released the preimage to the
+   * receiving swap client so it can accept payment.
+   */
   PaymentReceived = 4,
   /** The swap has been formally completed and both sides have confirmed they've received payment. */
   SwapCompleted = 5,

lib/orderbook/OrderBook.ts

@@ -173,7 +173,7 @@ class OrderBook extends EventEmitter {
       }
     });
     this.swaps.on('swap.failed', (deal) => {
-      if (deal.role === SwapRole.Maker && (deal.phase === SwapPhase.SwapAgreed || deal.phase === SwapPhase.SendingPayment)) {
+      if (deal.role === SwapRole.Maker && (deal.phase === SwapPhase.SwapAccepted || deal.phase === SwapPhase.SendingPayment)) {
         // if our order is the maker and the swap failed after it was agreed to but before it was executed
         // we must release the hold on the order that we set when we agreed to the deal
         this.removeOrderHold(deal.orderId, deal.pairId, deal.quantity!);
@@ -883,6 +883,7 @@ class OrderBook extends EventEmitter {
       const quantity = Math.min(proposedQuantity, availableQuantity);
 
       this.addOrderHold(order.id, pairId, quantity);
+      await this.repository.addOrderIfNotExists(order);
 
       // try to accept the deal
       const orderToAccept = {
@@ -892,9 +893,7 @@ class OrderBook extends EventEmitter {
         isBuy: order.isBuy,
       };
       const dealAccepted = await this.swaps.acceptDeal(orderToAccept, requestPacket, peer);
-      if (dealAccepted) {
-        await this.repository.addOrderIfNotExists(order);
-      } else {
+      if (!dealAccepted) {
         this.removeOrderHold(order.id, pairId, quantity);
       }
     } else {

lib/swaps/SwapRepository.ts

@@ -19,14 +19,18 @@ class SwapRepository {
     });
   }
 
-  public addSwapDeal = async (swapDeal: db.SwapDealFactory): Promise<db.SwapDealInstance> => {
+  public saveSwapDeal = async (swapDeal: db.SwapDealFactory, swapOrder?: db.OrderFactory) => {
+    if (swapOrder) {
+      await this.models.Order.upsert(swapOrder);
+    }
+
     const node = await this.models.Node.findOne({
       where: {
         nodePubKey: swapDeal.peerPubKey,
       },
     });
     const attributes = { ...swapDeal, nodeId: node!.id } as db.SwapDealAttributes;
-    return this.models.SwapDeal.create(attributes);
+    await this.models.SwapDeal.upsert(attributes);
   }
 }
 export default SwapRepository;

lib/swaps/Swaps.ts

@@ -176,6 +176,11 @@ class Swaps extends EventEmitter {
       try {
         const rPreimage = await this.resolveHash(rHash, amount, currency);
         await swapClient.settleInvoice(rHash, rPreimage);
+
+        const deal = this.getDeal(rHash);
+        if (deal) {
+          await this.setDealPhase(deal, SwapPhase.PaymentReceived);
+        }
       } catch (err) {
         this.logger.error('could not settle invoice', err);
       }
@@ -213,12 +218,12 @@ class Swaps extends EventEmitter {
   }
 
   /**
-   * Saves deal to database and deletes from memory.
+   * Saves deal to database and deletes it from memory if it is no longer active.
    * @param deal The deal to persist.
    */
   private persistDeal = async (deal: SwapDeal) => {
-    if (this.usedHashes.has(deal.rHash)) {
-      await this.repository.addSwapDeal(deal);
+    await this.repository.saveSwapDeal(deal);
+    if (deal.state !== SwapState.Active) {
       this.removeDeal(deal);
     }
   }
@@ -234,6 +239,7 @@ class Swaps extends EventEmitter {
 
   public addDeal = (deal: SwapDeal) => {
     this.deals.set(deal.rHash, deal);
+    this.usedHashes.add(deal.rHash);
     this.logger.debug(`New deal: ${JSON.stringify(deal)}`);
   }
 
@@ -417,7 +423,7 @@ class Swaps extends EventEmitter {
     }
     await peer.sendPacket(new packets.SwapRequestPacket(swapRequestBody));
 
-    this.setDealPhase(deal, SwapPhase.SwapRequested);
+    await this.setDealPhase(deal, SwapPhase.SwapRequested);
     return deal.rHash;
   }
 
@@ -613,6 +619,9 @@ class Swaps extends EventEmitter {
       return false;
     }
 
+    // persist the swap deal to the database after we've added an invoice for it
+    await this.setDealPhase(deal, SwapPhase.SwapAccepted);
+
     const responseBody: packets.SwapAcceptedPacketBody = {
       makerCltvDelta: deal.makerCltvDelta || 1,
       rHash: requestBody.rHash,
@@ -621,7 +630,6 @@ class Swaps extends EventEmitter {
 
     this.logger.debug(`sending swap accepted packet: ${JSON.stringify(responseBody)} to peer: ${peer.nodePubKey}`);
     await peer.sendPacket(new packets.SwapAcceptedPacket(responseBody, requestPacket.header.id));
-    this.setDealPhase(deal, SwapPhase.SwapAgreed);
     return true;
   }
 
@@ -689,17 +697,12 @@ class Swaps extends EventEmitter {
       return;
     }
 
+    // persist the deal to the database before we attempt to send
+    await this.setDealPhase(deal, SwapPhase.SendingPayment);
+
     try {
-      this.setDealPhase(deal, SwapPhase.SendingPayment);
       await makerSwapClient.sendPayment(deal);
       // TODO: check preimage from payment response vs deal.preImage
-
-      // swap succeeded!
-      this.setDealPhase(deal, SwapPhase.SwapCompleted);
-      const responseBody: packets.SwapCompletePacketBody = { rHash };
-
-      this.logger.debug(`Sending swap complete to peer: ${JSON.stringify(responseBody)}`);
-      await peer.sendPacket(new packets.SwapCompletePacket(responseBody));
     } catch (err) {
       this.failDeal(deal, SwapFailureReason.SendPaymentFailure, err.message);
       await this.sendErrorToPeer({
@@ -709,6 +712,13 @@ class Swaps extends EventEmitter {
         errorMessage: err.message,
       });
     }
+
+    // swap succeeded!
+    await this.setDealPhase(deal, SwapPhase.SwapCompleted);
+    const responseBody: packets.SwapCompletePacketBody = { rHash };
+
+    this.logger.debug(`Sending swap complete to peer: ${JSON.stringify(responseBody)}`);
+    await peer.sendPacket(new packets.SwapCompletePacket(responseBody));
   }
 
   /**
@@ -818,10 +828,11 @@ class Swaps extends EventEmitter {
 
       const swapClient = this.swapClientManager.get(deal.takerCurrency)!;
 
+      // we update the phase persist the deal to the database before we attempt to send payment
+      await this.setDealPhase(deal, SwapPhase.SendingPayment);
+
       try {
-        this.setDealPhase(deal, SwapPhase.SendingPayment);
         deal.rPreimage = await swapClient.sendPayment(deal);
-        this.setDealPhase(deal, SwapPhase.PaymentReceived);
         return deal.rPreimage;
       } catch (err) {
         this.failDeal(deal, SwapFailureReason.SendPaymentFailure, err.message);
@@ -833,7 +844,6 @@ class Swaps extends EventEmitter {
       assert(htlcCurrency === undefined || htlcCurrency === deal.takerCurrency, 'incoming htlc does not match expected deal currency');
       this.logger.debug('Executing taker code to resolve hash');
 
-      this.setDealPhase(deal, SwapPhase.PaymentReceived);
       return deal.rPreimage!;
     }
   }
@@ -849,10 +859,17 @@ class Swaps extends EventEmitter {
       if (!this.validateResolveRequest(deal, resolveRequest)) {
         return deal.errorMessage || '';
       }
+    } else {
+      return 'swap deal not found';
     }
 
     try {
-      return this.resolveHash(rHash, amount);
+      const preimage = await this.resolveHash(rHash, amount);
+
+      // we treat responding to a resolve request as having received payment and persist the state
+      await this.setDealPhase(deal, SwapPhase.PaymentReceived);
+
+      return preimage;
     } catch (err) {
       this.logger.error(err.message);
       return err.message;
@@ -866,6 +883,8 @@ class Swaps extends EventEmitter {
   }
 
   private failDeal = (deal: SwapDeal, failureReason: SwapFailureReason, errorMessage?: string): void => {
+    assert(deal.state !== SwapState.Completed, 'Can not fail a completed deal.');
+
     // If we are already in error state and got another error report we
     // aggregate all error reasons by concatenation
     if (deal.state === SwapState.Error) {
@@ -875,6 +894,7 @@ class Swaps extends EventEmitter {
       this.logger.debug(`new deal error message for ${deal.rHash}: + ${deal.errorMessage}`);
       return;
     }
+
     if (errorMessage) {
       this.logger.debug(`deal ${deal.rHash} failed due to ${SwapFailureReason[failureReason]}`);
     } else {
@@ -910,10 +930,16 @@ class Swaps extends EventEmitter {
         break;
     }
 
-    assert(deal.state === SwapState.Active, 'deal is not Active. Can not change deal state');
     deal.state = SwapState.Error;
+    deal.completeTime = Date.now();
     deal.failureReason = failureReason;
     deal.errorMessage = errorMessage;
+
+    if (deal.phase !== SwapPhase.SwapCreated && deal.phase !== SwapPhase.SwapRequested) {
+      // persist the deal failure if it had been accepted
+      this.persistDeal(deal).catch(this.logger.error);
+    }
+
     clearTimeout(this.timeouts.get(deal.rHash));
     this.timeouts.delete(deal.rHash);
     const swapClient = this.swapClientManager.get(deal.role === SwapRole.Maker ? deal.makerCurrency : deal.takerCurrency);
@@ -923,7 +949,11 @@ class Swaps extends EventEmitter {
     this.emit('swap.failed', deal);
   }
 
-  private setDealPhase = (deal: SwapDeal, newPhase: SwapPhase): void => {
+  /**
+   * Updates the phase of a swap deal and handles logic directly related to that phase change,
+   * including persisting the deal state to the database.
+   */
+  private setDealPhase = async (deal: SwapDeal, newPhase: SwapPhase) => {
     assert(deal.state === SwapState.Active, 'deal is not Active. Can not change deal phase');
 
     switch (newPhase) {
@@ -935,15 +965,14 @@ class Swaps extends EventEmitter {
         assert(deal.phase === SwapPhase.SwapCreated, 'SwapRequested can be only be set after SwapCreated');
         this.logger.debug(`Requesting deal: ${JSON.stringify(deal)}`);
         break;
-      case SwapPhase.SwapAgreed:
-        assert(deal.role === SwapRole.Maker, 'SwapAgreed can only be set by the maker');
-        assert(deal.phase === SwapPhase.SwapCreated, 'SwapAgreed can be only be set after SwapCreated');
-        this.logger.debug('Sending swap response to peer ');
+      case SwapPhase.SwapAccepted:
+        assert(deal.role === SwapRole.Maker, 'SwapAccepted can only be set by the maker');
+        assert(deal.phase === SwapPhase.SwapCreated, 'SwapAccepted can be only be set after SwapCreated');
         break;
       case SwapPhase.SendingPayment:
         assert(deal.role === SwapRole.Taker && deal.phase === SwapPhase.SwapRequested ||
-          deal.role === SwapRole.Maker && deal.phase === SwapPhase.SwapAgreed,
-            'SendingPayment can only be set after SwapRequested (taker) or SwapAgreed (maker)');
+          deal.role === SwapRole.Maker && deal.phase === SwapPhase.SwapAccepted,
+            'SendingPayment can only be set after SwapRequested (taker) or SwapAccepted (maker)');
         deal.executeTime = Date.now();
         break;
       case SwapPhase.PaymentReceived:
@@ -957,11 +986,16 @@ class Swaps extends EventEmitter {
         this.logger.debug(`Swap completed. preimage = ${deal.rPreimage}`);
         break;
       default:
-        assert(false, 'unknown deal phase');
+        assert.fail('unknown deal phase');
     }
 
     deal.phase = newPhase;
 
+    if (deal.phase !== SwapPhase.SwapCreated && deal.phase !== SwapPhase.SwapRequested) {
+      // once a deal is accepted, we persist its state to the database on every phase update
+      await this.persistDeal(deal);
+    }
+
     if (deal.phase === SwapPhase.PaymentReceived) {
       const wasMaker = deal.role === SwapRole.Maker;
       const swapSuccess = {
@@ -986,18 +1020,17 @@ class Swaps extends EventEmitter {
     }
   }
 
-  private handleSwapComplete = (response: packets.SwapCompletePacket) => {
+  private handleSwapComplete = async (response: packets.SwapCompletePacket) => {
     const { rHash } = response.body!;
     const deal = this.getDeal(rHash);
     if (!deal) {
       this.logger.error(`received swap complete for unknown deal payment hash ${rHash}`);
       return;
     }
-    this.setDealPhase(deal, SwapPhase.SwapCompleted);
-    return this.persistDeal(deal);
+    await this.setDealPhase(deal, SwapPhase.SwapCompleted);
   }
 
-  private handleSwapFailed = (packet: packets.SwapFailedPacket) => {
+  private handleSwapFailed = async (packet: packets.SwapFailedPacket) => {
     const { rHash, errorMessage, failureReason } = packet.body!;
     const deal = this.getDeal(rHash);
     // TODO: penalize for unexpected swap failed packets
@@ -1011,9 +1044,7 @@ class Swaps extends EventEmitter {
     }
 
     this.failDeal(deal, failureReason, errorMessage);
-    return this.persistDeal(deal);
   }
-
 }
 
 export default Swaps;

lib/swaps/types.ts

@@ -60,8 +60,11 @@ export type SwapDeal = {
   makerToTakerRoutes?: Route[];
   /** The identifier for the payment channel network node we should pay to complete the swap.  */
   destination?: string;
+  /** The time when we created this swap deal locally. */
   createTime: number;
+  /** The time when we began executing the swap by sending payment. */
   executeTime?: number;
+  /** The time when the swap either completed successfully or failed. */
   completeTime?: number;
 };
 

test/jest/integration/Swaps.spec.ts

@@ -20,6 +20,13 @@ jest.mock('../../../lib/p2p/Peer');
 const mockedPeer = <jest.Mock<Peer>><any>Peer;
 jest.mock('../../../lib/lndclient/LndClient');
 const mockedLnd = <jest.Mock<LndClient>><any>LndClient;
+jest.mock('../../../lib/swaps/SwapRepository', () => {
+  return jest.fn().mockImplementation(() => {
+    return {
+      saveSwapDeal: jest.fn(),
+    };
+  });
+});
 const getMockedLnd = (cltvDelta: number) => {
   const lnd = new mockedLnd();
   // @ts-ignore

test/unit/DB.spec.ts

@@ -108,7 +108,7 @@ describe('Database', () => {
     const { rHash } = deal;
     const trade: TradeFactory = { rHash, quantity: deal.quantity!, makerOrderId: order.id };
     await orderBookRepo.addTrade(trade);
-    await swapRepo.addSwapDeal(deal);
+    await swapRepo.saveSwapDeal(deal);
 
     const swapInstance = await db.models.SwapDeal.findOne({ where: { rHash } });
     expect(swapInstance!.orderId).to.equal(order.id);