From 34a5629e436da9206892f3e3f5a924eb518296b0 Mon Sep 17 00:00:00 2001 From: dwn1998 <42262393+dwn1998@users.noreply.github.com> Date: Mon, 5 Dec 2022 15:50:03 -0500 Subject: [PATCH 01/21] Update txpool.go Implemented DoS defense schemes --- core/txpool/txpool.go | 156 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 762210b7b74a..2c000b9e540f 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -87,6 +87,22 @@ var ( // than some meaningful limit a user might use. This is not a consensus error // making the transaction invalid, rather a DOS protection. ErrOversizedData = errors.New("oversized data") + + //[Warning] ED1, Discard Future tx when txpool is full + ErrRejFuture = errors.New("ED1, txpool is full, discard future tx") + + //[Warning] Discard low price tx when txpool is full + ErrRejLowPrice = errors.New("txpool is full, discard low price tx") + + //[Warning]ED3 + ErrRejVaildToFuture = errors.New("ED3, txpool is full, discard this tx that will change other vaild tx(s) to future") + + //[Warning]ED2 + ErrRejOverdraftWhenEvictOtherTx = errors.New("ED2, txpool is full, discard this tx because it will evict other vaild tx(s) and it will cause overdraft") + + //[Warning]ED4 + ErrRejOverdraftWhenReplaceOtherTx = errors.New("ED4, discard this tx becuase it will replace itself and then cause overdraft") + ) var ( @@ -678,8 +694,77 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e invalidTxMeter.Mark(1) return false, err } + + // ED4 + from, _ := types.Sender(pool.signer, tx) // already validated + if list := pool.pending[from]; list != nil && list.Overlaps(tx) { + // Nonce already pending, check if required price bump is met + //inserted, old := list.TryToAdd(tx, pool.config.PriceBump) + old := list.txs.Get(tx.Nonce()) + if old != nil { + + // new tx's fee and tip should both more than old one's fee and tip + if old.GasFeeCapCmp(tx) >= 0 || old.GasTipCapCmp(tx) >= 0 { + return false, ErrUnderpriced + } + // thresholdFeeCap = oldFC * (100 + priceBump) / 100 + a := big.NewInt(100 + int64(pool.config.PriceBump)) + aFeeCap := new(big.Int).Mul(a, old.GasFeeCap()) + aTip := a.Mul(a, old.GasTipCap()) + + // thresholdTip = oldTip * (100 + priceBump) / 100 + b := big.NewInt(100) + thresholdFeeCap := aFeeCap.Div(aFeeCap, b) + thresholdTip := aTip.Div(aTip, b) + + // We have to ensure that both the new fee cap and tip are higher than the + // old ones as well as checking the percentage threshold to ensure that + // this is accurate for low (Wei-level) gas price replacements. + if tx.GasFeeCapIntCmp(thresholdFeeCap) < 0 || tx.GasTipCapIntCmp(thresholdTip) < 0 { + return false, ErrUnderpriced + } + + //sum the pending txs spending + sum := big.NewInt(0) + for _, pendingTx := range list.Flatten() { + if pendingTx.Nonce() != tx.Nonce(){ + curVal := pendingTx.Value() + curGas := big.NewInt(int64(pendingTx.Gas())) + curGasFee := big.NewInt(1).Mul(pendingTx.GasPrice(),curGas) + curSum := big.NewInt(0) + curSum.Add(curVal, curGasFee) + log.Error("CURsum", curSum, tx.Cost()) + sum.Add(sum, curSum) + log.Error("[Warning], in pool tx info ", "value",curVal, "gas",curGas, "nonce", pendingTx.Nonce()) + } + } + + //pending tx + new tx total spending + log.Error("[Warning] ED4", "tx in pool total spending value", sum) + txVal := tx.Value() + txGas := big.NewInt(int64(tx.Gas())) + txGasFee := big.NewInt(1).Mul(tx.GasPrice(),txGas) + txSum := big.NewInt(0) + txSum.Add(txVal, txGasFee) + sum.Add(sum, txSum) + log.Error("[Warning] ED4", "tx in pool spending value + incoming spending",sum) + log.Error("[Warning] ED4", "incoming tx sender balance ",pool.currentState.GetBalance(from)) + log.Error("[Warning] ED4", "comparsion result ",sum.Cmp(pool.currentState.GetBalance(from))) + if sum.Cmp(pool.currentState.GetBalance(from)) > 0{ + return false, ErrRejOverdraftWhenReplaceOtherTx + } + } + } + + // If the transaction pool is full, discard underpriced transactions if uint64(pool.all.Slots()+numSlots(tx)) > pool.config.GlobalSlots+pool.config.GlobalQueue { + //[Warning] ED1: If the new transaction is a future transaction, discard it + from, _ := types.Sender(pool.signer, tx) + if pool.isFuture(from, tx) { + return false, ErrRejFuture + } + // If the new transaction is underpriced, don't accept it if !isLocal && pool.priced.Underpriced(tx) { log.Trace("Discarding underpriced transaction", "hash", hash, "gasTipCap", tx.GasTipCap(), "gasFeeCap", tx.GasFeeCap()) @@ -706,6 +791,58 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e overflowedTxMeter.Mark(1) return false, ErrTxPoolOverflow } + for _, dropTx := range drop { + //log.Error("[Warning] gasPrice",tx.GasPrice(),"drop gasPrice",*dropTx.GasPrice(), "drop nonce", *dropTx.Nonce()) + //if dropTx.GasPrice().Cmp(tx.GasPrice()) >= 0{ + // return false, ErrRejLowPrice + //} + + // ED3 + dropFrom, _ := types.Sender(pool.signer, dropTx) + log.Error("[Warning] ED3","pending pool curr nonce",pool.pendingNonces.get(dropFrom),"to dropTx nonce",dropTx.Nonce()) + if pool.pendingNonces.get(dropFrom) != (dropTx.Nonce() + 1){ + return false, ErrRejVaildToFuture + } + if from == dropFrom{ + isReplace = true + } + } + + // ED2 + if len(drop) != 0 { + if !isReplace { + list := pool.pending[from] + if list != nil { // Sender already has pending txs + sum := big.NewInt(0) + for _, pendingTx := range list.Flatten() { + curVal := pendingTx.Value() + curGas := big.NewInt(int64(pendingTx.Gas())) + curGasFee := big.NewInt(1).Mul(pendingTx.GasPrice(),curGas) + curSum := big.NewInt(0) + curSum.Add(curVal, curGasFee) + sum.Add(sum, curSum) + log.Error("[Warning] ED2, in pool tx info ", "value",curVal, "gas",curGas, "nonce", pendingTx.Nonce()) + } + log.Error("[Warning] ED2"," tx in pool total spending value",sum,) + txVal := tx.Value() + txGas := big.NewInt(int64(tx.Gas())) + txGasFee := big.NewInt(1).Mul(tx.GasPrice(),txGas) + txSum := big.NewInt(0) + txSum.Add(txVal, txGasFee) + sum.Add(sum, txSum) + log.Error("[Warning] ED2","tx in pool spending value + incoming spending",sum) + log.Error("[Warning] ED2","incoming tx sender balance ",pool.currentState.GetBalance(from)) + log.Error("[Warning] ED2", "comparsion result ",sum.Cmp(pool.currentState.GetBalance(from))) + if sum.Cmp(pool.currentState.GetBalance(from)) > 0{ + return false, ErrRejOverdraftWhenEvictOtherTx + } + } + } + //else { + + // } + } + // Bump the counter of rejections-since-reorg pool.changesSinceReorg += len(drop) // Kick out the underpriced remote transactions. @@ -760,6 +897,25 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e return replaced, nil } +func (pool *TxPool) isFuture(from common.Address, tx *types.Transaction) bool { + list := pool.pending[from] + if list != nil { // Sender already has pending txs + if old := list.txs.Get(tx.Nonce()); old != nil { // Replacing a pending, check bump + return false + } + // Not replacing, check if parent nonce exist + if list.txs.Get(tx.Nonce()-1) != nil { + return false + } + return true + } + // Sender has no pending + if pool.pendingNonces.get(from) == tx.Nonce() { + return false + } + return true + } + // enqueueTx inserts a new transaction into the non-executable transaction queue. // // Note, this method assumes the pool lock is held! From 72243e094b900cffb7c364ace35905d4d412228a Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 6 Dec 2022 18:05:30 +0100 Subject: [PATCH 02/21] core/txpool: add initial txpool redesign --- core/txpool/list.go | 2 + core/txpool/txpool.go | 205 ++++++++++-------------------------- core/txpool/txpool2_test.go | 197 ++++++++++++++++++++++++++++++++++ core/txpool/txpool_test.go | 4 +- 4 files changed, 256 insertions(+), 152 deletions(-) create mode 100644 core/txpool/txpool2_test.go diff --git a/core/txpool/list.go b/core/txpool/list.go index 062cbbf63e6a..d293aea18fbb 100644 --- a/core/txpool/list.go +++ b/core/txpool/list.go @@ -571,6 +571,7 @@ func (l *pricedList) Discard(slots int, force bool) (types.Transactions, bool) { tx := heap.Pop(&l.urgent).(*types.Transaction) if l.all.GetRemote(tx.Hash()) == nil { // Removed or migrated atomic.AddInt64(&l.stales, -1) + slots -= numSlots(tx) continue } // Non stale transaction found, move to floating heap @@ -584,6 +585,7 @@ func (l *pricedList) Discard(slots int, force bool) (types.Transactions, bool) { tx := heap.Pop(&l.floating).(*types.Transaction) if l.all.GetRemote(tx.Hash()) == nil { // Removed or migrated atomic.AddInt64(&l.stales, -1) + slots -= numSlots(tx) continue } // Non stale transaction found, discard it diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 2c000b9e540f..69c7711ffd38 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -88,21 +88,13 @@ var ( // making the transaction invalid, rather a DOS protection. ErrOversizedData = errors.New("oversized data") - //[Warning] ED1, Discard Future tx when txpool is full - ErrRejFuture = errors.New("ED1, txpool is full, discard future tx") - - //[Warning] Discard low price tx when txpool is full - ErrRejLowPrice = errors.New("txpool is full, discard low price tx") - - //[Warning]ED3 - ErrRejVaildToFuture = errors.New("ED3, txpool is full, discard this tx that will change other vaild tx(s) to future") - - //[Warning]ED2 - ErrRejOverdraftWhenEvictOtherTx = errors.New("ED2, txpool is full, discard this tx because it will evict other vaild tx(s) and it will cause overdraft") - - //[Warning]ED4 - ErrRejOverdraftWhenReplaceOtherTx = errors.New("ED4, discard this tx becuase it will replace itself and then cause overdraft") + // ErrFutureReplacePending is returned if a future transaction replaces a pending + // transaction. Future transactions should only be able to replace other future transactions. + ErrFutureReplacePending = errors.New("future transaction tries to replace pending") + // ErrOverdraft is returned if a transaction would cause the senders balance to go negative + // thus invalidating a potential large number of transactions. + ErrOverdraft = errors.New("transaction would cause overdraft") ) var ( @@ -694,76 +686,9 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e invalidTxMeter.Mark(1) return false, err } - - // ED4 from, _ := types.Sender(pool.signer, tx) // already validated - if list := pool.pending[from]; list != nil && list.Overlaps(tx) { - // Nonce already pending, check if required price bump is met - //inserted, old := list.TryToAdd(tx, pool.config.PriceBump) - old := list.txs.Get(tx.Nonce()) - if old != nil { - - // new tx's fee and tip should both more than old one's fee and tip - if old.GasFeeCapCmp(tx) >= 0 || old.GasTipCapCmp(tx) >= 0 { - return false, ErrUnderpriced - } - // thresholdFeeCap = oldFC * (100 + priceBump) / 100 - a := big.NewInt(100 + int64(pool.config.PriceBump)) - aFeeCap := new(big.Int).Mul(a, old.GasFeeCap()) - aTip := a.Mul(a, old.GasTipCap()) - - // thresholdTip = oldTip * (100 + priceBump) / 100 - b := big.NewInt(100) - thresholdFeeCap := aFeeCap.Div(aFeeCap, b) - thresholdTip := aTip.Div(aTip, b) - - // We have to ensure that both the new fee cap and tip are higher than the - // old ones as well as checking the percentage threshold to ensure that - // this is accurate for low (Wei-level) gas price replacements. - if tx.GasFeeCapIntCmp(thresholdFeeCap) < 0 || tx.GasTipCapIntCmp(thresholdTip) < 0 { - return false, ErrUnderpriced - } - - //sum the pending txs spending - sum := big.NewInt(0) - for _, pendingTx := range list.Flatten() { - if pendingTx.Nonce() != tx.Nonce(){ - curVal := pendingTx.Value() - curGas := big.NewInt(int64(pendingTx.Gas())) - curGasFee := big.NewInt(1).Mul(pendingTx.GasPrice(),curGas) - curSum := big.NewInt(0) - curSum.Add(curVal, curGasFee) - log.Error("CURsum", curSum, tx.Cost()) - sum.Add(sum, curSum) - log.Error("[Warning], in pool tx info ", "value",curVal, "gas",curGas, "nonce", pendingTx.Nonce()) - } - } - - //pending tx + new tx total spending - log.Error("[Warning] ED4", "tx in pool total spending value", sum) - txVal := tx.Value() - txGas := big.NewInt(int64(tx.Gas())) - txGasFee := big.NewInt(1).Mul(tx.GasPrice(),txGas) - txSum := big.NewInt(0) - txSum.Add(txVal, txGasFee) - sum.Add(sum, txSum) - log.Error("[Warning] ED4", "tx in pool spending value + incoming spending",sum) - log.Error("[Warning] ED4", "incoming tx sender balance ",pool.currentState.GetBalance(from)) - log.Error("[Warning] ED4", "comparsion result ",sum.Cmp(pool.currentState.GetBalance(from))) - if sum.Cmp(pool.currentState.GetBalance(from)) > 0{ - return false, ErrRejOverdraftWhenReplaceOtherTx - } - } - } - - // If the transaction pool is full, discard underpriced transactions if uint64(pool.all.Slots()+numSlots(tx)) > pool.config.GlobalSlots+pool.config.GlobalQueue { - //[Warning] ED1: If the new transaction is a future transaction, discard it - from, _ := types.Sender(pool.signer, tx) - if pool.isFuture(from, tx) { - return false, ErrRejFuture - } // If the new transaction is underpriced, don't accept it if !isLocal && pool.priced.Underpriced(tx) { @@ -771,6 +696,22 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e underpricedTxMeter.Mark(1) return false, ErrUnderpriced } + + // Verify that replacing transactions will not result in overdraft + list := pool.pending[from] + if list != nil { // Sender already has pending txs + sum := tx.Cost() + for _, pendingTx := range list.Flatten() { + if pendingTx.Nonce() != tx.Nonce() { + sum.Add(sum, pendingTx.Cost()) + } + } + if sum.Cmp(pool.currentState.GetBalance(from)) > 0 { + log.Trace("Replacing transactions would overdraft", "sender", from, "balance", pool.currentState.GetBalance(from), "required", sum) + return false, ErrOverdraft + } + } + // We're about to replace a transaction. The reorg does a more thorough // analysis of what to remove and how, but it runs async. We don't want to // do too many replacements between reorg-runs, so we cap the number of @@ -791,57 +732,21 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e overflowedTxMeter.Mark(1) return false, ErrTxPoolOverflow } - for _, dropTx := range drop { - //log.Error("[Warning] gasPrice",tx.GasPrice(),"drop gasPrice",*dropTx.GasPrice(), "drop nonce", *dropTx.Nonce()) - //if dropTx.GasPrice().Cmp(tx.GasPrice()) >= 0{ - // return false, ErrRejLowPrice - //} - - // ED3 - dropFrom, _ := types.Sender(pool.signer, dropTx) - log.Error("[Warning] ED3","pending pool curr nonce",pool.pendingNonces.get(dropFrom),"to dropTx nonce",dropTx.Nonce()) - if pool.pendingNonces.get(dropFrom) != (dropTx.Nonce() + 1){ - return false, ErrRejVaildToFuture - } - if from == dropFrom{ - isReplace = true - } - } - - // ED2 - if len(drop) != 0 { - if !isReplace { - list := pool.pending[from] - if list != nil { // Sender already has pending txs - sum := big.NewInt(0) - for _, pendingTx := range list.Flatten() { - curVal := pendingTx.Value() - curGas := big.NewInt(int64(pendingTx.Gas())) - curGasFee := big.NewInt(1).Mul(pendingTx.GasPrice(),curGas) - curSum := big.NewInt(0) - curSum.Add(curVal, curGasFee) - sum.Add(sum, curSum) - log.Error("[Warning] ED2, in pool tx info ", "value",curVal, "gas",curGas, "nonce", pendingTx.Nonce()) - } - log.Error("[Warning] ED2"," tx in pool total spending value",sum,) - txVal := tx.Value() - txGas := big.NewInt(int64(tx.Gas())) - txGasFee := big.NewInt(1).Mul(tx.GasPrice(),txGas) - txSum := big.NewInt(0) - txSum.Add(txVal, txGasFee) - sum.Add(sum, txSum) - log.Error("[Warning] ED2","tx in pool spending value + incoming spending",sum) - log.Error("[Warning] ED2","incoming tx sender balance ",pool.currentState.GetBalance(from)) - log.Error("[Warning] ED2", "comparsion result ",sum.Cmp(pool.currentState.GetBalance(from))) - if sum.Cmp(pool.currentState.GetBalance(from)) > 0{ - return false, ErrRejOverdraftWhenEvictOtherTx - } - } - } - //else { - - // } - } + + // If the new transaction is a future transaction it should never churn pending transactions + if pool.isFuture(from, tx) { + for _, dropTx := range drop { + dropSender, _ := types.Sender(pool.signer, dropTx) + if list := pool.pending[dropSender]; list != nil && list.Overlaps(dropTx) { + // Add all transactions back to the priced queue + for _, d := range drop { + pool.priced.urgent.Push(d) + } + log.Trace("Discarding future transaction replacing pending tx", "hash", hash) + return false, ErrFutureReplacePending + } + } + } // Bump the counter of rejections-since-reorg pool.changesSinceReorg += len(drop) @@ -852,8 +757,8 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e pool.removeTx(tx.Hash(), false) } } + // Try to replace an existing transaction in the pending pool - from, _ := types.Sender(pool.signer, tx) // already validated if list := pool.pending[from]; list != nil && list.Overlaps(tx) { // Nonce already pending, check if required price bump is met inserted, old := list.Add(tx, pool.config.PriceBump) @@ -898,23 +803,23 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e } func (pool *TxPool) isFuture(from common.Address, tx *types.Transaction) bool { - list := pool.pending[from] - if list != nil { // Sender already has pending txs - if old := list.txs.Get(tx.Nonce()); old != nil { // Replacing a pending, check bump - return false - } - // Not replacing, check if parent nonce exist - if list.txs.Get(tx.Nonce()-1) != nil { - return false - } - return true - } - // Sender has no pending - if pool.pendingNonces.get(from) == tx.Nonce() { - return false - } - return true - } + list := pool.pending[from] + if list != nil { // Sender already has pending txs + if old := list.txs.Get(tx.Nonce()); old != nil { // Replacing a pending, check bump + return false + } + // Not replacing, check if parent nonce exist + if list.txs.Get(tx.Nonce()-1) != nil { + return false + } + return true + } + // Sender has no pending + if pool.pendingNonces.get(from) == tx.Nonce() { + return false + } + return true +} // enqueueTx inserts a new transaction into the non-executable transaction queue. // diff --git a/core/txpool/txpool2_test.go b/core/txpool/txpool2_test.go new file mode 100644 index 000000000000..662bf88fb641 --- /dev/null +++ b/core/txpool/txpool2_test.go @@ -0,0 +1,197 @@ +package txpool + +import ( + "crypto/ecdsa" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/core/state" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/event" +) + +func pricedValuedTransaction(nonce uint64, value int64, gaslimit uint64, gasprice *big.Int, key *ecdsa.PrivateKey) *types.Transaction { + tx, _ := types.SignTx(types.NewTransaction(nonce, common.Address{}, big.NewInt(value), gaslimit, gasprice, nil), types.HomesteadSigner{}, key) + return tx +} + +func count(t *testing.T, pool *TxPool) (pending int, queued int) { + t.Helper() + pending, queued = pool.stats() + if err := validatePoolInternals(pool); err != nil { + t.Fatalf("pool internal state corrupted: %v", err) + } + return pending, queued +} + +func fillPool(t *testing.T, pool *TxPool) { + t.Helper() + // Create a number of test accounts, fund them and make transactions + executableTxs := types.Transactions{} + nonExecutableTxs := types.Transactions{} + for i := 0; i < 384; i++ { + key, _ := crypto.GenerateKey() + pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(10000000000)) + // Add executable ones + for j := 0; j < int(pool.config.AccountSlots); j++ { + executableTxs = append(executableTxs, pricedTransaction(uint64(j), 100000, big.NewInt(300), key)) + } + } + // Import the batch and verify that limits have been enforced + pool.AddRemotesSync(executableTxs) + pool.AddRemotesSync(nonExecutableTxs) + pending, queued := pool.Stats() + slots := pool.all.Slots() + // sanity-check that the test prerequisites are ok (pending full) + if have, want := pending, slots; have != want { + t.Fatalf("have %d, want %d", have, want) + } + if have, want := queued, 0; have != want { + t.Fatalf("have %d, want %d", have, want) + } + + t.Logf("pool.config: GlobalSlots=%d, GlobalQueue=%d\n", pool.config.GlobalSlots, pool.config.GlobalQueue) + t.Logf("pending: %d queued: %d, all: %d\n", pending, queued, slots) +} + +// Tests that if a batch high-priced of non-executables arrive, they do not kick out +// executable transactions +func TestTransactionFutureAttack(t *testing.T) { + t.Parallel() + + // Create the pool to test the limit enforcement with + statedb, _ := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + blockchain := &testBlockChain{1000000, statedb, new(event.Feed)} + config := testTxPoolConfig + config.GlobalQueue = 100 + config.GlobalSlots = 100 + pool := NewTxPool(config, eip1559Config, blockchain) + defer pool.Stop() + fillPool(t, pool) + pending, _ := pool.Stats() + // Now, future transaction attack starts, let's add a bunch of expensive non-executables, and see if the pending-count drops + { + key, _ := crypto.GenerateKey() + pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000)) + futureTxs := types.Transactions{} + for j := 0; j < int(pool.config.GlobalSlots+pool.config.GlobalQueue); j++ { + futureTxs = append(futureTxs, pricedTransaction(1000+uint64(j), 100000, big.NewInt(500), key)) + } + for i := 0; i < 5; i++ { + pool.AddRemotesSync(futureTxs) + newPending, newQueued := count(t, pool) + t.Logf("pending: %d queued: %d, all: %d\n", newPending, newQueued, pool.all.Slots()) + } + } + newPending, _ := pool.Stats() + // Pending should not have been touched + if have, want := newPending, pending; have < want { + t.Errorf("wrong pending-count, have %d, want %d (GlobalSlots: %d)", + have, want, pool.config.GlobalSlots) + } +} + +// Tests that if a batch high-priced of non-executables arrive, they do not kick out +// executable transactions +func TestTransactionFuture1559(t *testing.T) { + t.Parallel() + // Create the pool to test the pricing enforcement with + statedb, _ := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + blockchain := &testBlockChain{1000000, statedb, new(event.Feed)} + pool := NewTxPool(testTxPoolConfig, eip1559Config, blockchain) + defer pool.Stop() + + // Create a number of test accounts, fund them and make transactions + fillPool(t, pool) + pending, _ := pool.Stats() + + // Now, future transaction attack starts, let's add a bunch of expensive non-executables, and see if the pending-count drops + { + key, _ := crypto.GenerateKey() + pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000)) + futureTxs := types.Transactions{} + for j := 0; j < int(pool.config.GlobalSlots+pool.config.GlobalQueue); j++ { + futureTxs = append(futureTxs, dynamicFeeTx(1000+uint64(j), 100000, big.NewInt(200), big.NewInt(101), key)) + } + pool.AddRemotesSync(futureTxs) + } + newPending, _ := pool.Stats() + // Pending should not have been touched + if have, want := newPending, pending; have != want { + t.Errorf("Wrong pending-count, have %d, want %d (GlobalSlots: %d)", + have, want, pool.config.GlobalSlots) + } +} + +// Tests that if a batch of balance-overdraft txs arrive, they do not kick out +// executable transactions +func TestTransactionZAttack(t *testing.T) { + t.Parallel() + // Create the pool to test the pricing enforcement with + statedb, _ := state.New(common.Hash{}, state.NewDatabase(rawdb.NewMemoryDatabase()), nil) + blockchain := &testBlockChain{1000000, statedb, new(event.Feed)} + pool := NewTxPool(testTxPoolConfig, eip1559Config, blockchain) + defer pool.Stop() + // Create a number of test accounts, fund them and make transactions + fillPool(t, pool) + + countInvalidPending := func() int { + t.Helper() + var ivpendingNum int + pendingtxs, _ := pool.Content() + for account, txs := range pendingtxs { + cur_balance := new(big.Int).Set(pool.currentState.GetBalance(account)) + for _, tx := range txs { + if cur_balance.Cmp(tx.Value()) <= 0 { + ivpendingNum++ + } else { + cur_balance.Sub(cur_balance, tx.Value()) + } + } + } + if err := validatePoolInternals(pool); err != nil { + t.Fatalf("pool internal state corrupted: %v", err) + } + return ivpendingNum + } + ivPending := countInvalidPending() + t.Logf("invalid pending: %d\n", ivPending) + + // Now, DETER-Z attack starts, let's add a bunch of expensive non-executables (from N accounts) along with balance-overdraft txs (from one account), and see if the pending-count drops + for j := 0; j < int(pool.config.GlobalQueue); j++ { + futureTxs := types.Transactions{} + key, _ := crypto.GenerateKey() + pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000)) + futureTxs = append(futureTxs, pricedTransaction(1000+uint64(j), 21000, big.NewInt(500), key)) + pool.AddRemotesSync(futureTxs) + } + + overDraftTxs := types.Transactions{} + { + key, _ := crypto.GenerateKey() + pool.currentState.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(100000000000)) + for j := 0; j < int(pool.config.GlobalSlots); j++ { + overDraftTxs = append(overDraftTxs, pricedValuedTransaction(uint64(j), 60000000000, 21000, big.NewInt(500), key)) + } + } + pool.AddRemotesSync(overDraftTxs) + pool.AddRemotesSync(overDraftTxs) + pool.AddRemotesSync(overDraftTxs) + pool.AddRemotesSync(overDraftTxs) + pool.AddRemotesSync(overDraftTxs) + + newPending, newQueued := count(t, pool) + newIvPending := countInvalidPending() + t.Logf("pool.all.Slots(): %d\n", pool.all.Slots()) + t.Logf("pending: %d queued: %d, all: %d\n", newPending, newQueued, pool.all.Slots()) + t.Logf("invalid pending: %d\n", newIvPending) + + // Pending should not have been touched + if newIvPending != ivPending { + t.Errorf("Wrong invalid pending-count, have %d, want %d (GlobalSlots: %d, queued: %d)", + newIvPending, ivPending, pool.config.GlobalSlots, newQueued) + } +} diff --git a/core/txpool/txpool_test.go b/core/txpool/txpool_test.go index 237f97afe434..07534d002b6e 100644 --- a/core/txpool/txpool_test.go +++ b/core/txpool/txpool_test.go @@ -1663,7 +1663,7 @@ func TestUnderpricing(t *testing.T) { defer sub.Unsubscribe() // Create a number of test accounts and fund them - keys := make([]*ecdsa.PrivateKey, 4) + keys := make([]*ecdsa.PrivateKey, 5) for i := 0; i < len(keys); i++ { keys[i], _ = crypto.GenerateKey() testAddBalance(pool, crypto.PubkeyToAddress(keys[i].PublicKey), big.NewInt(1000000)) @@ -1710,7 +1710,7 @@ func TestUnderpricing(t *testing.T) { t.Fatalf("failed to add well priced transaction: %v", err) } pending, queued = pool.Stats() - if pending != 2 { + if pending != 4 { t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 2) } if queued != 2 { From 7676a284f65c49540c11e9cfd1fc3d02ff4429c7 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Tue, 6 Dec 2022 18:17:50 +0100 Subject: [PATCH 03/21] core/txpool: fix tests --- core/txpool/txpool_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/core/txpool/txpool_test.go b/core/txpool/txpool_test.go index 07534d002b6e..b84249045efb 100644 --- a/core/txpool/txpool_test.go +++ b/core/txpool/txpool_test.go @@ -1699,6 +1699,10 @@ func TestUnderpricing(t *testing.T) { if err := pool.AddRemote(pricedTransaction(0, 100000, big.NewInt(1), keys[1])); err != ErrUnderpriced { t.Fatalf("adding underpriced pending transaction error mismatch: have %v, want %v", err, ErrUnderpriced) } + // Replace a future transaction with a future transaction + if err := pool.AddRemote(pricedTransaction(1, 100000, big.NewInt(2), keys[1])); err != nil { // +K1:1 => -K1:1 => Pend K0:0, K0:1, K2:0; Que K1:1 + t.Fatalf("failed to add well priced transaction: %v", err) + } // Ensure that adding high priced transactions drops cheap ones, but not own if err := pool.AddRemote(pricedTransaction(0, 100000, big.NewInt(3), keys[1])); err != nil { // +K1:0 => -K1:1 => Pend K0:0, K0:1, K1:0, K2:0; Que - t.Fatalf("failed to add well priced transaction: %v", err) @@ -1709,14 +1713,18 @@ func TestUnderpricing(t *testing.T) { if err := pool.AddRemote(pricedTransaction(3, 100000, big.NewInt(5), keys[1])); err != nil { // +K1:3 => -K0:1 => Pend K1:0, K2:0; Que K1:2 K1:3 t.Fatalf("failed to add well priced transaction: %v", err) } + // Ensure that replacing a pending transaction with a future transaction fails + if err := pool.AddRemote(pricedTransaction(5, 100000, big.NewInt(6), keys[1])); err != ErrFutureReplacePending { + t.Fatalf("adding future replace transaction error mismatch: have %v, want %v", err, ErrFutureReplacePending) + } pending, queued = pool.Stats() - if pending != 4 { + if pending != 2 { t.Fatalf("pending transactions mismatched: have %d, want %d", pending, 2) } if queued != 2 { t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 2) } - if err := validateEvents(events, 1); err != nil { + if err := validateEvents(events, 2); err != nil { t.Fatalf("additional event firing failed: %v", err) } if err := validatePoolInternals(pool); err != nil { @@ -1878,11 +1886,11 @@ func TestUnderpricingDynamicFee(t *testing.T) { t.Fatalf("failed to add well priced transaction: %v", err) } - tx = pricedTransaction(2, 100000, big.NewInt(3), keys[1]) + tx = pricedTransaction(1, 100000, big.NewInt(3), keys[1]) if err := pool.AddRemote(tx); err != nil { // +K1:2, -K0:1 => Pend K0:0 K1:0, K2:0; Que K1:2 t.Fatalf("failed to add well priced transaction: %v", err) } - tx = dynamicFeeTx(3, 100000, big.NewInt(4), big.NewInt(1), keys[1]) + tx = dynamicFeeTx(2, 100000, big.NewInt(4), big.NewInt(1), keys[1]) if err := pool.AddRemote(tx); err != nil { // +K1:3, -K1:0 => Pend K0:0 K2:0; Que K1:2 K1:3 t.Fatalf("failed to add well priced transaction: %v", err) } @@ -1893,7 +1901,7 @@ func TestUnderpricingDynamicFee(t *testing.T) { if queued != 2 { t.Fatalf("queued transactions mismatched: have %d, want %d", queued, 2) } - if err := validateEvents(events, 1); err != nil { + if err := validateEvents(events, 2); err != nil { t.Fatalf("additional event firing failed: %v", err) } if err := validatePoolInternals(pool); err != nil { From 050c7125d5de5f3f1f0f60dc3359e1e33c63a441 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 3 Feb 2023 11:52:36 +0100 Subject: [PATCH 04/21] core/txpool: move totalcost into sender list --- core/txpool/list.go | 44 ++++++++++++++++++++++++++++++-------- core/txpool/txpool.go | 7 +----- core/txpool/txpool_test.go | 3 +++ 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/core/txpool/list.go b/core/txpool/list.go index d293aea18fbb..aa0ecdb9387b 100644 --- a/core/txpool/list.go +++ b/core/txpool/list.go @@ -254,17 +254,19 @@ type list struct { strict bool // Whether nonces are strictly continuous or not txs *sortedMap // Heap indexed sorted hash map of the transactions - costcap *big.Int // Price of the highest costing transaction (reset only if exceeds balance) - gascap uint64 // Gas limit of the highest spending transaction (reset only if exceeds block limit) + costcap *big.Int // Price of the highest costing transaction (reset only if exceeds balance) + gascap uint64 // Gas limit of the highest spending transaction (reset only if exceeds block limit) + totalcost *big.Int // Total cost of all transactions in the list } // newList create a new transaction list for maintaining nonce-indexable fast, // gapped, sortable transaction lists. func newList(strict bool) *list { return &list{ - strict: strict, - txs: newSortedMap(), - costcap: new(big.Int), + strict: strict, + txs: newSortedMap(), + costcap: new(big.Int), + totalcost: new(big.Int), } } @@ -302,7 +304,11 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64) (bool, *types.Transa if tx.GasFeeCapIntCmp(thresholdFeeCap) < 0 || tx.GasTipCapIntCmp(thresholdTip) < 0 { return false, nil } + // Old is being replaced, substract old cost + l.subTotalCost(old) } + // Add new tx cost to totalcost + l.totalcost.Add(l.totalcost, tx.Cost()) // Otherwise overwrite the old transaction with the current one l.txs.Put(tx) if cost := tx.Cost(); l.costcap.Cmp(cost) < 0 { @@ -318,7 +324,9 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64) (bool, *types.Transa // provided threshold. Every removed transaction is returned for any post-removal // maintenance. func (l *list) Forward(threshold uint64) types.Transactions { - return l.txs.Forward(threshold) + txs := l.txs.Forward(threshold) + l.subTotalCost(txs...) + return txs } // Filter removes all transactions from the list with a cost or gas limit higher @@ -357,6 +365,8 @@ func (l *list) Filter(costLimit *big.Int, gasLimit uint64) (types.Transactions, } invalids = l.txs.filter(func(tx *types.Transaction) bool { return tx.Nonce() > lowest }) } + // Reset total cost + l.subTotalCost(append(removed, invalids...)...) l.txs.reheap() return removed, invalids } @@ -364,7 +374,9 @@ func (l *list) Filter(costLimit *big.Int, gasLimit uint64) (types.Transactions, // Cap places a hard limit on the number of items, returning all transactions // exceeding that limit. func (l *list) Cap(threshold int) types.Transactions { - return l.txs.Cap(threshold) + txs := l.txs.Cap(threshold) + l.subTotalCost(txs...) + return txs } // Remove deletes a transaction from the maintained list, returning whether the @@ -376,9 +388,12 @@ func (l *list) Remove(tx *types.Transaction) (bool, types.Transactions) { if removed := l.txs.Remove(nonce); !removed { return false, nil } + l.subTotalCost(tx) // In strict mode, filter out non-executable transactions if l.strict { - return true, l.txs.Filter(func(tx *types.Transaction) bool { return tx.Nonce() > nonce }) + txs := l.txs.Filter(func(tx *types.Transaction) bool { return tx.Nonce() > nonce }) + l.subTotalCost(txs...) + return true, txs } return true, nil } @@ -391,7 +406,9 @@ func (l *list) Remove(tx *types.Transaction) (bool, types.Transactions) { // prevent getting into and invalid state. This is not something that should ever // happen but better to be self correcting than failing! func (l *list) Ready(start uint64) types.Transactions { - return l.txs.Ready(start) + txs := l.txs.Ready(start) + l.subTotalCost(txs...) + return txs } // Len returns the length of the transaction list. @@ -417,6 +434,14 @@ func (l *list) LastElement() *types.Transaction { return l.txs.LastElement() } +// subTotalCost substracts the cost of the given transactions from the +// total cost of all transactions. +func (l *list) subTotalCost(txs ...*types.Transaction) { + for _, tx := range txs { + l.totalcost.Sub(l.totalcost, tx.Cost()) + } +} + // priceHeap is a heap.Interface implementation over transactions for retrieving // price-sorted transactions to discard when the pool fills up. If baseFee is set // then the heap is sorted based on the effective tip based on the given base fee. @@ -561,6 +586,7 @@ func (l *pricedList) underpricedFor(h *priceHeap, tx *types.Transaction) bool { // Discard finds a number of most underpriced transactions, removes them from the // priced list and returns them for further removal from the entire pool. +// If noPending is set to true, we will only consider the floating list // // Note local transaction won't be considered for eviction. func (l *pricedList) Discard(slots int, force bool) (types.Transactions, bool) { diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 69c7711ffd38..2acc26cce9db 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -700,12 +700,7 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e // Verify that replacing transactions will not result in overdraft list := pool.pending[from] if list != nil { // Sender already has pending txs - sum := tx.Cost() - for _, pendingTx := range list.Flatten() { - if pendingTx.Nonce() != tx.Nonce() { - sum.Add(sum, pendingTx.Cost()) - } - } + sum := new(big.Int).Add(tx.Cost(), list.totalcost) if sum.Cmp(pool.currentState.GetBalance(from)) > 0 { log.Trace("Replacing transactions would overdraft", "sender", from, "balance", pool.currentState.GetBalance(from), "required", sum) return false, ErrOverdraft diff --git a/core/txpool/txpool_test.go b/core/txpool/txpool_test.go index b84249045efb..42b3db620d0b 100644 --- a/core/txpool/txpool_test.go +++ b/core/txpool/txpool_test.go @@ -158,6 +158,9 @@ func validatePoolInternals(pool *TxPool) error { if nonce := pool.pendingNonces.get(addr); nonce != last+1 { return fmt.Errorf("pending nonce mismatch: have %v, want %v", nonce, last+1) } + if txs.totalcost.Cmp(common.Big0) < 0 { + return fmt.Errorf("totalcost went negative: %v", txs.totalcost) + } } return nil } From 892332ee466e5c15d557216e58f1b905d471c770 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 3 Feb 2023 13:15:39 +0100 Subject: [PATCH 05/21] core/txpool: give sender enough funds for benchmark --- core/txpool/txpool_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/txpool/txpool_test.go b/core/txpool/txpool_test.go index 42b3db620d0b..dc7acd522d4d 100644 --- a/core/txpool/txpool_test.go +++ b/core/txpool/txpool_test.go @@ -2498,7 +2498,7 @@ func benchmarkBatchInsert(b *testing.B, size int, local bool) { defer pool.Stop() account := crypto.PubkeyToAddress(key.PublicKey) - testAddBalance(pool, account, big.NewInt(1000000)) + testAddBalance(pool, account, big.NewInt(1000000000000000000)) batches := make([]types.Transactions, b.N) for i := 0; i < b.N; i++ { From 07a97dc53942b268123b95054db7bdcb8aaad018 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 3 Feb 2023 13:33:12 +0100 Subject: [PATCH 06/21] core/txpool: fix edge case on replacement txs --- core/txpool/txpool.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 2acc26cce9db..b1aef24c177b 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -701,6 +701,10 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e list := pool.pending[from] if list != nil { // Sender already has pending txs sum := new(big.Int).Add(tx.Cost(), list.totalcost) + if repl := list.txs.Get(tx.Nonce()); repl != nil { + // Deduct the cost of a transaction replaced by this + sum.Sub(sum, repl.Cost()) + } if sum.Cmp(pool.currentState.GetBalance(from)) > 0 { log.Trace("Replacing transactions would overdraft", "sender", from, "balance", pool.currentState.GetBalance(from), "required", sum) return false, ErrOverdraft From e2259e9eaaf09baf9124a85a6ee7e23bdca4d818 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 8 Feb 2023 15:07:01 +0100 Subject: [PATCH 07/21] core/txpool: move overdraft check --- core/txpool/txpool.go | 29 +++++++++++++++-------------- core/txpool/txpool_test.go | 4 ++-- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index b1aef24c177b..bb0f0e6b2ff9 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -686,7 +686,22 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e invalidTxMeter.Mark(1) return false, err } + + // Verify that replacing transactions will not result in overdraft from, _ := types.Sender(pool.signer, tx) // already validated + list := pool.pending[from] + if list != nil { // Sender already has pending txs + sum := new(big.Int).Add(tx.Cost(), list.totalcost) + if repl := list.txs.Get(tx.Nonce()); repl != nil { + // Deduct the cost of a transaction replaced by this + sum.Sub(sum, repl.Cost()) + } + if sum.Cmp(pool.currentState.GetBalance(from)) > 0 { + log.Trace("Replacing transactions would overdraft", "sender", from, "balance", pool.currentState.GetBalance(from), "required", sum) + return false, ErrOverdraft + } + } + // If the transaction pool is full, discard underpriced transactions if uint64(pool.all.Slots()+numSlots(tx)) > pool.config.GlobalSlots+pool.config.GlobalQueue { @@ -697,20 +712,6 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e return false, ErrUnderpriced } - // Verify that replacing transactions will not result in overdraft - list := pool.pending[from] - if list != nil { // Sender already has pending txs - sum := new(big.Int).Add(tx.Cost(), list.totalcost) - if repl := list.txs.Get(tx.Nonce()); repl != nil { - // Deduct the cost of a transaction replaced by this - sum.Sub(sum, repl.Cost()) - } - if sum.Cmp(pool.currentState.GetBalance(from)) > 0 { - log.Trace("Replacing transactions would overdraft", "sender", from, "balance", pool.currentState.GetBalance(from), "required", sum) - return false, ErrOverdraft - } - } - // We're about to replace a transaction. The reorg does a more thorough // analysis of what to remove and how, but it runs async. We don't want to // do too many replacements between reorg-runs, so we cap the number of diff --git a/core/txpool/txpool_test.go b/core/txpool/txpool_test.go index dc7acd522d4d..c4c62db2d6da 100644 --- a/core/txpool/txpool_test.go +++ b/core/txpool/txpool_test.go @@ -1108,7 +1108,7 @@ func TestPendingLimiting(t *testing.T) { defer pool.Stop() account := crypto.PubkeyToAddress(key.PublicKey) - testAddBalance(pool, account, big.NewInt(1000000)) + testAddBalance(pool, account, big.NewInt(1000000000000)) // Keep track of transaction events to ensure all executables get announced events := make(chan core.NewTxsEvent, testTxPoolConfig.AccountQueue+5) @@ -1587,7 +1587,7 @@ func TestRepricingKeepsLocals(t *testing.T) { keys := make([]*ecdsa.PrivateKey, 3) for i := 0; i < len(keys); i++ { keys[i], _ = crypto.GenerateKey() - testAddBalance(pool, crypto.PubkeyToAddress(keys[i].PublicKey), big.NewInt(1000*1000000)) + testAddBalance(pool, crypto.PubkeyToAddress(keys[i].PublicKey), big.NewInt(100000*1000000)) } // Create transaction (both pending and queued) with a linearly growing gasprice for i := uint64(0); i < 500; i++ { From ec012b978f8966d1fd7a724700b42b40e24bcdbc Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 8 Feb 2023 16:32:18 +0100 Subject: [PATCH 08/21] core/txpool: calculate churn more correctly --- core/txpool/txpool.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index bb0f0e6b2ff9..ab454bc105ef 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -754,7 +754,8 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e for _, tx := range drop { log.Trace("Discarding freshly underpriced transaction", "hash", tx.Hash(), "gasTipCap", tx.GasTipCap(), "gasFeeCap", tx.GasFeeCap()) underpricedTxMeter.Mark(1) - pool.removeTx(tx.Hash(), false) + dropped := pool.removeTx(tx.Hash(), false) + pool.changesSinceReorg += dropped } } @@ -1057,11 +1058,12 @@ func (pool *TxPool) Has(hash common.Hash) bool { // removeTx removes a single transaction from the queue, moving all subsequent // transactions back to the future queue. -func (pool *TxPool) removeTx(hash common.Hash, outofbound bool) { +// Returns the number of transactions moved to the future queue. +func (pool *TxPool) removeTx(hash common.Hash, outofbound bool) int { // Fetch the transaction we wish to delete tx := pool.all.Get(hash) if tx == nil { - return + return 0 } addr, _ := types.Sender(pool.signer, tx) // already validated during insertion @@ -1089,7 +1091,7 @@ func (pool *TxPool) removeTx(hash common.Hash, outofbound bool) { pool.pendingNonces.setIfLower(addr, tx.Nonce()) // Reduce the pending counter pendingGauge.Dec(int64(1 + len(invalids))) - return + return len(invalids) } } // Transaction is in the future queue @@ -1103,6 +1105,7 @@ func (pool *TxPool) removeTx(hash common.Hash, outofbound bool) { delete(pool.beats, addr) } } + return 0 } // requestReset requests a pool reset to the new head block. From e6328cd393de8b5b3c413139cf8dc00fd8e9b58b Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Feb 2023 10:10:04 +0100 Subject: [PATCH 09/21] core/txpool: happy lint --- core/txpool/list.go | 4 ++-- core/txpool/txpool.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/txpool/list.go b/core/txpool/list.go index aa0ecdb9387b..73ca8ddaa0ea 100644 --- a/core/txpool/list.go +++ b/core/txpool/list.go @@ -304,7 +304,7 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64) (bool, *types.Transa if tx.GasFeeCapIntCmp(thresholdFeeCap) < 0 || tx.GasTipCapIntCmp(thresholdTip) < 0 { return false, nil } - // Old is being replaced, substract old cost + // Old is being replaced, subtract old cost l.subTotalCost(old) } // Add new tx cost to totalcost @@ -434,7 +434,7 @@ func (l *list) LastElement() *types.Transaction { return l.txs.LastElement() } -// subTotalCost substracts the cost of the given transactions from the +// subTotalCost subtracts the cost of the given transactions from the // total cost of all transactions. func (l *list) subTotalCost(txs ...*types.Transaction) { for _, tx := range txs { diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index ab454bc105ef..92ae9d9fd444 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -704,7 +704,6 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e // If the transaction pool is full, discard underpriced transactions if uint64(pool.all.Slots()+numSlots(tx)) > pool.config.GlobalSlots+pool.config.GlobalQueue { - // If the new transaction is underpriced, don't accept it if !isLocal && pool.priced.Underpriced(tx) { log.Trace("Discarding underpriced transaction", "hash", hash, "gasTipCap", tx.GasTipCap(), "gasFeeCap", tx.GasFeeCap()) From e5b2889f1c5aff35c21935ed15b4fad8b45bd502 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 11:37:27 +0100 Subject: [PATCH 10/21] core/txpool: move checks, make checks less costly --- core/txpool/txpool.go | 81 ++++++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 92ae9d9fd444..e58dae204487 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -17,6 +17,7 @@ package txpool import ( + "container/heap" "errors" "fmt" "math" @@ -595,70 +596,86 @@ func (pool *TxPool) local() map[common.Address]types.Transactions { // validateTx checks whether a transaction is valid according to the consensus // rules and adheres to some heuristic limits of the local node (price and size). -func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { +func (pool *TxPool) validateTx(tx *types.Transaction, local bool) (common.Address, error) { // Accept only legacy transactions until EIP-2718/2930 activates. if !pool.eip2718 && tx.Type() != types.LegacyTxType { - return core.ErrTxTypeNotSupported + return common.Address{}, core.ErrTxTypeNotSupported } // Reject dynamic fee transactions until EIP-1559 activates. if !pool.eip1559 && tx.Type() == types.DynamicFeeTxType { - return core.ErrTxTypeNotSupported + return common.Address{}, core.ErrTxTypeNotSupported } // Reject transactions over defined size to prevent DOS attacks if tx.Size() > txMaxSize { - return ErrOversizedData + return common.Address{}, ErrOversizedData } // Check whether the init code size has been exceeded. if pool.shanghai && tx.To() == nil && len(tx.Data()) > params.MaxInitCodeSize { - return fmt.Errorf("%w: code size %v limit %v", core.ErrMaxInitCodeSizeExceeded, len(tx.Data()), params.MaxInitCodeSize) + return common.Address{}, fmt.Errorf("%w: code size %v limit %v", core.ErrMaxInitCodeSizeExceeded, len(tx.Data()), params.MaxInitCodeSize) } // Transactions can't be negative. This may never happen using RLP decoded // transactions but may occur if you create a transaction using the RPC. if tx.Value().Sign() < 0 { - return ErrNegativeValue + return common.Address{}, ErrNegativeValue } // Ensure the transaction doesn't exceed the current block limit gas. if pool.currentMaxGas < tx.Gas() { - return ErrGasLimit + return common.Address{}, ErrGasLimit } // Sanity check for extremely large numbers if tx.GasFeeCap().BitLen() > 256 { - return core.ErrFeeCapVeryHigh + return common.Address{}, core.ErrFeeCapVeryHigh } if tx.GasTipCap().BitLen() > 256 { - return core.ErrTipVeryHigh + return common.Address{}, core.ErrTipVeryHigh } // Ensure gasFeeCap is greater than or equal to gasTipCap. if tx.GasFeeCapIntCmp(tx.GasTipCap()) < 0 { - return core.ErrTipAboveFeeCap + return common.Address{}, core.ErrTipAboveFeeCap } // Make sure the transaction is signed properly. from, err := types.Sender(pool.signer, tx) if err != nil { - return ErrInvalidSender + return common.Address{}, ErrInvalidSender } // Drop non-local transactions under our own minimal accepted gas price or tip if !local && tx.GasTipCapIntCmp(pool.gasPrice) < 0 { - return ErrUnderpriced + return common.Address{}, ErrUnderpriced } // Ensure the transaction adheres to nonce ordering if pool.currentState.GetNonce(from) > tx.Nonce() { - return core.ErrNonceTooLow + return common.Address{}, core.ErrNonceTooLow } // Transactor should have enough funds to cover the costs // cost == V + GP * GL - if pool.currentState.GetBalance(from).Cmp(tx.Cost()) < 0 { - return core.ErrInsufficientFunds + balance := pool.currentState.GetBalance(from) + if balance.Cmp(tx.Cost()) < 0 { + return common.Address{}, core.ErrInsufficientFunds } + + // Verify that replacing transactions will not result in overdraft + list := pool.pending[from] + if list != nil { // Sender already has pending txs + sum := new(big.Int).Add(tx.Cost(), list.totalcost) + if repl := list.txs.Get(tx.Nonce()); repl != nil { + // Deduct the cost of a transaction replaced by this + sum.Sub(sum, repl.Cost()) + } + if balance.Cmp(sum) < 0 { + log.Trace("Replacing transactions would overdraft", "sender", from, "balance", pool.currentState.GetBalance(from), "required", sum) + return common.Address{}, ErrOverdraft + } + } + // Ensure the transaction has more gas than the basic tx fee. intrGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, true, pool.istanbul, pool.shanghai) if err != nil { - return err + return common.Address{}, err } if tx.Gas() < intrGas { - return core.ErrIntrinsicGas + return common.Address{}, core.ErrIntrinsicGas } - return nil + return from, nil } // add validates a transaction and inserts it into the non-executable queue for later @@ -681,27 +698,13 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e isLocal := local || pool.locals.containsTx(tx) // If the transaction fails basic validation, discard it - if err := pool.validateTx(tx, isLocal); err != nil { + from, err := pool.validateTx(tx, isLocal) + if err != nil { log.Trace("Discarding invalid transaction", "hash", hash, "err", err) invalidTxMeter.Mark(1) return false, err } - // Verify that replacing transactions will not result in overdraft - from, _ := types.Sender(pool.signer, tx) // already validated - list := pool.pending[from] - if list != nil { // Sender already has pending txs - sum := new(big.Int).Add(tx.Cost(), list.totalcost) - if repl := list.txs.Get(tx.Nonce()); repl != nil { - // Deduct the cost of a transaction replaced by this - sum.Sub(sum, repl.Cost()) - } - if sum.Cmp(pool.currentState.GetBalance(from)) > 0 { - log.Trace("Replacing transactions would overdraft", "sender", from, "balance", pool.currentState.GetBalance(from), "required", sum) - return false, ErrOverdraft - } - } - // If the transaction pool is full, discard underpriced transactions if uint64(pool.all.Slots()+numSlots(tx)) > pool.config.GlobalSlots+pool.config.GlobalQueue { // If the new transaction is underpriced, don't accept it @@ -734,12 +737,12 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e // If the new transaction is a future transaction it should never churn pending transactions if pool.isFuture(from, tx) { - for _, dropTx := range drop { - dropSender, _ := types.Sender(pool.signer, dropTx) - if list := pool.pending[dropSender]; list != nil && list.Overlaps(dropTx) { + for i := 0; i < len(drop); i++ { + dropSender, _ := types.Sender(pool.signer, drop[i]) + if list := pool.pending[dropSender]; list != nil && list.Overlaps(drop[i]) { // Add all transactions back to the priced queue - for _, d := range drop { - pool.priced.urgent.Push(d) + for k := 0; k < len(drop); k++ { + heap.Push(&pool.priced.urgent, drop[k]) } log.Trace("Discarding future transaction replacing pending tx", "hash", hash) return false, ErrFutureReplacePending From 7a52cb1c6b3998271a4527930b46d8bd58d4c905 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 12:25:02 +0100 Subject: [PATCH 11/21] core/txpool: use len of index --- core/txpool/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/txpool/list.go b/core/txpool/list.go index 73ca8ddaa0ea..cb7d50d5ced8 100644 --- a/core/txpool/list.go +++ b/core/txpool/list.go @@ -213,7 +213,7 @@ func (m *sortedMap) Ready(start uint64) types.Transactions { // Len returns the length of the transaction map. func (m *sortedMap) Len() int { - return len(m.items) + return m.index.Len() } func (m *sortedMap) flatten() types.Transactions { From 73b65658670dac036358b484c395b3c9444cd8e8 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 12:56:40 +0100 Subject: [PATCH 12/21] core/txpool: revert change --- core/txpool/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/txpool/list.go b/core/txpool/list.go index cb7d50d5ced8..73ca8ddaa0ea 100644 --- a/core/txpool/list.go +++ b/core/txpool/list.go @@ -213,7 +213,7 @@ func (m *sortedMap) Ready(start uint64) types.Transactions { // Len returns the length of the transaction map. func (m *sortedMap) Len() int { - return m.index.Len() + return len(m.items) } func (m *sortedMap) flatten() types.Transactions { From fa66e339dc7aa217700d564819a9d111bc1a7b70 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 15:10:20 +0100 Subject: [PATCH 13/21] core/txpool: apply changes from code review --- core/txpool/list.go | 3 +- core/txpool/txpool.go | 99 ++++++++++++++++++++++--------------------- 2 files changed, 52 insertions(+), 50 deletions(-) diff --git a/core/txpool/list.go b/core/txpool/list.go index 73ca8ddaa0ea..45e2380d12b0 100644 --- a/core/txpool/list.go +++ b/core/txpool/list.go @@ -366,7 +366,8 @@ func (l *list) Filter(costLimit *big.Int, gasLimit uint64) (types.Transactions, invalids = l.txs.filter(func(tx *types.Transaction) bool { return tx.Nonce() > lowest }) } // Reset total cost - l.subTotalCost(append(removed, invalids...)...) + l.subTotalCost(removed...) + l.subTotalCost(invalids...) l.txs.reheap() return removed, invalids } diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index e58dae204487..8517c32c52d6 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -264,11 +264,13 @@ type TxPool struct { locals *accountSet // Set of local transaction to exempt from eviction rules journal *journal // Journal of local transaction to back up to disk - 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 + pending map[common.Address]*list // All currently processable transactions + queue map[common.Address]*list // Queued but non-processable transactions + queuedCount atomic.Int32 + pendingCount atomic.Int32 + 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 chainHeadCh chan core.ChainHeadEvent chainHeadSub event.Subscription @@ -596,61 +598,61 @@ func (pool *TxPool) local() map[common.Address]types.Transactions { // validateTx checks whether a transaction is valid according to the consensus // rules and adheres to some heuristic limits of the local node (price and size). -func (pool *TxPool) validateTx(tx *types.Transaction, local bool) (common.Address, error) { +func (pool *TxPool) validateTx(tx *types.Transaction, local bool) error { // Accept only legacy transactions until EIP-2718/2930 activates. if !pool.eip2718 && tx.Type() != types.LegacyTxType { - return common.Address{}, core.ErrTxTypeNotSupported + return core.ErrTxTypeNotSupported } // Reject dynamic fee transactions until EIP-1559 activates. if !pool.eip1559 && tx.Type() == types.DynamicFeeTxType { - return common.Address{}, core.ErrTxTypeNotSupported + return core.ErrTxTypeNotSupported } // Reject transactions over defined size to prevent DOS attacks if tx.Size() > txMaxSize { - return common.Address{}, ErrOversizedData + return ErrOversizedData } // Check whether the init code size has been exceeded. if pool.shanghai && tx.To() == nil && len(tx.Data()) > params.MaxInitCodeSize { - return common.Address{}, fmt.Errorf("%w: code size %v limit %v", core.ErrMaxInitCodeSizeExceeded, len(tx.Data()), params.MaxInitCodeSize) + return fmt.Errorf("%w: code size %v limit %v", core.ErrMaxInitCodeSizeExceeded, len(tx.Data()), params.MaxInitCodeSize) } // Transactions can't be negative. This may never happen using RLP decoded // transactions but may occur if you create a transaction using the RPC. if tx.Value().Sign() < 0 { - return common.Address{}, ErrNegativeValue + return ErrNegativeValue } // Ensure the transaction doesn't exceed the current block limit gas. if pool.currentMaxGas < tx.Gas() { - return common.Address{}, ErrGasLimit + return ErrGasLimit } // Sanity check for extremely large numbers if tx.GasFeeCap().BitLen() > 256 { - return common.Address{}, core.ErrFeeCapVeryHigh + return core.ErrFeeCapVeryHigh } if tx.GasTipCap().BitLen() > 256 { - return common.Address{}, core.ErrTipVeryHigh + return core.ErrTipVeryHigh } // Ensure gasFeeCap is greater than or equal to gasTipCap. if tx.GasFeeCapIntCmp(tx.GasTipCap()) < 0 { - return common.Address{}, core.ErrTipAboveFeeCap + return core.ErrTipAboveFeeCap } // Make sure the transaction is signed properly. from, err := types.Sender(pool.signer, tx) if err != nil { - return common.Address{}, ErrInvalidSender + return ErrInvalidSender } // Drop non-local transactions under our own minimal accepted gas price or tip if !local && tx.GasTipCapIntCmp(pool.gasPrice) < 0 { - return common.Address{}, ErrUnderpriced + return ErrUnderpriced } // Ensure the transaction adheres to nonce ordering if pool.currentState.GetNonce(from) > tx.Nonce() { - return common.Address{}, core.ErrNonceTooLow + return core.ErrNonceTooLow } // Transactor should have enough funds to cover the costs // cost == V + GP * GL balance := pool.currentState.GetBalance(from) if balance.Cmp(tx.Cost()) < 0 { - return common.Address{}, core.ErrInsufficientFunds + return core.ErrInsufficientFunds } // Verify that replacing transactions will not result in overdraft @@ -663,19 +665,19 @@ func (pool *TxPool) validateTx(tx *types.Transaction, local bool) (common.Addres } if balance.Cmp(sum) < 0 { log.Trace("Replacing transactions would overdraft", "sender", from, "balance", pool.currentState.GetBalance(from), "required", sum) - return common.Address{}, ErrOverdraft + return ErrOverdraft } } // Ensure the transaction has more gas than the basic tx fee. intrGas, err := core.IntrinsicGas(tx.Data(), tx.AccessList(), tx.To() == nil, true, pool.istanbul, pool.shanghai) if err != nil { - return common.Address{}, err + return err } if tx.Gas() < intrGas { - return common.Address{}, core.ErrIntrinsicGas + return core.ErrIntrinsicGas } - return from, nil + return nil } // add validates a transaction and inserts it into the non-executable queue for later @@ -698,13 +700,15 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e isLocal := local || pool.locals.containsTx(tx) // If the transaction fails basic validation, discard it - from, err := pool.validateTx(tx, isLocal) - if err != nil { + if err := pool.validateTx(tx, isLocal); err != nil { log.Trace("Discarding invalid transaction", "hash", hash, "err", err) invalidTxMeter.Mark(1) return false, err } + // already validated by this point + from, _ := types.Sender(pool.signer, tx) + // If the transaction pool is full, discard underpriced transactions if uint64(pool.all.Slots()+numSlots(tx)) > pool.config.GlobalSlots+pool.config.GlobalQueue { // If the new transaction is underpriced, don't accept it @@ -737,21 +741,23 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e // If the new transaction is a future transaction it should never churn pending transactions if pool.isFuture(from, tx) { - for i := 0; i < len(drop); i++ { - dropSender, _ := types.Sender(pool.signer, drop[i]) - if list := pool.pending[dropSender]; list != nil && list.Overlaps(drop[i]) { - // Add all transactions back to the priced queue - for k := 0; k < len(drop); k++ { - heap.Push(&pool.priced.urgent, drop[k]) - } - log.Trace("Discarding future transaction replacing pending tx", "hash", hash) - return false, ErrFutureReplacePending + var replacesPending bool + for _, dropTx := range drop { + dropSender, _ := types.Sender(pool.signer, dropTx) + if list := pool.pending[dropSender]; list != nil && list.Overlaps(dropTx) { + replacesPending = true } } + // Add all transactions back to the priced queue + if replacesPending { + for _, dropTx := range drop { + heap.Push(&pool.priced.urgent, dropTx) + } + log.Trace("Discarding future transaction replacing pending tx", "hash", hash) + return false, ErrFutureReplacePending + } } - // Bump the counter of rejections-since-reorg - pool.changesSinceReorg += len(drop) // Kick out the underpriced remote transactions. for _, tx := range drop { log.Trace("Discarding freshly underpriced transaction", "hash", tx.Hash(), "gasTipCap", tx.GasTipCap(), "gasFeeCap", tx.GasFeeCap()) @@ -805,23 +811,18 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e return replaced, nil } +// isFuture reports whether the given transaction is immediately executable. func (pool *TxPool) isFuture(from common.Address, tx *types.Transaction) bool { list := pool.pending[from] - if list != nil { // Sender already has pending txs - if old := list.txs.Get(tx.Nonce()); old != nil { // Replacing a pending, check bump - return false - } - // Not replacing, check if parent nonce exist - if list.txs.Get(tx.Nonce()-1) != nil { - return false - } - return true + if list == nil { + return pool.pendingNonces.get(from) != tx.Nonce() } - // Sender has no pending - if pool.pendingNonces.get(from) == tx.Nonce() { - return false + // Sender has pending transations. + if old := list.txs.Get(tx.Nonce()); old != nil { + return false // It replaces a pending transaction. } - return true + // Not replacing, check if parent nonce exists in pending. + return list.txs.Get(tx.Nonce()-1) == nil } // enqueueTx inserts a new transaction into the non-executable transaction queue. From 0dda16cce64ef6fe48defd25696ccf8660a78b20 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 15:12:22 +0100 Subject: [PATCH 14/21] core/txpool: apply changes from code review --- core/txpool/list.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/txpool/list.go b/core/txpool/list.go index 45e2380d12b0..1b187caea490 100644 --- a/core/txpool/list.go +++ b/core/txpool/list.go @@ -305,7 +305,7 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64) (bool, *types.Transa return false, nil } // Old is being replaced, subtract old cost - l.subTotalCost(old) + l.subTotalCost([]*types.Transaction{old}) } // Add new tx cost to totalcost l.totalcost.Add(l.totalcost, tx.Cost()) @@ -325,7 +325,7 @@ func (l *list) Add(tx *types.Transaction, priceBump uint64) (bool, *types.Transa // maintenance. func (l *list) Forward(threshold uint64) types.Transactions { txs := l.txs.Forward(threshold) - l.subTotalCost(txs...) + l.subTotalCost(txs) return txs } @@ -366,8 +366,8 @@ func (l *list) Filter(costLimit *big.Int, gasLimit uint64) (types.Transactions, invalids = l.txs.filter(func(tx *types.Transaction) bool { return tx.Nonce() > lowest }) } // Reset total cost - l.subTotalCost(removed...) - l.subTotalCost(invalids...) + l.subTotalCost(removed) + l.subTotalCost(invalids) l.txs.reheap() return removed, invalids } @@ -376,7 +376,7 @@ func (l *list) Filter(costLimit *big.Int, gasLimit uint64) (types.Transactions, // exceeding that limit. func (l *list) Cap(threshold int) types.Transactions { txs := l.txs.Cap(threshold) - l.subTotalCost(txs...) + l.subTotalCost(txs) return txs } @@ -389,11 +389,11 @@ func (l *list) Remove(tx *types.Transaction) (bool, types.Transactions) { if removed := l.txs.Remove(nonce); !removed { return false, nil } - l.subTotalCost(tx) + l.subTotalCost([]*types.Transaction{tx}) // In strict mode, filter out non-executable transactions if l.strict { txs := l.txs.Filter(func(tx *types.Transaction) bool { return tx.Nonce() > nonce }) - l.subTotalCost(txs...) + l.subTotalCost(txs) return true, txs } return true, nil @@ -408,7 +408,7 @@ func (l *list) Remove(tx *types.Transaction) (bool, types.Transactions) { // happen but better to be self correcting than failing! func (l *list) Ready(start uint64) types.Transactions { txs := l.txs.Ready(start) - l.subTotalCost(txs...) + l.subTotalCost(txs) return txs } @@ -437,7 +437,7 @@ func (l *list) LastElement() *types.Transaction { // subTotalCost subtracts the cost of the given transactions from the // total cost of all transactions. -func (l *list) subTotalCost(txs ...*types.Transaction) { +func (l *list) subTotalCost(txs []*types.Transaction) { for _, tx := range txs { l.totalcost.Sub(l.totalcost, tx.Cost()) } From e49d6f7ead9965e48582b45b3e0ca7a3afce6662 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 15:13:35 +0100 Subject: [PATCH 15/21] core/txpool: fixup --- core/txpool/txpool.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 8517c32c52d6..b1a95d4dff17 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -264,13 +264,11 @@ type TxPool struct { locals *accountSet // Set of local transaction to exempt from eviction rules journal *journal // Journal of local transaction to back up to disk - pending map[common.Address]*list // All currently processable transactions - queue map[common.Address]*list // Queued but non-processable transactions - queuedCount atomic.Int32 - pendingCount atomic.Int32 - 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 + 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 chainHeadCh chan core.ChainHeadEvent chainHeadSub event.Subscription From 523ae867e11597ade3c1a4db1e11fbac37a97a09 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 15:36:09 +0100 Subject: [PATCH 16/21] Update core/txpool/txpool.go Co-authored-by: Martin Holst Swende --- core/txpool/txpool.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index b1a95d4dff17..7a8b09c0b539 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -1092,7 +1092,7 @@ func (pool *TxPool) removeTx(hash common.Hash, outofbound bool) int { pool.pendingNonces.setIfLower(addr, tx.Nonce()) // Reduce the pending counter pendingGauge.Dec(int64(1 + len(invalids))) - return len(invalids) + return 1+ len(invalids) } } // Transaction is in the future queue From 0f7423a2eef3418b527477878a0614d8df7fd85b Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 16:09:22 +0100 Subject: [PATCH 17/21] core/txpool: doc --- core/txpool/txpool.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 7a8b09c0b539..11464252ddcf 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -1059,7 +1059,7 @@ func (pool *TxPool) Has(hash common.Hash) bool { // removeTx removes a single transaction from the queue, moving all subsequent // transactions back to the future queue. -// Returns the number of transactions moved to the future queue. +// Returns the number of transactions removed from the pending queue. func (pool *TxPool) removeTx(hash common.Hash, outofbound bool) int { // Fetch the transaction we wish to delete tx := pool.all.Get(hash) @@ -1092,7 +1092,7 @@ func (pool *TxPool) removeTx(hash common.Hash, outofbound bool) int { pool.pendingNonces.setIfLower(addr, tx.Nonce()) // Reduce the pending counter pendingGauge.Dec(int64(1 + len(invalids))) - return 1+ len(invalids) + return 1 + len(invalids) } } // Transaction is in the future queue From 3612a8e59264136cb5af9f4e655f6f3df11974a1 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 16:26:06 +0100 Subject: [PATCH 18/21] core/txpool: don't take stale txs into account --- core/txpool/list.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/txpool/list.go b/core/txpool/list.go index 1b187caea490..724bb6caca99 100644 --- a/core/txpool/list.go +++ b/core/txpool/list.go @@ -598,7 +598,6 @@ func (l *pricedList) Discard(slots int, force bool) (types.Transactions, bool) { tx := heap.Pop(&l.urgent).(*types.Transaction) if l.all.GetRemote(tx.Hash()) == nil { // Removed or migrated atomic.AddInt64(&l.stales, -1) - slots -= numSlots(tx) continue } // Non stale transaction found, move to floating heap @@ -612,7 +611,6 @@ func (l *pricedList) Discard(slots int, force bool) (types.Transactions, bool) { tx := heap.Pop(&l.floating).(*types.Transaction) if l.all.GetRemote(tx.Hash()) == nil { // Removed or migrated atomic.AddInt64(&l.stales, -1) - slots -= numSlots(tx) continue } // Non stale transaction found, discard it From 3307da89e6a54d5d26271d8e8a23d4c4a25815cc Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 16:32:01 +0100 Subject: [PATCH 19/21] core/txpool: happy lint --- core/txpool/txpool.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 11464252ddcf..38f712b33932 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -815,7 +815,7 @@ func (pool *TxPool) isFuture(from common.Address, tx *types.Transaction) bool { if list == nil { return pool.pendingNonces.get(from) != tx.Nonce() } - // Sender has pending transations. + // Sender has pending transactions. if old := list.txs.Get(tx.Nonce()); old != nil { return false // It replaces a pending transaction. } From d0eeccaaf5ac0e9c9f5e8f3184a93406cde81fa2 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 17:08:41 +0100 Subject: [PATCH 20/21] core/txpool: license --- core/txpool/txpool2_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core/txpool/txpool2_test.go b/core/txpool/txpool2_test.go index 662bf88fb641..20d6dd713a95 100644 --- a/core/txpool/txpool2_test.go +++ b/core/txpool/txpool2_test.go @@ -1,3 +1,18 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . package txpool import ( From d1de0bfe43f764867fc4b6d256b647913e1e0f80 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 10 Mar 2023 17:13:49 +0100 Subject: [PATCH 21/21] core/txpool: add break --- core/txpool/txpool.go | 1 + 1 file changed, 1 insertion(+) diff --git a/core/txpool/txpool.go b/core/txpool/txpool.go index 38f712b33932..4306d5aee6f5 100644 --- a/core/txpool/txpool.go +++ b/core/txpool/txpool.go @@ -744,6 +744,7 @@ func (pool *TxPool) add(tx *types.Transaction, local bool) (replaced bool, err e dropSender, _ := types.Sender(pool.signer, dropTx) if list := pool.pending[dropSender]; list != nil && list.Overlaps(dropTx) { replacesPending = true + break } } // Add all transactions back to the priced queue