Repository: stacks-network/stacks-core (language: python)

Issue:
/v3/sortitions bad optimization for `last_sortition_ch`
`last_sortition_ch` is wrong in the `/v3/sortitions` endpoint. My recent consolidation PR changed this optimization:

```rust
// optimization heuristic: short-circuit the load if its just `stacks_parent_sn`
let last_sortition_ch = if sortition_sn.num_sortitions == stacks_parent_sn.num_sortitions + 1 {
    stacks_parent_sn.consensus_hash.clone()
```

To this:

```rust
// optimization heuristic: short-circuit the load if its just `stacks_parent_sn`
let last_sortition_ch = if stacks_parent_sn.sortition {
    stacks_parent_sn.consensus_hash.clone()
```

This is just wrong, and it leads to signers rejecting some proposals that they shouldn't (and _accepting_ some proposals that they shouldn't).


Tenure extension bug in naka-miner logic
Fixed in 541d13b4cc69a15ae3767499e19e99455064506e. I'm _pretty sure_ I'm seeing the behavior in nakamoto testnet (which I don't think is running that patch), but I need to investigate further to confirm:


```
  |   | 2024-09-27T15:22:26.613746583Z - Cycle #80 Signer #1: Block proposal invalid |  
  |   | 2024-09-27T15:22:26.61367519Z - Miner block proposal contains a tenure change, but we've already signed a block in this tenure. Considering proposal invalid. |  
  |   | 2024-09-27T15:22:26.611946469Z - Cycle #80 Signer #0: Block proposal invalid |  
  |   | 2024-09-27T15:22:26.611832519Z - Miner block proposal contains a tenure change, but we've already signed a block in this tenure. Considering proposal invalid. |  
  |   | 2024-09-27T15:22:26.611059779Z - Cycle #80 Signer #3: Block proposal invalid |  
  |   | 2024-09-27T15:22:26.610965315Z - Miner block proposal contains a tenure change, but we've already signed a block in this tenure. Considering proposal invalid.
```




Discussion / hints:

Fix patch:
diff --git a/stackslib/src/net/api/getsortition.rs b/stackslib/src/net/api/getsortition.rs
index 7b594530c2..ccfa4efec9 100644
--- a/stackslib/src/net/api/getsortition.rs
+++ b/stackslib/src/net/api/getsortition.rs
@@ -182,7 +182,11 @@ impl GetSortitionHandler {
                 // try to figure out what the last snapshot in this fork was with a successful
                 //  sortition.
                 // optimization heuristic: short-circuit the load if its just `stacks_parent_sn`
-                let last_sortition_ch = if stacks_parent_sn.sortition {
+                //   if the sortition count incremented by exactly 1 between us and our **stacks** parent,
+                //   then the stacks parent's sortition *must* be the last one with a winner.
+                let sortitions_incremented_by_1 =
+                    sortition_sn.num_sortitions == stacks_parent_sn.num_sortitions + 1;
+                let last_sortition_ch = if sortitions_incremented_by_1 {
                     stacks_parent_sn.consensus_hash.clone()
                 } else {
                     // we actually need to perform the marf lookup


Test patch:
diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs
index 4d9d4f968e..850fffaab6 100644
--- a/testnet/stacks-node/src/nakamoto_node/miner.rs
+++ b/testnet/stacks-node/src/nakamoto_node/miner.rs
@@ -1123,6 +1123,12 @@ impl BlockMinerThread {
                 tenure_change_tx: None,
             });
         };
+        if self.last_block_mined.is_some() {
+            return Ok(NakamotoTenureInfo {
+                coinbase_tx: None,
+                tenure_change_tx: None,
+            });
+        }
 
         let parent_block_id = parent_block_info.stacks_parent_header.index_block_hash();
         let mut payload = TenureChangePayload {
@@ -1152,10 +1158,10 @@ impl BlockMinerThread {
                     &parent_block_id,
                 )
                 .map_err(NakamotoNodeError::MiningFailure)?;
-                debug!("Miner: Extending tenure";
-                    "burn_view_consensus_hash" => %burn_view_consensus_hash,
-                    "parent_block_id" => %parent_block_id,
-                    "num_blocks_so_far" => num_blocks_so_far,
+                info!("Miner: Extending tenure";
+                      "burn_view_consensus_hash" => %burn_view_consensus_hash,
+                      "parent_block_id" => %parent_block_id,
+                      "num_blocks_so_far" => num_blocks_so_far,
                 );
                 payload = payload.extend(
                     *burn_view_consensus_hash,
diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs
index 3c238153ac..717e33c9c9 100644
--- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs
+++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs
@@ -8460,8 +8460,10 @@ fn mock_mining() {
     let mock_mining_blocks_end = follower_naka_mined_blocks.load(Ordering::SeqCst);
     let blocks_mock_mined = mock_mining_blocks_end - mock_mining_blocks_start;
     assert!(
-        blocks_mock_mined > tenure_count,
-        "Should have mock mined at least `tenure_count` nakamoto blocks"
+        blocks_mock_mined >= tenure_count,
+        "Should have mock mined at least `tenure_count` nakamoto blocks. Mined = {}. Expected = {}",
+        blocks_mock_mined,
+        tenure_count,
     );
 
     // wait for follower to reach the chain tip