Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions lib/entities/HardQuoteRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,36 @@ export class HardQuoteRequest {
return utils.getAddress(this.order.info.outputs[0].token);
}

/**
* Whether every output pays outputs[0].token.
*
* `totalOutputAmountStart` below sums the outputs as raw integers, and `toCleanJSON`
* sends quoters that one scalar alongside a single `tokenOut` taken from outputs[0].
* Both are only meaningful when the outputs agree on a token: otherwise the amount
* mixes assets of different denominations, and the quoter prices something the order
* does not contain. Multi-output orders are the normal case, since fee outputs use the
* swapper output's token.
*/
public get hasUniformOutputTokens(): boolean {
const tokenOut = this.tokenOut;
for (const output of this.order.info.outputs) {
if (utils.getAddress(output.token) !== tokenOut) {
return false;
}
}

return true;
}

public get outputTokens(): string[] {
const tokens: string[] = [];
for (const output of this.order.info.outputs) {
tokens.push(utils.getAddress(output.token));
}

return tokens;
}

public get totalOutputAmountStart(): BigNumber {
let amount = BigNumber.from(0);
for (const output of this.order.info.outputs) {
Expand Down
2 changes: 2 additions & 0 deletions lib/entities/aws-metrics-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export enum Metric {

QUOTE_POST_ERROR = 'QUOTE_POST_ERROR',
QUOTE_POST_ATTEMPT = 'QUOTE_POST_ATTEMPT',
// Hard quote rejected before the auction because its outputs span multiple tokens.
QUOTE_MIXED_OUTPUT_TOKENS = 'QUOTE_MIXED_OUTPUT_TOKENS',

RFQ_REQUESTED = 'RFQ_REQUESTED',
RFQ_SUCCESS = 'RFQ_SUCCESS',
Expand Down
24 changes: 23 additions & 1 deletion lib/handlers/hard-quote/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ import { V3HardQuoteResponse } from '../../entities/V3HardQuoteResponse';
import { checkDefined } from '../../preconditions/preconditions';
import { getBestQuote } from '../../quoters/best-quote';
import { ChainId } from '../../util/chains';
import { NoQuotesAvailable, OrderDeadlineExpired, OrderPostError, UnknownOrderCosignerError } from '../../util/errors';
import {
MixedOutputTokensError,
NoQuotesAvailable,
OrderDeadlineExpired,
OrderPostError,
UnknownOrderCosignerError,
} from '../../util/errors';
import { timestampInMstoSeconds } from '../../util/time';
import { APIGLambdaHandler } from '../base';
import { APIHandleRequestParams, ErrorResponse, Response } from '../base/api-handler';
Expand Down Expand Up @@ -68,6 +74,22 @@ export class QuoteHandler extends APIGLambdaHandler<
requestBody.tokenInChainId
);
const request = HardQuoteRequest.fromHardRequestBody(requestBody, orderType);

// Reject before the auction rather than after. We quote this order as a single
// tokenOut plus one summed amount, so a quoter pricing an order whose outputs span
// multiple tokens is bidding on something the order does not contain. Winning that
// auction hands it exclusivity on an order it has to fade, which costs it fill rate
// and circuit-breaker standing for a request it was never able to price. The order
// service rejects the same shape at POST /order, so the fill could not happen.
if (!request.hasUniformOutputTokens) {
log.error(
{ tokenOut: request.tokenOut, outputTokens: request.outputTokens, requestId: request.requestId },
'Order outputs span multiple tokens'
);
metric.putMetric(Metric.QUOTE_MIXED_OUTPUT_TOKENS, 1, MetricLoggerUnit.Count);
throw new MixedOutputTokensError();
}

// re-create KmsClient every call to avoid clock skew issue
// https://github.com/aws/aws-sdk-js-v3/issues/6400
const kmsKeyId = checkDefined(process.env.KMS_KEY_ID, 'KMS_KEY_ID is not defined');
Expand Down
22 changes: 22 additions & 0 deletions lib/util/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,28 @@ export class OrderDeadlineExpired extends CustomError {
}
}

export class MixedOutputTokensError extends CustomError {
private static MESSAGE =
'All order outputs must pay the same token. A hard quote is priced as a single tokenOut and a single summed amount, so an order whose outputs span multiple tokens cannot be quoted.';

constructor(message?: string) {
super(message ?? MixedOutputTokensError.MESSAGE);
// Set the prototype explicitly.
Object.setPrototypeOf(this, MixedOutputTokensError.prototype);
}

toJSON(id?: string): APIGatewayProxyResult {
return {
statusCode: 400,
body: JSON.stringify({
errorCode: ErrorCode.ValidationError,
detail: this.message,
id,
}),
};
}
}

export class UnknownOrderCosignerError extends CustomError {
private static MESSAGE = 'Unknown cosigner';

Expand Down
52 changes: 52 additions & 0 deletions test/entities/HardQuoteRequest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,58 @@ describe('QuoteRequest', () => {
expect(request.type).toEqual(TradeType.EXACT_INPUT);
});

describe('hasUniformOutputTokens', () => {
const requestWithOutputs = (outputs: { token: string; startAmount: BigNumber }[]) => {
const order = new UnsignedV2DutchOrder(
getOrderInfo({
outputs: outputs.map((output) => ({
token: output.token,
startAmount: output.startAmount,
endAmount: output.startAmount,
recipient: ethers.constants.AddressZero,
})),
}),
CHAIN_ID
);
return makeRequest({ encodedInnerOrder: order.serialize(), innerSig: '0x' });
};

it('is true for a single output', () => {
expect(requestWithOutputs([{ token: TOKEN_OUT, startAmount: RAW_AMOUNT }]).hasUniformOutputTokens).toBe(true);
});

it('is true for a fee output in the swapper output token', () => {
const request = requestWithOutputs([
{ token: TOKEN_OUT, startAmount: RAW_AMOUNT },
{ token: TOKEN_OUT, startAmount: RAW_AMOUNT.div(1000) },
]);
expect(request.hasUniformOutputTokens).toBe(true);
expect(request.outputTokens).toEqual([TOKEN_OUT, TOKEN_OUT]);
});

it('is true regardless of address casing', () => {
const request = requestWithOutputs([
{ token: TOKEN_OUT.toLowerCase(), startAmount: RAW_AMOUNT },
{ token: TOKEN_OUT, startAmount: RAW_AMOUNT.div(1000) },
]);
expect(request.hasUniformOutputTokens).toBe(true);
});

// The scalar the quoter is asked to beat would otherwise mix 1 WETH with a raw
// 1000000 of a 6 decimal token.
it('is false when a higher output pays a different token', () => {
const request = requestWithOutputs([
{ token: TOKEN_OUT, startAmount: RAW_AMOUNT },
{ token: TOKEN_IN, startAmount: BigNumber.from('1000000') },
]);
expect(request.hasUniformOutputTokens).toBe(false);
expect(request.outputTokens).toEqual([TOKEN_OUT, TOKEN_IN]);
// tokenOut and amount both stay silent about the second token
expect(request.tokenOut).toEqual(TOKEN_OUT);
expect(request.numOutputs).toEqual(2);
});
});

it('toCleanJSON', async () => {
const order = new UnsignedV2DutchOrder(
getOrderInfo({
Expand Down
84 changes: 84 additions & 0 deletions test/handlers/hard-quote/handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,90 @@ describe('Quote handler', () => {
jest.clearAllMocks();
});

describe('orders whose outputs span multiple tokens', () => {
const mixedOutputOrder = () =>
getOrder({
cosigner: cosignerWallet.address,
outputs: [
{
token: TOKEN_OUT,
startAmount: RAW_AMOUNT,
endAmount: RAW_AMOUNT,
recipient: ethers.constants.AddressZero,
},
{
token: TOKEN_IN,
startAmount: BigNumber.from('4800000000000'),
endAmount: BigNumber.from('4800000000000'),
recipient: ethers.constants.AddressZero,
},
],
});

it('are rejected with a 400', async () => {
const quoters = [new MockQuoter(logger, 1, 1)];
const request = await getRequest(mixedOutputOrder());

const response: APIGatewayProxyResult = await getQuoteHandler(quoters).handler(
getEvent(request),
{} as unknown as Context
);

expect(response.statusCode).toEqual(400);
expect(JSON.parse(response.body).detail).toContain('All order outputs must pay the same token');
});

// The point of rejecting here rather than after the auction: a quoter that wins on a
// price it could not have computed gets exclusivity on an order it has to fade.
it('never reach the quoters', async () => {
const quoter = new MockQuoter(logger, 1, 1);
const quoteSpy = jest.spyOn(quoter, 'quote');
const request = await getRequest(mixedOutputOrder());

await getQuoteHandler([quoter]).handler(getEvent(request), {} as unknown as Context);

expect(quoteSpy).not.toHaveBeenCalled();
});

it('are rejected before the order is cosigned', async () => {
const request = await getRequest(mixedOutputOrder());

await getQuoteHandler([new MockQuoter(logger, 1, 1)]).handler(getEvent(request), {} as unknown as Context);

expect(mockSignDigest).not.toHaveBeenCalled();
});

it('still accepts a fee output in the swapper output token', async () => {
const quoters = [new MockQuoter(logger, 1, 1)];
const request = await getRequest(
getOrder({
cosigner: cosignerWallet.address,
outputs: [
{
token: TOKEN_OUT,
startAmount: RAW_AMOUNT,
endAmount: RAW_AMOUNT,
recipient: ethers.constants.AddressZero,
},
{
token: TOKEN_OUT,
startAmount: RAW_AMOUNT.div(1000),
endAmount: RAW_AMOUNT.div(1000),
recipient: ethers.constants.AddressZero,
},
],
})
);

const response: APIGatewayProxyResult = await getQuoteHandler(quoters).handler(
getEvent(request),
{} as unknown as Context
);

expect(response.statusCode).toEqual(200);
});
});

it('Simple request and response', async () => {
const quoters = [new MockQuoter(logger, 1, 1)];
const request = await getRequest(getOrder({ cosigner: cosignerWallet.address }));
Expand Down
Loading