diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index f9c71f059d..913016b400 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "16.49.4", + "changes": [ + { + "note": "Reverts 'Improve Uniswap V3 gas schedule' due to issue with buys", + "pr": 419 + } + ] + }, { "version": "16.49.3", "changes": [ diff --git a/packages/asset-swapper/contracts/src/UniswapV3Sampler.sol b/packages/asset-swapper/contracts/src/UniswapV3Sampler.sol index 93311e87de..188919a9df 100644 --- a/packages/asset-swapper/contracts/src/UniswapV3Sampler.sol +++ b/packages/asset-swapper/contracts/src/UniswapV3Sampler.sol @@ -22,43 +22,17 @@ pragma experimental ABIEncoderV2; import "@0x/contracts-erc20/contracts/src/v06/IERC20TokenV06.sol"; -interface IUniswapV3QuoterV2 { +interface IUniswapV3Quoter { function factory() external view returns (IUniswapV3Factory factory); - - // @notice Returns the amount out received for a given exact input swap without executing the swap - // @param path The path of the swap, i.e. each token pair and the pool fee - // @param amountIn The amount of the first token to swap - // @return amountOut The amount of the last token that would be received - // @return sqrtPriceX96AfterList List of the sqrt price after the swap for each pool in the path - // @return initializedTicksCrossedList List of the initialized ticks that the swap crossed for each pool in the path - // @return gasEstimate The estimate of the gas that the swap consumes function quoteExactInput(bytes memory path, uint256 amountIn) external - returns ( - uint256 amountOut, - uint160[] memory sqrtPriceX96AfterList, - uint32[] memory initializedTicksCrossedList, - uint256 gasEstimate - ); - - // @notice Returns the amount in required for a given exact output swap without executing the swap - // @param path The path of the swap, i.e. each token pair and the pool fee. Path must be provided in reverse order - // @param amountOut The amount of the last token to receive - // @return amountIn The amount of first token required to be paid - // @return sqrtPriceX96AfterList List of the sqrt price after the swap for each pool in the path - // @return initializedTicksCrossedList List of the initialized ticks that the swap crossed for each pool in the path - // @return gasEstimate The estimate of the gas that the swap consumes + returns (uint256 amountOut); function quoteExactOutput(bytes memory path, uint256 amountOut) external - returns ( - uint256 amountIn, - uint160[] memory sqrtPriceX96AfterList, - uint32[] memory initializedTicksCrossedList, - uint256 gasEstimate - ); + returns (uint256 amountIn); } interface IUniswapV3Factory { @@ -87,15 +61,14 @@ contract UniswapV3Sampler /// @return makerTokenAmounts Maker amounts bought at each taker token /// amount. function sampleSellsFromUniswapV3( - IUniswapV3QuoterV2 quoter, + IUniswapV3Quoter quoter, IERC20TokenV06[] memory path, uint256[] memory takerTokenAmounts ) public returns ( bytes[] memory uniswapPaths, - uint256[] memory makerTokenAmounts, - uint256[] memory uniswapGasUsed + uint256[] memory makerTokenAmounts ) { IUniswapV3Pool[][] memory poolPaths = @@ -103,39 +76,31 @@ contract UniswapV3Sampler makerTokenAmounts = new uint256[](takerTokenAmounts.length); uniswapPaths = new bytes[](takerTokenAmounts.length); - uniswapGasUsed = new uint256[](takerTokenAmounts.length); for (uint256 i = 0; i < takerTokenAmounts.length; ++i) { // Pick the best result from all the paths. + bytes memory topUniswapPath; uint256 topBuyAmount = 0; for (uint256 j = 0; j < poolPaths.length; ++j) { bytes memory uniswapPath = _toUniswapPath(path, poolPaths[j]); - try quoter.quoteExactInput - { gas: QUOTE_GAS } - (uniswapPath, takerTokenAmounts[i]) - returns ( - uint256 buyAmount, - uint160[] memory, /* sqrtPriceX96AfterList */ - uint32[] memory, /* initializedTicksCrossedList */ - uint256 gasUsed - ) + try + quoter.quoteExactInput + { gas: QUOTE_GAS } + (uniswapPath, takerTokenAmounts[i]) + returns (uint256 buyAmount) { if (topBuyAmount <= buyAmount) { topBuyAmount = buyAmount; - uniswapPaths[i] = uniswapPath; - uniswapGasUsed[i] = gasUsed; + topUniswapPath = uniswapPath; } - } catch {} + } catch { } } - // Break early if we can't complete the sells. + // Break early if we can't complete the buys. if (topBuyAmount == 0) { - // HACK(kimpers): To avoid too many local variables, paths and gas used is set directly in the loop - // then reset if no valid valid quote was found - uniswapPaths[i] = ""; - uniswapGasUsed[i] = 0; break; } makerTokenAmounts[i] = topBuyAmount; + uniswapPaths[i] = topUniswapPath; } } @@ -147,15 +112,14 @@ contract UniswapV3Sampler /// @return takerTokenAmounts Taker amounts sold at each maker token /// amount. function sampleBuysFromUniswapV3( - IUniswapV3QuoterV2 quoter, + IUniswapV3Quoter quoter, IERC20TokenV06[] memory path, uint256[] memory makerTokenAmounts ) public returns ( bytes[] memory uniswapPaths, - uint256[] memory takerTokenAmounts, - uint256[] memory uniswapGasUsed + uint256[] memory takerTokenAmounts ) { IUniswapV3Pool[][] memory poolPaths = @@ -164,12 +128,11 @@ contract UniswapV3Sampler takerTokenAmounts = new uint256[](makerTokenAmounts.length); uniswapPaths = new bytes[](makerTokenAmounts.length); - uniswapGasUsed = new uint256[](makerTokenAmounts.length); for (uint256 i = 0; i < makerTokenAmounts.length; ++i) { - uint256 topSellAmount; - // Pick the best result from all the paths. + bytes memory topUniswapPath; + uint256 topSellAmount = 0; for (uint256 j = 0; j < poolPaths.length; ++j) { // quoter requires path to be reversed for buys. bytes memory uniswapPath = _toUniswapPath( @@ -180,30 +143,21 @@ contract UniswapV3Sampler quoter.quoteExactOutput { gas: QUOTE_GAS } (uniswapPath, makerTokenAmounts[i]) - returns ( - uint256 sellAmount, - uint160[] memory, /* sqrtPriceX96AfterList */ - uint32[] memory, /* initializedTicksCrossedList */ - uint256 gasUsed - ) + returns (uint256 sellAmount) { if (topSellAmount == 0 || topSellAmount >= sellAmount) { topSellAmount = sellAmount; // But the output path should still be encoded for sells. - uniswapPaths[i] = _toUniswapPath(path, poolPaths[j]); - uniswapGasUsed[i] = gasUsed; + topUniswapPath = _toUniswapPath(path, poolPaths[j]); } } catch {} } // Break early if we can't complete the buys. if (topSellAmount == 0) { - // HACK(kimpers): To avoid too many local variables, paths and gas used is set directly in the loop - // then reset if no valid valid quote was found - uniswapPaths[i] = ""; - uniswapGasUsed[i] = 0; break; } takerTokenAmounts[i] = topSellAmount; + uniswapPaths[i] = topUniswapPath; } } @@ -282,7 +236,6 @@ contract UniswapV3Sampler function _reverseTokenPath(IERC20TokenV06[] memory tokenPath) private - pure returns (IERC20TokenV06[] memory reversed) { reversed = new IERC20TokenV06[](tokenPath.length); @@ -293,7 +246,6 @@ contract UniswapV3Sampler function _reversePoolPath(IUniswapV3Pool[] memory poolPath) private - pure returns (IUniswapV3Pool[] memory reversed) { reversed = new IUniswapV3Pool[](poolPath.length); diff --git a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts index f43259dc58..823af56f29 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -2,7 +2,6 @@ import { ChainId, getContractAddressesForChainOrThrow } from '@0x/contract-addre import { FillQuoteTransformerOrderType } from '@0x/protocol-utils'; import { BigNumber } from '@0x/utils'; import { formatBytes32String } from '@ethersproject/strings'; -import * as _ from 'lodash'; import { TokenAdjacencyGraphBuilder } from '../token_adjacency_graph_builder'; @@ -18,9 +17,7 @@ import { ERC20BridgeSource, FeeSchedule, FillData, - FinalUniswapV3FillData, GetMarketOrdersOpts, - isFinalUniswapV3FillData, KyberSamplerOpts, LidoInfo, LiquidityProviderFillData, @@ -2042,19 +2039,19 @@ export const BEETHOVEN_X_SUBGRAPH_URL_BY_CHAIN = valueByChainId( export const UNISWAPV3_CONFIG_BY_CHAIN_ID = valueByChainId( { [ChainId.Mainnet]: { - quoter: '0x61ffe014ba17989e743c5f6cb21bf9697530b21e', + quoter: '0xb27308f9f90d607463bb33ea1bebb41c27ce5ab6', router: '0xe592427a0aece92de3edee1f18e0157c05861564', }, [ChainId.Ropsten]: { - quoter: '0x61ffe014ba17989e743c5f6cb21bf9697530b21e', + quoter: '0x2f9e608fd881861b8916257b76613cb22ee0652c', router: '0x03782388516e94fcd4c18666303601a12aa729ea', }, [ChainId.Polygon]: { - quoter: '0x61ffe014ba17989e743c5f6cb21bf9697530b21e', + quoter: '0xb27308f9f90d607463bb33ea1bebb41c27ce5ab6', router: '0xe592427a0aece92de3edee1f18e0157c05861564', }, [ChainId.Optimism]: { - quoter: '0x61ffe014ba17989e743c5f6cb21bf9697530b21e', + quoter: '0xb27308f9f90d607463bb33ea1bebb41c27ce5ab6', router: '0xe592427a0aece92de3edee1f18e0157c05861564', }, }, @@ -2346,33 +2343,11 @@ export const DEFAULT_GAS_SCHEDULE: Required = { return gas; }, [ERC20BridgeSource.UniswapV3]: (fillData?: FillData) => { - const uniFillData = fillData as UniswapV3FillData | FinalUniswapV3FillData; - // NOTE: This base value was heuristically chosen by looking at how much it generally - // underestimated gas usage - const base = 34e3; // 34k base - let gas = base; - if (isFinalUniswapV3FillData(uniFillData)) { - gas += uniFillData.gasUsed; - } else { - // NOTE: We don't actually know which of the paths would be used in the router - // therefore we estimate using the mean of gas prices returned from UniswapV3 - // For the best case scenario (least amount of hops & ticks) this will - // over estimate the gas usage - const pathAmountsWithGasUsed = uniFillData.pathAmounts.filter(p => p.gasUsed > 0); - const meanGasUsedForPath = Math.round(_.meanBy(pathAmountsWithGasUsed, p => p.gasUsed)); - gas += meanGasUsedForPath; + let gas = 100e3; + const path = (fillData as UniswapV3FillData).tokenAddressPath; + if (path.length > 2) { + gas += (path.length - 2) * 32e3; // +32k for each hop. } - - // If we for some reason could not read `gasUsed` when sampling - // fall back to legacy gas estimation - if (gas === base) { - gas = 100e3; - const path = uniFillData.tokenAddressPath; - if (path.length > 2) { - gas += (path.length - 2) * 32e3; // +32k for each hop. - } - } - return gas; }, [ERC20BridgeSource.Lido]: () => 226e3, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts index 7a4256e2d9..860d3b6824 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts @@ -36,7 +36,6 @@ import { ShellFillData, UniswapV2FillData, UniswapV3FillData, - UniswapV3PathAmount, } from './types'; // tslint:disable completed-docs @@ -388,14 +387,11 @@ function createFinalBridgeOrderFillDataFromCollapsedFill(fill: CollapsedFill): F switch (fill.source) { case ERC20BridgeSource.UniswapV3: { const fd = fill.fillData as UniswapV3FillData; - const { uniswapPath, gasUsed } = getBestUniswapV3PathAmountForInputAmount(fd, fill.input); - const finalFillData: FinalUniswapV3FillData = { + return { router: fd.router, tokenAddressPath: fd.tokenAddressPath, - uniswapPath, - gasUsed, + uniswapPath: getBestUniswapV3PathForInputAmount(fd, fill.input), }; - return finalFillData; } default: break; @@ -403,21 +399,18 @@ function createFinalBridgeOrderFillDataFromCollapsedFill(fill: CollapsedFill): F return fill.fillData; } -function getBestUniswapV3PathAmountForInputAmount( - fillData: UniswapV3FillData, - inputAmount: BigNumber, -): UniswapV3PathAmount { +function getBestUniswapV3PathForInputAmount(fillData: UniswapV3FillData, inputAmount: BigNumber): string { if (fillData.pathAmounts.length === 0) { throw new Error(`No Uniswap V3 paths`); } // Find the best path that can satisfy `inputAmount`. // Assumes `fillData.pathAmounts` is sorted ascending. - for (const pathAmount of fillData.pathAmounts) { - if (pathAmount.inputAmount.gte(inputAmount)) { - return pathAmount; + for (const { inputAmount: pathInputAmount, uniswapPath } of fillData.pathAmounts) { + if (pathInputAmount.gte(inputAmount)) { + return uniswapPath; } } - return fillData.pathAmounts[fillData.pathAmounts.length - 1]; + return fillData.pathAmounts[fillData.pathAmounts.length - 1].uniswapPath; } export function getMakerTakerTokens(opts: CreateOrderFromPathOpts): [string, string] { diff --git a/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts b/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts index a602a197fd..71efe7272f 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts @@ -767,17 +767,16 @@ export class SamplerOperations { function: this._samplerContract.sampleSellsFromUniswapV3, params: [quoter, tokenAddressPath, takerFillAmounts], callback: (callResults: string, fillData: UniswapV3FillData): BigNumber[] => { - const [paths, samples, gasUsed] = this._samplerContract.getABIDecodedReturnData< - [string[], BigNumber[], BigNumber[]] - >('sampleSellsFromUniswapV3', callResults); + const [paths, samples] = this._samplerContract.getABIDecodedReturnData<[string[], BigNumber[]]>( + 'sampleSellsFromUniswapV3', + callResults, + ); fillData.router = router; fillData.tokenAddressPath = tokenAddressPath; fillData.pathAmounts = paths.map((uniswapPath, i) => ({ uniswapPath, inputAmount: takerFillAmounts[i], - gasUsed: gasUsed[i].toNumber(), })); - return samples; }, }); @@ -796,15 +795,15 @@ export class SamplerOperations { function: this._samplerContract.sampleBuysFromUniswapV3, params: [quoter, tokenAddressPath, makerFillAmounts], callback: (callResults: string, fillData: UniswapV3FillData): BigNumber[] => { - const [paths, samples, gasUsed] = this._samplerContract.getABIDecodedReturnData< - [string[], BigNumber[], BigNumber[]] - >('sampleBuysFromUniswapV3', callResults); + const [paths, samples] = this._samplerContract.getABIDecodedReturnData<[string[], BigNumber[]]>( + 'sampleBuysFromUniswapV3', + callResults, + ); fillData.router = router; fillData.tokenAddressPath = tokenAddressPath; fillData.pathAmounts = paths.map((uniswapPath, i) => ({ uniswapPath, inputAmount: makerFillAmounts[i], - gasUsed: gasUsed[i].toNumber(), })); return samples; }, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/types.ts b/packages/asset-swapper/src/utils/market_operation_utils/types.ts index cbeef03f99..df74af4975 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -268,34 +268,19 @@ export interface HopInfo { returnData: string; } -export interface UniswapV3PathAmount { - uniswapPath: string; - inputAmount: BigNumber; - gasUsed: number; -} export interface UniswapV3FillData extends FillData { tokenAddressPath: string[]; router: string; - pathAmounts: UniswapV3PathAmount[]; + pathAmounts: Array<{ uniswapPath: string; inputAmount: BigNumber }>; } export interface KyberDmmFillData extends UniswapV2FillData { poolsPath: string[]; } -/** - * Determines whether FillData is UniswapV3FillData or FinalUniswapV3FillData - */ -export function isFinalUniswapV3FillData( - data: UniswapV3FillData | FinalUniswapV3FillData, -): data is FinalUniswapV3FillData { - return !!(data as FinalUniswapV3FillData).uniswapPath; -} - -export interface FinalUniswapV3FillData extends Omit { +export interface FinalUniswapV3FillData extends Omit { // The uniswap-encoded path that can fll the maximum input amount. uniswapPath: string; - gasUsed: number; } export interface LidoFillData extends FillData {