Skip to content

Commit

Permalink
fix: sanitize quote response amount
Browse files Browse the repository at this point in the history
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
  • Loading branch information
marktoda committed Feb 9, 2024
1 parent 7b420e0 commit 7ed45eb
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 16 deletions.
11 changes: 9 additions & 2 deletions lib/entities/QuoteResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
),
Expand Down
7 changes: 4 additions & 3 deletions lib/entities/analytics-events.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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
Expand All @@ -27,4 +28,4 @@ export class AnalyticsEvent {
this.eventTime = timestampInMstoISOString(Date.now());
this.eventProperties = eventProperties;
}
};
}
2 changes: 1 addition & 1 deletion lib/entities/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export * from './analytics-events';
export * from './aws-metrics-logger';
export * from './QuoteRequest';
export * from './QuoteResponse';
export * from './analytics-events';
4 changes: 2 additions & 2 deletions test/entities/QuoteResponse.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
});
Expand Down
16 changes: 8 additions & 8 deletions test/providers/quoters/WebhookQuoter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('WebhookQuoter tests', () => {
data: {
...quote,
tokenIn: request.tokenOut,
tokenOut: request.tokenIn,
tokenOut: request.tokenIn,
}
});
});
Expand All @@ -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,
Expand All @@ -127,7 +127,7 @@ describe('WebhookQuoter tests', () => {
data: {
...quote,
tokenIn: request.tokenOut,
tokenOut: request.tokenIn,
tokenOut: request.tokenIn,
}
});
});
Expand Down Expand Up @@ -223,7 +223,7 @@ describe('WebhookQuoter tests', () => {
data: {
...quote,
tokenIn: request.tokenOut,
tokenOut: request.tokenIn,
tokenOut: request.tokenIn,
}
});
});
Expand Down Expand Up @@ -265,7 +265,7 @@ describe('WebhookQuoter tests', () => {
data: {
...quote,
tokenIn: request.tokenOut,
tokenOut: request.tokenIn,
tokenOut: request.tokenIn,
}
});
});
Expand Down Expand Up @@ -301,7 +301,7 @@ describe('WebhookQuoter tests', () => {
data: {
...quote,
tokenIn: request.tokenOut,
tokenOut: request.tokenIn,
tokenOut: request.tokenIn,
}
});
});
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 7ed45eb

Please sign in to comment.