Merge branch 'develop' into refactor/add-epoch-index

Author: obycode

.github/workflows/bitcoin-tests.yml

@@ -99,6 +99,8 @@ jobs:
           - tests::signer::v0::forked_tenure_okay
           - tests::signer::v0::forked_tenure_invalid
           - tests::signer::v0::empty_sortition
+          - tests::signer::v0::empty_sortition_before_approval
+          - tests::signer::v0::empty_sortition_before_proposal
           - tests::signer::v0::bitcoind_forking_test
           - tests::signer::v0::multiple_miners
           - tests::signer::v0::mock_sign_epoch_25
@@ -112,6 +114,7 @@ jobs:
           - tests::signer::v0::locally_accepted_blocks_overriden_by_global_rejection
           - tests::signer::v0::locally_rejected_blocks_overriden_by_global_acceptance
           - tests::signer::v0::reorg_locally_accepted_blocks_across_tenures_succeeds
+          - tests::signer::v0::reorg_locally_accepted_blocks_across_tenures_fails
           - tests::signer::v0::miner_recovers_when_broadcast_block_delay_across_tenures_occurs
           - tests::signer::v0::multiple_miners_with_nakamoto_blocks
           - tests::signer::v0::partial_tenure_fork
@@ -120,6 +123,8 @@ jobs:
           - tests::signer::v0::signing_in_0th_tenure_of_reward_cycle
           - tests::signer::v0::continue_after_tenure_extend
           - tests::signer::v0::multiple_miners_with_custom_chain_id
+          - tests::signer::v0::block_commit_delay
+          - tests::signer::v0::continue_after_fast_block_no_sortition
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state
@@ -133,6 +138,7 @@ jobs:
           - tests::nakamoto_integrations::utxo_check_on_startup_panic
           - tests::nakamoto_integrations::utxo_check_on_startup_recover
           - tests::nakamoto_integrations::v3_signer_api_endpoint
+          - tests::nakamoto_integrations::signer_chainstate
           # TODO: enable these once v1 signer is supported by a new nakamoto epoch
           # - tests::signer::v1::dkg
           # - tests::signer::v1::sign_request_rejected

CHANGELOG.md

@@ -10,6 +10,23 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 ### Changed
 - Add index for StacksBlockId to nakamoto block headers table (improves node performance)
 - Remove the panic for reporting DB deadlocks (just error and continue waiting)
+- Add index to `metadata_table` in Clarity DB on `blockhash`
+- Add `block_commit_delay_ms` to the config file to control the time to wait after seeing a new burn block, before submitting a block commit, to allow time for the first Nakamoto block of the new tenure to be mined, allowing this miner to avoid the need to RBF the block commit.
+
+## [3.0.0.0.1]
+
+### Changed
+- Add index for StacksBlockId to nakamoto block headers table (improves node performance)
+- Remove the panic for reporting DB deadlocks (just error and continue waiting)
+- Various test fixes for CI (5353, 5368, 5372, 5371, 5380, 5378, 5387, 5396, 5390, 5394)
+- Various log fixes:
+    - don't say proceeding to mine blocks if not a miner
+    - misc. warns downgraded to debugs
+- 5391: Update default block proposal timeout to 10 minutes
+- 5406: After block rejection, miner pauses
+- Docs fixes
+    - Fix signer docs link
+    - Specify burn block in clarity docs
 
 ## [3.0.0.0.0]
 

clarity/src/vm/database/sqlite.rs

@@ -248,6 +248,12 @@ impl SqliteConnection {
         )
         .map_err(|x| InterpreterError::SqliteError(IncomparableError { err: x }))?;
 
+        conn.execute(
+            "CREATE INDEX IF NOT EXISTS md_blockhashes ON metadata_table(blockhash)",
+            NO_PARAMS,
+        )
+        .map_err(|x| InterpreterError::SqliteError(IncomparableError { err: x }))?;
+
         Self::check_schema(conn)?;
 
         Ok(())

docs/mining.md

@@ -19,14 +19,26 @@ nakamoto_attempt_time_ms = 20000
 [burnchain]
 # Maximum amount (in sats) of "burn commitment" to broadcast for the next block's leader election
 burn_fee_cap = 20000
-# Amount (in sats) per byte - Used to calculate the transaction fees
-satoshis_per_byte = 25
-# Amount of sats to add when RBF'ing bitcoin tx  (default: 5)
+# Amount in sats per byte used to calculate the Bitcoin transaction fee (default: 50)
+satoshis_per_byte = 50
+# Amount of sats per byte to add when RBF'ing a Bitcoin tx  (default: 5)
 rbf_fee_increment = 5
-# Maximum percentage to RBF bitcoin tx (default: 150% of satsv/B)
+# Maximum percentage of satoshis_per_byte to allow in RBF fee (default: 150)
 max_rbf = 150
 ```
 
+NOTE: Ensuring that your miner can successfully use RBF (Replace-by-Fee) is
+critical for reliable block production. If a miner fails to replace an outdated
+block commit with a higher-fee transaction, it risks committing to an incorrect
+tenure. This would prevent the miner from producing valid blocks during its
+tenure, as it would be building on an invalid chain tip, causing the signers to
+reject its blocks.
+
+To avoid this, configure satoshis_per_byte, rbf_fee_increment, and max_rbf to
+allow for at least three fee increments within the max_rbf limit. This helps
+ensure that your miner can adjust its fees sufficiently to stay on the canonical
+chain.
+
 You can verify that your node is operating as a miner by checking its log output
 to verify that it was able to find its Bitcoin UTXOs:
 

stacks-signer/CHANGELOG.md

@@ -11,7 +11,21 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ### Changed
 
-## [3.0.0.0.0]
+## [3.0.0.0.1.0]
+
+### Changed
+
+- Change block rejection message to generic block response
+
+## [3.0.0.0.0.1]
+
+### Added
+
+### Changed
+- Update block proposal timeout default to 10 minutes (#5391)
+- Updated documentation link in output (#5363)
+
+## [3.0.0.0.0.0]
 
 ### Added
 

stacks-signer/src/chainstate.rs

@@ -19,15 +19,15 @@ use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
 use blockstack_lib::chainstate::stacks::TenureChangePayload;
 use blockstack_lib::net::api::getsortition::SortitionInfo;
 use blockstack_lib::util_lib::db::Error as DBError;
-use clarity::types::chainstate::BurnchainHeaderHash;
 use slog::{slog_info, slog_warn};
-use stacks_common::types::chainstate::{ConsensusHash, StacksPublicKey};
+use stacks_common::types::chainstate::{BurnchainHeaderHash, ConsensusHash, StacksPublicKey};
+use stacks_common::util::get_epoch_time_secs;
 use stacks_common::util::hash::Hash160;
 use stacks_common::{info, warn};
 
 use crate::client::{ClientError, CurrentAndLastSortition, StacksClient};
 use crate::config::SignerConfig;
-use crate::signerdb::{BlockState, SignerDb};
+use crate::signerdb::{BlockInfo, BlockState, SignerDb};
 
 #[derive(thiserror::Error, Debug)]
 /// Error type for the signer chainstate module
@@ -119,13 +119,17 @@ pub struct ProposalEvalConfig {
     pub first_proposal_burn_block_timing: Duration,
     /// Time between processing a sortition and proposing a block before the block is considered invalid
     pub block_proposal_timeout: Duration,
+    /// Time to wait for the last block of a tenure to be globally accepted or rejected before considering
+    /// a new miner's block at the same height as valid.
+    pub tenure_last_block_proposal_timeout: Duration,
 }
 
 impl From<&SignerConfig> for ProposalEvalConfig {
     fn from(value: &SignerConfig) -> Self {
         Self {
             first_proposal_burn_block_timing: value.first_proposal_burn_block_timing,
             block_proposal_timeout: value.block_proposal_timeout,
+            tenure_last_block_proposal_timeout: value.tenure_last_block_proposal_timeout,
         }
     }
 }
@@ -460,7 +464,36 @@ impl SortitionsView {
         Ok(true)
     }
 
-    /// Check if the tenure change block confirms the expected parent block (i.e., the last globally accepted block in the parent tenure)
+    /// Get the last block from the given tenure
+    /// Returns the last locally accepted block if it is not timed out, otherwise it will return the last globally accepted block.
+    fn get_tenure_last_block_info(
+        consensus_hash: &ConsensusHash,
+        signer_db: &SignerDb,
+        tenure_last_block_proposal_timeout: Duration,
+    ) -> Result<Option<BlockInfo>, ClientError> {
+        // Get the last known block in the previous tenure
+        let last_locally_accepted_block = signer_db
+            .get_last_accepted_block(consensus_hash)
+            .map_err(|e| ClientError::InvalidResponse(e.to_string()))?;
+
+        if let Some(local_info) = last_locally_accepted_block {
+            if let Some(signed_over_time) = local_info.signed_self {
+                if signed_over_time + tenure_last_block_proposal_timeout.as_secs()
+                    > get_epoch_time_secs()
+                {
+                    // The last locally accepted block is not timed out, return it
+                    return Ok(Some(local_info));
+                }
+            }
+        }
+        // The last locally accepted block is timed out, get the last globally accepted block
+        signer_db
+            .get_last_globally_accepted_block(consensus_hash)
+            .map_err(|e| ClientError::InvalidResponse(e.to_string()))
+    }
+
+    /// Check if the tenure change block confirms the expected parent block
+    /// (i.e., the last locally accepted block in the parent tenure, or if that block is timed out, the last globally accepted block in the parent tenure)
     /// It checks the local DB first, and if the block is not present in the local DB, it asks the
     /// Stacks node for the highest processed block header in the given tenure (and then caches it
     /// in the DB).
@@ -473,24 +506,27 @@ impl SortitionsView {
         reward_cycle: u64,
         signer_db: &mut SignerDb,
         client: &StacksClient,
+        tenure_last_block_proposal_timeout: Duration,
     ) -> Result<bool, ClientError> {
-        // If the tenure change block confirms the expected parent block, it should confirm at least one more block than the last globally accepted block in the parent tenure.
-        let last_globally_accepted_block = signer_db
-            .get_last_globally_accepted_block(&tenure_change.prev_tenure_consensus_hash)
-            .map_err(|e| ClientError::InvalidResponse(e.to_string()))?;
+        // If the tenure change block confirms the expected parent block, it should confirm at least one more block than the last accepted block in the parent tenure.
+        let last_block_info = Self::get_tenure_last_block_info(
+            &tenure_change.prev_tenure_consensus_hash,
+            signer_db,
+            tenure_last_block_proposal_timeout,
+        )?;
 
-        if let Some(global_info) = last_globally_accepted_block {
+        if let Some(info) = last_block_info {
             // N.B. this block might not be the last globally accepted block across the network;
             // it's just the highest one in this tenure that we know about.  If this given block is
             // no higher than it, then it's definitely no higher than the last globally accepted
             // block across the network, so we can do an early rejection here.
-            if block.header.chain_length <= global_info.block.header.chain_length {
+            if block.header.chain_length <= info.block.header.chain_length {
                 warn!(
                     "Miner's block proposal does not confirm as many blocks as we expect";
                     "proposed_block_consensus_hash" => %block.header.consensus_hash,
                     "proposed_block_signer_sighash" => %block.header.signer_signature_hash(),
                     "proposed_chain_length" => block.header.chain_length,
-                    "expected_at_least" => global_info.block.header.chain_length + 1,
+                    "expected_at_least" => info.block.header.chain_length + 1,
                 );
                 return Ok(false);
             }
@@ -558,6 +594,7 @@ impl SortitionsView {
             reward_cycle,
             signer_db,
             client,
+            self.config.tenure_last_block_proposal_timeout,
         )?;
         if !confirms_expected_parent {
             return Ok(false);
@@ -573,15 +610,15 @@ impl SortitionsView {
         if !is_valid_parent_tenure {
             return Ok(false);
         }
-        let last_in_tenure = signer_db
+        let last_in_current_tenure = signer_db
             .get_last_globally_accepted_block(&block.header.consensus_hash)
             .map_err(|e| ClientError::InvalidResponse(e.to_string()))?;
-        if let Some(last_in_tenure) = last_in_tenure {
+        if let Some(last_in_current_tenure) = last_in_current_tenure {
             warn!(
                 "Miner block proposal contains a tenure change, but we've already signed a block in this tenure. Considering proposal invalid.";
                 "proposed_block_consensus_hash" => %block.header.consensus_hash,
                 "proposed_block_signer_sighash" => %block.header.signer_signature_hash(),
-                "last_in_tenure_signer_sighash" => %last_in_tenure.block.header.signer_signature_hash(),
+                "last_in_tenure_signer_sighash" => %last_in_current_tenure.block.header.signer_signature_hash(),
             );
             return Ok(false);
         }

stacks-signer/src/client/mod.rs

@@ -411,6 +411,7 @@ pub(crate) mod tests {
             db_path: config.db_path.clone(),
             first_proposal_burn_block_timing: config.first_proposal_burn_block_timing,
             block_proposal_timeout: config.block_proposal_timeout,
+            tenure_last_block_proposal_timeout: config.tenure_last_block_proposal_timeout,
         }
     }
 

stacks-signer/src/config.rs

@@ -36,6 +36,7 @@ use crate::client::SignerSlotID;
 const EVENT_TIMEOUT_MS: u64 = 5000;
 const BLOCK_PROPOSAL_TIMEOUT_MS: u64 = 600_000;
 const DEFAULT_FIRST_PROPOSAL_BURN_BLOCK_TIMING_SECS: u64 = 60;
+const DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS: u64 = 30;
 
 #[derive(thiserror::Error, Debug)]
 /// An error occurred parsing the provided configuration
@@ -128,6 +129,9 @@ pub struct SignerConfig {
     pub first_proposal_burn_block_timing: Duration,
     /// How much time to wait for a miner to propose a block following a sortition
     pub block_proposal_timeout: Duration,
+    /// Time to wait for the last block of a tenure to be globally accepted or rejected
+    /// before considering a new miner's block at the same height as potentially valid.
+    pub tenure_last_block_proposal_timeout: Duration,
 }
 
 /// The parsed configuration for the signer
@@ -158,6 +162,9 @@ pub struct GlobalConfig {
     pub block_proposal_timeout: Duration,
     /// An optional custom Chain ID
     pub chain_id: Option<u32>,
+    /// Time to wait for the last block of a tenure to be globally accepted or rejected
+    /// before considering a new miner's block at the same height as potentially valid.
+    pub tenure_last_block_proposal_timeout: Duration,
 }
 
 /// Internal struct for loading up the config file
@@ -180,13 +187,16 @@ struct RawConfigFile {
     pub db_path: String,
     /// Metrics endpoint
     pub metrics_endpoint: Option<String>,
-    /// How much time must pass between the first block proposal in a tenure and the next bitcoin block
+    /// How much time must pass in seconds between the first block proposal in a tenure and the next bitcoin block
     ///  before a subsequent miner isn't allowed to reorg the tenure
     pub first_proposal_burn_block_timing_secs: Option<u64>,
     /// How much time to wait for a miner to propose a block following a sortition in milliseconds
     pub block_proposal_timeout_ms: Option<u64>,
     /// An optional custom Chain ID
     pub chain_id: Option<u32>,
+    /// Time in seconds to wait for the last block of a tenure to be globally accepted or rejected
+    /// before considering a new miner's block at the same height as potentially valid.
+    pub tenure_last_block_proposal_timeout_secs: Option<u64>,
 }
 
 impl RawConfigFile {
@@ -266,6 +276,12 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
                 .unwrap_or(BLOCK_PROPOSAL_TIMEOUT_MS),
         );
 
+        let tenure_last_block_proposal_timeout = Duration::from_secs(
+            raw_data
+                .tenure_last_block_proposal_timeout_secs
+                .unwrap_or(DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS),
+        );
+
         Ok(Self {
             node_host: raw_data.node_host,
             endpoint,
@@ -279,6 +295,7 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
             first_proposal_burn_block_timing,
             block_proposal_timeout,
             chain_id: raw_data.chain_id,
+            tenure_last_block_proposal_timeout,
         })
     }
 }

stacks-signer/src/runloop.rs

@@ -283,6 +283,7 @@ impl<Signer: SignerTrait<T>, T: StacksMessageCodec + Clone + Send + Debug> RunLo
             mainnet: self.config.network.is_mainnet(),
             db_path: self.config.db_path.clone(),
             block_proposal_timeout: self.config.block_proposal_timeout,
+            tenure_last_block_proposal_timeout: self.config.tenure_last_block_proposal_timeout,
         }))
     }
 

stacks-signer/src/signerdb.rs

@@ -24,7 +24,6 @@ use blockstack_lib::util_lib::db::{
     Error as DBError,
 };
 use clarity::types::chainstate::{BurnchainHeaderHash, StacksAddress};
-use clarity::util::get_epoch_time_secs;
 use libsigner::BlockProposal;
 use rusqlite::{
     params, Connection, Error as SqliteError, OpenFlags, OptionalExtension, Transaction,
@@ -33,6 +32,7 @@ use serde::{Deserialize, Serialize};
 use slog::{slog_debug, slog_error};
 use stacks_common::codec::{read_next, write_next, Error as CodecError, StacksMessageCodec};
 use stacks_common::types::chainstate::ConsensusHash;
+use stacks_common::util::get_epoch_time_secs;
 use stacks_common::util::hash::Sha512Trunc256Sum;
 use stacks_common::util::secp256k1::MessageSignature;
 use stacks_common::{debug, define_u8_enum, error};