Merge branch 'develop' into fix/tenure-extend-with-other-transactions

Author: jcnelson

clarity/src/vm/contexts.rs

@@ -1639,7 +1639,7 @@ impl<'a, 'hooks> GlobalContext<'a, 'hooks> {
             );
             f(&mut exec_env)
         };
-        self.roll_back().map_err(crate::vm::errors::Error::from)?;
+        self.roll_back()?;
 
         match result {
             Ok(return_value) => Ok(return_value),

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

@@ -168,7 +168,7 @@ impl<'a> Bech32Writer<'a> {
 
     fn polymod_step(&mut self, v: u5) {
         let b = (self.chk >> 25) as u8;
-        self.chk = (self.chk & 0x01ff_ffff) << 5 ^ (u32::from(*v.as_ref()));
+        self.chk = ((self.chk & 0x01ff_ffff) << 5) ^ (u32::from(*v.as_ref()));
 
         for (i, item) in GEN.iter().enumerate() {
             if (b >> i) & 1 == 1 {
@@ -616,7 +616,7 @@ fn polymod(values: &[u5]) -> u32 {
     let mut b: u8;
     for v in values {
         b = (chk >> 25) as u8;
-        chk = (chk & 0x01ff_ffff) << 5 ^ (u32::from(*v.as_ref()));
+        chk = ((chk & 0x01ff_ffff) << 5) ^ (u32::from(*v.as_ref()));
 
         for (i, item) in GEN.iter().enumerate() {
             if (b >> i) & 1 == 1 {

stacks-common/src/deps_common/bech32/mod.rs

@@ -733,7 +733,7 @@ mod tests {
     #[test]
     pub fn hex_codec() {
         let init =
-            Uint256::from_u64(0xDEADBEEFDEADBEEF) << 64 | Uint256::from_u64(0x0102030405060708);
+            (Uint256::from_u64(0xDEADBEEFDEADBEEF) << 64) | Uint256::from_u64(0x0102030405060708);
 
         // little-endian representation
         let hex_init = "0807060504030201efbeaddeefbeadde00000000000000000000000000000000";

stacks-common/src/util/uint.rs

@@ -17,6 +17,8 @@ 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.
+- When allowing a reorg within the `reorg_attempts_activity_timeout_ms`, the signer will now watch the responses from other signers and if >30% of them reject this reorg attempt, then the signer will mark the miner as invalid, reject further attempts to reorg and allow the previous miner to extend their tenure.
 
 ## [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,16 @@ 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()? {
+
+            // If the current proposal is also for this current
+            // sortition, then we can return early here.
+            if self.cur_sortition.consensus_hash == block.header.consensus_hash {
+                return Err(RejectReason::InvalidMiner);
+            }
+        } 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,
@@ -243,6 +259,12 @@ impl SortitionsView {
                     );
                     self.cur_sortition.miner_status =
                         SortitionMinerStatus::InvalidatedBeforeFirstBlock;
+
+                    // If the current proposal is also for this current
+                    // sortition, then we can return early here.
+                    if self.cur_sortition.consensus_hash == block.header.consensus_hash {
+                        return Err(RejectReason::ReorgNotAllowed);
+                    }
                 }
             }
         }
@@ -266,7 +288,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 +315,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 +326,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 +338,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 +350,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 +366,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 +411,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(
@@ -471,7 +493,7 @@ impl SortitionsView {
                         0
                     };
                     if Duration::from_secs(proposal_to_sortition)
-                        <= *first_proposal_burn_block_timing
+                        < *first_proposal_burn_block_timing
                     {
                         info!(
                             "Miner is not building off of most recent tenure. A tenure they reorg has already mined blocks, but the block was poorly timed, allowing the reorg.";
@@ -577,7 +599,7 @@ impl SortitionsView {
                     "proposed_chain_length" => block.header.chain_length,
                     "expected_at_least" => info.block.header.chain_length + 1,
                 );
-                if info.signed_group.map_or(true, |signed_time| {
+                if info.signed_group.is_none_or(|signed_time| {
                     signed_time + reorg_attempts_activity_timeout.as_secs() > get_epoch_time_secs()
                 }) {
                     // Note if there is no signed_group time, this is a locally accepted block (i.e. tenure_last_block_proposal_timeout has not been exceeded).
@@ -649,7 +671,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,
@@ -658,9 +680,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(
@@ -671,21 +694,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 {