Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable gas estimator based on alloy. There is a logic error in the implementation of the new estimator in configurable_alloy.rs. The base fee is calculated using the oldest block in the fee history range instead of the latest, which could lead to inaccurate gas price estimations. A specific fix is suggested in the review comments.
MartinquaXD
left a comment
There was a problem hiding this comment.
Change looks good overall. Just some comments how to integrate it into the existing code base.
| #[instrument(skip(self), fields( | ||
| past_blocks = %self.config.past_blocks, | ||
| reward_percentile = %self.config.reward_percentile | ||
| ))] |
There was a problem hiding this comment.
The fields only get printed when a log inside the instrumented function actually emits a log, right?
If they would always get logged I don't think we should have this as this will never change during the runtime of the process and will result in tons of logs.
There was a problem hiding this comment.
The fields only get printed when a log inside the instrumented function actually emits a log, right?
Yes.
These are also attached the spans, so you can see it in Tempo.
Description
Adds the ability to configure past_blocks and reward_percentile parameters for the EIP-1559 gas price estimator. Previously, alloy's hardcoded defaults (10 blocks, 20th percentile) were always used.
Changes
[x] Add configurable_alloy.rs - a gas estimator that calls eth_feeHistory with custom parameters, then uses alloy's default estimation algorithm
[x] Extend GasEstimatorType::Alloy config variant with optional past-blocks and reward-percentile fields
[x] Default values match alloy's hardcoded constants (10 blocks, 20.0 percentile) for backwards compatibility