Fix validation queue race condition in block approval vs validaiton submission

.github/workflows/bitcoin-tests.yml

@@ -132,6 +132,8 @@ 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::block_proposal_max_age_rejections
+          - tests::signer::v0::global_acceptance_depends_on_block_announcement
           - tests::signer::v0::no_reorg_due_to_successive_block_validation_ok
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights

libsigner/src/v0/messages.rs

@@ -686,6 +686,22 @@ impl BlockResponse {
             BlockResponse::Rejected(rejection) => rejection.signer_signature_hash,
         }
     }
+
+    /// Get the block accept data from the block response
+    pub fn as_block_accepted(&self) -> Option<&BlockAccepted> {
+        match self {
+            BlockResponse::Accepted(accepted) => Some(accepted),
+            _ => None,
+        }
+    }
+
+    /// Get the block accept data from the block response
+    pub fn as_block_rejection(&self) -> Option<&BlockRejection> {
+        match self {
+            BlockResponse::Rejected(rejection) => Some(rejection),
+            _ => None,
+        }
+    }
 }
 
 impl StacksMessageCodec for BlockResponse {

stacks-signer/src/client/stacks_client.rs

@@ -312,24 +312,8 @@ 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

@@ -1490,6 +1490,286 @@ mod tests {
         assert_eq!(db.get_canonical_tip().unwrap().unwrap(), block_info_2);
     }
 
+    #[test]
+    fn get_accepted_blocks() {
+        let db_path = tmp_db_path();
+        let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+        let consensus_hash_1 = ConsensusHash([0x01; 20]);
+        let consensus_hash_2 = ConsensusHash([0x02; 20]);
+        let consensus_hash_3 = ConsensusHash([0x03; 20]);
+        let (mut block_info_1, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            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) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.miner_signature = MessageSignature([0x02; 65]);
+            b.block.header.chain_length = 2;
+            b.burn_height = 2;
+        });
+        let (mut block_info_3, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.miner_signature = MessageSignature([0x03; 65]);
+            b.block.header.chain_length = 3;
+            b.burn_height = 3;
+        });
+        let (mut block_info_4, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_2;
+            b.block.header.miner_signature = MessageSignature([0x03; 65]);
+            b.block.header.chain_length = 3;
+            b.burn_height = 4;
+        });
+        block_info_1.mark_globally_accepted().unwrap();
+        block_info_2.mark_locally_accepted(false).unwrap();
+        block_info_3.mark_locally_accepted(false).unwrap();
+        block_info_4.mark_globally_accepted().unwrap();
+
+        db.insert_block(&block_info_1).unwrap();
+        db.insert_block(&block_info_2).unwrap();
+        db.insert_block(&block_info_3).unwrap();
+        db.insert_block(&block_info_4).unwrap();
+
+        // Verify tenure consensus_hash_1
+        let block_info = db
+            .get_last_accepted_block(&consensus_hash_1)
+            .unwrap()
+            .unwrap();
+        assert_eq!(block_info, block_info_3);
+        let block_info = db
+            .get_last_globally_accepted_block(&consensus_hash_1)
+            .unwrap()
+            .unwrap();
+        assert_eq!(block_info, block_info_1);
+
+        // Verify tenure consensus_hash_2
+        let block_info = db
+            .get_last_accepted_block(&consensus_hash_2)
+            .unwrap()
+            .unwrap();
+        assert_eq!(block_info, block_info_4);
+        let block_info = db
+            .get_last_globally_accepted_block(&consensus_hash_2)
+            .unwrap()
+            .unwrap();
+        assert_eq!(block_info, block_info_4);
+
+        // Verify tenure consensus_hash_3
+        assert!(db
+            .get_last_accepted_block(&consensus_hash_3)
+            .unwrap()
+            .is_none());
+        assert!(db
+            .get_last_globally_accepted_block(&consensus_hash_3)
+            .unwrap()
+            .is_none());
+    }
+
+    fn generate_tenure_blocks() -> Vec<BlockInfo> {
+        let tenure_change_payload = TenureChangePayload {
+            tenure_consensus_hash: ConsensusHash([0x04; 20]), // same as in nakamoto header
+            prev_tenure_consensus_hash: ConsensusHash([0x01; 20]),
+            burn_view_consensus_hash: ConsensusHash([0x04; 20]),
+            previous_tenure_end: StacksBlockId([0x03; 32]),
+            previous_tenure_blocks: 1,
+            cause: TenureChangeCause::BlockFound,
+            pubkey_hash: Hash160::from_node_public_key(&StacksPublicKey::from_private(
+                &StacksPrivateKey::new(),
+            )),
+        };
+        let tenure_change_tx_payload =
+            TransactionPayload::TenureChange(tenure_change_payload.clone());
+        let tenure_change_tx = StacksTransaction::new(
+            TransactionVersion::Testnet,
+            TransactionAuth::from_p2pkh(&StacksPrivateKey::new()).unwrap(),
+            tenure_change_tx_payload.clone(),
+        );
+
+        let consensus_hash_1 = ConsensusHash([0x01; 20]);
+        let consensus_hash_2 = ConsensusHash([0x02; 20]);
+        let (mut block_info_1, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.miner_signature = MessageSignature([0x01; 65]);
+            b.block.header.chain_length = 1;
+            b.burn_height = 1;
+        });
+        block_info_1.state = BlockState::GloballyAccepted;
+        block_info_1.block.txs.push(tenure_change_tx.clone());
+        block_info_1.validation_time_ms = Some(1000);
+        block_info_1.proposed_time = get_epoch_time_secs() + 500;
+
+        let (mut block_info_2, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.miner_signature = MessageSignature([0x02; 65]);
+            b.block.header.chain_length = 2;
+            b.burn_height = 2;
+        });
+        block_info_2.state = BlockState::GloballyAccepted;
+        block_info_2.validation_time_ms = Some(2000);
+        block_info_2.proposed_time = block_info_1.proposed_time + 5;
+
+        let (mut block_info_3, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.miner_signature = MessageSignature([0x03; 65]);
+            b.block.header.chain_length = 3;
+            b.burn_height = 2;
+        });
+        block_info_3.state = BlockState::GloballyAccepted;
+        block_info_3.block.txs.push(tenure_change_tx);
+        block_info_3.validation_time_ms = Some(5000);
+        block_info_3.proposed_time = block_info_1.proposed_time + 10;
+
+        // This should have no effect on the time calculations as its not a globally accepted block
+        let (mut block_info_4, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.miner_signature = MessageSignature([0x04; 65]);
+            b.block.header.chain_length = 3;
+            b.burn_height = 2;
+        });
+        block_info_4.state = BlockState::LocallyAccepted;
+        block_info_4.validation_time_ms = Some(9000);
+        block_info_4.proposed_time = block_info_1.proposed_time + 15;
+
+        let (mut block_info_5, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_2;
+            b.block.header.miner_signature = MessageSignature([0x05; 65]);
+            b.block.header.chain_length = 4;
+            b.burn_height = 3;
+        });
+        block_info_5.state = BlockState::GloballyAccepted;
+        block_info_5.validation_time_ms = Some(20000);
+        block_info_5.proposed_time = block_info_1.proposed_time + 20;
+
+        // This should have no effect on the time calculations as its not a globally accepted block
+        let (mut block_info_6, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_2;
+            b.block.header.miner_signature = MessageSignature([0x06; 65]);
+            b.block.header.chain_length = 5;
+            b.burn_height = 3;
+        });
+        block_info_6.state = BlockState::LocallyAccepted;
+        block_info_6.validation_time_ms = Some(40000);
+        block_info_6.proposed_time = block_info_1.proposed_time + 25;
+
+        vec![
+            block_info_1,
+            block_info_2,
+            block_info_3,
+            block_info_4,
+            block_info_5,
+            block_info_6,
+        ]
+    }
+
+    #[test]
+    fn tenure_times() {
+        let db_path = tmp_db_path();
+        let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+        let block_infos = generate_tenure_blocks();
+        let consensus_hash_1 = block_infos[0].block.header.consensus_hash;
+        let consensus_hash_2 = block_infos.last().unwrap().block.header.consensus_hash;
+        let consensus_hash_3 = ConsensusHash([0x03; 20]);
+
+        db.insert_block(&block_infos[0]).unwrap();
+        db.insert_block(&block_infos[1]).unwrap();
+
+        // Verify tenure consensus_hash_1
+        let (start_time, processing_time) = db.get_tenure_times(&consensus_hash_1).unwrap();
+        assert_eq!(start_time, block_infos[0].proposed_time);
+        assert_eq!(processing_time, 3000);
+
+        db.insert_block(&block_infos[2]).unwrap();
+        db.insert_block(&block_infos[3]).unwrap();
+
+        let (start_time, processing_time) = db.get_tenure_times(&consensus_hash_1).unwrap();
+        assert_eq!(start_time, block_infos[2].proposed_time);
+        assert_eq!(processing_time, 5000);
+
+        db.insert_block(&block_infos[4]).unwrap();
+        db.insert_block(&block_infos[5]).unwrap();
+
+        // Verify tenure consensus_hash_2
+        let (start_time, processing_time) = db.get_tenure_times(&consensus_hash_2).unwrap();
+        assert_eq!(start_time, block_infos[4].proposed_time);
+        assert_eq!(processing_time, 20000);
+
+        // Verify tenure consensus_hash_3 (unknown hash)
+        let (start_time, validation_time) = db.get_tenure_times(&consensus_hash_3).unwrap();
+        assert!(start_time < block_infos[0].proposed_time, "Should have been generated from get_epoch_time_secs() making it much older than our artificially late proposal times");
+        assert_eq!(validation_time, 0);
+    }
+
+    #[test]
+    fn tenure_extend_timestamp() {
+        let db_path = tmp_db_path();
+        let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+
+        let block_infos = generate_tenure_blocks();
+        let mut unknown_block = block_infos[0].block.clone();
+        unknown_block.header.consensus_hash = ConsensusHash([0x03; 20]);
+
+        db.insert_block(&block_infos[0]).unwrap();
+        db.insert_block(&block_infos[1]).unwrap();
+
+        let tenure_idle_timeout = Duration::from_secs(10);
+        // Verify tenure consensus_hash_1
+        let timestamp_hash_1_before =
+            db.calculate_tenure_extend_timestamp(tenure_idle_timeout, &block_infos[0].block, true);
+        assert_eq!(
+            timestamp_hash_1_before,
+            block_infos[0]
+                .proposed_time
+                .saturating_add(tenure_idle_timeout.as_secs())
+                .saturating_add(3)
+        );
+
+        db.insert_block(&block_infos[2]).unwrap();
+        db.insert_block(&block_infos[3]).unwrap();
+
+        let timestamp_hash_1_after =
+            db.calculate_tenure_extend_timestamp(tenure_idle_timeout, &block_infos[0].block, true);
+
+        assert_eq!(
+            timestamp_hash_1_after,
+            block_infos[2]
+                .proposed_time
+                .saturating_add(tenure_idle_timeout.as_secs())
+                .saturating_add(5)
+        );
+
+        db.insert_block(&block_infos[4]).unwrap();
+        db.insert_block(&block_infos[5]).unwrap();
+
+        // Verify tenure consensus_hash_2
+        let timestamp_hash_2 = db.calculate_tenure_extend_timestamp(
+            tenure_idle_timeout,
+            &block_infos.last().unwrap().block,
+            true,
+        );
+        assert_eq!(
+            timestamp_hash_2,
+            block_infos[4]
+                .proposed_time
+                .saturating_add(tenure_idle_timeout.as_secs())
+                .saturating_add(20)
+        );
+
+        let now = get_epoch_time_secs().saturating_add(tenure_idle_timeout.as_secs());
+        let timestamp_hash_2_no_tenure_extend =
+            db.calculate_tenure_extend_timestamp(tenure_idle_timeout, &block_infos[0].block, false);
+        assert_ne!(timestamp_hash_2, timestamp_hash_2_no_tenure_extend);
+        assert!(now < timestamp_hash_2_no_tenure_extend);
+
+        // Verify tenure consensus_hash_3 (unknown hash)
+        let timestamp_hash_3 =
+            db.calculate_tenure_extend_timestamp(tenure_idle_timeout, &unknown_block, true);
+        assert!(
+            timestamp_hash_3.saturating_add(tenure_idle_timeout.as_secs())
+                < block_infos[0].proposed_time
+        );
+    }
+
     #[test]
     fn signer_last_accepted_block() {
         let db_path = tmp_db_path();