Merge branch 'develop' into fix/5502

Author: jcnelson

.github/workflows/bitcoin-tests.yml

@@ -130,6 +130,7 @@ jobs:
           - tests::signer::v0::continue_after_fast_block_no_sortition
           - tests::signer::v0::block_validation_response_timeout
           - tests::signer::v0::tenure_extend_after_bad_commit
+          - tests::signer::v0::block_proposal_max_age_rejections
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state

clarity/src/vm/contexts.rs

@@ -499,7 +499,7 @@ impl EventBatch {
 
 impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
     #[cfg(any(test, feature = "testing"))]
-    pub fn new(database: ClarityDatabase<'a>, epoch: StacksEpochId) -> OwnedEnvironment<'a, '_> {
+    pub fn new(database: ClarityDatabase<'a>, epoch: StacksEpochId) -> OwnedEnvironment<'a, 'a> {
         OwnedEnvironment {
             context: GlobalContext::new(
                 false,
@@ -513,7 +513,7 @@ impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
     }
 
     #[cfg(any(test, feature = "testing"))]
-    pub fn new_toplevel(mut database: ClarityDatabase<'a>) -> OwnedEnvironment<'a, '_> {
+    pub fn new_toplevel(mut database: ClarityDatabase<'a>) -> OwnedEnvironment<'a, 'a> {
         database.begin();
         let epoch = database.get_clarity_epoch_version().unwrap();
         let version = ClarityVersion::default_for_epoch(epoch);
@@ -540,7 +540,7 @@ impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
         mut database: ClarityDatabase<'a>,
         epoch: StacksEpochId,
         use_mainnet: bool,
-    ) -> OwnedEnvironment<'a, '_> {
+    ) -> OwnedEnvironment<'a, 'a> {
         use crate::vm::tests::test_only_mainnet_to_chain_id;
         let cost_track = LimitedCostTracker::new_max_limit(&mut database, epoch, use_mainnet)
             .expect("FAIL: problem instantiating cost tracking");
@@ -557,7 +557,7 @@ impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
         chain_id: u32,
         database: ClarityDatabase<'a>,
         epoch_id: StacksEpochId,
-    ) -> OwnedEnvironment<'a, '_> {
+    ) -> OwnedEnvironment<'a, 'a> {
         OwnedEnvironment {
             context: GlobalContext::new(
                 mainnet,
@@ -576,7 +576,7 @@ impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
         database: ClarityDatabase<'a>,
         cost_tracker: LimitedCostTracker,
         epoch_id: StacksEpochId,
-    ) -> OwnedEnvironment<'a, '_> {
+    ) -> OwnedEnvironment<'a, 'a> {
         OwnedEnvironment {
             context: GlobalContext::new(mainnet, chain_id, database, cost_tracker, epoch_id),
             call_stack: CallStack::new(),
@@ -1546,7 +1546,7 @@ impl<'a, 'hooks> GlobalContext<'a, 'hooks> {
         database: ClarityDatabase<'a>,
         cost_track: LimitedCostTracker,
         epoch_id: StacksEpochId,
-    ) -> GlobalContext {
+    ) -> GlobalContext<'a, 'hooks> {
         GlobalContext {
             database,
             cost_track,

clarity/src/vm/database/key_value_wrapper.rs

@@ -205,7 +205,7 @@ where
 }
 
 impl<'a> RollbackWrapper<'a> {
-    pub fn new(store: &'a mut dyn ClarityBackingStore) -> RollbackWrapper {
+    pub fn new(store: &'a mut dyn ClarityBackingStore) -> RollbackWrapper<'a> {
         RollbackWrapper {
             store,
             lookup_map: HashMap::new(),
@@ -218,7 +218,7 @@ impl<'a> RollbackWrapper<'a> {
     pub fn from_persisted_log(
         store: &'a mut dyn ClarityBackingStore,
         log: RollbackWrapperPersistedLog,
-    ) -> RollbackWrapper {
+    ) -> RollbackWrapper<'a> {
         RollbackWrapper {
             store,
             lookup_map: log.lookup_map,

stacks-signer/CHANGELOG.md

@@ -9,7 +9,10 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ## Added
 
+- Introduced the `block_proposal_max_age_secs` configuration option for signers, enabling them to automatically ignore block proposals that exceed the specified age in seconds.
+
 ## Changed
+- Improvements to the stale signer cleanup logic: deletes the prior signer if it has no remaining unprocessed blocks in its database
 
 ## [3.1.0.0.1.0]
 

stacks-signer/src/chainstate.rs

@@ -367,7 +367,7 @@ impl SortitionsView {
                 tenure_extend.burn_view_consensus_hash != sortition_consensus_hash;
             let extend_timestamp = signer_db.calculate_tenure_extend_timestamp(
                 self.config.tenure_idle_timeout,
-                &block,
+                block,
                 false,
             );
             let epoch_time = get_epoch_time_secs();

stacks-signer/src/client/mod.rs

@@ -414,6 +414,7 @@ pub(crate) mod tests {
             tenure_last_block_proposal_timeout: config.tenure_last_block_proposal_timeout,
             block_proposal_validation_timeout: config.block_proposal_validation_timeout,
             tenure_idle_timeout: config.tenure_idle_timeout,
+            block_proposal_max_age_secs: config.block_proposal_max_age_secs,
         }
     }
 

stacks-signer/src/config.rs

@@ -39,6 +39,7 @@ const BLOCK_PROPOSAL_VALIDATION_TIMEOUT_MS: u64 = 120_000;
 const DEFAULT_FIRST_PROPOSAL_BURN_BLOCK_TIMING_SECS: u64 = 60;
 const DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS: u64 = 30;
 const TENURE_IDLE_TIMEOUT_SECS: u64 = 300;
+const DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS: u64 = 600;
 
 #[derive(thiserror::Error, Debug)]
 /// An error occurred parsing the provided configuration
@@ -138,6 +139,8 @@ pub struct SignerConfig {
     pub block_proposal_validation_timeout: Duration,
     /// How much idle time must pass before allowing a tenure extend
     pub tenure_idle_timeout: Duration,
+    /// The maximum age of a block proposal in seconds that will be processed by the signer
+    pub block_proposal_max_age_secs: u64,
 }
 
 /// The parsed configuration for the signer
@@ -176,6 +179,8 @@ pub struct GlobalConfig {
     pub block_proposal_validation_timeout: Duration,
     /// How much idle time must pass before allowing a tenure extend
     pub tenure_idle_timeout: Duration,
+    /// The maximum age of a block proposal that will be processed by the signer
+    pub block_proposal_max_age_secs: u64,
 }
 
 /// Internal struct for loading up the config file
@@ -213,6 +218,8 @@ struct RawConfigFile {
     pub block_proposal_validation_timeout_ms: Option<u64>,
     /// How much idle time (in seconds) must pass before a tenure extend is allowed
     pub tenure_idle_timeout_secs: Option<u64>,
+    /// The maximum age of a block proposal (in secs) that will be processed by the signer.
+    pub block_proposal_max_age_secs: Option<u64>,
 }
 
 impl RawConfigFile {
@@ -310,6 +317,10 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
                 .unwrap_or(TENURE_IDLE_TIMEOUT_SECS),
         );
 
+        let block_proposal_max_age_secs = raw_data
+            .block_proposal_max_age_secs
+            .unwrap_or(DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS);
+
         Ok(Self {
             node_host: raw_data.node_host,
             endpoint,
@@ -326,6 +337,7 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
             tenure_last_block_proposal_timeout,
             block_proposal_validation_timeout,
             tenure_idle_timeout,
+            block_proposal_max_age_secs,
         })
     }
 }

stacks-signer/src/runloop.rs

@@ -286,6 +286,7 @@ impl<Signer: SignerTrait<T>, T: StacksMessageCodec + Clone + Send + Debug> RunLo
             tenure_last_block_proposal_timeout: self.config.tenure_last_block_proposal_timeout,
             block_proposal_validation_timeout: self.config.block_proposal_validation_timeout,
             tenure_idle_timeout: self.config.tenure_idle_timeout,
+            block_proposal_max_age_secs: self.config.block_proposal_max_age_secs,
         }))
     }
 
@@ -423,23 +424,15 @@ impl<Signer: SignerTrait<T>, T: StacksMessageCodec + Clone + Send + Debug> RunLo
         let mut to_delete = Vec::new();
         for (idx, signer) in &mut self.stacks_signers {
             let reward_cycle = signer.reward_cycle();
-            let next_reward_cycle = reward_cycle.wrapping_add(1);
-            let stale = match next_reward_cycle.cmp(&current_reward_cycle) {
-                std::cmp::Ordering::Less => true, // We are more than one reward cycle behind, so we are stale
-                std::cmp::Ordering::Equal => {
-                    // We are the next reward cycle, so check if we were registered and have any pending blocks to process
-                    match signer {
-                        ConfiguredSigner::RegisteredSigner(signer) => {
-                            !signer.has_unprocessed_blocks()
-                        }
-                        _ => true,
-                    }
+            if reward_cycle >= current_reward_cycle {
+                // We are either the current or a future reward cycle, so we are not stale.
+                continue;
+            }
+            if let ConfiguredSigner::RegisteredSigner(signer) = signer {
+                if !signer.has_unprocessed_blocks() {
+                    debug!("{signer}: Signer's tenure has completed.");
+                    to_delete.push(*idx);
                 }
-                std::cmp::Ordering::Greater => false, // We are the current reward cycle, so we are not stale
-            };
-            if stale {
-                debug!("{signer}: Signer's tenure has completed.");
-                to_delete.push(*idx);
             }
         }
         for idx in to_delete {

stacks-signer/src/v0/signer.rs

@@ -92,6 +92,8 @@ pub struct Signer {
     pub block_proposal_validation_timeout: Duration,
     /// The current submitted block proposal and its submission time
     pub submitted_block_proposal: Option<(BlockProposal, Instant)>,
+    /// Maximum age of a block proposal in seconds before it is dropped without processing
+    pub block_proposal_max_age_secs: u64,
 }
 
 impl std::fmt::Display for Signer {
@@ -284,6 +286,7 @@ impl From<SignerConfig> for Signer {
             proposal_config,
             submitted_block_proposal: None,
             block_proposal_validation_timeout: signer_config.block_proposal_validation_timeout,
+            block_proposal_max_age_secs: signer_config.block_proposal_max_age_secs,
         }
     }
 }
@@ -344,6 +347,24 @@ impl Signer {
             return;
         }
 
+        if block_proposal
+            .block
+            .header
+            .timestamp
+            .saturating_add(self.block_proposal_max_age_secs)
+            < get_epoch_time_secs()
+        {
+            // Block is too old. Drop it with a warning. Don't even bother broadcasting to the node.
+            warn!("{self}: Received a block proposal that is more than {} secs old. Ignoring...", self.block_proposal_max_age_secs;
+                "signer_sighash" => %block_proposal.block.header.signer_signature_hash(),
+                "block_id" => %block_proposal.block.block_id(),
+                "block_height" => block_proposal.block.header.chain_length,
+                "burn_height" => block_proposal.burn_height,
+                "timestamp" => block_proposal.block.header.timestamp,
+            );
+            return;
+        }
+
         // TODO: should add a check to ignore an old burn block height if we know its outdated. Would require us to store the burn block height we last saw on the side.
         //  the signer needs to be able to determine whether or not the block they're about to sign would conflict with an already-signed Stacks block
         let signer_signature_hash = block_proposal.block.header.signer_signature_hash();

stackslib/src/burnchains/bitcoin/indexer.rs

@@ -924,7 +924,7 @@ impl BitcoinIndexer {
             return Ok(());
         }
         warn!(
-            "Header at height {} is not wihtin 2 hours of now (is at {})",
+            "Header at height {} is not within 2 hours of now (is at {})",
             highest_header_height, highest_header.block_header.header.time
         );
         self.drop_headers(highest_header_height.saturating_sub(1))?;

stackslib/src/net/tests/mod.rs

@@ -355,7 +355,7 @@ impl NakamotoBootPlan {
     fn boot_nakamoto_peers<'a>(
         mut self,
         observer: Option<&'a TestEventObserver>,
-    ) -> (TestPeer<'a>, Vec<TestPeer>) {
+    ) -> (TestPeer<'a>, Vec<TestPeer<'a>>) {
         let mut peer_config = TestPeerConfig::new(&self.test_name, 0, 0);
         peer_config.network_id = self.network_id;
         peer_config.private_key = self.private_key.clone();
@@ -666,7 +666,7 @@ impl NakamotoBootPlan {
         self,
         boot_plan: Vec<NakamotoBootTenure>,
         observer: Option<&'a TestEventObserver>,
-    ) -> (TestPeer<'a>, Vec<TestPeer>) {
+    ) -> (TestPeer<'a>, Vec<TestPeer<'a>>) {
         let test_signers = self.test_signers.clone();
         let pox_constants = self.pox_constants.clone();
         let test_stackers = self.test_stackers.clone();

testnet/stacks-node/src/tests/signer/mod.rs

@@ -310,7 +310,8 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
     }
 
     /// Mine a BTC block and wait for a new Stacks block to be mined
-    fn mine_nakamoto_block(&mut self, timeout: Duration) {
+    /// Note: do not use nakamoto blocks mined heuristic if running a test with multiple miners
+    fn mine_nakamoto_block(&mut self, timeout: Duration, use_nakamoto_blocks_mined: bool) {
         let commits_submitted = self.running_nodes.commits_submitted.clone();
         let mined_block_time = Instant::now();
         let mined_before = self.running_nodes.nakamoto_blocks_mined.get();
@@ -327,7 +328,7 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
             let info_after = self.get_peer_info();
             let blocks_mined = self.running_nodes.nakamoto_blocks_mined.get();
             Ok(info_after.stacks_tip_height > info_before.stacks_tip_height
-                && blocks_mined > mined_before)
+                && (!use_nakamoto_blocks_mined || blocks_mined > mined_before))
         })
         .unwrap();
         let mined_block_elapsed_time = mined_block_time.elapsed();