Merge pull request #6307 from jferrant/fix/do-not-hammer-node-in-capitulation

Do not capitulate if signer has recently contributed to a globally accepted block or recently already checked capitulation

Author: jferrant

libsigner/src/tests/signer_state.rs

@@ -13,7 +13,6 @@
 // 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 blockstack_lib::chainstate::stacks::{
     StacksTransaction, TokenTransferMemo, TransactionAnchorMode, TransactionAuth,
@@ -239,7 +238,6 @@ fn determine_global_states() {
         current_miner: (&current_miner).into(),
         active_signer_protocol_version: local_supported_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
         tx_replay_set: ReplayTransactionSet::none(),
-        update_time: SystemTime::now(),
     };
 
     global_eval.insert_update(local_address, local_update);
@@ -279,7 +277,6 @@ fn determine_global_states() {
         current_miner: (&new_miner).into(),
         active_signer_protocol_version: local_supported_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
         tx_replay_set: ReplayTransactionSet::none(),
-        update_time: SystemTime::now(),
     };
 
     global_eval.insert_update(local_address, new_update);
@@ -320,7 +317,6 @@ fn determine_global_states_with_tx_replay_set() {
         current_miner: (&current_miner).into(),
         active_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
         tx_replay_set: ReplayTransactionSet::none(),
-        update_time: SystemTime::now(),
     };
 
     let burn_block = ConsensusHash([20u8; 20]);
@@ -358,7 +354,6 @@ fn determine_global_states_with_tx_replay_set() {
         current_miner: (&current_miner).into(),
         active_signer_protocol_version: local_supported_signer_protocol_version, // a majority of signers are saying they support version the same local_supported_signer_protocol_version, so update it here...
         tx_replay_set: ReplayTransactionSet::none(),
-        update_time: SystemTime::now(),
     };
 
     // Let's tip the scales over to the correct burn view
@@ -415,7 +410,6 @@ fn determine_global_states_with_tx_replay_set() {
         current_miner: (&current_miner).into(),
         active_signer_protocol_version,
         tx_replay_set: ReplayTransactionSet::new(vec![tx]),
-        update_time: SystemTime::now(),
     };
 
     assert_eq!(

libsigner/src/v0/signer_state.rs

@@ -15,7 +15,6 @@
 
 use std::collections::HashMap;
 use std::hash::{Hash, Hasher};
-use std::time::SystemTime;
 
 use blockstack_lib::chainstate::stacks::StacksTransaction;
 use clarity::types::chainstate::StacksAddress;
@@ -120,7 +119,6 @@ impl GlobalStateEvaluator {
                 active_signer_protocol_version,
                 // We need to calculate the threshold for the tx_replay_set separately
                 tx_replay_set: ReplayTransactionSet::none(),
-                update_time: SystemTime::now(),
             };
             let key = SignerStateMachineKey(state_machine.clone());
             let entry = state_views.entry(key).or_insert_with(|| 0);
@@ -240,7 +238,7 @@ impl Default for ReplayTransactionSet {
 /// A signer state machine view. This struct can
 ///  be used to encode the local signer's view or
 ///  the global view.
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
 pub struct SignerStateMachine {
     /// The tip burn block (i.e., the latest bitcoin block) seen by this signer
     pub burn_block: ConsensusHash,
@@ -252,42 +250,16 @@ pub struct SignerStateMachine {
     pub active_signer_protocol_version: u64,
     /// Transaction replay set
     pub tx_replay_set: ReplayTransactionSet,
-    /// The time when this state machine was last updated
-    pub update_time: SystemTime,
-}
-
-impl PartialEq for SignerStateMachine {
-    fn eq(&self, other: &Self) -> bool {
-        // update_time is intentionally ignored
-        self.burn_block == other.burn_block
-            && self.burn_block_height == other.burn_block_height
-            && self.current_miner == other.current_miner
-            && self.active_signer_protocol_version == other.active_signer_protocol_version
-            && self.tx_replay_set == other.tx_replay_set
-    }
-}
-
-impl Eq for SignerStateMachine {}
-
-impl Hash for SignerStateMachine {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        // update_time is intentionally ignored
-        self.burn_block.hash(state);
-        self.burn_block_height.hash(state);
-        self.current_miner.hash(state);
-        self.active_signer_protocol_version.hash(state);
-        self.tx_replay_set.hash(state);
-    }
 }
 
 #[derive(Debug)]
 /// A wrapped SignerStateMachine that implements a very specific hash that enables properly ignoring the
-/// tx_replay_set and update_time when evaluating the global signer state machine
+/// tx_replay_set when evaluating the global signer state machine
 pub struct SignerStateMachineKey(SignerStateMachine);
 
 impl PartialEq for SignerStateMachineKey {
     fn eq(&self, other: &Self) -> bool {
-        // NOTE: tx_replay_set and update_time are intentionally ignored
+        // NOTE: tx_replay_set is intentionally ignored
         self.0.burn_block == other.0.burn_block
             && self.0.burn_block_height == other.0.burn_block_height
             && self.0.current_miner == other.0.current_miner

stacks-node/src/tests/signer/multiversion.rs

@@ -104,7 +104,6 @@ pub fn signer_state_machine_v3_1_00_13_to_current(
                 .map(stacks_transaction_v3_1_00_13_to_current)
                 .collect(),
         ),
-        update_time: SystemTime::now(),
     }
 }
 

stacks-signer/src/chainstate/tests/v2.rs

@@ -142,7 +142,6 @@ fn setup_test_environment(
         },
         active_signer_protocol_version: 0,
         tx_replay_set: ReplayTransactionSet::none(),
-        update_time: SystemTime::now(),
     };
 
     let sortitions_view = GlobalStateView {

stacks-signer/src/signerdb.rs

@@ -17,7 +17,7 @@
 use std::collections::HashMap;
 use std::fmt::Display;
 use std::path::Path;
-use std::time::{Duration, SystemTime};
+use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
 use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
 use blockstack_lib::chainstate::stacks::TransactionPayload;
@@ -632,6 +632,11 @@ static ADD_BURN_BLOCK_RECEIVED_TIMES_CONSENSUS_HASH_INDEX: &str = r#"
 CREATE INDEX IF NOT EXISTS burn_block_updates_received_times_consensus_hash ON burn_block_updates_received_times(burn_block_consensus_hash, received_time ASC);
 "#;
 
+// Used by get_last_globally_accepted_block_signed_self
+static ADD_BLOCK_SIGNED_SELF_INDEX: &str = r#"
+CREATE INDEX idx_blocks_query_opt ON blocks (consensus_hash, state, signed_self, burn_block_height DESC);
+"#;
+
 static DROP_BLOCK_SIGNATURES_TABLE: &str = r#"
 DROP TABLE IF EXISTS block_signatures;
 "#;
@@ -773,6 +778,7 @@ static SCHEMA_15: &[&str] = &[
 static SCHEMA_16: &[&str] = &[
     CREATE_BURN_BLOCK_UPDATES_RECEIVED_TIME_TABLE,
     ADD_BURN_BLOCK_RECEIVED_TIMES_CONSENSUS_HASH_INDEX,
+    ADD_BLOCK_SIGNED_SELF_INDEX,
     DROP_BLOCK_SIGNATURES_TABLE,
     CREATE_BLOCK_SIGNATURES_TABLE_V16,
     DROP_BLOCK_REJECTION_SIGNER_ADDRS,
@@ -1152,6 +1158,25 @@ impl SignerDb {
         try_deserialize(result)
     }
 
+    /// Return the last globally accepted block self_signed time in a given tenure (identified by its consensus hash).
+    pub fn get_last_globally_accepted_block_signed_self(
+        &self,
+        tenure: &ConsensusHash,
+    ) -> Result<Option<SystemTime>, DBError> {
+        let query = r#"
+            SELECT signed_self
+            FROM blocks
+            WHERE consensus_hash = ?1
+            AND state = ?2
+            AND signed_self IS NOT NULL
+            ORDER BY burn_block_height DESC
+            LIMIT 1;
+        "#;
+        let args = params![tenure, &BlockState::GloballyAccepted.to_string()];
+        let result: Option<u64> = query_row(&self.db, query, args)?;
+        Ok(result.map(|signed_self| UNIX_EPOCH + Duration::from_secs(signed_self)))
+    }
+
     /// Return the canonical tip -- the last globally accepted block.
     pub fn get_canonical_tip(&self) -> Result<Option<BlockInfo>, DBError> {
         let query = "SELECT block_info FROM blocks WHERE state = ?1 ORDER BY stacks_height DESC, signed_group DESC LIMIT 1";
@@ -3296,4 +3321,64 @@ pub mod tests {
             .unwrap()
             .is_none());
     }
+
+    #[test]
+    fn test_get_last_globally_accepted_block_signed_self() {
+        let db_path = tmp_db_path();
+        let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+
+        let consensus_hash_1 = ConsensusHash([0x01; 20]);
+        let consensus_hash_2 = ConsensusHash([0x02; 20]);
+
+        // Create blocks with different burn heights and signed_self timestamps (seconds since epoch)
+        let (mut block_info_1, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.miner_signature = MessageSignature([0x01; 65]);
+            b.block.header.chain_length = 1;
+            b.burn_height = 1;
+        });
+        block_info_1.mark_locally_accepted(false).unwrap();
+        let (mut block_info_2, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.miner_signature = MessageSignature([0x02; 65]);
+            b.block.header.chain_length = 2;
+            b.burn_height = 2;
+        });
+        block_info_2.mark_locally_accepted(false).unwrap();
+        let (mut block_info_3, _block_proposal) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_2;
+            b.block.header.miner_signature = MessageSignature([0x03; 65]);
+            b.block.header.chain_length = 3;
+            b.burn_height = 3;
+        });
+        block_info_3.mark_locally_accepted(false).unwrap();
+
+        // Mark only one of the blocks as globally accepted
+        block_info_1.mark_globally_accepted().unwrap();
+
+        // Insert into db
+        db.insert_block(&block_info_1).unwrap();
+        db.insert_block(&block_info_2).unwrap();
+        db.insert_block(&block_info_3).unwrap();
+
+        // Query for consensus_hash_1 should return signed_self of block_info_2 (highest burn_height)
+        db.get_last_globally_accepted_block_signed_self(&consensus_hash_1)
+            .unwrap()
+            .expect("Expected a signed_self timestamp");
+
+        // Query for consensus_hash_2 should return none since we only contributed to a locally signed block
+        let result_2 = db
+            .get_last_globally_accepted_block_signed_self(&consensus_hash_2)
+            .unwrap();
+
+        assert!(result_2.is_none());
+
+        // Query for a consensus hash with no blocks should return None
+        let consensus_hash_3 = ConsensusHash([0x03; 20]);
+        let result_3 = db
+            .get_last_globally_accepted_block_signed_self(&consensus_hash_3)
+            .unwrap();
+
+        assert!(result_3.is_none());
+    }
 }

stacks-signer/src/tests/signer_state.rs

@@ -181,7 +181,6 @@ fn check_capitulate_miner_view() {
         current_miner: (&new_miner).into(),
         tx_replay_set: ReplayTransactionSet::none(),
         active_signer_protocol_version,
-        update_time: SystemTime::now(),
     };
 
     let mut local_state_machine = LocalStateMachine::Initialized(signer_state_machine.clone());
@@ -392,7 +391,6 @@ fn check_miner_inactivity_timeout() {
         current_miner: inactive_miner,
         active_signer_protocol_version: 0,
         tx_replay_set: ReplayTransactionSet::none(),
-        update_time: SystemTime::now(),
     };
     local_state_machine = LocalStateMachine::Initialized(signer_state.clone());
     local_state_machine

stacks-signer/src/v0/signer.rs

@@ -131,6 +131,8 @@ pub struct Signer {
     pub tx_replay_scope: ReplayScopeOpt,
     /// Time to wait between updating our local state machine view point and capitulating to other signers miner view
     pub capitulate_miner_view_timeout: Duration,
+    /// The last time we capitulated our miner viewpoint
+    pub last_capitulate_miner_view: SystemTime,
     /// The signer supported protocol version. used only in testing
     #[cfg(any(test, feature = "testing"))]
     pub supported_signer_protocol_version: u64,
@@ -262,6 +264,7 @@ impl SignerTrait<SignerMessage> for Signer {
             validate_with_replay_tx: signer_config.validate_with_replay_tx,
             tx_replay_scope: None,
             capitulate_miner_view_timeout: signer_config.capitulate_miner_view_timeout,
+            last_capitulate_miner_view: SystemTime::now(),
             #[cfg(any(test, feature = "testing"))]
             supported_signer_protocol_version: signer_config.supported_signer_protocol_version,
         }
@@ -301,6 +304,7 @@ impl SignerTrait<SignerMessage> for Signer {
             sortition_state,
             self.capitulate_miner_view_timeout,
             self.proposal_config.tenure_last_block_proposal_timeout,
+            &mut self.last_capitulate_miner_view,
         );
 
         if prior_state != self.local_state_machine {