fix: update miner mempool iterator query to consider both nonces and fee rates

stackslib/src/core/mempool.rs

@@ -820,6 +820,20 @@ const MEMPOOL_SCHEMA_7_TIME_ESTIMATES: &'static [&'static str] = &[
     "#,
 ];
 
+const MEMPOOL_SCHEMA_8_NONCE_SORTING: &'static [&'static str] = &[
+    r#"
+    -- Drop redundant mempool indexes, covered by unique constraints
+    DROP INDEX IF EXISTS "by_txid";
+    DROP INDEX IF EXISTS "by_sponsor";
+    DROP INDEX IF EXISTS "by_origin";
+    -- Add index to help comparing address nonces against mempool content
+    CREATE INDEX IF NOT EXISTS by_address_nonce ON nonces(address, nonce);
+    "#,
+    r#"
+    INSERT INTO schema_version (version) VALUES (8)
+    "#,
+];
+
 const MEMPOOL_INDEXES: &'static [&'static str] = &[
     "CREATE INDEX IF NOT EXISTS by_txid ON mempool(txid);",
     "CREATE INDEX IF NOT EXISTS by_height ON mempool(height);",
@@ -1393,6 +1407,16 @@ impl MemPoolDB {
         Ok(())
     }
 
+    /// Optimize indexes for mempool visits
+    #[cfg_attr(test, mutants::skip)]
+    fn instantiate_schema_8(tx: &DBTx) -> Result<(), db_error> {
+        for sql_exec in MEMPOOL_SCHEMA_8_NONCE_SORTING {
+            tx.execute_batch(sql_exec)?;
+        }
+
+        Ok(())
+    }
+
     #[cfg_attr(test, mutants::skip)]
     pub fn db_path(chainstate_root_path: &str) -> Result<String, db_error> {
         let mut path = PathBuf::from(chainstate_root_path);
@@ -1645,39 +1669,56 @@ impl MemPoolDB {
 
         debug!("Mempool walk for {}ms", settings.max_walk_time_ms,);
 
-        let tx_consideration_sampler = Uniform::new(0, 100);
-        let mut rng = rand::thread_rng();
         let mut candidate_cache = CandidateCache::new(settings.candidate_retry_cache_size);
         let mut nonce_cache = NonceCache::new(settings.nonce_cache_size);
 
         // set of (address, nonce) to store after the inner loop completes.  This will be done in a
         // single transaction.  This cannot grow to more than `settings.nonce_cache_size` entries.
         let mut retry_store = HashMap::new();
 
+        // Iterate pending mempool transactions using a heuristic that maximizes miner fee profitability and minimizes CPU time
+        // wasted on already-mined or not-yet-mineable transactions. This heuristic takes the following steps:
+        //
+        // 1. Filters out transactions that have nonces smaller than the origin address' next expected nonce as stated in the
+        //    `nonces` table, when possible
+        // 2. Adds a "simulated" fee rate to transactions that don't have it by multiplying the mempool's maximum current fee rate
+        //    by a random number. This helps us mix these transactions with others to guarantee they get processed in a reasonable
+        //    order
+        // 3. Ranks transactions by prioritizing those with next nonces and higher fees (per origin address)
+        // 4. Sorts all ranked transactions by fee and returns them for evaluation
+        //
+        // This logic prevents miners from repeatedly visiting (and then skipping) high fee transactions that would get evaluated
+        // first based on their `fee_rate` but are otherwise non-mineable because they have very high or invalid nonces. A large
+        // volume of these transactions would cause considerable slowness when selecting valid transactions to mine.
+        //
+        // This query also makes sure transactions that have NULL `fee_rate`s are visited, because they will also get ranked
+        // according to their origin address nonce.
         let sql = "
-             SELECT txid, origin_nonce, origin_address, sponsor_nonce, sponsor_address, fee_rate
-             FROM mempool
-             WHERE fee_rate IS NULL
-             ";
-        let mut query_stmt_null = self
-            .db
-            .prepare(&sql)
-            .map_err(|err| Error::SqliteError(err))?;
-        let mut null_iterator = query_stmt_null
-            .query(NO_PARAMS)
-            .map_err(|err| Error::SqliteError(err))?;
-
-        let sql = "
+            WITH nonce_filtered AS (
+                SELECT txid, origin_nonce, origin_address, sponsor_nonce, sponsor_address, fee_rate,
+                    CASE
+                        WHEN fee_rate IS NULL THEN (ABS(RANDOM()) % 10000 / 10000.0) * (SELECT MAX(fee_rate) FROM mempool)
+                        ELSE fee_rate
+                    END AS sort_fee_rate
+                FROM mempool
+                LEFT JOIN nonces ON mempool.origin_address = nonces.address AND mempool.origin_nonce >= nonces.nonce
+            ),
+            address_nonce_ranked AS (
+                SELECT *, ROW_NUMBER() OVER (
+                    PARTITION BY origin_address
+                    ORDER BY origin_nonce ASC, sort_fee_rate DESC
+                ) AS rank
+                FROM nonce_filtered
+            )
             SELECT txid, origin_nonce, origin_address, sponsor_nonce, sponsor_address, fee_rate
-            FROM mempool
-            WHERE fee_rate IS NOT NULL
-            ORDER BY fee_rate DESC
+            FROM address_nonce_ranked
+            ORDER BY rank ASC, sort_fee_rate DESC
             ";
-        let mut query_stmt_fee = self
+        let mut query_stmt = self
             .db
             .prepare(&sql)
             .map_err(|err| Error::SqliteError(err))?;
-        let mut fee_iterator = query_stmt_fee
+        let mut tx_iterator = query_stmt
             .query(NO_PARAMS)
             .map_err(|err| Error::SqliteError(err))?;
 
@@ -1688,46 +1729,23 @@ impl MemPoolDB {
                 break MempoolIterationStopReason::DeadlineReached;
             }
 
-            let start_with_no_estimate =
-                tx_consideration_sampler.sample(&mut rng) < settings.consider_no_estimate_tx_prob;
-
             // First, try to read from the retry list
             let (candidate, update_estimate) = match candidate_cache.next() {
                 Some(tx) => {
                     let update_estimate = tx.fee_rate.is_none();
                     (tx, update_estimate)
                 }
                 None => {
-                    // When the retry list is empty, read from the mempool db,
-                    // randomly selecting from either the null fee-rate transactions
-                    // or those with fee-rate estimates.
-                    let opt_tx = if start_with_no_estimate {
-                        null_iterator
-                            .next()
-                            .map_err(|err| Error::SqliteError(err))?
-                    } else {
-                        fee_iterator.next().map_err(|err| Error::SqliteError(err))?
-                    };
-                    match opt_tx {
-                        Some(row) => (MemPoolTxInfoPartial::from_row(row)?, start_with_no_estimate),
+                    // When the retry list is empty, read from the mempool db
+                    match tx_iterator.next().map_err(|err| Error::SqliteError(err))? {
+                        Some(row) => {
+                            let tx = MemPoolTxInfoPartial::from_row(row)?;
+                            let update_estimate = tx.fee_rate.is_none();
+                            (tx, update_estimate)
+                        }
                         None => {
-                            // If the selected iterator is empty, check the other
-                            match if start_with_no_estimate {
-                                fee_iterator.next().map_err(|err| Error::SqliteError(err))?
-                            } else {
-                                null_iterator
-                                    .next()
-                                    .map_err(|err| Error::SqliteError(err))?
-                            } {
-                                Some(row) => (
-                                    MemPoolTxInfoPartial::from_row(row)?,
-                                    !start_with_no_estimate,
-                                ),
-                                None => {
-                                    debug!("No more transactions to consider in mempool");
-                                    break MempoolIterationStopReason::NoMoreCandidates;
-                                }
-                            }
+                            debug!("No more transactions to consider in mempool");
+                            break MempoolIterationStopReason::NoMoreCandidates;
                         }
                     }
                 }
@@ -1928,10 +1946,8 @@ impl MemPoolDB {
         // drop these rusqlite statements and queries, since their existence as immutable borrows on the
         // connection prevents us from beginning a transaction below (which requires a mutable
         // borrow).
-        drop(null_iterator);
-        drop(fee_iterator);
-        drop(query_stmt_null);
-        drop(query_stmt_fee);
+        drop(tx_iterator);
+        drop(query_stmt);
 
         if retry_store.len() > 0 {
             let tx = self.tx_begin()?;