feat(p2p/db): add failureReason field for swaps

This adds the `failureReason` field to the `SwapDeal` database model as well as the `SwapFailed` packet. It removes the `PeerFailedSwap` failure reason, and assigns swaps the failure reason specified by the peer in cases when the peer fails the swap. It adds a `PaymentHashReuse` reason which was missing to cover all current swap failure reasons. BREAKING CHANGE: New `failureReason` p2p packet and database field.
ExchangeUnion · Dec 27, 2018 · 9cd0f4f · 9cd0f4f
1 parent 732a6d0
commit 9cd0f4f
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 15 deletions.
diff --git a/lib/db/models/SwapDeal.ts b/lib/db/models/SwapDeal.ts
@@ -7,6 +7,7 @@ export default (sequelize: Sequelize.Sequelize, DataTypes: Sequelize.DataTypes)
     role: { type: DataTypes.TINYINT, allowNull: false },
     state: { type: DataTypes.TINYINT, allowNull: false },
     phase: { type: DataTypes.TINYINT, allowNull: false },
+    failureReason: { type: DataTypes.TINYINT, allowNull: true },
     errorMessage: { type: DataTypes.STRING, allowNull: true },
     rPreimage: { type: DataTypes.STRING, allowNull: true },
     peerPubKey: {

diff --git a/lib/orderbook/OrderBook.ts b/lib/orderbook/OrderBook.ts
@@ -619,6 +619,7 @@ class OrderBook extends EventEmitter {
       // TODO: penalize peer for invalid swap request
       peer.sendPacket(new SwapFailedPacket({
         rHash,
+        failureReason: SwapFailureReason.InvalidSwapRequest,
       }, requestPacket.header.id));
       return;
     }
@@ -627,6 +628,7 @@ class OrderBook extends EventEmitter {
     if (!order) {
       peer.sendPacket(new SwapFailedPacket({
         rHash,
+        failureReason: SwapFailureReason.OrderNotFound,
       }, requestPacket.header.id));
       return;
     }
@@ -654,6 +656,7 @@ class OrderBook extends EventEmitter {
     } else {
       peer.sendPacket(new SwapFailedPacket({
         rHash,
+        failureReason: SwapFailureReason.OrderOnHold,
       }, requestPacket.header.id));
     }
   }

diff --git a/lib/p2p/packets/types/SwapFailedPacket.ts b/lib/p2p/packets/types/SwapFailedPacket.ts
@@ -2,10 +2,12 @@ import Packet, { PacketDirection } from '../Packet';
 import PacketType from '../PacketType';
 import * as pb from '../../../proto/xudp2p_pb';
 import { removeUndefinedProps } from '../../../utils/utils';
+import { SwapFailureReason } from '../../../types/enums';
 
 // TODO: proper error handling
 export type SwapFailedPacketBody = {
   rHash: string;
+  failureReason: SwapFailureReason;
   errorMessage?: string;
 };
 
@@ -44,6 +46,7 @@ class SwapFailedPacket extends Packet<SwapFailedPacketBody> {
       body: removeUndefinedProps({
         rHash: obj.rHash,
         errorMessage: obj.errorMessage || undefined,
+        failureReason: obj.failureReason,
       }),
     });
   }
@@ -57,6 +60,7 @@ class SwapFailedPacket extends Packet<SwapFailedPacketBody> {
     if (this.body!.errorMessage) {
       msg.setErrorMessage(this.body!.errorMessage!);
     }
+    msg.setFailureReason(this.body!.failureReason);
 
     return msg.serializeBinary();
   }

diff --git a/lib/proto/xudp2p_pb.d.ts b/lib/proto/xudp2p_pb.d.ts
diff --git a/lib/proto/xudp2p_pb.js b/lib/proto/xudp2p_pb.js
diff --git a/lib/swaps/Swaps.ts b/lib/swaps/Swaps.ts
@@ -107,9 +107,10 @@ class Swaps extends EventEmitter {
   /**
    * Sends an error to peer. Sets reqId if packet is a response to a request.
    */
-  private sendErrorToPeer = (peer: Peer, rHash: string, errorMessage?: string, reqId?: string) => {
+  private sendErrorToPeer = (peer: Peer, rHash: string, failureReason: SwapFailureReason, errorMessage?: string, reqId?: string) => {
     const errorBody: packets.SwapFailedPacketBody = {
       rHash,
+      failureReason,
       errorMessage,
     };
     this.logger.debug('Sending swap error to peer: ' + JSON.stringify(errorBody));
@@ -344,7 +345,7 @@ class Swaps extends EventEmitter {
     // TODO: multi route support (currently only 1)
 
     if (this.usedHashes.has(requestPacket.body!.rHash)) {
-      this.sendErrorToPeer(peer, requestPacket.body!.rHash, 'this rHash already exists', requestPacket.header.id);
+      this.sendErrorToPeer(peer, requestPacket.body!.rHash, SwapFailureReason.PaymentHashReuse, undefined, requestPacket.header.id);
       return false;
     }
     const requestBody = requestPacket.body!;
@@ -383,14 +384,14 @@ class Swaps extends EventEmitter {
     const errMsg = this.verifyLndSetup(deal, peer);
     if (errMsg) {
       this.failDeal(deal, SwapFailureReason.SwapClientNotSetup, errMsg);
-      this.sendErrorToPeer(peer, deal.rHash, deal.errorMessage!, requestPacket.header.id);
+      this.sendErrorToPeer(peer, deal.rHash, deal.failureReason!, deal.errorMessage, requestPacket.header.id);
       return false;
     }
 
     const lndclient = this.getClientForCurrency(deal.takerCurrency);
     if (!lndclient) {
       this.failDeal(deal, SwapFailureReason.SwapClientNotSetup, 'Unsupported taker currency');
-      this.sendErrorToPeer(peer, deal.rHash, deal.errorMessage!, requestPacket.header.id);
+      this.sendErrorToPeer(peer, deal.rHash, deal.failureReason!, deal.errorMessage, requestPacket.header.id);
       return false;
     }
 
@@ -403,7 +404,7 @@ class Swaps extends EventEmitter {
         ? `${cannotSwap}`
         : `${cannotSwap}: ${err.message}`;
       this.failDeal(deal, SwapFailureReason.NoRouteFound, errMsg);
-      this.sendErrorToPeer(peer, deal.rHash, deal.errorMessage!, requestPacket.header.id);
+      this.sendErrorToPeer(peer, deal.rHash, deal.failureReason!, deal.errorMessage, requestPacket.header.id);
       return false;
     }
 
@@ -414,7 +415,7 @@ class Swaps extends EventEmitter {
       this.logger.debug('got block height of ' + height);
     } catch (err) {
       this.failDeal(deal, SwapFailureReason.UnexpectedLndError, 'Unable to fetch block height: ' + err.message);
-      this.sendErrorToPeer(peer, deal.rHash, deal.errorMessage!, requestPacket.header.id);
+      this.sendErrorToPeer(peer, deal.rHash, deal.failureReason!, deal.errorMessage, requestPacket.header.id);
       return false;
     }
 
@@ -721,14 +722,14 @@ class Swaps extends EventEmitter {
     return this.persistDeal(deal);
   }
 
-  private handleSwapFailed = (error: packets.SwapFailedPacket) => {
-    const { rHash, errorMessage } = error.body!;
+  private handleSwapFailed = (packet: packets.SwapFailedPacket) => {
+    const { rHash, errorMessage, failureReason } = packet.body!;
     const deal = this.getDeal(rHash);
     if (!deal) {
       this.logger.error(`received swap error for unknown deal payment hash ${rHash}`);
       return;
     }
-    this.failDeal(deal, SwapFailureReason.PeerFailedSwap, errorMessage);
+    this.failDeal(deal, failureReason, errorMessage);
     return this.persistDeal(deal);
   }
 

diff --git a/lib/types/enums.ts b/lib/types/enums.ts
@@ -82,7 +82,8 @@ export enum SwapFailureReason {
   SendPaymentFailure = 7,
   /** The swap resolver request from lnd was invalid. */
   InvalidResolveRequest = 8,
-  PeerFailedSwap, // TODO: this generic reason can be replaced with the failure reason reported by the peer
+  /** The swap request attempts to reuse a payment hash. */
+  PaymentHashReuse = 9,
 }
 
 export enum DisconnectionReason {

diff --git a/proto/xudp2p.proto b/proto/xudp2p.proto
@@ -118,4 +118,5 @@ message SwapFailedPacket {
     string hash = 3 [json_name = "hash"];
     string r_hash = 4 [json_name = "r_hash"];
     string error_message = 5 [json_name = "error_message"];
+    uint32 failure_reason = 6[json_name = "failure_reason"];
 }
diff --git a/test/unit/Parser.spec.ts b/test/unit/Parser.spec.ts
@@ -4,9 +4,10 @@ import Parser, { ParserErrorType } from '../../lib/p2p/Parser';
 import { Packet, PacketType } from '../../lib/p2p/packets';
 import * as packets from '../../lib/p2p/packets/types';
 import { removeUndefinedProps } from '../../lib/utils/utils';
-import { DisconnectionReason } from '../../lib/types/enums';
+import { DisconnectionReason, SwapFailureReason } from '../../lib/types/enums';
 import uuid = require('uuid');
 import { Address } from '../../lib/types/p2p';
+import stringify from 'json-stable-stringify';
 
 chai.use(chaiAsPromised);
 
@@ -240,12 +241,14 @@ describe('Parser', () => {
 
     const swapFailedPacketBody = {
       rHash: uuid(),
-      errorMessage: uuid(),
+      errorMessage: 'this is a test',
+      failureReason: SwapFailureReason.SendPaymentFailure,
     };
     testValidPacket(new packets.SwapFailedPacket(swapFailedPacketBody));
     testValidPacket(new packets.SwapFailedPacket(swapFailedPacketBody, uuid()));
-    testValidPacket(new packets.SwapCompletePacket(removeUndefinedProps({ ...swapCompletePacketBody, errorMessage: undefined })));
-    testInvalidPacket(new packets.SwapCompletePacket(removeUndefinedProps({ ...swapCompletePacketBody, rHash: undefined })));
+    testValidPacket(new packets.SwapFailedPacket(removeUndefinedProps({ ...swapFailedPacketBody, errorMessage: undefined })));
+    testInvalidPacket(new packets.SwapFailedPacket(removeUndefinedProps({ ...swapFailedPacketBody, rHash: undefined })));
+    testInvalidPacket(new packets.SwapFailedPacket(removeUndefinedProps({ ...swapFailedPacketBody, failureReason: undefined })));
 
   });
   describe('test TCP segmentation/concatenation support', () => {