Skip to content
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 28 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 8 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 Jan 10, 2025
0e94791
Separate native gas cost and op stack l1 gas cost calculation from to…
nicholaspai Jan 11, 2025
ccb8fa8
Use gas price cache for Linea as well
nicholaspai Jan 11, 2025
ea1585e
fix: only cron cache gas prices for non Linea chains
nicholaspai Jan 11, 2025
5e4fd39
Update limits.ts
nicholaspai Jan 11, 2025
296056f
Only pass in depositArgs for Linea
nicholaspai Jan 11, 2025
c1f6e03
add extra part to cache key
nicholaspai Jan 11, 2025
5f7ab07
Use sdk for helper methods
nicholaspai Jan 11, 2025
d6dac13
Update limits.ts
nicholaspai Jan 11, 2025
b06f9cc
Fix gas-prices
nicholaspai Jan 11, 2025
9e18813
Use utils in gas-prices.ts to read data from cache
nicholaspai Jan 11, 2025
bc5aec1
add gas costs to cron job
nicholaspai Jan 11, 2025
f42156f
cache gas prices before cost
nicholaspai Jan 11, 2025
be317b7
remove promise.all
nicholaspai Jan 11, 2025
bbfe3f7
Update _utils.ts
nicholaspai Jan 11, 2025
ea1d1a7
cache op stack l1 costs for op chains only
nicholaspai Jan 11, 2025
129e1b8
Test only cache gas prices
nicholaspai Jan 11, 2025
01981bd
debug
nicholaspai Jan 11, 2025
39027b3
Fix cron job
nicholaspai Jan 11, 2025
a63bf04
Update cron-cache-gas-prices.ts
nicholaspai Jan 11, 2025
2161690
fix promise nesting
nicholaspai Jan 11, 2025
a845217
Update cron-cache-gas-prices.ts
nicholaspai Jan 11, 2025
5d00e70
update cache times
nicholaspai Jan 11, 2025
ce45f12
Update _utils.ts
nicholaspai Jan 11, 2025
035ef1b
Update cron-cache-gas-prices.ts
nicholaspai Jan 11, 2025
7449e8c
Add native gas cost caching
nicholaspai Jan 11, 2025
5441dc6
Increase ttl of native gas cost, add gasFeeDetails to response
nicholaspai Jan 12, 2025
05c27df
sdk
nicholaspai Jan 12, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 110 additions & 50 deletions api/_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1970,94 +1970,154 @@ 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,
Copy link
Member Author

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

};

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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we consider the cron job cron-cache-gas-prices, this TTL doesn't matter much. The more correct value to tweak would be this one

const updateIntervalsSecPerChain = {
default: 10,
1: 12,
};

Copy link
Member Author

Choose a reason for hiding this comment

The 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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check the TTL's in _utils and the cron-cache-gas-price secondsToUpdate to make sure the values make sense?

};

return makeCacheGetterAndSetter(
buildInternalCacheKey("latestGasPriceCache", chainId),
ttlPerChain[chainId] || ttlPerChain.default,
async () => (await getMaxFeePerGas(chainId)).maxFeePerGas,
// 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)).maxFeePerGas,
(bnFromCache) => BigNumber.from(bnFromCache)
);
}

/**
* 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,
});
Expand Down
35 changes: 20 additions & 15 deletions api/cron-cache-gas-prices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { UnauthorizedError } from "./_errors";

import mainnetChains from "../src/data/chains_1.json";
import { utils } from "@across-protocol/sdk";
import { CHAIN_IDs } from "./_constants";

const updateIntervalsSecPerChain = {
default: 10,
Expand Down Expand Up @@ -52,23 +53,27 @@ const handler = async (
// But we want to update gas prices more frequently than that.
// To circumvent this, we run the function in a loop and update gas prices every
// `secondsPerUpdateForChain` seconds and stop after `maxDurationSec` seconds (1 minute).
const gasPricePromises = mainnetChains.map(async (chain) => {
const secondsPerUpdateForChain =
updateIntervalsSecPerChain[
chain.chainId as keyof typeof updateIntervalsSecPerChain
] || updateIntervalsSecPerChain.default;
const cache = latestGasPriceCache(chain.chainId);
const gasPricePromises = mainnetChains
.filter((chain) => CHAIN_IDs.LINEA !== chain.chainId)
.map(async (chain) => {
const secondsPerUpdateForChain =
updateIntervalsSecPerChain[
chain.chainId as keyof typeof updateIntervalsSecPerChain
] || updateIntervalsSecPerChain.default;
// The deposit args don't matter for any chain besides Linea, which is why we filter it out
// above, because gas price on Linea is dependent on the fill transaction args.
const cache = latestGasPriceCache(chain.chainId);

while (true) {
const diff = Date.now() - functionStart;
// Stop after `maxDurationSec` seconds
if (diff >= maxDurationSec * 1000) {
break;
while (true) {
const diff = Date.now() - functionStart;
// Stop after `maxDurationSec` seconds
if (diff >= maxDurationSec * 1000) {
break;
}
await cache.set();
await utils.delay(secondsPerUpdateForChain);
}
await cache.set();
await utils.delay(secondsPerUpdateForChain);
}
});
});
await Promise.all(gasPricePromises);

logger.debug({
Expand Down
3 changes: 1 addition & 2 deletions api/gas-prices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ import {
sendResponse,
} from "./_utils";
import { TypedVercelRequest } from "./_types";
import { ethers, providers, VoidSigner } from "ethers";
import { ethers } from "ethers";
import * as sdk from "@across-protocol/sdk";
import { L2Provider } from "@eth-optimism/sdk/dist/interfaces/l2-provider";

import mainnetChains from "../src/data/chains_1.json";
import {
Expand Down
74 changes: 47 additions & 27 deletions api/limits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ import {
getCachedLatestBlock,
parsableBigNumberString,
validateDepositMessage,
getCachedFillGasUsage,
latestGasPriceCache,
getCachedNativeGasCost,
getCachedOpStackL1DataFee,
} from "./_utils";
import { MissingParamError } from "./_errors";

Expand Down Expand Up @@ -164,34 +165,47 @@ const handler = async (
message,
};

const [tokenPriceNative, _tokenPriceUsd, latestBlock, gasPrice] =
await Promise.all([
getCachedTokenPrice(
l1Token.address,
sdk.utils.getNativeTokenSymbol(destinationChainId).toLowerCase()
),
getCachedTokenPrice(l1Token.address, "usd"),
getCachedLatestBlock(HUB_POOL_CHAIN_ID),
// If Linea, then we will defer gas price estimation to the SDK in getCachedFillGasUsage because
// the priority fee depends upon the fill transaction calldata.
destinationChainId === CHAIN_IDs.LINEA
? undefined
: latestGasPriceCache(destinationChainId).get(),
]);
const [
tokenPriceNative,
_tokenPriceUsd,
latestBlock,
gasPrice,
nativeGasCost,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added nativeGasCost to this promise.all since now its no longer dependent on gasPrice.

] = await Promise.all([
getCachedTokenPrice(
l1Token.address,
sdk.utils.getNativeTokenSymbol(destinationChainId).toLowerCase()
),
getCachedTokenPrice(l1Token.address, "usd"),
getCachedLatestBlock(HUB_POOL_CHAIN_ID),
// We only want to derive an unsigned fill txn from the deposit args if the destination chain is Linea
// because only Linea's priority fee depends on the destination chain call data.
latestGasPriceCache(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latestGasPriceCache gets called now for every single chain, as opposed to skipping for Linea, but for Linea, the depositArgs are defined

destinationChainId,
CHAIN_IDs.LINEA === destinationChainId ? depositArgs : undefined,
{
relayerAddress: relayer,
}
).get(),
isMessageDefined
? undefined // Only use cached gas units if message is not defined, i.e. standard for standard bridges
: getCachedNativeGasCost(depositArgs, { relayerAddress: relayer }),
]);
const tokenPriceUsd = ethers.utils.parseUnits(_tokenPriceUsd.toString());

const [
gasCosts,
opStackL1GasCost,
multicallOutput,
fullRelayerBalances,
transferRestrictedBalances,
fullRelayerMainnetBalances,
] = await Promise.all([
isMessageDefined
? undefined // Only use cached gas units if message is not defined, i.e. standard for standard bridges
: getCachedFillGasUsage(depositArgs, gasPrice, {
nativeGasCost && sdk.utils.chainIsOPStack(destinationChainId)
? // Only use cached gas units if message is not defined, i.e. standard for standard bridges
getCachedOpStackL1DataFee(depositArgs, nativeGasCost, {
relayerAddress: relayer,
}),
})
: undefined,
callViaMulticall3(provider, multiCalls, {
blockTag: latestBlock.number,
}),
Expand Down Expand Up @@ -221,15 +235,21 @@ const handler = async (
)
),
]);
// This call should not make any additional RPC queries if gasCosts is defined--for any deposit
// with an empty message.
// This call should not make any additional RPC queries since we are passing in gasPrice, nativeGasCost
// and tokenGasCost.
const tokenGasCost =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer cache the tokenGasCost as its dependent on the gasPrice, the nativeGasCost and the opStackL1GasCost, each of which should be cached at different intervals and using those values we can trivially derive the tokenGasCost

nativeGasCost && gasPrice
? nativeGasCost
.mul(gasPrice)
.add(opStackL1GasCost ?? ethers.BigNumber.from("0"))
: undefined;
const relayerFeeDetails = await getRelayerFeeDetails(
depositArgs,
tokenPriceNative,
relayer,
gasPrice,
gasCosts?.nativeGasCost,
gasCosts?.tokenGasCost
nativeGasCost,
tokenGasCost
Comment on lines +253 to +254
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Should we also return those values as part of the response? I think it could be useful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

);
logger.debug({
at: "Limits",
Expand Down Expand Up @@ -401,9 +421,9 @@ const handler = async (
message: "Response data",
responseJson,
});
// Respond with a 200 status code and 10 seconds of cache with
// 45 seconds of stale-while-revalidate.
sendResponse(response, responseJson, 200, 10, 45);
// Respond with a 200 status code and 1 second of cache time with
// 59s to keep serving the stale data while recomputing the cached value.
sendResponse(response, responseJson, 200, 1, 59);
} catch (error: unknown) {
return handleErrorCondition("limits", response, logger, error);
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"@across-protocol/constants": "^3.1.24",
"@across-protocol/contracts": "^3.0.19",
"@across-protocol/contracts-v3.0.6": "npm:@across-protocol/[email protected]",
"@across-protocol/sdk": "^3.4.8",
"@across-protocol/sdk": "^3.4.10-beta.1",
"@amplitude/analytics-browser": "^2.3.5",
"@balancer-labs/sdk": "1.1.6-beta.16",
"@emotion/react": "^11.13.0",
Expand Down
Loading
Loading