Merge pull request #5003 from stacks-network/feature/timed-out-block-proposal-handling

[signer] time out an empty sortition and mark a miner as misbehaving if an initial block proposal fails to arrive in time

Author: kantai

.github/workflows/bitcoin-tests.yml

@@ -89,6 +89,7 @@ jobs:
           - tests::signer::v0::end_of_tenure
           - tests::signer::v0::forked_tenure_okay
           - tests::signer::v0::forked_tenure_invalid
+          - tests::signer::v0::empty_sortition
           - tests::signer::v0::bitcoind_forking_test
           - tests::nakamoto_integrations::stack_stx_burn_op_integration_test
           - tests::nakamoto_integrations::check_block_heights

stacks-signer/src/chainstate.rs

@@ -13,7 +13,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-use std::time::Duration;
+use std::time::{Duration, UNIX_EPOCH};
 
 use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
 use blockstack_lib::chainstate::stacks::TenureChangePayload;
@@ -78,18 +78,54 @@ pub struct SortitionState {
     pub burn_block_hash: BurnchainHeaderHash,
 }
 
+impl SortitionState {
+    /// Check if the sortition is timed out (i.e., the miner did not propose a block in time)
+    pub fn is_timed_out(
+        &self,
+        timeout: Duration,
+        signer_db: &SignerDb,
+    ) -> Result<bool, SignerChainstateError> {
+        // if the miner has already been invalidated, we don't need to check if they've timed out.
+        if self.miner_status != SortitionMinerStatus::Valid {
+            return Ok(false);
+        }
+        // if we've already signed a block in this tenure, the miner can't have timed out.
+        let has_blocks = signer_db
+            .get_last_signed_block_in_tenure(&self.consensus_hash)?
+            .is_some();
+        if has_blocks {
+            return Ok(false);
+        }
+        let Some(received_ts) = signer_db.get_burn_block_receive_time(&self.burn_block_hash)?
+        else {
+            return Ok(false);
+        };
+        let received_time = UNIX_EPOCH + Duration::from_secs(received_ts);
+        let Ok(elapsed) = std::time::SystemTime::now().duration_since(received_time) else {
+            return Ok(false);
+        };
+        if elapsed > timeout {
+            return Ok(true);
+        }
+        Ok(false)
+    }
+}
+
 /// Captures the configuration settings used by the signer when evaluating block proposals.
 #[derive(Debug, Clone)]
 pub struct ProposalEvalConfig {
     /// How much time must pass between the first block proposal in a tenure and the next bitcoin block
     ///  before a subsequent miner isn't allowed to reorg the tenure
     pub first_proposal_burn_block_timing: Duration,
+    /// Time between processing a sortition and proposing a block before the block is considered invalid
+    pub block_proposal_timeout: Duration,
 }
 
 impl From<&SignerConfig> for ProposalEvalConfig {
     fn from(value: &SignerConfig) -> Self {
         Self {
-            first_proposal_burn_block_timing: value.first_proposal_burn_block_timing.clone(),
+            first_proposal_burn_block_timing: value.first_proposal_burn_block_timing,
+            block_proposal_timeout: value.block_proposal_timeout,
         }
     }
 }
@@ -147,12 +183,23 @@ impl<'a> ProposedBy<'a> {
 impl SortitionsView {
     /// Apply checks from the SortitionsView on the block proposal.
     pub fn check_proposal(
-        &self,
+        &mut self,
         client: &StacksClient,
         signer_db: &SignerDb,
         block: &NakamotoBlock,
         block_pk: &StacksPublicKey,
     ) -> Result<bool, SignerChainstateError> {
+        if self
+            .cur_sortition
+            .is_timed_out(self.config.block_proposal_timeout, signer_db)?
+        {
+            self.cur_sortition.miner_status = SortitionMinerStatus::InvalidatedBeforeFirstBlock;
+        }
+        if let Some(last_sortition) = self.last_sortition.as_mut() {
+            if last_sortition.is_timed_out(self.config.block_proposal_timeout, signer_db)? {
+                last_sortition.miner_status = SortitionMinerStatus::InvalidatedBeforeFirstBlock;
+            }
+        }
         let bitvec_all_1s = block.header.pox_treatment.iter().all(|entry| entry);
         if !bitvec_all_1s {
             warn!(

stacks-signer/src/client/mod.rs

@@ -565,7 +565,8 @@ pub(crate) mod tests {
             tx_fee_ustx: config.tx_fee_ustx,
             max_tx_fee_ustx: config.max_tx_fee_ustx,
             db_path: config.db_path.clone(),
-            first_proposal_burn_block_timing: Duration::from_secs(30),
+            first_proposal_burn_block_timing: config.first_proposal_burn_block_timing,
+            block_proposal_timeout: config.block_proposal_timeout,
         }
     }
 

stacks-signer/src/config.rs

@@ -36,6 +36,7 @@ use wsts::curve::scalar::Scalar;
 use crate::client::SignerSlotID;
 
 const EVENT_TIMEOUT_MS: u64 = 5000;
+const BLOCK_PROPOSAL_TIMEOUT_MS: u64 = 45_000;
 // Default transaction fee to use in microstacks (if unspecificed in the config file)
 const TX_FEE_USTX: u64 = 10_000;
 
@@ -154,6 +155,8 @@ pub struct SignerConfig {
     /// How much time must pass between the first block proposal in a tenure and the next bitcoin block
     ///  before a subsequent miner isn't allowed to reorg the tenure
     pub first_proposal_burn_block_timing: Duration,
+    /// How much time to wait for a miner to propose a block following a sortition
+    pub block_proposal_timeout: Duration,
 }
 
 /// The parsed configuration for the signer
@@ -196,6 +199,8 @@ pub struct GlobalConfig {
     /// How much time between the first block proposal in a tenure and the next bitcoin block
     ///  must pass before a subsequent miner isn't allowed to reorg the tenure
     pub first_proposal_burn_block_timing: Duration,
+    /// How much time to wait for a miner to propose a block following a sortition
+    pub block_proposal_timeout: Duration,
 }
 
 /// Internal struct for loading up the config file
@@ -236,6 +241,8 @@ struct RawConfigFile {
     /// How much time must pass between the first block proposal in a tenure and the next bitcoin block
     ///  before a subsequent miner isn't allowed to reorg the tenure
     pub first_proposal_burn_block_timing_secs: Option<u64>,
+    /// How much time to wait for a miner to propose a block following a sortition in milliseconds
+    pub block_proposal_timeout_ms: Option<u64>,
 }
 
 impl RawConfigFile {
@@ -324,6 +331,12 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
             None => None,
         };
 
+        let block_proposal_timeout = Duration::from_millis(
+            raw_data
+                .block_proposal_timeout_ms
+                .unwrap_or(BLOCK_PROPOSAL_TIMEOUT_MS),
+        );
+
         Ok(Self {
             node_host: raw_data.node_host,
             endpoint,
@@ -343,6 +356,7 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
             db_path,
             metrics_endpoint,
             first_proposal_burn_block_timing,
+            block_proposal_timeout,
         })
     }
 }

stacks-signer/src/runloop.rs

@@ -270,6 +270,7 @@ impl<Signer: SignerTrait<T>, T: StacksMessageCodec + Clone + Send + Debug> RunLo
             tx_fee_ustx: self.config.tx_fee_ustx,
             max_tx_fee_ustx: self.config.max_tx_fee_ustx,
             db_path: self.config.db_path.clone(),
+            block_proposal_timeout: self.config.block_proposal_timeout,
         })
     }
 

stacks-signer/src/signerdb.rs

@@ -339,7 +339,7 @@ impl SignerDb {
         tenure: &ConsensusHash,
     ) -> Result<Option<BlockInfo>, DBError> {
         let query = "SELECT block_info FROM blocks WHERE consensus_hash = ? AND signed_over = 1 ORDER BY stacks_height ASC LIMIT 1";
-        let result: Option<String> = query_row(&self.db, query, &[tenure])?;
+        let result: Option<String> = query_row(&self.db, query, [tenure])?;
 
         try_deserialize(result)
     }

stacks-signer/src/tests/chainstate.rs

@@ -86,6 +86,7 @@ fn setup_test_environment(
         last_sortition,
         config: ProposalEvalConfig {
             first_proposal_burn_block_timing: Duration::from_secs(30),
+            block_proposal_timeout: Duration::from_secs(5),
         },
     };
 
@@ -110,7 +111,7 @@ fn setup_test_environment(
             parent_block_id: StacksBlockId([0; 32]),
             tx_merkle_root: Sha512Trunc256Sum([0; 32]),
             state_index_root: TrieHash([0; 32]),
-            timestamp: 11,
+            timestamp: 3,
             miner_signature: MessageSignature::empty(),
             signer_signature: vec![],
             pox_treatment: BitVec::ones(1).unwrap(),
@@ -139,7 +140,7 @@ fn check_proposal_units() {
 
 #[test]
 fn check_proposal_miner_pkh_mismatch() {
-    let (stacks_client, signer_db, _block_pk, view, mut block) =
+    let (stacks_client, signer_db, _block_pk, mut view, mut block) =
         setup_test_environment("miner_pkh_mismatch");
     block.header.consensus_hash = view.cur_sortition.consensus_hash;
     let different_block_pk = StacksPublicKey::from_private(&StacksPrivateKey::from_seed(&[2, 3]));
@@ -327,7 +328,7 @@ fn make_tenure_change_tx(payload: TenureChangePayload) -> StacksTransaction {
 
 #[test]
 fn check_proposal_tenure_extend_invalid_conditions() {
-    let (stacks_client, signer_db, block_pk, view, mut block) =
+    let (stacks_client, signer_db, block_pk, mut view, mut block) =
         setup_test_environment("tenure_extend");
     block.header.consensus_hash = view.cur_sortition.consensus_hash;
     let mut extend_payload = make_tenure_change_payload();
@@ -350,3 +351,116 @@ fn check_proposal_tenure_extend_invalid_conditions() {
         .check_proposal(&stacks_client, &signer_db, &block, &block_pk)
         .unwrap());
 }
+
+#[test]
+fn check_block_proposal_timeout() {
+    let (stacks_client, mut signer_db, block_pk, mut view, mut curr_sortition_block) =
+        setup_test_environment("block_proposal_timeout");
+    curr_sortition_block.header.consensus_hash = view.cur_sortition.consensus_hash;
+    let mut last_sortition_block = curr_sortition_block.clone();
+    last_sortition_block.header.consensus_hash =
+        view.last_sortition.as_ref().unwrap().consensus_hash;
+
+    // Ensure we have a burn height to compare against
+    let burn_hash = view.cur_sortition.burn_block_hash;
+    let burn_height = 1;
+    let received_time = SystemTime::now();
+    signer_db
+        .insert_burn_block(&burn_hash, burn_height, &received_time)
+        .unwrap();
+
+    assert!(view
+        .check_proposal(&stacks_client, &signer_db, &curr_sortition_block, &block_pk)
+        .unwrap());
+
+    assert!(!view
+        .check_proposal(&stacks_client, &signer_db, &last_sortition_block, &block_pk)
+        .unwrap());
+
+    // Sleep a bit to time out the block proposal
+    std::thread::sleep(Duration::from_secs(5));
+    assert!(!view
+        .check_proposal(&stacks_client, &signer_db, &curr_sortition_block, &block_pk)
+        .unwrap());
+
+    assert!(view
+        .check_proposal(&stacks_client, &signer_db, &last_sortition_block, &block_pk)
+        .unwrap());
+}
+
+#[test]
+fn check_sortition_timeout() {
+    let signer_db_dir = "/tmp/stacks-node-tests/signer-units/";
+    let signer_db_path = format!(
+        "{signer_db_dir}/sortition_timeout.{}.sqlite",
+        get_epoch_time_secs()
+    );
+    fs::create_dir_all(signer_db_dir).unwrap();
+    let mut signer_db = SignerDb::new(signer_db_path).unwrap();
+
+    let mut sortition = SortitionState {
+        miner_pkh: Hash160([0; 20]),
+        miner_pubkey: None,
+        prior_sortition: ConsensusHash([0; 20]),
+        parent_tenure_id: ConsensusHash([0; 20]),
+        consensus_hash: ConsensusHash([1; 20]),
+        miner_status: SortitionMinerStatus::Valid,
+        burn_header_timestamp: 2,
+        burn_block_hash: BurnchainHeaderHash([1; 32]),
+    };
+    // Ensure we have a burn height to compare against
+    let burn_hash = sortition.burn_block_hash;
+    let burn_height = 1;
+    let received_time = SystemTime::now();
+    signer_db
+        .insert_burn_block(&burn_hash, burn_height, &received_time)
+        .unwrap();
+
+    std::thread::sleep(Duration::from_secs(1));
+    // We have not yet timed out
+    assert!(!sortition
+        .is_timed_out(Duration::from_secs(10), &signer_db)
+        .unwrap());
+    // We are a valid sortition, have an empty tenure, and have now timed out
+    assert!(sortition
+        .is_timed_out(Duration::from_secs(1), &signer_db)
+        .unwrap());
+    // This will not be marked as timed out as the status is no longer valid
+    sortition.miner_status = SortitionMinerStatus::InvalidatedAfterFirstBlock;
+    assert!(!sortition
+        .is_timed_out(Duration::from_secs(1), &signer_db)
+        .unwrap());
+
+    // Revert the status to continue other checks
+    sortition.miner_status = SortitionMinerStatus::Valid;
+    // Insert a signed over block so its no longer an empty tenure
+    let block_proposal = BlockProposal {
+        block: NakamotoBlock {
+            header: NakamotoBlockHeader {
+                version: 1,
+                chain_length: 10,
+                burn_spent: 10,
+                consensus_hash: sortition.consensus_hash,
+                parent_block_id: StacksBlockId([0; 32]),
+                tx_merkle_root: Sha512Trunc256Sum([0; 32]),
+                state_index_root: TrieHash([0; 32]),
+                timestamp: 11,
+                miner_signature: MessageSignature::empty(),
+                signer_signature: vec![],
+                pox_treatment: BitVec::ones(1).unwrap(),
+            },
+            txs: vec![],
+        },
+        burn_height: 2,
+        reward_cycle: 1,
+    };
+
+    let mut block_info = BlockInfo::from(block_proposal);
+    block_info.signed_over = true;
+    signer_db.insert_block(&block_info).unwrap();
+
+    // This will no longer be timed out as we have a non-empty tenure
+    assert!(!sortition
+        .is_timed_out(Duration::from_secs(1), &signer_db)
+        .unwrap());
+}

testnet/stacks-node/src/tests/nakamoto_integrations.rs

@@ -4982,8 +4982,10 @@ fn signer_chainstate() {
         // this config disallows any reorg due to poorly timed block commits
         let proposal_conf = ProposalEvalConfig {
             first_proposal_burn_block_timing: Duration::from_secs(0),
+            block_proposal_timeout: Duration::from_secs(100),
         };
-        let sortitions_view = SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap();
+        let mut sortitions_view =
+            SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap();
 
         // check the prior tenure's proposals again, confirming that the sortitions_view
         //  will reject them.
@@ -5095,8 +5097,10 @@ fn signer_chainstate() {
         // this config disallows any reorg due to poorly timed block commits
         let proposal_conf = ProposalEvalConfig {
             first_proposal_burn_block_timing: Duration::from_secs(0),
+            block_proposal_timeout: Duration::from_secs(100),
         };
-        let sortitions_view = SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap();
+        let mut sortitions_view =
+            SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap();
         let valid = sortitions_view
             .check_proposal(
                 &signer_client,
@@ -5161,6 +5165,7 @@ fn signer_chainstate() {
     // this config disallows any reorg due to poorly timed block commits
     let proposal_conf = ProposalEvalConfig {
         first_proposal_burn_block_timing: Duration::from_secs(0),
+        block_proposal_timeout: Duration::from_secs(100),
     };
     let mut sortitions_view = SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap();