Fix validation queue race condition by ensuring that the sortition view hasn't updated between validation sumbission and approval

Signed-off-by: Jacinta Ferrant <[email protected]>

Author: jferrant

stacks-signer/src/chainstate.rs

@@ -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,

stacks-signer/src/signerdb.rs

@@ -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,
@@ -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> {
@@ -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::*;
@@ -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) {
@@ -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");
@@ -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
@@ -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]
@@ -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");
@@ -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;

stacks-signer/src/tests/chainstate.rs

@@ -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();
 
@@ -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]),
@@ -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();