Skip to content

Commit

Permalink
core/txpool: fix error logs flood caused by removeAuthorities (#31249)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
buddh0 and lightclient committed Feb 27, 2025
1 parent ce1ac50 commit 3cb35d5
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 5 deletions.
7 changes: 4 additions & 3 deletions core/txpool/legacypool/legacypool.go
Original file line number Diff line number Diff line change
Expand Up @@ -1875,12 +1875,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))

Expand Down Expand Up @@ -1918,8 +1918,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 {
Expand Down
44 changes: 44 additions & 0 deletions core/txpool/legacypool/legacypool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"math/big"
"math/rand"
"slices"
"sync"
"sync/atomic"
"testing"
Expand Down Expand Up @@ -239,6 +240,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
}

Expand Down Expand Up @@ -2482,6 +2500,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()
Expand Down
12 changes: 10 additions & 2 deletions core/types/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,15 +493,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)
}
}
Expand Down

0 comments on commit 3cb35d5

Please sign in to comment.