From 7ed45ebea741f65dcea35313dfb36a2120fa0540 Mon Sep 17 00:00:00 2001 From: Mark Toda Date: Fri, 9 Feb 2024 14:58:02 -0500 Subject: [PATCH] fix: sanitize quote response amount I realized that we build quote responses with the _specified_ swap amount returned by the quoter as well as the unspecified one. This could cause downstream bugs if we assume the values returned by quoter are the ones expected by swapper. this commit just munches the specified amount to be the requested one from the swapper --- lib/entities/QuoteResponse.ts | 11 +++++++++-- lib/entities/analytics-events.ts | 7 ++++--- lib/entities/index.ts | 2 +- test/entities/QuoteResponse.test.ts | 4 ++-- test/providers/quoters/WebhookQuoter.test.ts | 16 ++++++++-------- 5 files changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/entities/QuoteResponse.ts b/lib/entities/QuoteResponse.ts index 050bcf8f..b9267932 100644 --- a/lib/entities/QuoteResponse.ts +++ b/lib/entities/QuoteResponse.ts @@ -61,6 +61,7 @@ export class QuoteResponse implements QuoteResponseData { }; } + // ensure quoted tokens match if ( request?.tokenIn?.toLowerCase() !== data?.tokenIn?.toLowerCase() || request?.tokenOut?.toLowerCase() !== data?.tokenOut?.toLowerCase() @@ -71,14 +72,20 @@ export class QuoteResponse implements QuoteResponseData { }; } + // take quoted amount from RFQ response + // but specified amount from request to avoid any inaccuracies from incorrect echoed response + const [amountIn, amountOut] = + request.type === TradeType.EXACT_INPUT + ? [request.amount, BigNumber.from(data.amountOut ?? 0)] + : [BigNumber.from(data.amountIn ?? 0), request.amount]; return { response: new QuoteResponse( { ...data, quoteId: data.quoteId ?? uuidv4(), swapper: request.swapper, - amountIn: BigNumber.from(data.amountIn ?? 0), - amountOut: BigNumber.from(data.amountOut ?? 0), + amountIn, + amountOut, }, type ), diff --git a/lib/entities/analytics-events.ts b/lib/entities/analytics-events.ts index 93345e22..7ef15c09 100644 --- a/lib/entities/analytics-events.ts +++ b/lib/entities/analytics-events.ts @@ -1,9 +1,10 @@ import { v4 as uuidv4 } from 'uuid'; + import { timestampInMstoISOString } from '../util/time'; export enum AnalyticsEventType { WEBHOOK_RESPONSE = 'WebhookQuoterResponse', -}; +} export enum WebhookResponseType { OK = 'OK', @@ -13,7 +14,7 @@ export enum WebhookResponseType { TIMEOUT = 'TIMEOUT', HTTP_ERROR = 'HTTP_ERROR', OTHER_ERROR = 'OTHER_ERROR', -}; +} export class AnalyticsEvent { eventId: string; // gets set in constructor @@ -27,4 +28,4 @@ export class AnalyticsEvent { this.eventTime = timestampInMstoISOString(Date.now()); this.eventProperties = eventProperties; } -}; +} diff --git a/lib/entities/index.ts b/lib/entities/index.ts index 629a371f..2e97229b 100644 --- a/lib/entities/index.ts +++ b/lib/entities/index.ts @@ -1,4 +1,4 @@ +export * from './analytics-events'; export * from './aws-metrics-logger'; export * from './QuoteRequest'; export * from './QuoteResponse'; -export * from './analytics-events'; diff --git a/test/entities/QuoteResponse.test.ts b/test/entities/QuoteResponse.test.ts index 5567f333..6b3fa775 100644 --- a/test/entities/QuoteResponse.test.ts +++ b/test/entities/QuoteResponse.test.ts @@ -1,5 +1,4 @@ import { TradeType } from '@uniswap/sdk-core'; -import { BigNumber } from 'ethers'; import { parseEther } from 'ethers/lib/utils'; import { QuoteResponse } from '../../lib/entities'; @@ -101,7 +100,8 @@ describe('QuoteRequest', () => { quoteId: QUOTE_ID, }; const response = QuoteResponse.fromRFQ(quoteRequest, invalidResponse, TradeType.EXACT_INPUT); - expect(response.response.amountIn).toEqual(BigNumber.from(100)); + // ensure we overwrite amount with the request amount, dont just accept what the quoter returned + expect(response.response.amountIn).toEqual(quoteRequest.amount); expect(response.validationError?.message).toBe('"amountIn" must be a string'); expect(response.validationError?.value).toBe(invalidResponse); }); diff --git a/test/providers/quoters/WebhookQuoter.test.ts b/test/providers/quoters/WebhookQuoter.test.ts index 4595e51f..9d2d64b9 100644 --- a/test/providers/quoters/WebhookQuoter.test.ts +++ b/test/providers/quoters/WebhookQuoter.test.ts @@ -94,7 +94,7 @@ describe('WebhookQuoter tests', () => { data: { ...quote, tokenIn: request.tokenOut, - tokenOut: request.tokenIn, + tokenOut: request.tokenIn, } }); }); @@ -103,7 +103,7 @@ describe('WebhookQuoter tests', () => { expect(response.length).toEqual(1); expect(response[0].toResponseJSON()).toEqual({ ...quote, quoteId: expect.any(String) }); }); - + it('Respects filler compliance requirements', async () => { const webhookQuoter = new WebhookQuoter( logger, @@ -127,7 +127,7 @@ describe('WebhookQuoter tests', () => { data: { ...quote, tokenIn: request.tokenOut, - tokenOut: request.tokenIn, + tokenOut: request.tokenIn, } }); }); @@ -223,7 +223,7 @@ describe('WebhookQuoter tests', () => { data: { ...quote, tokenIn: request.tokenOut, - tokenOut: request.tokenIn, + tokenOut: request.tokenIn, } }); }); @@ -265,7 +265,7 @@ describe('WebhookQuoter tests', () => { data: { ...quote, tokenIn: request.tokenOut, - tokenOut: request.tokenIn, + tokenOut: request.tokenIn, } }); }); @@ -301,7 +301,7 @@ describe('WebhookQuoter tests', () => { data: { ...quote, tokenIn: request.tokenOut, - tokenOut: request.tokenIn, + tokenOut: request.tokenIn, } }); }); @@ -363,7 +363,7 @@ describe('WebhookQuoter tests', () => { ...quote, quoteId: expect.any(String), amountOut: BigNumber.from(quote.amountOut), - amountIn: BigNumber.from(0), + amountIn: BigNumber.from(request.amount), }, type: 0, }, @@ -416,7 +416,7 @@ describe('WebhookQuoter tests', () => { responseRequestId: quote.requestId, }, 'Webhook ResponseId does not match request' - ); + ); expect(mockFirehoseLogger.sendAnalyticsEvent).toHaveBeenCalledWith( expect.objectContaining({ eventType: AnalyticsEventType.WEBHOOK_RESPONSE,