Merge pull request #5452 from stacks-network/feat/tenure-extend-no-blocks

Tenure extend when the previous tenure is bad

Author: obycode

.github/workflows/bitcoin-tests.yml

@@ -126,6 +126,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::tenure_extend_after_bad_commit
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 - 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.
 - Add `tenure_cost_limit_per_block_percentage` to the miner config file to control the percentage remaining tenure cost limit to consume per nakamoto block.
 - Add `/v3/blocks/height/:block_height` rpc endpoint
+- If the winning miner of a sortition is committed to the wrong parent tenure, the previous miner can immediately tenure extend and continue mining since the winning miner would never be able to propose a valid block. (#5361)
 
 ## [3.0.0.0.1]
 

stacks-signer/CHANGELOG.md

@@ -11,6 +11,8 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ### Changed
 
+- Allow a miner to extend their tenure immediately if the winner of the next tenure has committed to the wrong parent tenure (#5361)
+
 ## [3.0.0.0.1.0]
 
 ### Changed

stacks-signer/src/chainstate.rs

@@ -203,7 +203,40 @@ impl SortitionsView {
                 "current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
             );
             self.cur_sortition.miner_status = SortitionMinerStatus::InvalidatedBeforeFirstBlock;
+        } else if let Some(tip) = signer_db.get_canonical_tip()? {
+            // Check if the current sortition is aligned with the expected tenure:
+            // - If the tip is in the current tenure, we are in the process of mining this tenure.
+            // - If the tip is not in the current tenure, then we’re starting a new tenure,
+            //   and the current sortition's parent tenure must match the tenure of the tip.
+            // - If the tip is not building off of the current sortition's parent tenure, then
+            //   check to see if the tip's parent is within the first proposal burn block timeout,
+            //   which allows for forks when a burn block arrives quickly.
+            // - Else the miner of the current sortition has committed to an incorrect parent tenure.
+            let consensus_hash_match =
+                self.cur_sortition.consensus_hash == tip.block.header.consensus_hash;
+            let parent_tenure_id_match =
+                self.cur_sortition.parent_tenure_id == tip.block.header.consensus_hash;
+            if !consensus_hash_match && !parent_tenure_id_match {
+                // More expensive check, so do it only if we need to.
+                let is_valid_parent_tenure = Self::check_parent_tenure_choice(
+                    &self.cur_sortition,
+                    block,
+                    signer_db,
+                    client,
+                    &self.config.first_proposal_burn_block_timing,
+                )?;
+                if !is_valid_parent_tenure {
+                    warn!(
+                        "Current sortition does not build off of canonical tip tenure, marking as invalid";
+                        "current_sortition_parent" => ?self.cur_sortition.parent_tenure_id,
+                        "tip_consensus_hash" => ?tip.block.header.consensus_hash,
+                    );
+                    self.cur_sortition.miner_status =
+                        SortitionMinerStatus::InvalidatedBeforeFirstBlock;
+                }
+            }
         }
+
         if let Some(last_sortition) = self.last_sortition.as_mut() {
             if last_sortition.is_timed_out(self.config.block_proposal_timeout, signer_db)? {
                 info!(
@@ -304,6 +337,7 @@ impl SortitionsView {
                         "Miner block proposal is from last sortition winner, when the new sortition winner is still valid. Considering proposal invalid.";
                         "proposed_block_consensus_hash" => %block.header.consensus_hash,
                         "proposed_block_signer_sighash" => %block.header.signer_signature_hash(),
+                        "current_sortition_miner_status" => ?self.cur_sortition.miner_status,
                     );
                     return Ok(false);
                 }
@@ -439,6 +473,8 @@ impl SortitionsView {
                             "violating_tenure_proposed_time" => local_block_info.proposed_time,
                             "new_tenure_received_time" => sortition_state_received_time,
                             "new_tenure_burn_timestamp" => sortition_state.burn_header_timestamp,
+                            "first_proposal_burn_block_timing_secs" => first_proposal_burn_block_timing.as_secs(),
+                            "proposal_to_sortition" => proposal_to_sortition,
                         );
                         continue;
                     }

stacks-signer/src/signerdb.rs

@@ -324,6 +324,11 @@ static CREATE_INDEXES_3: &str = r#"
 CREATE INDEX IF NOT EXISTS block_rejection_signer_addrs_on_block_signature_hash ON block_rejection_signer_addrs(signer_signature_hash);
 "#;
 
+static CREATE_INDEXES_4: &str = r#"
+CREATE INDEX IF NOT EXISTS blocks_state ON blocks ((json_extract(block_info, '$.state')));
+CREATE INDEX IF NOT EXISTS blocks_signed_group ON blocks ((json_extract(block_info, '$.signed_group')));
+"#;
+
 static CREATE_SIGNER_STATE_TABLE: &str = "
 CREATE TABLE IF NOT EXISTS signer_states (
     reward_cycle INTEGER PRIMARY KEY,
@@ -421,9 +426,14 @@ static SCHEMA_3: &[&str] = &[
     "INSERT INTO db_config (version) VALUES (3);",
 ];
 
+static SCHEMA_4: &[&str] = &[
+    CREATE_INDEXES_4,
+    "INSERT OR REPLACE INTO db_config (version) VALUES (4);",
+];
+
 impl SignerDb {
     /// The current schema version used in this build of the signer binary.
-    pub const SCHEMA_VERSION: u32 = 3;
+    pub const SCHEMA_VERSION: u32 = 4;
 
     /// Create a new `SignerState` instance.
     /// This will create a new SQLite database at the given path
@@ -443,7 +453,7 @@ impl SignerDb {
             return Ok(0);
         }
         let result = conn
-            .query_row("SELECT version FROM db_config LIMIT 1", [], |row| {
+            .query_row("SELECT MAX(version) FROM db_config LIMIT 1", [], |row| {
                 row.get(0)
             })
             .optional();
@@ -495,6 +505,20 @@ impl SignerDb {
         Ok(())
     }
 
+    /// Migrate from schema 3 to schema 4
+    fn schema_4_migration(tx: &Transaction) -> Result<(), DBError> {
+        if Self::get_schema_version(tx)? >= 4 {
+            // no migration necessary
+            return Ok(());
+        }
+
+        for statement in SCHEMA_4.iter() {
+            tx.execute_batch(statement)?;
+        }
+
+        Ok(())
+    }
+
     /// Either instantiate a new database, or migrate an existing one
     /// If the detected version of the existing database is 0 (i.e., a pre-migration
     /// logic DB, the DB will be dropped).
@@ -506,7 +530,8 @@ impl SignerDb {
                 0 => Self::schema_1_migration(&sql_tx)?,
                 1 => Self::schema_2_migration(&sql_tx)?,
                 2 => Self::schema_3_migration(&sql_tx)?,
-                3 => break,
+                3 => Self::schema_4_migration(&sql_tx)?,
+                4 => break,
                 x => return Err(DBError::Other(format!(
                     "Database schema is newer than supported by this binary. Expected version = {}, Database version = {x}",
                     Self::SCHEMA_VERSION,
@@ -616,6 +641,15 @@ impl SignerDb {
         try_deserialize(result)
     }
 
+    /// Return the canonical tip -- the last globally accepted block.
+    pub fn get_canonical_tip(&self) -> Result<Option<BlockInfo>, DBError> {
+        let query = "SELECT block_info FROM blocks WHERE json_extract(block_info, '$.state') = ?1 ORDER BY stacks_height DESC, json_extract(block_info, '$.signed_group') DESC LIMIT 1";
+        let args = params![&BlockState::GloballyAccepted.to_string()];
+        let result: Option<String> = query_row(&self.db, query, args)?;
+
+        try_deserialize(result)
+    }
+
     /// Insert or replace a burn block into the database
     pub fn insert_burn_block(
         &mut self,
@@ -1242,4 +1276,45 @@ mod tests {
         assert!(!block.check_state(BlockState::GloballyAccepted));
         assert!(block.check_state(BlockState::GloballyRejected));
     }
+
+    #[test]
+    fn test_get_canonical_tip() {
+        let db_path = tmp_db_path();
+        let mut db = SignerDb::new(db_path).expect("Failed to create signer db");
+
+        let (mut block_info_1, _block_proposal_1) = create_block_override(|b| {
+            b.block.header.miner_signature = MessageSignature([0x01; 65]);
+            b.block.header.chain_length = 1;
+            b.burn_height = 1;
+        });
+
+        let (mut block_info_2, _block_proposal_2) = create_block_override(|b| {
+            b.block.header.miner_signature = MessageSignature([0x02; 65]);
+            b.block.header.chain_length = 2;
+            b.burn_height = 2;
+        });
+
+        db.insert_block(&block_info_1)
+            .expect("Unable to insert block into db");
+        db.insert_block(&block_info_2)
+            .expect("Unable to insert block into db");
+
+        assert!(db.get_canonical_tip().unwrap().is_none());
+
+        block_info_1
+            .mark_globally_accepted()
+            .expect("Failed to mark block as globally accepted");
+        db.insert_block(&block_info_1)
+            .expect("Unable to insert block into db");
+
+        assert_eq!(db.get_canonical_tip().unwrap().unwrap(), block_info_1);
+
+        block_info_2
+            .mark_globally_accepted()
+            .expect("Failed to mark block as globally accepted");
+        db.insert_block(&block_info_2)
+            .expect("Unable to insert block into db");
+
+        assert_eq!(db.get_canonical_tip().unwrap().unwrap(), block_info_2);
+    }
 }

stackslib/src/chainstate/burn/db/sortdb.rs

@@ -3690,6 +3690,12 @@ impl SortitionDB {
             .try_into()
             .ok()
     }
+
+    /// Get the Stacks block ID for the canonical tip.
+    pub fn get_canonical_stacks_tip_block_id(&self) -> StacksBlockId {
+        let (ch, bh) = SortitionDB::get_canonical_stacks_chain_tip_hash(self.conn()).unwrap();
+        StacksBlockId::new(&ch, &bh)
+    }
 }
 
 impl<'a> SortitionDBTx<'a> {

testnet/stacks-node/src/event_dispatcher.rs

@@ -18,7 +18,7 @@ use std::collections::hash_map::Entry;
 use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
 use std::sync::mpsc::{channel, Receiver, Sender};
-use std::sync::Mutex;
+use std::sync::{Arc, Mutex};
 use std::thread::sleep;
 use std::time::Duration;
 
@@ -107,17 +107,8 @@ pub const PATH_BLOCK_PROCESSED: &str = "new_block";
 pub const PATH_ATTACHMENT_PROCESSED: &str = "attachments/new";
 pub const PATH_PROPOSAL_RESPONSE: &str = "proposal_response";
 
-pub static STACKER_DB_CHANNEL: StackerDBChannel = StackerDBChannel::new();
-
 /// This struct receives StackerDB event callbacks without registering
-/// over the JSON/RPC interface. To ensure that any event observer
-/// uses the same channel, we use a lazy_static global for the channel (this
-/// implements a singleton using STACKER_DB_CHANNEL).
-///
-/// This is in place because a Nakamoto miner needs to receive
-/// StackerDB events. It could either poll the database (seems like a
-/// bad idea) or listen for events. Registering for RPC callbacks
-/// seems bad. So instead, it uses a singleton sync channel.
+/// over the JSON/RPC interface.
 pub struct StackerDBChannel {
     sender_info: Mutex<Option<InnerStackerDBChannel>>,
 }
@@ -923,6 +914,8 @@ pub struct EventDispatcher {
     /// Index into `registered_observers` that will receive block proposal events (Nakamoto and
     /// later)
     block_proposal_observers_lookup: HashSet<u16>,
+    /// Channel for sending StackerDB events to the miner coordinator
+    pub stackerdb_channel: Arc<Mutex<StackerDBChannel>>,
 }
 
 /// This struct is used specifically for receiving proposal responses.
@@ -1115,6 +1108,7 @@ impl Default for EventDispatcher {
 impl EventDispatcher {
     pub fn new() -> EventDispatcher {
         EventDispatcher {
+            stackerdb_channel: Arc::new(Mutex::new(StackerDBChannel::new())),
             registered_observers: vec![],
             contract_events_observers_lookup: HashMap::new(),
             assets_observers_lookup: HashMap::new(),
@@ -1544,7 +1538,11 @@ impl EventDispatcher {
 
         let interested_observers = self.filter_observers(&self.stackerdb_observers_lookup, false);
 
-        let interested_receiver = STACKER_DB_CHANNEL.is_active(&contract_id);
+        let stackerdb_channel = self
+            .stackerdb_channel
+            .lock()
+            .expect("FATAL: failed to lock StackerDB channel mutex");
+        let interested_receiver = stackerdb_channel.is_active(&contract_id);
         if interested_observers.is_empty() && interested_receiver.is_none() {
             return;
         }

testnet/stacks-node/src/nakamoto_node.rs

@@ -28,6 +28,8 @@ use stacks::monitoring::update_active_miners_count_gauge;
 use stacks::net::atlas::AtlasConfig;
 use stacks::net::relay::Relayer;
 use stacks::net::stackerdb::StackerDBs;
+use stacks::net::Error as NetError;
+use stacks::util_lib::db::Error as DBError;
 use stacks_common::types::chainstate::SortitionId;
 use stacks_common::types::StacksEpochId;
 
@@ -71,46 +73,71 @@ pub struct StacksNode {
 }
 
 /// Types of errors that can arise during Nakamoto StacksNode operation
-#[derive(Debug)]
+#[derive(thiserror::Error, Debug)]
 pub enum Error {
     /// Can't find the block sortition snapshot for the chain tip
+    #[error("Can't find the block sortition snapshot for the chain tip")]
     SnapshotNotFoundForChainTip,
     /// The burnchain tip changed while this operation was in progress
+    #[error("The burnchain tip changed while this operation was in progress")]
     BurnchainTipChanged,
     /// The Stacks tip changed while this operation was in progress
+    #[error("The Stacks tip changed while this operation was in progress")]
     StacksTipChanged,
     /// Signers rejected a block
+    #[error("Signers rejected a block")]
     SignersRejected,
     /// Error while spawning a subordinate thread
+    #[error("Error while spawning a subordinate thread: {0}")]
     SpawnError(std::io::Error),
     /// Injected testing errors
+    #[error("Injected testing errors")]
     FaultInjection,
     /// This miner was elected, but another sortition occurred before mining started
+    #[error("This miner was elected, but another sortition occurred before mining started")]
     MissedMiningOpportunity,
     /// Attempted to mine while there was no active VRF key
+    #[error("Attempted to mine while there was no active VRF key")]
     NoVRFKeyActive,
     /// The parent block or tenure could not be found
+    #[error("The parent block or tenure could not be found")]
     ParentNotFound,
     /// Something unexpected happened (e.g., hash mismatches)
+    #[error("Something unexpected happened (e.g., hash mismatches)")]
     UnexpectedChainState,
     /// A burnchain operation failed when submitting it to the burnchain
+    #[error("A burnchain operation failed when submitting it to the burnchain: {0}")]
     BurnchainSubmissionFailed(BurnchainsError),
     /// A new parent has been discovered since mining started
+    #[error("A new parent has been discovered since mining started")]
     NewParentDiscovered,
     /// A failure occurred while constructing a VRF Proof
+    #[error("A failure occurred while constructing a VRF Proof")]
     BadVrfConstruction,
-    CannotSelfSign,
-    MiningFailure(ChainstateError),
+    #[error("A failure occurred while mining: {0}")]
+    MiningFailure(#[from] ChainstateError),
     /// The miner didn't accept their own block
+    #[error("The miner didn't accept their own block: {0}")]
     AcceptFailure(ChainstateError),
+    #[error("A failure occurred while signing a miner's block: {0}")]
     MinerSignatureError(&'static str),
+    #[error("A failure occurred while signing a signer's block: {0}")]
     SignerSignatureError(String),
     /// A failure occurred while configuring the miner thread
+    #[error("A failure occurred while configuring the miner thread: {0}")]
     MinerConfigurationFailed(&'static str),
     /// An error occurred while operating as the signing coordinator
+    #[error("An error occurred while operating as the signing coordinator: {0}")]
     SigningCoordinatorFailure(String),
     // The thread that we tried to send to has closed
+    #[error("The thread that we tried to send to has closed")]
     ChannelClosed,
+    /// DBError wrapper
+    #[error("DBError: {0}")]
+    DBError(#[from] DBError),
+    /// NetError wrapper
+    #[error("NetError: {0}")]
+    NetError(#[from] NetError),
 }
 
 impl StacksNode {