feat: handle burn reorgs during tenure extends

* update signer logic to check node's tenure info after local state times out (do not consult local state for global acceptance status)
* update node get_tenuretip endpoint to return highest tip in tenure with canonical burn_view
* update proposal validation and mining logic to do the same
* update miner to be more defensive with burn_view check

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,41 @@ 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, get the last globally accepted block
+            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 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 +633,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 +643,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 +670,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)
-        }
+        return 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 +753,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

@@ -945,7 +945,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
@@ -957,7 +956,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,
@@ -980,26 +979,30 @@ 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(
                         RejectReason::SortitionViewMismatch,
                         proposed_block,
                     ));
+                } else {
+                    return None;
                 }
             }
-            Ok(_) => {}
             Err(e) => {
                 warn!("{self}: Failed to check block against signer db: {e}";
                     "signer_signature_hash" => %signer_signature_hash,
@@ -1013,7 +1016,6 @@ impl Signer {
                 ));
             }
         }
-        None
     }
 
     /// Handle the block validate ok response. Returns our block response if we have one

stackslib/src/chainstate/nakamoto/mod.rs

@@ -2887,6 +2887,75 @@ impl NakamotoChainState {
         Ok(StacksChainState::get_stacks_block_header_info_by_consensus_hash(db, consensus_hash)?)
     }
 
+    /// DO NOT USE IN CONSENSUS CODE.  Different nodes can have different blocks for the same
+    /// tenure.
+    ///
+    /// Get the highest block in a given tenure (identified by its consensus hash) with a canonical
+    ///  burn_view (i.e., burn_view on the canonical sortition fork)
+    pub fn find_highest_known_block_header_in_tenure(
+        chainstate: &StacksChainState,
+        sort_db: &SortitionDB,
+        tenure_id: &ConsensusHash,
+    ) -> Result<Option<StacksHeaderInfo>, ChainstateError> {
+        let chainstate_db_conn = chainstate.db();
+
+        let candidates = Self::get_highest_known_block_header_in_tenure_at_each_burnview(
+            chainstate_db_conn,
+            tenure_id,
+        )?;
+
+        let canonical_sortition_handle = sort_db.index_handle_at_tip();
+        for candidate in candidates.into_iter() {
+            let Some(ref candidate_ch) = candidate.burn_view else {
+                // this is an epoch 2.x header, no burn view to check
+                return Ok(Some(candidate));
+            };
+            let in_canonical_fork = canonical_sortition_handle.processed_block(&candidate_ch)?;
+            if in_canonical_fork {
+                return Ok(Some(candidate));
+            }
+        }
+
+        // did not find any blocks in the tenure
+        Ok(None)
+    }
+
+    /// DO NOT USE IN CONSENSUS CODE.  Different nodes can have different blocks for the same
+    /// tenure.
+    ///
+    /// Get the highest blocks in a given tenure (identified by its consensus hash) at each burn view
+    ///  active in that tenure. If there are ties at a given burn view, they will both be returned
+    fn get_highest_known_block_header_in_tenure_at_each_burnview(
+        db: &Connection,
+        tenure_id: &ConsensusHash,
+    ) -> Result<Vec<StacksHeaderInfo>, ChainstateError> {
+        // see if we have a nakamoto block in this tenure
+        let qry = "
+        SELECT h.*
+        FROM nakamoto_block_headers h
+        JOIN (
+            SELECT burn_view, MAX(block_height) AS max_height
+            FROM nakamoto_block_headers
+            WHERE consensus_hash = ?1
+            GROUP BY burn_view
+        ) maxed
+        ON h.burn_view = maxed.burn_view
+        AND h.block_height = maxed.max_height
+        WHERE h.consensus_hash = ?1
+        ORDER BY h.block_height DESC, h.timestamp
+        ";
+        let args = params![tenure_id];
+        let out = query_rows(db, qry, args)?;
+        if !out.is_empty() {
+            return Ok(out);
+        }
+
+        // see if this is an epoch2 header. If it exists, then there will only be one.
+        let epoch2_x =
+            StacksChainState::get_stacks_block_header_info_by_consensus_hash(db, tenure_id)?;
+        Ok(Vec::from_iter(epoch2_x))
+    }
+
     /// Get the VRF proof for a Stacks block.
     /// For Nakamoto blocks, this is the VRF proof contained in the coinbase of the tenure-start
     /// block of the given tenure identified by the consensus hash.