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(GasPriceOracle): Remove eip1559Bad and lineaEthers adapters #819

Merged
merged 10 commits into from
Feb 13, 2025
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@across-protocol/sdk",
"author": "UMA Team",
"version": "4.1.11",
"version": "4.1.12",
"license": "AGPL-3.0",
"homepage": "https://docs.across.to/reference/sdk",
"files": [
Expand Down
44 changes: 2 additions & 42 deletions src/gasPriceOracle/adapters/ethereum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,16 @@ import { GasPriceEstimate } from "../types";
import { gasPriceError } from "../util";
import { GasPriceEstimateOptions } from "../oracle";

// TODO: We intend to remove `eip1559Bad()` as an option and make eip1559Raw the only option eventually. The reason
// they both exist currently is because eip1559Raw is new and untested on production so we will slowly roll it out
// by using the convenient environment variable safety guard.

/**
* @dev If GAS_PRICE_EIP1559_RAW_${chainId}=true, then constructs total fee by adding
* eth_getBlock("pending").baseFee to eth_maxPriorityFeePerGas, otherwise calls the ethers provider's
* getFeeData() method which adds eth_getBlock("latest").baseFee to a hardcoded priority fee of 1.5 gwei.
* @dev Constructs total fee by adding eth_getBlock("pending").baseFee to eth_maxPriorityFeePerGas
* @param provider ethers RPC provider instance.
* @param {GasPriceEstimateOptions} opts See notes below on specific parameters.
* @param baseFeeMultiplier Amount to multiply base fee or total fee for legacy gas pricing.
* @param priorityFeeMultiplier Amount to multiply priority fee or unused for legacy gas pricing.
* @returns Promise of gas price estimate object.
*/
export function eip1559(provider: providers.Provider, opts: GasPriceEstimateOptions): Promise<GasPriceEstimate> {
const useRaw = process.env[`GAS_PRICE_EIP1559_RAW_${opts.chainId}`] === "true";
return useRaw
? eip1559Raw(provider, opts.chainId, opts.baseFeeMultiplier, opts.priorityFeeMultiplier)
: eip1559Bad(provider, opts.chainId, opts.baseFeeMultiplier, opts.priorityFeeMultiplier);
return eip1559Raw(provider, opts.chainId, opts.baseFeeMultiplier, opts.priorityFeeMultiplier);
}

/**
Expand Down Expand Up @@ -58,37 +49,6 @@ export async function eip1559Raw(
};
}

/**
* @notice Returns fee data using provider's getFeeData() method.
* @note Resolves priority gas pricing poorly, because the priority fee is hardcoded to 1.5 Gwei in ethers v5's
* getFeeData() method
* @dev TODO: Remove this function soon. See note above about slowly rolling out eip1559Raw.
* @param provider ethers RPC provider instance.
* @param chainId Chain ID of the provider instance.
* @returns Promise of gas price estimate object.
*/
export async function eip1559Bad(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a dumb question, but we don't currently use this anywhere? No chain relies on it?

Totally fine with reversing the default, but want to make sure we're not breaking anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

in production no, we do not use it at all

provider: providers.Provider,
chainId: number,
baseFeeMultiplier: BigNumber,
priorityFeeMultiplier: BigNumber
): Promise<GasPriceEstimate> {
const feeData = await provider.getFeeData();

[feeData.lastBaseFeePerGas, feeData.maxPriorityFeePerGas].forEach((field: BigNumber | null) => {
if (!BigNumber.isBigNumber(field) || field.lt(bnZero)) gasPriceError("getFeeData()", chainId, feeData);
});

const maxPriorityFeePerGas = feeData.maxPriorityFeePerGas as BigNumber;
const scaledPriorityFee = maxPriorityFeePerGas.mul(priorityFeeMultiplier).div(fixedPointAdjustment);
const scaledLastBaseFeePerGas = (feeData.lastBaseFeePerGas as BigNumber)
.mul(baseFeeMultiplier)
.div(fixedPointAdjustment);
const maxFeePerGas = scaledPriorityFee.add(scaledLastBaseFeePerGas);

return { maxPriorityFeePerGas: scaledPriorityFee, maxFeePerGas };
}

/**
* @notice Returns result of eth_gasPrice RPC call
* @dev Its recommended to use the eip1559Raw method over this one where possible as it will be more accurate.
Expand Down
24 changes: 0 additions & 24 deletions src/gasPriceOracle/adapters/linea.ts

This file was deleted.

9 changes: 6 additions & 3 deletions src/gasPriceOracle/oracle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { GasPriceEstimate } from "./types";
import { getPublicClient } from "./util";
import * as arbitrum from "./adapters/arbitrum";
import * as ethereum from "./adapters/ethereum";
import * as linea from "./adapters/linea";
import * as polygon from "./adapters/polygon";
import * as lineaViem from "./adapters/linea-viem";

Expand All @@ -30,6 +29,9 @@ const GAS_PRICE_ESTIMATE_DEFAULTS = {
legacyFallback: true,
};

// Chains that use the Viem gas price oracle.
const VIEM_CHAINS = [CHAIN_IDs.LINEA];

/**
* Provide an estimate for the current gas price for a particular chain.
* @param provider A valid ethers provider.
Expand Down Expand Up @@ -61,7 +63,7 @@ export async function getGasPriceEstimate(
};

// We only use the unsignedTx in the viem flow.
const useViem = process.env[`NEW_GAS_PRICE_ORACLE_${chainId}`] === "true";
const useViem = VIEM_CHAINS.includes(chainId);
return useViem
? _getViemGasPriceEstimate(chainId, optsWithDefaults)
: _getEthersGasPriceEstimate(provider, optsWithDefaults);
Expand All @@ -80,10 +82,11 @@ function _getEthersGasPriceEstimate(
): Promise<GasPriceEstimate> {
const { chainId, legacyFallback } = opts;

// There shouldn't be any chains in here that we have a Viem adapter for because we'll always use Viem in that case.
assert(!VIEM_CHAINS.includes(chainId), `Chain ID ${chainId} will use Viem gas price estimation`);
const gasPriceFeeds = {
[CHAIN_IDs.ALEPH_ZERO]: arbitrum.eip1559,
[CHAIN_IDs.ARBITRUM]: arbitrum.eip1559,
[CHAIN_IDs.LINEA]: linea.eip1559, // @todo: Support linea_estimateGas in adapter.
[CHAIN_IDs.MAINNET]: ethereum.eip1559,
[CHAIN_IDs.POLYGON]: polygon.gasStation,
[CHAIN_IDs.SCROLL]: ethereum.legacy,
Expand Down
36 changes: 1 addition & 35 deletions test/GasPriceOracle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ dotenv.config({ path: ".env" });
const stdLastBaseFeePerGas = parseUnits("12", 9);
const stdMaxPriorityFeePerGas = parseUnits("1", 9); // EIP-1559 chains only
const expectedLineaMaxFeePerGas = BigNumber.from("7");
const legacyChainIds = [324, 59144, 534352];
const legacyChainIds = [324, 534352];
const arbOrbitChainIds = [42161, 41455];
const ethersProviderChainIds = [10, 8453, ...legacyChainIds, ...arbOrbitChainIds];
const lineaEstimateGasUnsignedTxMultiplier = 2; // Amount that priority fee scales by if unsignedTx has data. Applied
Expand Down Expand Up @@ -81,8 +81,6 @@ describe("Gas Price Oracle", function () {
});
it("Linea Viem gas price retrieval with unsignedTx", async function () {
const chainId = 59144;
const chainKey = `NEW_GAS_PRICE_ORACLE_${chainId}`;
process.env[chainKey] = "true";
const priorityFeeMultiplier = toBNWei("2.0");
const unsignedTx = {
to: randomAddress(),
Expand All @@ -108,12 +106,9 @@ describe("Gas Price Oracle", function () {
);
expect(maxFeePerGas).to.equal(expectedLineaMaxFeePerGas.add(expectedPriorityFee));
expect(maxPriorityFeePerGas).to.equal(expectedPriorityFee);
delete process.env[chainKey];
});
it("Linea Viem gas price retrieval", async function () {
const chainId = 59144;
const chainKey = `NEW_GAS_PRICE_ORACLE_${chainId}`;
process.env[chainKey] = "true";
const priorityFeeMultiplier = toBNWei("2.0");
const { maxFeePerGas, maxPriorityFeePerGas } = await getGasPriceEstimate(provider, {
chainId,
Expand All @@ -127,7 +122,6 @@ describe("Gas Price Oracle", function () {
const expectedPriorityFee = stdMaxPriorityFeePerGas.mul(priorityFeeMultiplier).div(fixedPointAdjustment);
expect(maxFeePerGas).to.equal(expectedLineaMaxFeePerGas.add(expectedPriorityFee));
expect(maxPriorityFeePerGas).to.equal(expectedPriorityFee);
delete process.env[chainKey];
});
it("Ethers gas price retrieval", async function () {
const baseFeeMultiplier = toBNWei("2.0");
Expand Down Expand Up @@ -188,9 +182,7 @@ describe("Gas Price Oracle", function () {
const minPriorityFeeScaler = "1.5";
const minPriorityFee = parseUnits(minPriorityFeeScaler, 9);
const chainId = 1;
const chainKey = `GAS_PRICE_EIP1559_RAW_${chainId}`;
const minPriorityFeeKey = `MIN_PRIORITY_FEE_PER_GAS_${chainId}`;
process.env[chainKey] = "true";
process.env[minPriorityFeeKey] = minPriorityFeeScaler;

const { maxFeePerGas: markedUpMaxFeePerGas, maxPriorityFeePerGas: markedUpMaxPriorityFeePerGas } =
Expand All @@ -207,7 +199,6 @@ describe("Gas Price Oracle", function () {

// Priority fees should be scaled.
expect(markedUpMaxPriorityFeePerGas).to.equal(expectedMarkedUpPriorityFee);
delete process.env[chainKey];
delete process.env[minPriorityFeeKey];
});
it("Ethers EIP1559 Raw: min priority fee is ignored", async function () {
Expand All @@ -217,9 +208,7 @@ describe("Gas Price Oracle", function () {
const minPriorityFee = parseUnits(minPriorityFeeScaler, 9);
expect(minPriorityFee.lt(stdMaxPriorityFeePerGas)).to.be.true;
const chainId = 1;
const chainKey = `GAS_PRICE_EIP1559_RAW_${chainId}`;
const minPriorityFeeKey = `MIN_PRIORITY_FEE_PER_GAS_${chainId}`;
process.env[chainKey] = "true";
process.env[minPriorityFeeKey] = minPriorityFeeScaler.toString();

const { maxFeePerGas: markedUpMaxFeePerGas, maxPriorityFeePerGas: markedUpMaxPriorityFeePerGas } =
Expand All @@ -236,31 +225,8 @@ describe("Gas Price Oracle", function () {

// Priority fees should be scaled.
expect(markedUpMaxPriorityFeePerGas).to.equal(expectedMarkedUpPriorityFee);
delete process.env[chainKey];
delete process.env[minPriorityFeeKey];
});
it("Ethers EIP1559 Bad", async function () {
// This test should return identical results to the Raw test but it makes different
// provider calls, so we're really testing that the expected provider functions are called.
const baseFeeMultiplier = toBNWei("2.0");
const priorityFeeMultiplier = toBNWei("1.5");
const chainId = 1;

const { maxFeePerGas: markedUpMaxFeePerGas, maxPriorityFeePerGas: markedUpMaxPriorityFeePerGas } =
await getGasPriceEstimate(provider, { chainId, baseFeeMultiplier, priorityFeeMultiplier });

// Base fee should be multiplied by multiplier. Returned max fee includes priority fee
// so back it out before scaling.
const expectedMarkedUpPriorityFee = stdMaxPriorityFeePerGas.mul(priorityFeeMultiplier).div(fixedPointAdjustment);
const expectedMarkedUpMaxFeePerGas = stdLastBaseFeePerGas
.mul(baseFeeMultiplier)
.div(fixedPointAdjustment)
.add(expectedMarkedUpPriorityFee);
expect(markedUpMaxFeePerGas).to.equal(expectedMarkedUpMaxFeePerGas);

// Priority fees should be scaled.
expect(markedUpMaxPriorityFeePerGas).to.equal(expectedMarkedUpPriorityFee);
});
it("Ethers Legacy", async function () {
const baseFeeMultiplier = toBNWei("2.0");
const priorityFeeMultiplier = toBNWei("1.5");
Expand Down