Skip to content

Commit e03d97a

Browse files
authored
core/txpool/legacypool: fix pricedList updates (#32906)
This pr addresses a few issues brought by the #32270 - Add updates to pricedList after dropping transactions. - Remove redundant deletions in queue.evictList, since pool.removeTx(hash, true, true) already performs the removal. - Prevent duplicate addresses during promotion when Reset is not nil.
1 parent fb8d229 commit e03d97a

File tree

2 files changed

+40
-37
lines changed

2 files changed

+40
-37
lines changed

core/txpool/legacypool/legacypool.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,7 @@ func (pool *LegacyPool) loop() {
367367
// Handle inactive account transaction eviction
368368
case <-evict.C:
369369
pool.mu.Lock()
370-
evicted := pool.queue.evict(false)
371-
for _, hash := range evicted {
370+
for _, hash := range pool.queue.evictList() {
372371
pool.removeTx(hash, true, true)
373372
}
374373
pool.mu.Unlock()
@@ -813,7 +812,7 @@ func (pool *LegacyPool) isGapped(from common.Address, tx *types.Transaction) boo
813812
//
814813
// Note, this method assumes the pool lock is held!
815814
func (pool *LegacyPool) enqueueTx(hash common.Hash, tx *types.Transaction, addAll bool) (bool, error) {
816-
replaced, err := pool.queue.add(hash, tx)
815+
replaced, err := pool.queue.add(tx)
817816
if err != nil {
818817
return false, err
819818
}
@@ -1093,7 +1092,7 @@ func (pool *LegacyPool) removeTx(hash common.Hash, outofbound bool, unreserve bo
10931092
}
10941093
}
10951094
// Transaction is in the future queue
1096-
pool.queue.removeTx(addr, tx)
1095+
pool.queue.remove(addr, tx)
10971096
return 0
10981097
}
10991098

@@ -1241,7 +1240,7 @@ func (pool *LegacyPool) runReorg(done chan struct{}, reset *txpoolResetRequest,
12411240
}
12421241
}
12431242
// Reset needs promote for all addresses
1244-
promoteAddrs = append(promoteAddrs, pool.queue.addresses()...)
1243+
promoteAddrs = pool.queue.addresses()
12451244
}
12461245
// Check for pending transactions for every account that sent new ones
12471246
promoted := pool.promoteExecutables(promoteAddrs)
@@ -1397,9 +1396,9 @@ func (pool *LegacyPool) reset(oldHead, newHead *types.Header) {
13971396
func (pool *LegacyPool) promoteExecutables(accounts []common.Address) []*types.Transaction {
13981397
gasLimit := pool.currentHead.Load().GasLimit
13991398
promotable, dropped, removedAddresses := pool.queue.promoteExecutables(accounts, gasLimit, pool.currentState, pool.pendingNonces)
1400-
promoted := make([]*types.Transaction, 0, len(promotable))
14011399

1402-
// promote all promoteable transactions
1400+
// promote all promotable transactions
1401+
promoted := make([]*types.Transaction, 0, len(promotable))
14031402
for _, tx := range promotable {
14041403
from, _ := pool.signer.Sender(tx)
14051404
if pool.promoteTx(from, tx.Hash(), tx) {
@@ -1411,16 +1410,15 @@ func (pool *LegacyPool) promoteExecutables(accounts []common.Address) []*types.T
14111410
for _, hash := range dropped {
14121411
pool.all.Remove(hash)
14131412
}
1413+
pool.priced.Removed(len(dropped))
14141414

14151415
// release all accounts that have no more transactions in the pool
14161416
for _, addr := range removedAddresses {
14171417
_, hasPending := pool.pending[addr]
1418-
_, hasQueued := pool.queue.get(addr)
1419-
if !hasPending && !hasQueued {
1418+
if !hasPending {
14201419
pool.reserver.Release(addr)
14211420
}
14221421
}
1423-
14241422
return promoted
14251423
}
14261424

@@ -1510,10 +1508,11 @@ func (pool *LegacyPool) truncatePending() {
15101508
func (pool *LegacyPool) truncateQueue() {
15111509
removed, removedAddresses := pool.queue.truncate()
15121510

1513-
// remove all removable transactions
1511+
// Remove all removable transactions from the lookup and global price list
15141512
for _, hash := range removed {
15151513
pool.all.Remove(hash)
15161514
}
1515+
pool.priced.Removed(len(removed))
15171516

15181517
for _, addr := range removedAddresses {
15191518
_, hasPending := pool.pending[addr]

core/txpool/legacypool/queue.go

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
"github.com/ethereum/go-ethereum/log"
2828
)
2929

30+
// queue manages nonce-gapped transactions that have been validated but are
31+
// not yet processable.
3032
type queue struct {
3133
config Config
3234
signer types.Signer
@@ -43,19 +45,17 @@ func newQueue(config Config, signer types.Signer) *queue {
4345
}
4446
}
4547

46-
func (q *queue) evict(force bool) []common.Hash {
47-
removed := make([]common.Hash, 0)
48+
// evictList returns the hashes of transactions that are old enough to be evicted.
49+
func (q *queue) evictList() []common.Hash {
50+
var removed []common.Hash
4851
for addr, list := range q.queued {
49-
// Any transactions old enough should be removed
50-
if force || time.Since(q.beats[addr]) > q.config.Lifetime {
51-
list := list.Flatten()
52-
for _, tx := range list {
53-
q.removeTx(addr, tx)
52+
if time.Since(q.beats[addr]) > q.config.Lifetime {
53+
for _, tx := range list.Flatten() {
5454
removed = append(removed, tx.Hash())
5555
}
56-
queuedEvictionMeter.Mark(int64(len(list)))
5756
}
5857
}
58+
queuedEvictionMeter.Mark(int64(len(removed)))
5959
return removed
6060
}
6161

@@ -100,7 +100,7 @@ func (q *queue) addresses() []common.Address {
100100
return addrs
101101
}
102102

103-
func (q queue) removeTx(addr common.Address, tx *types.Transaction) {
103+
func (q *queue) remove(addr common.Address, tx *types.Transaction) {
104104
if future := q.queued[addr]; future != nil {
105105
if txOld := future.txs.Get(tx.Nonce()); txOld != nil && txOld.Hash() != tx.Hash() {
106106
// Edge case, a different transaction
@@ -118,7 +118,7 @@ func (q queue) removeTx(addr common.Address, tx *types.Transaction) {
118118
}
119119
}
120120

121-
func (q *queue) add(hash common.Hash, tx *types.Transaction) (*common.Hash, error) {
121+
func (q *queue) add(tx *types.Transaction) (*common.Hash, error) {
122122
// Try to insert the transaction into the future queue
123123
from, _ := types.Sender(q.signer, tx) // already validated
124124
if q.queued[from] == nil {
@@ -149,15 +149,17 @@ func (q *queue) add(hash common.Hash, tx *types.Transaction) (*common.Hash, erro
149149
// for promotion any that are now executable. It also drops any transactions that are
150150
// deemed too old (nonce too low) or too costly (insufficient funds or over gas limit).
151151
//
152-
// Returns three lists: all transactions that were removed from the queue and selected
153-
// for promotion; all other transactions that were removed from the queue and dropped;
154-
// the list of addresses removed.
152+
// Returns three lists:
153+
// - all transactions that were removed from the queue and selected for promotion;
154+
// - all other transactions that were removed from the queue and dropped;
155+
// - the list of addresses removed.
155156
func (q *queue) promoteExecutables(accounts []common.Address, gasLimit uint64, currentState *state.StateDB, nonces *noncer) ([]*types.Transaction, []common.Hash, []common.Address) {
156157
// Track the promotable transactions to broadcast them at once
157-
var promotable []*types.Transaction
158-
var dropped []common.Hash
159-
var removedAddresses []common.Address
160-
158+
var (
159+
promotable []*types.Transaction
160+
dropped []common.Hash
161+
removedAddresses []common.Address
162+
)
161163
// Iterate over all accounts and promote any executable transactions
162164
for _, addr := range accounts {
163165
list := q.queued[addr]
@@ -170,6 +172,7 @@ func (q *queue) promoteExecutables(accounts []common.Address, gasLimit uint64, c
170172
dropped = append(dropped, tx.Hash())
171173
}
172174
log.Trace("Removing old queued transactions", "count", len(forwards))
175+
173176
// Drop all transactions that are too costly (low balance or out of gas)
174177
drops, _ := list.Filter(currentState.GetBalance(addr), gasLimit)
175178
for _, tx := range drops {
@@ -205,9 +208,9 @@ func (q *queue) promoteExecutables(accounts []common.Address, gasLimit uint64, c
205208
}
206209

207210
// truncate drops the oldest transactions from the queue until the total
208-
// number is below the configured limit.
209-
// Returns the hashes of all dropped transactions, and the addresses of
210-
// accounts that became empty due to the truncation.
211+
// number is below the configured limit. Returns the hashes of all dropped
212+
// transactions and the addresses of accounts that became empty due to
213+
// the truncation.
211214
func (q *queue) truncate() ([]common.Hash, []common.Address) {
212215
queued := uint64(0)
213216
for _, list := range q.queued {
@@ -223,10 +226,12 @@ func (q *queue) truncate() ([]common.Hash, []common.Address) {
223226
addresses = append(addresses, addressByHeartbeat{addr, q.beats[addr]})
224227
}
225228
sort.Sort(sort.Reverse(addresses))
226-
removed := make([]common.Hash, 0)
227-
removedAddresses := make([]common.Address, 0)
228229

229230
// Drop transactions until the total is below the limit
231+
var (
232+
removed = make([]common.Hash, 0)
233+
removedAddresses = make([]common.Address, 0)
234+
)
230235
for drop := queued - q.config.GlobalQueue; drop > 0 && len(addresses) > 0; {
231236
addr := addresses[len(addresses)-1]
232237
list := q.queued[addr.address]
@@ -236,7 +241,7 @@ func (q *queue) truncate() ([]common.Hash, []common.Address) {
236241
// Drop all transactions if they are less than the overflow
237242
if size := uint64(list.Len()); size <= drop {
238243
for _, tx := range list.Flatten() {
239-
q.removeTx(addr.address, tx)
244+
q.remove(addr.address, tx)
240245
removed = append(removed, tx.Hash())
241246
}
242247
drop -= size
@@ -247,14 +252,13 @@ func (q *queue) truncate() ([]common.Hash, []common.Address) {
247252
// Otherwise drop only last few transactions
248253
txs := list.Flatten()
249254
for i := len(txs) - 1; i >= 0 && drop > 0; i-- {
250-
q.removeTx(addr.address, txs[i])
255+
q.remove(addr.address, txs[i])
251256
removed = append(removed, txs[i].Hash())
252257
drop--
253258
queuedRateLimitMeter.Mark(1)
254259
}
255260
}
256-
257-
// no need to clear empty accounts, removeTx already does that
261+
// No need to clear empty accounts, remove already does that
258262
return removed, removedAddresses
259263
}
260264

0 commit comments

Comments
 (0)