diff --git a/core/tx_pool.go b/core/tx_pool.go index b9828789cdd..0109d567767 100644 --- a/core/tx_pool.go +++ b/core/tx_pool.go @@ -79,10 +79,6 @@ var ( // with a different one without the required price bump. ErrReplaceUnderpriced = errors.New("replacement transaction underpriced") - // ErrAccountLimitExceeded is returned if a transaction would exceed the number - // allowed by a pool for a single account. - ErrAccountLimitExceeded = errors.New("account limit exceeded") - // ErrGasLimit is returned if a transaction's requested gas limit exceeds the // maximum allowance of the current block. ErrGasLimit = errors.New("exceeds block gas limit") @@ -96,6 +92,10 @@ var ( // making the transaction invalid, rather a DOS protection. ErrOversizedData = errors.New("oversized data") + // ErrInflightTxLimitReached is returned when the maximum number of in-flight + // transactions is reached for specific accounts. + ErrInflightTxLimitReached = errors.New("in-flight transaction limit reached for delegated accounts") + // ErrAuthorityReserved is returned if a transaction has an authorization // signed by an address which already has in-flight transactions known to the // pool. @@ -809,19 +809,8 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { if pool.currentState.GetBalance(from).Cmp(tx.Cost()) < 0 { return ErrInsufficientFunds } - list := pool.pending[from] - if list == nil || !list.Overlaps(tx) { - // Transaction takes a new nonce value out of the pool. Ensure it doesn't - // overflow the number of permitted transactions from a single account - // (i.e. max cancellable via out-of-bound transaction). - if used, left := usedAndLeftSlots(pool, from); left <= 0 { - return fmt.Errorf("%w: pooled %d txs", ErrAccountLimitExceeded, used) - } - // Verify no authorizations will invalidate existing transactions known to - // the pool. - if conflicts := knownConflicts(pool, tx.SetCodeAuthorities()); len(conflicts) > 0 { - return fmt.Errorf("%w: authorization conflicts with other known tx", ErrAuthorityReserved) - } + if err := pool.validateAuth(from, tx); err != nil { + return err } if tx.Type() == types.SetCodeTxType { if len(tx.SetCodeAuthorizations()) == 0 { @@ -1895,6 +1884,43 @@ func (pool *TxPool) calculateTxsLifecycle(txs types.Transactions, t time.Time) { } } +// validateAuth verifies that the transaction complies with code authorization +// restrictions brought by SetCode transaction type. +func (pool *TxPool) validateAuth(from common.Address, tx *types.Transaction) error { + // Allow at most one in-flight tx for delegated accounts or those with a + // pending authorization. + if pool.currentState.GetKeccakCodeHash(from) != codehash.EmptyKeccakCodeHash || len(pool.all.auths[from]) != 0 { + var ( + count int + exists bool + ) + pending := pool.pending[from] + if pending != nil { + count += pending.Len() + exists = pending.Overlaps(tx) + } + queue := pool.queue[from] + if queue != nil { + count += queue.Len() + exists = exists || queue.Overlaps(tx) + } + // Replace the existing in-flight transaction for delegated accounts + // are still supported + if count >= 1 && !exists { + return ErrInflightTxLimitReached + } + } + // Authorities cannot conflict with any pending or queued transactions. + if auths := tx.SetCodeAuthorities(); len(auths) > 0 { + for _, auth := range auths { + if pool.pending[auth] != nil || pool.queue[auth] != nil { + return ErrAuthorityReserved + } + } + } + return nil +} + // PauseReorgs stops any new reorg jobs to be started but doesn't interrupt any existing ones that are in flight // Keep in mind this function might block, although it is not expected to block for any significant amount of time func (pool *TxPool) PauseReorgs() { @@ -2132,7 +2158,6 @@ func (t *txLookup) Remove(hash common.Hash) { t.lock.Lock() defer t.lock.Unlock() - t.removeAuthorities(hash) tx, ok := t.locals[hash] if !ok { tx, ok = t.remotes[hash] @@ -2141,6 +2166,7 @@ func (t *txLookup) Remove(hash common.Hash) { log.Error("No transaction found to be deleted", "hash", hash) return } + t.removeAuthorities(tx) t.slots -= numSlots(tx) slotsGauge.Update(int64(t.slots)) @@ -2196,8 +2222,9 @@ func (t *txLookup) addAuthorities(tx *types.Transaction) { // removeAuthorities stops tracking the supplied tx in relation to its // authorities. -func (t *txLookup) removeAuthorities(hash common.Hash) { - for addr := range t.auths { +func (t *txLookup) 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 { @@ -2218,34 +2245,3 @@ func (t *txLookup) removeAuthorities(hash common.Hash) { func numSlots(tx *types.Transaction) int { return int((tx.Size() + txSlotSize - 1) / txSlotSize) } - -// usedAndLeftSlots returns the number of slots used and left for the given address. -func usedAndLeftSlots(pool *TxPool, addr common.Address) (int, int) { - var have int - if list := pool.pending[addr]; list != nil { - have += list.Len() - } - if list := pool.queue[addr]; list != nil { - have += list.Len() - } - if pool.currentState.GetKeccakCodeHash(addr) != codehash.EmptyKeccakCodeHash || len(pool.all.auths[addr]) != 0 { - // Allow at most one in-flight tx for delegated accounts or those with - // a pending authorization. - return have, max(0, 1-have) - } - return have, math.MaxInt -} - -// knownConflicts returns a list of addresses that conflict with the given authorities. -func knownConflicts(pool *TxPool, auths []common.Address) []common.Address { - var conflicts []common.Address - // Authorities cannot conflict with any pending or queued transactions. - for _, addr := range auths { - if list := pool.pending[addr]; list != nil { - conflicts = append(conflicts, addr) - } else if list := pool.queue[addr]; list != nil { - conflicts = append(conflicts, addr) - } - } - return conflicts -} diff --git a/core/tx_pool_test.go b/core/tx_pool_test.go index e97b8be96a2..f7d3b298db5 100644 --- a/core/tx_pool_test.go +++ b/core/tx_pool_test.go @@ -24,6 +24,7 @@ import ( "math/big" "math/rand" "os" + "slices" "sync/atomic" "testing" "time" @@ -156,6 +157,10 @@ func pricedSetCodeTx(nonce uint64, gaslimit uint64, gasFee, tip *uint256.Int, ke }) authList = append(authList, auth) } + return pricedSetCodeTxWithAuth(nonce, gaslimit, gasFee, tip, key, authList) +} + +func pricedSetCodeTxWithAuth(nonce uint64, gaslimit uint64, gasFee, tip *uint256.Int, key *ecdsa.PrivateKey, authList []types.SetCodeAuthorization) *types.Transaction { return types.MustSignNewTx(key, types.LatestSignerForChainID(params.TestChainConfig.ChainID), &types.SetCodeTx{ ChainID: uint256.MustFromBig(params.TestChainConfig.ChainID), Nonce: nonce, @@ -214,6 +219,34 @@ func validateTxPoolInternals(pool *TxPool) 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.locals { + 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()) + } + } + } + for _, tx := range pool.all.remotes { + 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 in locals or remotes. + for addr, hashes := range pool.all.auths { + for _, hash := range hashes { + _, inLocals := pool.all.locals[hash] + _, inRemotes := pool.all.remotes[hash] + + if !inLocals && !inRemotes { + return fmt.Errorf("dangling authority, missing originating tx: addr %s, hash %s", addr, hash.Hex()) + } + } + } return nil } @@ -2701,12 +2734,12 @@ func TestSetCodeTransactions(t *testing.T) { 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), keyA)); !errors.Is(err, ErrAccountLimitExceeded) { - t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrAccountLimitExceeded, err) + if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyA)); !errors.Is(err, ErrInflightTxLimitReached) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrInflightTxLimitReached, err) } // Also check gapped transaction. - if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1), keyA)); !errors.Is(err, ErrAccountLimitExceeded) { - t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrAccountLimitExceeded, err) + if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1), keyA)); !errors.Is(err, ErrInflightTxLimitReached) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrInflightTxLimitReached, err) } // Replace by fee. if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(10), keyA)); err != nil { @@ -2740,8 +2773,8 @@ func TestSetCodeTransactions(t *testing.T) { t.Fatalf("%s: failed to add with pending delegation: %v", name, err) } // Also check gapped transaction is rejected. - if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyC)); !errors.Is(err, ErrAccountLimitExceeded) { - t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrAccountLimitExceeded, err) + if err := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1), keyC)); !errors.Is(err, ErrInflightTxLimitReached) { + t.Fatalf("%s: error mismatch: want %v, have %v", name, ErrInflightTxLimitReached, err) } }, }, @@ -2815,7 +2848,7 @@ func TestSetCodeTransactions(t *testing.T) { if err := pool.addRemoteSync(pricedTransaction(0, 100000, big.NewInt(1000), keyC)); err != nil { t.Fatalf("%s: failed to added single pooled for account with pending delegation: %v", name, err) } - if err, want := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1000), keyC)), ErrAccountLimitExceeded; !errors.Is(err, want) { + if err, want := pool.addRemoteSync(pricedTransaction(1, 100000, big.NewInt(1000), keyC)), ErrInflightTxLimitReached; !errors.Is(err, want) { t.Fatalf("%s: error mismatch: want %v, have %v", name, want, err) } }, @@ -2846,6 +2879,32 @@ func TestSetCodeTransactions(t *testing.T) { } }, }, + { + name: "remove-hash-from-authority-tracker", + pending: 10, + run: func(name string, pool *TxPool, statedb *state.StateDB) { + 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) + } + }, + }, } { // Create the pool to test the status retrievals with statedb, _ := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) @@ -2875,6 +2934,64 @@ func TestSetCodeTransactions(t *testing.T) { } } +func TestSetCodeTransactionsReorg(t *testing.T) { + t.Parallel() + + // Create the pool to test the status retrievals with + statedb, _ := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + blockchain := &testBlockChain{1000000, statedb, new(event.Feed)} + + pool := NewTxPool(testTxPoolConfig, params.TestChainConfig, blockchain) + defer pool.Stop() + + // Create the test accounts + var ( + keyA, _ = crypto.GenerateKey() + addrA = crypto.PubkeyToAddress(keyA.PublicKey) + ) + testAddBalance(pool, addrA, big.NewInt(params.Ether)) + // Send an authorization for 0x42 + var authList []types.SetCodeAuthorization + auth, _ := types.SignSetCode(keyA, types.SetCodeAuthorization{ + ChainID: *uint256.MustFromBig(params.TestChainConfig.ChainID), + Address: common.Address{0x42}, + Nonce: 0, + }) + authList = append(authList, auth) + if err := pool.addRemoteSync(pricedSetCodeTxWithAuth(0, 250000, uint256.NewInt(10), uint256.NewInt(3), keyA, authList)); err != nil { + t.Fatalf("failed to add with remote setcode transaction: %v", err) + } + // Simulate the chain moving + blockchain.statedb.SetNonce(addrA, 1) + blockchain.statedb.SetCode(addrA, types.AddressToDelegation(auth.Address)) + <-pool.requestReset(nil, nil) + // Set an authorization for 0x00 + auth, _ = types.SignSetCode(keyA, types.SetCodeAuthorization{ + ChainID: *uint256.MustFromBig(params.TestChainConfig.ChainID), + Address: common.Address{}, + Nonce: 0, + }) + authList = append(authList, auth) + if err := pool.addRemoteSync(pricedSetCodeTxWithAuth(1, 250000, uint256.NewInt(10), uint256.NewInt(3), keyA, authList)); err != nil { + t.Fatalf("failed to add with remote setcode transaction: %v", err) + } + // Try to add a transactions in + if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1000), keyA)); !errors.Is(err, ErrInflightTxLimitReached) { + t.Fatalf("unexpected error %v, expecting %v", err, ErrInflightTxLimitReached) + } + // Simulate the chain moving + blockchain.statedb.SetNonce(addrA, 2) + blockchain.statedb.SetCode(addrA, nil) + <-pool.requestReset(nil, nil) + // Now send two transactions from addrA + if err := pool.addRemoteSync(pricedTransaction(2, 100000, big.NewInt(1000), keyA)); err != nil { + t.Fatalf("failed to added single transaction: %v", err) + } + if err := pool.addRemoteSync(pricedTransaction(3, 100000, big.NewInt(1000), keyA)); err != nil { + t.Fatalf("failed to added single transaction: %v", err) + } +} + // Benchmarks the speed of validating the contents of the pending queue of the // transaction pool. func BenchmarkPendingDemotion100(b *testing.B) { benchmarkPendingDemotion(b, 100) } diff --git a/core/types/transaction.go b/core/types/transaction.go index 6a5a50f8b28..4f88df833dd 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -477,15 +477,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) } } diff --git a/params/version.go b/params/version.go index 133999bae33..99a9625f3fa 100644 --- a/params/version.go +++ b/params/version.go @@ -24,7 +24,7 @@ import ( const ( VersionMajor = 5 // Major version component of the current release VersionMinor = 8 // Minor version component of the current release - VersionPatch = 27 // Patch version component of the current release + VersionPatch = 28 // Patch version component of the current release VersionMeta = "mainnet" // Version metadata to append to the version string )