fix(swaps): cancel taker invoice on swap fail (#1704)

This commit ensures that the taker cancels its incoming invoice when
it fails while attempting to send payment.

Fixes #1695

lib/swaps/Swaps.ts

@@ -1247,6 +1247,9 @@ class Swaps extends EventEmitter {
         const swapDealInstance = await this.repository.getSwapDeal(deal.rHash);
         await this.swapRecovery.recoverDeal(swapDealInstance!);
       }
+    } else if (deal.phase === SwapPhase.SendingPayment) {
+      const swapClient = this.swapClientManager.get(deal.takerCurrency)!;
+      swapClient.removeInvoice(deal.rHash).catch(this.logger.error); // we don't need to await the remove invoice call
     }
 
     this.logger.trace(`emitting swap.failed event for ${deal.rHash}`);

test/jest/integration/Swaps.spec.ts

@@ -1,4 +1,4 @@
-import { SwapClientType, SwapFailureReason, SwapPhase, ReputationEvent } from '../../../lib/constants/enums';
+import { SwapClientType, SwapFailureReason, SwapPhase, ReputationEvent, SwapRole, SwapState } from '../../../lib/constants/enums';
 import DB from '../../../lib/db/DB';
 import LndClient from '../../../lib/lndclient/LndClient';
 import Logger from '../../../lib/Logger';
@@ -337,20 +337,47 @@ describe('Swaps Integration', () => {
       expect(addReputationEvent).toHaveBeenCalledWith(deal.takerPubKey, ReputationEvent.SwapDelay);
       expect(deal.phase).toEqual(SwapPhase.PreimageResolved);
     });
+
+    test('it sets the phase to PreimageResolved but bans the peer if it is very late', async () => {
+      const deal: SwapDeal = getValidDeal(SwapPhase.SendingPayment);
+
+      deal.executeTime = Date.now() - Swaps['SWAP_COMPLETE_TIMEOUT'] - Swaps['SWAP_ABUSE_TIME_LIMIT'] - 1;
+      const swapPaidCallback = jest.fn();
+      swaps.on('swap.paid', swapPaidCallback);
+
+      await swaps['setDealPhase'](deal, SwapPhase.PreimageResolved);
+
+      expect(swapPaidCallback).toHaveBeenCalledTimes(1);
+      expect(addReputationEvent).toHaveBeenCalledTimes(1);
+      expect(addReputationEvent).toHaveBeenCalledWith(deal.takerPubKey, ReputationEvent.SwapAbuse);
+      expect(deal.phase).toEqual(SwapPhase.PreimageResolved);
+    });
   });
 
-  test('it sets the phase to PreimageResolved but bans the peer if it is very late', async () => {
-    const deal: SwapDeal = getValidDeal(SwapPhase.SendingPayment);
+  describe('failDeal', () => {
+    test('it fails a deal in SendingPayment phase as taker', async () => {
+      const deal: SwapDeal = getValidDeal(SwapPhase.SendingPayment, SwapRole.Taker);
+
+      swapClientManager.get = jest.fn().mockImplementation((currency) => {
+        if (currency === deal.takerCurrency) {
+          return lndBtc;
+        }
+        return;
+      });
 
-    deal.executeTime = Date.now() - Swaps['SWAP_COMPLETE_TIMEOUT'] - Swaps['SWAP_ABUSE_TIME_LIMIT'] - 1;
-    const swapPaidCallback = jest.fn();
-    swaps.on('swap.paid', swapPaidCallback);
+      const swapFailedCallback = jest.fn();
+      swaps.on('swap.failed', swapFailedCallback);
 
-    await swaps['setDealPhase'](deal, SwapPhase.PreimageResolved);
+      await swaps['failDeal']({
+        deal,
+        failureReason: SwapFailureReason.UnknownError,
+      });
 
-    expect(swapPaidCallback).toHaveBeenCalledTimes(1);
-    expect(addReputationEvent).toHaveBeenCalledTimes(1);
-    expect(addReputationEvent).toHaveBeenCalledWith(deal.takerPubKey, ReputationEvent.SwapAbuse);
-    expect(deal.phase).toEqual(SwapPhase.PreimageResolved);
+      expect(swapFailedCallback).toHaveBeenCalledTimes(1);
+      expect(swapFailedCallback).toHaveBeenCalledWith(deal);
+      expect(lndBtc.removeInvoice).toHaveBeenCalledTimes(1);
+      expect(lndBtc.removeInvoice).toHaveBeenCalledWith(deal.rHash);
+      expect(deal.state).toEqual(SwapState.Error);
+    });
   });
 });

test/simulation/custom-xud.patch

@@ -36,7 +36,7 @@ index 090618c4b..cd33a5d58 100644
    }
 
 diff --git a/lib/swaps/Swaps.ts b/lib/swaps/Swaps.ts
-index d236d8931..d6a8ed2a9 100644
+index 7fd02cbb8..2bc60fe51 100644
 --- a/lib/swaps/Swaps.ts
 +++ b/lib/swaps/Swaps.ts
 @@ -710,6 +710,24 @@ class Swaps extends EventEmitter {
@@ -143,7 +143,21 @@ index d236d8931..d6a8ed2a9 100644
        return deal.rPreimage;
      }
    }
-@@ -1310,9 +1372,14 @@ class Swaps extends EventEmitter {
+@@ -1248,8 +1310,11 @@ class Swaps extends EventEmitter {
+         await this.swapRecovery.recoverDeal(swapDealInstance!);
+       }
+     } else if (deal.phase === SwapPhase.SendingPayment) {
+-      const swapClient = this.swapClientManager.get(deal.takerCurrency)!;
+-      swapClient.removeInvoice(deal.rHash).catch(this.logger.error); // we don't need to await the remove invoice call
++      // don't cancel any invoices if the taker is stalling
++      if (process.env.CUSTOM_SCENARIO !== 'SECURITY::TAKER_2ND_HTLC_STALL') {
++        const swapClient = this.swapClientManager.get(deal.takerCurrency)!;
++        swapClient.removeInvoice(deal.rHash).catch(this.logger.error); // we don't need to await the remove invoice call
++      }
+     }
+
+     this.logger.trace(`emitting swap.failed event for ${deal.rHash}`);
+@@ -1313,9 +1378,14 @@ class Swaps extends EventEmitter {
 
          if (deal.role === SwapRole.Maker) {
            // the maker begins execution of the swap upon accepting the deal

test/utils.ts

@@ -1,12 +1,12 @@
+import fs from 'fs';
 import net from 'net';
-import { SinonSpy } from 'sinon';
-import uuidv1 from 'uuid';
 import os from 'os';
 import path from 'path';
-import fs from 'fs';
+import { SinonSpy } from 'sinon';
+import uuidv1 from 'uuid';
+import { SwapPhase, SwapRole, SwapState } from '../lib/constants/enums';
+import { OwnOrder, PeerOrder } from '../lib/orderbook/types';
 import { ms } from '../lib/utils/utils';
-import { PeerOrder, OwnOrder } from '../lib/orderbook/types';
-import { SwapPhase } from '../lib/constants/enums';
 
 /**
  * Discovers and returns a dynamically assigned, unused port available for testing.
@@ -91,8 +91,10 @@ export const createPeerOrder = (
   id: uuidv1(),
 });
 
-export const getValidDeal = (phase?: SwapPhase) => {
+export const getValidDeal = (phase = SwapPhase.SendingPayment, role = SwapRole.Maker) => {
   return {
+    phase,
+    role,
     proposedQuantity: 10000,
     pairId: 'LTC/BTC',
     orderId: '53bc8a30-81f0-11e9-9259-a5617f44d209',
@@ -112,9 +114,7 @@ export const getValidDeal = (phase?: SwapPhase) => {
     destination: '034c5266591bff232d1647f45bcf6bbc548d3d6f70b2992d28aba0afae067880ac',
     peerPubKey: '021ea6d67c850a0811b01c78c8117dca044b224601791a4186bf5748f667f73517',
     localId: '53bc8a30-81f0-11e9-9259-a5617f44d209',
-    phase: phase ?? 3,
-    state: 0,
-    role: 1,
+    state: SwapState.Active,
     createTime: 1559120485138,
     takerMaxTimeLock: 100,
   };