From bd573612bf0bc108318b8f37cb0bfaf836fd4ec9 Mon Sep 17 00:00:00 2001 From: Karl Ranna Date: Thu, 22 Aug 2019 15:20:56 +0000 Subject: [PATCH] fix(swaps): validate Raiden's resolve request In this commit we validate Raiden's resolve request to make sure the payment we're receiving has the previously agreed upon `deal.makerCltvDelta` as the minimum duration of the lock. In addition we change the error handling to return a status code 400 (Bad Request) to prevent Raiden from crashing. Previously we returned a status code of 200 with the response `{ secret: 'error message' }`. --- lib/http/HttpServer.ts | 5 ++++- lib/http/HttpService.ts | 2 +- lib/raidenclient/types.ts | 1 + lib/swaps/Swaps.ts | 21 +++++++++------------ lib/swaps/errors.ts | 5 +++++ lib/swaps/types.ts | 1 + test/jest/HttpService.spec.ts | 5 ++++- 7 files changed, 25 insertions(+), 15 deletions(-) diff --git a/lib/http/HttpServer.ts b/lib/http/HttpServer.ts index 77648c287..ecc565fd9 100644 --- a/lib/http/HttpServer.ts +++ b/lib/http/HttpServer.ts @@ -67,7 +67,10 @@ class HttpServer { res.writeHead(200, { 'Content-Type': 'application/json' }); res.end(JSON.stringify(resJson)); } catch (err) { - if (err.code === errorCodes.INVALID_ARGUMENT) { + if ( + err.code === errorCodes.INVALID_ARGUMENT || + err.code === swapErrorCodes.INVALID_RESOLVE_REQUEST + ) { res.writeHead(400); res.end(err.message); } else if (err.code === swapErrorCodes.PAYMENT_HASH_NOT_FOUND) { diff --git a/lib/http/HttpService.ts b/lib/http/HttpService.ts index 88784a79a..4d61625d8 100644 --- a/lib/http/HttpService.ts +++ b/lib/http/HttpService.ts @@ -5,12 +5,12 @@ class HttpService { constructor(private service: Service) {} public resolveHashRaiden = async (resolveRequest: RaidenResolveRequest): Promise => { - // TODO: add settle time out, etc const secret = await this.service.resolveHash({ rHash: resolveRequest.secrethash.slice(2), amount: resolveRequest.amount, tokenAddress: resolveRequest.token, expiration: resolveRequest.expiration, + chain_height: resolveRequest.chain_height, }); return { secret }; } diff --git a/lib/raidenclient/types.ts b/lib/raidenclient/types.ts index 97170a90d..cbabb2ace 100644 --- a/lib/raidenclient/types.ts +++ b/lib/raidenclient/types.ts @@ -68,6 +68,7 @@ export type RaidenResolveRequest = { reveal_timeout: number; /** The lock expiration for the incoming payment. */ expiration: number; + chain_height: number; // 'settle_timeout': raiden.config['settle_timeout'], // unused fields on the raiden request listed below, taken from raiden codebase // 'payment_identifier': secret_request_event.payment_identifier, diff --git a/lib/swaps/Swaps.ts b/lib/swaps/Swaps.ts index e61daa6c4..b22ca3a3b 100644 --- a/lib/swaps/Swaps.ts +++ b/lib/swaps/Swaps.ts @@ -719,25 +719,22 @@ class Swaps extends EventEmitter { * @returns `true` if the resolve request is valid, `false` otherwise */ private validateResolveRequest = (deal: SwapDeal, resolveRequest: ResolveRequest) => { - const { amount, tokenAddress, expiration } = resolveRequest; + const { amount, tokenAddress, expiration, chain_height } = resolveRequest; let expectedAmount: number; let expectedTokenAddress: string | undefined; let source: string; let destination: string; - // TODO: check cltv value switch (deal.role) { case SwapRole.Maker: expectedAmount = deal.makerUnits; expectedTokenAddress = this.swapClientManager.raidenClient.tokenAddresses.get(deal.makerCurrency); source = 'Taker'; destination = 'Maker'; - if (deal.makerCltvDelta! > expiration) { - // temporary code to relax the lock time check for raiden - this.logger.warn(`lock expiration of ${expiration} does not meet ${deal.makerCltvDelta} minimum for ${deal.rHash}`); - // end temp code - // this.logger.error(`cltvDelta of ${expiration} does not meet ${deal.makerCltvDelta!} minimum`); - // this.failDeal(deal, SwapFailureReason.InvalidResolveRequest, 'Insufficient CLTV received on first leg'); - // return false; + const lockExpirationDelta = expiration - chain_height; + if (deal.makerCltvDelta! > lockExpirationDelta) { + this.logger.error(`cltvDelta of ${lockExpirationDelta} does not meet ${deal.makerCltvDelta!} minimum`); + this.failDeal(deal, SwapFailureReason.InvalidResolveRequest, 'Insufficient CLTV received on first leg'); + return false; } break; case SwapRole.Taker: @@ -862,10 +859,10 @@ class Swaps extends EventEmitter { if (deal) { if (!this.validateResolveRequest(deal, resolveRequest)) { - return deal.errorMessage || ''; + throw errors.INVALID_RESOLVE_REQUEST(rHash, deal.errorMessage || ''); } } else { - return 'swap deal not found'; + throw errors.PAYMENT_HASH_NOT_FOUND(rHash); } try { @@ -878,7 +875,7 @@ class Swaps extends EventEmitter { return preimage; } catch (err) { this.logger.error(err.message); - return err.message; + throw err; } } diff --git a/lib/swaps/errors.ts b/lib/swaps/errors.ts index ab1a9d1c7..8204e2417 100644 --- a/lib/swaps/errors.ts +++ b/lib/swaps/errors.ts @@ -5,6 +5,7 @@ const errorCodes = { SWAP_CLIENT_NOT_FOUND: codesPrefix.concat('.1'), SWAP_CLIENT_NOT_CONFIGURED: codesPrefix.concat('.2'), PAYMENT_HASH_NOT_FOUND: codesPrefix.concat('.3'), + INVALID_RESOLVE_REQUEST: codesPrefix.concat('.4'), }; const errors = { @@ -20,6 +21,10 @@ const errors = { message: `deal for rHash ${rHash} not found`, code: errorCodes.PAYMENT_HASH_NOT_FOUND, }), + INVALID_RESOLVE_REQUEST: (rHash: string, errorMessage: string) => ({ + message: `invalid resolve request for rHash ${rHash}: ${errorMessage}`, + code: errorCodes.INVALID_RESOLVE_REQUEST, + }), }; export { errorCodes, errors }; diff --git a/lib/swaps/types.ts b/lib/swaps/types.ts index 1d769101c..90bf59f49 100644 --- a/lib/swaps/types.ts +++ b/lib/swaps/types.ts @@ -105,4 +105,5 @@ export type ResolveRequest = { tokenAddress: string, /** The number of blocks before the incoming payment expires. */ expiration: number, + chain_height: number, }; diff --git a/test/jest/HttpService.spec.ts b/test/jest/HttpService.spec.ts index 0ce16b251..69bd04b3b 100644 --- a/test/jest/HttpService.spec.ts +++ b/test/jest/HttpService.spec.ts @@ -6,11 +6,13 @@ jest.mock('../../lib/service/Service'); const mockedService = >Service; const token = '0x4c354c76d5f73a63a90be776897dc81fb6238772'; -const expiration = 75; +const expiration = 7360; +const chain_height = 1000; const secretHash = '2ea852a816e4390f1468b9b1389be14e3a965479beb2c97354a409993eb52e46'; const resolveRequest: RaidenResolveRequest = { token, expiration, + chain_height, secrethash: `0x${secretHash}`, amount: 1, reveal_timeout: 50, @@ -35,6 +37,7 @@ describe('HttpService', () => { expect(service.resolveHash) .toHaveBeenCalledWith({ expiration, + chain_height, amount: resolveRequest.amount, rHash: secretHash, tokenAddress: token,