Track the last submitted block proposal and do not submit a new one if we already are processing one

Signed-off-by: Jacinta Ferrant <[email protected]>

Author: jferrant

stacks-signer/src/chainstate.rs

@@ -119,13 +119,16 @@ pub struct ProposalEvalConfig {
     pub first_proposal_burn_block_timing: Duration,
     /// Time between processing a sortition and proposing a block before the block is considered invalid
     pub block_proposal_timeout: Duration,
+    /// How long to wait for a block proposal validation response
+    pub block_proposal_validation_timeout: Duration,
 }
 
 impl From<&SignerConfig> for ProposalEvalConfig {
     fn from(value: &SignerConfig) -> Self {
         Self {
             first_proposal_burn_block_timing: value.first_proposal_burn_block_timing,
             block_proposal_timeout: value.block_proposal_timeout,
+            block_proposal_validation_timeout: value.block_proposal_validation_timeout,
         }
     }
 }

stacks-signer/src/client/mod.rs

@@ -411,6 +411,7 @@ pub(crate) mod tests {
             db_path: config.db_path.clone(),
             first_proposal_burn_block_timing: config.first_proposal_burn_block_timing,
             block_proposal_timeout: config.block_proposal_timeout,
+            block_proposal_validation_timeout: config.block_proposal_validation_timeout,
         }
     }
 

stacks-signer/src/config.rs

@@ -129,6 +129,8 @@ pub struct SignerConfig {
     pub first_proposal_burn_block_timing: Duration,
     /// How much time to wait for a miner to propose a block following a sortition
     pub block_proposal_timeout: Duration,
+    /// How much time ot wait for a block proposal validation response before marking the block invalid
+    pub block_proposal_validation_timeout: Duration,
 }
 
 /// The parsed configuration for the signer

stacks-signer/src/runloop.rs

@@ -283,6 +283,7 @@ impl<Signer: SignerTrait<T>, T: StacksMessageCodec + Clone + Send + Debug> RunLo
             mainnet: self.config.network.is_mainnet(),
             db_path: self.config.db_path.clone(),
             block_proposal_timeout: self.config.block_proposal_timeout,
+            block_proposal_validation_timeout: self.config.block_proposal_validation_timeout,
         }))
     }
 

stacks-signer/src/tests/chainstate.rs

@@ -89,6 +89,7 @@ fn setup_test_environment(
         config: ProposalEvalConfig {
             first_proposal_burn_block_timing: Duration::from_secs(30),
             block_proposal_timeout: Duration::from_secs(5),
+            block_proposal_validation_timeout: Duration::from_secs(60),
         },
     };
 

stacks-signer/src/v0/signer.rs

@@ -15,6 +15,7 @@
 use std::collections::HashMap;
 use std::fmt::Debug;
 use std::sync::mpsc::Sender;
+use std::time::Instant;
 
 use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader};
 use blockstack_lib::net::api::postblock_proposal::{
@@ -85,6 +86,8 @@ pub struct Signer {
     pub signer_db: SignerDb,
     /// Configuration for proposal evaluation
     pub proposal_config: ProposalEvalConfig,
+    /// The current submitted block proposal and its submission time
+    pub submitted_block_proposal: Option<(BlockProposal, Instant)>,
 }
 
 impl std::fmt::Display for Signer {
@@ -127,6 +130,7 @@ impl SignerTrait<SignerMessage> for Signer {
         if event_parity == Some(other_signer_parity) {
             return;
         }
+        self.check_submitted_block_proposal();
         debug!("{self}: Processing event: {event:?}");
         let Some(event) = event else {
             // No event. Do nothing.
@@ -274,6 +278,7 @@ impl From<SignerConfig> for Signer {
             reward_cycle: signer_config.reward_cycle,
             signer_db,
             proposal_config,
+            submitted_block_proposal: None,
         }
     }
 }
@@ -355,7 +360,7 @@ impl Signer {
         }
 
         info!(
-            "{self}: received a block proposal for a new block. Submit block for validation. ";
+            "{self}: received a block proposal for a new block.";
             "signer_sighash" => %signer_signature_hash,
             "block_id" => %block_proposal.block.block_id(),
             "block_height" => block_proposal.block.header.chain_length,
@@ -456,14 +461,35 @@ impl Signer {
                 Ok(_) => debug!("{self}: Block rejection accepted by stacker-db"),
             }
         } else {
-            // We don't know if proposal is valid, submit to stacks-node for further checks and store it locally.
-            // Do not store invalid blocks as this could DOS the signer. We only store blocks that are valid or unknown.
-            stacks_client
-                .submit_block_for_validation(block_info.block.clone())
-                .unwrap_or_else(|e| {
-                    warn!("{self}: Failed to submit block for validation: {e:?}");
-                });
+            // Just in case check if the last block validation submission timed out.
+            self.check_submitted_block_proposal();
+            if self.submitted_block_proposal.is_none() {
+                // We don't know if proposal is valid, submit to stacks-node for further checks and store it locally.
+                info!(
+                    "{self}: submitting block proposal for validation";
+                    "signer_sighash" => %signer_signature_hash,
+                    "block_id" => %block_proposal.block.block_id(),
+                    "block_height" => block_proposal.block.header.chain_length,
+                    "burn_height" => block_proposal.burn_height,
+                );
+                match stacks_client.submit_block_for_validation(block_info.block.clone()) {
+                    Ok(_) => {
+                        self.submitted_block_proposal =
+                            Some((block_proposal.clone(), Instant::now()));
+                    }
+                    Err(e) => {
+                        warn!("{self}: Failed to submit block for validation: {e:?}");
+                    }
+                };
+            } else {
+                // Still store the block but log we can't submit it for validation. We may receive enough signatures/rejections
+                // 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")
+            }
 
+            // Do not store KNOWN invalid blocks as this could DOS the signer. We only store blocks that are valid or unknown.
             self.signer_db
                 .insert_block(&block_info)
                 .unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB"));
@@ -493,6 +519,16 @@ impl Signer {
     ) -> Option<BlockResponse> {
         crate::monitoring::increment_block_validation_responses(true);
         let signer_signature_hash = block_validate_ok.signer_signature_hash;
+        if self
+            .submitted_block_proposal
+            .as_ref()
+            .map(|(proposal, _)| {
+                proposal.block.header.signer_signature_hash() == signer_signature_hash
+            })
+            .unwrap_or(false)
+        {
+            self.submitted_block_proposal = None;
+        }
         // For mutability reasons, we need to take the block_info out of the map and add it back after processing
         let mut block_info = match self
             .signer_db
@@ -542,6 +578,16 @@ impl Signer {
     ) -> Option<BlockResponse> {
         crate::monitoring::increment_block_validation_responses(false);
         let signer_signature_hash = block_validate_reject.signer_signature_hash;
+        if self
+            .submitted_block_proposal
+            .as_ref()
+            .map(|(proposal, _)| {
+                proposal.block.header.signer_signature_hash() == signer_signature_hash
+            })
+            .unwrap_or(false)
+        {
+            self.submitted_block_proposal = None;
+        }
         let mut block_info = match self
             .signer_db
             .block_lookup(self.reward_cycle, &signer_signature_hash)
@@ -617,6 +663,85 @@ impl Signer {
         }
     }
 
+    /// Check the current tracked submitted block proposal to see if it has timed out.
+    /// Broadcasts a rejection and marks the block locally rejected if it has.
+    fn check_submitted_block_proposal(&mut self) {
+        let Some((block_proposal, block_submission)) = self.submitted_block_proposal.clone() else {
+            // Nothing to check.
+            return;
+        };
+        if block_submission.elapsed() < self.proposal_config.block_proposal_validation_timeout {
+            // Not expired yet.
+            return;
+        }
+        // Let us immediately flush, even if we later encounter an error broadcasting our responses.
+        // We should still attempt to handle a new proposal at this point.
+        self.submitted_block_proposal = None;
+        let signature_sighash = block_proposal.block.header.signer_signature_hash();
+        // For mutability reasons, we need to take the block_info out of the map and add it back after processing
+        let mut block_info = match self
+            .signer_db
+            .block_lookup(self.reward_cycle, &signature_sighash)
+        {
+            Ok(Some(block_info)) => {
+                if block_info.state == BlockState::GloballyRejected
+                    || block_info.state == BlockState::GloballyAccepted
+                {
+                    // The block has already reached consensus. This should never really hit, but check just to be safe.
+                    self.submitted_block_proposal = None;
+                    return;
+                }
+                block_info
+            }
+            Ok(None) => {
+                // This is weird. If this is reached, its probably an error in code logic or the db was flushed.
+                // Why are we tracking a block submission for a block we have never seen / stored before.
+                error!("{self}: tracking an unknown block validation submission.";
+                    "signer_sighash" => %signature_sighash,
+                    "block_id" => %block_proposal.block.block_id(),
+                );
+                self.submitted_block_proposal = None;
+                return;
+            }
+            Err(e) => {
+                error!("{self}: Failed to lookup block in signer db: {e:?}",);
+                self.submitted_block_proposal = None;
+                return;
+            }
+        };
+        warn!(
+            "{self}: Failed to receive block validation response within {} ms. Rejecting block.", self.proposal_config.block_proposal_validation_timeout.as_millis();
+            "signer_sighash" => %signature_sighash,
+            "block_id" => %block_proposal.block.block_id(),
+        );
+        let rejection = BlockResponse::rejected(
+            block_proposal.block.header.signer_signature_hash(),
+            RejectCode::ConnectivityIssues,
+            &self.private_key,
+            self.mainnet,
+        );
+        // We know proposal is invalid. Send rejection message, do not do further validation
+        if let Err(e) = block_info.mark_locally_rejected() {
+            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());
+
+        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.signer_db
+            .insert_block(&block_info)
+            .unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB"));
+    }
+
     /// Compute the signing weight, given a list of signatures
     fn compute_signature_signing_weight<'a>(
         &self,
@@ -723,6 +848,14 @@ impl Signer {
             error!("{self}: Failed to update block state: {e:?}",);
             panic!("{self} Failed to update block state: {e}");
         }
+        if self
+            .submitted_block_proposal
+            .as_ref()
+            .map(|(proposal, _)| &proposal.block.header.signer_signature_hash() == block_hash)
+            .unwrap_or(false)
+        {
+            self.submitted_block_proposal = None;
+        }
     }
 
     /// Handle an observed signature from another signer
@@ -865,6 +998,14 @@ impl Signer {
             }
         }
         self.broadcast_signed_block(stacks_client, block_info.block, &addrs_to_sigs);
+        if self
+            .submitted_block_proposal
+            .as_ref()
+            .map(|(proposal, _)| &proposal.block.header.signer_signature_hash() == block_hash)
+            .unwrap_or(false)
+        {
+            self.submitted_block_proposal = None;
+        }
     }
 
     fn broadcast_signed_block(

testnet/stacks-node/src/tests/nakamoto_integrations.rs

@@ -6450,6 +6450,7 @@ fn signer_chainstate() {
         let proposal_conf = ProposalEvalConfig {
             first_proposal_burn_block_timing: Duration::from_secs(0),
             block_proposal_timeout: Duration::from_secs(100),
+            block_proposal_validation_timeout: Duration::from_secs(100),
         };
         let mut sortitions_view =
             SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap();
@@ -6588,6 +6589,7 @@ fn signer_chainstate() {
         let proposal_conf = ProposalEvalConfig {
             first_proposal_burn_block_timing: Duration::from_secs(0),
             block_proposal_timeout: Duration::from_secs(100),
+            block_proposal_validation_timeout: Duration::from_secs(100),
         };
         let burn_block_height = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn())
             .unwrap()
@@ -6665,6 +6667,7 @@ fn signer_chainstate() {
     let proposal_conf = ProposalEvalConfig {
         first_proposal_burn_block_timing: Duration::from_secs(0),
         block_proposal_timeout: Duration::from_secs(100),
+        block_proposal_validation_timeout: Duration::from_secs(100),
     };
     let mut sortitions_view = SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap();
     let burn_block_height = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn())

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

@@ -456,6 +456,7 @@ fn block_proposal_rejection() {
     let proposal_conf = ProposalEvalConfig {
         first_proposal_burn_block_timing: Duration::from_secs(0),
         block_proposal_timeout: Duration::from_secs(100),
+        block_proposal_validation_timeout: Duration::from_secs(100),
     };
     let mut block = NakamotoBlock {
         header: NakamotoBlockHeader::empty(),