From 9352ddc023ffa1dd3a72ff86b4ee9edec6d0ee96 Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Fri, 5 Jul 2019 11:59:41 -0400 Subject: [PATCH] feat(swap): persist active swap deals to db This commit persists the state of swap deals to the database upon each phase change once a deal has been accepted to. This is done to be able to prevent loss of funds and recover gracefully from any active swaps should `xud` crash unexpectedly. This is the first step towards closing #1079. --- lib/constants/enums.ts | 18 +++--- lib/orderbook/OrderBook.ts | 7 +-- lib/swaps/SwapRepository.ts | 8 ++- lib/swaps/Swaps.ts | 95 +++++++++++++++++++---------- lib/swaps/types.ts | 3 + test/jest/integration/Swaps.spec.ts | 7 +++ test/unit/DB.spec.ts | 2 +- 7 files changed, 94 insertions(+), 46 deletions(-) diff --git a/lib/constants/enums.ts b/lib/constants/enums.ts index 1b2c11265..ed0ab63fb 100644 --- a/lib/constants/enums.ts +++ b/lib/constants/enums.ts @@ -55,19 +55,23 @@ export enum SwapRole { } export enum SwapPhase { - /** The swap has been created locally. */ + /** The swap deal has been created locally. */ SwapCreated = 0, - /** The swap has been sent to a peer to request approval. */ + /** We've made a request to a peer to accept this swap. */ SwapRequested = 1, /** The terms of the swap have been agreed to, and we will attempt to execute it. */ - SwapAgreed = 2, + SwapAccepted = 2, /** - * We have commanded swap client to send payment according to the agreed terms. The payment (and swap) - * could still fail due to no route with sufficient capacity, lack of cooperation from the - * receiver or any intermediary node along the route, or an unexpected error from swap client. + * We have made a request to the swap client to send payment according to the agreed terms. + * The payment (and swap) could still fail due to no route with sufficient capacity, lack of + * cooperation from thereceiver or any intermediary node along the route, or an unexpected + * error from the swap client. */ SendingPayment = 3, - /** We have received the agreed amount of the swap, and the preimage is now known to both sides. */ + /** + * We have received the agreed amount of the swap and released the preimage to the + * receiving swap client so it can accept payment. + */ PaymentReceived = 4, /** The swap has been formally completed and both sides have confirmed they've received payment. */ SwapCompleted = 5, diff --git a/lib/orderbook/OrderBook.ts b/lib/orderbook/OrderBook.ts index 715e720c5..998e12b54 100644 --- a/lib/orderbook/OrderBook.ts +++ b/lib/orderbook/OrderBook.ts @@ -173,7 +173,7 @@ class OrderBook extends EventEmitter { } }); this.swaps.on('swap.failed', (deal) => { - if (deal.role === SwapRole.Maker && (deal.phase === SwapPhase.SwapAgreed || deal.phase === SwapPhase.SendingPayment)) { + if (deal.role === SwapRole.Maker && (deal.phase === SwapPhase.SwapAccepted || deal.phase === SwapPhase.SendingPayment)) { // if our order is the maker and the swap failed after it was agreed to but before it was executed // we must release the hold on the order that we set when we agreed to the deal this.removeOrderHold(deal.orderId, deal.pairId, deal.quantity!); @@ -882,6 +882,7 @@ class OrderBook extends EventEmitter { const quantity = Math.min(proposedQuantity, availableQuantity); this.addOrderHold(order.id, pairId, quantity); + await this.repository.addOrderIfNotExists(order); // try to accept the deal const orderToAccept = { @@ -891,9 +892,7 @@ class OrderBook extends EventEmitter { isBuy: order.isBuy, }; const dealAccepted = await this.swaps.acceptDeal(orderToAccept, requestPacket, peer); - if (dealAccepted) { - await this.repository.addOrderIfNotExists(order); - } else { + if (!dealAccepted) { this.removeOrderHold(order.id, pairId, quantity); } } else { diff --git a/lib/swaps/SwapRepository.ts b/lib/swaps/SwapRepository.ts index 8a8f4b94f..1a5b2c5a3 100644 --- a/lib/swaps/SwapRepository.ts +++ b/lib/swaps/SwapRepository.ts @@ -19,14 +19,18 @@ class SwapRepository { }); } - public addSwapDeal = async (swapDeal: db.SwapDealFactory): Promise => { + public saveSwapDeal = async (swapDeal: db.SwapDealFactory, swapOrder?: db.OrderFactory) => { + if (swapOrder) { + await this.models.Order.upsert(swapOrder); + } + const node = await this.models.Node.findOne({ where: { nodePubKey: swapDeal.peerPubKey, }, }); const attributes = { ...swapDeal, nodeId: node!.id } as db.SwapDealAttributes; - return this.models.SwapDeal.create(attributes); + await this.models.SwapDeal.upsert(attributes); } } export default SwapRepository; diff --git a/lib/swaps/Swaps.ts b/lib/swaps/Swaps.ts index 8565c71ca..351bfbf9f 100644 --- a/lib/swaps/Swaps.ts +++ b/lib/swaps/Swaps.ts @@ -171,6 +171,11 @@ class Swaps extends EventEmitter { try { const rPreimage = await this.resolveHash(rHash, amount, currency); await swapClient.settleInvoice(rHash, rPreimage); + + const deal = this.getDeal(rHash); + if (deal) { + await this.setDealPhase(deal, SwapPhase.PaymentReceived); + } } catch (err) { this.logger.error('could not settle invoice', err); } @@ -208,12 +213,12 @@ class Swaps extends EventEmitter { } /** - * Saves deal to database and deletes from memory. + * Saves deal to database and deletes it from memory if it is no longer active. * @param deal The deal to persist. */ private persistDeal = async (deal: SwapDeal) => { - if (this.usedHashes.has(deal.rHash)) { - await this.repository.addSwapDeal(deal); + await this.repository.saveSwapDeal(deal); + if (deal.state !== SwapState.Active) { this.removeDeal(deal); } } @@ -229,6 +234,7 @@ class Swaps extends EventEmitter { public addDeal = (deal: SwapDeal) => { this.deals.set(deal.rHash, deal); + this.usedHashes.add(deal.rHash); this.logger.debug(`New deal: ${JSON.stringify(deal)}`); } @@ -412,7 +418,7 @@ class Swaps extends EventEmitter { } await peer.sendPacket(new packets.SwapRequestPacket(swapRequestBody)); - this.setDealPhase(deal, SwapPhase.SwapRequested); + await this.setDealPhase(deal, SwapPhase.SwapRequested); return deal.rHash; } @@ -608,6 +614,9 @@ class Swaps extends EventEmitter { return false; } + // persist the swap deal to the database after we've added an invoice for it + await this.setDealPhase(deal, SwapPhase.SwapAccepted); + const responseBody: packets.SwapAcceptedPacketBody = { makerCltvDelta: deal.makerCltvDelta || 1, rHash: requestBody.rHash, @@ -616,7 +625,6 @@ class Swaps extends EventEmitter { this.logger.debug(`sending swap accepted packet: ${JSON.stringify(responseBody)} to peer: ${peer.nodePubKey}`); await peer.sendPacket(new packets.SwapAcceptedPacket(responseBody, requestPacket.header.id)); - this.setDealPhase(deal, SwapPhase.SwapAgreed); return true; } @@ -684,17 +692,12 @@ class Swaps extends EventEmitter { return; } + // persist the deal to the database before we attempt to send + await this.setDealPhase(deal, SwapPhase.SendingPayment); + try { - this.setDealPhase(deal, SwapPhase.SendingPayment); await makerSwapClient.sendPayment(deal); // TODO: check preimage from payment response vs deal.preImage - - // swap succeeded! - this.setDealPhase(deal, SwapPhase.SwapCompleted); - const responseBody: packets.SwapCompletePacketBody = { rHash }; - - this.logger.debug(`Sending swap complete to peer: ${JSON.stringify(responseBody)}`); - await peer.sendPacket(new packets.SwapCompletePacket(responseBody)); } catch (err) { this.failDeal(deal, SwapFailureReason.SendPaymentFailure, err.message); await this.sendErrorToPeer({ @@ -704,6 +707,13 @@ class Swaps extends EventEmitter { errorMessage: err.message, }); } + + // swap succeeded! + await this.setDealPhase(deal, SwapPhase.SwapCompleted); + const responseBody: packets.SwapCompletePacketBody = { rHash }; + + this.logger.debug(`Sending swap complete to peer: ${JSON.stringify(responseBody)}`); + await peer.sendPacket(new packets.SwapCompletePacket(responseBody)); } /** @@ -813,10 +823,11 @@ class Swaps extends EventEmitter { const swapClient = this.swapClientManager.get(deal.takerCurrency)!; + // we update the phase persist the deal to the database before we attempt to send payment + await this.setDealPhase(deal, SwapPhase.SendingPayment); + try { - this.setDealPhase(deal, SwapPhase.SendingPayment); deal.rPreimage = await swapClient.sendPayment(deal); - this.setDealPhase(deal, SwapPhase.PaymentReceived); return deal.rPreimage; } catch (err) { this.failDeal(deal, SwapFailureReason.SendPaymentFailure, err.message); @@ -828,7 +839,6 @@ class Swaps extends EventEmitter { assert(htlcCurrency === undefined || htlcCurrency === deal.takerCurrency, 'incoming htlc does not match expected deal currency'); this.logger.debug('Executing taker code to resolve hash'); - this.setDealPhase(deal, SwapPhase.PaymentReceived); return deal.rPreimage!; } } @@ -844,10 +854,17 @@ class Swaps extends EventEmitter { if (!this.validateResolveRequest(deal, resolveRequest)) { return deal.errorMessage || ''; } + } else { + return 'swap deal not found'; } try { - return this.resolveHash(rHash, amount); + const preimage = await this.resolveHash(rHash, amount); + + // we treat responding to a resolve request as having received payment and persist the state + await this.setDealPhase(deal, SwapPhase.PaymentReceived); + + return preimage; } catch (err) { this.logger.error(err.message); return err.message; @@ -861,6 +878,8 @@ class Swaps extends EventEmitter { } private failDeal = (deal: SwapDeal, failureReason: SwapFailureReason, errorMessage?: string): void => { + assert(deal.state !== SwapState.Completed, 'Can not fail a completed deal.'); + // If we are already in error state and got another error report we // aggregate all error reasons by concatenation if (deal.state === SwapState.Error) { @@ -870,6 +889,7 @@ class Swaps extends EventEmitter { this.logger.debug(`new deal error message for ${deal.rHash}: + ${deal.errorMessage}`); return; } + if (errorMessage) { this.logger.debug(`deal ${deal.rHash} failed due to ${SwapFailureReason[failureReason]}`); } else { @@ -905,10 +925,16 @@ class Swaps extends EventEmitter { break; } - assert(deal.state === SwapState.Active, 'deal is not Active. Can not change deal state'); deal.state = SwapState.Error; + deal.completeTime = Date.now(); deal.failureReason = failureReason; deal.errorMessage = errorMessage; + + if (deal.phase !== SwapPhase.SwapCreated && deal.phase !== SwapPhase.SwapRequested) { + // persist the deal failure if it had been accepted + this.persistDeal(deal).catch(this.logger.error); + } + clearTimeout(this.timeouts.get(deal.rHash)); this.timeouts.delete(deal.rHash); const swapClient = this.swapClientManager.get(deal.role === SwapRole.Maker ? deal.makerCurrency : deal.takerCurrency); @@ -918,7 +944,11 @@ class Swaps extends EventEmitter { this.emit('swap.failed', deal); } - private setDealPhase = (deal: SwapDeal, newPhase: SwapPhase): void => { + /** + * Updates the phase of a swap deal and handles logic directly related to that phase change, + * including persisting the deal state to the database. + */ + private setDealPhase = async (deal: SwapDeal, newPhase: SwapPhase) => { assert(deal.state === SwapState.Active, 'deal is not Active. Can not change deal phase'); switch (newPhase) { @@ -930,15 +960,14 @@ class Swaps extends EventEmitter { assert(deal.phase === SwapPhase.SwapCreated, 'SwapRequested can be only be set after SwapCreated'); this.logger.debug(`Requesting deal: ${JSON.stringify(deal)}`); break; - case SwapPhase.SwapAgreed: - assert(deal.role === SwapRole.Maker, 'SwapAgreed can only be set by the maker'); - assert(deal.phase === SwapPhase.SwapCreated, 'SwapAgreed can be only be set after SwapCreated'); - this.logger.debug('Sending swap response to peer '); + case SwapPhase.SwapAccepted: + assert(deal.role === SwapRole.Maker, 'SwapAccepted can only be set by the maker'); + assert(deal.phase === SwapPhase.SwapCreated, 'SwapAccepted can be only be set after SwapCreated'); break; case SwapPhase.SendingPayment: assert(deal.role === SwapRole.Taker && deal.phase === SwapPhase.SwapRequested || - deal.role === SwapRole.Maker && deal.phase === SwapPhase.SwapAgreed, - 'SendingPayment can only be set after SwapRequested (taker) or SwapAgreed (maker)'); + deal.role === SwapRole.Maker && deal.phase === SwapPhase.SwapAccepted, + 'SendingAmount can only be set after SwapRequested (taker) or SwapAccepted (maker)'); deal.executeTime = Date.now(); break; case SwapPhase.PaymentReceived: @@ -952,11 +981,16 @@ class Swaps extends EventEmitter { this.logger.debug(`Swap completed. preimage = ${deal.rPreimage}`); break; default: - assert(false, 'unknown deal phase'); + assert.fail('unknown deal phase'); } deal.phase = newPhase; + if (deal.phase !== SwapPhase.SwapCreated && deal.phase !== SwapPhase.SwapRequested) { + // once a deal is accepted, we persist its state to the database on every phase update + await this.persistDeal(deal); + } + if (deal.phase === SwapPhase.PaymentReceived) { const wasMaker = deal.role === SwapRole.Maker; const swapSuccess = { @@ -981,18 +1015,17 @@ class Swaps extends EventEmitter { } } - private handleSwapComplete = (response: packets.SwapCompletePacket) => { + private handleSwapComplete = async (response: packets.SwapCompletePacket) => { const { rHash } = response.body!; const deal = this.getDeal(rHash); if (!deal) { this.logger.error(`received swap complete for unknown deal payment hash ${rHash}`); return; } - this.setDealPhase(deal, SwapPhase.SwapCompleted); - return this.persistDeal(deal); + await this.setDealPhase(deal, SwapPhase.SwapCompleted); } - private handleSwapFailed = (packet: packets.SwapFailedPacket) => { + private handleSwapFailed = async (packet: packets.SwapFailedPacket) => { const { rHash, errorMessage, failureReason } = packet.body!; const deal = this.getDeal(rHash); // TODO: penalize for unexpected swap failed packets @@ -1006,9 +1039,7 @@ class Swaps extends EventEmitter { } this.failDeal(deal, failureReason, errorMessage); - return this.persistDeal(deal); } - } export default Swaps; diff --git a/lib/swaps/types.ts b/lib/swaps/types.ts index 052321589..fa98a49ea 100644 --- a/lib/swaps/types.ts +++ b/lib/swaps/types.ts @@ -60,8 +60,11 @@ export type SwapDeal = { makerToTakerRoutes?: Route[]; /** The identifier for the payment channel network node we should pay to complete the swap. */ destination?: string; + /** The time when we created this swap deal locally. */ createTime: number; + /** The time when we began executing the swap by sending payment. */ executeTime?: number; + /** The time when the swap either completed successfully or failed. */ completeTime?: number; }; diff --git a/test/jest/integration/Swaps.spec.ts b/test/jest/integration/Swaps.spec.ts index 4d0075b6a..a50eccfb3 100644 --- a/test/jest/integration/Swaps.spec.ts +++ b/test/jest/integration/Swaps.spec.ts @@ -20,6 +20,13 @@ jest.mock('../../../lib/p2p/Peer'); const mockedPeer = >Peer; jest.mock('../../../lib/lndclient/LndClient'); const mockedLnd = >LndClient; +jest.mock('../../../lib/swaps/SwapRepository', () => { + return jest.fn().mockImplementation(() => { + return { + saveSwapDeal: jest.fn(), + }; + }); +}); const getMockedLnd = (cltvDelta: number) => { const lnd = new mockedLnd(); // @ts-ignore diff --git a/test/unit/DB.spec.ts b/test/unit/DB.spec.ts index 0a9a1ed11..b3f06d095 100644 --- a/test/unit/DB.spec.ts +++ b/test/unit/DB.spec.ts @@ -108,7 +108,7 @@ describe('Database', () => { const { rHash } = deal; const trade: TradeFactory = { rHash, quantity: deal.quantity!, makerOrderId: order.id }; await orderBookRepo.addTrade(trade); - await swapRepo.addSwapDeal(deal); + await swapRepo.saveSwapDeal(deal); const swapInstance = await db.models.SwapDeal.findOne({ where: { rHash } }); expect(swapInstance!.orderId).to.equal(order.id);