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: Use base fee multiplier option in SDK to resolve gas fees #1969

Merged
merged 27 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
713f792
improve(TransactionUtils): Use base fee multiplier
nicholaspai Dec 24, 2024
294893d
Update TransactionUtils.ts
nicholaspai Dec 25, 2024
c6a01fe
Add unsignedTx to getGasPriceEstimate call in TransactionUtils `runTr…
nicholaspai Dec 27, 2024
712da98
Add warning
nicholaspai Jan 2, 2025
f0531a1
Use baseFeeMultiplier in ProfitClient
nicholaspai Jan 3, 2025
7825206
Remove relayer specific gas padding, add unit test about relayer fee …
nicholaspai Jan 3, 2025
450d134
Fix test
nicholaspai Jan 3, 2025
98ab1ed
Revert changes to ProfitClient
nicholaspai Jan 3, 2025
110f3ea
Merge branch 'master' into apply-base-fee-multiplier
nicholaspai Jan 3, 2025
adada46
Update TransactionUtils.ts
nicholaspai Jan 3, 2025
02d28b1
import sdk
nicholaspai Jan 3, 2025
5a7c517
3.4.1
nicholaspai Jan 3, 2025
a68e847
3.4.2
nicholaspai Jan 3, 2025
3a0784d
3.4.3
nicholaspai Jan 3, 2025
bacb4e2
Update logs
nicholaspai Jan 5, 2025
9cb8d13
Update ProfitClient.ts
nicholaspai Jan 5, 2025
f5db268
Merge branch 'master' into apply-base-fee-multiplier
nicholaspai Jan 6, 2025
dfa5c6c
Update TransactionUtils.ts
nicholaspai Jan 6, 2025
b7d3bf8
Update package.json
nicholaspai Jan 6, 2025
47b5a5b
Update yarn.lock
nicholaspai Jan 6, 2025
c210db2
Update TransactionUtils.ts
nicholaspai Jan 6, 2025
6f7c456
Update TransactionUtils.ts
nicholaspai Jan 6, 2025
5d2c1ca
Merge branch 'master' into apply-base-fee-multiplier
nicholaspai Jan 6, 2025
6e27a77
Update package.json
nicholaspai Jan 6, 2025
d586295
3.4.5-beta.1
nicholaspai Jan 6, 2025
7b2ae88
3.4.5
nicholaspai Jan 6, 2025
c1cc89b
Merge branch 'master' into apply-base-fee-multiplier
nicholaspai Jan 7, 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
2 changes: 1 addition & 1 deletion scripts/zkSyncDemo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export async function run(): Promise<void> {
connectedSigner
);
const l2PubdataByteLimit = zksync.utils.REQUIRED_L1_TO_L2_GAS_PER_PUBDATA_LIMIT;
const l1GasPriceData = await gasPriceOracle.getGasPriceEstimate(l1Provider, l1ChainId);
const l1GasPriceData = await gasPriceOracle.getGasPriceEstimate(l1Provider, { chainId: l1ChainId });
const estimatedL1GasPrice = l1GasPriceData.maxPriorityFeePerGas.add(l1GasPriceData.maxFeePerGas);
// The ZkSync Mailbox contract checks that the msg.value of the transaction is enough to cover the transaction base
// cost. The transaction base cost can be queried from the Mailbox by passing in an L1 "executed" gas price,
Expand Down
20 changes: 12 additions & 8 deletions src/clients/ProfitClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class ProfitClient {
protected totalGasCosts: { [chainId: number]: TransactionCostEstimate } = {};

// Queries needed to fetch relay gas costs.
private relayerFeeQueries: { [chainId: number]: relayFeeCalculator.QueryInterface } = {};
protected relayerFeeQueries: { [chainId: number]: relayFeeCalculator.QueryInterface } = {};

private readonly isTestnet: boolean;

Expand Down Expand Up @@ -205,7 +205,9 @@ export class ProfitClient {

private async _getTotalGasCost(deposit: Deposit, relayer: string): Promise<TransactionCostEstimate> {
try {
return await this.relayerFeeQueries[deposit.destinationChainId].getGasCosts(deposit, relayer);
return await this.relayerFeeQueries[deposit.destinationChainId].getGasCosts(deposit, relayer, {
baseFeeMultiplier: this.gasPadding,
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
});
} catch (err) {
const reason = isEthersError(err) ? err.reason : isError(err) ? err.message : "unknown error";
this.logger.warn({
Expand Down Expand Up @@ -243,8 +245,15 @@ export class ProfitClient {

const gasToken = this.resolveGasToken(chainId);
const gasTokenPriceUsd = this.getPriceOfToken(gasToken.symbol);
// This is where the relayer's computation of the tokenGasCost (gasUnits * gasFee) can diverge from the
// quoting API's. The latter uses the SDK's GasPriceOracle.getGasPriceEstimate() function and passes in an optional
// baseFeeMultiplier. The relayer uses this.getTotalGasCost() to compute the gasUnits the same way that the
// API does and then computes the tokenGasCost by computing the gasPrice using a baseFeeMultiplier set as this
// client's this.gasPadding. Therefore, this.getTotalGasCost will importantly set
// baseFeeMultiplier = this.gasPadding.
const totalGasCost = await this.getTotalGasCost(deposit);
let { nativeGasCost, tokenGasCost } = totalGasCost;
let tokenGasCost = totalGasCost.tokenGasCost;
const nativeGasCost = totalGasCost.nativeGasCost;
const gasPrice = totalGasCost.gasPrice;

Object.entries({
Expand All @@ -257,11 +266,6 @@ export class ProfitClient {
}
});

// Fills with messages have arbitrary execution and therefore lower certainty about the simulated execution cost.
// Pad these estimates before computing profitability to allow execution headroom and reduce the chance of an OoG.
nativeGasCost = nativeGasCost.mul(this.gasPadding).div(fixedPoint);
tokenGasCost = tokenGasCost.mul(this.gasPadding).div(fixedPoint);

// Gas estimates for token-only fills are stable and reliable. Allow these to be scaled up or down via the
// configured gasMultiplier. Do not scale the nativeGasCost, since it might be used to set the transaction gasLimit.
// @todo Consider phasing this out and relying solely on the minimum profitability config.
Expand Down
22 changes: 15 additions & 7 deletions src/utils/TransactionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,12 @@ export async function runTransaction(
Number(process.env[`MAX_FEE_PER_GAS_SCALER_${chainId}`] || process.env.MAX_FEE_PER_GAS_SCALER) ||
DEFAULT_GAS_FEE_SCALERS[chainId]?.maxFeePerGasScaler;

const gas = await getGasPrice(provider, priorityFeeScaler, maxFeePerGasScaler);
const gas = await getGasPrice(
provider,
priorityFeeScaler,
maxFeePerGasScaler,
await contract.populateTransaction[method](...(args as Array<unknown>), { value })
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
);

logger.debug({
at: "TxUtil",
Expand Down Expand Up @@ -154,16 +159,19 @@ export async function runTransaction(
}
}

// TODO: add in gasPrice when the SDK has this for the given chainId. TODO: improve how we fetch prices.
// For now this method will extract the provider's Fee data from the associated network and scale it by a priority
// scaler. This works on both mainnet and L2's by the utility switching the response structure accordingly.
export async function getGasPrice(
provider: ethers.providers.Provider,
priorityScaler = 1.2,
maxFeePerGasScaler = 3
maxFeePerGasScaler = 3,
transactionObject?: ethers.PopulatedTransaction
): Promise<Partial<FeeData>> {
const { chainId } = await provider.getNetwork();
const feeData = await gasPriceOracle.getGasPriceEstimate(provider, chainId);
// Pass in unsignedTx here for better Linea gas price estimations via the Linea Viem provider.
const feeData = await gasPriceOracle.getGasPriceEstimate(provider, {
chainId,
baseFeeMultiplier: toBNWei(maxFeePerGasScaler),
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
unsignedTx: transactionObject,
});

if (feeData.maxPriorityFeePerGas.gt(feeData.maxFeePerGas)) {
feeData.maxFeePerGas = scaleByNumber(feeData.maxPriorityFeePerGas, 1.5);
Expand All @@ -176,7 +184,7 @@ export async function getGasPrice(

// Default to EIP-1559 (type 2) pricing.
return {
maxFeePerGas: scaleByNumber(feeData.maxFeePerGas, Math.max(priorityScaler * maxFeePerGasScaler, 1)),
maxFeePerGas: feeData.maxFeePerGas,
maxPriorityFeePerGas: scaleByNumber(feeData.maxPriorityFeePerGas, priorityScaler),
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
};
}
Expand Down
57 changes: 40 additions & 17 deletions test/ProfitClient.ConsiderProfitability.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { assert } from "chai";
import { random } from "lodash";
import { constants as sdkConstants, utils as sdkUtils } from "@across-protocol/sdk";
import { constants as sdkConstants, utils as sdkUtils, relayFeeCalculator } from "@across-protocol/sdk";
import { ConfigStoreClient, FillProfit, SpokePoolClient } from "../src/clients";
import { Deposit } from "../src/interfaces";
import {
Expand All @@ -16,6 +16,7 @@ import {
toBNWei,
toGWei,
TOKEN_SYMBOLS_MAP,
fixedPointAdjustment,
} from "../src/utils";
import { MockHubPoolClient, MockProfitClient } from "./mocks";
import { originChainId, destinationChainId, ZERO_ADDRESS } from "./constants";
Expand Down Expand Up @@ -207,28 +208,50 @@ describe("ProfitClient: Consider relay profit", () => {
}
});

it("Verify gas padding", async () => {
it("Verify gas padding on SDK call is set correctly", async () => {
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
class ExampleQueries implements relayFeeCalculator.QueryInterface {
GAS_PRICE(): BigNumber {
return toGWei("1");
}
// We only test that the baseFeeMultiplier is being correctly passed from the ProfitClient
// to the RelayFeeCalculator using this class
getGasCosts(_deposit: Deposit, _relayer: string, options?: any): Promise<TransactionCostEstimate> {
const gasCost = BigNumber.from(300_000);
const gasPrice = this.GAS_PRICE()
.mul(options?.baseFeeMultiplier ?? fixedPointAdjustment)
.div(fixedPointAdjustment);
return Promise.resolve({
nativeGasCost: gasCost,
tokenGasCost: toBN(gasCost).mul(gasPrice),
gasPrice,
});
}

// Unused functions in the QueryInterface
getTokenPrice(): Promise<number> {
return Promise.resolve(1);
}
getTokenDecimals(): number {
return 18;
}
}
const gasPadding = ["0", "0.10", "0.20", "0.50", "1"].map((padding) => toBNWei("1").add(toBNWei(padding)));

profitClient.setGasMultiplier(toBNWei("1")); // Neutralise any gas multiplier.
profitClient.setGasCosts({}); // Neutralise any hardcoded gas costs so ExampleQueries.getGasCosts() is called

for (const destinationChainId of chainIds) {
spyLogger.debug({ message: `Verifying gas padding for chainId ${destinationChainId}.` });
const deposit = { ...v3DepositTemplate, destinationChainId };

const { nativeGasCost: defaultNativeGasCost, tokenGasCost: defaultTokenGasCost } =
await profitClient.getTotalGasCost(deposit);
const chainId = chainIds[0];
const exampleQuery = new ExampleQueries();
profitClient.setRelayerFeeQueries({ [chainId]: exampleQuery });
spyLogger.debug({ message: `Verifying gas padding for chainId ${destinationChainId}.` });
const deposit = { ...v3DepositTemplate, destinationChainId: chainId };

for (const padding of gasPadding) {
profitClient.setGasPadding(padding);
for (const padding of gasPadding) {
profitClient.setGasPadding(padding);

const expectedNativeGasCost = defaultNativeGasCost.mul(padding).div(fixedPoint);
const expectedTokenGasCost = defaultTokenGasCost.mul(padding).div(fixedPoint);

const { nativeGasCost, tokenGasCost } = await profitClient.estimateFillCost(deposit);
expect(expectedNativeGasCost.eq(nativeGasCost)).to.be.true;
expect(expectedTokenGasCost.eq(tokenGasCost)).to.be.true;
}
const expectedGasPrice = exampleQuery.GAS_PRICE().mul(padding).div(fixedPoint);
const { gasPrice } = await profitClient.estimateFillCost(deposit);
expect(expectedGasPrice.eq(gasPrice)).to.be.true;
}
});

Expand Down
6 changes: 5 additions & 1 deletion test/mocks/MockProfitClient.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { utils as sdkUtils } from "@across-protocol/sdk";
import { utils as sdkUtils, relayFeeCalculator } from "@across-protocol/sdk";
import { ProfitClient } from "../../src/clients";
import { SpokePoolClientsByChain } from "../../src/interfaces";
import { bnOne, isDefined, TOKEN_SYMBOLS_MAP } from "../../src/utils";
Expand Down Expand Up @@ -77,6 +77,10 @@ export class MockProfitClient extends ProfitClient {
this.tokenSymbolMap[symbol] = address;
}

setRelayerFeeQueries(queries: { [chainId: number]: relayFeeCalculator.QueryInterface }): void {
this.relayerFeeQueries = queries;
}

setTokenPrice(token: string, price: BigNumber | undefined): void {
const address = this.resolveTokenAddress(token);
if (price) {
Expand Down
Loading