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

eth/gasprice: feeHistory improvements #23422

Merged
merged 5 commits into from Aug 23, 2021
Merged

Conversation

zsfelfoldi
Copy link
Contributor

This PR changes the block count limitation rules in light client mode so that only the length of the queried section is limited per request but querying older sections of the chain is not limited. The feeHistory spec only mandates providing access to the most recent section because some clients might not be able to serve older data but the Geth light client is capable of doing so and therefore the limitations can be eased, allowing fee estimators to examine/sample a longer history if necessary.
The processed results are also cached per block now which greatly improves the performance of the calibration process of the Economical Fee Oracle.

@zsfelfoldi zsfelfoldi added this to the 1.10.8 milestone Aug 19, 2021
@@ -54,10 +52,15 @@ type blockFees struct {
block *types.Block // only set if reward percentiles are requested
receipts types.Receipts
// filled by processBlock
processedFees
Copy link
Member

Choose a reason for hiding this comment

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

Can we separate the "result" out from the blockFees struct? It's not so nice to have the embedded struct(result).

Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

Nitpicks, otherwise lgtm.

}
}
if fees.block != nil {
fees.header = fees.block.Header()
}
if fees.header != nil {
oracle.processBlock(fees, rewardPercentiles)
if fees.err == nil && !pending {
Copy link
Member

Choose a reason for hiding this comment

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

nitpicks, it's not obvious that logic is using the cached result or process to get the result.
Can we send back the result by results <- fees immediately if we find the result in Cache?
So that the logic is more clear.

@zsfelfoldi zsfelfoldi merged commit f38abc5 into ethereum:master Aug 23, 2021
sidhujag pushed a commit to sidhujag/go-ethereum that referenced this pull request Aug 23, 2021
* eth/gasprice: cache feeHistory results

* eth/gasprice: changed feeHistory block count limitation

* eth/gasprice: do not use embedded struct in blockFees

* eth/gasprice: fee processing logic cleanup

* eth/gasprice: purge feeHistory cache at chain reorgs
reds pushed a commit to reds/go-ethereum that referenced this pull request Aug 28, 2021
* eth/gasprice: cache feeHistory results

* eth/gasprice: changed feeHistory block count limitation

* eth/gasprice: do not use embedded struct in blockFees

* eth/gasprice: fee processing logic cleanup

* eth/gasprice: purge feeHistory cache at chain reorgs
i-norden pushed a commit to cerc-io/go-ethereum that referenced this pull request Sep 10, 2021
* eth/gasprice: cache feeHistory results

* eth/gasprice: changed feeHistory block count limitation

* eth/gasprice: do not use embedded struct in blockFees

* eth/gasprice: fee processing logic cleanup

* eth/gasprice: purge feeHistory cache at chain reorgs
atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
* eth/gasprice: cache feeHistory results

* eth/gasprice: changed feeHistory block count limitation

* eth/gasprice: do not use embedded struct in blockFees

* eth/gasprice: fee processing logic cleanup

* eth/gasprice: purge feeHistory cache at chain reorgs
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

2 participants