Skip to content

Commit

Permalink
eth, miner: fix enforcing the minimum miner tip (#28933)
Browse files Browse the repository at this point in the history
* eth, miner: fix enforcing the minimum miner tip

* ethclient/simulated: fix failing test due the min tip change

* accounts/abi/bind: fix simulater gas tip issue
  • Loading branch information
karalabe authored and pratikspatil024 committed Apr 5, 2024
1 parent 694eadb commit 1460d0b
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 13 deletions.
3 changes: 2 additions & 1 deletion accounts/abi/bind/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/params"
)

var testKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
Expand Down Expand Up @@ -64,7 +65,7 @@ func TestWaitDeployed(t *testing.T) {

// Create the transaction
head, _ := backend.HeaderByNumber(context.Background(), nil) // Should be child's, good enough
gasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(1))
gasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(params.GWei))

tx := types.NewContractCreation(0, big.NewInt(0), test.gas, gasPrice, common.FromHex(test.code))
tx, _ = types.SignTx(tx, types.HomesteadSigner{}, testKey)
Expand Down
1 change: 1 addition & 0 deletions eth/api_miner.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func (api *MinerAPI) SetGasPrice(gasPrice hexutil.Big) bool {
api.e.lock.Unlock()

api.e.txPool.SetGasTip((*big.Int)(&gasPrice))
api.e.Miner().SetGasTip((*big.Int)(&gasPrice))

Check warning on line 67 in eth/api_miner.go

View check run for this annotation

Codecov / codecov/patch

eth/api_miner.go#L67

Added line #L67 was not covered by tests
return true
}

Expand Down
5 changes: 5 additions & 0 deletions miner/miner.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ func (miner *Miner) SetExtra(extra []byte) error {
return nil
}

func (miner *Miner) SetGasTip(tip *big.Int) error {
miner.worker.setGasTip(tip)
return nil

Check warning on line 224 in miner/miner.go

View check run for this annotation

Codecov / codecov/patch

miner/miner.go#L222-L224

Added lines #L222 - L224 were not covered by tests
}

// SetRecommitInterval sets the interval for sealing work resubmitting.
func (miner *Miner) SetRecommitInterval(interval time.Duration) {
miner.worker.setRecommitInterval(interval)
Expand Down
6 changes: 3 additions & 3 deletions miner/ordering.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,11 @@ func newTransactionsByPriceAndNonce(signer types.Signer, txs map[common.Address]
}

// Peek returns the next transaction by price.
func (t *transactionsByPriceAndNonce) Peek() *txpool.LazyTransaction {
func (t *transactionsByPriceAndNonce) Peek() (*txpool.LazyTransaction, *big.Int) {
if len(t.heads) == 0 {
return nil
return nil, nil
}
return t.heads[0].tx
return t.heads[0].tx, t.heads[0].fees
}

// Shift replaces the current best head with the next one from the same account.
Expand Down
4 changes: 2 additions & 2 deletions miner/ordering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func testTransactionPriceNonceSort(t *testing.T, baseFee *big.Int) {
txset := newTransactionsByPriceAndNonce(signer, groups, baseFee)

txs := types.Transactions{}
for tx := txset.Peek(); tx != nil; tx = txset.Peek() {
for tx, _ := txset.Peek(); tx != nil; tx, _ = txset.Peek() {
txs = append(txs, tx.Tx)
txset.Shift()
}
Expand Down Expand Up @@ -167,7 +167,7 @@ func TestTransactionTimeSort(t *testing.T) {
txset := newTransactionsByPriceAndNonce(signer, groups, nil)

txs := types.Transactions{}
for tx := txset.Peek(); tx != nil; tx = txset.Peek() {
for tx, _ := txset.Peek(); tx != nil; tx, _ = txset.Peek() {
txs = append(txs, tx.Tx)
txset.Shift()
}
Expand Down
5 changes: 3 additions & 2 deletions miner/test_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"math/big"
"os"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -175,7 +176,7 @@ func (w *worker) mainLoopWithDelay(ctx context.Context, delay uint, opcodeDelay
}
txset := newTransactionsByPriceAndNonce(w.current.signer, txs, w.current.header.BaseFee)
tcount := w.current.tcount
w.commitTransactions(w.current, txset, nil, context.Background())
w.commitTransactions(w.current, txset, nil, new(big.Int), context.Background())

Check warning on line 179 in miner/test_backend.go

View check run for this annotation

Codecov / codecov/patch

miner/test_backend.go#L179

Added line #L179 was not covered by tests

// Only update the snapshot if any new transactons were added
// to the pending block
Expand Down Expand Up @@ -587,7 +588,7 @@ mainloop:
break
}
// Retrieve the next transaction and abort if all done.
ltx := txs.Peek()
ltx, _ := txs.Peek()
if ltx == nil {
breakCause = "all transactions has been included"
break
Expand Down
28 changes: 23 additions & 5 deletions miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ type worker struct {
mu sync.RWMutex // The lock used to protect the coinbase and extra fields
coinbase common.Address
extra []byte
tip *big.Int // Minimum tip needed for non-local transaction to include them

pendingMu sync.RWMutex
pendingTasks map[common.Hash]*task
Expand Down Expand Up @@ -295,6 +296,7 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus
isLocalBlock: isLocalBlock,
coinbase: config.Etherbase,
extra: config.ExtraData,
tip: config.GasPrice,
pendingTasks: make(map[common.Hash]*task),
txsCh: make(chan core.NewTxsEvent, txChanSize),
chainHeadCh: make(chan core.ChainHeadEvent, chainHeadChanSize),
Expand Down Expand Up @@ -395,6 +397,13 @@ func (w *worker) setExtra(extra []byte) {
w.extra = extra
}

// setGasTip sets the minimum miner tip needed to include a non-local transaction.
func (w *worker) setGasTip(tip *big.Int) {
w.mu.Lock()
defer w.mu.Unlock()
w.tip = tip

Check warning on line 404 in miner/worker.go

View check run for this annotation

Codecov / codecov/patch

miner/worker.go#L401-L404

Added lines #L401 - L404 were not covered by tests
}

// setRecommitInterval updates the interval for miner sealing work recommitting.
func (w *worker) setRecommitInterval(interval time.Duration) {
select {
Expand Down Expand Up @@ -648,7 +657,7 @@ func (w *worker) mainLoop(ctx context.Context) {
}
txset := newTransactionsByPriceAndNonce(w.current.signer, txs, w.current.header.BaseFee)
tcount := w.current.tcount
w.commitTransactions(w.current, txset, nil, context.Background())
w.commitTransactions(w.current, txset, nil, new(big.Int), context.Background())

Check warning on line 660 in miner/worker.go

View check run for this annotation

Codecov / codecov/patch

miner/worker.go#L660

Added line #L660 was not covered by tests

// Only update the snapshot if any new transactons were added
// to the pending block
Expand Down Expand Up @@ -908,7 +917,7 @@ func (w *worker) commitTransaction(env *environment, tx *types.Transaction, inte
return receipt.Logs, nil
}

func (w *worker) commitTransactions(env *environment, txs *transactionsByPriceAndNonce, interrupt *atomic.Int32, interruptCtx context.Context) error {
func (w *worker) commitTransactions(env *environment, txs *transactionsByPriceAndNonce, interrupt *atomic.Int32, minTip *big.Int, interruptCtx context.Context) error {
gasLimit := env.header.GasLimit
if env.gasPool == nil {
env.gasPool = new(core.GasPool).AddGas(gasLimit)
Expand Down Expand Up @@ -996,7 +1005,7 @@ mainloop:
break
}
// Retrieve the next transaction and abort if all done.
ltx := txs.Peek()
ltx, tip := txs.Peek()
if ltx == nil {
breakCause = "all transactions has been included"
break
Expand All @@ -1012,6 +1021,11 @@ mainloop:
txs.Pop()
continue
}
// If we don't receive enough tip for the next transaction, skip the account
if tip.Cmp(minTip) < 0 {
log.Trace("Not enough tip for transaction", "hash", ltx.Hash, "tip", tip, "needed", minTip)
break // If the next-best is too low, surely no better will be available

Check warning on line 1027 in miner/worker.go

View check run for this annotation

Codecov / codecov/patch

miner/worker.go#L1026-L1027

Added lines #L1026 - L1027 were not covered by tests
}
// Transaction seems to fit, pull it up from the pool
tx := ltx.Resolve()
if tx == nil {
Expand Down Expand Up @@ -1469,6 +1483,10 @@ func (w *worker) fillTransactions(ctx context.Context, interrupt *atomic.Int32,
err error
)

w.mu.RLock()
tip := w.tip
w.mu.RUnlock()

if len(localTxs) > 0 {
var txs *transactionsByPriceAndNonce

Expand All @@ -1487,7 +1505,7 @@ func (w *worker) fillTransactions(ctx context.Context, interrupt *atomic.Int32,
})

tracing.Exec(ctx, "", "worker.LocalCommitTransactions", func(ctx context.Context, span trace.Span) {
err = w.commitTransactions(env, txs, interrupt, interruptCtx)
err = w.commitTransactions(env, txs, interrupt, new(big.Int), interruptCtx)
})

if err != nil {
Expand Down Expand Up @@ -1515,7 +1533,7 @@ func (w *worker) fillTransactions(ctx context.Context, interrupt *atomic.Int32,
})

tracing.Exec(ctx, "", "worker.RemoteCommitTransactions", func(ctx context.Context, span trace.Span) {
err = w.commitTransactions(env, txs, interrupt, interruptCtx)
err = w.commitTransactions(env, txs, interrupt, tip, interruptCtx)

Check warning on line 1536 in miner/worker.go

View check run for this annotation

Codecov / codecov/patch

miner/worker.go#L1536

Added line #L1536 was not covered by tests
})

if err != nil {
Expand Down

0 comments on commit 1460d0b

Please sign in to comment.