Merge pull request #5787 from stacks-network/feat/extend-when-needed

feat: do not issue a time-based tenure extend earlier than needed

Author: obycode

CHANGELOG.md

@@ -8,15 +8,13 @@ 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.
+
+- 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)
+- Miner will not issue a tenure extend until at least half of the block budget has been spent (#5757)
 
 ### Fixed
 

stacks-signer/CHANGELOG.md

@@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to the versioning scheme outlined in the [README.md](README.md).
 
+## [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.
+
 ## [3.1.0.0.4.0]
 
 ## Added

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

@@ -40,7 +40,7 @@ use crate::chainstate::burn::db::sortdb::{
 };
 use crate::chainstate::burn::{BlockSnapshot, ConsensusHash};
 use crate::chainstate::coordinator::OnChainRewardSetProvider;
-use crate::chainstate::nakamoto::miner::{NakamotoBlockBuilder, NakamotoTenureInfo};
+use crate::chainstate::nakamoto::miner::{BlockMetadata, NakamotoBlockBuilder, NakamotoTenureInfo};
 use crate::chainstate::nakamoto::{NakamotoBlock, NakamotoChainState};
 use crate::chainstate::stacks::db::blocks::StagingBlock;
 use crate::chainstate::stacks::db::{StacksBlockHeaderTypes, StacksChainState, StacksHeaderInfo};
@@ -504,7 +504,21 @@ pub fn command_try_mine(argv: &[String], conf: Option<&Config>) {
                 None,
                 0,
             )
-            .map(|(block, cost, size, _)| (block.header.block_hash(), block.txs, cost, size))
+            .map(
+                |BlockMetadata {
+                     block,
+                     tenure_consumed,
+                     tenure_size,
+                     ..
+                 }| {
+                    (
+                        block.header.block_hash(),
+                        block.txs,
+                        tenure_consumed,
+                        tenure_size,
+                    )
+                },
+            )
         }
     };
 

stackslib/src/config/mod.rs

@@ -86,18 +86,41 @@ pub const OP_TX_ANY_ESTIM_SIZE: u64 = fmax!(
     OP_TX_VOTE_AGG_ESTIM_SIZE
 );
 
+/// Default maximum percentage of `satoshis_per_byte` that a Bitcoin fee rate
+/// may be increased to when RBFing a transaction
 const DEFAULT_MAX_RBF_RATE: u64 = 150; // 1.5x
+/// Amount to increment the fee by, in Sats/vByte, when RBFing a Bitcoin
+/// transaction
 const DEFAULT_RBF_FEE_RATE_INCREMENT: u64 = 5;
+/// Default number of reward cycles of blocks to sync in a non-full inventory
+/// sync
 const INV_REWARD_CYCLES_TESTNET: u64 = 6;
+/// Default minimum time to wait between mining blocks in milliseconds. The
+/// value must be greater than or equal to 1000 ms because if a block is mined
+/// within the same second as its parent, it will be rejected by the signers.
 const DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS: u64 = 1_000;
+/// Default time in milliseconds to pause after receiving the first threshold
+/// rejection, before proposing a new block.
 const DEFAULT_FIRST_REJECTION_PAUSE_MS: u64 = 5_000;
+/// Default time in milliseconds to pause after receiving subsequent threshold
+/// rejections, before proposing a new block.
 const DEFAULT_SUBSEQUENT_REJECTION_PAUSE_MS: u64 = 10_000;
+/// Default time in milliseconds to wait for a Nakamoto block after seeing a
+/// burnchain block before submitting a block commit.
 const DEFAULT_BLOCK_COMMIT_DELAY_MS: u64 = 20_000;
+/// Default percentage of the remaining tenure cost limit to consume each block
 const DEFAULT_TENURE_COST_LIMIT_PER_BLOCK_PERCENTAGE: u8 = 25;
+/// Default number of seconds to wait in-between polling the sortition DB to
+/// see if we need to extend the ongoing tenure (e.g. because the current
+/// sortition is empty or invalid).
 const DEFAULT_TENURE_EXTEND_POLL_SECS: u64 = 1;
-
-// This should be greater than the signers' timeout. This is used for issuing fallback tenure extends
-const DEFAULT_TENURE_TIMEOUT_SECS: u64 = 420;
+/// Default duration to wait before attempting to issue a tenure extend.
+/// This should be greater than the signers' timeout. This is used for issuing
+/// fallback tenure extends
+const DEFAULT_TENURE_TIMEOUT_SECS: u64 = 180;
+/// Default percentage of block budget that must be used before attempting a
+/// time-based tenure extend
+const DEFAULT_TENURE_EXTEND_COST_THRESHOLD: u64 = 50;
 
 static HELIUM_DEFAULT_CONNECTION_OPTIONS: LazyLock<ConnectionOptions> =
     LazyLock::new(|| ConnectionOptions {
@@ -1191,9 +1214,13 @@ pub struct BurnchainConfig {
     pub process_exit_at_block_height: Option<u64>,
     pub poll_time_secs: u64,
     pub satoshis_per_byte: u64,
+    /// Maximum percentage of `satoshis_per_byte` that a Bitcoin fee rate may
+    /// be increased to when RBFing a transaction
     pub max_rbf: u64,
     pub leader_key_tx_estimated_size: u64,
     pub block_commit_tx_estimated_size: u64,
+    /// Amount to increment the fee by, in Sats/vByte, when RBFing a Bitcoin
+    /// transaction
     pub rbf_fee_increment: u64,
     pub first_burn_block_height: Option<u64>,
     pub first_burn_block_timestamp: Option<u32>,
@@ -2155,6 +2182,8 @@ pub struct MinerConfig {
     pub tenure_extend_poll_secs: Duration,
     /// Duration to wait before attempting to issue a tenure extend
     pub tenure_timeout: Duration,
+    /// Percentage of block budget that must be used before attempting a time-based tenure extend
+    pub tenure_extend_cost_threshold: u64,
 }
 
 impl Default for MinerConfig {
@@ -2193,6 +2222,7 @@ impl Default for MinerConfig {
             ),
             tenure_extend_poll_secs: Duration::from_secs(DEFAULT_TENURE_EXTEND_POLL_SECS),
             tenure_timeout: Duration::from_secs(DEFAULT_TENURE_TIMEOUT_SECS),
+            tenure_extend_cost_threshold: DEFAULT_TENURE_EXTEND_COST_THRESHOLD,
         }
     }
 }
@@ -2589,6 +2619,7 @@ pub struct MinerConfigFile {
     pub tenure_cost_limit_per_block_percentage: Option<u8>,
     pub tenure_extend_poll_secs: Option<u64>,
     pub tenure_timeout_secs: Option<u64>,
+    pub tenure_extend_cost_threshold: Option<u64>,
 }
 
 impl MinerConfigFile {
@@ -2731,6 +2762,7 @@ impl MinerConfigFile {
             tenure_cost_limit_per_block_percentage,
             tenure_extend_poll_secs: self.tenure_extend_poll_secs.map(Duration::from_secs).unwrap_or(miner_default_config.tenure_extend_poll_secs),
             tenure_timeout: self.tenure_timeout_secs.map(Duration::from_secs).unwrap_or(miner_default_config.tenure_timeout),
+            tenure_extend_cost_threshold: self.tenure_extend_cost_threshold.unwrap_or(miner_default_config.tenure_extend_cost_threshold),
         })
     }
 }

stackslib/src/net/connection.rs

@@ -379,6 +379,7 @@ pub struct ConnectionOptions {
     /// Units are milliseconds.  A value of 0 means "never".
     pub log_neighbors_freq: u64,
     pub inv_sync_interval: u64,
+    // how many reward cycles of blocks to sync in a non-full inventory sync
     pub inv_reward_cycles: u64,
     pub download_interval: u64,
     pub pingback_timeout: u64,

testnet/stacks-node/src/nakamoto_node/miner.rs

@@ -21,6 +21,7 @@ use std::thread;
 use std::time::{Duration, Instant};
 
 use clarity::boot_util::boot_code_id;
+use clarity::vm::costs::ExecutionCost;
 use clarity::vm::types::PrincipalData;
 use libsigner::v0::messages::{MinerSlotID, SignerMessage};
 use libsigner::StackerDBSession;
@@ -177,6 +178,10 @@ pub struct BlockMinerThread {
     last_block_mined: Option<NakamotoBlock>,
     /// Number of blocks mined since a tenure change/extend was attempted
     mined_blocks: u64,
+    /// Cost consumed by the current tenure
+    tenure_cost: ExecutionCost,
+    /// Cost budget for the current tenure
+    tenure_budget: ExecutionCost,
     /// Copy of the node's registered VRF key
     registered_key: RegisteredKey,
     /// Burnchain block snapshot which elected this miner
@@ -237,6 +242,8 @@ impl BlockMinerThread {
             burn_tip_at_start: burn_tip_at_start.clone(),
             tenure_change_time: Instant::now(),
             abort_flag: Arc::new(AtomicBool::new(false)),
+            tenure_cost: ExecutionCost::ZERO,
+            tenure_budget: ExecutionCost::ZERO,
         }
     }
 
@@ -1183,7 +1190,7 @@ impl BlockMinerThread {
         }
 
         // build the block itself
-        let (mut block, consumed, size, tx_events) = NakamotoBlockBuilder::build_nakamoto_block(
+        let mut block_metadata = NakamotoBlockBuilder::build_nakamoto_block(
             &chain_state,
             &burn_db
                 .index_handle_at_ch(&self.burn_block.consensus_hash)
@@ -1210,39 +1217,48 @@ impl BlockMinerThread {
             e
         })?;
 
-        if block.txs.is_empty() {
+        if block_metadata.block.txs.is_empty() {
             return Err(ChainstateError::NoTransactionsToMine.into());
         }
         let mining_key = self.keychain.get_nakamoto_sk();
         let miner_signature = mining_key
-            .sign(block.header.miner_signature_hash().as_bytes())
+            .sign(
+                block_metadata
+                    .block
+                    .header
+                    .miner_signature_hash()
+                    .as_bytes(),
+            )
             .map_err(NakamotoNodeError::MinerSignatureError)?;
-        block.header.miner_signature = miner_signature;
+        block_metadata.block.header.miner_signature = miner_signature;
 
         info!(
             "Miner: Assembled block #{} for signer set proposal: {}, with {} txs",
-            block.header.chain_length,
-            block.header.block_hash(),
-            block.txs.len();
-            "signer_sighash" => %block.header.signer_signature_hash(),
-            "consensus_hash" => %block.header.consensus_hash,
-            "parent_block_id" => %block.header.parent_block_id,
-            "timestamp" => block.header.timestamp,
+            block_metadata.block.header.chain_length,
+            block_metadata.block.header.block_hash(),
+            block_metadata.block.txs.len();
+            "signer_sighash" => %block_metadata.block.header.signer_signature_hash(),
+            "consensus_hash" => %block_metadata.block.header.consensus_hash,
+            "parent_block_id" => %block_metadata.block.header.parent_block_id,
+            "timestamp" => block_metadata.block.header.timestamp,
         );
 
         self.event_dispatcher.process_mined_nakamoto_block_event(
             self.burn_block.block_height,
-            &block,
-            size,
-            &consumed,
-            tx_events,
+            &block_metadata.block,
+            block_metadata.tenure_size,
+            &block_metadata.tenure_consumed,
+            block_metadata.tx_events,
         );
 
+        self.tenure_cost = block_metadata.tenure_consumed;
+        self.tenure_budget = block_metadata.tenure_budget;
+
         // last chance -- confirm that the stacks tip is unchanged (since it could have taken long
         // enough to build this block that another block could have arrived), and confirm that all
         // Stacks blocks with heights higher than the canonical tip are processed.
         self.check_burn_tip_changed(&burn_db)?;
-        Ok(block)
+        Ok(block_metadata.block)
     }
 
     #[cfg_attr(test, mutants::skip)]
@@ -1273,8 +1289,20 @@ impl BlockMinerThread {
                 }
             }
         };
+        // Check if we can and should include a time-based tenure extend.
         if self.last_block_mined.is_some() {
-            // Check if we can extend the current tenure
+            // Do not extend if we have spent < 50% of the budget, since it is
+            // not necessary.
+            let usage = self
+                .tenure_budget
+                .proportion_largest_dimension(&self.tenure_cost);
+            if usage < self.config.miner.tenure_extend_cost_threshold {
+                return Ok(NakamotoTenureInfo {
+                    coinbase_tx: None,
+                    tenure_change_tx: None,
+                });
+            }
+
             let tenure_extend_timestamp = coordinator.get_tenure_extend_timestamp();
             if get_epoch_time_secs() <= tenure_extend_timestamp
                 && self.tenure_change_time.elapsed() <= self.config.miner.tenure_timeout
@@ -1284,6 +1312,7 @@ impl BlockMinerThread {
                     tenure_change_tx: None,
                 });
             }
+
             info!("Miner: Time-based tenure extend";
                 "current_timestamp" => get_epoch_time_secs(),
                 "tenure_extend_timestamp" => tenure_extend_timestamp,