core: enforce min 10% price bump for replacement txs

core/tx_list.go

@@ -236,6 +236,12 @@ func newTxList(strict bool) *txList {
 	}
 }
 
+// Overlaps returns whether the transaction specified has the same nonce as one
+// already contained within the list.
+func (l *txList) Overlaps(tx *types.Transaction) bool {
+	return l.txs.Get(tx.Nonce()) != nil
+}
+
 // Add tries to insert a new transaction into the list, returning whether the
 // transaction was accepted, and if yes, any previous transaction it replaced.
 //
@@ -244,8 +250,11 @@ func newTxList(strict bool) *txList {
 func (l *txList) Add(tx *types.Transaction) (bool, *types.Transaction) {
 	// If there's an older better transaction, abort
 	old := l.txs.Get(tx.Nonce())
-	if old != nil && old.GasPrice().Cmp(tx.GasPrice()) >= 0 {
+	if old != nil {
-		return false, nil
+		threshold := new(big.Int).Div(new(big.Int).Mul(old.GasPrice(), big.NewInt(100+minPriceBumpPercent)), big.NewInt(100))
+		if threshold.Cmp(tx.GasPrice()) >= 0 {
+			return false, nil
+		}
 	}
 	// Otherwise overwrite the old transaction with the current one
 	l.txs.Put(tx)

core/tx_pool.go

@@ -36,14 +36,15 @@ import (
 
 var (
 	// Transaction Pool Errors
-	ErrInvalidSender     = errors.New("invalid sender")
+	ErrInvalidSender      = errors.New("invalid sender")
-	ErrNonce             = errors.New("nonce too low")
+	ErrNonce              = errors.New("nonce too low")
-	ErrUnderpriced       = errors.New("underpriced transaction")
+	ErrUnderpriced        = errors.New("transaction underpriced")
-	ErrBalance           = errors.New("tnsufficient balance")
+	ErrReplaceUnderpriced = errors.New("replacement transaction underpriced")
-	ErrInsufficientFunds = errors.New("tnsufficient funds for gas * price + value")
+	ErrBalance            = errors.New("insufficient balance")
-	ErrIntrinsicGas      = errors.New("tntrinsic gas too low")
+	ErrInsufficientFunds  = errors.New("insufficient funds for gas * price + value")
-	ErrGasLimit          = errors.New("exceeds block gas limit")
+	ErrIntrinsicGas       = errors.New("intrinsic gas too low")
-	ErrNegativeValue     = errors.New("negative value")
+	ErrGasLimit           = errors.New("exceeds block gas limit")
+	ErrNegativeValue      = errors.New("negative value")
 )
 
 var (
@@ -52,6 +53,7 @@ var (
 	maxQueuedPerAccount  = uint64(64)      // Max limit of queued transactions per address
 	maxQueuedTotal       = uint64(1024)    // Max limit of queued transactions from all accounts
 	maxQueuedLifetime    = 3 * time.Hour   // Max amount of time transactions from idle accounts are queued
+	minPriceBumpPercent  = int64(10)       // Minimum price bump needed to replace an old transaction
 	evictionInterval     = time.Minute     // Time interval to check for evictable transactions
 	statsReportInterval  = 8 * time.Second // Time interval to report transaction pool stats
 )
@@ -368,19 +370,21 @@ func (pool *TxPool) validateTx(tx *types.Transaction) error {
 }
 
 // add validates a transaction and inserts it into the non-executable queue for
-// later pending promotion and execution.
+// later pending promotion and execution. If the transaction is a replacement for
-func (pool *TxPool) add(tx *types.Transaction) error {
+// an already pending or queued one, it overwrites the previous and returns this
+// so outer code doesn't uselessly call promote.
+func (pool *TxPool) add(tx *types.Transaction) (bool, error) {
 	// If the transaction is already known, discard it
 	hash := tx.Hash()
 	if pool.all[hash] != nil {
 		log.Trace("Discarding already known transaction", "hash", hash)
-		return fmt.Errorf("known transaction: %x", hash)
+		return false, fmt.Errorf("known transaction: %x", hash)
 	}
 	// If the transaction fails basic validation, discard it
 	if err := pool.validateTx(tx); err != nil {
 		log.Trace("Discarding invalid transaction", "hash", hash, "err", err)
 		invalidTxCounter.Inc(1)
-		return err
+		return false, err
 	}
 	// If the transaction pool is full, discard underpriced transactions
 	if uint64(len(pool.all)) >= maxPendingTotal+maxQueuedTotal {
@@ -388,7 +392,7 @@ func (pool *TxPool) add(tx *types.Transaction) error {
 		if pool.priced.Underpriced(tx, pool.locals) {
 			log.Trace("Discarding underpriced transaction", "hash", hash, "price", tx.GasPrice())
 			underpricedTxCounter.Inc(1)
-			return ErrUnderpriced
+			return false, ErrUnderpriced
 		}
 		// New transaction is better than our worse ones, make room for it
 		drop := pool.priced.Discard(len(pool.all)-int(maxPendingTotal+maxQueuedTotal-1), pool.locals)
@@ -398,17 +402,40 @@ func (pool *TxPool) add(tx *types.Transaction) error {
 			pool.removeTx(tx.Hash())
 		}
 	}
-	// Great, queue the transaction
+	// If the transaction is replacing an already pending one, do directly
-	pool.enqueueTx(hash, tx)
+	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)
+		if !inserted {
+			pendingDiscardCounter.Inc(1)
+			return false, ErrReplaceUnderpriced
+		}
+		// New transaction is better, replace old one
+		if old != nil {
+			delete(pool.all, old.Hash())
+			pool.priced.Remove(old)
+			pendingReplaceCounter.Inc(1)
+		}
+		pool.all[tx.Hash()] = tx
+		pool.priced.Put(tx)
 
-	log.Trace("Pooled new transaction", "hash", hash, "from", log.Lazy{Fn: func() common.Address { from, _ := types.Sender(pool.signer, tx); return from }}, "to", tx.To())
+		log.Trace("Pooled new executable transaction", "hash", hash, "from", from, "to", tx.To())
-	return nil
+		return old != nil, nil
+	}
+	// New transaction isn't replacing a pending one, push into queue
+	replace, err := pool.enqueueTx(hash, tx)
+	if err != nil {
+		return false, err
+	}
+	log.Trace("Pooled new future transaction", "hash", hash, "from", from, "to", tx.To())
+	return replace, nil
 }
 
 // enqueueTx inserts a new transaction into the non-executable transaction queue.
 //
 // Note, this method assumes the pool lock is held!
-func (pool *TxPool) enqueueTx(hash common.Hash, tx *types.Transaction) {
+func (pool *TxPool) enqueueTx(hash common.Hash, tx *types.Transaction) (bool, error) {
 	// Try to insert the transaction into the future queue
 	from, _ := types.Sender(pool.signer, tx) // already validated
 	if pool.queue[from] == nil {
@@ -416,18 +443,19 @@ func (pool *TxPool) enqueueTx(hash common.Hash, tx *types.Transaction) {
 	}
 	inserted, old := pool.queue[from].Add(tx)
 	if !inserted {
+		// An older transaction was better, discard this
 		queuedDiscardCounter.Inc(1)
-		return // An older transaction was better, discard this
+		return false, ErrReplaceUnderpriced
 	}
 	// Discard any previous transaction and mark this
 	if old != nil {
 		delete(pool.all, old.Hash())
 		pool.priced.Remove(old)
-
 		queuedReplaceCounter.Inc(1)
 	}
 	pool.all[hash] = tx
 	pool.priced.Put(tx)
+	return old != nil, nil
 }
 
 // promoteTx adds a transaction to the pending (processable) list of transactions.
@@ -472,15 +500,19 @@ func (pool *TxPool) Add(tx *types.Transaction) error {
 	pool.mu.Lock()
 	defer pool.mu.Unlock()
 
-	if err := pool.add(tx); err != nil {
+	// Try to inject the transaction and update any state
+	replace, err := pool.add(tx)
+	if err != nil {
 		return err
 	}
 	state, err := pool.currentState()
 	if err != nil {
 		return err
 	}
-	pool.promoteExecutables(state)
+	// If we added a new transaction, run promotion checks and return
-
+	if !replace {
+		pool.promoteExecutables(state)
+	}
 	return nil
 }
 
@@ -490,10 +522,13 @@ func (pool *TxPool) AddBatch(txs []*types.Transaction) error {
 	defer pool.mu.Unlock()
 
 	// Add the batch of transaction, tracking the accepted ones
-	added := 0
+	replaced, added := true, 0
 	for _, tx := range txs {
-		if err := pool.add(tx); err == nil {
+		if replace, err := pool.add(tx); err == nil {
 			added++
+			if !replace {
+				replaced = false
+			}
 		}
 	}
 	// Only reprocess the internal state if something was actually added
@@ -502,7 +537,9 @@ func (pool *TxPool) AddBatch(txs []*types.Transaction) error {
 		if err != nil {
 			return err
 		}
-		pool.promoteExecutables(state)
+		if !replaced {
+			pool.promoteExecutables(state)
+		}
 	}
 	return nil
 }

core/tx_pool_test.go

@@ -266,14 +266,14 @@ func TestTransactionChainFork(t *testing.T) {
 	resetState()
 
 	tx := transaction(0, big.NewInt(100000), key)
-	if err := pool.add(tx); err != nil {
+	if _, err := pool.add(tx); err != nil {
 		t.Error("didn't expect error", err)
 	}
 	pool.RemoveBatch([]*types.Transaction{tx})
 
 	// reset the pool's internal state
 	resetState()
-	if err := pool.add(tx); err != nil {
+	if _, err := pool.add(tx); err != nil {
 		t.Error("didn't expect error", err)
 	}
 }
@@ -297,11 +297,11 @@ func TestTransactionDoubleNonce(t *testing.T) {
 	tx3, _ := types.SignTx(types.NewTransaction(0, common.Address{}, big.NewInt(100), big.NewInt(1000000), big.NewInt(1), nil), signer, key)
 
 	// Add the first two transaction, ensure higher priced stays only
-	if err := pool.add(tx1); err != nil {
+	if replace, err := pool.add(tx1); err != nil || replace {
-		t.Error("didn't expect error", err)
+		t.Errorf("first transaction insert failed (%v) or reported replacement (%v)", err, replace)
 	}
-	if err := pool.add(tx2); err != nil {
+	if replace, err := pool.add(tx2); err != nil || !replace {
-		t.Error("didn't expect error", err)
+		t.Errorf("second transaction insert failed (%v) or not reported replacement (%v)", err, replace)
 	}
 	state, _ := pool.currentState()
 	pool.promoteExecutables(state)
@@ -312,9 +312,7 @@ func TestTransactionDoubleNonce(t *testing.T) {
 		t.Errorf("transaction mismatch: have %x, want %x", tx.Hash(), tx2.Hash())
 	}
 	// Add the thid transaction and ensure it's not saved (smaller price)
-	if err := pool.add(tx3); err != nil {
+	pool.add(tx3)
-		t.Error("didn't expect error", err)
-	}
 	pool.promoteExecutables(state)
 	if pool.pending[addr].Len() != 1 {
 		t.Error("expected 1 pending transactions, got", pool.pending[addr].Len())
@@ -334,7 +332,7 @@ func TestMissingNonce(t *testing.T) {
 	currentState, _ := pool.currentState()
 	currentState.AddBalance(addr, big.NewInt(100000000000000))
 	tx := transaction(1, big.NewInt(100000), key)
-	if err := pool.add(tx); err != nil {
+	if _, err := pool.add(tx); err != nil {
 		t.Error("didn't expect error", err)
 	}
 	if len(pool.pending) != 0 {
@@ -955,6 +953,67 @@ func TestTransactionPoolUnderpricing(t *testing.T) {
 	}
 }
 
+// Tests that the pool rejects replacement transactions that don't meet the minimum
+// price bump required.
+func TestTransactionReplacement(t *testing.T) {
+	// Create the pool to test the pricing enforcement with
+	db, _ := ethdb.NewMemDatabase()
+	statedb, _ := state.New(common.Hash{}, db)
+
+	pool := NewTxPool(params.TestChainConfig, new(event.TypeMux), func() (*state.StateDB, error) { return statedb, nil }, func() *big.Int { return big.NewInt(1000000) })
+	pool.resetState()
+
+	// Create a a test account to add transactions with
+	key, _ := crypto.GenerateKey()
+
+	state, _ := pool.currentState()
+	state.AddBalance(crypto.PubkeyToAddress(key.PublicKey), big.NewInt(1000000000))
+
+	// Add pending transactions, ensuring the minimum price bump is enforced for replacement (for ultra low prices too)
+	price := int64(100)
+	threshold := (price * (100 + minPriceBumpPercent)) / 100
+
+	if err := pool.Add(pricedTransaction(0, big.NewInt(100000), big.NewInt(1), key)); err != nil {
+		t.Fatalf("failed to add original cheap pending transaction: %v", err)
+	}
+	if err := pool.Add(pricedTransaction(0, big.NewInt(100001), big.NewInt(1), key)); err != ErrReplaceUnderpriced {
+		t.Fatalf("original cheap pending transaction replacement error mismatch: have %v, want %v", err, ErrReplaceUnderpriced)
+	}
+	if err := pool.Add(pricedTransaction(0, big.NewInt(100000), big.NewInt(2), key)); err != nil {
+		t.Fatalf("failed to replace original cheap pending transaction: %v", err)
+	}
+
+	if err := pool.Add(pricedTransaction(0, big.NewInt(100000), big.NewInt(price), key)); err != nil {
+		t.Fatalf("failed to add original proper pending transaction: %v", err)
+	}
+	if err := pool.Add(pricedTransaction(0, big.NewInt(100000), big.NewInt(threshold), key)); err != ErrReplaceUnderpriced {
+		t.Fatalf("original proper pending transaction replacement error mismatch: have %v, want %v", err, ErrReplaceUnderpriced)
+	}
+	if err := pool.Add(pricedTransaction(0, big.NewInt(100000), big.NewInt(threshold+1), key)); err != nil {
+		t.Fatalf("failed to replace original proper pending transaction: %v", err)
+	}
+	// Add queued transactions, ensuring the minimum price bump is enforced for replacement (for ultra low prices too)
+	if err := pool.Add(pricedTransaction(2, big.NewInt(100000), big.NewInt(1), key)); err != nil {
+		t.Fatalf("failed to add original queued transaction: %v", err)
+	}
+	if err := pool.Add(pricedTransaction(2, big.NewInt(100001), big.NewInt(1), key)); err != ErrReplaceUnderpriced {
+		t.Fatalf("original queued transaction replacement error mismatch: have %v, want %v", err, ErrReplaceUnderpriced)
+	}
+	if err := pool.Add(pricedTransaction(2, big.NewInt(100000), big.NewInt(2), key)); err != nil {
+		t.Fatalf("failed to replace original queued transaction: %v", err)
+	}
+
+	if err := pool.Add(pricedTransaction(2, big.NewInt(100000), big.NewInt(price), key)); err != nil {
+		t.Fatalf("failed to add original queued transaction: %v", err)
+	}
+	if err := pool.Add(pricedTransaction(2, big.NewInt(100001), big.NewInt(threshold), key)); err != ErrReplaceUnderpriced {
+		t.Fatalf("original queued transaction replacement error mismatch: have %v, want %v", err, ErrReplaceUnderpriced)
+	}
+	if err := pool.Add(pricedTransaction(2, big.NewInt(100000), big.NewInt(threshold+1), key)); err != nil {
+		t.Fatalf("failed to replace original queued 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) }