fix(swaps): ensure correct takerCltvDelta and makerCltvDelta usage

ExchangeUnion · Jun 19, 2019 · 5dc728b · 5dc728b
1 parent d11bee6
commit 5dc728b
Show file tree

Hide file tree

Showing 5 changed files with 319 additions and 35 deletions.
diff --git a/lib/lndclient/LndClient.ts b/lib/lndclient/LndClient.ts
@@ -379,15 +379,15 @@ class LndClient extends SwapClient {
     return this.unaryCall<lndrpc.ListChannelsRequest, lndrpc.ListChannelsResponse>('listChannels', new lndrpc.ListChannelsRequest());
   }
 
-  public getRoutes =  async (amount: number, destination: string): Promise<lndrpc.Route[]> => {
+  public getRoutes =  async (amount: number, destination: string, finalCltvDelta = this.cltvDelta): Promise<lndrpc.Route[]> => {
     const request = new lndrpc.QueryRoutesRequest();
     request.setAmt(amount);
-    request.setFinalCltvDelta(this.cltvDelta);
+    request.setFinalCltvDelta(finalCltvDelta);
     request.setNumRoutes(1);
     request.setPubKey(destination);
     try {
       const routes = (await this.queryRoutes(request)).getRoutesList();
-      this.logger.debug(`got ${routes.length} route(s) to destination ${destination}: ${routes}, FinalCltvDelta: ${this.cltvDelta}`);
+      this.logger.debug(`got ${routes.length} route(s) to destination ${destination}: ${routes}, finalCltvDelta: ${finalCltvDelta}`);
       return routes;
     } catch (err) {
       if (typeof err.message === 'string' && (
@@ -396,7 +396,7 @@ class LndClient extends SwapClient {
       )) {
         return [];
       } else {
-        this.logger.error(`error calling queryRoutes to ${destination}, amount ${amount} FinalCltvDelta ${this.cltvDelta}: ${JSON.stringify(err)}`);
+        this.logger.error(`error calling queryRoutes to ${destination}, amount ${amount} finalCltvDelta ${finalCltvDelta}: ${JSON.stringify(err)}`);
         throw err;
       }
     }
@@ -444,13 +444,13 @@ class LndClient extends SwapClient {
     return unlockWalletResponse.toObject();
   }
 
-  public addInvoice = async (rHash: string, amount: number) => {
+  public addInvoice = async (rHash: string, amount: number, cltvExpiry: number) => {
     const addHoldInvoiceRequest = new lndinvoices.AddHoldInvoiceRequest();
     addHoldInvoiceRequest.setHash(hexToUint8Array(rHash));
     addHoldInvoiceRequest.setValue(amount);
-    addHoldInvoiceRequest.setCltvExpiry(this.cltvDelta); // TODO: use peer's cltv delta
+    addHoldInvoiceRequest.setCltvExpiry(cltvExpiry);
     await this.addHoldInvoice(addHoldInvoiceRequest);
-    this.logger.debug(`added invoice of ${amount} for ${rHash} with CltvExpiry ${this.cltvDelta}`);
+    this.logger.debug(`added invoice of ${amount} for ${rHash} with cltvExpiry ${cltvExpiry}`);
     this.subscribeSingleInvoice(rHash);
   }
 

diff --git a/lib/swaps/SwapClient.ts b/lib/swaps/SwapClient.ts
@@ -113,9 +113,9 @@ abstract class SwapClient extends EventEmitter {
    * @param destination target node for the route
    * @returns routes
    */
-  public abstract async getRoutes(amount: number, destination: string): Promise<Route[]>;
+  public abstract async getRoutes(amount: number, destination: string, finalCltvDelta?: number): Promise<Route[]>;
 
-  public abstract async addInvoice(rHash: string, amount: number): Promise<void>;
+  public abstract async addInvoice(rHash: string, amount: number, cltvExpiry: number): Promise<void>;
 
   public abstract async settleInvoice(rHash: string, rPreimage: string): Promise<void>;
 

diff --git a/lib/swaps/Swaps.ts b/lib/swaps/Swaps.ts
@@ -15,7 +15,7 @@ import { PacketType } from '../p2p/packets';
 import SwapClientManager from './SwapClientManager';
 import { errors } from './errors';
 
-type OrderToAccept = Pick<SwapDeal, 'quantity' | 'price' | 'localId' | 'isBuy'> & {
+export type OrderToAccept = Pick<SwapDeal, 'quantity' | 'price' | 'localId' | 'isBuy'> & {
   quantity: number;
 };
 
@@ -140,7 +140,7 @@ class Swaps extends EventEmitter {
       this.sanitySwaps.set(rHash, sanitySwap);
       const swapClient = this.swapClientManager.get(currency)!;
       try {
-        await swapClient.addInvoice(rHash, 1);
+        await swapClient.addInvoice(rHash, 1, swapClient.cltvDelta);
       } catch (err) {
         this.logger.error('could not add invoice for sanity swap', err);
         return;
@@ -325,7 +325,7 @@ class Swaps extends EventEmitter {
 
     try {
       await Promise.all([
-        swapClient.addInvoice(rHash, 1),
+        swapClient.addInvoice(rHash, 1, swapClient.cltvDelta),
         peer.sendPacket(sanitySwapInitPacket),
         peer.wait(sanitySwapInitPacket.header.id, PacketType.SanitySwapAck, Swaps.SANITY_SWAP_INIT_TIMEOUT),
       ]);
@@ -484,14 +484,7 @@ class Swaps extends EventEmitter {
     }
 
     try {
-      this.logger.debug(`trying to query routes from maker to taker with ${takerPubKey}
-        and amount of ${takerAmount} and FinalCltvDelta ${takerSwapClient.cltvDelta}`);
-      deal.makerToTakerRoutes = await takerSwapClient.getRoutes(takerAmount, takerPubKey);
-      this.logger.debug(`queried routes total of ${deal.makerToTakerRoutes.length} available routes from maker to taker`);
-      deal.makerToTakerRoutes.forEach((availableRoute) => {
-
-        this.logger.debug(`available route from maker to taker with total time lock: ${availableRoute.getTotalTimeLock()}`);
-      });
+      deal.makerToTakerRoutes = await takerSwapClient.getRoutes(takerAmount, takerPubKey, deal.takerCltvDelta);
     } catch (err) {
       this.failDeal(deal, SwapFailureReason.UnexpectedClientError, err.message);
       await this.sendErrorToPeer({
@@ -552,11 +545,30 @@ class Swaps extends EventEmitter {
       this.logger.debug(`makerCltvDelta: ${deal.makerCltvDelta}`);
     }
 
+    if (!deal.makerCltvDelta) {
+      this.failDeal(deal, SwapFailureReason.UnexpectedClientError, 'Could not calculate makerCltvDelta.');
+      await this.sendErrorToPeer({
+        peer,
+        rHash,
+        failureReason: deal.failureReason!,
+        errorMessage: deal.errorMessage,
+        reqId: requestPacket.header.id,
+      });
+      return false;
+    }
+
     const makerSwapClient = this.swapClientManager.get(makerCurrency)!;
     try {
-      await makerSwapClient.addInvoice(deal.rHash, deal.makerAmount);
+      await makerSwapClient.addInvoice(deal.rHash, deal.makerAmount, deal.makerCltvDelta);
     } catch (err) {
-      this.logger.error('could not add invoice for while accepting deal', err);
+      this.failDeal(deal, SwapFailureReason.UnexpectedClientError, `could not add invoice for while accepting deal: ${err.message}`);
+      await this.sendErrorToPeer({
+        peer,
+        rHash,
+        failureReason: deal.failureReason!,
+        errorMessage: deal.errorMessage,
+        reqId: requestPacket.header.id,
+      });
       return false;
     }
 
@@ -624,7 +636,7 @@ class Swaps extends EventEmitter {
     }
 
     try {
-      await takerSwapClient.addInvoice(deal.rHash, deal.takerAmount);
+      await takerSwapClient.addInvoice(deal.rHash, deal.takerAmount, takerSwapClient.cltvDelta);
     } catch (err) {
       this.failDeal(deal, SwapFailureReason.UnexpectedClientError, err.message);
       await this.sendErrorToPeer({

diff --git a/test/integration/Swaps.spec.ts b/test/integration/Swaps.spec.ts
@@ -219,16 +219,4 @@ describe('Swaps.Integration', () => {
 
   });
 
-  describe.skip('acceptDeal', () => {
-
-    it('should reject unsupported currency', async () => {
-      expect(true).to.equal(false);
-    });
-
-    it('should reject already used hash', async () => {
-      expect(true).to.equal(false);
-    });
-
-  });
-
 });