Merge branch 'develop' into test/flaky-miner-forking

Author: kantai

.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/blockstack_cli.rs

@@ -864,7 +864,7 @@ fn main_handler(mut argv: Vec<String>) -> Result<String, CliError> {
         if let Some(custom_chain_id) = flag.split('=').nth(1) {
             // Attempt to parse the custom chain ID from hex
             chain_id = u32::from_str_radix(custom_chain_id.trim_start_matches("0x"), 16)
-                .map_err(|err| CliError::InvalidChainId(err))?;
+                .map_err(CliError::InvalidChainId)?;
         } else {
             // Use the default testnet chain ID
             chain_id = CHAIN_ID_TESTNET;

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/bits.rs

@@ -223,10 +223,7 @@ impl BitcoinTxInputStructured {
                         Instruction::Op(btc_opcodes::OP_CHECKMULTISIG),
                     ) => {
                         // op1 and op2 must be integers
-                        match (
-                            btc_opcodes::from(*op1).classify(),
-                            btc_opcodes::from(*op2).classify(),
-                        ) {
+                        match (op1.classify(), op2.classify()) {
                             (Class::PushNum(num_sigs), Class::PushNum(num_pubkeys)) => {
                                 // the "#instructions - 3" comes from the OP_m, OP_n, and OP_CHECKMULTISIG
                                 if num_sigs < 1

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/db.rs

@@ -393,7 +393,7 @@ impl BurnchainDBTransaction<'_> {
         let args = params![u64_to_sql(target_reward_cycle)?];
         self.sql_tx
             .execute(sql, args)
-            .map_err(|e| DBError::SqliteError(e))?;
+            .map_err(DBError::SqliteError)?;
 
         let sql = "UPDATE block_commit_metadata SET anchor_block = ?1 WHERE burn_block_hash = ?2 AND txid = ?3";
         let args = params![
@@ -424,7 +424,7 @@ impl BurnchainDBTransaction<'_> {
         self.sql_tx
             .execute(sql, args)
             .map(|_| ())
-            .map_err(|e| DBError::SqliteError(e))
+            .map_err(DBError::SqliteError)
     }
 
     /// Calculate a burnchain block's block-commits' descendancy information.