Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix validation queue race condition in block approval vs validaiton submission #5497

Merged
merged 13 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stacks-signer/src/chainstate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ impl SortitionsView {

/// Get the last block from the given tenure
/// Returns the last locally accepted block if it is not timed out, otherwise it will return the last globally accepted block.
fn get_tenure_last_block_info(
pub fn get_tenure_last_block_info(
consensus_hash: &ConsensusHash,
signer_db: &SignerDb,
tenure_last_block_proposal_timeout: Duration,
Expand Down
45 changes: 33 additions & 12 deletions stacks-signer/src/signerdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use blockstack_lib::util_lib::db::{
Error as DBError,
};
use clarity::types::chainstate::{BurnchainHeaderHash, StacksAddress};
use clarity::util::secp256k1::Secp256k1PublicKey;
use libsigner::BlockProposal;
use rusqlite::{
params, Connection, Error as SqliteError, OpenFlags, OptionalExtension, Transaction,
Expand Down Expand Up @@ -157,28 +158,31 @@ pub struct BlockInfo {
pub signed_group: Option<u64>,
/// The block state relative to the signer's view of the stacks blockchain
pub state: BlockState,
/// The miner pubkey that proposed this block
pub miner_pubkey: Secp256k1PublicKey,
/// Extra data specific to v0, v1, etc.
pub ext: ExtraBlockInfo,
}

impl From<BlockProposal> for BlockInfo {
fn from(value: BlockProposal) -> Self {
impl BlockInfo {
/// Create a new block info from the provided proposal and corresponding miner pubkey
pub fn new(block_proposal: BlockProposal, miner_pubkey: Secp256k1PublicKey) -> Self {
Self {
block: value.block,
burn_block_height: value.burn_height,
reward_cycle: value.reward_cycle,
block: block_proposal.block,
burn_block_height: block_proposal.burn_height,
reward_cycle: block_proposal.reward_cycle,
vote: None,
valid: None,
signed_over: false,
proposed_time: get_epoch_time_secs(),
signed_self: None,
signed_group: None,
ext: ExtraBlockInfo::default(),
miner_pubkey,
state: BlockState::Unprocessed,
}
}
}
impl BlockInfo {

/// Mark this block as locally accepted, valid, signed over, and records either the self or group signed timestamp in the block info if it wasn't
/// already set.
pub fn mark_locally_accepted(&mut self, group_signed: bool) -> Result<(), String> {
Expand Down Expand Up @@ -853,7 +857,7 @@ mod tests {
use std::path::PathBuf;

use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader};
use clarity::util::secp256k1::MessageSignature;
use clarity::util::secp256k1::{MessageSignature, Secp256k1PrivateKey};
use libsigner::BlockProposal;

use super::*;
Expand All @@ -879,7 +883,13 @@ mod tests {
reward_cycle: 42,
};
overrides(&mut block_proposal);
(BlockInfo::from(block_proposal.clone()), block_proposal)
(
BlockInfo::new(
block_proposal.clone(),
Secp256k1PublicKey::from_private(&Secp256k1PrivateKey::new()),
),
block_proposal,
)
}

fn create_block() -> (BlockInfo, BlockProposal) {
Expand All @@ -896,6 +906,7 @@ mod tests {
fn test_basic_signer_db_with_path(db_path: impl AsRef<Path>) {
let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
let (block_info, block_proposal) = create_block();
let miner_pubkey = block_info.miner_pubkey;
let reward_cycle = block_info.reward_cycle;
db.insert_block(&block_info)
.expect("Unable to insert block into db");
Expand All @@ -907,7 +918,10 @@ mod tests {
.unwrap()
.expect("Unable to get block from db");

assert_eq!(BlockInfo::from(block_proposal.clone()), block_info);
assert_eq!(
BlockInfo::new(block_proposal.clone(), miner_pubkey),
block_info
);

// Test looking up a block from a different reward cycle
let block_info = db
Expand All @@ -927,7 +941,10 @@ mod tests {
.unwrap()
.expect("Unable to get block state from db");

assert_eq!(block_state, BlockInfo::from(block_proposal.clone()).state);
assert_eq!(
block_state,
BlockInfo::new(block_proposal.clone(), miner_pubkey).state
);
}

#[test]
Expand All @@ -947,6 +964,7 @@ mod tests {
let db_path = tmp_db_path();
let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
let (block_info, block_proposal) = create_block();
let miner_pubkey = block_info.miner_pubkey;
let reward_cycle = block_info.reward_cycle;
db.insert_block(&block_info)
.expect("Unable to insert block into db");
Expand All @@ -959,7 +977,10 @@ mod tests {
.unwrap()
.expect("Unable to get block from db");

assert_eq!(BlockInfo::from(block_proposal.clone()), block_info);
assert_eq!(
BlockInfo::new(block_proposal.clone(), miner_pubkey),
block_info
);

let old_block_info = block_info;
let old_block_proposal = block_proposal;
Expand Down
10 changes: 7 additions & 3 deletions stacks-signer/src/tests/chainstate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ fn reorg_timing_testing(
reward_cycle: 1,
};
let mut header_clone = block_proposal_1.block.header.clone();
let mut block_info_1 = BlockInfo::from(block_proposal_1);
let mut block_info_1 = BlockInfo::new(block_proposal_1, block_pk);
block_info_1.mark_locally_accepted(false).unwrap();
signer_db.insert_block(&block_info_1).unwrap();

Expand Down Expand Up @@ -457,8 +457,12 @@ fn check_sortition_timeout() {
fs::create_dir_all(signer_db_dir).unwrap();
let mut signer_db = SignerDb::new(signer_db_path).unwrap();

let block_sk = StacksPrivateKey::from_seed(&[0, 1]);
let block_pk = StacksPublicKey::from_private(&block_sk);
let block_pkh = Hash160::from_node_public_key(&block_pk);

let mut sortition = SortitionState {
miner_pkh: Hash160([0; 20]),
miner_pkh: block_pkh,
miner_pubkey: None,
prior_sortition: ConsensusHash([0; 20]),
parent_tenure_id: ConsensusHash([0; 20]),
Expand Down Expand Up @@ -514,7 +518,7 @@ fn check_sortition_timeout() {
reward_cycle: 1,
};

let mut block_info = BlockInfo::from(block_proposal);
let mut block_info = BlockInfo::new(block_proposal, block_pk);
block_info.signed_over = true;
signer_db.insert_block(&block_info).unwrap();

Expand Down
Loading