Merge branch 'develop' into feat/audit-fixes

Author: hstove

stackslib/src/chainstate/stacks/miner.rs

@@ -2228,10 +2228,10 @@ impl StacksBlockBuilder {
 
         debug!("Block transaction selection begins (parent height = {tip_height})");
         let result = {
-            let mut intermediate_result: Result<_, Error> = Ok(0);
+            let mut loop_result = Ok(());
             while block_limit_hit != BlockLimitFunction::LIMIT_REACHED {
                 let mut num_considered = 0;
-                intermediate_result = mempool.iterate_candidates(
+                let intermediate_result = mempool.iterate_candidates(
                     epoch_tx,
                     &mut tx_events,
                     mempool_settings.clone(),
@@ -2390,16 +2390,27 @@ impl StacksBlockBuilder {
                     let _ = mempool.drop_and_blacklist_txs(&to_drop_and_blacklist);
                 }
 
-                if intermediate_result.is_err() {
-                    break;
+                match intermediate_result {
+                    Err(e) => {
+                        loop_result = Err(e);
+                        break;
+                    }
+                    Ok((_txs_considered, stop_reason)) => {
+                        match stop_reason {
+                            MempoolIterationStopReason::NoMoreCandidates => break,
+                            MempoolIterationStopReason::DeadlineReached => break,
+                            // if the iterator function exited, let the loop tick: it checks the block limits
+                            MempoolIterationStopReason::IteratorExited => {}
+                        }
+                    }
                 }
 
                 if num_considered == 0 {
                     break;
                 }
             }
             debug!("Block transaction selection finished (parent height {}): {} transactions selected ({} considered)", &tip_height, num_txs, considered.len());
-            intermediate_result
+            loop_result
         };
 
         mempool.drop_txs(&invalidated_txs)?;

stackslib/src/chainstate/stacks/tests/block_construction.rs

@@ -5072,6 +5072,7 @@ fn paramaterized_mempool_walk_test(
                         },
                     )
                     .unwrap()
+                    .0
                     == 0
                 {
                     break;

stackslib/src/core/mempool.rs

@@ -144,6 +144,14 @@ pub enum MemPoolSyncData {
     TxTags([u8; 32], Vec<TxTag>),
 }
 
+pub enum MempoolIterationStopReason {
+    NoMoreCandidates,
+    DeadlineReached,
+    /// If the iteration function supplied to mempool iteration exited
+    ///  (i.e., the transaction evaluator returned an early exit command)
+    IteratorExited,
+}
+
 impl StacksMessageCodec for MemPoolSyncData {
     fn consensus_serialize<W: Write>(&self, fd: &mut W) -> Result<(), codec_error> {
         match *self {
@@ -1592,7 +1600,7 @@ impl MemPoolDB {
         output_events: &mut Vec<TransactionEvent>,
         settings: MemPoolWalkSettings,
         mut todo: F,
-    ) -> Result<u64, E>
+    ) -> Result<(u64, MempoolIterationStopReason), E>
     where
         C: ClarityConnection,
         F: FnMut(
@@ -1643,11 +1651,11 @@ impl MemPoolDB {
             .query(NO_PARAMS)
             .map_err(|err| Error::SqliteError(err))?;
 
-        loop {
+        let stop_reason = loop {
             if start_time.elapsed().as_millis() > settings.max_walk_time_ms as u128 {
                 debug!("Mempool iteration deadline exceeded";
                        "deadline_ms" => settings.max_walk_time_ms);
-                break;
+                break MempoolIterationStopReason::DeadlineReached;
             }
 
             let start_with_no_estimate =
@@ -1687,7 +1695,7 @@ impl MemPoolDB {
                                 ),
                                 None => {
                                     debug!("No more transactions to consider in mempool");
-                                    break;
+                                    break MempoolIterationStopReason::NoMoreCandidates;
                                 }
                             }
                         }
@@ -1875,7 +1883,7 @@ impl MemPoolDB {
                 }
                 None => {
                     debug!("Mempool iteration early exit from iterator");
-                    break;
+                    break MempoolIterationStopReason::IteratorExited;
                 }
             }
 
@@ -1885,7 +1893,7 @@ impl MemPoolDB {
                 candidate_cache.len()
             );
             candidate_cache.reset();
-        }
+        };
 
         // 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
@@ -1908,7 +1916,7 @@ impl MemPoolDB {
             "considered_txs" => u128::from(total_considered),
             "elapsed_ms" => start_time.elapsed().as_millis()
         );
-        Ok(total_considered)
+        Ok((total_considered, stop_reason))
     }
 
     pub fn conn(&self) -> &DBConn {

testnet/stacks-node/src/tests/signer/v0.rs

@@ -231,27 +231,20 @@ impl SignerTest<SpawnedSigner> {
             Some(self.num_stacking_cycles),
         );
         info!("Waiting for signer set calculation.");
-        let mut reward_set_calculated = false;
-        let short_timeout = Duration::from_secs(60);
-        let now = std::time::Instant::now();
         // Make sure the signer set is calculated before continuing or signers may not
         // recognize that they are registered signers in the subsequent burn block event
         let reward_cycle = self.get_current_reward_cycle() + 1;
-        while !reward_set_calculated {
-            let reward_set = self
+        wait_for(30, || {
+            Ok(self
                 .stacks_client
                 .get_reward_set_signers(reward_cycle)
-                .expect("Failed to check if reward set is calculated");
-            reward_set_calculated = reward_set.is_some();
-            if reward_set_calculated {
-                debug!("Signer set: {:?}", reward_set.unwrap());
-            }
-            std::thread::sleep(Duration::from_secs(1));
-            assert!(
-                now.elapsed() < short_timeout,
-                "Timed out waiting for reward set calculation"
-            );
-        }
+                .expect("Failed to check if reward set is calculated")
+                .map(|reward_set| {
+                    debug!("Signer set: {:?}", reward_set);
+                })
+                .is_some())
+        })
+        .expect("Timed out waiting for reward set calculation");
         info!("Signer set calculated");
 
         // Manually consume one more block to ensure signers refresh their state
@@ -264,8 +257,7 @@ impl SignerTest<SpawnedSigner> {
         info!("Signers initialized");
 
         self.run_until_epoch_3_boundary();
-        std::thread::sleep(Duration::from_secs(1));
-        wait_for(60, || {
+        wait_for(30, || {
             Ok(get_chain_info_opt(&self.running_nodes.conf).is_some())
         })
         .expect("Timed out waiting for network to restart after 3.0 boundary reached");
@@ -275,11 +267,11 @@ impl SignerTest<SpawnedSigner> {
         // could be other miners mining blocks.
         let height_before = get_chain_info(&self.running_nodes.conf).stacks_tip_height;
         info!("Waiting for first Nakamoto block: {}", height_before + 1);
-        next_block_and(&mut self.running_nodes.btc_regtest_controller, 60, || {
-            let height = get_chain_info(&self.running_nodes.conf).stacks_tip_height;
-            Ok(height > height_before)
+        self.mine_nakamoto_block(Duration::from_secs(30));
+        wait_for(30, || {
+            Ok(get_chain_info(&self.running_nodes.conf).stacks_tip_height > height_before)
         })
-        .unwrap();
+        .expect("Timed out waiting for first Nakamoto block after 3.0 boundary");
         info!("Ready to mine Nakamoto blocks!");
     }
 
@@ -553,19 +545,9 @@ fn miner_gather_signatures() {
     let num_signers = 5;
     let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new(num_signers, vec![]);
     let timeout = Duration::from_secs(30);
-    let mined_blocks = signer_test.running_nodes.nakamoto_blocks_mined.clone();
-    let blocks_mined_before = mined_blocks.load(Ordering::SeqCst);
 
     signer_test.boot_to_epoch_3();
 
-    // give the system a chance to reach the Nakamoto start tip
-    // mine a Nakamoto block
-    wait_for(30, || {
-        let blocks_mined = mined_blocks.load(Ordering::SeqCst);
-        Ok(blocks_mined > blocks_mined_before)
-    })
-    .unwrap();
-
     info!("------------------------- Test Mine and Verify Confirmed Nakamoto Block -------------------------");
     signer_test.mine_and_verify_confirmed_naka_block(timeout, num_signers);
 
@@ -2317,7 +2299,7 @@ fn empty_sortition() {
     let send_amt = 100;
     let send_fee = 180;
     let recipient = PrincipalData::from(StacksAddress::burn_address(false));
-    let block_proposal_timeout = Duration::from_secs(5);
+    let block_proposal_timeout = Duration::from_secs(20);
     let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
         num_signers,
         vec![(sender_addr.clone(), send_amt + send_fee)],