Limit mempool size to million transactions and remove the least profitable transactions (#1483)

* Limit mempool size to million transactions and remove the least profitable transactions

* Simplify insert

* Fix typo

* Improve findTxIndexInOrderedTransactionsByFeeRate readability

app/protocol/flowcontext/orphans.go

@@ -194,6 +194,9 @@ func (f *FlowContext) GetOrphanRoots(orphan *externalapi.DomainHash) ([]*externa
 
 			if !blockInfo.Exists || blockInfo.BlockStatus == externalapi.StatusHeaderOnly {
 				roots = append(roots, current)
+			} else {
+				log.Debugf("Block %s was skipped when checking for orphan roots: "+
+					"exists: %t, status: %s", current, blockInfo.Exists, blockInfo.BlockStatus)
 			}
 			continue
 		}

domain/consensus/model/externalapi/hash.go

@@ -85,6 +85,16 @@ func (hash *DomainHash) Equal(other *DomainHash) bool {
 	return hash.hashArray == other.hashArray
 }
 
+// Less returns true if hash is less than other
+func (hash *DomainHash) Less(other *DomainHash) bool {
+	return bytes.Compare(hash.hashArray[:], other.hashArray[:]) < 0
+}
+
+// LessOrEqual returns true if hash is smaller or equal to other
+func (hash *DomainHash) LessOrEqual(other *DomainHash) bool {
+	return bytes.Compare(hash.hashArray[:], other.hashArray[:]) <= 0
+}
+
 // CloneHashes returns a clone of the given hashes slice.
 // Note: since DomainHash is a read-only type, the clone is shallow
 func CloneHashes(hashes []*DomainHash) []*DomainHash {
@@ -106,8 +116,3 @@ func HashesEqual(a, b []*DomainHash) bool {
 	}
 	return true
 }
-
-// Less returns true iff hash a is less than hash b
-func Less(a, b *DomainHash) bool {
-	return bytes.Compare(a.hashArray[:], b.hashArray[:]) < 0
-}

domain/consensus/model/externalapi/transaction.go

@@ -313,6 +313,16 @@ func (id *DomainTransactionID) Equal(other *DomainTransactionID) bool {
 	return (*DomainHash)(id).Equal((*DomainHash)(other))
 }
 
+// Less returns true if id is less than other
+func (id *DomainTransactionID) Less(other *DomainTransactionID) bool {
+	return (*DomainHash)(id).Less((*DomainHash)(other))
+}
+
+// LessOrEqual returns true if id is smaller or equal to other
+func (id *DomainTransactionID) LessOrEqual(other *DomainTransactionID) bool {
+	return (*DomainHash)(id).LessOrEqual((*DomainHash)(other))
+}
+
 // ByteArray returns the bytes in this transactionID represented as a byte array.
 // The transactionID bytes are cloned, therefore it is safe to modify the resulting array.
 func (id *DomainTransactionID) ByteArray() *[DomainHashSize]byte {

domain/consensus/processes/blockbuilder/block_builder.go

@@ -9,7 +9,6 @@ import (
 	"github.com/kaspanet/kaspad/domain/consensus/utils/consensushashing"
 	"github.com/kaspanet/kaspad/domain/consensus/utils/constants"
 	"github.com/kaspanet/kaspad/domain/consensus/utils/merkle"
-	"github.com/kaspanet/kaspad/domain/consensus/utils/transactionid"
 	"github.com/kaspanet/kaspad/infrastructure/logger"
 	"github.com/kaspanet/kaspad/util/mstime"
 )
@@ -197,7 +196,7 @@ func (bb *blockBuilder) calculateAcceptedIDMerkleRoot(acceptanceData externalapi
 	sort.Slice(acceptedTransactions, func(i, j int) bool {
 		acceptedTransactionIID := consensushashing.TransactionID(acceptedTransactions[i])
 		acceptedTransactionJID := consensushashing.TransactionID(acceptedTransactions[j])
-		return transactionid.Less(acceptedTransactionIID, acceptedTransactionJID)
+		return acceptedTransactionIID.Less(acceptedTransactionJID)
 	})
 
 	return merkle.CalculateIDMerkleRoot(acceptedTransactions), nil

domain/consensus/processes/consensusstatemanager/verify_and_build_utxo.go

@@ -5,8 +5,6 @@ import (
 
 	"github.com/kaspanet/kaspad/domain/consensus/utils/transactionhelper"
 
-	"github.com/kaspanet/kaspad/domain/consensus/utils/transactionid"
-
 	"github.com/kaspanet/kaspad/domain/consensus/utils/consensushashing"
 
 	"github.com/kaspanet/kaspad/domain/consensus/utils/merkle"
@@ -142,8 +140,7 @@ func calculateAcceptedIDMerkleRoot(multiblockAcceptanceData externalapi.Acceptan
 		}
 	}
 	sort.Slice(acceptedTransactions, func(i, j int) bool {
-		return transactionid.Less(
+		return consensushashing.TransactionID(acceptedTransactions[i]).Less(
-			consensushashing.TransactionID(acceptedTransactions[i]),
 			consensushashing.TransactionID(acceptedTransactions[j]))
 	})
 

domain/consensus/processes/ghostdagmanager/compare.go

@@ -60,7 +60,7 @@ func (gm *ghostdagManager) Less(blockHashA *externalapi.DomainHash, ghostdagData
 	case 1:
 		return false
 	case 0:
-		return externalapi.Less(blockHashA, blockHashB)
+		return blockHashA.Less(blockHashB)
 	default:
 		panic("big.Int.Cmp is defined to always return -1/1/0 and nothing else")
 	}

domain/consensus/utils/transactionid/compare.go

@@ -1,10 +0,0 @@
-package transactionid
-
-import (
-	"github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
-)
-
-// Less returns true iff transaction ID a is less than hash b
-func Less(a, b *externalapi.DomainTransactionID) bool {
-	return externalapi.Less((*externalapi.DomainHash)(a), (*externalapi.DomainHash)(b))
-}

domain/miningmanager/mempool/mempool.go

@@ -7,6 +7,7 @@ package mempool
 import (
 	"container/list"
 	"fmt"
+	"sort"
 	"sync"
 	"time"
 
@@ -78,6 +79,8 @@ type mempool struct {
 	mempoolUTXOSet *mempoolUTXOSet
 	consensus      consensusexternalapi.Consensus
 
+	orderedTransactionsByFeeRate []*consensusexternalapi.DomainTransaction
+
 	// nextExpireScan is the time after which the orphan pool will be
 	// scanned in order to evict orphans. This is NOT a hard deadline as
 	// the scan will only run when an orphan is added to the pool as opposed
@@ -378,16 +381,18 @@ func (mp *mempool) removeBlockTransactionsFromPool(txs []*consensusexternalapi.D
 	for _, tx := range txs[transactionhelper.CoinbaseTransactionIndex+1:] {
 		txID := consensushashing.TransactionID(tx)
 
-		if _, exists := mp.fetchTxDesc(txID); !exists {
+		// We use the mempool transaction, because it has populated fee and mass
+		mempoolTx, exists := mp.fetchTxDesc(txID)
+		if !exists {
 			continue
 		}
 
-		err := mp.cleanTransactionFromSets(tx)
+		err := mp.cleanTransactionFromSets(mempoolTx.DomainTransaction)
 		if err != nil {
 			return err
 		}
 
-		mp.updateBlockTransactionChainedTransactions(tx)
+		mp.updateBlockTransactionChainedTransactions(mempoolTx.DomainTransaction)
 	}
 	return nil
 }
@@ -434,7 +439,7 @@ func (mp *mempool) cleanTransactionFromSets(tx *consensusexternalapi.DomainTrans
 	delete(mp.pool, *txID)
 	delete(mp.chainedTransactions, *txID)
 
-	return nil
+	return mp.removeTransactionFromOrderedTransactionsByFeeRate(tx)
 }
 
 // updateBlockTransactionChainedTransactions processes the dependencies of a
@@ -504,7 +509,7 @@ func (mp *mempool) removeDoubleSpends(tx *consensusexternalapi.DomainTransaction
 // helper for maybeAcceptTransaction.
 //
 // This function MUST be called with the mempool lock held (for writes).
-func (mp *mempool) addTransaction(tx *consensusexternalapi.DomainTransaction, mass uint64, fee uint64, parentsInPool []consensusexternalapi.DomainOutpoint) (*txDescriptor, error) {
+func (mp *mempool) addTransaction(tx *consensusexternalapi.DomainTransaction, parentsInPool []consensusexternalapi.DomainOutpoint) (*txDescriptor, error) {
 	// Add the transaction to the pool and mark the referenced outpoints
 	// as spent by the pool.
 	txDescriptor := &txDescriptor{
@@ -527,9 +532,77 @@ func (mp *mempool) addTransaction(tx *consensusexternalapi.DomainTransaction, ma
 		return nil, err
 	}
 
+	err = mp.addTransactionToOrderedTransactionsByFeeRate(tx)
+	if err != nil {
+		return nil, err
+	}
+
 	return txDescriptor, nil
 }
 
+func (mp *mempool) findTxIndexInOrderedTransactionsByFeeRate(tx *consensusexternalapi.DomainTransaction) (int, error) {
+	if tx.Fee == 0 || tx.Mass == 0 {
+		return 0, errors.Errorf("findTxIndexInOrderedTransactionsByFeeRate expects a transaction with " +
+			"populated fee and mass")
+	}
+	txID := consensushashing.TransactionID(tx)
+	txFeeRate := float64(tx.Fee) / float64(tx.Mass)
+
+	return sort.Search(len(mp.orderedTransactionsByFeeRate), func(i int) bool {
+		elementFeeRate := float64(mp.orderedTransactionsByFeeRate[i].Fee) / float64(mp.orderedTransactionsByFeeRate[i].Mass)
+		if elementFeeRate > txFeeRate {
+			return true
+		}
+
+		if elementFeeRate == txFeeRate && txID.LessOrEqual(consensushashing.TransactionID(mp.orderedTransactionsByFeeRate[i])) {
+			return true
+		}
+
+		return false
+	}), nil
+}
+
+func (mp *mempool) addTransactionToOrderedTransactionsByFeeRate(tx *consensusexternalapi.DomainTransaction) error {
+	index, err := mp.findTxIndexInOrderedTransactionsByFeeRate(tx)
+	if err != nil {
+		return err
+	}
+
+	mp.orderedTransactionsByFeeRate = append(mp.orderedTransactionsByFeeRate[:index],
+		append([]*consensusexternalapi.DomainTransaction{tx}, mp.orderedTransactionsByFeeRate[index:]...)...)
+
+	return nil
+}
+
+func (mp *mempool) removeTransactionFromOrderedTransactionsByFeeRate(tx *consensusexternalapi.DomainTransaction) error {
+	index, err := mp.findTxIndexInOrderedTransactionsByFeeRate(tx)
+	if err != nil {
+		return err
+	}
+
+	txID := consensushashing.TransactionID(tx)
+	if !consensushashing.TransactionID(mp.orderedTransactionsByFeeRate[index]).Equal(txID) {
+		return errors.Errorf("Couldn't find %s in mp.orderedTransactionsByFeeRate", txID)
+	}
+
+	mp.orderedTransactionsByFeeRate = append(mp.orderedTransactionsByFeeRate[:index], mp.orderedTransactionsByFeeRate[index+1:]...)
+	return nil
+}
+
+func (mp *mempool) enforceTransactionLimit() error {
+	const limit = 1_000_000
+	if len(mp.pool)+len(mp.chainedTransactions) > limit {
+		// mp.orderedTransactionsByFeeRate[0] is the least profitable transaction
+		txToRemove := mp.orderedTransactionsByFeeRate[0]
+		log.Debugf("Mempool size is over the limit of %d transactions. Removing %s",
+			limit,
+			consensushashing.TransactionID(txToRemove),
+		)
+		return mp.removeTransactionAndItsChainedTransactions(txToRemove)
+	}
+	return nil
+}
+
 // checkPoolDoubleSpend checks whether or not the passed transaction is
 // attempting to spend coins already spent by other transactions in the pool.
 // Note it does not check for double spends against transactions already in the
@@ -673,7 +746,7 @@ func (mp *mempool) maybeAcceptTransaction(tx *consensusexternalapi.DomainTransac
 		return nil, nil, txRuleError(RejectInsufficientFee, str)
 	}
 	// Add to transaction pool.
-	txDesc, err := mp.addTransaction(tx, tx.Mass, tx.Fee, parentsInPool)
+	txDesc, err := mp.addTransaction(tx, parentsInPool)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -681,6 +754,11 @@ func (mp *mempool) maybeAcceptTransaction(tx *consensusexternalapi.DomainTransac
 	log.Debugf("Accepted transaction %s (pool size: %d)", txID,
 		len(mp.pool))
 
+	err = mp.enforceTransactionLimit()
+	if err != nil {
+		return nil, nil, err
+	}
+
 	return nil, txDesc, nil
 }