refactor(swaps): rename errorReason swap field

This renames the `errorReason` swap deal property to `errorMessage` to
help differentiate it from `failureReason` and describe it as a field
for details about why a swap failed or what caused an error. It also
makes it optional on the `SwapFailed` packet to accommodate cases
where there are no relevant details to include besides the
`failureReason`.

BREAKING CHANGE: Renames `errorReason` field in database.

lib/db/models/SwapDeal.ts

@@ -7,7 +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 },
-    errorReason: { type: DataTypes.STRING, allowNull: true },
+    errorMessage: { type: DataTypes.STRING, allowNull: true },
     rPreimage: { type: DataTypes.STRING, allowNull: true },
     peerPubKey: {
       type: DataTypes.VIRTUAL,

lib/orderbook/OrderBook.ts

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

lib/p2p/packets/types/SwapCompletePacket.ts

@@ -1,7 +1,6 @@
 import Packet, { PacketDirection } from '../Packet';
 import PacketType from '../PacketType';
 import * as pb from '../../../proto/xudp2p_pb';
-import SwapAcceptedPacket from './SwapAcceptedPacket';
 
 export type SwapCompletePacketBody = {
   rHash: string;

lib/p2p/packets/types/SwapFailedPacket.ts

@@ -1,13 +1,12 @@
 import Packet, { PacketDirection } from '../Packet';
 import PacketType from '../PacketType';
 import * as pb from '../../../proto/xudp2p_pb';
-import SwapCompletePacket from './SwapCompletePacket';
 import { removeUndefinedProps } from '../../../utils/utils';
 
 // TODO: proper error handling
 export type SwapFailedPacketBody = {
   rHash: string;
-  errorMessage: string;
+  errorMessage?: string;
 };
 
 class SwapFailedPacket extends Packet<SwapFailedPacketBody> {
@@ -42,10 +41,10 @@ class SwapFailedPacket extends Packet<SwapFailedPacketBody> {
         hash: obj.hash,
         reqId: obj.reqId || undefined,
       }),
-      body: {
+      body: removeUndefinedProps({
         rHash: obj.rHash,
-        errorMessage: obj.errorMessage,
-      },
+        errorMessage: obj.errorMessage || undefined,
+      }),
     });
   }
 
@@ -55,7 +54,9 @@ class SwapFailedPacket extends Packet<SwapFailedPacketBody> {
     msg.setHash(this.header.hash!);
     msg.setReqId(this.header.reqId!);
     msg.setRHash(this.body!.rHash);
-    msg.setErrorMessage(this.body!.errorMessage);
+    if (this.body!.errorMessage) {
+      msg.setErrorMessage(this.body!.errorMessage!);
+    }
 
     return msg.serializeBinary();
   }

lib/swaps/Swaps.ts

@@ -107,7 +107,7 @@ 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, errorMessage?: string, reqId?: string) => {
     const errorBody: packets.SwapFailedPacketBody = {
       rHash,
       errorMessage,
@@ -383,14 +383,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.errorReason!, requestPacket.header.id);
+      this.sendErrorToPeer(peer, deal.rHash, 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.errorReason!, requestPacket.header.id);
+      this.sendErrorToPeer(peer, deal.rHash, deal.errorMessage!, requestPacket.header.id);
       return false;
     }
 
@@ -403,7 +403,7 @@ class Swaps extends EventEmitter {
         ? `${cannotSwap}`
         : `${cannotSwap}: ${err.message}`;
       this.failDeal(deal, SwapFailureReason.NoRouteFound, errMsg);
-      this.sendErrorToPeer(peer, deal.rHash, deal.errorReason!, requestPacket.header.id);
+      this.sendErrorToPeer(peer, deal.rHash, deal.errorMessage!, requestPacket.header.id);
       return false;
     }
 
@@ -414,7 +414,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.errorReason!, requestPacket.header.id);
+      this.sendErrorToPeer(peer, deal.rHash, deal.errorMessage!, requestPacket.header.id);
       return false;
     }
 
@@ -587,7 +587,7 @@ class Swaps extends EventEmitter {
     }
 
     if (!this.validateResolveRequest(deal, resolveRequest)) {
-      return deal.errorReason;
+      return deal.errorMessage;
     }
 
     if (deal.role === SwapRole.Maker) {
@@ -635,18 +635,20 @@ class Swaps extends EventEmitter {
 
   }
 
-  private failDeal = (deal: SwapDeal, failureReason: SwapFailureReason, errorReason: string): void => {
+  private failDeal = (deal: SwapDeal, failureReason: SwapFailureReason, errorMessage?: string): void => {
     // If we are already in error state and got another error report we
     // aggregate all error reasons by concatenation
     if (deal.state === SwapState.Error) {
-      deal.errorReason = deal.errorReason + '; ' + errorReason;
-      this.logger.debug('new deal state reason: ' + deal.errorReason);
+      if (errorMessage) {
+        deal.errorMessage = deal.errorMessage ? deal.errorMessage + '; ' + errorMessage : errorMessage;
+      }
+      this.logger.debug('new deal error message: ' + deal.errorMessage);
       return;
     }
     assert(deal.state === SwapState.Active, 'deal is not Active. Can not change deal state');
     deal.state = SwapState.Error;
     deal.failureReason = failureReason;
-    deal.errorReason = errorReason;
+    deal.errorMessage = errorMessage;
     this.emit('swap.failed', deal);
   }
 

lib/swaps/types.ts

@@ -12,7 +12,7 @@ export type SwapDeal = {
    */
   state: SwapState;
   /** The reason for being in the current state. */
-  errorReason?: string;
+  errorMessage?: string;
   failureReason?: SwapFailureReason;
   /** The xud node pub key of the counterparty to this swap deal. */
   peerPubKey: string;

test/integration/Swap.spec.ts

@@ -78,7 +78,7 @@ const validSwapDeal = () => {
     makerCltvDelta: 1152,
     quantity: 0.00001,
     executeTime: 1542033726871,
-    errorReason: 'UnknownPaymentHash',
+    errorMessage: 'UnknownPaymentHash',
   };
 };