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

[TT-1601] More flexible gas estimation #1116

Merged

Conversation

Tofel
Copy link
Contributor

@Tofel Tofel commented Sep 3, 2024

Scope:

  • allow to disable congestion metrics, when using automated gas estimations, by setting gas_price_estimation_blocks to 0. Rationale? more flexibility, avoiding rate limiting on RPC endpoints until configuration that allows to limit call concurrency is introduced
  • change default gas price: 1 gwei -> 100 gwei to avoid stuck transactions in live networks on RPC health-check
  • fix how Stats work by calculating from block number instead of using block count; if gas_price_estimation_blocks is set to 0 use 100 blocks

Tests:

  • use live RPC with gas_price_estimation_blocks set to 0 and still get valid gas estimations

Below is a summarization created by an LLM (gpt-4-0125-preview). Be mindful of hallucinations and verify accuracy.

Why

The changes improve the gas price estimation logic and adjust the default gas price settings to better align with current network conditions. It also fine-tunes the behavior of gas estimation based on network congestion and provides clearer logging for developers.

What

  • README.md: Enabled gas price estimations by changing WithGasPriceEstimations from false to true. This change allows the client to automatically adjust gas prices based on network activity.
  • README.md: Clarified the behavior of gas price estimation during network congestion, including how congestion analysis is disabled if gas_price_estimation_blocks equals 0.
  • README.md: Updated the description for fee selection to use the greatest of node's suggested tip or the historical average tip, improving the accuracy of gas price estimations.
  • README.md: Specified that if gas_price_estimation_blocks is higher than 0, gas prices are adjusted based on congestion rate, enhancing the detail provided on how gas prices are modified.
  • client.go: Modified log message in ValidateConfig when gas estimation is enabled but block headers to use is set to 0. This change makes the log message more informative by explaining the impact on gas estimation.
  • config.go: Increased the default gas price from 1 Gwei to 100 Gwei, reflecting changes in network conditions to ensure transactions are competitive.
  • gas.go: Added logic to set a default starting block number for gas estimation if the current block number is greater than 100, ensuring there's a fallback mechanism for gas estimation.
  • gas_adjuster.go: Added checks to skip congestion metric calculations if GasPriceEstimationBlocks is 0, optimizing performance and clarifying when congestion adjustments apply.

@Tofel Tofel requested a review from skudasov September 3, 2024 08:52
@Tofel Tofel marked this pull request as ready for review September 3, 2024 08:52
@Tofel Tofel requested review from sebawo and a team as code owners September 3, 2024 08:52
@@ -25,6 +25,13 @@ func (m *GasEstimator) Stats(fromNumber uint64, priorityPerc float64) (GasSugges
if err != nil {
return GasSuggestions{}, err
}
if fromNumber == 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Implicit, should we add this to docs that we'll use the last 100 blocks if no fromNumber was specified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea, added

@Tofel Tofel force-pushed the tt_1601_allow_0_blocks_headers_for_gas_price_estimation branch from 1095ce2 to c51c0b8 Compare September 3, 2024 09:06
@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
12.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube

@Tofel Tofel merged commit 363c82b into main Sep 3, 2024
43 of 44 checks passed
@Tofel Tofel deleted the tt_1601_allow_0_blocks_headers_for_gas_price_estimation branch September 3, 2024 09:21
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.

2 participants