Merge branch 'develop' into test/replay-block-naka

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,9 @@ 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::signer::v0::block_validation_response_timeout
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state
@@ -133,6 +139,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

@@ -7,6 +7,27 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ## [Unreleased]
 
+### 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]
 
 ### Added

Cargo.lock

@@ -21,6 +21,7 @@ rand = "0.8"
 rand_chacha = "0.3.1"
 tikv-jemallocator = "0.5.4"
 rusqlite = { version = "0.31.0", features = ["blob", "serde_json", "i128_blob", "bundled", "trace"] }
+thiserror = { version = "1.0.65" }
 
 # Use a bit more than default optimization for
 #  dev builds to speed up test execution

Cargo.toml

@@ -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(())

clarity/src/vm/database/sqlite.rs

@@ -1775,17 +1775,17 @@ this value is less than or equal to the value for `miner-spend-total` at the sam
 const GET_BURN_BLOCK_INFO_API: SpecialAPI = SpecialAPI {
     input_type: "BurnBlockInfoPropertyName, uint",
     output_type: "(optional buff) | (optional (tuple (addrs (list 2 (tuple (hashbytes (buff 32)) (version (buff 1))))) (payout uint)))",
-    snippet: "get-burn-block-info? ${1:prop} ${2:block-height}",
-    signature: "(get-burn-block-info? prop-name block-height)",
+    snippet: "get-burn-block-info? ${1:prop} ${2:burn-block-height}",
+    signature: "(get-burn-block-info? prop-name burn-block-height)",
     description: "The `get-burn-block-info?` function fetches data for a block of the given *burnchain* block height. The
-value and type returned are determined by the specified `BlockInfoPropertyName`.  Valid values for `block-height` only
+value and type returned are determined by the specified `BlockInfoPropertyName`.  Valid values for `burn-block-height` only
 include heights between the burnchain height at the time the Stacks chain was launched, and the last-processed burnchain
-block.  If the `block-height` argument falls outside of this range, then `none` shall be returned.
+block.  If the `burn-block-height` argument falls outside of this range, then `none` shall be returned.
 
 The following `BlockInfoPropertyName` values are defined:
 
 * The `header-hash` property returns a 32-byte buffer representing the header hash of the burnchain block at
-burnchain height `block-height`.
+burnchain height `burn-block-height`.
 
 * The `pox-addrs` property returns a tuple with two items: a list of up to two PoX addresses that received a PoX payout at that block height, and the amount of burnchain
 tokens paid to each address (note that per the blockchain consensus rules, each PoX payout will be the same for each address in the block-commit transaction).
@@ -1811,11 +1811,11 @@ The `addrs` list contains the same PoX address values passed into the PoX smart
 
 const GET_STACKS_BLOCK_INFO_API: SpecialAPI = SpecialAPI {
     input_type: "StacksBlockInfoPropertyName, uint",
-    snippet: "get-stacks-block-info? ${1:prop} ${2:block-height}",
+    snippet: "get-stacks-block-info? ${1:prop} ${2:stacks-block-height}",
     output_type: "(optional buff) | (optional uint)",
-    signature: "(get-stacks-block-info? prop-name block-height)",
+    signature: "(get-stacks-block-info? prop-name stacks-block-height)",
     description: "The `get-stacks-block-info?` function fetches data for a block of the given *Stacks* block height. The
-value and type returned are determined by the specified `StacksBlockInfoPropertyName`. If the provided `block-height` does
+value and type returned are determined by the specified `StacksBlockInfoPropertyName`. If the provided `stacks-block-height` does
 not correspond to an existing block prior to the current block, the function returns `none`. The currently available property names
 are as follows:
 
@@ -1840,11 +1840,11 @@ the mining of this block started, but is not guaranteed to be accurate. This tim
 
 const GET_TENURE_INFO_API: SpecialAPI = SpecialAPI {
     input_type: "TenureInfoPropertyName, uint",
-    snippet: "get-tenure-info? ${1:prop} ${2:block-height}",
+    snippet: "get-tenure-info? ${1:prop} ${2:stacks-block-height}",
     output_type: "(optional buff) | (optional uint)",
-    signature: "(get-tenure-info? prop-name block-height)",
+    signature: "(get-tenure-info? prop-name stacks-block-height)",
     description: "The `get-tenure-info?` function fetches data for the tenure at the given block height. The
-value and type returned are determined by the specified `TenureInfoPropertyName`. If the provided `block-height` does
+value and type returned are determined by the specified `TenureInfoPropertyName`. If the provided `stacks-block-height` does
 not correspond to an existing block prior to the current block, the function returns `none`. The currently available property names
 are as follows:
 

clarity/src/vm/docs/mod.rs

@@ -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:
 

docs/mining.md

@@ -30,7 +30,7 @@ slog-term = "2.6.0"
 slog-json = { version = "2.3.0", optional = true }
 stacks-common = { path = "../stacks-common" }
 stackslib = { path = "../stackslib"}
-thiserror = "1.0"
+thiserror = { workspace = true }
 tiny_http = "0.12"
 
 [dev-dependencies]

libsigner/Cargo.toml

@@ -1,5 +1,6 @@
 use std::cmp::Ordering;
 use std::fmt;
+use std::ops::{Deref, DerefMut, Index, IndexMut};
 
 #[cfg(feature = "canonical")]
 pub mod sqlite;
@@ -460,3 +461,83 @@ impl<L: PartialEq + Eq> Ord for StacksEpoch<L> {
         self.epoch_id.cmp(&other.epoch_id)
     }
 }
+
+/// A wrapper for holding a list of Epochs, indexable by StacksEpochId
+#[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq)]
+pub struct EpochList<L: Clone>(Vec<StacksEpoch<L>>);
+
+impl<L: Clone> EpochList<L> {
+    pub fn new(epochs: &[StacksEpoch<L>]) -> EpochList<L> {
+        EpochList(epochs.to_vec())
+    }
+
+    pub fn get(&self, index: StacksEpochId) -> Option<&StacksEpoch<L>> {
+        self.0.get(StacksEpoch::find_epoch_by_id(&self.0, index)?)
+    }
+
+    pub fn get_mut(&mut self, index: StacksEpochId) -> Option<&mut StacksEpoch<L>> {
+        let index = StacksEpoch::find_epoch_by_id(&self.0, index)?;
+        self.0.get_mut(index)
+    }
+
+    /// Truncates the list after the given epoch id
+    pub fn truncate_after(&mut self, epoch_id: StacksEpochId) {
+        if let Some(index) = StacksEpoch::find_epoch_by_id(&self.0, epoch_id) {
+            self.0.truncate(index + 1);
+        }
+    }
+
+    /// Determine which epoch, if any, a given burnchain height falls into.
+    pub fn epoch_id_at_height(&self, height: u64) -> Option<StacksEpochId> {
+        StacksEpoch::find_epoch(self, height).map(|idx| self.0[idx].epoch_id)
+    }
+
+    /// Determine which epoch, if any, a given burnchain height falls into.
+    pub fn epoch_at_height(&self, height: u64) -> Option<StacksEpoch<L>> {
+        StacksEpoch::find_epoch(self, height).map(|idx| self.0[idx].clone())
+    }
+
+    /// Pushes a new `StacksEpoch` to the end of the list
+    pub fn push(&mut self, epoch: StacksEpoch<L>) {
+        if let Some(last) = self.0.last() {
+            assert!(
+                epoch.start_height == last.end_height && epoch.epoch_id > last.epoch_id,
+                "Epochs must be pushed in order"
+            );
+        }
+        self.0.push(epoch);
+    }
+
+    pub fn to_vec(&self) -> Vec<StacksEpoch<L>> {
+        self.0.clone()
+    }
+}
+
+impl<L: Clone> Index<StacksEpochId> for EpochList<L> {
+    type Output = StacksEpoch<L>;
+    fn index(&self, index: StacksEpochId) -> &StacksEpoch<L> {
+        self.get(index)
+            .expect("Invalid StacksEpochId: could not find corresponding epoch")
+    }
+}
+
+impl<L: Clone> IndexMut<StacksEpochId> for EpochList<L> {
+    fn index_mut(&mut self, index: StacksEpochId) -> &mut StacksEpoch<L> {
+        self.get_mut(index)
+            .expect("Invalid StacksEpochId: could not find corresponding epoch")
+    }
+}
+
+impl<L: Clone> Deref for EpochList<L> {
+    type Target = [StacksEpoch<L>];
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<L: Clone> DerefMut for EpochList<L> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}

stacks-common/src/types/mod.rs

@@ -51,26 +51,25 @@ pub fn update_lock_table(conn: &Connection) {
 /// Called by `rusqlite` if we are waiting too long on a database lock
 /// If called too many times, will assume a deadlock and panic
 pub fn tx_busy_handler(run_count: i32) -> bool {
-    const TIMEOUT: Duration = Duration::from_secs(300);
     const AVG_SLEEP_TIME_MS: u64 = 100;
 
+    // Every ~5min, report an error with a backtrace
+    //   5min * 60s/min * 1_000ms/s / 100ms
+    const ERROR_COUNT: u32 = 3_000;
+
     // First, check if this is taking unreasonably long. If so, it's probably a deadlock
     let run_count = run_count.unsigned_abs();
-    let approx_time_elapsed =
-        Duration::from_millis(AVG_SLEEP_TIME_MS.saturating_mul(u64::from(run_count)));
-    if approx_time_elapsed > TIMEOUT {
-        error!("Deadlock detected. Waited {} seconds (estimated) for database lock. Giving up", approx_time_elapsed.as_secs();
+    if run_count > 0 && run_count % ERROR_COUNT == 0 {
+        error!("Deadlock detected. Waited 5 minutes (estimated) for database lock.";
             "run_count" => run_count,
             "backtrace" => ?Backtrace::capture()
         );
         for (k, v) in LOCK_TABLE.lock().unwrap().iter() {
             error!("Database '{k}' last locked by {v}");
         }
-        panic!("Deadlock in thread {:?}", thread::current().name());
     }
 
     let mut sleep_time_ms = 2u64.saturating_pow(run_count);
-
     sleep_time_ms = sleep_time_ms.saturating_add(thread_rng().gen_range(0..sleep_time_ms));
 
     if sleep_time_ms > AVG_SLEEP_TIME_MS {