core/txpool/legacypool: fix incorrect error in authority tracker when removing tx

[lightclient]

While refactoring #31073 to track authorities based on tx hash instead of pointer, I changed how removeAuthorities(..) works. Before, it had access to the transaction object so it could directly iterate the authorities on the object itself. I changed it to be the transaction hash and modified the logic to check every tracked authority and see if it was tracking that hash.

This created an issue where before it was an error if the tx wasn't found in the list, but after the change, it is more the expected state since most authorities in the tracker will be from unrelated txs.

Added a testcase which would catch this (if the log were a panic) and add some integrity checks for the auth tracker in the validatePoolInternals(..) function.

[rjl493456442]

We are essentially iterate the entire auth maps for every transaction, not sure if it's too expensive.

diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go
index e959bdb55f..c194411c70 100644
--- a/core/txpool/legacypool/legacypool.go
+++ b/core/txpool/legacypool/legacypool.go
@@ -637,7 +637,7 @@ func (pool *LegacyPool) validateAuth(tx *types.Transaction) error {
 		}
 	}
 	// Authorities cannot conflict with any pending or queued transactions.
-	if auths := tx.SetCodeAuthorities(); len(auths) > 0 {
+	if auths := tx.SetCodeAuthorities(false); len(auths) > 0 {
 		for _, auth := range auths {
 			if pool.pending[auth] != nil || pool.queue[auth] != nil {
 				return ErrAuthorityReserved
@@ -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(hash, tx.SetCodeAuthorities(false))
 	t.slots -= numSlots(tx)
 	slotsGauge.Update(int64(t.slots))
 
@@ -1792,7 +1792,7 @@ func (t *lookup) TxsBelowTip(threshold *big.Int) types.Transactions {
 // addAuthorities tracks the supplied tx in relation to each authority it
 // specifies.
 func (t *lookup) addAuthorities(tx *types.Transaction) {
-	for _, addr := range tx.SetCodeAuthorities() {
+	for _, addr := range tx.SetCodeAuthorities(false) {
 		list, ok := t.auths[addr]
 		if !ok {
 			list = []common.Hash{}
@@ -1808,12 +1808,14 @@ 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(hash common.Hash, auths []common.Address) {
+	for _, addr := range auths {
 		list := t.auths[addr]
 		// Remove tx from tracker.
 		if i := slices.Index(list, hash); i >= 0 {
 			list = append(list[:i], list[i+1:]...)
+		} else {
+			log.Error("Authority with untracked tx", "addr", addr, "hash", hash)
 		}
 		if len(list) == 0 {
 			// If list is newly empty, delete it entirely.
diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go
index 5cc00785a4..0db18dbf0f 100644
--- a/core/txpool/legacypool/legacypool_test.go
+++ b/core/txpool/legacypool/legacypool_test.go
@@ -241,7 +241,7 @@ func validatePoolInternals(pool *LegacyPool) error {
 	}
 	// Ensure all auths in pool are tracked
 	for _, tx := range pool.all.txs {
-		for _, addr := range tx.SetCodeAuthorities() {
+		for _, addr := range tx.SetCodeAuthorities(false) {
 			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())
diff --git a/core/types/transaction.go b/core/types/transaction.go
index 7df13e04bb..e7f53a33b4 100644
--- a/core/types/transaction.go
+++ b/core/types/transaction.go
@@ -484,14 +484,21 @@ func (tx *Transaction) SetCodeAuthorizations() []SetCodeAuthorization {
 }
 
 // SetCodeAuthorities returns a list of each authorization's corresponding authority.
-func (tx *Transaction) SetCodeAuthorities() []common.Address {
+func (tx *Transaction) SetCodeAuthorities(allowDuplicates bool) []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 !allowDuplicates && marks[addr] {
+				continue
+			}
+			marks[addr] = true
 			auths = append(auths, addr)
 		}
 	}

Alternatively we can obtain the auth list from the cached tx.

The question is what if the tx is somehow not founded? I guess the txpool will be very wrong at that point

[lightclient]

This was a good idea - I modified it slightly to always return a unique list in bcb9836

[fjl]

There is also #31249, not sure what's the difference.

[lightclient]

Closing in favor of #31249