diff --git a/stacks-signer/src/chainstate.rs b/stacks-signer/src/chainstate.rs index dbf03c1f91..31454c96b6 100644 --- a/stacks-signer/src/chainstate.rs +++ b/stacks-signer/src/chainstate.rs @@ -539,7 +539,7 @@ impl SortitionsView { /// /// The rationale here is that the signer DB can be out-of-sync with the node. For example, /// the signer may have been added to an already-running node. - fn check_tenure_change_confirms_parent( + pub fn check_tenure_change_confirms_parent( tenure_change: &TenureChangePayload, block: &NakamotoBlock, signer_db: &mut SignerDb, diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs index 0da2c1adcc..4cdc61471a 100644 --- a/stacks-signer/src/signerdb.rs +++ b/stacks-signer/src/signerdb.rs @@ -733,18 +733,6 @@ impl SignerDb { try_deserialize(result) } - /// Return the last accepted block the signer (highest stacks height). It will tie break a match based on which was more recently signed. - pub fn get_signer_last_accepted_block(&self) -> Result, DBError> { - let query = "SELECT block_info FROM blocks WHERE state IN (?1, ?2) ORDER BY stacks_height DESC, signed_group DESC, signed_self DESC LIMIT 1"; - let args = params![ - &BlockState::GloballyAccepted.to_string(), - &BlockState::LocallyAccepted.to_string() - ]; - let result: Option = query_row(&self.db, query, args)?; - - try_deserialize(result) - } - /// Return the last accepted block in a tenure (identified by its consensus hash). pub fn get_last_accepted_block( &self, @@ -1769,69 +1757,4 @@ mod tests { < block_infos[0].proposed_time ); } - - #[test] - fn signer_last_accepted_block() { - 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_1) = create_block_override(|b| { - b.block.header.miner_signature = MessageSignature([0x01; 65]); - b.block.header.chain_length = 1; - b.burn_height = 1; - }); - - let (mut block_info_2, _block_proposal_2) = create_block_override(|b| { - b.block.header.miner_signature = MessageSignature([0x02; 65]); - b.block.header.chain_length = 2; - b.burn_height = 1; - }); - - let (mut block_info_3, _block_proposal_3) = create_block_override(|b| { - b.block.header.miner_signature = MessageSignature([0x02; 65]); - b.block.header.chain_length = 2; - b.burn_height = 4; - }); - block_info_3 - .mark_locally_accepted(false) - .expect("Failed to mark block as locally accepted"); - - db.insert_block(&block_info_1) - .expect("Unable to insert block into db"); - db.insert_block(&block_info_2) - .expect("Unable to insert block into db"); - - assert!(db.get_signer_last_accepted_block().unwrap().is_none()); - - block_info_1 - .mark_globally_accepted() - .expect("Failed to mark block as globally accepted"); - db.insert_block(&block_info_1) - .expect("Unable to insert block into db"); - - assert_eq!( - db.get_signer_last_accepted_block().unwrap().unwrap(), - block_info_1 - ); - - block_info_2 - .mark_globally_accepted() - .expect("Failed to mark block as globally accepted"); - block_info_2.signed_self = Some(get_epoch_time_secs()); - db.insert_block(&block_info_2) - .expect("Unable to insert block into db"); - - assert_eq!( - db.get_signer_last_accepted_block().unwrap().unwrap(), - block_info_2 - ); - - db.insert_block(&block_info_3) - .expect("Unable to insert block into db"); - - assert_eq!( - db.get_signer_last_accepted_block().unwrap().unwrap(), - block_info_3 - ); - } } diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index af5572c070..5ba7ff7423 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -582,60 +582,80 @@ impl Signer { } } - /// WARNING: Do NOT call this function PRIOR to check_proposal or block_proposal validation succeeds. + /// WARNING: This is an incomplete check. Do NOT call this function PRIOR to check_proposal or block_proposal validation succeeds. /// /// Re-verify a block's chain length against the last signed block within signerdb. /// This is required in case a block has been approved since the initial checks of the block validation endpoint. fn check_block_against_signer_db_state( - &self, + &mut self, + stacks_client: &StacksClient, proposed_block: &NakamotoBlock, ) -> Option { let signer_signature_hash = proposed_block.header.signer_signature_hash(); let proposed_block_consensus_hash = proposed_block.header.consensus_hash; + // If this is a tenure change block, ensure that it confirms the correct number of blocks from the parent tenure. + if let Some(tenure_change) = proposed_block.get_tenure_change_tx_payload() { + // Ensure that the tenure change block confirms the expected parent block + match SortitionsView::check_tenure_change_confirms_parent( + tenure_change, + proposed_block, + &mut self.signer_db, + stacks_client, + self.proposal_config.tenure_last_block_proposal_timeout, + ) { + Ok(true) => {} + Ok(false) => { + return Some( + self.create_block_rejection( + RejectCode::SortitionViewMismatch, + proposed_block, + ), + ) + } + Err(e) => { + warn!("{self}: Error checking block proposal: {e}"; + "signer_sighash" => %signer_signature_hash, + "block_id" => %proposed_block.block_id() + ); + return Some( + self.create_block_rejection(RejectCode::ConnectivityIssues, proposed_block), + ); + } + } + } - match self.signer_db.get_signer_last_accepted_block() { + // Ensure that the block is the last block in the chain of its current tenure. + match self + .signer_db + .get_last_accepted_block(&proposed_block_consensus_hash) + { Ok(Some(last_block_info)) => { if proposed_block.header.chain_length <= last_block_info.block.header.chain_length { - // We do not allow reorgs at any time within the same consensus hash OR of globally accepted blocks - let non_reorgable_block = last_block_info.block.header.consensus_hash - == proposed_block_consensus_hash - || last_block_info.state == BlockState::GloballyAccepted; - // Is the reorg timeout requirement exceeded? - let reorg_timeout_exceeded = last_block_info - .signed_self - .map(|signed_over_time| { - signed_over_time.saturating_add( - self.proposal_config - .tenure_last_block_proposal_timeout - .as_secs(), - ) <= get_epoch_time_secs() - }) - .unwrap_or(false); - if non_reorgable_block || !reorg_timeout_exceeded { - warn!( - "Miner's block proposal does not confirm as many blocks as we expect"; - "proposed_block_consensus_hash" => %proposed_block_consensus_hash, - "proposed_block_signer_sighash" => %signer_signature_hash, - "proposed_chain_length" => proposed_block.header.chain_length, - "expected_at_least" => last_block_info.block.header.chain_length + 1, - ); - return Some(self.create_block_rejection( - RejectCode::SortitionViewMismatch, - proposed_block, - )); - } + warn!( + "Miner's block proposal does not confirm as many blocks as we expect"; + "proposed_block_consensus_hash" => %proposed_block_consensus_hash, + "proposed_block_signer_sighash" => %signer_signature_hash, + "proposed_chain_length" => proposed_block.header.chain_length, + "expected_at_least" => last_block_info.block.header.chain_length + 1, + ); + return Some(self.create_block_rejection( + RejectCode::SortitionViewMismatch, + proposed_block, + )); } - None } - Ok(_) => None, + Ok(_) => {} Err(e) => { warn!("{self}: Failed to check block against signer db: {e}"; "signer_sighash" => %signer_signature_hash, "block_id" => %proposed_block.block_id() ); - Some(self.create_block_rejection(RejectCode::ConnectivityIssues, proposed_block)) + return Some( + self.create_block_rejection(RejectCode::ConnectivityIssues, proposed_block), + ); } } + None } /// Handle the block validate ok response. Returns our block response if we have one @@ -684,7 +704,9 @@ impl Signer { } }; - if let Some(block_response) = self.check_block_against_signer_db_state(&block_info.block) { + if let Some(block_response) = + self.check_block_against_signer_db_state(stacks_client, &block_info.block) + { // The signer db state has changed. We no longer view this block as valid. Override the validation response. if let Err(e) = block_info.mark_locally_rejected() { if !block_info.has_reached_consensus() {