diff --git a/stacks-signer/src/chainstate.rs b/stacks-signer/src/chainstate.rs index 66bf173941..5a6aa04200 100644 --- a/stacks-signer/src/chainstate.rs +++ b/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 { - // TODO - Ok(false) - } - /// Fetch a new view of the recent sortitions pub fn fetch_view( config: ProposalEvalConfig, diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs index bc163ee686..c7bd9f9a6a 100644 --- a/stacks-signer/src/signerdb.rs +++ b/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, 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(s: Option) -> Result, DBError> diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index e28703f56f..d6ae24b83d 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/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) - } } diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 2952fcbae3..e3d7ba98b5 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/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" );