Merge branch 'develop' into fix/clippy-ci-needless-borrowed-ref

Author: jferrant

.github/workflows/bitcoin-tests.yml

@@ -54,6 +54,7 @@ jobs:
           # - tests::neon_integrations::size_overflow_unconfirmed_microblocks_integration_test
           # - tests::neon_integrations::size_overflow_unconfirmed_stream_microblocks_integration_test
           # - tests::neon_integrations::runtime_overflow_unconfirmed_microblocks_integration_test
+          # - tests::epoch_25::microblocks_disabled
           # Disable this flaky test. Microblocks are no longer supported anyways.
           # - tests::neon_integrations::microblock_large_tx_integration_test_FLAKY
           - tests::neon_integrations::miner_submit_twice
@@ -80,7 +81,6 @@ jobs:
           - tests::neon_integrations::bitcoin_reorg_flap
           - tests::neon_integrations::bitcoin_reorg_flap_with_follower
           - tests::neon_integrations::start_stop_bitcoind
-          - tests::epoch_25::microblocks_disabled
           - tests::should_succeed_handling_malformed_and_valid_txs
           - tests::nakamoto_integrations::simple_neon_integration
           - tests::nakamoto_integrations::flash_blocks_on_epoch_3
@@ -141,6 +141,8 @@ jobs:
           - tests::signer::v0::incoming_signers_ignore_block_proposals
           - tests::signer::v0::outgoing_signers_ignore_block_proposals
           - tests::signer::v0::injected_signatures_are_ignored_across_boundaries
+          - tests::signer::v0::block_proposal_timeout
+          - tests::signer::v0::rejected_blocks_count_towards_miner_validity
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state

stacks-signer/src/chainstate.rs

@@ -89,10 +89,8 @@ impl SortitionState {
         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 we've already seen a proposed block from this miner. It cannot have timed out.
+        let has_blocks = signer_db.has_proposed_block_in_tenure(&self.consensus_hash)?;
         if has_blocks {
             return Ok(false);
         }

stacks-signer/src/config.rs

@@ -39,7 +39,7 @@ const BLOCK_PROPOSAL_TIMEOUT_MS: u64 = 600_000;
 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 TENURE_IDLE_TIMEOUT_SECS: u64 = 120;
 
 #[derive(thiserror::Error, Debug)]
 /// An error occurred parsing the provided configuration

stacks-signer/src/signerdb.rs

@@ -746,15 +746,13 @@ impl SignerDb {
         try_deserialize(result)
     }
 
-    /// Return the last signed block in a tenure (identified by its consensus hash)
-    pub fn get_last_signed_block_in_tenure(
-        &self,
-        tenure: &ConsensusHash,
-    ) -> Result<Option<BlockInfo>, DBError> {
-        let query = "SELECT block_info FROM blocks WHERE consensus_hash = ? AND signed_over = 1 ORDER BY stacks_height DESC LIMIT 1";
+    /// Return whether a block proposal has been stored for a tenure (identified by its consensus hash)
+    /// Does not consider the block's state.
+    pub fn has_proposed_block_in_tenure(&self, tenure: &ConsensusHash) -> Result<bool, DBError> {
+        let query = "SELECT block_info FROM blocks WHERE consensus_hash = ? LIMIT 1";
         let result: Option<String> = query_row(&self.db, query, [tenure])?;
 
-        try_deserialize(result)
+        Ok(result.is_some())
     }
 
     /// Return the first signed block in a tenure (identified by its consensus hash)
@@ -1904,4 +1902,28 @@ mod tests {
         let pendings = db.get_all_pending_block_validations().unwrap();
         assert_eq!(pendings.len(), 0);
     }
+
+    #[test]
+    fn has_proposed_block() {
+        let db_path = tmp_db_path();
+        let consensus_hash_1 = ConsensusHash([0x01; 20]);
+        let consensus_hash_2 = ConsensusHash([0x02; 20]);
+        let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+        let (mut block_info, _) = create_block_override(|b| {
+            b.block.header.consensus_hash = consensus_hash_1;
+            b.block.header.chain_length = 1;
+        });
+
+        assert!(!db.has_proposed_block_in_tenure(&consensus_hash_1).unwrap());
+        assert!(!db.has_proposed_block_in_tenure(&consensus_hash_2).unwrap());
+
+        db.insert_block(&block_info).unwrap();
+
+        block_info.block.header.chain_length = 2;
+
+        db.insert_block(&block_info).unwrap();
+
+        assert!(db.has_proposed_block_in_tenure(&consensus_hash_1).unwrap());
+        assert!(!db.has_proposed_block_in_tenure(&consensus_hash_2).unwrap());
+    }
 }

stackslib/src/burnchains/bitcoin/address.rs

@@ -591,10 +591,10 @@ impl BitcoinAddress {
         } else {
             BitcoinNetworkType::Testnet
         };
-        if let Some(addr) = BitcoinAddress::from_scriptpubkey(network_id, scriptpubkey) {
-            if let BitcoinAddress::Segwit(sw) = addr {
-                return Some(BitcoinAddress::Segwit(sw));
-            }
+        if let Some(BitcoinAddress::Segwit(sw)) =
+            BitcoinAddress::from_scriptpubkey(network_id, scriptpubkey)
+        {
+            return Some(BitcoinAddress::Segwit(sw));
         }
         return None;
     }

stackslib/src/burnchains/bitcoin/indexer.rs

@@ -503,13 +503,11 @@ impl BitcoinIndexer {
         start_block: u64,
         remove_old: bool,
     ) -> Result<SpvClient, btc_error> {
-        if remove_old {
-            if PathBuf::from(&reorg_headers_path).exists() {
-                fs::remove_file(&reorg_headers_path).map_err(|e| {
-                    error!("Failed to remove {}", reorg_headers_path);
-                    btc_error::Io(e)
-                })?;
-            }
+        if remove_old && PathBuf::from(&reorg_headers_path).exists() {
+            fs::remove_file(&reorg_headers_path).map_err(|e| {
+                error!("Failed to remove {}", reorg_headers_path);
+                btc_error::Io(e)
+            })?;
         }
 
         // bootstrap reorg client

stackslib/src/burnchains/bitcoin/spv.rs

@@ -328,13 +328,11 @@ impl SpvClient {
             } else {
                 return Err(btc_error::DBError(db_error::NoDBError));
             }
-        } else {
+        } else if readwrite {
             // can just open
-            if readwrite {
-                OpenFlags::SQLITE_OPEN_READ_WRITE
-            } else {
-                OpenFlags::SQLITE_OPEN_READ_ONLY
-            }
+            OpenFlags::SQLITE_OPEN_READ_WRITE
+        } else {
+            OpenFlags::SQLITE_OPEN_READ_ONLY
         };
 
         let mut conn = sqlite_open(headers_path, open_flags, false)

stackslib/src/burnchains/tests/affirmation.rs

@@ -351,29 +351,27 @@ pub fn make_reward_cycle_with_vote(
                 let append = if !burnchain.is_in_prepare_phase(block_commit.block_height) {
                     // non-prepare-phase commits always confirm their parent
                     true
+                } else if confirm_anchor_block {
+                    // all block-commits confirm anchor block
+                    true
                 } else {
-                    if confirm_anchor_block {
-                        // all block-commits confirm anchor block
+                    // fewer than anchor_threshold commits confirm anchor block
+                    let next_rc_start = burnchain.reward_cycle_to_block_height(
+                        burnchain
+                            .block_height_to_reward_cycle(block_commit.block_height)
+                            .unwrap()
+                            + 1,
+                    );
+                    if block_commit.block_height
+                        + (burnchain.pox_constants.anchor_threshold as u64)
+                        + 1
+                        < next_rc_start
+                    {
+                        // in first half of prepare phase, so confirm
                         true
                     } else {
-                        // fewer than anchor_threshold commits confirm anchor block
-                        let next_rc_start = burnchain.reward_cycle_to_block_height(
-                            burnchain
-                                .block_height_to_reward_cycle(block_commit.block_height)
-                                .unwrap()
-                                + 1,
-                        );
-                        if block_commit.block_height
-                            + (burnchain.pox_constants.anchor_threshold as u64)
-                            + 1
-                            < next_rc_start
-                        {
-                            // in first half of prepare phase, so confirm
-                            true
-                        } else {
-                            // in second half of prepare phase, so don't confirm
-                            false
-                        }
+                        // in second half of prepare phase, so don't confirm
+                        false
                     }
                 };
 

stackslib/src/chainstate/burn/db/sortdb.rs

@@ -1849,21 +1849,19 @@ impl SortitionHandleTx<'_> {
                     true
                 } else if cur_height > stacks_block_height {
                     false
+                } else if &cur_ch == consensus_hash {
+                    // same sortition (i.e. nakamoto block)
+                    // no replacement
+                    false
                 } else {
-                    if &cur_ch == consensus_hash {
-                        // same sortition (i.e. nakamoto block)
-                        // no replacement
-                        false
-                    } else {
-                        // tips come from different sortitions
-                        // break ties by going with the latter-signed block
-                        let sn_current = SortitionDB::get_block_snapshot_consensus(self, &cur_ch)?
+                    // tips come from different sortitions
+                    // break ties by going with the latter-signed block
+                    let sn_current = SortitionDB::get_block_snapshot_consensus(self, &cur_ch)?
+                        .ok_or(db_error::NotFoundError)?;
+                    let sn_accepted =
+                        SortitionDB::get_block_snapshot_consensus(self, &consensus_hash)?
                             .ok_or(db_error::NotFoundError)?;
-                        let sn_accepted =
-                            SortitionDB::get_block_snapshot_consensus(self, &consensus_hash)?
-                                .ok_or(db_error::NotFoundError)?;
-                        sn_current.block_height < sn_accepted.block_height
-                    }
+                    sn_current.block_height < sn_accepted.block_height
                 };
 
                 debug!("Setting Stacks tip as accepted";
@@ -5774,10 +5772,9 @@ impl SortitionHandleTx<'_> {
             .map(|parent_commit_sn| parent_commit_sn.sortition_id)
             .unwrap_or(SortitionId([0x00; 32]));
 
-        if !cfg!(test) {
-            if block_commit.parent_block_ptr != 0 || block_commit.parent_vtxindex != 0 {
-                assert!(parent_sortition_id != SortitionId([0x00; 32]));
-            }
+        if !cfg!(test) && (block_commit.parent_block_ptr != 0 || block_commit.parent_vtxindex != 0)
+        {
+            assert!(parent_sortition_id != SortitionId([0x00; 32]));
         }
 
         let args = params![

stackslib/src/chainstate/burn/operations/leader_block_commit.rs

@@ -313,13 +313,11 @@ impl LeaderBlockCommitOp {
         })?;
 
         // basic sanity checks
-        if data.parent_block_ptr == 0 {
-            if data.parent_vtxindex != 0 {
-                warn!("Invalid tx: parent block back-pointer must be positive");
-                return Err(op_error::ParseError);
-            }
-            // if parent block ptr and parent vtxindex are both 0, then this block's parent is
-            // the genesis block.
+        // if parent block ptr and parent vtxindex are both 0, then this block's parent is
+        // the genesis block.
+        if data.parent_block_ptr == 0 && data.parent_vtxindex != 0 {
+            warn!("Invalid tx: parent block back-pointer must be positive");
+            return Err(op_error::ParseError);
         }
 
         if u64::from(data.parent_block_ptr) >= block_height {