diff --git a/eth/gasprice/feehistory_test.go b/eth/gasprice/feehistory_test.go index b54874d688..746596cec3 100644 --- a/eth/gasprice/feehistory_test.go +++ b/eth/gasprice/feehistory_test.go @@ -58,7 +58,7 @@ func TestFeeHistory(t *testing.T) { MaxHeaderHistory: c.maxHeader, MaxBlockHistory: c.maxBlock, } - backend := newTestBackend(t, big.NewInt(16), c.pending) + backend := newTestBackend(t, big.NewInt(16), c.pending, false) oracle := NewOracle(backend, config) first, reward, baseFee, ratio, err := oracle.FeeHistory(context.Background(), c.count, c.last, c.percent) diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index 604ad5e104..387fa0298a 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -188,10 +188,9 @@ func (oracle *Oracle) SuggestTipCap(ctx context.Context) (*big.Int, error) { return new(big.Int).Set(lastPrice), res.err } exp-- - // Nothing returned. There are two special cases here: - // - The block is empty - // - All the transactions included are sent by the miner itself. - // In these cases, use the latest calculated price for sampling. + // Nothing returned, which means all the transactions included are sent + // by the miner itself. In this case, use the latest calculated price + // for sampling. if len(res.values) == 0 { res.values = []*big.Int{lastPrice} } @@ -244,17 +243,19 @@ func (s *txSorter) Swap(i, j int) { s.txs[i], s.txs[j] = s.txs[j], s.txs[i] } func (s *txSorter) Less(i, j int) bool { - // It's okay to discard the error because a tx would never be - // accepted into a block with an invalid effective tip. - tip1, _ := s.txs[i].EffectiveGasTip(s.baseFee) - tip2, _ := s.txs[j].EffectiveGasTip(s.baseFee) + tip1 := s.txs[i].EffectiveGasTipValue(s.baseFee) + tip2 := s.txs[j].EffectiveGasTipValue(s.baseFee) return tip1.Cmp(tip2) < 0 } -// getBlockPrices calculates the lowest transaction gas price in a given block -// and sends it to the result channel. If the block is empty or all transactions -// are sent by the miner itself(it doesn't make any sense to include this kind of -// transaction prices for sampling), nil gasprice is returned. +// getBlockValues calculates the lowest 'limit' transaction gas prices in a +// given block and sends it to the result channel. If all transactions are sent +// by the miner itself(it doesn't make any sense to include this kind of +// transaction prices for sampling), nil gasprice is returned. If the block is +// mostly empty (defined as using less than half the gas target), then the +// returned array is padded with ignoreUnder values up to the requested limit +// instead of returning prices from the transactions, since in these cases the +// tx tip is not predictive of inclusion. func (oracle *Oracle) getBlockValues(ctx context.Context, signer types.Signer, blockNum uint64, limit int, ignoreUnder *big.Int, result chan results, quit chan struct{}) { block, err := oracle.backend.BlockByNumber(ctx, rpc.BlockNumber(blockNum)) if block == nil { @@ -264,23 +265,37 @@ func (oracle *Oracle) getBlockValues(ctx context.Context, signer types.Signer, b } return } - // Sort the transaction by effective tip in ascending sort. - txs := make([]*types.Transaction, len(block.Transactions())) - copy(txs, block.Transactions()) - sorter := newSorter(txs, block.BaseFee()) - sort.Sort(sorter) + gasTarget := block.GasLimit() + if oracle.backend.ChainConfig().IsLondon(block.Number()) { + gasTarget /= oracle.backend.ChainConfig().ElasticityMultiplier() + } var prices []*big.Int - for _, tx := range sorter.txs { - tip, _ := tx.EffectiveGasTip(block.BaseFee()) - if ignoreUnder != nil && tip.Cmp(ignoreUnder) == -1 { - continue + if block.GasUsed() < gasTarget/2 { + // If the block uses less gas than half the target, we consider it + // "mostly empty" and return only the minimum allowed estimate. + prices = make([]*big.Int, limit) + for i := range prices { + prices[i] = ignoreUnder } - sender, err := types.Sender(signer, tx) - if err == nil && sender != block.Coinbase() { - prices = append(prices, tip) - if len(prices) >= limit { - break + } else { + // Sort the transaction by effective tip in ascending sort. + txs := make([]*types.Transaction, len(block.Transactions())) + copy(txs, block.Transactions()) + sorter := newSorter(txs, block.BaseFee()) + sort.Sort(sorter) + + for _, tx := range sorter.txs { + tip := tx.EffectiveGasTipValue(block.BaseFee()) + if ignoreUnder != nil && tip.Cmp(ignoreUnder) == -1 { + continue + } + sender, err := types.Sender(signer, tx) + if err == nil && sender != block.Coinbase() { + prices = append(prices, tip) + if len(prices) >= limit { + break + } } } } diff --git a/eth/gasprice/gasprice_test.go b/eth/gasprice/gasprice_test.go index 4ee5a0d1b2..e76bc190b5 100644 --- a/eth/gasprice/gasprice_test.go +++ b/eth/gasprice/gasprice_test.go @@ -119,7 +119,7 @@ func (b *testBackend) teardown() { // newTestBackend creates a test backend. OBS: don't forget to invoke tearDown // after use, otherwise the blockchain instance will mem-leak via goroutines. -func newTestBackend(t *testing.T, londonBlock *big.Int, pending bool) *testBackend { +func newTestBackend(t *testing.T, londonBlock *big.Int, pending bool, fullBlocks bool) *testBackend { var ( key, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") addr = crypto.PubkeyToAddress(key.PublicKey) @@ -130,6 +130,10 @@ func newTestBackend(t *testing.T, londonBlock *big.Int, pending bool) *testBacke } signer = types.LatestSigner(gspec.Config) ) + if fullBlocks { + // this prevents blocks from seeming empty to test fee estimation + gspec.GasLimit = 30000 + } config.LondonBlock = londonBlock config.ArrowGlacierBlock = londonBlock config.GrayGlacierBlock = londonBlock @@ -199,7 +203,7 @@ func TestSuggestTipCap(t *testing.T) { {big.NewInt(33), big.NewInt(params.GWei * int64(30))}, // Fork point in the future } for _, c := range cases { - backend := newTestBackend(t, c.fork, false) + backend := newTestBackend(t, c.fork, false, true) oracle := NewOracle(backend, config) // The gas price sampled is: 32G, 31G, 30G, 29G, 28G, 27G @@ -212,4 +216,20 @@ func TestSuggestTipCap(t *testing.T) { t.Fatalf("Gas price mismatch, want %d, got %d", c.expect, got) } } + + // Run the tests again, only configured for mostly empty blocks + for _, c := range cases { + backend := newTestBackend(t, c.fork, false, false) + oracle := NewOracle(backend, config) + + // The gas prices sampled will all be DefaultIgnorePrice when blocks are mostly empty + got, err := oracle.SuggestTipCap(context.Background()) + backend.teardown() + if err != nil { + t.Fatalf("Failed to retrieve recommended gas price: %v", err) + } + if got.Cmp(DefaultIgnorePrice) != 0 { + t.Fatalf("Gas price mismatch, want %d, got %d", DefaultIgnorePrice, got) + } + } } diff --git a/ethclient/ethclient_test.go b/ethclient/ethclient_test.go index c22446f6f3..2282e4e285 100644 --- a/ethclient/ethclient_test.go +++ b/ethclient/ethclient_test.go @@ -39,6 +39,7 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/ethconfig" + "github.com/ethereum/go-ethereum/eth/gasprice" "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" @@ -582,24 +583,6 @@ func testStatusFunctions(t *testing.T, client *rpc.Client) { t.Fatalf("unexpected networkID: %v", networkID) } - // SuggestGasPrice - gasPrice, err := ec.SuggestGasPrice(context.Background()) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if gasPrice.Cmp(big.NewInt(1000000000)) != 0 { - t.Fatalf("unexpected gas price: %v", gasPrice) - } - - // SuggestGasTipCap - gasTipCap, err := ec.SuggestGasTipCap(context.Background()) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if gasTipCap.Cmp(big.NewInt(234375000)) != 0 { - t.Fatalf("unexpected gas tip cap: %v", gasTipCap) - } - // FeeHistory history, err := ec.FeeHistory(context.Background(), 1, big.NewInt(2), []float64{95, 99}) if err != nil { @@ -622,6 +605,27 @@ func testStatusFunctions(t *testing.T, client *rpc.Client) { if !reflect.DeepEqual(history, want) { t.Fatalf("FeeHistory result doesn't match expected: (got: %v, want: %v)", history, want) } + + // SuggestGasTipCap + gasTipCap, err := ec.SuggestGasTipCap(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Since these blocks are mostly empty, the suggested tip should be the minimum value + if gasTipCap.Cmp(gasprice.DefaultIgnorePrice) != 0 { + t.Fatalf("unexpected gas tip cap: %v", gasTipCap) + } + + // SuggestGasPrice + gasPrice, err := ec.SuggestGasPrice(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Suggested gas price should be the base fee plus suggested tip cap + if gasPrice.Cmp(gasTipCap.Add(gasTipCap, history.BaseFee[0])) != 0 { + t.Fatalf("unexpected gas price: %v", gasPrice) + } + } func testCallContractAtHash(t *testing.T, client *rpc.Client) {