Skip to content

allow gas tip estimate to drop when blocks are mostly empty #68

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.

Sign up for GitHub

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

Closed
roberto-bayardo wants to merge 1 commit into from
Closed

allow gas tip estimate to drop when blocks are mostly empty #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eth/gasprice/feehistory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
67 changes: 41 additions & 26 deletions eth/gasprice/gasprice.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
}
}
}
Expand Down
24 changes: 22 additions & 2 deletions eth/gasprice/gasprice_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
}
}
40 changes: 22 additions & 18 deletions ethclient/ethclient_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down