Skip to content

Commit

Permalink
fix(swaps): validate Raiden's resolve request
Browse files Browse the repository at this point in the history
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' }`.
  • Loading branch information
Karl Ranna committed Aug 23, 2019
1 parent 9e027b4 commit bd57361
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 15 deletions.
5 changes: 4 additions & 1 deletion lib/http/HttpServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/http/HttpService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
Expand Down
1 change: 1 addition & 0 deletions lib/raidenclient/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 9 additions & 12 deletions lib/swaps/Swaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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 {
Expand All @@ -878,7 +875,7 @@ class Swaps extends EventEmitter {
return preimage;
} catch (err) {
this.logger.error(err.message);
return err.message;
throw err;
}
}

Expand Down
5 changes: 5 additions & 0 deletions lib/swaps/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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 };
1 change: 1 addition & 0 deletions lib/swaps/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,5 @@ export type ResolveRequest = {
tokenAddress: string,
/** The number of blocks before the incoming payment expires. */
expiration: number,
chain_height: number,
};
5 changes: 4 additions & 1 deletion test/jest/HttpService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ 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,
Expand All @@ -35,6 +37,7 @@ describe('HttpService', () => {
expect(service.resolveHash)
.toHaveBeenCalledWith({
expiration,
chain_height,
amount: resolveRequest.amount,
rHash: secretHash,
tokenAddress: token,
Expand Down

0 comments on commit bd57361

Please sign in to comment.