From 9cd0f4f7fd12767687a58453bd73902cd01f9f04 Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Tue, 25 Dec 2018 17:46:29 -0500 Subject: [PATCH] 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. --- lib/db/models/SwapDeal.ts | 1 + lib/orderbook/OrderBook.ts | 3 +++ lib/p2p/packets/types/SwapFailedPacket.ts | 4 ++++ lib/proto/xudp2p_pb.d.ts | 4 ++++ lib/proto/xudp2p_pb.js | 29 ++++++++++++++++++++++- lib/swaps/Swaps.ts | 19 ++++++++------- lib/types/enums.ts | 3 ++- proto/xudp2p.proto | 1 + test/unit/Parser.spec.ts | 11 +++++---- 9 files changed, 60 insertions(+), 15 deletions(-) diff --git a/lib/db/models/SwapDeal.ts b/lib/db/models/SwapDeal.ts index 25dc3cf0e..f9e25c71e 100644 --- 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 index 258f660b4..70c8b4a49 100644 --- 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 index 6804fd89a..8e78ffc0e 100644 --- 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 { body: removeUndefinedProps({ rHash: obj.rHash, errorMessage: obj.errorMessage || undefined, + failureReason: obj.failureReason, }), }); } @@ -57,6 +60,7 @@ class SwapFailedPacket extends Packet { 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 index ea0daa56f..5831af6f3 100644 --- a/lib/proto/xudp2p_pb.d.ts +++ b/lib/proto/xudp2p_pb.d.ts @@ -537,6 +537,9 @@ export class SwapFailedPacket extends jspb.Message { getErrorMessage(): string; setErrorMessage(value: string): void; + getFailureReason(): number; + setFailureReason(value: number): void; + serializeBinary(): Uint8Array; toObject(includeInstance?: boolean): SwapFailedPacket.AsObject; static toObject(includeInstance: boolean, msg: SwapFailedPacket): SwapFailedPacket.AsObject; @@ -554,6 +557,7 @@ export namespace SwapFailedPacket { hash: string, rHash: string, errorMessage: string, + failureReason: number, } } diff --git a/lib/proto/xudp2p_pb.js b/lib/proto/xudp2p_pb.js index 7f7b9700a..7de120efa 100644 --- a/lib/proto/xudp2p_pb.js +++ b/lib/proto/xudp2p_pb.js @@ -3751,7 +3751,8 @@ proto.xudp2p.SwapFailedPacket.toObject = function(includeInstance, msg) { reqId: jspb.Message.getFieldWithDefault(msg, 2, ""), hash: jspb.Message.getFieldWithDefault(msg, 3, ""), rHash: jspb.Message.getFieldWithDefault(msg, 4, ""), - errorMessage: jspb.Message.getFieldWithDefault(msg, 5, "") + errorMessage: jspb.Message.getFieldWithDefault(msg, 5, ""), + failureReason: jspb.Message.getFieldWithDefault(msg, 6, 0) }; if (includeInstance) { @@ -3808,6 +3809,10 @@ proto.xudp2p.SwapFailedPacket.deserializeBinaryFromReader = function(msg, reader var value = /** @type {string} */ (reader.readString()); msg.setErrorMessage(value); break; + case 6: + var value = /** @type {number} */ (reader.readUint32()); + msg.setFailureReason(value); + break; default: reader.skipField(); break; @@ -3872,6 +3877,13 @@ proto.xudp2p.SwapFailedPacket.serializeBinaryToWriter = function(message, writer f ); } + f = message.getFailureReason(); + if (f !== 0) { + writer.writeUint32( + 6, + f + ); + } }; @@ -3950,4 +3962,19 @@ proto.xudp2p.SwapFailedPacket.prototype.setErrorMessage = function(value) { }; +/** + * optional uint32 failure_reason = 6; + * @return {number} + */ +proto.xudp2p.SwapFailedPacket.prototype.getFailureReason = function() { + return /** @type {number} */ (jspb.Message.getFieldWithDefault(this, 6, 0)); +}; + + +/** @param {number} value */ +proto.xudp2p.SwapFailedPacket.prototype.setFailureReason = function(value) { + jspb.Message.setField(this, 6, value); +}; + + goog.object.extend(exports, proto.xudp2p); diff --git a/lib/swaps/Swaps.ts b/lib/swaps/Swaps.ts index ea6ba2995..539d9268b 100644 --- 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 index b1c7aa41d..53eb618c7 100644 --- 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 index 30010a4d0..15fcfbc1b 100644 --- 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 index f99fcb1d2..b7cd338e9 100644 --- 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', () => {