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

Jump to bottom

feat(ethereum-storage): gas fee multiplier & max gas fee #1318

Merged
merged 5 commits into from
Jan 10, 2024
Merged

feat(ethereum-storage): gas fee multiplier & max gas fee #1318

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
26b2527
feat(ethereum-storage): gas fee multiplier & max gas fee
alexandre-abrioux Jan 9, 2024
412b1c7
add comments
alexandre-abrioux Jan 10, 2024
acdbcdb
Merge branch 'master' into max-fee
alexandre-abrioux Jan 10, 2024
32498c0
add comments
alexandre-abrioux Jan 10, 2024
7ba25f3
Merge branch 'master' into max-fee
alexandre-abrioux Jan 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 18 additions & 2 deletions packages/ethereum-storage/src/ethereum-tx-submitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { SimpleLogger, isEip1559Supported } from '@requestnetwork/utils';
export type SubmitterProps = {
signer: Signer;
gasPriceMin?: BigNumber;
gasPriceMax?: BigNumber;
gasPriceMultiplier?: number;
alexandre-abrioux marked this conversation as resolved.
Show resolved Hide resolved
network: CurrencyTypes.EvmChainName;
logger?: LogTypes.ILogger;
debugProvider?: boolean;
Expand All @@ -23,15 +25,29 @@ export class EthereumTransactionSubmitter implements StorageTypes.ITransactionSu
private readonly provider: providers.JsonRpcProvider;
private readonly gasFeeDefiner: GasFeeDefiner;

constructor({ network, signer, logger, gasPriceMin, debugProvider }: SubmitterProps) {
constructor({
network,
signer,
logger,
gasPriceMin,
gasPriceMax,
gasPriceMultiplier,
debugProvider,
}: SubmitterProps) {
this.logger = logger || new SimpleLogger();
const provider = signer.provider as providers.JsonRpcProvider;
this.provider = provider;
this.hashSubmitter = requestHashSubmitterArtifact.connect(
network,
signer,
) as RequestOpenHashSubmitter; // type mismatch with ethers.
this.gasFeeDefiner = new GasFeeDefiner({ provider, gasPriceMin, logger: this.logger });
this.gasFeeDefiner = new GasFeeDefiner({
provider,
gasPriceMin,
gasPriceMax,
gasPriceMultiplier,
logger: this.logger,
});
if (debugProvider) {
this.provider.on('debug', (event) => {
this.logger.debug('JsonRpcProvider debug event', event);
Expand Down
16 changes: 13 additions & 3 deletions packages/ethereum-storage/src/gas-fee-definer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,41 @@
import { suggestFees } from '@rainbow-me/fee-suggestions';
import { BigNumber, providers, constants } from 'ethers';
import { BigNumber, providers } from 'ethers';
import { normalizeGasFees } from '@requestnetwork/utils';
import { FeeTypes, LogTypes } from '@requestnetwork/types';

export class GasFeeDefiner {
private readonly logger: LogTypes.ILogger;
private readonly provider: providers.JsonRpcProvider;
private readonly gasPriceMin: BigNumber;
private readonly gasPriceMin?: BigNumber;
private readonly gasPriceMax?: BigNumber;
private readonly gasPriceMultiplier?: number;

constructor({
logger,
provider,
gasPriceMin,
gasPriceMax,
gasPriceMultiplier,
}: {
logger: LogTypes.ILogger;
gasPriceMin?: BigNumber;
gasPriceMax?: BigNumber;
gasPriceMultiplier?: number;
provider: providers.JsonRpcProvider;
}) {
this.logger = logger;
this.provider = provider;
this.gasPriceMin = gasPriceMin || constants.Zero;
this.gasPriceMin = gasPriceMin;
this.gasPriceMax = gasPriceMax;
this.gasPriceMultiplier = gasPriceMultiplier;
}

public async getGasFees(): Promise<FeeTypes.EstimatedGasFees> {
return normalizeGasFees({
logger: this.logger,
gasPriceMin: this.gasPriceMin,
gasPriceMax: this.gasPriceMax,
gasPriceMultiplier: this.gasPriceMultiplier,
suggestFees: async () => {
const { baseFeeSuggestion, maxPriorityFeeSuggestions } = await suggestFees(this.provider);
return {
Expand Down
17 changes: 13 additions & 4 deletions packages/utils/src/normalize-gas-fees.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { BigNumber, constants } from 'ethers';

import { maxBigNumber } from './index';
import { maxBigNumber, minBigNumber } from './index';
import { LogTypes, FeeTypes } from '@requestnetwork/types';

/**
Expand All @@ -14,22 +14,31 @@ import { LogTypes, FeeTypes } from '@requestnetwork/types';
async function normalizeGasFees({
logger,
gasPriceMin,
gasPriceMax,
gasPriceMultiplier,
suggestFees,
}: {
logger: LogTypes.ILogger;
gasPriceMin?: BigNumber;
gasPriceMax?: BigNumber;
gasPriceMultiplier?: number;
suggestFees: () => Promise<FeeTypes.SuggestedFees>;
}): Promise<FeeTypes.EstimatedGasFees> {
try {
const suggestedFee = await suggestFees();

const baseFee = maxBigNumber(suggestedFee.baseFee, gasPriceMin || constants.Zero);

const maxPriorityFeePerGas = maxBigNumber(
suggestedFee.maxPriorityFee,
gasPriceMin || constants.Zero,
);
const maxFeePerGas = baseFee.add(maxPriorityFeePerGas);

const maxFeePerGasInit = baseFee
.add(maxPriorityFeePerGas)
.mul(gasPriceMultiplier || 100)
.div(100);
const maxFeePerGas = gasPriceMax
? minBigNumber(maxFeePerGasInit, gasPriceMax)
: maxFeePerGasInit;

if (maxPriorityFeePerGas.eq(0) || maxFeePerGas.eq(0)) {
logger.warn(
Expand Down
26 changes: 26 additions & 0 deletions packages/utils/test/normalize-gas-fees.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,30 @@ describe('Normalize Gas Fees', () => {
expect(normalizedGasFees.maxPriorityFeePerGas?.toString()).toBe('1000000000');
expect(normalizedGasFees.maxFeePerGas?.toString()).toBe('2000000000');
});

it('should respect maximum gas fee', async () => {
const normalizedGasFees = await normalizeGasFees({
logger: console,
suggestFees: async () => ({
baseFee: '400000000000', // 400 Gwei
maxPriorityFee: '2000000000', // 2 Gwei
}),
gasPriceMax: BigNumber.from('250000000000'), // 250 Gwei
});
expect(normalizedGasFees.maxPriorityFeePerGas?.toString()).toBe('2000000000'); // 2 Gwei
expect(normalizedGasFees.maxFeePerGas?.toString()).toBe('250000000000'); // 250 Gwei
});

it('should respect gas multiplier', async () => {
const normalizedGasFees = await normalizeGasFees({
logger: console,
suggestFees: async () => ({
baseFee: '20000000000', // 20 Gwei
maxPriorityFee: '2000000000', // 2 Gwei
}),
gasPriceMultiplier: 200, // x2
});
expect(normalizedGasFees.maxPriorityFeePerGas?.toString()).toBe('2000000000'); // 2 Gwei
expect(normalizedGasFees.maxFeePerGas?.toString()).toBe('44000000000'); // (20 + 2) x 2 = 44 Gwei
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a question, according to the formula:
Total Fee = Gas unit (limits)*(Base fee + Tip)

Could it be useful to have a parameter that multiplies only the Tip? Allowing to be better than other tips, but without a gasFees value too high.
In the test, gasFees => 20 + (2) x 2 = 24 Gwei

I might be lacking of context (google didn't help that much).

Copy link
Member Author

Choose a reason for hiding this comment

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

The formula for eip-1559 is maxFeePerGas = baseFeePerGas + maxPriorityFeePerGas, see https://docs.alchemy.com/docs/maxpriorityfeepergas-vs-maxfeepergas

The priority fee is the fee given to validators for including our transaction. The priority fee is set by us: we use the urgent value of the lib rainbow-me/fee-suggestions, which is set to a minimum of 2Gwei, see here. This signals the network that we are willing to pay for our transaction to be included ASAP. We have control over this value, this is not an issue.

However, we have no control over the base fee. The base fee is set by the network, and it is burned when the transaction is included. It is approximated by the lib rainbow-me/fee-suggestions, but between the time of approximation and the time of the transaction, the base fee can change a bit.

Because the base fee can change, the total will also change, which is the second value we control: we can set a cap for the total value with maxFeePerGas. However if maxFeePerGas is set too low and the base fee increases between approximation and inclusion, there might be a very low amount of gas remaining for validators to take their priority fee cut.

To fix this, I introduced a multiplier that acts on the total. We can virtually increase the total (hence the approximation) and this will give room for our transaction to be included. Note that this doesn't mean we will pay more gas in the end, because miners/validators cannot cheat on the amount of gas used (defined by the EVM) and the base fee (defined by the network on each block). They can only decide which cut of the priority fee they want to take (they usually take all of it).

});
});