Signers: Use tenure extend timestamp to determine if enough time has passed for a tenure extend

stacks-signer/src/chainstate.rs

@@ -337,9 +337,14 @@ impl SortitionsView {
             // in tenure extends, we need to check:
             // (1) if this is the most recent sortition, an extend is allowed if it changes the burnchain view
             // (2) if this is the most recent sortition, an extend is allowed if enough time has passed to refresh the block limit
+            let sortition_consensus_hash = proposed_by.state().consensus_hash;
             let changed_burn_view =
-                tenure_extend.burn_view_consensus_hash != proposed_by.state().consensus_hash;
-            let enough_time_passed = Self::tenure_time_passed_block_lim()?;
+                tenure_extend.burn_view_consensus_hash != sortition_consensus_hash;
+            let enough_time_passed = get_epoch_time_secs()
+                > signer_db.get_tenure_extend_timestamp(
+                    self.config.tenure_idle_timeout,
+                    &sortition_consensus_hash,
+                );
             if !changed_burn_view && !enough_time_passed {
                 warn!(
                     "Miner block proposal contains a tenure extend, but the burnchain view has not changed and enough time has not passed to refresh the block limit. Considering proposal invalid.";
@@ -658,12 +663,6 @@ impl SortitionsView {
         }
     }
 
-    /// Has the current tenure lasted long enough to extend the block limit?
-    pub fn tenure_time_passed_block_lim() -> Result<bool, ClientError> {
-        // TODO
-        Ok(false)
-    }
-
     /// Fetch a new view of the recent sortitions
     pub fn fetch_view(
         config: ProposalEvalConfig,

stacks-signer/src/signerdb.rs

@@ -16,9 +16,10 @@
 
 use std::fmt::Display;
 use std::path::Path;
-use std::time::SystemTime;
+use std::time::{Duration, SystemTime};
 
 use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
+use blockstack_lib::chainstate::stacks::TransactionPayload;
 use blockstack_lib::util_lib::db::{
     query_row, query_rows, sqlite_open, table_exists, tx_begin_immediate, u64_to_sql,
     Error as DBError,
@@ -813,8 +814,8 @@ impl SignerDb {
         ))
     }
 
-    /// Return the all globally accepted block in a tenure (identified by its consensus hash).
-    pub fn get_globally_accepted_blocks(
+    /// Return the all globally accepted block in a tenure (identified by its consensus hash) in stacks height descending order
+    fn get_globally_accepted_blocks(
         &self,
         tenure: &ConsensusHash,
     ) -> Result<Vec<BlockInfo>, DBError> {
@@ -826,6 +827,47 @@ impl SignerDb {
             .map(|info| serde_json::from_str(info).map_err(DBError::from))
             .collect()
     }
+
+    /// Compute the tenure extend timestamp based on the tenure start and already consumed idle time of the
+    /// globally accepted blocks of the provided tenure (identified by the consensus hash)
+    pub fn get_tenure_extend_timestamp(
+        &self,
+        tenure_idle_timeout: Duration,
+        consensus_hash: &ConsensusHash,
+    ) -> u64 {
+        // We do not know our tenure start timestamp until we find the last processed tenure change transaction for the given consensus hash.
+        // We may not even have it in our database, in which case, we should use the oldest known block in the tenure.
+        // If we have no blocks known for this tenure, we will assume it has only JUST started and calculate
+        // our tenure extend timestamp based on the epoch time in secs.
+        let mut tenure_start_timestamp = None;
+        let mut tenure_process_time_ms = 0;
+        // Note that the globally accepted blocks are already returned in descending order of stacks height, therefore by newest block to oldest block
+        for block_info in self
+            .get_globally_accepted_blocks(consensus_hash)
+            .unwrap_or_default()
+            .iter()
+        {
+            // Always use the oldest block as our tenure start timestamp
+            tenure_start_timestamp = Some(block_info.proposed_time);
+            tenure_process_time_ms += block_info.validation_time_ms.unwrap_or(0);
+
+            if block_info
+                .block
+                .txs
+                .first()
+                .map(|tx| matches!(tx.payload, TransactionPayload::TenureChange(_)))
+                .unwrap_or(false)
+            {
+                // Tenure change found. No more blocks should count towards this tenure's processing time.
+                break;
+            }
+        }
+
+        tenure_start_timestamp
+            .unwrap_or(get_epoch_time_secs())
+            .saturating_add(tenure_idle_timeout.as_secs())
+            .saturating_sub(tenure_process_time_ms / 1000)
+    }
 }
 
 fn try_deserialize<T>(s: Option<String>) -> Result<Option<T>, DBError>

stacks-signer/src/v0/signer.rs

@@ -18,11 +18,10 @@ use std::sync::mpsc::Sender;
 use std::time::{Duration, Instant};
 
 use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader};
-use blockstack_lib::chainstate::stacks::TransactionPayload;
 use blockstack_lib::net::api::postblock_proposal::{
     BlockValidateOk, BlockValidateReject, BlockValidateResponse,
 };
-use clarity::types::chainstate::{ConsensusHash, StacksPrivateKey};
+use clarity::types::chainstate::StacksPrivateKey;
 use clarity::types::{PrivateKey, StacksEpochId};
 use clarity::util::hash::MerkleHashFunc;
 use clarity::util::secp256k1::Secp256k1PublicKey;
@@ -303,7 +302,10 @@ impl Signer {
             BlockResponse::accepted(
                 block_info.signer_signature_hash(),
                 signature,
-                self.calculate_tenure_extend_timestamp(&block_info.block.header.consensus_hash),
+                self.signer_db.get_tenure_extend_timestamp(
+                    self.proposal_config.tenure_idle_timeout,
+                    &block_info.block.header.consensus_hash,
+                ),
             )
         } else {
             debug!("{self}: Rejecting block {}", block_info.block.block_id());
@@ -312,7 +314,10 @@ impl Signer {
                 RejectCode::RejectedInPriorRound,
                 &self.private_key,
                 self.mainnet,
-                self.calculate_tenure_extend_timestamp(&block_info.block.header.consensus_hash),
+                self.signer_db.get_tenure_extend_timestamp(
+                    self.proposal_config.tenure_idle_timeout,
+                    &block_info.block.header.consensus_hash,
+                ),
             )
         };
         Some(response)
@@ -415,7 +420,8 @@ impl Signer {
                         RejectCode::ConnectivityIssues,
                         &self.private_key,
                         self.mainnet,
-                        self.calculate_tenure_extend_timestamp(
+                        self.signer_db.get_tenure_extend_timestamp(
+                            self.proposal_config.tenure_idle_timeout,
                             &block_proposal.block.header.consensus_hash,
                         ),
                     ))
@@ -432,7 +438,8 @@ impl Signer {
                         RejectCode::SortitionViewMismatch,
                         &self.private_key,
                         self.mainnet,
-                        self.calculate_tenure_extend_timestamp(
+                        self.signer_db.get_tenure_extend_timestamp(
+                            self.proposal_config.tenure_idle_timeout,
                             &block_proposal.block.header.consensus_hash,
                         ),
                     ))
@@ -451,7 +458,10 @@ impl Signer {
                 RejectCode::NoSortitionView,
                 &self.private_key,
                 self.mainnet,
-                self.calculate_tenure_extend_timestamp(&block_proposal.block.header.consensus_hash),
+                self.signer_db.get_tenure_extend_timestamp(
+                    self.proposal_config.tenure_idle_timeout,
+                    &block_proposal.block.header.consensus_hash,
+                ),
             ))
         };
 
@@ -588,7 +598,10 @@ impl Signer {
         let accepted = BlockAccepted::new(
             block_info.signer_signature_hash(),
             signature,
-            self.calculate_tenure_extend_timestamp(&block_info.block.header.consensus_hash),
+            self.signer_db.get_tenure_extend_timestamp(
+                self.proposal_config.tenure_idle_timeout,
+                &block_info.block.header.consensus_hash,
+            ),
         );
         // have to save the signature _after_ the block info
         self.handle_block_signature(stacks_client, &accepted);
@@ -643,7 +656,10 @@ impl Signer {
             block_validate_reject.clone(),
             &self.private_key,
             self.mainnet,
-            self.calculate_tenure_extend_timestamp(&block_info.block.header.consensus_hash),
+            self.signer_db.get_tenure_extend_timestamp(
+                self.proposal_config.tenure_idle_timeout,
+                &block_info.block.header.consensus_hash,
+            ),
         );
         self.signer_db
             .insert_block(&block_info)
@@ -741,7 +757,10 @@ impl Signer {
             RejectCode::ConnectivityIssues,
             &self.private_key,
             self.mainnet,
-            self.calculate_tenure_extend_timestamp(&block_proposal.block.header.consensus_hash),
+            self.signer_db.get_tenure_extend_timestamp(
+                self.proposal_config.tenure_idle_timeout,
+                &block_proposal.block.header.consensus_hash,
+            ),
         );
         if let Err(e) = block_info.mark_locally_rejected() {
             warn!("{self}: Failed to mark block as locally rejected: {e:?}",);
@@ -1124,7 +1143,10 @@ impl Signer {
                 RejectCode::TestingDirective,
                 &self.private_key,
                 self.mainnet,
-                self.calculate_tenure_extend_timestamp(&block_proposal.block.header.consensus_hash),
+                self.signer_db.get_tenure_extend_timestamp(
+                    self.proposal_config.tenure_idle_timeout,
+                    &block_proposal.block.header.consensus_hash,
+                ),
             ))
         } else {
             None
@@ -1143,41 +1165,4 @@ impl Signer {
             warn!("{self}: Failed to send mock signature to stacker-db: {e:?}",);
         }
     }
-
-    /// Calculate the tenure extend timestamp based on the tenure start and already consumed idle time.
-    fn calculate_tenure_extend_timestamp(&self, consensus_hash: &ConsensusHash) -> u64 {
-        // We do not know our tenure start timestamp until we find the last processed tenure change transaction for the given consensus hash.
-        // We may not even have it in our database, in which case, we should use the oldest known block in the tenure.
-        // If we have no blocks known for this tenure, we will assume it has only JUST started and calculate
-        // our tenure extend timestamp based on the epoch time in secs.
-        let mut tenure_start_timestamp = None;
-        let mut tenure_process_time_ms = 0;
-        // Note that the globally accepted blocks are already returned in descending order of stacks height, therefore by newest block to oldest block
-        for block_info in self
-            .signer_db
-            .get_globally_accepted_blocks(consensus_hash)
-            .unwrap_or_default()
-            .iter()
-        {
-            // Always use the oldest block as our tenure start timestamp
-            tenure_start_timestamp = Some(block_info.proposed_time);
-            tenure_process_time_ms += block_info.validation_time_ms.unwrap_or(0);
-
-            if block_info
-                .block
-                .txs
-                .first()
-                .map(|tx| matches!(tx.payload, TransactionPayload::TenureChange(_)))
-                .unwrap_or(false)
-            {
-                // Tenure change found. No more blocks should count towards this tenure's processing time.
-                break;
-            }
-        }
-
-        tenure_start_timestamp
-            .unwrap_or(get_epoch_time_secs())
-            .saturating_add(self.proposal_config.tenure_idle_timeout.as_secs())
-            .saturating_sub(tenure_process_time_ms / 1000)
-    }
 }

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

@@ -1866,10 +1866,15 @@ fn miner_forking() {
 
     info!("Flushing any pending commits to enable custom winner selection");
     let burn_height_before = get_burn_height();
+    let blocks_before = test_observer::get_blocks().len();
+    let nakamoto_blocks_count_before = get_nakamoto_headers(&conf).len();
     next_block_and(
         &mut signer_test.running_nodes.btc_regtest_controller,
         30,
-        || Ok(get_burn_height() > burn_height_before),
+        || {
+            Ok(get_burn_height() > burn_height_before
+                && test_observer::get_blocks().len() > blocks_before)
+        },
     )
     .unwrap();
 
@@ -2047,11 +2052,14 @@ fn miner_forking() {
     })
     .expect("Timed out waiting for miner 1 to RBF its old commit op");
 
+    let blocks_before = test_observer::get_blocks().len();
     info!("Mine RL1 Tenure");
-    signer_test
-        .running_nodes
-        .btc_regtest_controller
-        .build_next_block(1);
+    next_block_and(
+        &mut signer_test.running_nodes.btc_regtest_controller,
+        30,
+        || Ok(test_observer::get_blocks().len() > blocks_before),
+    )
+    .unwrap();
 
     // fetch the current sortition info
     let sortdb = conf.get_burnchain().open_sortition_db(true).unwrap();
@@ -2090,14 +2098,16 @@ fn miner_forking() {
 
     let peer_1_height = get_chain_info(&conf).stacks_tip_height;
     let peer_2_height = get_chain_info(&conf_node_2).stacks_tip_height;
+    let nakamoto_blocks_count = get_nakamoto_headers(&conf).len();
     info!("Peer height information"; "peer_1" => peer_1_height, "peer_2" => peer_2_height, "pre_naka_height" => pre_nakamoto_peer_1_height);
+    info!("Nakamoto blocks count before test: {nakamoto_blocks_count_before}, Nakamoto blocks count now: {nakamoto_blocks_count}");
     assert_eq!(peer_1_height, peer_2_height);
 
     let nakamoto_blocks_count = get_nakamoto_headers(&conf).len();
 
     assert_eq!(
         peer_1_height - pre_nakamoto_peer_1_height,
-        u64::try_from(nakamoto_blocks_count).unwrap() - 1, // subtract 1 for the first Nakamoto block
+        u64::try_from(nakamoto_blocks_count - nakamoto_blocks_count_before).unwrap(), // subtract 1 for the first Nakamoto block
         "There should be no forks in this test"
     );