Merge pull request #6282 from jferrant/bug/fix-handle-block-validate-ok

Properly handle block rejection case in handle_block_validate_ok

Author: jferrant

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

@@ -12066,6 +12066,28 @@ fn no_reorg_due_to_successive_block_validation_ok() {
     TEST_STALL_BLOCK_VALIDATION_SUBMISSION.set(false);
 
     info!("------------------------- Confirm N+1' is Rejected ------------------------");
+    // Confirm that every single signer has rejected the block and recorded its own rejection signature in its own DB
+    wait_for(30, || {
+        Ok(miners.signer_test.signer_configs.iter().all(|config| {
+            let conn = Connection::open(config.db_path.clone()).unwrap();
+            let mut stmt = conn
+                .prepare(
+                    "SELECT 1 FROM block_rejection_signer_addrs
+                WHERE signer_signature_hash = ?1 AND signer_addr = ?2
+                LIMIT 1",
+                )
+                .unwrap();
+
+            let mut rows = stmt
+                .query(rusqlite::params![
+                    block_n_1_prime_signature_hash,
+                    config.stacks_address
+                ])
+                .unwrap();
+            rows.next().unwrap().is_some()
+        }))
+    })
+    .expect("Failed to verify all signers recorded a signature rejection");
     wait_for_block_global_rejection(30, block_n_1_prime_signature_hash, num_signers)
         .expect("Failed to find block N+1'");
 

stacks-signer/src/chainstate/mod.rs

@@ -327,7 +327,7 @@ impl SortitionData {
 
         if let Some(info) = last_block_info {
             // N.B. this block might not be the last globally accepted block across the network;
-            // it's just the highest one in this tenure that we knnge: &TenureChangePow about.  If this given block is
+            // it's just the highest one in this tenure that we know about.  If this given block is
             // no higher than it, then it's definitely no higher than the last globally accepted
             // block across the network, so we can do an early rejection here.
             if block.header.chain_length <= info.block.header.chain_length {

stacks-signer/src/v0/signer.rs

@@ -1162,6 +1162,7 @@ impl Signer {
         &mut self,
         stacks_client: &StacksClient,
         block_validate_ok: &BlockValidateOk,
+        sortition_state: &mut Option<SortitionsView>,
     ) -> Option<BlockResponse> {
         crate::monitoring::actions::increment_block_validation_responses(true);
         let signer_signature_hash = block_validate_ok.signer_signature_hash;
@@ -1201,17 +1202,18 @@ impl Signer {
         if let Some(block_response) =
             self.check_block_against_signer_db_state(stacks_client, &block_info.block)
         {
+            let block_rejection = block_response.as_block_rejection()?;
             // The signer db state has changed. We no longer view this block as valid. Override the validation response.
             if let Err(e) = block_info.mark_locally_rejected() {
                 if !block_info.has_reached_consensus() {
                     warn!("{self}: Failed to mark block as locally rejected: {e:?}");
                 }
             };
-            self.impl_send_block_response(Some(&block_info.block), block_response);
             self.signer_db
                 .insert_block(&block_info)
                 .unwrap_or_else(|e| self.handle_insert_block_error(e));
-            None
+            self.handle_block_rejection(block_rejection, sortition_state);
+            Some(block_response)
         } else {
             if let Err(e) = block_info.mark_locally_accepted(false) {
                 if !block_info.has_reached_consensus() {
@@ -1301,7 +1303,7 @@ impl Signer {
                 crate::monitoring::actions::record_block_validation_latency(
                     block_validate_ok.validation_time_ms,
                 );
-                self.handle_block_validate_ok(stacks_client, block_validate_ok)
+                self.handle_block_validate_ok(stacks_client, block_validate_ok, sortition_state)
             }
             BlockValidateResponse::Reject(block_validate_reject) => {
                 self.handle_block_validate_reject(block_validate_reject, sortition_state)