Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/txpool: fix error logs flood caused by removeAuthorities #31249

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions core/txpool/legacypool/legacypool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down Expand Up @@ -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 {
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 @@ -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
}

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