fix: dont remove pending validation in tests

Author: hstove

stacks-signer/src/signerdb.rs

@@ -1025,18 +1025,25 @@ impl SignerDb {
 
     /// Get a pending block validation, sorted by the time at which it was added to the pending table.
     /// If found, remove it from the pending table.
-    pub fn get_pending_block_validation(&self) -> Result<Option<Sha512Trunc256Sum>, DBError> {
-        let qry =
-            "SELECT signer_signature_hash FROM block_validations_pending ORDER BY added_time ASC";
-        let sighash_opt: Option<String> = query_row(&self.db, qry, params![])?;
-        if let Some(sighash) = sighash_opt {
-            let sighash = Sha512Trunc256Sum::from_hex(&sighash).map_err(|_| DBError::Corruption)?;
+    pub fn get_and_remove_pending_block_validation(
+        &self,
+    ) -> Result<Option<Sha512Trunc256Sum>, DBError> {
+        if let Some(sighash) = self.get_pending_block_validation()? {
             self.remove_pending_block_validation(&sighash)?;
             return Ok(Some(sighash));
         }
         Ok(None)
     }
 
+    /// Get a pending block validation, sorted by the time at which it was added to the pending table.
+    pub fn get_pending_block_validation(&self) -> Result<Option<Sha512Trunc256Sum>, DBError> {
+        let qry =
+            "SELECT signer_signature_hash FROM block_validations_pending ORDER BY added_time ASC";
+        let args = params![];
+        let sighash: Option<String> = query_row(&self.db, qry, args)?;
+        Ok(sighash.and_then(|sighash| Sha512Trunc256Sum::from_hex(&sighash).ok()))
+    }
+
     /// Remove a pending block validation
     pub fn remove_pending_block_validation(
         &self,
@@ -1067,9 +1074,20 @@ impl SignerDb {
     pub fn get_all_pending_block_validations(
         &self,
     ) -> Result<Vec<PendingBlockValidation>, DBError> {
-        let qry = "SELECT signer_signature_hash, added_time FROM block_validations_pending";
-        let args = params![];
-        query_rows(&self.db, qry, args)
+        let qry = "SELECT signer_signature_hash, added_time FROM block_validations_pending ORDER BY added_time ASC";
+        query_rows(&self.db, qry, params![])
+    }
+
+    /// For tests, check if a pending block validation exists
+    #[cfg(any(test, feature = "testing"))]
+    pub fn has_pending_block_validation(
+        &self,
+        sighash: &Sha512Trunc256Sum,
+    ) -> Result<bool, DBError> {
+        let qry = "SELECT signer_signature_hash FROM block_validations_pending WHERE signer_signature_hash = ?1";
+        let args = params![sighash.to_string()];
+        let sighash_opt: Option<String> = query_row(&self.db, qry, args)?;
+        Ok(sighash_opt.is_some())
     }
 
     /// Return the start time (epoch time in seconds) and the processing time in milliseconds of the tenure (idenfitied by consensus_hash).

stacks-signer/src/v0/signer.rs

@@ -533,7 +533,14 @@ impl Signer {
                 // from other signers to push the proposed block into a global rejection/acceptance regardless of our participation.
                 // However, we will not be able to participate beyond this until our block submission times out or we receive a response
                 // from our node.
-                warn!("{self}: cannot submit block proposal for validation as we are already waiting for a response for a prior submission")
+                warn!("{self}: cannot submit block proposal for validation as we are already waiting for a response for a prior submission. Inserting pending proposal.";
+                    "signer_signature_hash" => signer_signature_hash.to_string(),
+                );
+                self.signer_db
+                    .insert_pending_block_validation(&signer_signature_hash, get_epoch_time_secs())
+                    .unwrap_or_else(|e| {
+                        warn!("{self}: Failed to insert pending block validation: {e:?}")
+                    });
             }
 
             // Do not store KNOWN invalid blocks as this could DOS the signer. We only store blocks that are valid or unknown.
@@ -726,7 +733,7 @@ impl Signer {
         }
 
         // Check if there is a pending block validation that we need to submit to the node
-        match self.signer_db.get_pending_block_validation() {
+        match self.signer_db.get_and_remove_pending_block_validation() {
             Ok(Some(signer_sig_hash)) => {
                 info!("{self}: Found a pending block validation: {signer_sig_hash:?}");
                 match self.signer_db.block_lookup(&signer_sig_hash) {

stackslib/src/net/api/postblock_proposal.rs

@@ -185,6 +185,19 @@ impl BlockValidateResponse {
     }
 }
 
+#[cfg(any(test, feature = "testing"))]
+fn get_test_delay() -> Option<u64> {
+    TEST_VALIDATE_DELAY_DURATION_SECS.lock().unwrap().clone()
+}
+
+#[cfg(any(test, feature = "testing"))]
+fn inject_validation_delay() {
+    if let Some(delay) = get_test_delay() {
+        warn!("Sleeping for {} seconds to simulate slow processing", delay);
+        thread::sleep(Duration::from_secs(delay));
+    }
+}
+
 /// Represents a block proposed to the `v3/block_proposal` endpoint for validation
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct NakamotoBlockProposal {
@@ -377,12 +390,7 @@ impl NakamotoBlockProposal {
         let start = Instant::now();
 
         #[cfg(any(test, feature = "testing"))]
-        {
-            if let Some(delay) = *TEST_VALIDATE_DELAY_DURATION_SECS.lock().unwrap() {
-                warn!("Sleeping for {} seconds to simulate slow processing", delay);
-                thread::sleep(Duration::from_secs(delay));
-            }
-        }
+        inject_validation_delay();
 
         let mainnet = self.chain_id == CHAIN_ID_MAINNET;
         if self.chain_id != chainstate.chain_id || mainnet != chainstate.mainnet {

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

@@ -7796,9 +7796,7 @@ fn block_validation_pending_table() {
     let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
         num_signers,
         vec![(sender_addr, send_amt + send_fee)],
-        |config| {
-            config.block_proposal_validation_timeout = timeout;
-        },
+        |_| {},
         |_| {},
         None,
         None,
@@ -7869,44 +7867,33 @@ fn block_validation_pending_table() {
     let mut last_log = Instant::now();
     last_log -= Duration::from_secs(5);
     wait_for(120, || {
-        let sighash = match signer_db.get_pending_block_validation() {
-            Ok(Some(sighash)) => sighash,
-            Err(e) => {
-                error!("Failed to get pending block validation: {e}");
-                panic!("Failed to get pending block validation");
-            }
-            Ok(None) => {
-                if last_log.elapsed() > Duration::from_secs(5) {
-                    info!("----- No pending block validations found -----");
-                    last_log = Instant::now();
-                }
-                return Ok(false);
-            }
-        };
-        if last_log.elapsed() > Duration::from_secs(5) && sighash != block_signer_signature_hash {
+        let is_pending = signer_db
+            .has_pending_block_validation(&block_signer_signature_hash)
+            .expect("Unexpected DBError");
+        if last_log.elapsed() > Duration::from_secs(5) && !is_pending {
             let pending_block_validations = signer_db
                 .get_all_pending_block_validations()
                 .expect("Failed to get pending block validations");
             info!(
-                "----- Received a different pending block proposal -----";
-                "db_signer_signature_hash" => sighash.to_hex(),
+                "----- Waiting for pending block proposal in SignerDB -----";
                 "proposed_signer_signature_hash" => block_signer_signature_hash.to_hex(),
+                "pending_block_validations_len" => pending_block_validations.len(),
                 "pending_block_validations" => pending_block_validations.iter()
                     .map(|p| p.signer_signature_hash.to_hex())
                     .collect::<Vec<String>>()
                     .join(", "),
             );
             last_log = Instant::now();
         }
-        Ok(sighash == block_signer_signature_hash)
+        Ok(is_pending)
     })
     .expect("Timed out waiting for pending block proposal");
 
-    // Set the delay to 0 so that the block validation finishes quickly
-    TEST_VALIDATE_DELAY_DURATION_SECS.lock().unwrap().take();
-
     info!("----- Waiting for pending block validation to be submitted -----");
 
+    // Set the delay to 0 so that the block validation finishes quickly
+    *TEST_VALIDATE_DELAY_DURATION_SECS.lock().unwrap() = None;
+
     wait_for(30, || {
         let proposal_responses = test_observer::get_proposal_responses();
         let found_proposal = proposal_responses
@@ -7918,11 +7905,10 @@ fn block_validation_pending_table() {
 
     info!("----- Waiting for pending block validation to be removed -----");
     wait_for(30, || {
-        let Ok(Some(sighash)) = signer_db.get_pending_block_validation() else {
-            // There are no pending block validations
-            return Ok(true);
-        };
-        Ok(sighash != block_signer_signature_hash)
+        let is_pending = signer_db
+            .has_pending_block_validation(&block_signer_signature_hash)
+            .expect("Unexpected DBError");
+        Ok(!is_pending)
     })
     .expect("Timed out waiting for pending block validation to be removed");