Merge branch 'develop' into default-mempool-walk-strategy

Author: Jiloc

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/lib.rs

@@ -53,6 +53,7 @@ use chainstate::SortitionsView;
 use config::GlobalConfig;
 use libsigner::{SignerEvent, SignerEventReceiver, SignerEventTrait, VERSION_STRING};
 use runloop::SignerResult;
+use signerdb::BlockInfo;
 use stacks_common::{info, warn};
 use v0::signer_state::LocalStateMachine;
 
@@ -81,6 +82,8 @@ pub trait Signer<T: SignerEventTrait>: Debug + Display {
     fn get_local_state_machine(&self) -> &LocalStateMachine;
     /// Get the number of pending block proposals
     fn get_pending_proposals_count(&self) -> u64;
+    /// Get canonical block according to this signer's db
+    fn get_canonical_tip(&self) -> Option<BlockInfo>;
 }
 
 /// A wrapper around the running signer type for the signer

stacks-signer/src/runloop.rs

@@ -25,6 +25,7 @@ use stacks_common::{debug, error, info, warn};
 use crate::chainstate::SortitionsView;
 use crate::client::{retry_with_exponential_backoff, ClientError, StacksClient};
 use crate::config::{GlobalConfig, SignerConfig, SignerConfigMode};
+use crate::signerdb::BlockInfo;
 use crate::v0::signer_state::LocalStateMachine;
 #[cfg(any(test, feature = "testing"))]
 use crate::v0::tests::TEST_SKIP_SIGNER_CLEANUP;
@@ -59,6 +60,9 @@ pub struct StateInfo {
     pub signer_state_machines: Vec<(u64, Option<LocalStateMachine>)>,
     /// The number of pending block proposals for this signer
     pub pending_proposals_count: u64,
+    /// The canonical tip block info according to the running signers
+    /// as a pair of (reward-cycle, block-info)
+    pub signer_canonical_tips: Vec<(u64, Option<BlockInfo>)>,
 }
 
 /// The signer result that can be sent across threads
@@ -544,6 +548,16 @@ impl<Signer: SignerTrait<T>, T: StacksMessageCodec + Clone + Send + Debug>
                         }
                     })
                     .unwrap_or(0),
+                signer_canonical_tips: self
+                    .stacks_signers
+                    .iter()
+                    .map(|(reward_cycle, signer)| {
+                        let ConfiguredSigner::RegisteredSigner(ref signer) = signer else {
+                            return (*reward_cycle, None);
+                        };
+                        (*reward_cycle, signer.get_canonical_tip())
+                    })
+                    .collect(),
             };
             info!("Signer status check requested: {state_info:?}");
 

stacks-signer/src/signerdb.rs

@@ -98,7 +98,7 @@ impl FromRow<BurnBlockInfo> for BurnBlockInfo {
     }
 }
 
-#[derive(Serialize, Deserialize, Debug, PartialEq, Default)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Default, Clone)]
 /// Store extra version-specific info in `BlockInfo`
 pub enum ExtraBlockInfo {
     #[default]
@@ -167,7 +167,7 @@ impl TryFrom<&str> for BlockState {
 }
 
 /// Additional Info about a proposed block
-#[derive(Serialize, Deserialize, Debug, PartialEq)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
 pub struct BlockInfo {
     /// The block we are considering
     pub block: NakamotoBlock,

stacks-signer/src/v0/signer.rs

@@ -344,6 +344,14 @@ impl SignerTrait<SignerMessage> for Signer {
             .map(|results| u64::try_from(results.len()).unwrap())
             .unwrap_or(0)
     }
+
+    fn get_canonical_tip(&self) -> Option<BlockInfo> {
+        self.signer_db
+            .get_canonical_tip()
+            .inspect_err(|e| error!("{self}: Failed to check for canonical tip: {e:?}"))
+            .ok()
+            .flatten()
+    }
 }
 
 impl Signer {
@@ -953,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
@@ -965,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,
@@ -988,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

stacks-signer/src/v0/signer_state.rs

@@ -757,7 +757,7 @@ impl LocalStateMachine {
 
             match signerdb.get_burn_block_by_ch(tenure_id) {
                 Ok(block) => {
-                    potential_matches.push((block.block_height, miner_state.clone()));
+                    potential_matches.push((block.block_height, miner_state));
                 }
                 Err(e) => {
                     warn!("Error retrieving burn block for consensus_hash {tenure_id} from signerdb: {e}");
@@ -767,7 +767,7 @@ impl LocalStateMachine {
 
         potential_matches.sort_by_key(|(block_height, _)| *block_height);
 
-        let new_miner = potential_matches.last().map(|(_, miner)| miner.clone());
+        let new_miner = potential_matches.last().map(|(_, miner)| (*miner).clone());
         if new_miner.is_none() {
             crate::monitoring::actions::increment_signer_agreement_state_conflict(
                 crate::monitoring::SignerAgreementStateConflict::MinerView,