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
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
0c4a098
eth/gasprice: cache feeHistory results
zsfelfoldi aebca7f
eth/gasprice: changed feeHistory block count limitation
zsfelfoldi ac4e010
eth/gasprice: do not use embedded struct in blockFees
zsfelfoldi 77d6825
eth/gasprice: fee processing logic cleanup
zsfelfoldi 27ef400
eth/gasprice: purge feeHistory cache at chain reorgs
zsfelfoldi File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,8 +18,10 @@ package gasprice | |
|
||
import ( | ||
"context" | ||
"encoding/binary" | ||
"errors" | ||
"fmt" | ||
"math" | ||
"math/big" | ||
"sort" | ||
"sync/atomic" | ||
|
@@ -37,10 +39,6 @@ var ( | |
) | ||
|
||
const ( | ||
// maxFeeHistory is the maximum number of blocks that can be retrieved for a | ||
// fee history request. | ||
maxFeeHistory = 1024 | ||
|
||
// maxBlockFetchers is the max number of goroutines to spin up to pull blocks | ||
// for the fee history calculation (mostly relevant for LES). | ||
maxBlockFetchers = 4 | ||
|
@@ -54,10 +52,15 @@ type blockFees struct { | |
block *types.Block // only set if reward percentiles are requested | ||
receipts types.Receipts | ||
// filled by processBlock | ||
processedFees | ||
err error | ||
} | ||
|
||
// processedFees contains the results of a processed block and is also used for caching | ||
type processedFees struct { | ||
reward []*big.Int | ||
baseFee, nextBaseFee *big.Int | ||
gasUsedRatio float64 | ||
err error | ||
} | ||
|
||
// txGasAndReward is sorted in ascending order based on reward | ||
|
@@ -134,7 +137,7 @@ func (oracle *Oracle) processBlock(bf *blockFees, percentiles []float64) { | |
// also returned if requested and available. | ||
// Note: an error is only returned if retrieving the head header has failed. If there are no | ||
// retrievable blocks in the specified range then zero block count is returned with no error. | ||
func (oracle *Oracle) resolveBlockRange(ctx context.Context, lastBlock rpc.BlockNumber, blocks, maxHistory int) (*types.Block, []*types.Receipt, uint64, int, error) { | ||
func (oracle *Oracle) resolveBlockRange(ctx context.Context, lastBlock rpc.BlockNumber, blocks int) (*types.Block, []*types.Receipt, uint64, int, error) { | ||
var ( | ||
headBlock rpc.BlockNumber | ||
pendingBlock *types.Block | ||
|
@@ -167,17 +170,6 @@ func (oracle *Oracle) resolveBlockRange(ctx context.Context, lastBlock rpc.Block | |
} else if pendingBlock == nil && lastBlock > headBlock { | ||
return nil, nil, 0, 0, fmt.Errorf("%w: requested %d, head %d", errRequestBeyondHead, lastBlock, headBlock) | ||
} | ||
if maxHistory != 0 { | ||
// limit retrieval to the given number of latest blocks | ||
if tooOldCount := int64(headBlock) - int64(maxHistory) - int64(lastBlock) + int64(blocks); tooOldCount > 0 { | ||
// tooOldCount is the number of requested blocks that are too old to be served | ||
if int64(blocks) > tooOldCount { | ||
blocks -= int(tooOldCount) | ||
} else { | ||
return nil, nil, 0, 0, nil | ||
} | ||
} | ||
} | ||
// ensure not trying to retrieve before genesis | ||
if rpc.BlockNumber(blocks) > lastBlock+1 { | ||
blocks = int(lastBlock + 1) | ||
|
@@ -202,6 +194,10 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast | |
if blocks < 1 { | ||
return common.Big0, nil, nil, nil, nil // returning with no data and no error means there are no retrievable blocks | ||
} | ||
maxFeeHistory := oracle.maxHeaderHistory | ||
if len(rewardPercentiles) != 0 { | ||
maxFeeHistory = oracle.maxBlockHistory | ||
} | ||
if blocks > maxFeeHistory { | ||
log.Warn("Sanitizing fee history length", "requested", blocks, "truncated", maxFeeHistory) | ||
blocks = maxFeeHistory | ||
|
@@ -214,17 +210,12 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast | |
return common.Big0, nil, nil, nil, fmt.Errorf("%w: #%d:%f > #%d:%f", errInvalidPercentile, i-1, rewardPercentiles[i-1], i, p) | ||
} | ||
} | ||
// Only process blocks if reward percentiles were requested | ||
maxHistory := oracle.maxHeaderHistory | ||
if len(rewardPercentiles) != 0 { | ||
maxHistory = oracle.maxBlockHistory | ||
} | ||
var ( | ||
pendingBlock *types.Block | ||
pendingReceipts []*types.Receipt | ||
err error | ||
) | ||
pendingBlock, pendingReceipts, lastBlock, blocks, err := oracle.resolveBlockRange(ctx, unresolvedLastBlock, blocks, maxHistory) | ||
pendingBlock, pendingReceipts, lastBlock, blocks, err := oracle.resolveBlockRange(ctx, unresolvedLastBlock, blocks) | ||
if err != nil || blocks == 0 { | ||
return common.Big0, nil, nil, nil, err | ||
} | ||
|
@@ -234,6 +225,10 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast | |
next = oldestBlock | ||
results = make(chan *blockFees, blocks) | ||
) | ||
percentileKey := make([]byte, 8*len(rewardPercentiles)) | ||
for i, p := range rewardPercentiles { | ||
binary.LittleEndian.PutUint64(percentileKey[i*8:(i+1)*8], math.Float64bits(p)) | ||
} | ||
for i := 0; i < maxBlockFetchers && i < blocks; i++ { | ||
go func() { | ||
for { | ||
|
@@ -243,24 +238,37 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast | |
return | ||
} | ||
|
||
var pending bool | ||
cacheKey := struct { | ||
number uint64 | ||
percentiles string | ||
}{blockNumber, string(percentileKey)} | ||
fees := &blockFees{blockNumber: blockNumber} | ||
if pendingBlock != nil && blockNumber >= pendingBlock.NumberU64() { | ||
fees.block, fees.receipts = pendingBlock, pendingReceipts | ||
pending = true | ||
} else { | ||
if len(rewardPercentiles) != 0 { | ||
fees.block, fees.err = oracle.backend.BlockByNumber(ctx, rpc.BlockNumber(blockNumber)) | ||
if fees.block != nil && fees.err == nil { | ||
fees.receipts, fees.err = oracle.backend.GetReceipts(ctx, fees.block.Hash()) | ||
} | ||
if p, ok := oracle.historyCache.Get(cacheKey); ok { | ||
fees.processedFees = p.(processedFees) | ||
} else { | ||
fees.header, fees.err = oracle.backend.HeaderByNumber(ctx, rpc.BlockNumber(blockNumber)) | ||
if len(rewardPercentiles) != 0 { | ||
fees.block, fees.err = oracle.backend.BlockByNumber(ctx, rpc.BlockNumber(blockNumber)) | ||
if fees.block != nil && fees.err == nil { | ||
fees.receipts, fees.err = oracle.backend.GetReceipts(ctx, fees.block.Hash()) | ||
} | ||
} else { | ||
fees.header, fees.err = oracle.backend.HeaderByNumber(ctx, rpc.BlockNumber(blockNumber)) | ||
} | ||
} | ||
} | ||
if fees.block != nil { | ||
fees.header = fees.block.Header() | ||
} | ||
if fees.header != nil { | ||
oracle.processBlock(fees, rewardPercentiles) | ||
if fees.err == nil && !pending { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
oracle.historyCache.Add(cacheKey, fees.processedFees) | ||
} | ||
} | ||
// send to results even if empty to guarantee that blocks items are sent in total | ||
results <- fees | ||
|
@@ -279,7 +287,7 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, unresolvedLast | |
return common.Big0, nil, nil, nil, fees.err | ||
} | ||
i := int(fees.blockNumber - oldestBlock) | ||
if fees.header != nil { | ||
if fees.baseFee != nil { | ||
reward[i], baseFee[i], baseFee[i+1], gasUsedRatio[i] = fees.reward, fees.baseFee, fees.nextBaseFee, fees.gasUsedRatio | ||
} else { | ||
// getting no block and no error means we are requesting into the future (might happen because of a reorg) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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).