Merge pull request #6139 from kantai/feat/extend-bitcoin-reorg

Feat: handle burn reorgs during tenure extends

Author: kantai

stacks-signer/src/chainstate.rs

@@ -389,9 +389,9 @@ impl SortitionsView {
             )?;
         } else {
             // check if the new block confirms the last block in the current tenure
-            let confirms_latest_in_tenure =
-                Self::confirms_latest_block_in_same_tenure(block, signer_db)
-                    .map_err(SignerChainstateError::from)?;
+            let confirms_latest_in_tenure = self
+                .confirms_latest_block_in_same_tenure(block, signer_db, client)
+                .map_err(SignerChainstateError::from)?;
             if !confirms_latest_in_tenure {
                 return Err(RejectReason::InvalidParentBlock);
             }
@@ -562,8 +562,7 @@ impl SortitionsView {
         Ok(true)
     }
 
-    /// 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.
+    /// Get the last locally signed block from the given tenure if it has not timed out
     pub fn get_tenure_last_block_info(
         consensus_hash: &ConsensusHash,
         signer_db: &SignerDb,
@@ -573,43 +572,45 @@ impl SortitionsView {
         let last_locally_accepted_block = signer_db
             .get_last_accepted_block(consensus_hash)
             .map_err(|e| ClientError::InvalidResponse(e.to_string()))?;
+        let Some(local_info) = last_locally_accepted_block else {
+            return Ok(None);
+        };
 
-        if let Some(local_info) = last_locally_accepted_block {
-            if let Some(signed_over_time) = local_info.signed_self {
-                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
-                    return Ok(Some(local_info));
-                }
-            }
+        let Some(signed_over_time) = local_info.signed_self else {
+            return Ok(None);
+        };
+
+        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
+            Ok(Some(local_info))
+        } else {
+            // The last locally accepted block is timed out
+            Ok(None)
         }
-        // The last locally accepted block is timed out, get the last globally accepted block
-        signer_db
-            .get_last_globally_accepted_block(consensus_hash)
-            .map_err(|e| ClientError::InvalidResponse(e.to_string()))
     }
 
-    /// Check if the tenure change block confirms the expected parent block
-    /// (i.e., the last locally accepted block in the parent tenure, or if that block is timed out, the last globally accepted block in the parent tenure)
-    /// It checks the local DB first, and if the block is not present in the local DB, it asks the
-    /// Stacks node for the highest processed block header in the given tenure (and then caches it
-    /// in the DB).
+    /// Check whether or not `block` is higher than the highest block in `tenure_id`.
+    ///  returns `Ok(true)` if `block` is higher, `Ok(false)` if not.
     ///
-    /// The rationale here is that the signer DB can be out-of-sync with the node.  For example,
-    /// the signer may have been added to an already-running node.
-    pub fn check_tenure_change_confirms_parent(
-        tenure_change: &TenureChangePayload,
+    /// If we can't look up `tenure_id`, assume `block` is higher.
+    /// This assumption is safe because this proposal ultimately must be passed
+    /// to the `stacks-node` for proposal processing: so, if we pass the block
+    /// height check here, we are relying on the `stacks-node` proposal endpoint
+    /// to do the validation on the chainstate data that it has.
+    ///
+    /// This updates the activity timer for the miner of `block`.
+    pub fn check_latest_block_in_tenure(
+        tenure_id: &ConsensusHash,
         block: &NakamotoBlock,
         signer_db: &mut SignerDb,
         client: &StacksClient,
         tenure_last_block_proposal_timeout: Duration,
         reorg_attempts_activity_timeout: Duration,
     ) -> Result<bool, ClientError> {
-        // If the tenure change block confirms the expected parent block, it should confirm at least one more block than the last accepted block in the parent tenure.
-        // NOTE: returns the locally accepted block if it is not timed out, otherwise it will return the last globally accepted block.
         let last_block_info = Self::get_tenure_last_block_info(
-            &tenure_change.prev_tenure_consensus_hash,
+            tenure_id,
             signer_db,
             tenure_last_block_proposal_timeout,
         )?;
@@ -636,7 +637,7 @@ impl SortitionsView {
                     // to give the miner some extra buffer time to wait for its chain tip to advance
                     // The miner may just be slow, so count this invalid block proposal towards valid miner activity.
                     if let Err(e) = signer_db.update_last_activity_time(
-                        &tenure_change.tenure_consensus_hash,
+                        &block.header.consensus_hash,
                         get_epoch_time_secs(),
                     ) {
                         warn!("Failed to update last activity time: {e}");
@@ -646,16 +647,16 @@ impl SortitionsView {
             }
         }
 
-        let tip = match client.get_tenure_tip(&tenure_change.prev_tenure_consensus_hash) {
+        let tip = match client.get_tenure_tip(tenure_id) {
             Ok(tip) => tip,
             Err(e) => {
                 warn!(
-                    "Miner block proposal contains a tenure change, but failed to fetch the tenure tip for the parent tenure: {e:?}. Considering proposal invalid.";
+                    "Failed to fetch the tenure tip for the parent tenure: {e:?}. Assuming proposal is higher than the parent tenure for now.";
                     "proposed_block_consensus_hash" => %block.header.consensus_hash,
                     "signer_signature_hash" => %block.header.signer_signature_hash(),
-                    "parent_tenure" => %tenure_change.prev_tenure_consensus_hash,
+                    "parent_tenure" => %tenure_id,
                 );
-                return Ok(false);
+                return Ok(true);
             }
         };
         if let Some(nakamoto_tip) = tip.as_stacks_nakamoto() {
@@ -673,19 +674,33 @@ impl SortitionsView {
                 }
             }
         }
-        let tip_height = tip.height();
-        if block.header.chain_length > tip_height {
-            Ok(true)
-        } else {
-            warn!(
-                "Miner's block proposal does not confirm as many blocks as we expect";
-                "proposed_block_consensus_hash" => %block.header.consensus_hash,
-                "signer_signature_hash" => %block.header.signer_signature_hash(),
-                "proposed_chain_length" => block.header.chain_length,
-                "expected_at_least" => tip_height + 1,
-            );
-            Ok(false)
-        }
+        Ok(tip.height() < block.header.chain_length)
+    }
+
+    /// Check if the tenure change block confirms the expected parent block
+    /// (i.e., the last locally accepted block in the parent tenure, or if that block is timed out, the last globally accepted block in the parent tenure)
+    /// It checks the local DB first, and if the block is not present in the local DB, it asks the
+    /// Stacks node for the highest processed block header in the given tenure (and then caches it
+    /// in the DB).
+    ///
+    /// The rationale here is that the signer DB can be out-of-sync with the node.  For example,
+    /// the signer may have been added to an already-running node.
+    pub fn check_tenure_change_confirms_parent(
+        tenure_change: &TenureChangePayload,
+        block: &NakamotoBlock,
+        signer_db: &mut SignerDb,
+        client: &StacksClient,
+        tenure_last_block_proposal_timeout: Duration,
+        reorg_attempts_activity_timeout: Duration,
+    ) -> Result<bool, ClientError> {
+        Self::check_latest_block_in_tenure(
+            &tenure_change.prev_tenure_consensus_hash,
+            block,
+            signer_db,
+            client,
+            tenure_last_block_proposal_timeout,
+            reorg_attempts_activity_timeout,
+        )
     }
 
     /// in tenure changes, we need to check:
@@ -742,33 +757,19 @@ impl SortitionsView {
     }
 
     fn confirms_latest_block_in_same_tenure(
+        &self,
         block: &NakamotoBlock,
-        signer_db: &SignerDb,
+        signer_db: &mut SignerDb,
+        client: &StacksClient,
     ) -> Result<bool, ClientError> {
-        let Some(last_known_block) = signer_db
-            .get_last_accepted_block(&block.header.consensus_hash)
-            .map_err(|e| ClientError::InvalidResponse(e.to_string()))?
-        else {
-            info!(
-                "Have no accepted blocks in the tenure, assuming block confirmation is correct";
-                "proposed_block_consensus_hash" => %block.header.consensus_hash,
-                "signer_signature_hash" => %block.header.signer_signature_hash(),
-                "proposed_block_height" => block.header.chain_length,
-            );
-            return Ok(true);
-        };
-        if block.header.chain_length > last_known_block.block.header.chain_length {
-            Ok(true)
-        } else {
-            warn!(
-                "Miner's block proposal does not confirm as many blocks as we expect";
-                "proposed_block_consensus_hash" => %block.header.consensus_hash,
-                "signer_signature_hash" => %block.header.signer_signature_hash(),
-                "proposed_chain_length" => block.header.chain_length,
-                "expected_at_least" => last_known_block.block.header.chain_length + 1,
-            );
-            Ok(false)
-        }
+        Self::check_latest_block_in_tenure(
+            &block.header.consensus_hash,
+            block,
+            signer_db,
+            client,
+            self.config.tenure_last_block_proposal_timeout,
+            self.config.reorg_attempts_activity_timeout,
+        )
     }
 
     /// Fetch a new view of the recent sortitions

stacks-signer/src/v0/signer.rs

@@ -961,7 +961,6 @@ impl Signer {
         proposed_block: &NakamotoBlock,
     ) -> Option<BlockResponse> {
         let signer_signature_hash = proposed_block.header.signer_signature_hash();
-        let proposed_block_consensus_hash = proposed_block.header.consensus_hash;
         // If this is a tenure change block, ensure that it confirms the correct number of blocks from the parent tenure.
         if let Some(tenure_change) = proposed_block.get_tenure_change_tx_payload() {
             // Ensure that the tenure change block confirms the expected parent block
@@ -973,7 +972,7 @@ impl Signer {
                 self.proposal_config.tenure_last_block_proposal_timeout,
                 self.proposal_config.reorg_attempts_activity_timeout,
             ) {
-                Ok(true) => {}
+                Ok(true) => return None,
                 Ok(false) => {
                     return Some(self.create_block_rejection(
                         RejectReason::SortitionViewMismatch,
@@ -996,40 +995,43 @@ impl Signer {
         }
 
         // Ensure that the block is the last block in the chain of its current tenure.
-        match self
-            .signer_db
-            .get_last_accepted_block(&proposed_block_consensus_hash)
-        {
-            Ok(Some(last_block_info)) => {
-                if proposed_block.header.chain_length <= last_block_info.block.header.chain_length {
+        match SortitionsView::check_latest_block_in_tenure(
+            &proposed_block.header.consensus_hash,
+            proposed_block,
+            &mut self.signer_db,
+            stacks_client,
+            self.proposal_config.tenure_last_block_proposal_timeout,
+            self.proposal_config.reorg_attempts_activity_timeout,
+        ) {
+            Ok(is_latest) => {
+                if !is_latest {
                     warn!(
                         "Miner's block proposal does not confirm as many blocks as we expect";
-                        "proposed_block_consensus_hash" => %proposed_block_consensus_hash,
+                        "proposed_block_consensus_hash" => %proposed_block.header.consensus_hash,
                         "proposed_block_signer_signature_hash" => %signer_signature_hash,
                         "proposed_chain_length" => proposed_block.header.chain_length,
-                        "expected_at_least" => last_block_info.block.header.chain_length + 1,
                     );
-                    return Some(self.create_block_rejection(
+                    Some(self.create_block_rejection(
                         RejectReason::SortitionViewMismatch,
                         proposed_block,
-                    ));
+                    ))
+                } else {
+                    None
                 }
             }
-            Ok(_) => {}
             Err(e) => {
                 warn!("{self}: Failed to check block against signer db: {e}";
                     "signer_signature_hash" => %signer_signature_hash,
                     "block_id" => %proposed_block.block_id()
                 );
-                return Some(self.create_block_rejection(
+                Some(self.create_block_rejection(
                     RejectReason::ConnectivityIssues(
                         "failed to check block against signer db".to_string(),
                     ),
                     proposed_block,
-                ));
+                ))
             }
         }
-        None
     }
 
     /// Handle the block validate ok response. Returns our block response if we have one