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' }`.

Author: Karl Ranna

lib/http/HttpServer.ts

@@ -63,19 +63,25 @@ class HttpServer {
         default:
           res.writeHead(404);
           res.end();
+          break;
       }
       res.writeHead(200, { 'Content-Type': 'application/json' });
       res.end(JSON.stringify(resJson));
     } catch (err) {
-      if (err.code === errorCodes.INVALID_ARGUMENT) {
-        res.writeHead(400);
-        res.end(err.message);
-      } else if (err.code === swapErrorCodes.PAYMENT_HASH_NOT_FOUND) {
-        res.writeHead(404);
-        res.end(err.message);
-      } else {
-        res.writeHead(500);
-        res.end(JSON.stringify(err));
+      switch (err.code) {
+        case swapErrorCodes.INVALID_RESOLVE_REQUEST:
+        case errorCodes.INVALID_ARGUMENT:
+          res.writeHead(400);
+          res.end(err.message);
+          break;
+        case swapErrorCodes.PAYMENT_HASH_NOT_FOUND:
+          res.writeHead(404);
+          res.end(err.message);
+          break;
+        default:
+          res.writeHead(500);
+          res.end(JSON.stringify(err));
+          break;
       }
     }
   }

lib/http/HttpService.ts

@@ -5,12 +5,12 @@ class HttpService {
   constructor(private service: Service) {}
 
   public resolveHashRaiden = async (resolveRequest: RaidenResolveRequest): Promise<RaidenResolveResponse> => {
-    // 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 };
   }

lib/p2p/Peer.ts

@@ -696,6 +696,7 @@ class Peer extends EventEmitter {
           this.logger.warn(`Peer (${this.label}): ${err.message}`);
           this.emit('reputation', ReputationEvent.WireProtocolErr);
           await this.close(DisconnectionReason.WireProtocolErr, err.message);
+          break;
       }
     });
   }

lib/raidenclient/types.ts

@@ -64,15 +64,10 @@ export type RaidenResolveRequest = {
   secrethash: string;
   /** The amount of the incoming payment pending resolution, in the smallest units supported by the token. */
   amount: number;
-  /** The maximum number of blocks allowed between the setting of a hashlock and the revealing of the related secret. */
-  reveal_timeout: number;
-  /** The lock expiration for the incoming payment. */
+  /** The lock expiration for the incoming payment (absolute block number). */
   expiration: 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,
-  // 'payment_sender': to_hex(secret_request_event.recipient),
-  // 'payment_recipient': to_hex(raiden.address),
+  /** The current height of the chain */
+  chain_height: number;
 };
 
 export type RaidenResolveResponse = {

lib/swaps/Swaps.ts

@@ -727,25 +727,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:
@@ -870,10 +867,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 {
@@ -886,7 +883,7 @@ class Swaps extends EventEmitter {
       return preimage;
     } catch (err) {
       this.logger.error(err.message);
-      return err.message;
+      throw err;
     }
   }
 
@@ -1001,6 +998,7 @@ class Swaps extends EventEmitter {
         break;
       default:
         assert.fail('unknown deal phase');
+        break;
     }
 
     deal.phase = newPhase;

lib/swaps/errors.ts

@@ -6,7 +6,8 @@ const errorCodes = {
   SWAP_CLIENT_NOT_CONFIGURED: codesPrefix.concat('.2'),
   PAYMENT_HASH_NOT_FOUND: codesPrefix.concat('.3'),
   PAYMENT_ERROR: codesPrefix.concat('.4'),
-  PAYMENT_REJECTED: codesPrefix.concat('.4'),
+  PAYMENT_REJECTED: codesPrefix.concat('.5'),
+  INVALID_RESOLVE_REQUEST: codesPrefix.concat('.6'),
 };
 
 const errors = {
@@ -30,6 +31,10 @@ const errors = {
     message: 'the recipient rejected our payment for the swap',
     code: errorCodes.PAYMENT_REJECTED,
   },
+  INVALID_RESOLVE_REQUEST: (rHash: string, errorMessage: string) => ({
+    message: `invalid resolve request for rHash ${rHash}: ${errorMessage}`,
+    code: errorCodes.INVALID_RESOLVE_REQUEST,
+  }),
 };
 
 export { errorCodes, errors };

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,
 };

test/jest/HttpService.spec.ts

@@ -6,14 +6,15 @@ jest.mock('../../lib/service/Service');
 const mockedService = <jest.Mock<Service>><any>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,
 };
 
 describe('HttpService', () => {
@@ -35,6 +36,7 @@ describe('HttpService', () => {
     expect(service.resolveHash)
       .toHaveBeenCalledWith({
         expiration,
+        chain_height,
         amount: resolveRequest.amount,
         rHash: secretHash,
         tokenAddress: token,