-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
patch: Improve autorouter #561
Changes from 6 commits
5b84bf9
96fda6e
548ad21
b1c6a70
f88b1ad
e4422f9
45fec35
163a6bc
1aa5d02
7443d7b
6baad54
39de14d
4521483
f7c3550
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,9 @@ import type { CalleeFunctions, CollateralConfig, Pool } from '../types'; | |
import { ethers } from 'ethers'; | ||
import BigNumber from '../bignumber'; | ||
import { getContractAddressByName, getJoinNameByCollateralType } from '../contracts'; | ||
import { convertCollateralToDaiUsingPool, encodePools } from './helpers/uniswapV3'; | ||
import { convertCollateralToDaiUsingPool, encodePools, getRouteAndGasQuote } from './helpers/uniswapV3'; | ||
import { getPools } from '.'; | ||
import { routeToPool } from './helpers/pools'; | ||
import { getRouteAndGasQuote } from './helpers/uniswapV3'; | ||
|
||
const getCalleeData = async function ( | ||
network: string, | ||
|
@@ -40,23 +39,28 @@ const getMarketPrice = async function ( | |
collateral: CollateralConfig, | ||
marketId: string, | ||
collateralAmount: BigNumber | ||
): Promise<BigNumber> { | ||
// convert collateral into DAI | ||
const { route, fees } = await getRouteAndGasQuote(network, collateral.symbol, collateralAmount, marketId); | ||
if (!route) { | ||
throw new Error(`No route found for ${collateral.symbol} to DAI`); | ||
} | ||
const pools = await routeToPool(network, route, collateral.symbol, fees); | ||
const daiAmount = await convertCollateralToDaiUsingPool( | ||
): Promise<{ price: BigNumber; pools: Pool[] }> { | ||
const { route, fees, totalPrice, pools } = await getRouteAndGasQuote( | ||
network, | ||
collateral.symbol, | ||
marketId, | ||
collateralAmount, | ||
pools | ||
marketId | ||
); | ||
|
||
// return price per unit | ||
return daiAmount.dividedBy(collateralAmount); | ||
if (!pools && route) { | ||
const generatedPools = await routeToPool(network, route, collateral.symbol, fees); | ||
const daiAmount = await convertCollateralToDaiUsingPool( | ||
network, | ||
collateral.symbol, | ||
marketId, | ||
collateralAmount, | ||
generatedPools | ||
); | ||
return { price: daiAmount.dividedBy(collateralAmount), pools: generatedPools }; | ||
} | ||
if (totalPrice && pools) { | ||
return { price: totalPrice.dividedBy(collateralAmount), pools: pools }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only remaining problem I see now is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i observe that the autorouter returns 2 amounts: /**
* The quote for the swap.
* For EXACT_IN swaps this will be an amount of token out.
* For EXACT_OUT this will be an amount of token in.
*/
quote: CurrencyAmount;
/**
* The quote adjusted for the estimated gas used by the swap.
* This is computed by estimating the amount of gas used by the swap, converting
* this estimate to be in terms of the quote token, and subtracting that from the quote.
* i.e. quoteGasAdjusted = quote - estimatedGasUsedQuoteToken
*/
quoteGasAdjusted: CurrencyAmount; My understanding of the above does not combine well with your claim. The documentation claims that quote is the amount of "token out". If the value does not account for exchange fees, then the amount out is different from Did you perhaps mean the gas fees? should i use |
||
} | ||
throw new Error(`Failed to compute market data due to lack of information`); | ||
}; | ||
|
||
const UniswapV2CalleeDai: CalleeFunctions = { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,10 +4,8 @@ import type { | |
MarketData, | ||
CollateralConfig, | ||
CollateralSymbol, | ||
RegularCalleeConfig, | ||
UniswapV2LpTokenCalleeConfig, | ||
AutoRouterCalleeConfig, | ||
Pool, | ||
PriceWithPools, | ||
} from '../types'; | ||
import memoizee from 'memoizee'; | ||
import BigNumber from '../bignumber'; | ||
|
@@ -64,63 +62,58 @@ export const getPools = async ( | |
return await routeToPool(network, calleeConfig.route, collateral.symbol); | ||
} | ||
if ('automaticRouter' in calleeConfig) { | ||
const { route, fees } = await fetchAutoRouteInformation(network, collateral.symbol, amount.toFixed()); | ||
if (!route) { | ||
throw new Error('No automatic route can be found'); | ||
const { pools } = await fetchAutoRouteInformation(network, collateral.symbol, amount.toFixed()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I will refactor the code a bit so that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed unused arg (amount) |
||
if (!pools) { | ||
throw new Error('No automatic pools can be found'); | ||
} | ||
return await routeToPool(network, route, collateral.symbol, fees); | ||
return pools; | ||
} | ||
return undefined; | ||
}; | ||
|
||
export const enrichCalleeConfigWithPools = async ( | ||
network: string, | ||
collateral: CollateralConfig, | ||
marketId: string, | ||
amount: BigNumber | ||
): Promise<UniswapV2LpTokenCalleeConfig | ((RegularCalleeConfig | AutoRouterCalleeConfig) & { pools: Pool[] })> => { | ||
const config = collateral.exchanges[marketId]; | ||
if (config.callee === 'UniswapV2LpTokenCalleeDai') { | ||
return { ...config }; | ||
} | ||
const pools = await getPools(network, collateral, marketId, amount); | ||
if (pools) { | ||
return { ...config, pools }; | ||
} | ||
throw new Error('Failed to get pools'); | ||
}; | ||
|
||
export const getMarketDataById = async function ( | ||
network: string, | ||
collateral: CollateralConfig, | ||
marketId: string, | ||
amount: BigNumber = new BigNumber('1') | ||
): Promise<MarketData> { | ||
const calleeConfig = await enrichCalleeConfigWithPools(network, collateral, marketId, amount); | ||
const calleeConfig = collateral.exchanges[marketId]; | ||
if (!allCalleeFunctions[calleeConfig?.callee]) { | ||
throw new Error(`Unsupported callee "${calleeConfig?.callee}" for collateral symbol "${collateral.symbol}"`); | ||
} | ||
let marketUnitPrice: BigNumber; | ||
let marketPrice: PriceWithPools; | ||
|
||
try { | ||
marketUnitPrice = await allCalleeFunctions[calleeConfig.callee].getMarketPrice( | ||
marketPrice = await allCalleeFunctions[calleeConfig.callee].getMarketPrice( | ||
network, | ||
collateral, | ||
marketId, | ||
amount | ||
); | ||
} catch (error: any) { | ||
const errorMessage = error?.message; | ||
marketUnitPrice = new BigNumber(NaN); | ||
return { | ||
...calleeConfig, | ||
marketUnitPrice, | ||
marketUnitPrice: new BigNumber(NaN), | ||
pools: [], | ||
errorMessage, | ||
}; | ||
} | ||
return { | ||
...calleeConfig, | ||
marketUnitPrice: marketUnitPrice ? marketUnitPrice : new BigNumber(NaN), | ||
}; | ||
const marketUnitPrice = marketPrice.price; | ||
if (calleeConfig.callee !== 'UniswapV2LpTokenCalleeDai' && marketPrice.pools) { | ||
return { | ||
...calleeConfig, | ||
marketUnitPrice: marketUnitPrice ? marketUnitPrice : new BigNumber(NaN), | ||
pools: marketPrice.pools, | ||
}; | ||
} | ||
if (calleeConfig.callee === 'UniswapV2LpTokenCalleeDai') { | ||
return { | ||
...calleeConfig, | ||
marketUnitPrice: marketUnitPrice ? marketUnitPrice : new BigNumber(NaN), | ||
}; | ||
} | ||
throw new Error('No pools found where expected'); | ||
}; | ||
|
||
export const getMarketDataRecords = async function ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,7 +116,7 @@ declare interface MarketDataBase extends Partial<ExchangeFees> { | |
export declare interface Pool { | ||
addresses: string[]; | ||
fee: number; | ||
routes: string[]; | ||
routes: (string | undefined)[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In which case it can be an array of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. uniswap autorouter does not promise to return the symbol of the token. It's optional there. will throw if the autorouter does not return symbol in pool |
||
} | ||
|
||
export declare interface MarketDataRegular extends MarketDataBase { | ||
|
@@ -183,6 +183,11 @@ export declare interface CalleeAddresses { | |
|
||
export type CalleeNames = keyof CalleeAddresses; | ||
|
||
export interface PriceWithPools { | ||
price: BigNumber; | ||
pools?: Pool[]; | ||
} | ||
|
||
export declare interface CalleeFunctions { | ||
getCalleeData: ( | ||
network: string, | ||
|
@@ -196,7 +201,7 @@ export declare interface CalleeFunctions { | |
collateral: CollateralConfig, | ||
marketId: string, | ||
amount: BigNumber | ||
) => Promise<BigNumber>; | ||
) => Promise<PriceWithPools>; | ||
} | ||
|
||
export declare interface MakerParams { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I would remove
getRouteAndGasQuote
whatsoever and move its logic here, insidegetMarketPrice
. Because it's just adds a very strange layer of abstraction which doesn't add any value, but complicates understanding of what's going on