Merge branch 'develop' into feat/tenure_boundary_heuristic

Author: jcnelson

contrib/tools/block-replay.sh

@@ -117,7 +117,7 @@ configure_replay_slices() {
 
 	## create a copy of the linked db with <number of CORES><number of RESERVED CORES>
 	##   decrement by 1 since we already have ${SLICE_DIR}0
-	for ((i=1;i<=$( CORES - RESERVED - 1);i++)); do
+	for ((i=1;i<=$(( CORES - RESERVED - 1));i++)); do
 		echo "Copying ${SLICE_DIR}0 -> ${COLYELLOW}${SLICE_DIR}${i}${COLRESET}"
 		cp -R "${SLICE_DIR}0" "${SLICE_DIR}${i}" || {
 			echo "${COLRED}Error${COLRESET} copying ${SLICE_DIR}0 -> ${SLICE_DIR}${i}"

libsigner/src/v0/messages.rs

@@ -17,6 +17,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 - Increase default `block_proposal_timeout_ms` from 10 minutes to 4 hours. Until #5729 is implemented, there is no value in rejecting a late block from a miner, since a late block is better than no block at all.
 - Signers no longer view any block proposal by a miner in their DB as indicative of valid miner activity.
 - Various index improvements to the signer's database to improve performance.
+- Add new reject codes to the signer response for better visibility into why a block was rejected.
 
 ## [3.1.0.0.5.0]
 

stacks-signer/CHANGELOG.md

@@ -19,6 +19,7 @@ use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
 use blockstack_lib::chainstate::stacks::TenureChangePayload;
 use blockstack_lib::net::api::getsortition::SortitionInfo;
 use blockstack_lib::util_lib::db::Error as DBError;
+use libsigner::v0::messages::RejectReason;
 use slog::{slog_info, slog_warn};
 use stacks_common::types::chainstate::{BurnchainHeaderHash, ConsensusHash, StacksPublicKey};
 use stacks_common::util::get_epoch_time_secs;
@@ -40,6 +41,12 @@ pub enum SignerChainstateError {
     ClientError(#[from] ClientError),
 }
 
+impl From<SignerChainstateError> for RejectReason {
+    fn from(error: SignerChainstateError) -> Self {
+        RejectReason::ConnectivityIssues(error.to_string())
+    }
+}
+
 /// Captures this signer's current view of a sortition's miner.
 #[derive(PartialEq, Eq, Debug)]
 pub enum SortitionMinerStatus {
@@ -201,7 +208,7 @@ impl SortitionsView {
         block: &NakamotoBlock,
         block_pk: &StacksPublicKey,
         reset_view_if_wrong_consensus_hash: bool,
-    ) -> Result<bool, SignerChainstateError> {
+    ) -> Result<(), RejectReason> {
         if self
             .cur_sortition
             .is_timed_out(self.config.block_proposal_timeout, signer_db)?
@@ -213,7 +220,10 @@ impl SortitionsView {
                 "current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
             );
             self.cur_sortition.miner_status = SortitionMinerStatus::InvalidatedBeforeFirstBlock;
-        } else if let Some(tip) = signer_db.get_canonical_tip()? {
+        } else if let Some(tip) = signer_db
+            .get_canonical_tip()
+            .map_err(SignerChainstateError::from)?
+        {
             // Check if the current sortition is aligned with the expected tenure:
             // - If the tip is in the current tenure, we are in the process of mining this tenure.
             // - If the tip is not in the current tenure, then we’re starting a new tenure,
@@ -266,7 +276,7 @@ impl SortitionsView {
                 "current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
                 "last_sortition_consensus_hash" => ?self.last_sortition.as_ref().map(|x| x.consensus_hash),
             );
-            return Ok(false);
+            return Err(RejectReason::InvalidBitvec);
         }
 
         let block_pkh = Hash160::from_data(&block_pk.to_bytes_compressed());
@@ -293,7 +303,8 @@ impl SortitionsView {
                     "current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
                     "last_sortition_consensus_hash" => ?self.last_sortition.as_ref().map(|x| x.consensus_hash),
                 );
-                self.reset_view(client)?;
+                self.reset_view(client)
+                    .map_err(SignerChainstateError::from)?;
                 return self.check_proposal(client, signer_db, block, block_pk, false);
             }
             warn!(
@@ -303,7 +314,7 @@ impl SortitionsView {
                 "current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
                 "last_sortition_consensus_hash" => ?self.last_sortition.as_ref().map(|x| x.consensus_hash),
             );
-            return Ok(false);
+            return Err(RejectReason::SortitionViewMismatch);
         };
 
         if proposed_by.state().miner_pkh != block_pkh {
@@ -315,7 +326,7 @@ impl SortitionsView {
                 "proposed_block_pubkey_hash" => %block_pkh,
                 "sortition_winner_pubkey_hash" => %proposed_by.state().miner_pkh,
             );
-            return Ok(false);
+            return Err(RejectReason::PubkeyHashMismatch);
         }
 
         // check that this miner is the most recent sortition
@@ -327,7 +338,7 @@ impl SortitionsView {
                         "proposed_block_consensus_hash" => %block.header.consensus_hash,
                         "proposed_block_signer_sighash" => %block.header.signer_signature_hash(),
                     );
-                    return Ok(false);
+                    return Err(RejectReason::InvalidMiner);
                 }
             }
             ProposedBy::LastSortition(last_sortition) => {
@@ -343,27 +354,26 @@ impl SortitionsView {
                         "current_sortition_miner_status" => ?self.cur_sortition.miner_status,
                         "last_sortition" => %last_sortition.consensus_hash
                     );
-                    return Ok(false);
+                    return Err(RejectReason::NotLatestSortitionWinner);
                 }
             }
         };
 
         if let Some(tenure_change) = block.get_tenure_change_tx_payload() {
-            if !self.validate_tenure_change_payload(
+            self.validate_tenure_change_payload(
                 &proposed_by,
                 tenure_change,
                 block,
                 signer_db,
                 client,
-            )? {
-                return Ok(false);
-            }
+            )?;
         } 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)?;
+                Self::confirms_latest_block_in_same_tenure(block, signer_db)
+                    .map_err(SignerChainstateError::from)?;
             if !confirms_latest_in_tenure {
-                return Ok(false);
+                return Err(RejectReason::InvalidParentBlock);
             }
         }
 
@@ -389,11 +399,11 @@ impl SortitionsView {
                     "extend_timestamp" => extend_timestamp,
                     "epoch_time" => epoch_time,
                 );
-                return Ok(false);
+                return Err(RejectReason::InvalidTenureExtend);
             }
         }
 
-        Ok(true)
+        Ok(())
     }
 
     fn check_parent_tenure_choice(
@@ -666,7 +676,7 @@ impl SortitionsView {
         block: &NakamotoBlock,
         signer_db: &mut SignerDb,
         client: &StacksClient,
-    ) -> Result<bool, SignerChainstateError> {
+    ) -> Result<(), RejectReason> {
         // Ensure that the tenure change block confirms the expected parent block
         let confirms_expected_parent = Self::check_tenure_change_confirms_parent(
             tenure_change,
@@ -675,9 +685,10 @@ impl SortitionsView {
             client,
             self.config.tenure_last_block_proposal_timeout,
             self.config.reorg_attempts_activity_timeout,
-        )?;
+        )
+        .map_err(SignerChainstateError::from)?;
         if !confirms_expected_parent {
-            return Ok(false);
+            return Err(RejectReason::InvalidParentBlock);
         }
         // now, we have to check if the parent tenure was a valid choice.
         let is_valid_parent_tenure = Self::check_parent_tenure_choice(
@@ -688,21 +699,23 @@ impl SortitionsView {
             &self.config.first_proposal_burn_block_timing,
         )?;
         if !is_valid_parent_tenure {
-            return Ok(false);
+            return Err(RejectReason::ReorgNotAllowed);
         }
         let last_in_current_tenure = signer_db
             .get_last_globally_accepted_block(&block.header.consensus_hash)
-            .map_err(|e| ClientError::InvalidResponse(e.to_string()))?;
+            .map_err(|e| {
+                SignerChainstateError::from(ClientError::InvalidResponse(e.to_string()))
+            })?;
         if let Some(last_in_current_tenure) = last_in_current_tenure {
             warn!(
                 "Miner block proposal contains a tenure change, but we've already signed a block in this tenure. Considering proposal invalid.";
                 "proposed_block_consensus_hash" => %block.header.consensus_hash,
                 "proposed_block_signer_sighash" => %block.header.signer_signature_hash(),
                 "last_in_tenure_signer_sighash" => %last_in_current_tenure.block.header.signer_signature_hash(),
             );
-            return Ok(false);
+            return Err(RejectReason::DuplicateBlockFound);
         }
-        Ok(true)
+        Ok(())
     }
 
     fn confirms_latest_block_in_same_tenure(

stacks-signer/src/chainstate.rs

@@ -281,7 +281,7 @@ mod tests {
     use clarity::util::hash::{MerkleTree, Sha512Trunc256Sum};
     use clarity::util::secp256k1::MessageSignature;
     use libsigner::v0::messages::{
-        BlockRejection, BlockResponse, BlockResponseData, RejectCode, SignerMessage,
+        BlockRejection, BlockResponse, BlockResponseData, RejectCode, RejectReason, SignerMessage,
         SignerMessageMetadata,
     };
     use rand::{thread_rng, RngCore};
@@ -332,7 +332,10 @@ mod tests {
             chain_id: thread_rng().next_u32(),
             signature: MessageSignature::empty(),
             metadata: SignerMessageMetadata::empty(),
-            response_data: BlockResponseData::new(thread_rng().next_u64()),
+            response_data: BlockResponseData::new(
+                thread_rng().next_u64(),
+                RejectReason::RejectedInPriorRound,
+            ),
         };
         let signer_message = SignerMessage::BlockResponse(BlockResponse::Rejected(block_reject));
         let ack = StackerDBChunkAckData {