Skip to content
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, miner: fix enforcing the minimum miner tip (#28933) #1209

Merged
merged 1 commit into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@
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 @@
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 @@
"context"
"errors"
"fmt"
"math/big"
"os"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -175,7 +176,7 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
err error
)

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

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

Expand All @@ -1487,7 +1505,7 @@
})

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 @@
})

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
Loading