Skip to content

Commit

Permalink
Merge pull request #260 from Uniswap/add_more_mm_response_validation
Browse files Browse the repository at this point in the history 
fix: add more MM response validation
  • Loading branch information
@ConjunctiveNormalForm
ConjunctiveNormalForm authored Jan 30, 2024
2 parents 4ba4e10 + 4218aea commit 74eb597
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 27 deletions.
27 changes: 24 additions & 3 deletions lib/entities/QuoteResponse.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { TradeType } from '@uniswap/sdk-core';
import { BigNumber } from 'ethers';
import { ValidationResult } from 'joi';
import { v4 as uuidv4 } from 'uuid';

import { PostQuoteResponse, RfqResponse, RfqResponseJoi } from '../handlers/quote/schema';
Expand All @@ -16,9 +15,14 @@ export interface QuoteResponseData
quoteId: string;
}

type ValidationError = {
message: string | undefined;
value: { [key: string]: any };
};

interface ValidatedResponse {
response: QuoteResponse;
validation: ValidationResult<QuoteResponse>;
validationError?: ValidationError;
}

// data class for QuoteRequest helpers and conversions
Expand All @@ -43,10 +47,27 @@ export class QuoteResponse implements QuoteResponseData {
}

public static fromRFQ(request: QuoteRequestData, data: RfqResponse, type: TradeType): ValidatedResponse {
let validationError: ValidationError | undefined;

const responseValidation = RfqResponseJoi.validate(data, {
allowUnknown: true,
stripUnknown: true,
});

if (responseValidation.error) {
validationError = {
message: responseValidation.error?.message,
value: data,
};
}

if (request.tokenIn !== data.tokenIn || request.tokenOut !== data.tokenOut) {
validationError = {
message: `RFQ response token mismatch: request tokenIn: ${request.tokenIn} tokenOut: ${request.tokenOut} response tokenIn: ${data.tokenIn} tokenOut: ${data.tokenOut}`,
value: data,
};
}

return {
response: new QuoteResponse(
{
Expand All @@ -58,7 +79,7 @@ export class QuoteResponse implements QuoteResponseData {
},
type
),
validation: responseValidation,
...(validationError && { validationError }),
};
}

Expand Down
12 changes: 6 additions & 6 deletions lib/quoters/WebhookQuoter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import axios, { AxiosError, AxiosResponse } from 'axios';
import Logger from 'bunyan';
import { v4 as uuidv4 } from 'uuid';

import { Quoter, QuoterType } from '.';
import {
AnalyticsEvent,
AnalyticsEventType,
Expand All @@ -19,6 +18,7 @@ import { FirehoseLogger } from '../providers/analytics';
import { CircuitBreakerConfigurationProvider } from '../providers/circuit-breaker';
import { FillerComplianceConfigurationProvider } from '../providers/compliance';
import { timestampInMstoISOString } from '../util/time';
import { Quoter, QuoterType } from '.';

// TODO: shorten, maybe take from env config
const WEBHOOK_TIMEOUT_MS = 500;
Expand Down Expand Up @@ -152,7 +152,7 @@ export class WebhookQuoter implements Quoter {
latencyMs: Date.now() - before,
};

const { response, validation } = QuoteResponse.fromRFQ(request, hookResponse.data, request.type);
const { response, validationError } = QuoteResponse.fromRFQ(request, hookResponse.data, request.type);

// RFQ provider explicitly elected not to quote
if (isNonQuote(request, hookResponse, response)) {
Expand All @@ -176,12 +176,12 @@ export class WebhookQuoter implements Quoter {
}

// RFQ provider response failed validation
if (validation.error) {
if (validationError) {
metric.putMetric(Metric.RFQ_FAIL_VALIDATION, 1, MetricLoggerUnit.Count);
metric.putMetric(metricContext(Metric.RFQ_FAIL_VALIDATION, name), 1, MetricLoggerUnit.Count);
this.log.error(
{
error: validation.error?.details,
error: validationError,
response,
webhookUrl: endpoint,
},
Expand All @@ -192,7 +192,7 @@ export class WebhookQuoter implements Quoter {
...requestContext,
...rawResponse,
responseType: WebhookResponseType.VALIDATION_ERROR,
validationError: validation.error?.details,
validationError: validationError,
})
);
return null;
Expand Down Expand Up @@ -246,7 +246,7 @@ export class WebhookQuoter implements Quoter {
if (
opposingResponse &&
!isNonQuote(opposingRequest, opposite, opposingResponse.response) &&
!opposingResponse.validation.error
!opposingResponse.validationError
) {
this.log.info({
eventType: 'QuoteResponse',
Expand Down
159 changes: 159 additions & 0 deletions test/entities/QuoteResponse.test.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import { TradeType } from '@uniswap/sdk-core';
import { BigNumber } from 'ethers';
import { parseEther } from 'ethers/lib/utils';

import { QuoteResponse } from '../../lib/entities';

const QUOTE_ID = 'a83f397c-8ef4-4801-a9b7-6e79155049f6';
const REQUEST_ID = 'a83f397c-8ef4-4801-a9b7-6e79155049f7';
const SWAPPER = '0x0000000000000000000000000000000000000000';
const TOKEN_IN = '0x1f9840a85d5aF5bf1D1762F925BDADdC4201F984';
const TOKEN_OUT = '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2';
const CHAIN_ID = 1;
const fixedTime = 4206969;
jest.spyOn(Date, 'now').mockImplementation(() => fixedTime);

describe('QuoteRequest', () => {
afterEach(() => {
jest.clearAllMocks();
});

const quoteResponse = new QuoteResponse(
{
chainId: CHAIN_ID,
amountOut: parseEther('1'),
amountIn: parseEther('1'),
quoteId: QUOTE_ID,
requestId: REQUEST_ID,
swapper: SWAPPER,
tokenIn: TOKEN_IN,
tokenOut: TOKEN_OUT,
},
TradeType.EXACT_INPUT
);
const quoteRequest = {
tokenInChainId: CHAIN_ID,
tokenOutChainId: CHAIN_ID,
requestId: REQUEST_ID,
swapper: SWAPPER,
tokenIn: TOKEN_IN,
tokenOut: TOKEN_OUT,
amount: parseEther('1'),
type: TradeType.EXACT_INPUT,
numOutputs: 1,
};

it('fromRequest', async () => {
const response = QuoteResponse.fromRequest(quoteRequest, parseEther('1'));
expect(response.createdAt).toBe(quoteResponse.createdAt);
expect(response.amountIn).toEqual(quoteResponse.amountIn);
expect(response.amountOut).toEqual(quoteResponse.amountOut);
expect(response.requestId).toBe(quoteResponse.requestId);
expect(response.swapper).toBe(quoteResponse.swapper);
expect(response.tokenIn).toBe(quoteResponse.tokenIn);
expect(response.tokenOut).toBe(quoteResponse.tokenOut);
});

describe('fromRFQ', () => {
it('fromRFQ with valid response', async () => {
const response = QuoteResponse.fromRFQ(
quoteRequest,
{
chainId: CHAIN_ID,
requestId: REQUEST_ID,
tokenIn: TOKEN_IN,
amountIn: parseEther('1').toString(),
tokenOut: TOKEN_OUT,
amountOut: parseEther('1').toString(),
quoteId: QUOTE_ID,
},
TradeType.EXACT_INPUT
);
expect(response.response).toEqual(quoteResponse);
expect(response.validationError).toBe(undefined);
});

it('fromRFQ with invalid response - wrong type amountIn', async () => {
const invalidResponse = {
chainId: CHAIN_ID,
requestId: REQUEST_ID,
tokenIn: TOKEN_IN,
amountIn: 100 as any,
tokenOut: TOKEN_OUT,
amountOut: parseEther('1').toString(),
quoteId: QUOTE_ID,
};
const response = QuoteResponse.fromRFQ(quoteRequest, invalidResponse, TradeType.EXACT_INPUT);
expect(response.response.amountIn).toEqual(BigNumber.from(100));
expect(response.validationError?.message).toBe('"amountIn" must be a string');
expect(response.validationError?.value).toBe(invalidResponse);
});

it('fromRFQ with invalid response - mismatched tokenIn', async () => {
const invalidResponse = {
chainId: CHAIN_ID,
requestId: REQUEST_ID,
tokenIn: '0x0000000000000000000000000000000000000000',
amountIn: parseEther('1').toString(),
tokenOut: TOKEN_OUT,
amountOut: parseEther('1').toString(),
quoteId: QUOTE_ID,
};
const response = QuoteResponse.fromRFQ(quoteRequest, invalidResponse, TradeType.EXACT_INPUT);
expect(response.response.tokenIn).toEqual('0x0000000000000000000000000000000000000000');
expect(response.validationError?.message).toBe(
'RFQ response token mismatch: request tokenIn: 0x1f9840a85d5aF5bf1D1762F925BDADdC4201F984 tokenOut: 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2 response tokenIn: 0x0000000000000000000000000000000000000000 tokenOut: 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2'
);
expect(response.validationError?.value).toBe(invalidResponse);
});

it('fromRFQ with invalid response - mismatched tokenOut', async () => {
const invalidResponse = {
chainId: CHAIN_ID,
requestId: REQUEST_ID,
tokenIn: TOKEN_IN,
amountIn: parseEther('1').toString(),
tokenOut: '0x0000000000000000000000000000000000000000',
amountOut: parseEther('1').toString(),
quoteId: QUOTE_ID,
};
const response = QuoteResponse.fromRFQ(quoteRequest, invalidResponse, TradeType.EXACT_INPUT);
expect(response.response.tokenOut).toEqual('0x0000000000000000000000000000000000000000');
expect(response.validationError?.message).toBe(
'RFQ response token mismatch: request tokenIn: 0x1f9840a85d5aF5bf1D1762F925BDADdC4201F984 tokenOut: 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2 response tokenIn: 0x1f9840a85d5aF5bf1D1762F925BDADdC4201F984 tokenOut: 0x0000000000000000000000000000000000000000'
);
expect(response.validationError?.value).toBe(invalidResponse);
});
});

it('toResponseJSON', async () => {
expect(quoteResponse.toResponseJSON()).toEqual({
chainId: CHAIN_ID,
amountOut: parseEther('1').toString(),
amountIn: parseEther('1').toString(),
quoteId: QUOTE_ID,
requestId: REQUEST_ID,
swapper: SWAPPER,
tokenIn: TOKEN_IN,
tokenOut: TOKEN_OUT,
filler: undefined,
});
});

it('toLog', async () => {
expect(quoteResponse.toLog()).toEqual({
createdAt: expect.any(String),
createdAtMs: expect.any(String),
amountOut: parseEther('1').toString(),
amountIn: parseEther('1').toString(),
quoteId: QUOTE_ID,
requestId: REQUEST_ID,
swapper: SWAPPER,
tokenIn: TOKEN_IN,
tokenOut: TOKEN_OUT,
filler: undefined,
tokenInChainId: CHAIN_ID,
tokenOutChainId: CHAIN_ID,
});
});
});
28 changes: 10 additions & 18 deletions test/providers/quoters/WebhookQuoter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,10 @@ describe('WebhookQuoter tests', () => {

expect(logger.error).toHaveBeenCalledWith(
{
error: [
{
context: { key: 'amountIn', label: 'amountIn' },
message: '"amountIn" is required',
path: ['amountIn'],
type: 'any.required',
},
],
error: {
message: '"amountIn" is required',
value: expect.any(Object),
},
response: {
createdAt: expect.any(String),
createdAtMs: expect.any(String),
Expand All @@ -383,16 +379,12 @@ describe('WebhookQuoter tests', () => {
status: 200,
data: quote,
responseType: WebhookResponseType.VALIDATION_ERROR,
validationError: [
{
context: { key: 'amountIn', label: 'amountIn' },
message: '"amountIn" is required',
path: ['amountIn'],
type: 'any.required',
},
],
}
}),
validationError: {
message: '"amountIn" is required',
value: expect.any(Object),
},
},
})
);
expect(response).toEqual([]);
});
Expand Down

0 comments on commit 74eb597

Please sign in to comment.