diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index b9125df25a..b0cc9340aa 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -21,6 +21,7 @@ import ( "errors" "math" "math/big" + "slices" "sort" "sync" "sync/atomic" @@ -209,13 +210,13 @@ type LegacyPool struct { currentState *state.StateDB // Current state in the blockchain head pendingNonces *noncer // Pending state tracking virtual nonces - reserve txpool.AddressReserver // Address reserver to ensure exclusivity across subpools - pending map[common.Address]*list // All currently processable transactions - queue map[common.Address]*list // Queued but non-processable transactions - beats map[common.Address]time.Time // Last heartbeat from each known account - all *lookup // All transactions to allow lookups - priced *pricedList // All transactions sorted by price - auths map[common.Address]*types.Transaction // All accounts with a pooled authorization + reserve txpool.AddressReserver // Address reserver to ensure exclusivity across subpools + pending map[common.Address]*list // All currently processable transactions + queue map[common.Address]*list // Queued but non-processable transactions + beats map[common.Address]time.Time // Last heartbeat from each known account + all *lookup // All transactions to allow lookups + priced *pricedList // All transactions sorted by price + auths map[common.Address][]*types.Transaction // All accounts with a pooled authorization reqResetCh chan *txpoolResetRequest reqPromoteCh chan *accountSet @@ -247,7 +248,7 @@ func New(config Config, chain BlockChain) *LegacyPool { pending: make(map[common.Address]*list), queue: make(map[common.Address]*list), beats: make(map[common.Address]time.Time), - auths: make(map[common.Address]*types.Transaction), + auths: make(map[common.Address][]*types.Transaction), all: newLookup(), reqResetCh: make(chan *txpoolResetRequest), reqPromoteCh: make(chan *accountSet), @@ -568,13 +569,9 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error { if list := pool.queue[addr]; list != nil { have += list.Len() } - // Limit the number of setcode tranasactions per account if pool.currentState.GetCode(addr) != nil { - if have >= 1 { - return have, 0 - } else { - return have, 1 - have - } + // Allow at most one in-flight tx for delegated accounts. + return have, max(0, 1-have) } return have, math.MaxInt }, @@ -592,12 +589,14 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error { } return nil }, - KnownConflicts: func(sender common.Address, addrs []common.Address) []common.Address { + KnownConflicts: func(from common.Address, auths []common.Address) []common.Address { var conflicts []common.Address - if _, ok := pool.auths[sender]; ok { - conflicts = append(conflicts, sender) + // The transaction sender cannot have an in-flight authorization. + if _, ok := pool.auths[from]; ok { + conflicts = append(conflicts, from) } - for _, addr := range addrs { + // Authorities cannot conflict with any pending or queued transactions. + for _, addr := range auths { var known bool if list := pool.pending[addr]; list != nil { known = true @@ -605,9 +604,6 @@ func (pool *LegacyPool) validateTx(tx *types.Transaction) error { if list := pool.queue[addr]; list != nil { known = true } - if _, ok := pool.auths[addr]; ok { - known = true - } if known { conflicts = append(conflicts, addr) } @@ -644,7 +640,6 @@ func (pool *LegacyPool) add(tx *types.Transaction) (replaced bool, err error) { // If the address is not yet known, request exclusivity to track the account // only by this subpool until all transactions are evicted - // TODO: need to track every authority from setcode txs var ( _, hasPending = pool.pending[from] _, hasQueued = pool.queue[from] @@ -744,9 +739,7 @@ func (pool *LegacyPool) add(tx *types.Transaction) (replaced bool, err error) { pool.all.Add(tx) pool.priced.Put(tx) pool.queueTxEvent(tx) - for _, addr := range tx.Authorities() { - pool.auths[addr] = tx - } + pool.addAuthorities(tx) log.Trace("Pooled new executable transaction", "hash", hash, "from", from, "to", tx.To()) // Successful promotion, bump the heartbeat @@ -758,9 +751,7 @@ func (pool *LegacyPool) add(tx *types.Transaction) (replaced bool, err error) { if err != nil { return false, err } - for _, addr := range tx.Authorities() { - pool.auths[addr] = tx - } + pool.addAuthorities(tx) log.Trace("Pooled new future transaction", "hash", hash, "from", from, "to", tx.To()) return replaced, nil @@ -828,10 +819,7 @@ func (pool *LegacyPool) enqueueTx(hash common.Hash, tx *types.Transaction, addAl if _, exist := pool.beats[from]; !exist { pool.beats[from] = time.Now() } - for _, auth := range tx.SetCodeAuthorizations() { - addr, _ := auth.Authority() - pool.auths[addr] = tx - } + pool.addAuthorities(tx) return old != nil, nil } @@ -1086,9 +1074,40 @@ func (pool *LegacyPool) removeTx(hash common.Hash, outofbound bool, unreserve bo return 0 } +// addAuthorities tracks the supplied tx in relation to each authority it +// specifies. +func (pool *LegacyPool) addAuthorities(tx *types.Transaction) { + for _, addr := range tx.Authorities() { + list, ok := pool.auths[addr] + if !ok { + list = []*types.Transaction{} + } + if slices.Contains(list, tx) { + // Don't add duplicates. + continue + } + list = append(list, tx) + pool.auths[addr] = list + } +} + +// removeAuthorities stops tracking the supplied tx in relation to its +// authorities. func (pool *LegacyPool) removeAuthorities(tx *types.Transaction) { for _, addr := range tx.Authorities() { - delete(pool.auths, addr) + // Remove tx from tracker. + list := pool.auths[addr] + if i := slices.Index(list, tx); i >= 0 { + list = append(list[:i], list[i+1:]...) + } else { + log.Error("Authority with untracked tx", "addr", addr, "hash", tx.Hash()) + } + if len(list) == 0 { + // If list is newly empty, delete it entirely. + delete(pool.auths, addr) + continue + } + pool.auths[addr] = list } } @@ -1823,5 +1842,5 @@ func (pool *LegacyPool) Clear() { pool.pending = make(map[common.Address]*list) pool.queue = make(map[common.Address]*list) pool.pendingNonces = newNoncer(pool.currentState) - pool.auths = make(map[common.Address]*types.Transaction) + pool.auths = make(map[common.Address][]*types.Transaction) } diff --git a/core/txpool/legacypool/legacypool_test.go b/core/txpool/legacypool/legacypool_test.go index 0d8f904ddb..60be01b58d 100644 --- a/core/txpool/legacypool/legacypool_test.go +++ b/core/txpool/legacypool/legacypool_test.go @@ -2212,13 +2212,17 @@ func TestSetCodeTransactions(t *testing.T) { defer pool.Close() // Create the test accounts - keys := make([]*ecdsa.PrivateKey, 4) - addrs := make([]common.Address, len(keys)) - for i := 0; i < len(keys); i++ { - keys[i], _ = crypto.GenerateKey() - addrs[i] = crypto.PubkeyToAddress(keys[i].PublicKey) - testAddBalance(pool, crypto.PubkeyToAddress(keys[i].PublicKey), big.NewInt(params.Ether)) - } + var ( + keyA, _ = crypto.GenerateKey() + keyB, _ = crypto.GenerateKey() + keyC, _ = crypto.GenerateKey() + addrA = crypto.PubkeyToAddress(keyA.PublicKey) + addrB = crypto.PubkeyToAddress(keyB.PublicKey) + addrC = crypto.PubkeyToAddress(keyC.PublicKey) + ) + testAddBalance(pool, addrA, big.NewInt(params.Ether)) + testAddBalance(pool, addrB, big.NewInt(params.Ether)) + testAddBalance(pool, addrC, big.NewInt(params.Ether)) // A few situations to test: // 1. Accounts with delegation set can only have one in-flight transaction. @@ -2237,106 +2241,140 @@ func TestSetCodeTransactions(t *testing.T) { name string pending int queued int - run func() error + run func(string) }{ { + // Check that only one in-flight transaction is allowed for accounts + // with delegation set. Also verify the accepted transaction can be + // replaced by fee. name: "only-one-in-flight", pending: 1, - run: func() error { - // Check that only one in-flight transaction is allowed for accounts - // with delegation set. Also verify the accepted transaction can be - // replaced by fee. + run: func(name string) { aa := common.Address{0xaa, 0xaa} - statedb.SetCode(addrs[0], append(types.DelegationPrefix, aa.Bytes()...)) + statedb.SetCode(addrA, append(types.DelegationPrefix, aa.Bytes()...)) statedb.SetCode(aa, []byte{byte(vm.ADDRESS), byte(vm.PUSH0), byte(vm.SSTORE)}) // Send transactions. First is accepted, second is rejected. - if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1), keys[0])); err != nil { - return fmt.Errorf("failed to add remote transaction: %v", err) + if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1), keyA)); err != nil { + t.Fatalf("%s: failed to add remote transaction: %v", name, err) } - if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keys[0])); !errors.Is(err, txpool.ErrAccountLimitExceeded) { - return fmt.Errorf("error mismatch: want %v, have %v", txpool.ErrAccountLimitExceeded, err) + if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyA)); !errors.Is(err, txpool.ErrAccountLimitExceeded) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrAccountLimitExceeded, err) } // Also check gapped transaction. - if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1), keys[0])); !errors.Is(err, txpool.ErrAccountLimitExceeded) { - return fmt.Errorf("error mismatch: want %v, have %v", txpool.ErrAccountLimitExceeded, err) + if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1), keyA)); !errors.Is(err, txpool.ErrAccountLimitExceeded) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrAccountLimitExceeded, err) } // Replace by fee. - if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(10), keys[0])); err != nil { - return fmt.Errorf("failed to replace with remote transaction: %v", err) + if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(10), keyA)); err != nil { + t.Fatalf("%s: failed to replace with remote transaction: %v", name, err) } - return nil }, }, { - name: "reject-setcode-tx-with-pending-authority-tx", - pending: 1, - run: func() error { + name: "allow-setcode-tx-with-pending-authority-tx", + pending: 2, + run: func(name string) { // Send two transactions where the first has no conflicting delegations and - // the second should be rejected due to a conflict with the tx sent in 1). - if err := pool.addRemoteSync(setCodeTx(0, keys[1], []unsignedAuth{{1, keys[2]}})); err != nil { - return fmt.Errorf("failed to add with remote setcode transaction: %v", err) + // the second should be allowed despite conflicting with the authorities in 1). + if err := pool.addRemoteSync(setCodeTx(0, keyA, []unsignedAuth{{1, keyC}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) } - if err := pool.addRemoteSync(setCodeTx(1, keys[1], []unsignedAuth{{1, keys[2]}})); !errors.Is(err, txpool.ErrAuthorityReserved) { - return fmt.Errorf("error mismatch: want %v, have %v", txpool.ErrAuthorityReserved, err) + if err := pool.addRemoteSync(setCodeTx(0, keyB, []unsignedAuth{{1, keyC}})); err != nil { + t.Fatalf("%s: error mismatch: want %v, have %v", txpool.ErrAuthorityReserved, name, err) } - return nil }, }, { name: "reject-tx-from-pooled-delegation", pending: 1, - run: func() error { - // Verify key[2] cannot originate another transaction when it has a pooled delegation. - if err := pool.addRemoteSync(setCodeTx(0, keys[0], []unsignedAuth{{0, keys[2]}})); err != nil { - return fmt.Errorf("failed to add with remote setcode transaction: %v", err) + run: func(name string) { + // Verify C cannot originate another transaction when it has a pooled delegation. + if err := pool.addRemoteSync(setCodeTx(0, keyA, []unsignedAuth{{0, keyC}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) } - if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1), keys[2])); !errors.Is(err, txpool.ErrAuthorityReserved) { - return fmt.Errorf("error mismatch: want %v, have %v", txpool.ErrAuthorityReserved, err) + if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1), keyC)); !errors.Is(err, txpool.ErrAuthorityReserved) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrAuthorityReserved, err) } // Also check gapped transaction is rejected. - if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keys[2])); !errors.Is(err, txpool.ErrAuthorityReserved) { - return fmt.Errorf("error mismatch: want %v, have %v", txpool.ErrAuthorityReserved, err) + if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyC)); !errors.Is(err, txpool.ErrAuthorityReserved) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, txpool.ErrAuthorityReserved, err) } - return nil }, }, { name: "replace-by-fee-setcode-tx", pending: 1, - run: func() error { + run: func(name string) { // 4. Fee bump the setcode tx send. - if err := pool.addRemoteSync(setCodeTx(0, keys[1], []unsignedAuth{{1, keys[2]}})); err != nil { - return fmt.Errorf("failed to add with remote setcode transaction: %v", err) + if err := pool.addRemoteSync(setCodeTx(0, keyB, []unsignedAuth{{1, keyC}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) } - if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(2000), uint256.NewInt(2), keys[1], []unsignedAuth{{0, keys[2]}})); err != nil { - t.Fatalf("failed to add with remote setcode transaction: %v", err) + if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(2000), uint256.NewInt(2), keyB, []unsignedAuth{{0, keyC}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) } - return nil }, }, { name: "allow-tx-from-replaced-authority", pending: 2, - run: func() error { + run: func(name string) { // Fee bump with a different auth list. Make sure that unlocks the authorities. - if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(10), uint256.NewInt(3), keys[0], []unsignedAuth{{0, keys[1]}})); err != nil { - t.Fatalf("failed to add with remote setcode transaction: %v", err) + if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(10), uint256.NewInt(3), keyA, []unsignedAuth{{0, keyB}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) } - if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(3000), uint256.NewInt(300), keys[0], []unsignedAuth{{0, keys[2]}})); err != nil { - t.Fatalf("failed to add with remote setcode transaction: %v", err) + if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(3000), uint256.NewInt(300), keyA, []unsignedAuth{{0, keyC}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) } - fmt.Println(pool.auths) - // Now send a regular tx from keys[1]. - if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(10), keys[1])); err != nil { - t.Fatalf("failed to replace with remote transaction: %v", err) + // Now send a regular tx from B. + if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(10), keyB)); err != nil { + t.Fatalf("%s: failed to replace with remote transaction: %v", name, err) + } + }, + }, + { + name: "allow-tx-from-replaced-self-sponsor-authority", + pending: 2, + run: func(name string) { + // + if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(10), uint256.NewInt(3), keyA, []unsignedAuth{{0, keyA}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) + } + if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(30), uint256.NewInt(30), keyA, []unsignedAuth{{0, keyB}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) + } + // Now send a regular tx from keyA. + if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keyA)); err != nil { + t.Fatalf("%s: failed to replace with remote transaction: %v", name, err) + } + // Make sure we can still send from keyB. + if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keyB)); err != nil { + t.Fatalf("%s: failed to replace with remote transaction: %v", name, err) + } + }, + }, + { + name: "track-multiple-conflicting-delegations", + pending: 2, + run: func(name string) { + // Send two setcode txs both with C as an authority. + if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(10), uint256.NewInt(3), keyA, []unsignedAuth{{0, keyC}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) + } + if err := pool.addRemoteSync(pricedSetCodeTx(0, 250000, uint256.NewInt(30), uint256.NewInt(30), keyB, []unsignedAuth{{0, keyC}})); err != nil { + t.Fatalf("%s: failed to add with remote setcode transaction: %v", name, err) + } + // Replace the tx from A with a non-setcode tx. + if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keyA)); err != nil { + t.Fatalf("%s: failed to replace with remote transaction: %v", name, err) + } + // Make sure we cannot send from keyC since it is still a pending authority. + if err, want := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keyC)), txpool.ErrAuthorityReserved; !errors.Is(err, want) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, want, err) } - return nil }, }, } { - if err := tt.run(); err != nil { - t.Fatalf("%s: %v", tt.name, err) - } + tt.run(tt.name) pending, queued := pool.Stats() if pending != tt.pending { t.Fatalf("%s: pending transactions mismatched: have %d, want %d", tt.name, pending, tt.pending) @@ -2349,16 +2387,6 @@ func TestSetCodeTransactions(t *testing.T) { } pool.Clear() } - - /* - - pool.Clear() - - - // if err := pool.addRemoteSync(setCodeTx(1, keys[3], []unsignedAuth{{1, keys[2]}})); !errors.Is(err, txpool.ErrAuthorityReserved) { - // t.Fatalf("expected to reject tx from in-flight authority: want %v, have %v", txpool.ErrAuthorityReserved, err) - // } - */ } // Benchmarks the speed of validating the contents of the pending queue of the