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

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

Merged
merged 5 commits into from Jan 10, 2024

Conversation

alexandre-abrioux
Copy link
Member

Description of the changes

This PR introduces two new options to the EthereumTransactionSubmitter:

  • gasPriceMax: this parameter can be used to protect against gas price spikes and adds an upper threshold
  • gasPriceMultiplier: this parameter can be used to add a margin to the computed gas fee

@coveralls
Copy link

coveralls commented Jan 9, 2024

Coverage Status

coverage: 87.128% (+0.04%) from 87.093%
when pulling 7ba25f3 on max-fee
into 8d0b368 on master.

Copy link
Contributor

@olivier7delf olivier7delf left a comment

Choose a reason for hiding this comment

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

very clear :)

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).

@alexandre-abrioux alexandre-abrioux enabled auto-merge (squash) January 10, 2024 15:54
@alexandre-abrioux alexandre-abrioux merged commit 98997ed into master Jan 10, 2024
25 checks passed
@alexandre-abrioux alexandre-abrioux deleted the max-fee branch January 10, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants