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.

Sign up for GitHub

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): Pass gasPrice into relayerFeeDetails call + add OP_STACK_L1_DATA_FEE_MARKUP #1366

Merged
merged 7 commits into from
Jan 10, 2025
Merged

improve(API): Pass gasPrice into relayerFeeDetails call + add OP_STACK_L1_DATA_FEE_MARKUP #1366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5cdec88
improve(API): Pass gasPrice into relayerFeeDetails call
nicholaspai Jan 9, 2025
3db3ac1
Add cache prefix
nicholaspai Jan 9, 2025
b9e4640
Add op stack multiplier env var
nicholaspai Jan 9, 2025
866ddda
Defer gas price computation for Linea which depends on the unsignedTx
nicholaspai Jan 9, 2025
5c78f58
import 3.4.8, smplify gas-prices, and don't pass in Linea chain ID in…
nicholaspai Jan 9, 2025
380ea4e
Use correct linea price in gas-prices and pass markups correctly in g…
nicholaspai Jan 10, 2025
dc6becb
Update _utils.ts
nicholaspai Jan 10, 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
5 changes: 4 additions & 1 deletion api/_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ export function buildCacheKey(
}

export function buildInternalCacheKey(...args: (string | number)[]): string {
return buildCacheKey("QUOTES_API", ...args);
return buildCacheKey(
`${process.env.CACHE_PREFIX ? process.env.CACHE_PREFIX + "-" : ""}QUOTES_API`,
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
...args
);
}

export async function getCachedValue<T>(
Expand Down
46 changes: 38 additions & 8 deletions api/_utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const {
REACT_APP_COINGECKO_PRO_API_KEY,
BASE_FEE_MARKUP,
PRIORITY_FEE_MARKUP,
OP_STACK_L1_DATA_FEE_MARKUP,
VERCEL_ENV,
LOG_LEVEL,
} = process.env;
Expand All @@ -97,6 +98,9 @@ export const baseFeeMarkup: {
export const priorityFeeMarkup: {
[chainId: string]: number;
} = JSON.parse(PRIORITY_FEE_MARKUP || "{}");
export const opStackL1DataFeeMarkup: {
[chainId: string]: number;
} = JSON.parse(OP_STACK_L1_DATA_FEE_MARKUP || "{}");
// Default to no markup.
export const DEFAULT_GAS_MARKUP = 0;

Expand Down Expand Up @@ -594,9 +598,14 @@ export const getHubPoolClient = () => {

export const getGasMarkup = (
chainId: string | number
): { baseFeeMarkup: BigNumber; priorityFeeMarkup: BigNumber } => {
): {
baseFeeMarkup: BigNumber;
priorityFeeMarkup: BigNumber;
opStackL1DataFeeMarkup: BigNumber;
} => {
let _baseFeeMarkup: BigNumber | undefined;
let _priorityFeeMarkup: BigNumber | undefined;
let _opStackL1DataFeeMarkup: BigNumber | undefined;
if (typeof baseFeeMarkup[chainId] === "number") {
_baseFeeMarkup = utils.parseEther((1 + baseFeeMarkup[chainId]).toString());
}
Expand All @@ -605,6 +614,11 @@ export const getGasMarkup = (
(1 + priorityFeeMarkup[chainId]).toString()
);
}
if (typeof opStackL1DataFeeMarkup[chainId] === "number") {
_opStackL1DataFeeMarkup = utils.parseEther(
(1 + opStackL1DataFeeMarkup[chainId]).toString()
);
}

// Otherwise, use default gas markup (or optimism's for OP stack).
if (_baseFeeMarkup === undefined) {
Expand All @@ -627,11 +641,21 @@ export const getGasMarkup = (
).toString()
);
}
if (_opStackL1DataFeeMarkup === undefined) {
_opStackL1DataFeeMarkup = utils.parseEther(
(
1 +
(sdk.utils.chainIsOPStack(Number(chainId))
? opStackL1DataFeeMarkup[CHAIN_IDs.OPTIMISM] ?? DEFAULT_GAS_MARKUP
: DEFAULT_GAS_MARKUP)
).toString()
);
}

// Otherwise, use default gas markup (or optimism's for OP stack).
return {
baseFeeMarkup: _baseFeeMarkup,
priorityFeeMarkup: _priorityFeeMarkup,
opStackL1DataFeeMarkup: _opStackL1DataFeeMarkup,
};
};

Expand Down Expand Up @@ -709,7 +733,7 @@ export const getRelayerFeeDetails = async (
},
tokenPrice: number,
relayerAddress: string,
gasPrice: sdk.utils.BigNumberish,
gasPrice?: sdk.utils.BigNumberish,
gasUnits?: sdk.utils.BigNumberish,
tokenGasCost?: sdk.utils.BigNumberish
): Promise<sdk.relayFeeCalculator.RelayerFeeDetails> => {
Expand Down Expand Up @@ -1948,8 +1972,8 @@ export function isContractCache(chainId: number, address: string) {

export function getCachedFillGasUsage(
deposit: Parameters<typeof buildDepositForSimulation>[0],
gasPrice?: BigNumber,
overrides?: Partial<{
spokePoolAddress: string;
relayerAddress: string;
}>
) {
Expand All @@ -1971,14 +1995,20 @@ export function getCachedFillGasUsage(
);
// 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,
{
// Scale the op stack L1 gas cost component by the base fee multiplier.
// Consider adding a new environment variable OP_STACK_L1_GAS_COST_MARKUP if we want finer-grained control.
opStackL1GasCostMultiplier: getGasMarkup(deposit.destinationChainId)
.baseFeeMarkup,
gasPrice,
Copy link
Member Author

Choose a reason for hiding this comment

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

Passing this in avoids a redundant SDK.GasPriceOracle.getGasPriceEstimate call

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, if we don't pass in this gas price, then this function will return gasCosts that we use in the relayerFeeCalculator but it recomputes the gas price without any multipliers. So, the resultant tokenGasCost is likely too low

// 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,
}
);
return {
Expand Down
70 changes: 33 additions & 37 deletions api/gas-prices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@
sendResponse,
} from "./_utils";
import { TypedVercelRequest } from "./_types";
import { ethers, providers, VoidSigner } from "ethers";

Check warning on line 12 in api/gas-prices.ts

View workflow job for this annotation

GitHub Actions / format-and-lint 

'providers' is defined but never used

Check warning on line 12 in api/gas-prices.ts

View workflow job for this annotation

GitHub Actions / format-and-lint 

'VoidSigner' is defined but never used
import * as sdk from "@across-protocol/sdk";
import { L2Provider } from "@eth-optimism/sdk/dist/interfaces/l2-provider";

Check warning on line 14 in api/gas-prices.ts

View workflow job for this annotation

GitHub Actions / format-and-lint 

'L2Provider' is defined but never used

import mainnetChains from "../src/data/chains_1.json";
import {
CHAIN_IDs,
DEFAULT_SIMULATED_RECIPIENT_ADDRESS,
TOKEN_SYMBOLS_MAP,
} from "./_constants";
Expand Down Expand Up @@ -70,50 +71,32 @@
const relayerFeeCalculatorQueries = getRelayerFeeCalculatorQueries(
Number(chainId)
);
const opStackL1GasCostMultiplier = getGasMarkup(
Copy link
Member Author

Choose a reason for hiding this comment

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

All the changes in this file align it with the /limits implementation for computing gas costs:
making sure for Linea we don't pass in gasPrice and let SDK.getGasCosts estimate gas costs using the unsignedTx from the deposit

Number(chainId)
).baseFeeMarkup;
const { nativeGasCost, tokenGasCost } =
const { baseFeeMarkup, priorityFeeMarkup, opStackL1DataFeeMarkup } =
getGasMarkup(Number(chainId));
const { nativeGasCost, tokenGasCost, opStackL1GasCost, gasPrice } =
await relayerFeeCalculatorQueries.getGasCosts(
deposit,
relayerFeeCalculatorQueries.simulatedRelayerAddress,
{
// Pass in the already-computed gasPrice into this query so that the tokenGasCost includes
// the scaled gas price,
// e.g. tokenGasCost = nativeGasCost * (baseFee * baseFeeMultiplier + priorityFee).
gasPrice: gasPrices[i].maxFeePerGas,
opStackL1GasCostMultiplier,
// Except for Linea, where the gas price is dependent on the unsignedTx produced from the deposit,
// so let the SDK compute its gas price here.
gasPrice:
Number(chainId) === CHAIN_IDs.LINEA
? undefined
: gasPrices[i].maxFeePerGas,
opStackL1GasCostMultiplier: opStackL1DataFeeMarkup,
baseFeeMultiplier: baseFeeMarkup,
priorityFeeMultiplier: priorityFeeMarkup,
}
);
// OPStack chains factor in the L1 gas cost of including the L2 transaction in an L1 rollup batch
// into the total gas cost of the L2 transaction.
let opStackL1GasCost: ethers.BigNumber | undefined = undefined;
if (sdk.utils.chainIsOPStack(Number(chainId))) {
Copy link
Member Author

Choose a reason for hiding this comment

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

All this opStackL1GasCost recomputation can be replaced because the SDK as of 3.4.8 returns the opStackL1GasCost

const provider = relayerFeeCalculatorQueries.provider;
const _unsignedTx = await sdk.utils.populateV3Relay(
relayerFeeCalculatorQueries.spokePool,
deposit,
relayerFeeCalculatorQueries.simulatedRelayerAddress
);
const voidSigner = new VoidSigner(
relayerFeeCalculatorQueries.simulatedRelayerAddress,
relayerFeeCalculatorQueries.provider
);
const unsignedTx = await voidSigner.populateTransaction({
..._unsignedTx,
gasLimit: nativeGasCost, // prevents additional gas estimation call
});
opStackL1GasCost = await (
provider as L2Provider<providers.Provider>
).estimateL1GasCost(unsignedTx);
opStackL1GasCost = opStackL1GasCostMultiplier
.mul(opStackL1GasCost)
.div(sdk.utils.fixedPointAdjustment);
}
return {
nativeGasCost,
tokenGasCost,
opStackL1GasCost,
gasPrice,
};
}
)
Expand All @@ -124,12 +107,23 @@
Object.keys(chainIdsWithToken).map((chainId, i) => [
chainId,
{
gasPrice: gasPrices[i].maxFeePerGas.toString(),
gasPrice:
Copy link
Member Author

@nicholaspai nicholaspai Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For linea we can infer the gas fee components trivially because the base is hardcoded at 7

Number(chainId) === CHAIN_IDs.LINEA
? gasCosts[i].gasPrice.toString()
: gasPrices[i].maxFeePerGas.toString(),
gasPriceComponents: {
maxFeePerGas: gasPrices[i].maxFeePerGas
.sub(gasPrices[i].maxPriorityFeePerGas)
.toString(),
priorityFeePerGas: gasPrices[i].maxPriorityFeePerGas.toString(),
// Linea hardcodes base fee at 7 wei so we can always back it out fromthe gasPrice returned by the
// getGasCosts method.
maxFeePerGas:
Number(chainId) === CHAIN_IDs.LINEA
? gasCosts[i].gasPrice.sub(7).toString()
: gasPrices[i].maxFeePerGas
.sub(gasPrices[i].maxPriorityFeePerGas)
.toString(),
priorityFeePerGas:
Number(chainId) === CHAIN_IDs.LINEA
? "7"
: gasPrices[i].maxPriorityFeePerGas.toString(),
baseFeeMultiplier: ethers.utils.formatEther(
getGasMarkup(chainId).baseFeeMarkup
),
Expand All @@ -139,7 +133,9 @@
opStackL1GasCostMultiplier: sdk.utils.chainIsOPStack(
Number(chainId)
)
? ethers.utils.formatEther(getGasMarkup(chainId).baseFeeMarkup)
? ethers.utils.formatEther(
getGasMarkup(chainId).opStackL1DataFeeMarkup
)
: undefined,
},
nativeGasCost: gasCosts[i].nativeGasCost.toString(),
Expand Down
37 changes: 21 additions & 16 deletions api/limits.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,39 +164,34 @@ const handler = async (
message,
};

const [tokenPriceNative, _tokenPriceUsd, latestBlock, gasCosts, gasPrice] =
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),
// Only use cached gas units if message is not defined, i.e. standard for standard bridges
isMessageDefined
// 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
: getCachedFillGasUsage(depositArgs, {
relayerAddress: relayer,
}),
latestGasPriceCache(destinationChainId).get(),
: latestGasPriceCache(destinationChainId).get(),
]);
const tokenPriceUsd = ethers.utils.parseUnits(_tokenPriceUsd.toString());

const [
relayerFeeDetails,
gasCosts,
multicallOutput,
fullRelayerBalances,
transferRestrictedBalances,
fullRelayerMainnetBalances,
] = await Promise.all([
getRelayerFeeDetails(
depositArgs,
tokenPriceNative,
relayer,
gasPrice,
gasCosts?.nativeGasCost,
gasCosts?.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.

Currently, we are using this tokenGasCost that wasn't dependent on the gasPrice we computed in the latestGasPriceCache(destinationChainId).get() call, which means that this tokenGasCost estimate is low

),
isMessageDefined
? undefined // Only use cached gas units if message is not defined, i.e. standard for standard bridges
: getCachedFillGasUsage(depositArgs, gasPrice, {
relayerAddress: relayer,
}),
callViaMulticall3(provider, multiCalls, {
blockTag: latestBlock.number,
}),
Expand Down Expand Up @@ -226,6 +221,16 @@ const handler = async (
)
),
]);
// This call should not make any additional RPC queries if gasCosts is defined--for any deposit
// with an empty message.
const relayerFeeDetails = await getRelayerFeeDetails(
depositArgs,
tokenPriceNative,
relayer,
gasPrice,
gasCosts?.nativeGasCost,
gasCosts?.tokenGasCost
);
logger.debug({
at: "Limits",
message: "Relayer fee details from SDK",
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
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.7",
"@across-protocol/sdk": "^3.4.8",
"@amplitude/analytics-browser": "^2.3.5",
"@balancer-labs/sdk": "1.1.6-beta.16",
"@emotion/react": "^11.13.0",
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@
yargs "^17.7.2"
zksync-web3 "^0.14.3"

"@across-protocol/sdk@^3.4.7":
version "3.4.7"
resolved "https://registry.yarnpkg.com/@across-protocol/sdk/-/sdk-3.4.7.tgz#6ddf9698f918d7b7e0216327d60b54b37fe14f22"
integrity sha512-GeyzDG8EzlN8oddmjXASqND+usZPkWDLpzbdWfAfBfHT3pjIMatntZqZghfCfjy+ICf+rlYrAb8I24H4jlct8Q==
"@across-protocol/sdk@^3.4.8":
version "3.4.8"
resolved "https://registry.yarnpkg.com/@across-protocol/sdk/-/sdk-3.4.8.tgz#070abb97b687cfe22d89349d776f68008af5f6a7"
integrity sha512-m4JnT3Sh+zmTZ/Oi7QrTv3IuNB+myBcbnPnEEssZroyBost/yEPyxXks+EHeU67KrT84t/otGyNb5YpTOvOK0A==
dependencies:
"@across-protocol/across-token" "^1.0.0"
"@across-protocol/constants" "^3.1.25"
Expand Down
Loading