From e5d60b807753e75059b310956e57d408037e46d6 Mon Sep 17 00:00:00 2001 From: Kim Persson Date: Wed, 2 Mar 2022 13:36:12 +0100 Subject: [PATCH] feat: Improve Uniswap V3 gas schedule redux (#424) * Revert "fix: Revert Improve Uniswap V3 gas schedule (#397) (#419)" This reverts commit df0e0866e4c12597bd8880fd50339bed3e96eaa4. * fix: UniswapV3Sampler return token amounts as the last value in return tuple * fix: bump Uniswap V3 quote max gas because QuoterV2 is more expensive * fix: don't try to rout 0 sellAmount/buyAmount quotes * fix: Linting issue * fix: use median gas usage instead of mean in UniV3 gas schedule * chore: add asset-swapper changelog entry * fix: remove contract-addresses changelog empty row failing linting --- packages/asset-swapper/CHANGELOG.json | 5 +- .../contracts/src/UniswapV3Sampler.sol | 89 ++++++++++++++----- .../utils/market_operation_utils/constants.ts | 41 +++++++-- .../market_operation_utils/geist_utils.ts | 8 +- .../utils/market_operation_utils/orders.ts | 21 +++-- .../market_operation_utils/path_optimizer.ts | 2 +- .../sampler_operations.ts | 17 ++-- .../src/utils/market_operation_utils/types.ts | 19 +++- packages/contract-addresses/CHANGELOG.json | 1 - 9 files changed, 151 insertions(+), 52 deletions(-) diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index ef9dd8ea4b..71eeb8b2dd 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -1,11 +1,14 @@ [ - { "version": "16.50.0", "changes": [ { "note": "Adding support for Geist on `Fantom`", "pr": 398 + }, + { + "note": "Improve Uniswap V3 gas schedule", + "pr": 424 } ] }, diff --git a/packages/asset-swapper/contracts/src/UniswapV3Sampler.sol b/packages/asset-swapper/contracts/src/UniswapV3Sampler.sol index 188919a9df..c964bacbef 100644 --- a/packages/asset-swapper/contracts/src/UniswapV3Sampler.sol +++ b/packages/asset-swapper/contracts/src/UniswapV3Sampler.sol @@ -22,17 +22,43 @@ pragma experimental ABIEncoderV2; import "@0x/contracts-erc20/contracts/src/v06/IERC20TokenV06.sol"; -interface IUniswapV3Quoter { +interface IUniswapV3QuoterV2 { 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); + 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 function quoteExactOutput(bytes memory path, uint256 amountOut) external - returns (uint256 amountIn); + returns ( + uint256 amountIn, + uint160[] memory sqrtPriceX96AfterList, + uint32[] memory initializedTicksCrossedList, + uint256 gasEstimate + ); } interface IUniswapV3Factory { @@ -51,23 +77,25 @@ interface IUniswapV3Pool { contract UniswapV3Sampler { /// @dev Gas limit for UniswapV3 calls. This is 100% a guess. - uint256 constant private QUOTE_GAS = 600e3; + uint256 constant private QUOTE_GAS = 900e3; /// @dev Sample sell quotes from UniswapV3. /// @param quoter UniswapV3 Quoter contract. /// @param path Token route. Should be takerToken -> makerToken /// @param takerTokenAmounts Taker token sell amount for each sample. /// @return uniswapPaths The encoded uniswap path for each sample. + /// @return uniswapGasUsed Estimated amount of gas used /// @return makerTokenAmounts Maker amounts bought at each taker token /// amount. function sampleSellsFromUniswapV3( - IUniswapV3Quoter quoter, + IUniswapV3QuoterV2 quoter, IERC20TokenV06[] memory path, uint256[] memory takerTokenAmounts ) public returns ( bytes[] memory uniswapPaths, + uint256[] memory uniswapGasUsed, uint256[] memory makerTokenAmounts ) { @@ -76,31 +104,39 @@ 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) + try quoter.quoteExactInput + { gas: QUOTE_GAS } + (uniswapPath, takerTokenAmounts[i]) + returns ( + uint256 buyAmount, + uint160[] memory, /* sqrtPriceX96AfterList */ + uint32[] memory, /* initializedTicksCrossedList */ + uint256 gasUsed + ) { if (topBuyAmount <= buyAmount) { topBuyAmount = buyAmount; - topUniswapPath = uniswapPath; + uniswapPaths[i] = uniswapPath; + uniswapGasUsed[i] = gasUsed; } - } catch { } + } catch {} } - // Break early if we can't complete the buys. + // Break early if we can't complete the sells. 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; } } @@ -109,16 +145,18 @@ contract UniswapV3Sampler /// @param path Token route. Should be takerToken -> makerToken. /// @param makerTokenAmounts Maker token buy amount for each sample. /// @return uniswapPaths The encoded uniswap path for each sample. + /// @return uniswapGasUsed Estimated amount of gas used /// @return takerTokenAmounts Taker amounts sold at each maker token /// amount. function sampleBuysFromUniswapV3( - IUniswapV3Quoter quoter, + IUniswapV3QuoterV2 quoter, IERC20TokenV06[] memory path, uint256[] memory makerTokenAmounts ) public returns ( bytes[] memory uniswapPaths, + uint256[] memory uniswapGasUsed, uint256[] memory takerTokenAmounts ) { @@ -128,10 +166,10 @@ 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) { // 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. @@ -143,21 +181,30 @@ contract UniswapV3Sampler quoter.quoteExactOutput { gas: QUOTE_GAS } (uniswapPath, makerTokenAmounts[i]) - returns (uint256 sellAmount) + returns ( + uint256 sellAmount, + uint160[] memory, /* sqrtPriceX96AfterList */ + uint32[] memory, /* initializedTicksCrossedList */ + uint256 gasUsed + ) { if (topSellAmount == 0 || topSellAmount >= sellAmount) { topSellAmount = sellAmount; // But the output path should still be encoded for sells. - topUniswapPath = _toUniswapPath(path, poolPaths[j]); + uniswapPaths[i] = _toUniswapPath(path, poolPaths[j]); + uniswapGasUsed[i] = gasUsed; } } 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; } } @@ -236,6 +283,7 @@ contract UniswapV3Sampler function _reverseTokenPath(IERC20TokenV06[] memory tokenPath) private + pure returns (IERC20TokenV06[] memory reversed) { reversed = new IERC20TokenV06[](tokenPath.length); @@ -246,6 +294,7 @@ 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 4207f0c5da..9d4cbf44cb 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -17,8 +17,10 @@ import { ERC20BridgeSource, FeeSchedule, FillData, + FinalUniswapV3FillData, GeistFillData, GetMarketOrdersOpts, + isFinalUniswapV3FillData, KyberSamplerOpts, LidoInfo, LiquidityProviderFillData, @@ -2065,19 +2067,19 @@ export const BEETHOVEN_X_SUBGRAPH_URL_BY_CHAIN = valueByChainId( export const UNISWAPV3_CONFIG_BY_CHAIN_ID = valueByChainId( { [ChainId.Mainnet]: { - quoter: '0xb27308f9f90d607463bb33ea1bebb41c27ce5ab6', + quoter: '0x61ffe014ba17989e743c5f6cb21bf9697530b21e', router: '0xe592427a0aece92de3edee1f18e0157c05861564', }, [ChainId.Ropsten]: { - quoter: '0x2f9e608fd881861b8916257b76613cb22ee0652c', + quoter: '0x61ffe014ba17989e743c5f6cb21bf9697530b21e', router: '0x03782388516e94fcd4c18666303601a12aa729ea', }, [ChainId.Polygon]: { - quoter: '0xb27308f9f90d607463bb33ea1bebb41c27ce5ab6', + quoter: '0x61ffe014ba17989e743c5f6cb21bf9697530b21e', router: '0xe592427a0aece92de3edee1f18e0157c05861564', }, [ChainId.Optimism]: { - quoter: '0xb27308f9f90d607463bb33ea1bebb41c27ce5ab6', + quoter: '0x61ffe014ba17989e743c5f6cb21bf9697530b21e', router: '0xe592427a0aece92de3edee1f18e0157c05861564', }, }, @@ -2369,11 +2371,34 @@ export const DEFAULT_GAS_SCHEDULE: Required = { return gas; }, [ERC20BridgeSource.UniswapV3]: (fillData?: FillData) => { - let gas = 100e3; - const path = (fillData as UniswapV3FillData).tokenAddressPath; - if (path.length > 2) { - gas += (path.length - 2) * 32e3; // +32k for each hop. + 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 median of gas usage returned from UniswapV3 + // For the best case scenario (least amount of hops & ticks) this will + // overestimate the gas usage + const pathAmountsWithGasUsed = uniFillData.pathAmounts.filter(p => p.gasUsed > 0); + const medianGasUsedForPath = + pathAmountsWithGasUsed[Math.floor(pathAmountsWithGasUsed.length / 2)]?.gasUsed ?? 0; + gas += medianGasUsedForPath; } + + // 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/geist_utils.ts b/packages/asset-swapper/src/utils/market_operation_utils/geist_utils.ts index 50dd9aba48..db572c803e 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/geist_utils.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/geist_utils.ts @@ -12,10 +12,10 @@ const gTokenToUnderlyingToken = new Map([ [FANTOM_TOKENS.gMIM, FANTOM_TOKENS.MIM], ]); -export function getGeistInfoForPair( - takerToken: string, - makerToken: string, -): GeistInfo | undefined { +/** + * Returns GeistInfo for a certain pair if that pair exists on Geist + */ +export function getGeistInfoForPair(takerToken: string, makerToken: string): GeistInfo | undefined { let gToken; let underlyingToken; if (gTokenToUnderlyingToken.get(takerToken) === makerToken) { 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 43b494f674..8fe6d98188 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts @@ -37,6 +37,7 @@ import { ShellFillData, UniswapV2FillData, UniswapV3FillData, + UniswapV3PathAmount, } from './types'; // tslint:disable completed-docs @@ -394,11 +395,14 @@ function createFinalBridgeOrderFillDataFromCollapsedFill(fill: CollapsedFill): F switch (fill.source) { case ERC20BridgeSource.UniswapV3: { const fd = fill.fillData as UniswapV3FillData; - return { + const { uniswapPath, gasUsed } = getBestUniswapV3PathAmountForInputAmount(fd, fill.input); + const finalFillData: FinalUniswapV3FillData = { router: fd.router, tokenAddressPath: fd.tokenAddressPath, - uniswapPath: getBestUniswapV3PathForInputAmount(fd, fill.input), + uniswapPath, + gasUsed, }; + return finalFillData; } default: break; @@ -406,18 +410,21 @@ function createFinalBridgeOrderFillDataFromCollapsedFill(fill: CollapsedFill): F return fill.fillData; } -function getBestUniswapV3PathForInputAmount(fillData: UniswapV3FillData, inputAmount: BigNumber): string { +function getBestUniswapV3PathAmountForInputAmount( + fillData: UniswapV3FillData, + inputAmount: BigNumber, +): UniswapV3PathAmount { 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 { inputAmount: pathInputAmount, uniswapPath } of fillData.pathAmounts) { - if (pathInputAmount.gte(inputAmount)) { - return uniswapPath; + for (const pathAmount of fillData.pathAmounts) { + if (pathAmount.inputAmount.gte(inputAmount)) { + return pathAmount; } } - return fillData.pathAmounts[fillData.pathAmounts.length - 1].uniswapPath; + return fillData.pathAmounts[fillData.pathAmounts.length - 1]; } export function getMakerTakerTokens(opts: CreateOrderFromPathOpts): [string, string] { diff --git a/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts index 14d1d6e251..aa4dde51c4 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts @@ -80,7 +80,7 @@ function findRoutesAndCreateOptimalPath( // Currently the rust router is unable to handle 1 base unit sized quotes and will error out // To avoid flooding the logs with these errors we just return an insufficient liquidity error // which is how the JS router handles these quotes today - if (input.eq(ONE_BASE_UNIT)) { + if (input.isLessThanOrEqualTo(ONE_BASE_UNIT)) { return undefined; } 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 443d3b9e77..5ea5d73920 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 @@ -771,16 +771,17 @@ export class SamplerOperations { function: this._samplerContract.sampleSellsFromUniswapV3, params: [quoter, tokenAddressPath, takerFillAmounts], callback: (callResults: string, fillData: UniswapV3FillData): BigNumber[] => { - const [paths, samples] = this._samplerContract.getABIDecodedReturnData<[string[], BigNumber[]]>( - 'sampleSellsFromUniswapV3', - callResults, - ); + const [paths, gasUsed, samples] = this._samplerContract.getABIDecodedReturnData< + [string[], BigNumber[], 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; }, }); @@ -799,15 +800,15 @@ export class SamplerOperations { function: this._samplerContract.sampleBuysFromUniswapV3, params: [quoter, tokenAddressPath, makerFillAmounts], callback: (callResults: string, fillData: UniswapV3FillData): BigNumber[] => { - const [paths, samples] = this._samplerContract.getABIDecodedReturnData<[string[], BigNumber[]]>( - 'sampleBuysFromUniswapV3', - callResults, - ); + const [paths, gasUsed, samples] = this._samplerContract.getABIDecodedReturnData< + [string[], BigNumber[], 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 929c08d26b..7063fbb05c 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -275,19 +275,34 @@ export interface HopInfo { returnData: string; } +export interface UniswapV3PathAmount { + uniswapPath: string; + inputAmount: BigNumber; + gasUsed: number; +} export interface UniswapV3FillData extends FillData { tokenAddressPath: string[]; router: string; - pathAmounts: Array<{ uniswapPath: string; inputAmount: BigNumber }>; + pathAmounts: UniswapV3PathAmount[]; } export interface KyberDmmFillData extends UniswapV2FillData { poolsPath: string[]; } -export interface FinalUniswapV3FillData extends Omit { +/** + * 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 { // The uniswap-encoded path that can fll the maximum input amount. uniswapPath: string; + gasUsed: number; } export interface LidoFillData extends FillData { diff --git a/packages/contract-addresses/CHANGELOG.json b/packages/contract-addresses/CHANGELOG.json index 5d712fc7fd..ee468e04be 100644 --- a/packages/contract-addresses/CHANGELOG.json +++ b/packages/contract-addresses/CHANGELOG.json @@ -1,5 +1,4 @@ [ - { "version": "6.12.0", "changes": [