-
Notifications
You must be signed in to change notification settings - Fork 43
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
improve(API): Make gasFeeTotal more accurate in /limits and increase cache hit frequency #1369
Changes from 11 commits
69b0962
0e94791
ccb8fa8
ea1585e
5e4fd39
296056f
c1f6e03
5f7ab07
d6dac13
b06f9cc
9e18813
bc5aec1
f42156f
be317b7
bbfe3f7
ea1d1a7
129e1b8
01981bd
39027b3
a63bf04
2161690
a845217
5d00e70
ce45f12
035ef1b
7449e8c
5441dc6
05c27df
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 | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1970,94 +1970,158 @@ export function isContractCache(chainId: number, address: string) { | |||||||||
); | ||||||||||
} | ||||||||||
|
||||||||||
export function getCachedFillGasUsage( | ||||||||||
export function getCachedNativeGasCost( | ||||||||||
deposit: Parameters<typeof buildDepositForSimulation>[0], | ||||||||||
gasPrice?: BigNumber, | ||||||||||
overrides?: Partial<{ | ||||||||||
relayerAddress: string; | ||||||||||
}> | ||||||||||
) { | ||||||||||
// We can use a long TTL since we are fetching only the native gas cost which should rarely change. | ||||||||||
const ttlPerChain = { | ||||||||||
default: 10, | ||||||||||
[CHAIN_IDs.ARBITRUM]: 10, | ||||||||||
default: 60, | ||||||||||
}; | ||||||||||
|
||||||||||
const cacheKey = buildInternalCacheKey( | ||||||||||
"fillGasUsage", | ||||||||||
"nativeGasCost", | ||||||||||
deposit.destinationChainId, | ||||||||||
deposit.outputToken | ||||||||||
); | ||||||||||
const ttl = ttlPerChain[deposit.destinationChainId] || ttlPerChain.default; | ||||||||||
const ttl = ttlPerChain.default; | ||||||||||
const fetchFn = async () => { | ||||||||||
const relayerAddress = | ||||||||||
overrides?.relayerAddress ?? | ||||||||||
sdk.constants.DEFAULT_SIMULATED_RELAYER_ADDRESS; | ||||||||||
const relayerFeeCalculatorQueries = getRelayerFeeCalculatorQueries( | ||||||||||
deposit.destinationChainId, | ||||||||||
overrides | ||||||||||
); | ||||||||||
const unsignedFillTxn = | ||||||||||
await relayerFeeCalculatorQueries.getUnsignedTxFromDeposit( | ||||||||||
buildDepositForSimulation(deposit), | ||||||||||
relayerAddress | ||||||||||
); | ||||||||||
const voidSigner = new ethers.VoidSigner( | ||||||||||
relayerAddress, | ||||||||||
relayerFeeCalculatorQueries.provider | ||||||||||
); | ||||||||||
return voidSigner.estimateGas(unsignedFillTxn); | ||||||||||
}; | ||||||||||
|
||||||||||
return getCachedValue(cacheKey, ttl, fetchFn, (nativeGasCostFromCache) => { | ||||||||||
return BigNumber.from(nativeGasCostFromCache); | ||||||||||
}); | ||||||||||
} | ||||||||||
|
||||||||||
export function getCachedOpStackL1DataFee( | ||||||||||
deposit: Parameters<typeof buildDepositForSimulation>[0], | ||||||||||
nativeGasCost: BigNumber, | ||||||||||
overrides?: Partial<{ | ||||||||||
relayerAddress: string; | ||||||||||
}> | ||||||||||
) { | ||||||||||
// This should roughly be the length of 1 block on Ethereum mainnet which is how often the L1 data fee should | ||||||||||
// change since its based on the L1 base fee. However, this L1 data fee is mostly affected by the L1 base fee which | ||||||||||
// should only change by 12.5% at most per block. | ||||||||||
const ttlPerChain = { | ||||||||||
default: 12, | ||||||||||
}; | ||||||||||
|
||||||||||
const cacheKey = buildInternalCacheKey( | ||||||||||
"opStackL1DataFee", | ||||||||||
deposit.destinationChainId, | ||||||||||
deposit.outputToken // This should technically differ based on the output token since the L2 calldata | ||||||||||
// size affects the L1 data fee and this calldata can differ based on the output token. | ||||||||||
); | ||||||||||
const ttl = ttlPerChain.default; | ||||||||||
const fetchFn = async () => { | ||||||||||
// We don't care about the gas token price or the token gas price, only the raw gas units. In the API | ||||||||||
// we'll compute the gas price separately. | ||||||||||
const markups = getGasMarkup(deposit.destinationChainId); | ||||||||||
const gasCosts = await relayerFeeCalculatorQueries.getGasCosts( | ||||||||||
buildDepositForSimulation(deposit), | ||||||||||
overrides?.relayerAddress, | ||||||||||
{ | ||||||||||
gasPrice, | ||||||||||
// We want the fee multipliers if the gasPrice is undefined: | ||||||||||
baseFeeMultiplier: markups.baseFeeMarkup, | ||||||||||
priorityFeeMultiplier: markups.priorityFeeMarkup, | ||||||||||
opStackL1GasCostMultiplier: sdk.utils.chainIsOPStack( | ||||||||||
deposit.destinationChainId | ||||||||||
) | ||||||||||
? getGasMarkup(deposit.destinationChainId).opStackL1DataFeeMarkup | ||||||||||
: undefined, | ||||||||||
} | ||||||||||
const { opStackL1DataFeeMarkup } = getGasMarkup(deposit.destinationChainId); | ||||||||||
const relayerFeeCalculatorQueries = getRelayerFeeCalculatorQueries( | ||||||||||
deposit.destinationChainId, | ||||||||||
overrides | ||||||||||
); | ||||||||||
return { | ||||||||||
nativeGasCost: gasCosts.nativeGasCost, | ||||||||||
tokenGasCost: gasCosts.tokenGasCost, | ||||||||||
}; | ||||||||||
const unsignedTx = | ||||||||||
await relayerFeeCalculatorQueries.getUnsignedTxFromDeposit( | ||||||||||
buildDepositForSimulation(deposit), | ||||||||||
overrides?.relayerAddress | ||||||||||
); | ||||||||||
const opStackL1GasCost = | ||||||||||
await relayerFeeCalculatorQueries.getOpStackL1DataFee( | ||||||||||
unsignedTx, | ||||||||||
overrides?.relayerAddress, | ||||||||||
{ | ||||||||||
opStackL2GasUnits: nativeGasCost, // Passed in here to avoid gas cost recomputation by the SDK | ||||||||||
opStackL1DataFeeMultiplier: opStackL1DataFeeMarkup, | ||||||||||
} | ||||||||||
); | ||||||||||
return opStackL1GasCost; | ||||||||||
}; | ||||||||||
|
||||||||||
return getCachedValue( | ||||||||||
cacheKey, | ||||||||||
ttl, | ||||||||||
fetchFn, | ||||||||||
(gasCosts: { nativeGasCost: BigNumber; tokenGasCost: BigNumber }) => { | ||||||||||
return { | ||||||||||
nativeGasCost: BigNumber.from(gasCosts.nativeGasCost), | ||||||||||
tokenGasCost: BigNumber.from(gasCosts.tokenGasCost), | ||||||||||
}; | ||||||||||
} | ||||||||||
); | ||||||||||
return getCachedValue(cacheKey, ttl, fetchFn, (l1DataFeeFromCache) => { | ||||||||||
return BigNumber.from(l1DataFeeFromCache); | ||||||||||
}); | ||||||||||
} | ||||||||||
|
||||||||||
export function latestGasPriceCache(chainId: number) { | ||||||||||
export function latestGasPriceCache( | ||||||||||
chainId: number, | ||||||||||
deposit?: Parameters<typeof buildDepositForSimulation>[0], | ||||||||||
overrides?: Partial<{ | ||||||||||
relayerAddress: string; | ||||||||||
}> | ||||||||||
) { | ||||||||||
const ttlPerChain = { | ||||||||||
default: 30, | ||||||||||
[CHAIN_IDs.ARBITRUM]: 15, | ||||||||||
default: 5, | ||||||||||
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. When we consider the cron job frontend/api/cron-cache-gas-prices.ts Lines 15 to 18 in 9e18813
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. Good call, updated this again. I've also updated the cron-cache-gas-prices file to also cache native gas cost and L1 data fees in the cron job. This should be a one-stop cron job we can use to keep all caches pretty warm 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. Can you check the TTL's in |
||||||||||
}; | ||||||||||
|
||||||||||
return makeCacheGetterAndSetter( | ||||||||||
buildInternalCacheKey("latestGasPriceCache", chainId), | ||||||||||
ttlPerChain[chainId] || ttlPerChain.default, | ||||||||||
async () => (await getMaxFeePerGas(chainId)).maxFeePerGas, | ||||||||||
(bnFromCache) => BigNumber.from(bnFromCache) | ||||||||||
// If deposit is defined, then the gas price will be dependent on the fill transaction derived from the deposit. | ||||||||||
// Therefore, we technically should cache a different gas price per different types of deposit so we add | ||||||||||
// an additional outputToken to the cache key to distinguish between gas prices dependent on deposit args | ||||||||||
// for different output tokens, which should be the main factor affecting the fill gas cost. | ||||||||||
buildInternalCacheKey( | ||||||||||
`latestGasPriceCache${deposit ? `-${deposit.outputToken}` : ""}`, | ||||||||||
chainId | ||||||||||
), | ||||||||||
ttlPerChain.default, | ||||||||||
async () => await getMaxFeePerGas(chainId, deposit, overrides), | ||||||||||
(gasPrice: sdk.gasPriceOracle.GasPriceEstimate) => { | ||||||||||
return { | ||||||||||
maxFeePerGas: BigNumber.from(gasPrice.maxFeePerGas), | ||||||||||
maxPriorityFeePerGas: BigNumber.from(gasPrice.maxPriorityFeePerGas), | ||||||||||
}; | ||||||||||
} | ||||||||||
); | ||||||||||
} | ||||||||||
|
||||||||||
/** | ||||||||||
* Resolve the current gas price for a given chain | ||||||||||
* @param chainId The chain ID to resolve the gas price for | ||||||||||
* @returns The gas price in the native currency of the chain | ||||||||||
*/ | ||||||||||
export function getMaxFeePerGas( | ||||||||||
chainId: number | ||||||||||
export async function getMaxFeePerGas( | ||||||||||
chainId: number, | ||||||||||
deposit?: Parameters<typeof buildDepositForSimulation>[0], | ||||||||||
overrides?: Partial<{ | ||||||||||
relayerAddress: string; | ||||||||||
}> | ||||||||||
): Promise<sdk.gasPriceOracle.GasPriceEstimate> { | ||||||||||
if (deposit && deposit.destinationChainId !== chainId) { | ||||||||||
throw new Error( | ||||||||||
"Chain ID must match the destination chain ID of the deposit" | ||||||||||
); | ||||||||||
} | ||||||||||
const { | ||||||||||
baseFeeMarkup: baseFeeMultiplier, | ||||||||||
priorityFeeMarkup: priorityFeeMultiplier, | ||||||||||
} = getGasMarkup(chainId); | ||||||||||
const relayerFeeCalculatorQueries = getRelayerFeeCalculatorQueries( | ||||||||||
chainId, | ||||||||||
overrides | ||||||||||
); | ||||||||||
const unsignedFillTxn = deposit | ||||||||||
? await relayerFeeCalculatorQueries.getUnsignedTxFromDeposit( | ||||||||||
buildDepositForSimulation(deposit), | ||||||||||
overrides?.relayerAddress | ||||||||||
) | ||||||||||
: undefined; | ||||||||||
return sdk.gasPriceOracle.getGasPriceEstimate(getProvider(chainId), { | ||||||||||
chainId, | ||||||||||
unsignedTx: unsignedFillTxn, | ||||||||||
baseFeeMultiplier, | ||||||||||
priorityFeeMultiplier, | ||||||||||
}); | ||||||||||
|
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.
We should ideally update this once per cron job if it runs every 60 seconds