Skip to content

Commit 07615ee

Browse files
buddh0lightclient
andcommitted
core/txpool: fix error logs flood caused by removeAuthorities (ethereum#31249)
when remove an non-SetCodeTxType transaction, error logs flood ``` t=2025-02-25T03:11:06+0000 lvl=error msg="Authority with untracked tx" addr=0xD5bf9221fCB1C31Cd1EE477a60c148d40dD63DC1 hash=0x626fdf205a5b1619deb2f9e51fed567353f80acbd522265b455daa0821c571d9 ``` in this PR, only try to removeAuthorities for txs with SetCodeTxType in addition, the performance of removeAuthorities improved a lot, because no need range all `t.auths` now. --------- Co-authored-by: lightclient <[email protected]>
1 parent e482db0 commit 07615ee

File tree

3 files changed

+58
-5
lines changed

3 files changed

+58
-5
lines changed

core/txpool/legacypool/legacypool.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1875,12 +1875,12 @@ func (t *lookup) Remove(hash common.Hash) {
18751875
t.lock.Lock()
18761876
defer t.lock.Unlock()
18771877

1878-
t.removeAuthorities(hash)
18791878
tx, ok := t.txs[hash]
18801879
if !ok {
18811880
log.Error("No transaction found to be deleted", "hash", hash)
18821881
return
18831882
}
1883+
t.removeAuthorities(tx)
18841884
t.slots -= numSlots(tx)
18851885
slotsGauge.Update(int64(t.slots))
18861886

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

19191919
// removeAuthorities stops tracking the supplied tx in relation to its
19201920
// authorities.
1921-
func (t *lookup) removeAuthorities(hash common.Hash) {
1922-
for addr := range t.auths {
1921+
func (t *lookup) removeAuthorities(tx *types.Transaction) {
1922+
hash := tx.Hash()
1923+
for _, addr := range tx.SetCodeAuthorities() {
19231924
list := t.auths[addr]
19241925
// Remove tx from tracker.
19251926
if i := slices.Index(list, hash); i >= 0 {

core/txpool/legacypool/legacypool_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"fmt"
2424
"math/big"
2525
"math/rand"
26+
"slices"
2627
"sync"
2728
"sync/atomic"
2829
"testing"
@@ -239,6 +240,23 @@ func validatePoolInternals(pool *LegacyPool) error {
239240
return fmt.Errorf("pending nonce mismatch: have %v, want %v", nonce, last+1)
240241
}
241242
}
243+
// Ensure all auths in pool are tracked
244+
for _, tx := range pool.all.txs {
245+
for _, addr := range tx.SetCodeAuthorities() {
246+
list := pool.all.auths[addr]
247+
if i := slices.Index(list, tx.Hash()); i < 0 {
248+
return fmt.Errorf("authority not tracked: addr %s, tx %s", addr, tx.Hash())
249+
}
250+
}
251+
}
252+
// Ensure all auths in pool have an associated tx.
253+
for addr, hashes := range pool.all.auths {
254+
for _, hash := range hashes {
255+
if _, ok := pool.all.txs[hash]; !ok {
256+
return fmt.Errorf("dangling authority, missing originating tx: addr %s, hash %s", addr, hash.Hex())
257+
}
258+
}
259+
}
242260
return nil
243261
}
244262

@@ -2482,6 +2500,32 @@ func TestSetCodeTransactions(t *testing.T) {
24822500
}
24832501
},
24842502
},
2503+
{
2504+
name: "remove-hash-from-authority-tracker",
2505+
pending: 10,
2506+
run: func(name string) {
2507+
var keys []*ecdsa.PrivateKey
2508+
for i := 0; i < 30; i++ {
2509+
key, _ := crypto.GenerateKey()
2510+
keys = append(keys, key)
2511+
addr := crypto.PubkeyToAddress(key.PublicKey)
2512+
testAddBalance(pool, addr, big.NewInt(params.Ether))
2513+
}
2514+
// Create a transactions with 3 unique auths so the lookup's auth map is
2515+
// filled with addresses.
2516+
for i := 0; i < 30; i += 3 {
2517+
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 {
2518+
t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err)
2519+
}
2520+
}
2521+
// Replace one of the transactions with a normal transaction so that the
2522+
// original hash is removed from the tracker. The hash should be
2523+
// associated with 3 different authorities.
2524+
if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keys[0])); err != nil {
2525+
t.Fatalf("%s: failed to replace with remote transaction: %v", name, err)
2526+
}
2527+
},
2528+
},
24852529
} {
24862530
tt.run(tt.name)
24872531
pending, queued := pool.Stats()

core/types/transaction.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -493,15 +493,23 @@ func (tx *Transaction) SetCodeAuthorizations() []SetCodeAuthorization {
493493
return setcodetx.AuthList
494494
}
495495

496-
// SetCodeAuthorities returns a list of each authorization's corresponding authority.
496+
// SetCodeAuthorities returns a list of unique authorities from the
497+
// authorization list.
497498
func (tx *Transaction) SetCodeAuthorities() []common.Address {
498499
setcodetx, ok := tx.inner.(*SetCodeTx)
499500
if !ok {
500501
return nil
501502
}
502-
auths := make([]common.Address, 0, len(setcodetx.AuthList))
503+
var (
504+
marks = make(map[common.Address]bool)
505+
auths = make([]common.Address, 0, len(setcodetx.AuthList))
506+
)
503507
for _, auth := range setcodetx.AuthList {
504508
if addr, err := auth.Authority(); err == nil {
509+
if marks[addr] {
510+
continue
511+
}
512+
marks[addr] = true
505513
auths = append(auths, addr)
506514
}
507515
}

0 commit comments

Comments
 (0)