From fc4dd183e9327af9b5443fdea0bfbf68f041826e Mon Sep 17 00:00:00 2001 From: buddho Date: Thu, 27 Feb 2025 17:08:33 +0800 Subject: [PATCH] core/txpool: fix error logs flood caused by removeAuthorities (#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 --- core/txpool/legacypool/legacypool.go | 7 ++-- core/txpool/legacypool/legacypool_test.go | 44 +++++++++++++++++++++++ core/types/transaction.go | 12 +++++-- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 4250283b03..0fda56986e 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -1765,12 +1765,12 @@ func (t *lookup) Remove(hash common.Hash) { t.lock.Lock() defer t.lock.Unlock() - t.removeAuthorities(hash) tx, ok := t.txs[hash] if !ok { log.Error("No transaction found to be deleted", "hash", hash) return } + t.removeAuthorities(tx) t.slots -= numSlots(tx) slotsGauge.Update(int64(t.slots)) @@ -1808,8 +1808,9 @@ func (t *lookup) addAuthorities(tx *types.Transaction) { // removeAuthorities stops tracking the supplied tx in relation to its // authorities. -func (t *lookup) removeAuthorities(hash common.Hash) { - for addr := range t.auths { +func (t *lookup) removeAuthorities(tx *types.Transaction) { + hash := tx.Hash() + for _, addr := range tx.SetCodeAuthorities() { list := t.auths[addr] // Remove tx from tracker. if i := slices.Index(list, hash); i >= 0 { diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index b2a068467f..ef887041ad 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -23,6 +23,7 @@ import ( "fmt" "math/big" "math/rand" + "slices" "sync" "sync/atomic" "testing" @@ -238,6 +239,23 @@ func validatePoolInternals(pool *LegacyPool) error { return fmt.Errorf("pending nonce mismatch: have %v, want %v", nonce, last+1) } } + // Ensure all auths in pool are tracked + for _, tx := range pool.all.txs { + for _, addr := range tx.SetCodeAuthorities() { + list := pool.all.auths[addr] + if i := slices.Index(list, tx.Hash()); i < 0 { + return fmt.Errorf("authority not tracked: addr %s, tx %s", addr, tx.Hash()) + } + } + } + // Ensure all auths in pool have an associated tx. + for addr, hashes := range pool.all.auths { + for _, hash := range hashes { + if _, ok := pool.all.txs[hash]; !ok { + return fmt.Errorf("dangling authority, missing originating tx: addr %s, hash %s", addr, hash.Hex()) + } + } + } return nil } @@ -2381,6 +2399,32 @@ func TestSetCodeTransactions(t *testing.T) { } }, }, + { + name: "remove-hash-from-authority-tracker", + pending: 10, + run: func(name string) { + var keys []*ecdsa.PrivateKey + for i := 0; i < 30; i++ { + key, _ := crypto.GenerateKey() + keys = append(keys, key) + addr := crypto.PubkeyToAddress(key.PublicKey) + testAddBalance(pool, addr, big.NewInt(params.Ether)) + } + // Create a transactions with 3 unique auths so the lookup's auth map is + // filled with addresses. + for i := 0; i < 30; i += 3 { + 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 { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) + } + } + // Replace one of the transactions with a normal transaction so that the + // original hash is removed from the tracker. The hash should be + // associated with 3 different authorities. + if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keys[0])); err != nil { + t.Fatalf("%s: failed to replace with remote transaction: %v", name, err) + } + }, + }, } { tt.run(tt.name) pending, queued := pool.Stats() diff --git a/core/types/transaction.go b/core/types/transaction.go index 7df13e04bb..4e48248b4a 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -483,15 +483,23 @@ func (tx *Transaction) SetCodeAuthorizations() []SetCodeAuthorization { return setcodetx.AuthList } -// SetCodeAuthorities returns a list of each authorization's corresponding authority. +// SetCodeAuthorities returns a list of unique authorities from the +// authorization list. func (tx *Transaction) SetCodeAuthorities() []common.Address { setcodetx, ok := tx.inner.(*SetCodeTx) if !ok { return nil } - auths := make([]common.Address, 0, len(setcodetx.AuthList)) + var ( + marks = make(map[common.Address]bool) + auths = make([]common.Address, 0, len(setcodetx.AuthList)) + ) for _, auth := range setcodetx.AuthList { if addr, err := auth.Authority(); err == nil { + if marks[addr] { + continue + } + marks[addr] = true auths = append(auths, addr) } }