Merge branch 'develop' into long-block-proposal-timeout

Author: kantai

.github/workflows/bitcoin-tests.yml

@@ -83,7 +83,8 @@ jobs:
           - tests::neon_integrations::start_stop_bitcoind
           - tests::should_succeed_handling_malformed_and_valid_txs
           - tests::nakamoto_integrations::simple_neon_integration
-          - tests::nakamoto_integrations::flash_blocks_on_epoch_3
+          # Disable this flaky test. We don't need continue testing Epoch 2 -> 3 transition
+          # - tests::nakamoto_integrations::flash_blocks_on_epoch_3_FLAKY
           - tests::nakamoto_integrations::mine_multiple_per_tenure_integration
           - tests::nakamoto_integrations::block_proposal_api_endpoint
           - tests::nakamoto_integrations::miner_writes_proposed_block_to_stackerdb
@@ -134,6 +135,7 @@ jobs:
           - tests::signer::v0::block_commit_delay
           - tests::signer::v0::continue_after_fast_block_no_sortition
           - tests::signer::v0::block_validation_response_timeout
+          - tests::signer::v0::block_validation_check_rejection_timeout_heuristic
           - tests::signer::v0::block_validation_pending_table
           - tests::signer::v0::new_tenure_while_validating_previous_scenario
           - tests::signer::v0::tenure_extend_after_bad_commit

CHANGELOG.md

@@ -8,15 +8,22 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 ## [Unreleased]
 
 ### Added
-- Add `dry_run` configuration option to `stacks-signer` config toml. Dry run mode will
-  run the signer binary as if it were a registered signer. Instead of broadcasting
-  `StackerDB` messages, it logs `INFO` messages. Other interactions with the `stacks-node`
-  behave normally (e.g., submitting validation requests, submitting finished blocks). A
-  dry run signer will error out if the supplied key is actually a registered signer.
+
+### Changed
+
+### Fixed
+
+## [3.1.0.0.5]
+
+### Added
+
+- Add miner configuration option `tenure_extend_cost_threshold` to specify the percentage of the tenure budget that must be spent before a time-based tenure extend is attempted
 
 ### Changed
 
 - Miner will include other transactions in blocks with tenure extend transactions (#5760)
+- Add `block_rejection_timeout_steps` to miner configuration for defining rejections-based timeouts while waiting for signers response (#5705)
+- Miner will not issue a tenure extend until at least half of the block budget has been spent (#5757)
 
 ### Fixed
 

stacks-signer/CHANGELOG.md

@@ -11,46 +11,56 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 - Increase default `block_proposal_timeout_ms` from 10 minutes to 4 hours. Until #5729 is implemented, there is no value in rejecting a late block from a miner, since a late block is better than no block at all.
 
+## [3.1.0.0.5.0]
+
+### Added
+
+- Add `dry_run` configuration option to `stacks-signer` config toml. Dry run mode will
+  run the signer binary as if it were a registered signer. Instead of broadcasting
+  `StackerDB` messages, it logs `INFO` messages. Other interactions with the `stacks-node`
+  behave normally (e.g., submitting validation requests, submitting finished blocks). A
+  dry run signer will error out if the supplied key is actually a registered signer.
+
 ## [3.1.0.0.4.0]
 
-## Added
+### Added
 
 - When a new block proposal is received while the signer is waiting for an existing proposal to be validated, the signer will wait until the existing block is done validating before submitting the new one for validating. ([#5453](https://github.com/stacks-network/stacks-core/pull/5453))
 - Introduced two new prometheus metrics:
   - `stacks_signer_block_validation_latencies_histogram`: the validation_time_ms reported by the node when validating a block proposal
   - `stacks_signer_block_response_latencies_histogram`: the "end-to-end" time it takes for the signer to issue a block response
 
-## Changed
+### Changed
 
 ## [3.1.0.0.3.0]
 
-## Added
+### Added
 
 - Introduced the `block_proposal_max_age_secs` configuration option for signers, enabling them to automatically ignore block proposals that exceed the specified age in seconds.
 
-## Changed
+### Changed
 - Improvements to the stale signer cleanup logic: deletes the prior signer if it has no remaining unprocessed blocks in its database
 - Signers now listen to new block events from the stacks node to determine whether a block has been successfully appended to the chain tip
 
-# [3.1.0.0.2.1]
+## [3.1.0.0.2.1]
 
-## Added
+### Added
 
-## Changed
+### Changed
 
 - Prevent old reward cycle signers from processing block validation response messages that do not apply to blocks from their cycle.
 
-# [3.1.0.0.2.1]
+## [3.1.0.0.2.1]
 
-## Added
+### Added
 
-## Changed
+### Changed
 
 - Prevent old reward cycle signers from processing block validation response messages that do not apply to blocks from their cycle.
 
 ## [3.1.0.0.2.0]
 
-## Added
+### Added
 
 - **SIP-029 consensus rules, activating in epoch 3.1 at block 875,000** (see [SIP-029](https://github.com/will-corcoran/sips/blob/feat/sip-029-halving-alignment/sips/sip-029/sip-029-halving-alignment.md) for details)
 

stackslib/src/burnchains/bitcoin/indexer.rs

@@ -265,40 +265,31 @@ impl BitcoinIndexer {
             Ok(s) => {
                 // Disable Nagle algorithm
                 s.set_nodelay(true).map_err(|_e| {
-                    test_debug!("Failed to set TCP_NODELAY: {:?}", &_e);
+                    test_debug!("Failed to set TCP_NODELAY: {_e:?}");
                     btc_error::ConnectionError
                 })?;
 
                 // set timeout
                 s.set_read_timeout(Some(Duration::from_secs(self.runtime.timeout)))
                     .map_err(|_e| {
-                        test_debug!("Failed to set TCP read timeout: {:?}", &_e);
+                        test_debug!("Failed to set TCP read timeout: {_e:?}");
                         btc_error::ConnectionError
                     })?;
 
                 s.set_write_timeout(Some(Duration::from_secs(self.runtime.timeout)))
                     .map_err(|_e| {
-                        test_debug!("Failed to set TCP write timeout: {:?}", &_e);
+                        test_debug!("Failed to set TCP write timeout: {_e:?}");
                         btc_error::ConnectionError
                     })?;
 
-                match self.runtime.sock.take() {
-                    Some(s) => {
-                        let _ = s.shutdown(Shutdown::Both);
-                    }
-                    None => {}
+                if let Some(s_old) = self.runtime.sock.replace(s) {
+                    let _ = s_old.shutdown(Shutdown::Both);
                 }
-
-                self.runtime.sock = Some(s);
                 Ok(())
             }
             Err(_e) => {
-                let s = self.runtime.sock.take();
-                match s {
-                    Some(s) => {
-                        let _ = s.shutdown(Shutdown::Both);
-                    }
-                    None => {}
+                if let Some(s) = self.runtime.sock.take() {
+                    let _ = s.shutdown(Shutdown::Both);
                 }
                 Err(btc_error::ConnectionError)
             }
@@ -926,11 +917,8 @@ impl BitcoinIndexer {
 
 impl Drop for BitcoinIndexer {
     fn drop(&mut self) {
-        match self.runtime.sock {
-            Some(ref mut s) => {
-                let _ = s.shutdown(Shutdown::Both);
-            }
-            None => {}
+        if let Some(ref mut s) = self.runtime.sock {
+            let _ = s.shutdown(Shutdown::Both);
         }
     }
 }

stackslib/src/burnchains/db.rs

@@ -1193,9 +1193,10 @@ impl BurnchainDB {
         let ops: Vec<BlockstackOperationType> =
             query_rows(&self.conn, qry, args).expect("FATAL: burnchain DB query error");
         for op in ops {
-            if let Some(_) = indexer
+            if indexer
                 .find_burnchain_header_height(&op.burn_header_hash())
                 .expect("FATAL: burnchain DB query error")
+                .is_some()
             {
                 // this is the op on the canonical fork
                 return Some(op);

stackslib/src/burnchains/tests/mod.rs

@@ -577,13 +577,10 @@ impl TestBurnchainBlock {
     pub fn patch_from_chain_tip(&mut self, parent_snapshot: &BlockSnapshot) {
         assert_eq!(parent_snapshot.block_height + 1, self.block_height);
 
-        for i in 0..self.txs.len() {
-            match self.txs[i] {
-                BlockstackOperationType::LeaderKeyRegister(ref mut data) => {
-                    assert_eq!(data.block_height, self.block_height);
-                    data.consensus_hash = parent_snapshot.consensus_hash.clone();
-                }
-                _ => {}
+        for tx in self.txs.iter_mut() {
+            if let BlockstackOperationType::LeaderKeyRegister(ref mut data) = tx {
+                assert_eq!(data.block_height, self.block_height);
+                data.consensus_hash = parent_snapshot.consensus_hash.clone();
             }
         }
     }

stackslib/src/chainstate/nakamoto/miner.rs

@@ -150,6 +150,21 @@ pub struct MinerTenureInfo<'a> {
     pub tenure_block_commit_opt: Option<LeaderBlockCommitOp>,
 }
 
+/// Structure returned from `NakamotoBlockBuilder::build_nakamoto_block` with
+/// information about the block that was built.
+pub struct BlockMetadata {
+    /// The block that was built
+    pub block: NakamotoBlock,
+    /// The execution cost consumed so far by the current tenure
+    pub tenure_consumed: ExecutionCost,
+    /// The cost budget for the current tenure
+    pub tenure_budget: ExecutionCost,
+    /// The size of the blocks in the current tenure in bytes
+    pub tenure_size: u64,
+    /// The events emitted by the transactions included in this block
+    pub tx_events: Vec<TransactionEvent>,
+}
+
 impl NakamotoBlockBuilder {
     /// Make a block builder from genesis (testing only)
     pub fn new_first_block(
@@ -526,7 +541,7 @@ impl NakamotoBlockBuilder {
         settings: BlockBuilderSettings,
         event_observer: Option<&dyn MemPoolEventDispatcher>,
         signer_bitvec_len: u16,
-    ) -> Result<(NakamotoBlock, ExecutionCost, u64, Vec<TransactionEvent>), Error> {
+    ) -> Result<BlockMetadata, Error> {
         let (tip_consensus_hash, tip_block_hash, tip_height) = (
             parent_stacks_header.consensus_hash.clone(),
             parent_stacks_header.anchored_header.block_hash(),
@@ -556,7 +571,7 @@ impl NakamotoBlockBuilder {
             builder.load_tenure_info(&mut chainstate, burn_dbconn, tenure_info.cause())?;
         let mut tenure_tx = builder.tenure_begin(burn_dbconn, &mut miner_tenure_info)?;
 
-        let block_limit = tenure_tx
+        let tenure_budget = tenure_tx
             .block_limit()
             .expect("Failed to obtain block limit from miner's block connection");
 
@@ -570,7 +585,7 @@ impl NakamotoBlockBuilder {
                 (1..=100).contains(&percentage),
                 "BUG: tenure_cost_limit_per_block_percentage: {percentage}%. Must be between between 1 and 100"
             );
-            let mut remaining_limit = block_limit.clone();
+            let mut remaining_limit = tenure_budget.clone();
             let cost_so_far = tenure_tx.cost_so_far();
             if remaining_limit.sub(&cost_so_far).is_ok() && remaining_limit.divide(100).is_ok() {
                 remaining_limit.multiply(percentage.into()).expect(
@@ -581,7 +596,7 @@ impl NakamotoBlockBuilder {
                     "Setting soft limit for clarity cost to {percentage}% of remaining block limit";
                     "remaining_limit" => %remaining_limit,
                     "cost_so_far" => %cost_so_far,
-                    "block_limit" => %block_limit,
+                    "block_limit" => %tenure_budget,
                 );
                 soft_limit = Some(remaining_limit);
             };
@@ -630,13 +645,13 @@ impl NakamotoBlockBuilder {
 
         // save the block so we can build microblocks off of it
         let block = builder.mine_nakamoto_block(&mut tenure_tx);
-        let size = builder.bytes_so_far;
-        let consumed = builder.tenure_finish(tenure_tx)?;
+        let tenure_size = builder.bytes_so_far;
+        let tenure_consumed = builder.tenure_finish(tenure_tx)?;
 
         let ts_end = get_epoch_time_ms();
 
         set_last_mined_block_transaction_count(block.txs.len() as u64);
-        set_last_mined_execution_cost_observed(&consumed, &block_limit);
+        set_last_mined_execution_cost_observed(&tenure_consumed, &tenure_budget);
 
         info!(
             "Miner: mined Nakamoto block";
@@ -645,14 +660,20 @@ impl NakamotoBlockBuilder {
             "height" => block.header.chain_length,
             "tx_count" => block.txs.len(),
             "parent_block_id" => %block.header.parent_block_id,
-            "block_size" => size,
-            "execution_consumed" => %consumed,
-            "percent_full" => block_limit.proportion_largest_dimension(&consumed),
+            "block_size" => tenure_size,
+            "execution_consumed" => %tenure_consumed,
+            "percent_full" => tenure_budget.proportion_largest_dimension(&tenure_consumed),
             "assembly_time_ms" => ts_end.saturating_sub(ts_start),
             "consensus_hash" => %block.header.consensus_hash
         );
 
-        Ok((block, consumed, size, tx_events))
+        Ok(BlockMetadata {
+            block,
+            tenure_consumed,
+            tenure_budget,
+            tenure_size,
+            tx_events,
+        })
     }
 
     pub fn get_bytes_so_far(&self) -> u64 {

stackslib/src/chainstate/nakamoto/mod.rs

@@ -2469,7 +2469,7 @@ impl NakamotoChainState {
     ) -> Result<bool, ChainstateError> {
         test_debug!("Consider Nakamoto block {}", &block.block_id());
         // do nothing if we already have this block
-        if let Some(_) = Self::get_block_header(headers_conn, &block.header.block_id())? {
+        if Self::get_block_header(headers_conn, &block.header.block_id())?.is_some() {
             debug!("Already have block {}", &block.header.block_id());
             return Ok(false);
         }

stackslib/src/chainstate/stacks/block.rs

@@ -353,16 +353,13 @@ impl StacksMessageCodec for StacksBlock {
         // must be only one coinbase
         let mut coinbase_count = 0;
         for tx in txs.iter() {
-            match tx.payload {
-                TransactionPayload::Coinbase(..) => {
-                    coinbase_count += 1;
-                    if coinbase_count > 1 {
-                        return Err(codec_error::DeserializeError(
-                            "Invalid block: multiple coinbases found".to_string(),
-                        ));
-                    }
+            if let TransactionPayload::Coinbase(..) = tx.payload {
+                coinbase_count += 1;
+                if coinbase_count > 1 {
+                    return Err(codec_error::DeserializeError(
+                        "Invalid block: multiple coinbases found".to_string(),
+                    ));
                 }
-                _ => {}
             }
         }
 
@@ -515,26 +512,23 @@ impl StacksBlock {
         let mut found_coinbase = false;
         let mut coinbase_index = 0;
         for (i, tx) in txs.iter().enumerate() {
-            match tx.payload {
-                TransactionPayload::Coinbase(..) => {
-                    if !check_present {
-                        warn!("Found unexpected coinbase tx {}", tx.txid());
-                        return false;
-                    }
-
-                    if found_coinbase {
-                        warn!("Found duplicate coinbase tx {}", tx.txid());
-                        return false;
-                    }
-
-                    if tx.anchor_mode != TransactionAnchorMode::OnChainOnly {
-                        warn!("Invalid coinbase tx {}: not on-chain only", tx.txid());
-                        return false;
-                    }
-                    found_coinbase = true;
-                    coinbase_index = i;
+            if let TransactionPayload::Coinbase(..) = tx.payload {
+                if !check_present {
+                    warn!("Found unexpected coinbase tx {}", tx.txid());
+                    return false;
+                }
+
+                if found_coinbase {
+                    warn!("Found duplicate coinbase tx {}", tx.txid());
+                    return false;
+                }
+
+                if tx.anchor_mode != TransactionAnchorMode::OnChainOnly {
+                    warn!("Invalid coinbase tx {}: not on-chain only", tx.txid());
+                    return false;
                 }
-                _ => {}
+                found_coinbase = true;
+                coinbase_index = i;
             }
         }
 

stackslib/src/chainstate/stacks/boot/contract_tests.rs

@@ -486,7 +486,7 @@ impl BurnStateDB for TestSimBurnStateDB {
         height: u32,
         sortition_id: &SortitionId,
     ) -> Option<(Vec<TupleData>, u128)> {
-        if let Some(_) = self.get_burn_header_hash(height, sortition_id) {
+        if self.get_burn_header_hash(height, sortition_id).is_some() {
             let first_block = self.get_burn_start_height();
             let prepare_len = self.get_pox_prepare_length();
             let rc_len = self.get_pox_reward_cycle_length();