Skip to content

Commit 414f06e

Browse files
committed
core/txpool: fix error logs flood caused by removeAuthorities
PR: ethereum/go-ethereum#31249
1 parent 606734a commit 414f06e

File tree

3 files changed

+67
-5
lines changed

3 files changed

+67
-5
lines changed

core/txpool/legacypool/legacypool.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -1955,7 +1955,6 @@ func (t *lookup) Remove(hash common.Hash) {
19551955
t.lock.Lock()
19561956
defer t.lock.Unlock()
19571957

1958-
t.removeAuthorities(hash)
19591958
tx, ok := t.locals[hash]
19601959
if !ok {
19611960
tx, ok = t.remotes[hash]
@@ -1964,6 +1963,7 @@ func (t *lookup) Remove(hash common.Hash) {
19641963
log.Error("No transaction found to be deleted", "hash", hash)
19651964
return
19661965
}
1966+
t.removeAuthorities(tx)
19671967
t.slots -= numSlots(tx)
19681968
slotsGauge.Update(int64(t.slots))
19691969

@@ -2026,8 +2026,9 @@ func (t *lookup) addAuthorities(tx *types.Transaction) {
20262026

20272027
// removeAuthorities stops tracking the supplied tx in relation to its
20282028
// authorities.
2029-
func (t *lookup) removeAuthorities(hash common.Hash) {
2030-
for addr := range t.auths {
2029+
func (t *lookup) removeAuthorities(tx *types.Transaction) {
2030+
hash := tx.Hash()
2031+
for _, addr := range tx.SetCodeAuthorities() {
20312032
list := t.auths[addr]
20322033
// Remove tx from tracker.
20332034
if i := slices.Index(list, hash); i >= 0 {

core/txpool/legacypool/legacypool_test.go

+54
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"math/big"
2525
"math/rand"
2626
"os"
27+
"slices"
2728
"sync"
2829
"sync/atomic"
2930
"testing"
@@ -174,6 +175,33 @@ func validatePoolInternals(pool *LegacyPool) error {
174175
return fmt.Errorf("totalcost went negative: %v", txs.totalcost)
175176
}
176177
}
178+
// Ensure all auths in pool are tracked
179+
for _, tx := range pool.all.locals {
180+
for _, addr := range tx.SetCodeAuthorities() {
181+
list := pool.all.auths[addr]
182+
if i := slices.Index(list, tx.Hash()); i < 0 {
183+
return fmt.Errorf("authority not tracked: addr %s, tx %s", addr, tx.Hash())
184+
}
185+
}
186+
}
187+
for _, tx := range pool.all.remotes {
188+
for _, addr := range tx.SetCodeAuthorities() {
189+
list := pool.all.auths[addr]
190+
if i := slices.Index(list, tx.Hash()); i < 0 {
191+
return fmt.Errorf("authority not tracked: addr %s, tx %s", addr, tx.Hash())
192+
}
193+
}
194+
}
195+
// Ensure all auths in pool have an associated tx.
196+
for addr, hashes := range pool.all.auths {
197+
for _, hash := range hashes {
198+
_, foundLocal := pool.all.locals[hash]
199+
_, foundRemote := pool.all.remotes[hash]
200+
if !foundLocal && !foundRemote {
201+
return fmt.Errorf("dangling authority, missing originating tx: addr %s, hash %s", addr, hash.Hex())
202+
}
203+
}
204+
}
177205
return nil
178206
}
179207

@@ -2779,6 +2807,32 @@ func TestSetCodeTransactions(t *testing.T) {
27792807
}
27802808
},
27812809
},
2810+
{
2811+
name: "remove-hash-from-authority-tracker",
2812+
pending: 10,
2813+
run: func(name string) {
2814+
var keys []*ecdsa.PrivateKey
2815+
for i := 0; i < 30; i++ {
2816+
key, _ := crypto.GenerateKey()
2817+
keys = append(keys, key)
2818+
addr := crypto.PubkeyToAddress(key.PublicKey)
2819+
testAddBalance(pool, addr, big.NewInt(params.Ether))
2820+
}
2821+
// Create a transactions with 3 unique auths so the lookup's auth map is
2822+
// filled with addresses.
2823+
for i := 0; i < 30; i += 3 {
2824+
if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(10), uint256.NewInt(3), keys[i], []unsignedAuth{{0, keys[i]}, {0, keys[i+1]}, {0, keys[i+2]}})); err != nil {
2825+
t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err)
2826+
}
2827+
}
2828+
// Replace one of the transactions with a normal transaction so that the
2829+
// original hash is removed from the tracker. The hash should be
2830+
// associated with 3 different authorities.
2831+
if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keys[0])); err != nil {
2832+
t.Fatalf("%s: failed to replace with remote transaction: %v", name, err)
2833+
}
2834+
},
2835+
},
27822836
} {
27832837
tt.run(tt.name)
27842838
pending, queued := pool.Stats()

core/types/transaction.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -556,15 +556,22 @@ func (tx *Transaction) SetCodeAuthorizations() []Authorization {
556556
return setcodetx.AuthList
557557
}
558558

559-
// SetCodeAuthorities returns a list of each authorization's corresponding authority.
559+
// SetCodeAuthorities returns a list of unique authorities from the authorization list.
560560
func (tx *Transaction) SetCodeAuthorities() []common.Address {
561561
setcodetx, ok := tx.inner.(*SetCodeTx)
562562
if !ok {
563563
return nil
564564
}
565-
auths := make([]common.Address, 0, len(setcodetx.AuthList))
565+
var (
566+
marks = make(map[common.Address]bool)
567+
auths = make([]common.Address, 0, len(setcodetx.AuthList))
568+
)
566569
for _, auth := range setcodetx.AuthList {
567570
if addr, err := auth.Authority(); err == nil {
571+
if marks[addr] {
572+
continue
573+
}
574+
marks[addr] = true
568575
auths = append(auths, addr)
569576
}
570577
}

0 commit comments

Comments
 (0)