-
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
Merged
Merged
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
69b0962
improve(API): Reduce stale-while-revalidate and gas price cache times
nicholaspai 0e94791
Separate native gas cost and op stack l1 gas cost calculation from to…
nicholaspai ccb8fa8
Use gas price cache for Linea as well
nicholaspai ea1585e
fix: only cron cache gas prices for non Linea chains
nicholaspai 5e4fd39
Update limits.ts
nicholaspai 296056f
Only pass in depositArgs for Linea
nicholaspai c1f6e03
add extra part to cache key
nicholaspai 5f7ab07
Use sdk for helper methods
nicholaspai d6dac13
Update limits.ts
nicholaspai b06f9cc
Fix gas-prices
nicholaspai 9e18813
Use utils in gas-prices.ts to read data from cache
nicholaspai bc5aec1
add gas costs to cron job
nicholaspai f42156f
cache gas prices before cost
nicholaspai be317b7
remove promise.all
nicholaspai bbfe3f7
Update _utils.ts
nicholaspai ea1d1a7
cache op stack l1 costs for op chains only
nicholaspai 129e1b8
Test only cache gas prices
nicholaspai 01981bd
debug
nicholaspai 39027b3
Fix cron job
nicholaspai a63bf04
Update cron-cache-gas-prices.ts
nicholaspai 2161690
fix promise nesting
nicholaspai a845217
Update cron-cache-gas-prices.ts
nicholaspai 5d00e70
update cache times
nicholaspai ce45f12
Update _utils.ts
nicholaspai 035ef1b
Update cron-cache-gas-prices.ts
nicholaspai 7449e8c
Add native gas cost caching
nicholaspai 5441dc6
Increase ttl of native gas cost, add gasFeeDetails to response
nicholaspai 05c27df
sdk
nicholaspai File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1970,94 +1970,169 @@ 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. | ||
// Set this longer than the secondsPerUpdate value in the cron cache gas prices job. | ||
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 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 makeCacheGetterAndSetter( | ||
cacheKey, | ||
ttlPerChain.default, | ||
fetchFn, | ||
(nativeGasCostFromCache) => { | ||
return BigNumber.from(nativeGasCostFromCache); | ||
} | ||
); | ||
} | ||
|
||
export function getCachedOpStackL1DataFee( | ||
deposit: Parameters<typeof buildDepositForSimulation>[0], | ||
nativeGasCost: BigNumber, | ||
overrides?: Partial<{ | ||
relayerAddress: string; | ||
}> | ||
) { | ||
// The L1 data fee should change after each Ethereum block since its based on the L1 base fee. | ||
// However, the L1 base fee should only change by 12.5% at most per block. | ||
// We set this higher than the secondsPerUpdate value in the cron cache gas prices job which will update this | ||
// more frequently. | ||
const ttlPerChain = { | ||
default: 60, | ||
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. We should update this ideally every 12s--every ethereum block |
||
}; | ||
|
||
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 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( | ||
return makeCacheGetterAndSetter( | ||
cacheKey, | ||
ttl, | ||
ttlPerChain.default, | ||
fetchFn, | ||
(gasCosts: { nativeGasCost: BigNumber; tokenGasCost: BigNumber }) => { | ||
return { | ||
nativeGasCost: BigNumber.from(gasCosts.nativeGasCost), | ||
tokenGasCost: BigNumber.from(gasCosts.tokenGasCost), | ||
}; | ||
(l1DataFeeFromCache) => { | ||
return BigNumber.from(l1DataFeeFromCache); | ||
} | ||
); | ||
} | ||
|
||
export function latestGasPriceCache(chainId: number) { | ||
export function latestGasPriceCache( | ||
chainId: number, | ||
deposit?: Parameters<typeof buildDepositForSimulation>[0], | ||
overrides?: Partial<{ | ||
relayerAddress: string; | ||
}> | ||
) { | ||
// We set this higher than the secondsPerUpdate value in the cron cache gas prices job which will update this | ||
// more frequently. | ||
const ttlPerChain = { | ||
default: 30, | ||
nicholaspai marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[CHAIN_IDs.ARBITRUM]: 15, | ||
}; | ||
|
||
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, | ||
}); | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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