Fix validation queue race condition in block approval vs validaiton submission

.github/workflows/bitcoin-tests.yml

@@ -127,6 +127,7 @@ jobs:
           - tests::signer::v0::continue_after_fast_block_no_sortition
           - tests::signer::v0::block_validation_response_timeout
           - tests::signer::v0::tenure_extend_after_bad_commit
+          - tests::signer::v0::no_reorg_due_to_successive_block_validation_ok
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state

stacks-signer/src/chainstate.rs

@@ -502,7 +502,7 @@ impl SortitionsView {
 
     /// Get the last block from the given tenure
     /// Returns the last locally accepted block if it is not timed out, otherwise it will return the last globally accepted block.
-    fn get_tenure_last_block_info(
+    pub fn get_tenure_last_block_info(
         consensus_hash: &ConsensusHash,
         signer_db: &SignerDb,
         tenure_last_block_proposal_timeout: Duration,
@@ -514,7 +514,7 @@ impl SortitionsView {
 
         if let Some(local_info) = last_locally_accepted_block {
             if let Some(signed_over_time) = local_info.signed_self {
-                if signed_over_time + tenure_last_block_proposal_timeout.as_secs()
+                if signed_over_time.saturating_add(tenure_last_block_proposal_timeout.as_secs())
                     > get_epoch_time_secs()
                 {
                     // The last locally accepted block is not timed out, return it

stacks-signer/src/client/stacks_client.rs

@@ -312,8 +312,24 @@ impl StacksClient {
         }
     }
 
+    #[cfg(any(test, feature = "testing"))]
+    fn test_stall_block_validation_submission() {
+        use crate::v0::signer::TEST_STALL_BLOCK_VALIDATION_SUBMISSION;
+
+        if *TEST_STALL_BLOCK_VALIDATION_SUBMISSION.lock().unwrap() == Some(true) {
+            // Do an extra check just so we don't log EVERY time.
+            warn!("Block validation submission is stalled due to testing directive");
+            while *TEST_STALL_BLOCK_VALIDATION_SUBMISSION.lock().unwrap() == Some(true) {
+                std::thread::sleep(std::time::Duration::from_millis(10));
+            }
+            warn!("Block validation submission is no longer stalled due to testing directive. Continuing...");
+        }
+    }
+
     /// Submit the block proposal to the stacks node. The block will be validated and returned via the HTTP endpoint for Block events.
     pub fn submit_block_for_validation(&self, block: NakamotoBlock) -> Result<(), ClientError> {
+        #[cfg(any(test, feature = "testing"))]
+        Self::test_stall_block_validation_submission();
         debug!("StacksClient: Submitting block for validation";
             "signer_sighash" => %block.header.signer_signature_hash(),
             "block_id" => %block.header.block_id(),

stacks-signer/src/signerdb.rs

@@ -613,6 +613,18 @@ impl SignerDb {
         try_deserialize(result)
     }
 
+    /// Return the last accepted block the signer (highest stacks height). It will tie break a match based on which was more recently signed.
+    pub fn get_signer_last_accepted_block(&self) -> Result<Option<BlockInfo>, DBError> {
+        let query = "SELECT block_info FROM blocks WHERE json_extract(block_info, '$.state') IN (?1, ?2) ORDER BY stacks_height DESC, json_extract(block_info, '$.signed_group') DESC, json_extract(block_info, '$.signed_self') DESC LIMIT 1";
+        let args = params![
+            &BlockState::GloballyAccepted.to_string(),
+            &BlockState::LocallyAccepted.to_string()
+        ];
+        let result: Option<String> = query_row(&self.db, query, args)?;
+
+        try_deserialize(result)
+    }
+
     /// Return the last accepted block in a tenure (identified by its consensus hash).
     pub fn get_last_accepted_block(
         &self,
@@ -1317,4 +1329,69 @@ mod tests {
 
         assert_eq!(db.get_canonical_tip().unwrap().unwrap(), block_info_2);
     }
+
+    #[test]
+    fn signer_last_accepted_block() {
+        let db_path = tmp_db_path();
+        let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+
+        let (mut block_info_1, _block_proposal_1) = create_block_override(|b| {
+            b.block.header.miner_signature = MessageSignature([0x01; 65]);
+            b.block.header.chain_length = 1;
+            b.burn_height = 1;
+        });
+
+        let (mut block_info_2, _block_proposal_2) = create_block_override(|b| {
+            b.block.header.miner_signature = MessageSignature([0x02; 65]);
+            b.block.header.chain_length = 2;
+            b.burn_height = 1;
+        });
+
+        let (mut block_info_3, _block_proposal_3) = create_block_override(|b| {
+            b.block.header.miner_signature = MessageSignature([0x02; 65]);
+            b.block.header.chain_length = 2;
+            b.burn_height = 4;
+        });
+        block_info_3
+            .mark_locally_accepted(false)
+            .expect("Failed to mark block as locally accepted");
+
+        db.insert_block(&block_info_1)
+            .expect("Unable to insert block into db");
+        db.insert_block(&block_info_2)
+            .expect("Unable to insert block into db");
+
+        assert!(db.get_signer_last_accepted_block().unwrap().is_none());
+
+        block_info_1
+            .mark_globally_accepted()
+            .expect("Failed to mark block as globally accepted");
+        db.insert_block(&block_info_1)
+            .expect("Unable to insert block into db");
+
+        assert_eq!(
+            db.get_signer_last_accepted_block().unwrap().unwrap(),
+            block_info_1
+        );
+
+        block_info_2
+            .mark_globally_accepted()
+            .expect("Failed to mark block as globally accepted");
+        block_info_2.signed_self = Some(get_epoch_time_secs());
+        db.insert_block(&block_info_2)
+            .expect("Unable to insert block into db");
+
+        assert_eq!(
+            db.get_signer_last_accepted_block().unwrap().unwrap(),
+            block_info_2
+        );
+
+        db.insert_block(&block_info_3)
+            .expect("Unable to insert block into db");
+
+        assert_eq!(
+            db.get_signer_last_accepted_block().unwrap().unwrap(),
+            block_info_3
+        );
+    }
 }

stacks-signer/src/tests/chainstate.rs

@@ -457,8 +457,12 @@ fn check_sortition_timeout() {
     fs::create_dir_all(signer_db_dir).unwrap();
     let mut signer_db = SignerDb::new(signer_db_path).unwrap();
 
+    let block_sk = StacksPrivateKey::from_seed(&[0, 1]);
+    let block_pk = StacksPublicKey::from_private(&block_sk);
+    let block_pkh = Hash160::from_node_public_key(&block_pk);
+
     let mut sortition = SortitionState {
-        miner_pkh: Hash160([0; 20]),
+        miner_pkh: block_pkh,
         miner_pubkey: None,
         prior_sortition: ConsensusHash([0; 20]),
         parent_tenure_id: ConsensusHash([0; 20]),