Merge branch 'develop' of https://github.com/stacks-network/stacks-core into bug/validation-queue-race-condition

Author: jferrant

.github/actions/dockerfiles/Dockerfile.debian-source

@@ -24,5 +24,5 @@ RUN --mount=type=tmpfs,target=${BUILD_DIR} cp -R /src/. ${BUILD_DIR}/ \
     && cp -R ${BUILD_DIR}/target/${TARGET}/release/. /out
 
 FROM --platform=${TARGETPLATFORM} debian:bookworm
-COPY --from=build /out/stacks-node /out/stacks-signer /bin/
+COPY --from=build /out/stacks-node /out/stacks-signer /out/stacks-inspect /bin/
 CMD ["stacks-node", "mainnet"]

.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
@@ -139,6 +140,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::test_shadow_recovery
           - tests::nakamoto_integrations::signer_chainstate
           - tests::nakamoto_integrations::clarity_cost_spend_down
           - tests::nakamoto_integrations::v3_blockbyheight_api_endpoint

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-common/src/types/mod.rs

@@ -170,6 +170,21 @@ impl StacksEpochId {
         }
     }
 
+    /// Whether or not this epoch supports shadow blocks
+    pub fn supports_shadow_blocks(&self) -> bool {
+        match self {
+            StacksEpochId::Epoch10
+            | StacksEpochId::Epoch20
+            | StacksEpochId::Epoch2_05
+            | StacksEpochId::Epoch21
+            | StacksEpochId::Epoch22
+            | StacksEpochId::Epoch23
+            | StacksEpochId::Epoch24
+            | StacksEpochId::Epoch25 => false,
+            StacksEpochId::Epoch30 => true,
+        }
+    }
+
     /// Does this epoch support unlocking PoX contributors that miss a slot?
     ///
     /// Epoch 2.0 - 2.05 didn't support this feature, but they weren't epoch-guarded on it. Instead,

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

@@ -328,6 +328,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,
@@ -425,9 +430,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
@@ -447,7 +457,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();
@@ -499,6 +509,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).
@@ -510,7 +534,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,
@@ -620,6 +645,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,
@@ -1263,4 +1297,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/burnchains/mod.rs

@@ -450,6 +450,7 @@ impl PoxConstants {
         )
     }
 
+    // NOTE: this is the *old* pre-Nakamoto testnet
     pub fn testnet_default() -> PoxConstants {
         PoxConstants::new(
             POX_REWARD_CYCLE_LENGTH / 2,   // 1050
@@ -468,6 +469,10 @@ impl PoxConstants {
         ) // total liquid supply is 40000000000000000 µSTX
     }
 
+    pub fn nakamoto_testnet_default() -> PoxConstants {
+        PoxConstants::new(900, 100, 51, 100, 0, u64::MAX, u64::MAX, 242, 243, 246, 244)
+    }
+
     // TODO: add tests from mutation testing results #4838
     #[cfg_attr(test, mutants::skip)]
     pub fn regtest_default() -> PoxConstants {

stackslib/src/burnchains/tests/mod.rs

@@ -351,10 +351,30 @@ impl TestMinerFactory {
 
 impl TestBurnchainBlock {
     pub fn new(parent_snapshot: &BlockSnapshot, fork_id: u64) -> TestBurnchainBlock {
+        let burn_header_hash = BurnchainHeaderHash::from_test_data(
+            parent_snapshot.block_height + 1,
+            &parent_snapshot.index_root,
+            fork_id,
+        );
         TestBurnchainBlock {
             parent_snapshot: parent_snapshot.clone(),
             block_height: parent_snapshot.block_height + 1,
-            txs: vec![],
+            txs: vec![
+                // make sure that no block-commit gets vtxindex == 0 unless explicitly structured.
+                // This prestx mocks a burnchain coinbase
+                BlockstackOperationType::PreStx(PreStxOp {
+                    output: StacksAddress::burn_address(false),
+                    txid: Txid::from_test_data(
+                        parent_snapshot.block_height + 1,
+                        0,
+                        &burn_header_hash,
+                        128,
+                    ),
+                    vtxindex: 0,
+                    block_height: parent_snapshot.block_height + 1,
+                    burn_header_hash,
+                }),
+            ],
             fork_id: fork_id,
             timestamp: get_epoch_time_secs(),
         }
@@ -397,6 +417,7 @@ impl TestBurnchainBlock {
         parent_block_snapshot: Option<&BlockSnapshot>,
         new_seed: Option<VRFSeed>,
         epoch_marker: u8,
+        parent_is_shadow: bool,
     ) -> LeaderBlockCommitOp {
         let pubks = miner
             .privks
@@ -435,6 +456,13 @@ impl TestBurnchainBlock {
         )
         .expect("FATAL: failed to read block commit");
 
+        if parent_is_shadow {
+            assert!(
+                get_commit_res.is_none(),
+                "FATAL: shadow parent should not have a block-commit"
+            );
+        }
+
         let input = SortitionDB::get_last_block_commit_by_sender(ic.conn(), &apparent_sender)
             .unwrap()
             .map(|commit| (commit.txid.clone(), 1 + (commit.commit_outs.len() as u32)))
@@ -454,7 +482,8 @@ impl TestBurnchainBlock {
                     block_hash,
                     self.block_height,
                     &new_seed,
-                    &parent,
+                    parent.block_height as u32,
+                    parent.vtxindex as u16,
                     leader_key.block_height as u32,
                     leader_key.vtxindex as u16,
                     burn_fee,
@@ -464,16 +493,42 @@ impl TestBurnchainBlock {
                 txop
             }
             None => {
-                // initial
-                let txop = LeaderBlockCommitOp::initial(
-                    block_hash,
-                    self.block_height,
-                    &new_seed,
-                    leader_key,
-                    burn_fee,
-                    &input,
-                    &apparent_sender,
-                );
+                let txop = if parent_is_shadow {
+                    test_debug!(
+                        "Block-commit for {} (burn height {}) builds on shadow sortition",
+                        block_hash,
+                        self.block_height
+                    );
+
+                    LeaderBlockCommitOp::new(
+                        block_hash,
+                        self.block_height,
+                        &new_seed,
+                        last_snapshot_with_sortition.block_height as u32,
+                        0,
+                        leader_key.block_height as u32,
+                        leader_key.vtxindex as u16,
+                        burn_fee,
+                        &input,
+                        &apparent_sender,
+                    )
+                } else {
+                    // initial
+                    test_debug!(
+                        "Block-commit for {} (burn height {}) builds on genesis",
+                        block_hash,
+                        self.block_height,
+                    );
+                    LeaderBlockCommitOp::initial(
+                        block_hash,
+                        self.block_height,
+                        &new_seed,
+                        leader_key,
+                        burn_fee,
+                        &input,
+                        &apparent_sender,
+                    )
+                };
                 txop
             }
         };
@@ -517,6 +572,7 @@ impl TestBurnchainBlock {
             parent_block_snapshot,
             None,
             STACKS_EPOCH_2_4_MARKER,
+            false,
         )
     }