From dea71556ccb101d805d354a1fbd94f81518e6cee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Thu, 8 Jul 2021 09:57:51 +0300 Subject: [PATCH] eth/gasprice, internal/ethapi, miner: minor feehistory fixes --- eth/gasprice/feehistory.go | 169 +++++++++++++++++--------------- eth/gasprice/feehistory_test.go | 5 +- internal/ethapi/api.go | 30 +++--- miner/worker.go | 4 +- 4 files changed, 108 insertions(+), 100 deletions(-) diff --git a/eth/gasprice/feehistory.go b/eth/gasprice/feehistory.go index 84ec8e0889..a14dd594be 100644 --- a/eth/gasprice/feehistory.go +++ b/eth/gasprice/feehistory.go @@ -19,6 +19,7 @@ package gasprice import ( "context" "errors" + "fmt" "math/big" "sort" "sync/atomic" @@ -30,11 +31,19 @@ import ( ) var ( - errInvalidPercentiles = errors.New("Invalid reward percentiles") - errRequestBeyondHead = errors.New("Request beyond head block") + errInvalidPercentile = errors.New("invalid reward percentile") + errRequestBeyondHead = errors.New("request beyond head block") ) -const maxBlockCount = 1024 // number of blocks retrievable with a single query +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 +) // blockFees represents a single block for processing type blockFees struct { @@ -124,23 +133,22 @@ 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, lastBlockNumber rpc.BlockNumber, blockCount, maxHistory int) (*types.Block, types.Receipts, rpc.BlockNumber, int, error) { +func (oracle *Oracle) resolveBlockRange(ctx context.Context, lastBlock rpc.BlockNumber, blocks, maxHistory int) (*types.Block, []*types.Receipt, rpc.BlockNumber, int, error) { var ( - headBlockNumber rpc.BlockNumber + headBlock rpc.BlockNumber pendingBlock *types.Block pendingReceipts types.Receipts ) - - // query either pending block or head header and set headBlockNumber - if lastBlockNumber == rpc.PendingBlockNumber { + // query either pending block or head header and set headBlock + if lastBlock == rpc.PendingBlockNumber { if pendingBlock, pendingReceipts = oracle.backend.PendingBlockAndReceipts(); pendingBlock != nil { - lastBlockNumber = rpc.BlockNumber(pendingBlock.NumberU64()) - headBlockNumber = lastBlockNumber - 1 + lastBlock = rpc.BlockNumber(pendingBlock.NumberU64()) + headBlock = lastBlock - 1 } else { // pending block not supported by backend, process until latest block - lastBlockNumber = rpc.LatestBlockNumber - blockCount-- - if blockCount == 0 { + lastBlock = rpc.LatestBlockNumber + blocks-- + if blocks == 0 { return nil, nil, 0, 0, nil } } @@ -148,32 +156,32 @@ func (oracle *Oracle) resolveBlockRange(ctx context.Context, lastBlockNumber rpc if pendingBlock == nil { // if pending block is not fetched then we retrieve the head header to get the head block number if latestHeader, err := oracle.backend.HeaderByNumber(ctx, rpc.LatestBlockNumber); err == nil { - headBlockNumber = rpc.BlockNumber(latestHeader.Number.Uint64()) + headBlock = rpc.BlockNumber(latestHeader.Number.Uint64()) } else { return nil, nil, 0, 0, err } } - if lastBlockNumber == rpc.LatestBlockNumber { - lastBlockNumber = headBlockNumber - } else if pendingBlock == nil && lastBlockNumber > headBlockNumber { - return nil, nil, 0, 0, errRequestBeyondHead + if lastBlock == rpc.LatestBlockNumber { + lastBlock = headBlock + } 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(headBlockNumber) - int64(maxHistory) - int64(lastBlockNumber) + int64(blockCount); tooOldCount > 0 { + 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(blockCount) > tooOldCount { - blockCount -= int(tooOldCount) + if int64(blocks) > tooOldCount { + blocks -= int(tooOldCount) } else { return nil, nil, 0, 0, nil } } } // ensure not trying to retrieve before genesis - if rpc.BlockNumber(blockCount) > lastBlockNumber+1 { - blockCount = int(lastBlockNumber + 1) + if rpc.BlockNumber(blocks) > lastBlock+1 { + blocks = int(lastBlock + 1) } - return pendingBlock, pendingReceipts, lastBlockNumber, blockCount, nil + return pendingBlock, pendingReceipts, lastBlock, blocks, nil } // FeeHistory returns data relevant for fee estimation based on the specified range of blocks. @@ -189,90 +197,89 @@ func (oracle *Oracle) resolveBlockRange(ctx context.Context, lastBlockNumber rpc // - gasUsedRatio: gasUsed/gasLimit in the given block // Note: baseFee includes the next block after the newest of the returned range, because this // value can be derived from the newest block. -func (oracle *Oracle) FeeHistory(ctx context.Context, blockCount int, lastBlockNumber rpc.BlockNumber, rewardPercentiles []float64) (firstBlockNumber rpc.BlockNumber, reward [][]*big.Int, baseFee []*big.Int, gasUsedRatio []float64, err error) { - if blockCount < 1 { - // returning with no data and no error means there are no retrievable blocks - return +func (oracle *Oracle) FeeHistory(ctx context.Context, blocks int, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (rpc.BlockNumber, [][]*big.Int, []*big.Int, []float64, error) { + if blocks < 1 { + return 0, nil, nil, nil, nil // returning with no data and no error means there are no retrievable blocks } - if blockCount > maxBlockCount { - blockCount = maxBlockCount + if blocks > maxFeeHistory { + log.Warn("Sanitizing fee history length", "requested", blocks, "truncated", maxFeeHistory) + blocks = maxFeeHistory } for i, p := range rewardPercentiles { - if p < 0 || p > 100 || (i > 0 && p < rewardPercentiles[i-1]) { - return 0, nil, nil, nil, errInvalidPercentiles + if p < 0 || p > 100 { + return 0, nil, nil, nil, fmt.Errorf("%w: %f", errInvalidPercentile, p) + } + if i > 0 && p < rewardPercentiles[i-1] { + return 0, nil, nil, nil, fmt.Errorf("%w: #%d:%f > #%d:%f", errInvalidPercentile, i-1, rewardPercentiles[i-1], i, p) } } - - processBlocks := len(rewardPercentiles) != 0 - // limit retrieval to maxHistory if set - var maxHistory int - if processBlocks { + // Only process blocks if reward percentiles were requested + maxHistory := oracle.maxHeaderHistory + if len(rewardPercentiles) != 0 { maxHistory = oracle.maxBlockHistory - } else { - maxHistory = oracle.maxHeaderHistory } - var ( pendingBlock *types.Block - pendingReceipts types.Receipts + pendingReceipts []*types.Receipt + err error ) - if pendingBlock, pendingReceipts, lastBlockNumber, blockCount, err = oracle.resolveBlockRange(ctx, lastBlockNumber, blockCount, maxHistory); err != nil || blockCount == 0 { - return + pendingBlock, pendingReceipts, lastBlock, blocks, err = oracle.resolveBlockRange(ctx, lastBlock, blocks, maxHistory) + if err != nil || blocks == 0 { + return 0, nil, nil, nil, err } - firstBlockNumber = lastBlockNumber + 1 - rpc.BlockNumber(blockCount) + oldestBlock := lastBlock + 1 - rpc.BlockNumber(blocks) - processNext := int64(firstBlockNumber) - resultCh := make(chan *blockFees, blockCount) - threadCount := 4 - if blockCount < threadCount { - threadCount = blockCount - } - for i := 0; i < threadCount; i++ { + var ( + next = int64(oldestBlock) + results = make(chan *blockFees, blocks) + ) + for i := 0; i < maxBlockFetchers && i < blocks; i++ { go func() { for { - blockNumber := rpc.BlockNumber(atomic.AddInt64(&processNext, 1) - 1) - if blockNumber > lastBlockNumber { + // Retrieve the next block number to fetch with this goroutine + blockNumber := rpc.BlockNumber(atomic.AddInt64(&next, 1) - 1) + if blockNumber > lastBlock { return } - bf := &blockFees{blockNumber: blockNumber} + fees := &blockFees{blockNumber: blockNumber} if pendingBlock != nil && blockNumber >= rpc.BlockNumber(pendingBlock.NumberU64()) { - bf.block, bf.receipts = pendingBlock, pendingReceipts + fees.block, fees.receipts = pendingBlock, pendingReceipts } else { - if processBlocks { - bf.block, bf.err = oracle.backend.BlockByNumber(ctx, blockNumber) - if bf.block != nil { - bf.receipts, bf.err = oracle.backend.GetReceipts(ctx, bf.block.Hash()) + if len(rewardPercentiles) != 0 { + fees.block, fees.err = oracle.backend.BlockByNumber(ctx, blockNumber) + if fees.block != nil && fees.err == nil { + fees.receipts, fees.err = oracle.backend.GetReceipts(ctx, fees.block.Hash()) } } else { - bf.header, bf.err = oracle.backend.HeaderByNumber(ctx, blockNumber) + fees.header, fees.err = oracle.backend.HeaderByNumber(ctx, blockNumber) } } - if bf.block != nil { - bf.header = bf.block.Header() + if fees.block != nil { + fees.header = fees.block.Header() } - if bf.header != nil { - oracle.processBlock(bf, rewardPercentiles) + if fees.header != nil { + oracle.processBlock(fees, rewardPercentiles) } - // send to resultCh even if empty to guarantee that blockCount items are sent in total - resultCh <- bf + // send to results even if empty to guarantee that blocks items are sent in total + results <- fees } }() } - - reward = make([][]*big.Int, blockCount) - baseFee = make([]*big.Int, blockCount+1) - gasUsedRatio = make([]float64, blockCount) - firstMissing := blockCount - - for ; blockCount > 0; blockCount-- { - bf := <-resultCh - if bf.err != nil { - return 0, nil, nil, nil, bf.err + var ( + reward = make([][]*big.Int, blocks) + baseFee = make([]*big.Int, blocks+1) + gasUsedRatio = make([]float64, blocks) + firstMissing = blocks + ) + for ; blocks > 0; blocks-- { + fees := <-results + if fees.err != nil { + return 0, nil, nil, nil, fees.err } - i := int(bf.blockNumber - firstBlockNumber) - if bf.header != nil { - reward[i], baseFee[i], baseFee[i+1], gasUsedRatio[i] = bf.reward, bf.baseFee, bf.nextBaseFee, bf.gasUsedRatio + i := int(fees.blockNumber - oldestBlock) + if fees.header != 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) if i < firstMissing { @@ -283,11 +290,11 @@ func (oracle *Oracle) FeeHistory(ctx context.Context, blockCount int, lastBlockN if firstMissing == 0 { return 0, nil, nil, nil, nil } - if processBlocks { + if len(rewardPercentiles) != 0 { reward = reward[:firstMissing] } else { reward = nil } baseFee, gasUsedRatio = baseFee[:firstMissing+1], gasUsedRatio[:firstMissing] - return + return oldestBlock, reward, baseFee, gasUsedRatio, nil } diff --git a/eth/gasprice/feehistory_test.go b/eth/gasprice/feehistory_test.go index 191e2f0ce6..57cfb260c4 100644 --- a/eth/gasprice/feehistory_test.go +++ b/eth/gasprice/feehistory_test.go @@ -18,6 +18,7 @@ package gasprice import ( "context" + "errors" "math/big" "testing" @@ -37,7 +38,7 @@ func TestFeeHistory(t *testing.T) { }{ {false, 0, 0, 10, 30, nil, 21, 10, nil}, {false, 0, 0, 10, 30, []float64{0, 10}, 21, 10, nil}, - {false, 0, 0, 10, 30, []float64{20, 10}, 0, 0, errInvalidPercentiles}, + {false, 0, 0, 10, 30, []float64{20, 10}, 0, 0, errInvalidPercentile}, {false, 0, 0, 1000000000, 30, nil, 0, 31, nil}, {false, 0, 0, 1000000000, rpc.LatestBlockNumber, nil, 0, 33, nil}, {false, 0, 0, 10, 40, nil, 0, 0, errRequestBeyondHead}, @@ -81,7 +82,7 @@ func TestFeeHistory(t *testing.T) { if len(ratio) != c.expCount { t.Fatalf("Test case %d: gasUsedRatio array length mismatch, want %d, got %d", i, c.expCount, len(ratio)) } - if err != c.expErr { + if err != c.expErr && !errors.Is(err, c.expErr) { t.Fatalf("Test case %d: error mismatch, want %v, got %v", i, c.expErr, err) } } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index becfc2f5b0..da779a9eee 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -80,28 +80,28 @@ func (s *PublicEthereumAPI) MaxPriorityFeePerGas(ctx context.Context) (*hexutil. return (*hexutil.Big)(tipcap), err } -type feeHistoryResults struct { - FirstBlock rpc.BlockNumber - Reward [][]*hexutil.Big - BaseFee []*hexutil.Big - GasUsedRatio []float64 +type feeHistoryResult struct { + OldestBlock rpc.BlockNumber `json:"oldestBlock"` + Reward [][]*hexutil.Big `json:"reward,omitempty"` + BaseFee []*hexutil.Big `json:"baseFeePerGas,omitempty"` + GasUsedRatio []float64 `json:"gasUsedRatio"` } -func (s *PublicEthereumAPI) FeeHistory(ctx context.Context, blockCount int, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (feeHistoryResults, error) { - firstBlock, reward, baseFee, gasUsedRatio, err := s.b.FeeHistory(ctx, blockCount, lastBlock, rewardPercentiles) +func (s *PublicEthereumAPI) FeeHistory(ctx context.Context, blockCount int, lastBlock rpc.BlockNumber, rewardPercentiles []float64) (*feeHistoryResult, error) { + oldest, reward, baseFee, gasUsed, err := s.b.FeeHistory(ctx, blockCount, lastBlock, rewardPercentiles) if err != nil { - return feeHistoryResults{}, err + return nil, err } - results := feeHistoryResults{ - FirstBlock: firstBlock, - GasUsedRatio: gasUsedRatio, + results := &feeHistoryResult{ + OldestBlock: oldest, + GasUsedRatio: gasUsed, } if reward != nil { results.Reward = make([][]*hexutil.Big, len(reward)) - for j, w := range reward { - results.Reward[j] = make([]*hexutil.Big, len(w)) - for i, v := range w { - results.Reward[j][i] = (*hexutil.Big)(v) + for i, w := range reward { + results.Reward[i] = make([]*hexutil.Big, len(w)) + for j, v := range w { + results.Reward[i][j] = (*hexutil.Big)(v) } } } diff --git a/miner/worker.go b/miner/worker.go index 457f329153..accf3dac90 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -162,7 +162,7 @@ type worker struct { pendingMu sync.RWMutex pendingTasks map[common.Hash]*task - snapshotMu sync.RWMutex // The lock used to protect the block snapshot and state snapshot + snapshotMu sync.RWMutex // The lock used to protect the snapshots below snapshotBlock *types.Block snapshotReceipts types.Receipts snapshotState *state.StateDB @@ -745,7 +745,7 @@ func (w *worker) updateSnapshot() { w.current.receipts, trie.NewStackTrie(nil), ) - w.snapshotReceipts = w.current.receipts + w.snapshotReceipts = copyReceipts(w.current.receipts) w.snapshotState = w.current.state.Copy() }