Merge pull request #6080 from jferrant/chore/cleanup-signers-send-response

Chore/signer: Use send_block_response where possible

Author: jferrant

stacks-signer/src/v0/signer.rs

@@ -408,13 +408,8 @@ impl Signer {
                             block_response,
                             sortition_state,
                         ),
-                        SignerMessage::StateMachineUpdate(update) => {
-                            self.handle_state_machine_update(
-                                signer_public_key,
-                                update,
-                                received_time,
-                            );
-                        }
+                        SignerMessage::StateMachineUpdate(update) => self
+                            .handle_state_machine_update(signer_public_key, update, received_time),
                         _ => {}
                     }
                 }
@@ -670,22 +665,43 @@ impl Signer {
 
     /// The actual `send_block_response` implementation. Declared so that we do
     /// not need to duplicate in testing.
-    fn impl_send_block_response(&mut self, block_response: BlockResponse) {
-        let res = self
+    fn impl_send_block_response(
+        &mut self,
+        block: Option<&NakamotoBlock>,
+        block_response: BlockResponse,
+    ) {
+        info!(
+            "{self}: Broadcasting a block response to stacks node: {block_response:?}";
+        );
+        let accepted = matches!(block_response, BlockResponse::Accepted(..));
+        match self
             .stackerdb
-            .send_message_with_retry::<SignerMessage>(block_response.clone().into());
-        match res {
-            Err(e) => warn!("{self}: Failed to send block rejection to stacker-db: {e:?}"),
-            Ok(ack) if !ack.accepted => warn!(
-                "{self}: Block rejection not accepted by stacker-db: {:?}",
-                ack.reason
-            ),
-            Ok(_) => debug!("{self}: Block rejection accepted by stacker-db"),
+            .send_message_with_retry::<SignerMessage>(block_response.into())
+        {
+            Ok(ack) => {
+                if !ack.accepted {
+                    warn!(
+                        "{self}: Block response not accepted by stacker-db: {:?}",
+                        ack.reason
+                    );
+                }
+                crate::monitoring::actions::increment_block_responses_sent(accepted);
+                if let Some(block) = block {
+                    crate::monitoring::actions::record_block_response_latency(block);
+                }
+            }
+            Err(e) => {
+                warn!("{self}: Failed to send block response to stacker-db: {e:?}",);
+            }
         }
     }
 
     #[cfg(any(test, feature = "testing"))]
-    fn send_block_response(&mut self, block_response: BlockResponse) {
+    fn send_block_response(
+        &mut self,
+        block: Option<&NakamotoBlock>,
+        block_response: BlockResponse,
+    ) {
         const NUM_REPEATS: usize = 1;
         let mut count = 0;
         let public_keys = TEST_REPEAT_PROPOSAL_RESPONSE.get();
@@ -695,16 +711,20 @@ impl Signer {
             count = NUM_REPEATS;
         }
         while count <= NUM_REPEATS {
-            self.impl_send_block_response(block_response.clone());
+            self.impl_send_block_response(block, block_response.clone());
 
             count += 1;
             sleep_ms(1000);
         }
     }
 
     #[cfg(not(any(test, feature = "testing")))]
-    fn send_block_response(&mut self, block_response: BlockResponse) {
-        self.impl_send_block_response(block_response)
+    fn send_block_response(
+        &mut self,
+        block: Option<&NakamotoBlock>,
+        block_response: BlockResponse,
+    ) {
+        self.impl_send_block_response(block, block_response)
     }
 
     /// Handle signer state update message
@@ -834,8 +854,7 @@ impl Signer {
 
         if let Some(block_response) = block_response {
             // We know proposal is invalid. Send rejection message, do not do further validation and do not store it.
-            debug!("{self}: Broadcasting a block response to stacks node: {block_response:?}");
-            self.send_block_response(block_response);
+            self.send_block_response(Some(&block_info.block), block_response);
         } else {
             // Just in case check if the last block validation submission timed out.
             self.check_submitted_block_proposal();
@@ -889,19 +908,7 @@ impl Signer {
             return;
         };
 
-        // Submit a proposal response to the .signers contract for miners
-        debug!("{self}: Broadcasting a block response to stacks node: {block_response:?}");
-
-        let accepted = matches!(block_response, BlockResponse::Accepted(..));
-        if let Err(e) = self
-            .stackerdb
-            .send_message_with_retry::<SignerMessage>(block_response.into())
-        {
-            warn!("{self}: Failed to send block response to stacker-db: {e:?}");
-        } else {
-            crate::monitoring::actions::increment_block_responses_sent(accepted);
-            crate::monitoring::actions::record_block_response_latency(&block_info.block);
-        }
+        self.impl_send_block_response(Some(&block_info.block), block_response);
     }
 
     /// Handle block response messages from a signer
@@ -1037,21 +1044,7 @@ impl Signer {
                     warn!("{self}: Failed to mark block as locally rejected: {e:?}");
                 }
             };
-            debug!("{self}: Broadcasting a block response to stacks node: {block_response:?}");
-            let res = self
-                .stackerdb
-                .send_message_with_retry::<SignerMessage>(block_response.into());
-
-            crate::monitoring::actions::record_block_response_latency(&block_info.block);
-
-            match res {
-                Err(e) => warn!("{self}: Failed to send block rejection to stacker-db: {e:?}"),
-                Ok(ack) if !ack.accepted => warn!(
-                    "{self}: Block rejection not accepted by stacker-db: {:?}",
-                    ack.reason
-                ),
-                Ok(_) => debug!("{self}: Block rejection accepted by stacker-db"),
-            }
+            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));
@@ -1158,30 +1151,12 @@ impl Signer {
             .unwrap_or_else(|e| warn!("{self}: Failed to remove pending block validation: {e:?}"));
 
         if let Some(response) = block_response {
-            // Submit a proposal response to the .signers contract for miners
-            info!(
-                "{self}: Broadcasting a block response to stacks node: {response:?}";
-            );
-            let accepted = matches!(response, BlockResponse::Accepted(..));
-            match self
-                .stackerdb
-                .send_message_with_retry::<SignerMessage>(response.into())
-            {
-                Ok(_) => {
-                    crate::monitoring::actions::increment_block_responses_sent(accepted);
-                    if let Ok(Some(block_info)) = self
-                        .signer_db
-                        .block_lookup(&block_validate_response.signer_signature_hash())
-                    {
-                        crate::monitoring::actions::record_block_response_latency(
-                            &block_info.block,
-                        );
-                    }
-                }
-                Err(e) => {
-                    warn!("{self}: Failed to send block rejection to stacker-db: {e:?}",);
-                }
-            }
+            let block = self
+                .signer_db
+                .block_lookup(&signer_sig_hash)
+                .unwrap_or_default()
+                .map(|info| info.block);
+            self.impl_send_block_response(block.as_ref(), response);
         };
 
         // Check if there is a pending block validation that we need to submit to the node
@@ -1274,21 +1249,7 @@ impl Signer {
                 warn!("{self}: Failed to mark block as locally rejected: {e:?}");
             }
         };
-        debug!("{self}: Broadcasting a block response to stacks node: {rejection:?}");
-        let res = self
-            .stackerdb
-            .send_message_with_retry::<SignerMessage>(rejection.into());
-
-        crate::monitoring::actions::record_block_response_latency(&block_info.block);
-
-        match res {
-            Err(e) => warn!("{self}: Failed to send block rejection to stacker-db: {e:?}"),
-            Ok(ack) if !ack.accepted => warn!(
-                "{self}: Block rejection not accepted by stacker-db: {:?}",
-                ack.reason
-            ),
-            Ok(_) => debug!("{self}: Block rejection accepted by stacker-db"),
-        }
+        self.impl_send_block_response(Some(&block_info.block), rejection);
 
         self.signer_db
             .insert_block(&block_info)