Merge branch 'develop' into fix/signers-verify-reward-cycle

Author: kantai

CHANGELOG.md

@@ -10,8 +10,10 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 ### Added
 
 - Add `tenure_timeout_secs` to the miner for determining when a time-based tenure extend should be attempted.
+- Added configuration option `block_proposal_max_age_secs` under `[connection_options]` to prevent processing stale block proposals
 
 ### Changed
+- The RPC endpoint `/v3/block_proposal` no longer will evaluate block proposals more than `block_proposal_max_age_secs` old
 
 - When a transaction is dropped due to replace-by-fee, the `/drop_mempool_tx` event observer payload now includes `new_txid`, which is the transaction that replaced this dropped transaction. When a transaction is dropped for other reasons, `new_txid` is `null`. [#5381](https://github.com/stacks-network/stacks-core/pull/5381)
 - Nodes will assume that all PoX anchor blocks exist by default, and stall initial block download indefinitely to await their arrival (#5502)

sample/conf/testnet-follower-conf.toml

@@ -77,8 +77,8 @@ start_height = 6
 
 [[burnchain.epochs]]
 epoch_name = "3.0"
-start_height = 56_457
+start_height = 1900
 
 [[burnchain.epochs]]
 epoch_name = "3.1"
-start_height = 77_770
+start_height = 2000

sample/conf/testnet-miner-conf.toml

@@ -73,8 +73,8 @@ start_height = 6
 
 [[burnchain.epochs]]
 epoch_name = "3.0"
-start_height = 56_457
+start_height = 1900
 
 [[burnchain.epochs]]
 epoch_name = "3.1"
-start_height = 77_770
+start_height = 2000

sample/conf/testnet-signer.toml

@@ -75,4 +75,8 @@ start_height = 6
 
 [[burnchain.epochs]]
 epoch_name = "3.0"
-start_height = 56_457
+start_height = 1900
+
+[[burnchain.epochs]]
+epoch_name = "3.1"
+start_height = 2000

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,

stacks-signer/src/config.rs

@@ -21,6 +21,7 @@ use std::path::PathBuf;
 use std::time::Duration;
 
 use blockstack_lib::chainstate::stacks::TransactionVersion;
+use blockstack_lib::net::connection::DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS;
 use clarity::util::hash::to_hex;
 use libsigner::SignerEntries;
 use serde::Deserialize;
@@ -39,7 +40,6 @@ const BLOCK_PROPOSAL_VALIDATION_TIMEOUT_MS: u64 = 120_000;
 const DEFAULT_FIRST_PROPOSAL_BURN_BLOCK_TIMING_SECS: u64 = 60;
 const DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS: u64 = 30;
 const TENURE_IDLE_TIMEOUT_SECS: u64 = 300;
-const DEFAULT_BLOCK_PROPOSAL_MAX_AGE_SECS: u64 = 600;
 
 #[derive(thiserror::Error, Debug)]
 /// An error occurred parsing the provided configuration

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<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,
@@ -1746,69 +1734,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
-        );
-    }
 }

stacks-signer/src/v0/signer.rs

@@ -589,60 +589,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
@@ -674,7 +694,9 @@ impl Signer {
             return None;
         }
 
-        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() {

stackslib/src/burnchains/bitcoin/blocks.rs

@@ -150,9 +150,7 @@ impl BitcoinMessageHandler for BitcoinBlockDownloader {
             None => panic!("No block header set"),
             Some(ref ipc_header) => {
                 let block_hash = ipc_header.block_header.header.bitcoin_hash().clone();
-                indexer
-                    .send_getdata(&vec![block_hash])
-                    .and_then(|_r| Ok(true))
+                indexer.send_getdata(&vec![block_hash]).map(|_r| true)
             }
         }
     }

stackslib/src/burnchains/bitcoin/indexer.rs

@@ -458,7 +458,7 @@ impl BitcoinIndexer {
         }
         spv_client
             .run(self)
-            .and_then(|_r| Ok(spv_client.end_block_height.unwrap()))
+            .map(|_r| spv_client.end_block_height.unwrap())
     }
 
     #[cfg(test)]