Merge branch 'develop' into feat/extend-bitcoin-reorg

Author: kantai

.github/workflows/lock-threads.yml

@@ -29,7 +29,7 @@ jobs:
         id: lock_threads
         uses: stacks-network/actions/lock-threads@main
         with:
-          github-token: ${{ secrets.GH_TOKEN }}
+          github-token: ${{ secrets.GITHUB_TOKEN }}
           issue-inactive-days: 7
           pr-inactive-days: 7
           discussion-inactive-days: 7

.github/workflows/workflow-cleanup.yml

@@ -12,15 +12,19 @@ on:
       workflow-ttl:
         description: "How many days to keep a successful workflow (default: 30)"
         required: false
-        default: "30"
+        default: "60"
       failed-workflow-ttl:
         description: "How many days to keep failed workflows (default: 15)"
         required: false
-        default: "15"
+        default: "60"
   schedule:
     ## Run every day at 00:00:00
     - cron: "0 0 * * *"
 
+permissions:
+  actions: write    # to delete workflow runs and caches
+  contents: read    # to access repo contents 
+
 ## env vars are transferred to composite action steps
 env:
   CACHE_TTL: 7 ## number of days to keep a cache
@@ -41,7 +45,7 @@ jobs:
         id: cleanup
         uses: stacks-network/actions/cleanup/workflows@main
         with:
-          token: ${{ secrets.GH_TOKEN }}
+          token: ${{ secrets.GITHUB_TOKEN }}
           cache-ttl: ${{ inputs.cache-ttl || env.CACHE_TTL}}
           workflow-ttl: ${{ inputs.workflow-ttl || env.WORKFLOW_TTL}}
           failed-workflow-ttl: ${{ inputs.failed-workflow-ttl || env.FAILED_WORKFLOW_TTL }}

CHANGELOG.md

@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to the versioning scheme outlined in the [README.md](README.md).
 
+## [3.1.0.0.11]
+
+- Hotfix for p2p stack misbehavior in mempool syncing conditions
+
 ## [3.1.0.0.10]
 
 ### Added

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/tests/signer_state.rs

@@ -13,9 +13,11 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 use std::collections::HashMap;
+use std::time::SystemTime;
 
 use clarity::types::chainstate::{
-    ConsensusHash, StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey,
+    BurnchainHeaderHash, ConsensusHash, StacksAddress, StacksBlockId, StacksPrivateKey,
+    StacksPublicKey,
 };
 use clarity::util::hash::Hash160;
 use clarity::util::secp256k1::MessageSignature;
@@ -32,7 +34,7 @@ use crate::v0::signer_state::LocalStateMachine;
 #[test]
 fn check_capitulate_miner_view() {
     let mut address_weights = HashMap::new();
-    for _ in 0..5 {
+    for _ in 0..10 {
         let stacks_address = StacksAddress::p2pkh(
             false,
             &StacksPublicKey::from_private(&StacksPrivateKey::random()),
@@ -43,17 +45,23 @@ fn check_capitulate_miner_view() {
     let active_signer_protocol_version = 0;
     let local_supported_signer_protocol_version = 0;
     let burn_block = ConsensusHash([0x55; 20]);
-    let burn_block_height = 100;
     let parent_tenure_id = ConsensusHash([0x22; 20]);
     let parent_tenure_last_block = StacksBlockId([0x33; 32]);
     let parent_tenure_last_block_height = 1;
+
+    let old_miner_tenure_id = ConsensusHash([0x01; 20]);
+    let new_miner_tenure_id = ConsensusHash([0x00; 20]);
+
+    let burn_block_height = 100;
+
     let old_miner = StateMachineUpdateMinerState::ActiveMiner {
         current_miner_pkh: Hash160([0xab; 20]),
-        tenure_id: ConsensusHash([0x44; 20]),
+        tenure_id: old_miner_tenure_id,
         parent_tenure_id,
         parent_tenure_last_block,
         parent_tenure_last_block_height,
     };
+    // Make sure the old update still has the newer burn block height
     let old_update = StateMachineUpdateMessage::new(
         active_signer_protocol_version,
         local_supported_signer_protocol_version,
@@ -86,35 +94,21 @@ fn check_capitulate_miner_view() {
             StateMachineUpdateContent::V0 {
                 burn_block,
                 burn_block_height,
-                current_miner,
+                ..
             },
         ..
     } = local_update.clone()
     else {
         panic!("Unexpected state machine update message version");
     };
     // Let's create a new miner view
-    let new_tenure_id = ConsensusHash([0x00; 20]);
-
-    let db_path = tmp_db_path();
-    let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
-    let (mut block_info_1, _block_proposal) = create_block_override(|b| {
-        b.block.header.consensus_hash = new_tenure_id;
-        b.block.header.miner_signature = MessageSignature([0x01; 65]);
-        b.block.header.chain_length = 1;
-        b.burn_height = burn_block_height;
-    });
-
-    db.insert_block(&block_info_1).unwrap();
     let new_miner = StateMachineUpdateMinerState::ActiveMiner {
         current_miner_pkh: Hash160([0x00; 20]),
-        tenure_id: new_tenure_id,
+        tenure_id: new_miner_tenure_id,
         parent_tenure_id,
         parent_tenure_last_block,
         parent_tenure_last_block_height,
     };
-
-    // Let's update only our own view: the evaluator will tell me to revert my viewpoint to the old miner
     let new_update = StateMachineUpdateMessage::new(
         active_signer_protocol_version,
         local_supported_signer_protocol_version,
@@ -126,6 +120,43 @@ fn check_capitulate_miner_view() {
     )
     .unwrap();
 
+    // Update the database to have both the burn blocks corresponding to the tenure ids
+    // and both tenures have a locally accepted block
+    let db_path = tmp_db_path();
+    let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+    // Make sure both burn block corresponding to the tenure id's exist in our DB.
+    db.insert_burn_block(
+        &BurnchainHeaderHash([0u8; 32]),
+        &old_miner_tenure_id,
+        burn_block_height.saturating_sub(1),
+        &SystemTime::now(),
+        &BurnchainHeaderHash([1u8; 32]),
+    )
+    .unwrap();
+    db.insert_burn_block(
+        &BurnchainHeaderHash([0u8; 32]),
+        &new_miner_tenure_id,
+        burn_block_height,
+        &SystemTime::now(),
+        &BurnchainHeaderHash([1u8; 32]),
+    )
+    .unwrap();
+    let (mut block_info_1, _block_proposal) = create_block_override(|b| {
+        b.block.header.consensus_hash = old_miner_tenure_id;
+        b.block.header.miner_signature = MessageSignature([0x02; 65]);
+        b.block.header.chain_length = 1;
+        b.burn_height = burn_block_height.saturating_sub(1);
+    });
+    db.insert_block(&block_info_1).unwrap();
+
+    let (mut block_info_2, _block_proposal) = create_block_override(|b| {
+        b.block.header.consensus_hash = new_miner_tenure_id;
+        b.block.header.miner_signature = MessageSignature([0x01; 65]);
+        b.block.header.chain_length = 1;
+        b.burn_height = burn_block_height;
+    });
+    db.insert_block(&block_info_2).unwrap();
+
     let signer_state_machine = SignerStateMachine {
         burn_block,
         burn_block_height,
@@ -135,35 +166,43 @@ fn check_capitulate_miner_view() {
     };
 
     let mut local_state_machine = LocalStateMachine::Initialized(signer_state_machine.clone());
-    assert_eq!(
-        local_state_machine
-            .capitulate_miner_view(&mut global_eval, &mut db, local_address, &new_update)
-            .unwrap(),
-        current_miner
-    );
 
-    // Let's set a blocking minority to this different view: evaluator should see no global blocks for the blocking majority and return none
-    // I.e. only if the blocking minority is attempting to reject an reorg should it take priority over the rest.
-    // Let's update 1 other signer to some new miner key (60 percent)
-    for address in addresses.into_iter().skip(1).take(1) {
+    // Let's update 40 percent of other signers to some new miner key
+    for address in addresses.into_iter().take(4) {
         global_eval.insert_update(address, new_update.clone());
     }
+    // Miner view should be None as we can't find consensus on a single miner
     assert!(
         local_state_machine
             .capitulate_miner_view(&mut global_eval, &mut db, local_address, &new_update)
             .is_none(),
-        "Evaluator should have been unable to determine a majority view and return none"
+        "Evaluator should have told me to capitulate to the old miner"
     );
 
+    // Mark the old miner's block as globally accepted
     db.mark_block_globally_accepted(&mut block_info_1).unwrap();
-
     db.insert_block(&block_info_1).unwrap();
 
-    // Now that the blocking minority references a tenure which would actually get reorged, lets capitulate to their view
+    // Miner view should stay as the old miner as it has a globally accepted block and 60% consider it valid.
+    assert_eq!(
+        local_state_machine
+            .capitulate_miner_view(&mut global_eval, &mut db, local_address, &new_update)
+            .unwrap(),
+        old_miner,
+        "Evaluator should have told me to capitulate to the old miner"
+    );
+
+    // Now that we have a globally approved block for the new miner
+    db.mark_block_globally_accepted(&mut block_info_2).unwrap();
+    db.insert_block(&block_info_2).unwrap();
+
+    // Now that the blocking minority references a tenure which would actually get reorged, lets capitulate to the NEW view
+    // even though both the old and new signer have > 30% approval (it has a higher burn block).
     assert_eq!(
         local_state_machine
             .capitulate_miner_view(&mut global_eval, &mut db, local_address, &new_update)
             .unwrap(),
-        new_miner
+        new_miner,
+        "Evaluator should have told me to capitulate to the new miner"
     );
 }

stacks-signer/src/v0/signer.rs

@@ -255,6 +255,22 @@ impl SignerTrait<SignerMessage> for Signer {
         _res: &Sender<SignerResult>,
         current_reward_cycle: u64,
     ) {
+        self.check_submitted_block_proposal();
+        self.check_pending_block_validations(stacks_client);
+
+        let mut prior_state = self.local_state_machine.clone();
+        if self.reward_cycle <= current_reward_cycle {
+            self.local_state_machine.handle_pending_update(&self.signer_db, stacks_client, &self.proposal_config)
+                .unwrap_or_else(|e| error!("{self}: failed to update local state machine for pending update"; "err" => ?e));
+        }
+
+        if prior_state != self.local_state_machine {
+            let version = self.get_signer_protocol_version();
+            self.local_state_machine
+                .send_signer_update_message(&mut self.stackerdb, version);
+            prior_state = self.local_state_machine.clone();
+        }
+
         let event_parity = match event {
             // Block proposal events do have reward cycles, but each proposal has its own cycle,
             //  and the vec could be heterogeneous, so, don't differentiate.
@@ -272,8 +288,6 @@ impl SignerTrait<SignerMessage> for Signer {
         if event_parity == Some(other_signer_parity) {
             return;
         }
-        self.check_submitted_block_proposal();
-        self.check_pending_block_validations(stacks_client);
         debug!("{self}: Processing event: {event:?}");
         let Some(event) = event else {
             // No event. Do nothing.
@@ -292,12 +306,6 @@ impl SignerTrait<SignerMessage> for Signer {
             return;
         }
 
-        let prior_state = self.local_state_machine.clone();
-        if self.reward_cycle <= current_reward_cycle {
-            self.local_state_machine.handle_pending_update(&self.signer_db, stacks_client, &self.proposal_config)
-                .unwrap_or_else(|e| error!("{self}: failed to update local state machine for pending update"; "err" => ?e));
-        }
-
         self.handle_event_match(stacks_client, sortition_state, event, current_reward_cycle);
 
         self.check_submitted_block_proposal();
@@ -336,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 {
@@ -1376,7 +1392,7 @@ impl Signer {
             // Not enough rejection signatures to make a decision
             return;
         }
-        debug!("{self}: {total_reject_weight}/{total_weight} signers voted to reject the block {block_hash}");
+        info!("{self}: {total_reject_weight}/{total_weight} signers voted to reject the block {block_hash}");
         if let Err(e) = self.signer_db.mark_block_globally_rejected(&mut block_info) {
             warn!("{self}: Failed to mark block as globally rejected: {e:?}",);
         }