Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

fix: unify the response data from /swap/v0/price and /meta_transaction/v0/price #228

Merged
merged 4 commits into from
May 19, 2020
Merged
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
5 changes: 3 additions & 2 deletions src/handlers/meta_transaction_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { logger } from '../logger';
import { isAPIError, isRevertError } from '../middleware/error_handling';
import { schemas } from '../schemas/schemas';
import { MetaTransactionService } from '../services/meta_transaction_service';
import { GetTransactionRequestParams, ZeroExTransactionWithoutDomain } from '../types';
import { GetMetaTransactionPriceResponse, GetTransactionRequestParams, ZeroExTransactionWithoutDomain } from '../types';
import { parseUtils } from '../utils/parse_utils';
import { schemaUtils } from '../utils/schema_utils';
import { findTokenAddressOrThrowApiError } from '../utils/token_metadata_utils';
Expand Down Expand Up @@ -147,12 +147,13 @@ export class MetaTransactionHandlers {
},
'price',
);
const metaTransactionPriceResponse = {
const metaTransactionPriceResponse: GetMetaTransactionPriceResponse = {
price: metaTransactionPrice.price,
buyAmount: metaTransactionPrice.buyAmount,
sellAmount: metaTransactionPrice.sellAmount,
sellTokenAddress,
buyTokenAddress,
sources: metaTransactionPrice.sources,
};
res.status(HttpStatus.OK).send(metaTransactionPriceResponse);
} catch (e) {
Expand Down
36 changes: 33 additions & 3 deletions src/handlers/swap_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ import { isAPIError, isRevertError } from '../middleware/error_handling';
import { schemas } from '../schemas/schemas';
import { SwapService } from '../services/swap_service';
import { TokenMetadatasForChains } from '../token_metadatas_for_networks';
import { CalculateSwapQuoteParams, GetSwapQuoteRequestParams, GetSwapQuoteResponse } from '../types';
import {
CalculateSwapQuoteParams,
GetSwapPriceResponse,
GetSwapQuoteRequestParams,
GetSwapQuoteResponse,
} from '../types';
import { parseUtils } from '../utils/parse_utils';
import { schemaUtils } from '../utils/schema_utils';
import {
Expand Down Expand Up @@ -70,7 +75,19 @@ export class SwapHandlers {
const params = parseGetSwapQuoteRequestParams(req, 'price');
params.skipValidation = true;
const quote = await this._calculateSwapQuoteAsync(params);
const { price, value, gasPrice, gas, protocolFee, buyAmount, sellAmount, sources, orders } = quote;
const {
price,
value,
gasPrice,
gas,
protocolFee,
buyAmount,
sellAmount,
sources,
orders,
buyTokenAddress,
sellTokenAddress,
} = quote;
logger.info({
indicativeQuoteServed: {
taker: params.takerAddress,
Expand All @@ -82,7 +99,20 @@ export class SwapHandlers {
makers: orders.map(o => o.makerAddress),
},
});
res.status(HttpStatus.OK).send({ price, value, gasPrice, gas, protocolFee, buyAmount, sellAmount, sources });

const response: GetSwapPriceResponse = {
price,
value,
gasPrice,
gas,
protocolFee,
buyTokenAddress,
buyAmount,
sellTokenAddress,
sellAmount,
sources,
};
res.status(HttpStatus.OK).send(response);
}
// tslint:disable-next-line:prefer-function-over-method
public async getTokenPricesAsync(req: express.Request, res: express.Response): Promise<void> {
Expand Down
4 changes: 2 additions & 2 deletions src/services/meta_transaction_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ export class MetaTransactionService {

const response: CalculateMetaTransactionPriceResponse = {
takerAddress,
sellAmount,
buyAmount,
buyAmount: makerAssetAmount,
sellAmount: totalTakerAssetAmount,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that the metatx endpoint would only return the argument you pass to it, whereas the swap endpoint always returns buyAmount and sellAmount.

price,
swapQuote,
sources: serviceUtils.convertSourceBreakdownToArray(swapQuote.sourceBreakdown),
Expand Down
22 changes: 17 additions & 5 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,22 @@ export interface Price {
price: BigNumber;
}

interface BasePriceResponse {
price: BigNumber;
buyAmount: BigNumber;
sellAmount: BigNumber;
sellTokenAddress: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This uses the token address to be in line with the quote endpoint. However, the prices endpoint returns symbols.

Either symbol or token address would work as long as it's consistent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah exactly.

It would be nice to have consistency across the board, if we go with token address here maybe we should add it there as well or vice versa.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it should be consistently address since we cannot guarantee symbol.

buyTokenAddress: string;
sources: GetSwapQuoteResponseLiquiditySource[];
}

export interface GetSwapPriceResponse extends BasePriceResponse {
value: BigNumber;
gasPrice: BigNumber;
gas: BigNumber;
protocolFee: BigNumber;
}

export type GetTokenPricesResponse = Price[];

export interface GetMetaTransactionQuoteResponse {
Expand All @@ -381,11 +397,7 @@ export interface GetMetaTransactionQuoteResponse {
sources: GetSwapQuoteResponseLiquiditySource[];
}

export interface GetMetaTransactionPriceResponse {
price: BigNumber;
buyAmount: BigNumber;
sellAmount: BigNumber;
}
export interface GetMetaTransactionPriceResponse extends BasePriceResponse {}

// takerAddress, sellAmount, buyAmount, swapQuote, price
export interface CalculateMetaTransactionPriceResponse {
Expand Down
33 changes: 18 additions & 15 deletions test/meta_transaction_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { GetMetaTransactionQuoteResponse } from '../src/types';
import { setupApiAsync, setupMeshAsync, teardownApiAsync, teardownMeshAsync } from './utils/deployment';
import { constructRoute, httpGetAsync, httpPostAsync } from './utils/http_utils';
import { DEFAULT_MAKER_ASSET_AMOUNT, MeshTestUtils } from './utils/mesh_test_utils';
import { liquiditySources0xOnly } from './utils/mocks';

const SUITE_NAME = 'meta transactions tests';

Expand Down Expand Up @@ -212,6 +213,8 @@ describe(SUITE_NAME, () => {

context('success tests', () => {
let meshUtils: MeshTestUtils;
const price = '1';
const sellAmount = calculateSellAmount(buyAmount, price);

beforeEach(async () => {
await blockchainLifecycle.startAsync();
Expand Down Expand Up @@ -239,10 +242,12 @@ describe(SUITE_NAME, () => {
expect(response.type).to.be.eq('application/json');
expect(response.status).to.be.eq(HttpStatus.OK);
expect(response.body).to.be.deep.eq({
price: '1',
price,
buyAmount,
sellAmount,
sellTokenAddress,
buyTokenAddress,
sources: liquiditySources0xOnly,
});
});

Expand All @@ -260,17 +265,21 @@ describe(SUITE_NAME, () => {
expect(response.type).to.be.eq('application/json');
expect(response.status).to.be.eq(HttpStatus.OK);
expect(response.body).to.be.deep.eq({
price: '1',
price,
buyAmount,
sellAmount,
sellTokenAddress,
buyTokenAddress,
sources: liquiditySources0xOnly,
});
});

it('should show the price of the combination of the two orders in Mesh', async () => {
const validationResults = await meshUtils.addOrdersWithPricesAsync([1, 2]);
expect(validationResults.rejected.length, 'mesh should not reject any orders').to.be.eq(0);
const largeOrderPrice = '1.5';
const largeBuyAmount = DEFAULT_MAKER_ASSET_AMOUNT.times(2).toString();
const largeSellAmount = calculateSellAmount(largeBuyAmount, largeOrderPrice);
const route = constructRoute({
baseRoute: `${META_TRANSACTION_PATH}/price`,
queryParams: {
Expand All @@ -283,10 +292,12 @@ describe(SUITE_NAME, () => {
expect(response.type).to.be.eq('application/json');
expect(response.status).to.be.eq(HttpStatus.OK);
expect(response.body).to.be.deep.eq({
price: '1.5',
price: largeOrderPrice,
buyAmount: largeBuyAmount,
sellAmount: largeSellAmount,
sellTokenAddress,
buyTokenAddress,
sources: liquiditySources0xOnly,
});
});
});
Expand Down Expand Up @@ -366,18 +377,7 @@ describe(SUITE_NAME, () => {
buyAmount: testCase.expectedBuyAmount,
sellAmount: calculateSellAmount(testCase.expectedBuyAmount, testCase.expectedPrice),
// NOTE(jalextowle): 0x is the only source that is currently being tested.
sources: [
{ name: '0x', proportion: '1' },
{ name: 'Uniswap', proportion: '0' },
{ name: 'Eth2Dai', proportion: '0' },
{ name: 'Kyber', proportion: '0' },
{ name: 'Curve_USDC_DAI', proportion: '0' },
{ name: 'Curve_USDC_DAI_USDT', proportion: '0' },
{ name: 'Curve_USDC_DAI_USDT_TUSD', proportion: '0' },
{ name: 'Curve_USDC_DAI_USDT_BUSD', proportion: '0' },
{ name: 'Curve_USDC_DAI_USDT_SUSD', proportion: '0' },
{ name: 'LiquidityProvider', proportion: '0' },
],
sources: liquiditySources0xOnly,
});
}

Expand Down Expand Up @@ -547,8 +547,10 @@ describe(SUITE_NAME, () => {
expect(response.body).to.be.deep.eq({
price,
buyAmount,
sellAmount,
sellTokenAddress,
buyTokenAddress,
sources: liquiditySources0xOnly,
});
});

Expand Down Expand Up @@ -659,6 +661,7 @@ describe(SUITE_NAME, () => {
buyAmount: largeBuyAmount,
sellTokenAddress,
buyTokenAddress,
sources: liquiditySources0xOnly,
});
});

Expand Down
13 changes: 13 additions & 0 deletions test/utils/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,16 @@ export const rfqtIndicativeQuoteResponse = {
takerAssetData: '0xf47261b00000000000000000000000000b1ba0af832d7c05fd64161e0db78e85978e8082',
expirationTimeSeconds: '1903620548', // in the year 2030
};

export const liquiditySources0xOnly = [
{ name: '0x', proportion: '1' },
{ name: 'Uniswap', proportion: '0' },
{ name: 'Eth2Dai', proportion: '0' },
{ name: 'Kyber', proportion: '0' },
{ name: 'Curve_USDC_DAI', proportion: '0' },
{ name: 'Curve_USDC_DAI_USDT', proportion: '0' },
{ name: 'Curve_USDC_DAI_USDT_TUSD', proportion: '0' },
{ name: 'Curve_USDC_DAI_USDT_BUSD', proportion: '0' },
{ name: 'Curve_USDC_DAI_USDT_SUSD', proportion: '0' },
{ name: 'LiquidityProvider', proportion: '0' },
];