mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-05-24 08:49:29 +00:00
core/txpool: guard legacypool reservation release ownership
Add an ownership guard before releasing reserved addresses from legacypool cleanup paths. Address presence in pending/queue and reservation ownership are tracked separately. Cleanup logic can observe an address as queue-empty while the reservation tracker no longer considers that address owned by the current subpool. In that case, a follow-up Release call can attempt to unreserve a non-owned address. Use an optional ownership capability when available, so legacypool only releases reservations it still owns without expanding the exported Reserver interface. Also add a focused regression test that exercises the queue-empty-without-reservation scenario.
This commit is contained in:
parent
acdd139717
commit
b9eb244a67
3 changed files with 75 additions and 6 deletions
|
|
@ -1006,6 +1006,19 @@ func (pool *LegacyPool) get(hash common.Hash) *types.Transaction {
|
|||
return pool.all.Get(hash)
|
||||
}
|
||||
|
||||
type ownerReserver interface {
|
||||
Owns(common.Address) bool
|
||||
}
|
||||
|
||||
func (pool *LegacyPool) releaseReservation(addr common.Address) {
|
||||
if ownerAware, ok := pool.reserver.(ownerReserver); ok {
|
||||
if !ownerAware.Owns(addr) {
|
||||
return
|
||||
}
|
||||
}
|
||||
_ = pool.reserver.Release(addr)
|
||||
}
|
||||
|
||||
// GetRLP returns a RLP-encoded transaction if it is contained in the pool.
|
||||
func (pool *LegacyPool) GetRLP(hash common.Hash) []byte {
|
||||
tx := pool.all.Get(hash)
|
||||
|
|
@ -1066,7 +1079,7 @@ func (pool *LegacyPool) removeTx(hash common.Hash, outofbound bool, unreserve bo
|
|||
_, hasQueued = pool.queue.get(addr)
|
||||
)
|
||||
if !hasPending && !hasQueued {
|
||||
pool.reserver.Release(addr)
|
||||
pool.releaseReservation(addr)
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
|
@ -1420,7 +1433,7 @@ func (pool *LegacyPool) promoteExecutables(accounts []common.Address) []*types.T
|
|||
for _, addr := range removedAddresses {
|
||||
_, hasPending := pool.pending[addr]
|
||||
if !hasPending {
|
||||
pool.reserver.Release(addr)
|
||||
pool.releaseReservation(addr)
|
||||
}
|
||||
}
|
||||
return promoted
|
||||
|
|
@ -1521,7 +1534,7 @@ func (pool *LegacyPool) truncateQueue() {
|
|||
for _, addr := range removedAddresses {
|
||||
_, hasPending := pool.pending[addr]
|
||||
if !hasPending {
|
||||
pool.reserver.Release(addr)
|
||||
pool.releaseReservation(addr)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1582,7 +1595,7 @@ func (pool *LegacyPool) demoteUnexecutables() {
|
|||
delete(pool.pending, addr)
|
||||
pendingAddrsGauge.Dec(1)
|
||||
if _, ok := pool.queue.get(addr); !ok {
|
||||
pool.reserver.Release(addr)
|
||||
pool.releaseReservation(addr)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1829,11 +1842,11 @@ func (pool *LegacyPool) Clear() {
|
|||
|
||||
for addr := range pool.pending {
|
||||
if _, ok := pool.queue.get(addr); !ok {
|
||||
pool.reserver.Release(addr)
|
||||
pool.releaseReservation(addr)
|
||||
}
|
||||
}
|
||||
for _, addr := range pool.queue.addresses() {
|
||||
pool.reserver.Release(addr)
|
||||
pool.releaseReservation(addr)
|
||||
}
|
||||
pool.all.Clear()
|
||||
pool.priced.Reheap()
|
||||
|
|
|
|||
|
|
@ -229,6 +229,13 @@ func (r *reserver) Has(address common.Address) bool {
|
|||
return false // reserver only supports a single pool
|
||||
}
|
||||
|
||||
func (r *reserver) Owns(address common.Address) bool {
|
||||
r.lock.RLock()
|
||||
defer r.lock.RUnlock()
|
||||
_, exists := r.accounts[address]
|
||||
return exists
|
||||
}
|
||||
|
||||
func setupPoolWithConfig(config *params.ChainConfig) (*LegacyPool, *ecdsa.PrivateKey) {
|
||||
statedb, _ := state.New(types.EmptyRootHash, state.NewDatabaseForTesting())
|
||||
blockchain := newTestBlockChain(config, 10000000, statedb, new(event.Feed))
|
||||
|
|
@ -393,6 +400,46 @@ func TestStateChangeDuringReset(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestPromoteExecutablesQueueEmptyWithoutReservation(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
statedb, _ := state.New(types.EmptyRootHash, state.NewDatabaseForTesting())
|
||||
chain := newTestBlockChain(params.TestChainConfig, 10000000, statedb, new(event.Feed))
|
||||
|
||||
key, err := crypto.GenerateKey()
|
||||
if err != nil {
|
||||
t.Fatalf("failed to generate key: %v", err)
|
||||
}
|
||||
addr := crypto.PubkeyToAddress(key.PublicKey)
|
||||
statedb.SetBalance(addr, uint256.NewInt(params.Ether), tracing.BalanceChangeUnspecified)
|
||||
|
||||
r := &reserver{accounts: make(map[common.Address]struct{})}
|
||||
pool := New(testTxPoolConfig, chain)
|
||||
if err := pool.Init(testTxPoolConfig.PriceLimit, chain.CurrentBlock(), r); err != nil {
|
||||
t.Fatalf("failed to init pool: %v", err)
|
||||
}
|
||||
defer pool.Close()
|
||||
<-pool.initDoneCh
|
||||
|
||||
queuedTx := pricedTransaction(5, 100000, big.NewInt(params.GWei), key)
|
||||
if err := pool.addRemoteSync(queuedTx); err != nil {
|
||||
t.Fatalf("failed to add queued tx: %v", err)
|
||||
}
|
||||
|
||||
r.lock.Lock()
|
||||
delete(r.accounts, addr)
|
||||
r.lock.Unlock()
|
||||
|
||||
pool.mu.Lock()
|
||||
defer pool.mu.Unlock()
|
||||
pool.currentState.SetNonce(addr, 10, tracing.NonceChangeUnspecified)
|
||||
pool.promoteExecutables([]common.Address{addr})
|
||||
|
||||
if _, ok := pool.queue.get(addr); ok {
|
||||
t.Fatal("queue should be empty after stale tx is dropped")
|
||||
}
|
||||
}
|
||||
|
||||
func testAddBalance(pool *LegacyPool, addr common.Address, amount *big.Int) {
|
||||
pool.mu.Lock()
|
||||
pool.currentState.AddBalance(addr, uint256.MustFromBig(amount), tracing.BalanceChangeUnspecified)
|
||||
|
|
|
|||
|
|
@ -136,3 +136,12 @@ func (h *ReservationHandle) Has(address common.Address) bool {
|
|||
id, exists := h.tracker.accounts[address]
|
||||
return exists && id != h.id
|
||||
}
|
||||
|
||||
// Owns reports whether this handle currently owns the reservation for address.
|
||||
func (h *ReservationHandle) Owns(address common.Address) bool {
|
||||
h.tracker.lock.RLock()
|
||||
defer h.tracker.lock.RUnlock()
|
||||
|
||||
id, exists := h.tracker.accounts[address]
|
||||
return exists && id == h.id
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue