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: update sdk queries to accurately estimate gas costs #64

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 11 additions & 25 deletions src/relayFeeCalculator/chain-queries/arbitrum.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { QueryInterface } from "../relayFeeCalculator";
import { BigNumberish } from "../../utils";
import { BigNumber, providers } from "ethers";
import {
BigNumberish,
createUnsignedFillRelayTransaction,
estimateTotalGasRequiredByUnsignedTransaction,
} from "../../utils";
import { providers } from "ethers";
import { SymbolMapping } from "./ethereum";
import { Coingecko } from "../../coingecko/Coingecko";
import { ArbitrumSpokePool__factory, ArbitrumSpokePool } from "@across-protocol/contracts-v2";
import { SpokePool__factory, SpokePool } from "@across-protocol/contracts-v2";

export class ArbitrumQueries implements QueryInterface {
private spokePool: ArbitrumSpokePool;
private spokePool: SpokePool;

constructor(
readonly provider: providers.Provider,
Expand All @@ -15,13 +19,12 @@ export class ArbitrumQueries implements QueryInterface {
private readonly usdcAddress = "0xFF970A61A04b1cA14834A43f5dE4533eBDDB5CC8",
private readonly simulatedRelayerAddress = "0x893d0d70ad97717052e3aa8903d9615804167759"
) {
this.spokePool = ArbitrumSpokePool__factory.connect(spokePoolAddress, provider);
this.spokePool = SpokePool__factory.connect(spokePoolAddress, provider);
}

async getGasCosts(_tokenSymbol: string): Promise<BigNumberish> {
Copy link
Contributor

Choose a reason for hiding this comment

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

There's quite a lot of duplication across these different classes (arbitrum, polygon, etc.). Can we potentially change the QueryInterface they currently implement to be a class they can inherit all these common methods from? I think currently only Polygon has a different implementation of getTokenPrice() and all other ones look exactly the same (just different addresses which can easily be passed in the constructor)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree - I did notice code-duplication within the chain queries. In this PR I focused primarily on refactoring the duplication within getGasCosts

But, I think changing QueryInterface to a class structure (assuming there aren't any frontend/relayer dependencies) would be a great move. As you mentioned, the inheritance would help refactor the code into a more succinct format.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would actually remove all the code for all chains, if I'm not mistaken, except for Polygon, which needs to override getTokenPrice().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! And if we pass a coinGeckoBaseCurrency variable to the base class, we can use the shared implementation of super.getTokenPrice() for the first half of the Polygon getTokenPrice custom implementation (further cutting the code reuse).

const gasEstimate = await this.estimateGas();
const gasPrice = BigNumber.from(await this.provider.getGasPrice());
return gasPrice.mul(gasEstimate).toString();
const tx = await createUnsignedFillRelayTransaction(this.spokePool, this.usdcAddress, this.simulatedRelayerAddress);
return estimateTotalGasRequiredByUnsignedTransaction(tx, this.simulatedRelayerAddress, this.provider);
}

async getTokenPrice(tokenSymbol: string): Promise<string | number> {
Expand All @@ -34,21 +37,4 @@ export class ArbitrumQueries implements QueryInterface {
if (!this.symbolMapping[tokenSymbol]) throw new Error(`${tokenSymbol} does not exist in mapping`);
return this.symbolMapping[tokenSymbol].decimals;
}

estimateGas() {
// Create a dummy transaction to estimate. Note: the simulated caller would need to be holding weth and have approved the contract.
return this.spokePool.estimateGas.fillRelay(
"0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B",
"0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B",
this.usdcAddress,
"10",
"10",
"1",
"1",
"1",
"1",
"1",
{ from: this.simulatedRelayerAddress }
);
}
}
37 changes: 15 additions & 22 deletions src/relayFeeCalculator/chain-queries/boba.ts
Original file line number Diff line number Diff line change
@@ -1,45 +1,38 @@
import { QueryInterface } from "../relayFeeCalculator";
import { BigNumberish } from "../../utils";
import {
BigNumberish,
createUnsignedFillRelayTransaction,
estimateTotalGasRequiredByUnsignedTransaction,
} from "../../utils";
import { utils, providers } from "ethers";
import { SymbolMapping } from "./ethereum";
import { Coingecko } from "../../coingecko/Coingecko";
import { OptimismSpokePool__factory, OptimismSpokePool } from "@across-protocol/contracts-v2";
import { SpokePool__factory, SpokePool } from "@across-protocol/contracts-v2";

const { parseUnits } = utils;

export class BobaQueries implements QueryInterface {
private spokePool: OptimismSpokePool;
private spokePool: SpokePool;

constructor(
provider: providers.Provider,
readonly provider: providers.Provider,
readonly symbolMapping = SymbolMapping,
spokePoolAddress = "0xBbc6009fEfFc27ce705322832Cb2068F8C1e0A58",
private readonly usdcAddress = "0x66a2A913e447d6b4BF33EFbec43aAeF87890FBbc",
private readonly simulatedRelayerAddress = "0x893d0d70ad97717052e3aa8903d9615804167759"
) {
// TODO: replace with address getter.
this.spokePool = OptimismSpokePool__factory.connect(spokePoolAddress, provider);
this.spokePool = SpokePool__factory.connect(spokePoolAddress, provider);
}

async getGasCosts(_tokenSymbol: string): Promise<BigNumberish> {
// Create a dummy transaction to estimate. Note: the simulated caller would need to be holding weth and have approved the contract.
const gasEstimate = await this.spokePool.estimateGas.fillRelay(
"0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B",
"0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B",
this.usdcAddress,
"10",
"10",
"1",
"1",
"1",
"1",
"1",
{ from: this.simulatedRelayerAddress }
const tx = await createUnsignedFillRelayTransaction(this.spokePool, this.usdcAddress, this.simulatedRelayerAddress);
return estimateTotalGasRequiredByUnsignedTransaction(
tx,
this.simulatedRelayerAddress,
this.provider,
parseUnits("1", 9)
);

// Boba's gas price is hardcoded to 1 gwei.
const bobaGasPrice = parseUnits("1", 9);
return gasEstimate.mul(bobaGasPrice).toString();
}

async getTokenPrice(tokenSymbol: string): Promise<number> {
Expand Down
30 changes: 19 additions & 11 deletions src/relayFeeCalculator/chain-queries/ethereum.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { QueryInterface } from "../relayFeeCalculator";
import { BigNumberish } from "../../utils";
import {
BigNumberish,
createUnsignedFillRelayTransaction,
estimateTotalGasRequiredByUnsignedTransaction,
} from "../../utils";
import { Coingecko } from "../../coingecko/Coingecko";
import { providers, BigNumber } from "ethers";
import { providers } from "ethers";
import { SpokePool__factory, SpokePool } from "@across-protocol/contracts-v2";

// Note: these are the mainnet addresses for these symbols meant to be used for pricing.
export const SymbolMapping: { [symbol: string]: { address: string; decimals: number } } = {
Expand Down Expand Up @@ -67,18 +72,21 @@ export const SymbolMapping: { [symbol: string]: { address: string; decimals: num
},
};

export const defaultAverageGas = 116006;

export class EthereumQueries implements QueryInterface {
private spokePool: SpokePool;

constructor(
public readonly provider: providers.Provider,
public readonly averageGas = defaultAverageGas,
readonly symbolMapping = SymbolMapping
) {}
readonly provider: providers.Provider,
readonly symbolMapping = SymbolMapping,
readonly spokePoolAddress = "0x4D9079Bb4165aeb4084c526a32695dCfd2F77381",
readonly usdcAddress = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48",
readonly simulatedRelayerAddress = "0x893d0D70AD97717052E3AA8903D9615804167759"
) {
this.spokePool = SpokePool__factory.connect(this.spokePoolAddress, this.provider);
}
async getGasCosts(_tokenSymbol: string): Promise<BigNumberish> {
return BigNumber.from(await this.provider.getGasPrice())
.mul(this.averageGas)
.toString();
const tx = await createUnsignedFillRelayTransaction(this.spokePool, this.usdcAddress, this.simulatedRelayerAddress);
return estimateTotalGasRequiredByUnsignedTransaction(tx, this.simulatedRelayerAddress, this.provider);
}

async getTokenPrice(tokenSymbol: string): Promise<number> {
Expand Down
35 changes: 12 additions & 23 deletions src/relayFeeCalculator/chain-queries/optimism.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
import { QueryInterface } from "../relayFeeCalculator";
import { BigNumberish } from "../../utils";
import { providers, VoidSigner } from "ethers";
import {
BigNumberish,
createUnsignedFillRelayTransaction,
estimateTotalGasRequiredByUnsignedTransaction,
} from "../../utils";
import { providers } from "ethers";
import { SymbolMapping } from "./ethereum";
import { Coingecko } from "../../coingecko/Coingecko";
import { OptimismSpokePool__factory, OptimismSpokePool } from "@across-protocol/contracts-v2";
import { SpokePool__factory, SpokePool } from "@across-protocol/contracts-v2";

import { L2Provider, asL2Provider } from "@eth-optimism/sdk";

export class OptimismQueries implements QueryInterface {
private spokePool: OptimismSpokePool;
private provider: L2Provider<providers.Provider>;
private spokePool: SpokePool;
readonly provider: L2Provider<providers.Provider>;

constructor(
provider: providers.Provider,
Expand All @@ -19,27 +23,12 @@ export class OptimismQueries implements QueryInterface {
private readonly simulatedRelayerAddress = "0x893d0d70ad97717052e3aa8903d9615804167759"
) {
this.provider = asL2Provider(provider);
this.spokePool = OptimismSpokePool__factory.connect(spokePoolAddress, provider);
this.spokePool = SpokePool__factory.connect(spokePoolAddress, provider);
}

async getGasCosts(_tokenSymbol: string): Promise<BigNumberish> {
// Create a dummy transaction to estimate. Note: the simulated caller would need to be holding weth and have approved the contract.
const tx = await this.spokePool.populateTransaction.fillRelay(
"0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B",
"0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B",
this.usdcAddress,
"10",
"10",
"1",
"1",
"1",
"1",
"1"
);
const populatedTransaction = await new VoidSigner(this.simulatedRelayerAddress, this.provider).populateTransaction(
tx
);
return (await this.provider.estimateTotalGasCost(populatedTransaction)).toString();
const tx = await createUnsignedFillRelayTransaction(this.spokePool, this.usdcAddress, this.simulatedRelayerAddress);
return estimateTotalGasRequiredByUnsignedTransaction(tx, this.simulatedRelayerAddress, this.provider);
}

async getTokenPrice(tokenSymbol: string): Promise<number> {
Expand Down
28 changes: 19 additions & 9 deletions src/relayFeeCalculator/chain-queries/polygon.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
import { QueryInterface } from "../relayFeeCalculator";
import { BigNumberish } from "../../utils";
import { providers, BigNumber } from "ethers";
import { defaultAverageGas, SymbolMapping } from "./ethereum";
import {
BigNumberish,
createUnsignedFillRelayTransaction,
estimateTotalGasRequiredByUnsignedTransaction,
} from "../../utils";
import { providers } from "ethers";
import { SymbolMapping } from "./ethereum";
import { Coingecko } from "../../coingecko/Coingecko";
import { SpokePool__factory, SpokePool } from "@across-protocol/contracts-v2";

export class PolygonQueries implements QueryInterface {
private spokePool: SpokePool;

constructor(
readonly provider: providers.Provider,
public readonly averageGas = defaultAverageGas,
readonly symbolMapping = SymbolMapping
) {}
readonly symbolMapping = SymbolMapping,
readonly spokePoolAddress = "0x69B5c72837769eF1e7C164Abc6515DcFf217F920",
readonly usdcAddress = "0x2791bca1f2de4661ed88a30c99a7a9449aa84174",
readonly simulatedRelayerAddress = "0x893d0D70AD97717052E3AA8903D9615804167759"
) {
this.spokePool = SpokePool__factory.connect(spokePoolAddress, provider);
}

async getGasCosts(_tokenSymbol: string): Promise<BigNumberish> {
return BigNumber.from(await this.provider.getGasPrice())
.mul(this.averageGas)
.toString();
const tx = await createUnsignedFillRelayTransaction(this.spokePool, this.usdcAddress, this.simulatedRelayerAddress);
return estimateTotalGasRequiredByUnsignedTransaction(tx, this.simulatedRelayerAddress, this.provider);
}

async getTokenPrice(tokenSymbol: string): Promise<string | number> {
Expand Down
8 changes: 6 additions & 2 deletions src/relayFeeCalculator/chain-queries/queries.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// NODE_URL_42161
// NODE_URL_288
// NODE_URL_10
// NODE_URL_1
// NODE_URL_137
Comment on lines +5 to +6
Copy link
Contributor Author

@james-a-morris james-a-morris Aug 8, 2022

Choose a reason for hiding this comment

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

These two environment variables may need to be added to the e2e test on Git Actions.

Copy link
Member

Choose a reason for hiding this comment

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

That seems easy to do

Alternatively we could use the public infura key here but I do think setting env vars in github actions is more sustainable

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to try to add these to the github actions file in ./.github/?


import dotenv from "dotenv";

Expand Down Expand Up @@ -35,7 +37,8 @@ describe("Queries", function () {
]);
});
test("Ethereum", async function () {
const ethereumQueries = new EthereumQueries();
const provider = new providers.JsonRpcProvider(process.env.NODE_URL_1);
const ethereumQueries = new EthereumQueries(provider);
await Promise.all([
ethereumQueries.getGasCosts("USDC"),
Copy link
Member

Choose a reason for hiding this comment

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

Should we test that the return values are sensible values? For example, the gas costs are a wei value, decimals are 6, etc?

ethereumQueries.getTokenDecimals("USDC"),
Expand All @@ -52,7 +55,8 @@ describe("Queries", function () {
]);
});
test("Polygon", async function () {
const polygonQueries = new PolygonQueries();
const provider = new providers.JsonRpcProvider(process.env.NODE_URL_137);
const polygonQueries = new PolygonQueries(provider);
await Promise.all([
polygonQueries.getGasCosts("USDC"),
polygonQueries.getTokenDecimals("USDC"),
Expand Down
16 changes: 16 additions & 0 deletions src/relayFeeCalculator/relayFeeCalculator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,22 @@ describe("RelayFeeCalculator", () => {
beforeAll(() => {
queries = new ExampleQueries();
});
it("gasPercentageFee", async () => {
client = new RelayFeeCalculator({ queries });
// A list of inputs and ground truth [input, ground truth]
const gasFeePercents = [
[1000, "30557200000000000000000"],
[5000, "6111440000000000000000"],
// A test with a prime number
[104729, "291774007199534035462"],
];
for (const [input, truth] of gasFeePercents) {
const result = (await client.gasFeePercent(input, "usdc")).toString();
expect(result).toEqual(truth);
}
// Test that zero amount fails
await expect(client.gasFeePercent(0, "USDC")).rejects.toThrowError();
});
it("relayerFeeDetails", async () => {
client = new RelayFeeCalculator({ queries });
const result = await client.relayerFeeDetails(100000000, "usdc");
Expand Down
64 changes: 63 additions & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { BigNumber, ethers } from "ethers";
import { BigNumber, ethers, PopulatedTransaction, providers, VoidSigner } from "ethers";
import * as uma from "@uma/sdk";
import Decimal from "decimal.js";
import { isL2Provider as isOptimismL2Provider, L2Provider } from "@eth-optimism/sdk";
import { SpokePool } from "@across-protocol/contracts-v2";

export type BigNumberish = string | number | BigNumber;
export type BN = BigNumber;
Expand Down Expand Up @@ -222,3 +224,63 @@ export async function retry<T>(call: () => Promise<T>, times: number, delayS: nu
});
return promiseChain;
}

/**
* Estimates the total gas cost required to submit an unsigned (populated) transaction on-chain
* @param unsignedTx The unsigned transaction that this function will estimate
* @param senderAddress The address that the transaction will be submitted from
* @param provider A valid ethers provider - will be used to reason the gas price
* @param gasPrice A manually provided gas price - if set, this function will not resolve the current gas price
* @returns The total gas cost to submit this transaction - i.e. gasPrice * estimatedGasUnits
*/
export async function estimateTotalGasRequiredByUnsignedTransaction(
unsignedTx: PopulatedTransaction,
senderAddress: string,
provider: providers.Provider | L2Provider<providers.Provider>,
gasPrice?: BigNumberish
): Promise<BigNumberish> {
const voidSigner = new VoidSigner(senderAddress, provider);
// This branches in the Optimism case because they use a special provider, called L2Provider, and special gas logic
// to compute gas costs on Optimism.
if (isOptimismL2Provider(provider)) {
const populatedTransaction = await voidSigner.populateTransaction(unsignedTx);
return (await provider.estimateTotalGasCost(populatedTransaction)).toString();
} else {
// Estimate the Gas units required to submit this transaction
const estimatedGasUnits = await voidSigner.estimateGas(unsignedTx);
// Provide a default gas price of the market rate if this condition has not been set
const resolvedGasPrice = gasPrice ?? (await provider.getGasPrice());
// Find the total gas cost by taking the product of the gas
// price & the estimated number of gas units needed
return BigNumber.from(resolvedGasPrice).mul(estimatedGasUnits).toString();
}
}

/**
* Create an unsigned transaction of a fillRelay contract call
* @param spokePool The specific spokepool that will populate this tx
* @param destinationTokenAddress A valid ERC20 token (system-wide default is UDSC)
* @param simulatedRelayerAddress The relayer address that relays this transaction
* @returns A populated (but unsigned) transaction that can be signed/sent or used for estimating gas costs
*/
export async function createUnsignedFillRelayTransaction(
mrice32 marked this conversation as resolved.
Show resolved Hide resolved
spokePool: SpokePool,
destinationTokenAddress: string,
simulatedRelayerAddress: string
): Promise<PopulatedTransaction> {
// Populate and return an unsigned tx as per the given spoke pool
// NOTE: 0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B is a dummy address
return await spokePool.populateTransaction.fillRelay(
"0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B",
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
"0xBb23Cd0210F878Ea4CcA50e9dC307fb0Ed65Cf6B",
destinationTokenAddress,
"10",
"10",
"1",
"1",
"1",
"1",
"1",
{ from: simulatedRelayerAddress }
);
}