Skip to content

Commit

Permalink
Merge pull request #5674 from stacks-network/fix/bitcoind-forking-tes…
Browse files Browse the repository at this point in the history 
…t-signerdb-state

Fix/bitcoind forking test signerdb state
  • Loading branch information
@kantai
kantai authored Jan 9, 2025
2 parents a70b3d5 + 942b434 commit 9db2f80
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 113 deletions.
2 changes: 1 addition & 1 deletion stacks-signer/src/chainstate.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
77 changes: 0 additions & 77 deletions stacks-signer/src/signerdb.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<BlockInfo>, 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<String> = 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,
Expand Down Expand Up @@ -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
);
}
}
92 changes: 57 additions & 35 deletions stacks-signer/src/v0/signer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<BlockResponse> {
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
Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 9db2f80

Please sign in to comment.