Merge branch 'fix/5044' of https://github.com/stacks-network/stacks-blockchain into fix/5044

Author: jcnelson

.github/workflows/bitcoin-tests.yml

@@ -75,6 +75,9 @@ jobs:
           - tests::neon_integrations::vote_for_aggregate_key_burn_op_test
           - tests::neon_integrations::mock_miner_replay
           - tests::neon_integrations::listunspent_max_utxos
+          - tests::neon_integrations::bitcoin_reorg_flap
+          - tests::neon_integrations::bitcoin_reorg_flap_with_follower
+          - tests::neon_integrations::start_stop_bitcoind
           - tests::epoch_25::microblocks_disabled
           - tests::should_succeed_handling_malformed_and_valid_txs
           - tests::nakamoto_integrations::simple_neon_integration
@@ -122,9 +125,6 @@ jobs:
           - tests::nakamoto_integrations::follower_bootup_across_multiple_cycles
           - tests::nakamoto_integrations::utxo_check_on_startup_panic
           - tests::nakamoto_integrations::utxo_check_on_startup_recover
-          # Do not run this one until we figure out why it fails in CI
-          # - tests::neon_integrations::bitcoin_reorg_flap
-          # - tests::neon_integrations::bitcoin_reorg_flap_with_follower
           # TODO: enable these once v1 signer is supported by a new nakamoto epoch
           # - tests::signer::v1::dkg
           # - tests::signer::v1::sign_request_rejected

docs/rpc/api/core-node/get_sortitions.example.json

@@ -0,0 +1,15 @@
+[
+  {
+    "burn_block_hash": "0x046f54cd1924a5d80fc3b8186d0334b7521acae90f9e136e2bee680c720d0e83",
+    "burn_block_height": 231,
+    "burn_header_timestamp": 1726797570,
+    "sortition_id": "0x8a5116b7b4306dc4f6db290d1adfff9e1347f3e921bb793fc4c33e2ff05056e2",
+    "parent_sortition_id": "0xdaf479110cf859e58c56b6ae941f8a14e7c7992c57027183dfbda4a4b820897c",
+    "consensus_hash": "0x8d2c51db737597a93191f49bcdc9c7bb44b90892",
+    "was_sortition": true,
+    "miner_pk_hash160": "0x6bc51b33e9f3626944eb879147e18111581f8f9b",
+    "stacks_parent_ch": "0x697357c72da55b759b1d6b721676c92c69f0b490",
+    "last_sortition_ch": "0x697357c72da55b759b1d6b721676c92c69f0b490",
+    "committed_block_hash": "0xeea47d6d639c565027110e192e308fb11656183d5c077bcd718d830652800183"
+  }
+]

docs/rpc/api/core-node/get_sortitions_latest_and_prior.example.json

@@ -0,0 +1,28 @@
+[
+  {
+    "burn_block_hash": "0x046f54cd1924a5d80fc3b8186d0334b7521acae90f9e136e2bee680c720d0e83",
+    "burn_block_height": 231,
+    "burn_header_timestamp": 1726797570,
+    "sortition_id": "0x8a5116b7b4306dc4f6db290d1adfff9e1347f3e921bb793fc4c33e2ff05056e2",
+    "parent_sortition_id": "0xdaf479110cf859e58c56b6ae941f8a14e7c7992c57027183dfbda4a4b820897c",
+    "consensus_hash": "0x8d2c51db737597a93191f49bcdc9c7bb44b90892",
+    "was_sortition": true,
+    "miner_pk_hash160": "0x6bc51b33e9f3626944eb879147e18111581f8f9b",
+    "stacks_parent_ch": "0x697357c72da55b759b1d6b721676c92c69f0b490",
+    "last_sortition_ch": "0x697357c72da55b759b1d6b721676c92c69f0b490",
+    "committed_block_hash": "0xeea47d6d639c565027110e192e308fb11656183d5c077bcd718d830652800183"
+  },
+  {
+    "burn_block_hash": "0x496ff02cb63a4850d0bdee5fab69284b6eb0392b4538e1c462f82362c5becfa4",
+    "burn_block_height": 230,
+    "burn_header_timestamp": 1726797570,
+    "sortition_id": "0xdaf479110cf859e58c56b6ae941f8a14e7c7992c57027183dfbda4a4b820897c",
+    "parent_sortition_id": "0xf9058692055cbd879d7f71e566e44b905a887b2b182407ed596b5d6499ceae2a",
+    "consensus_hash": "0x697357c72da55b759b1d6b721676c92c69f0b490",
+    "was_sortition": true,
+    "miner_pk_hash160": "0x6bc51b33e9f3626944eb879147e18111581f8f9b",
+    "stacks_parent_ch": "0xf7d1bd7d9d5c5a5c368402b6ef9510bd014d70f7",
+    "last_sortition_ch": "0xf7d1bd7d9d5c5a5c368402b6ef9510bd014d70f7",
+    "committed_block_hash": "0x36ee5f7f7271de1c1d4cd830e36320b51e01605547621267ae6e9b4e9b10f95e"
+  }
+]

docs/rpc/openapi.yaml

@@ -675,3 +675,44 @@ paths:
         schema:
           type: string
 
+  /v3/sortitions/{lookup_kind}/{lookup}:
+    get:
+      summary: Fetch information about evaluated burnchain blocks (i.e., sortitions).
+      tags:
+        - Blocks
+      operationId: get_sortitions
+      description:
+        Fetch sortition information about a burnchain block. If the `lookup_kind` and `lookup` parameters are empty, it will return information about the latest burn block.
+      responses:
+        "200":
+          description: Information for the burn block or in the case of `latest_and_last`, multiple burn blocks
+          content:
+            application/json:
+              examples:
+                Latest:
+                  description: A single element list is returned when just one sortition is requested
+                  value:
+                    $ref: ./api/core-node/get_sortitions.example.json
+                LatestAndLast:
+                  description: Sortition information about the latest burn block with a winning miner, and the previous such burn block.
+                  value:
+                    $ref: ./api/core-node/get_sortitions_latest_and_prior.example.json
+    parameters:
+      - name: lookup_kind
+        in: path
+        description: |-
+          The style of lookup that should be performed. If not given, the most recent burn block processed will be returned.
+          Otherwise, the `lookup_kind` should be one of the following strings:
+            * `consensus` - find the burn block using the consensus hash supplied in the `lookup` field.
+            * `burn_height` - find the burn block using the burn block height supplied in the `lookup` field.
+            * `burn` - find the burn block using the burn block hash supplied in the `lookup` field.
+            * `latest_and_last` - return information about the latest burn block with a winning miner *and* the previous such burn block
+        required: false
+        schema:
+          type: string
+      - name: lookup
+        in: path
+        description: The value to use for the lookup if `lookup_kind` is `consensus`, `burn_height`, or `burn`
+        required: false
+        schema:
+          type: string

stacks-signer/src/chainstate.rs

@@ -25,7 +25,7 @@ use stacks_common::types::chainstate::{ConsensusHash, StacksPublicKey};
 use stacks_common::util::hash::Hash160;
 use stacks_common::{info, warn};
 
-use crate::client::{ClientError, StacksClient};
+use crate::client::{ClientError, CurrentAndLastSortition, StacksClient};
 use crate::config::SignerConfig;
 use crate::signerdb::{BlockState, SignerDb};
 
@@ -138,8 +138,6 @@ pub struct SortitionsView {
     pub last_sortition: Option<SortitionState>,
     /// the current successful sortition (this corresponds to the "current" miner slot)
     pub cur_sortition: SortitionState,
-    /// the hash at which the sortitions view was fetched
-    pub latest_consensus_hash: ConsensusHash,
     /// configuration settings for evaluating proposals
     pub config: ProposalEvalConfig,
 }
@@ -608,42 +606,21 @@ impl SortitionsView {
         config: ProposalEvalConfig,
         client: &StacksClient,
     ) -> Result<Self, ClientError> {
-        let latest_state = client.get_latest_sortition()?;
-        let latest_ch = latest_state.consensus_hash;
-
-        // figure out what cur_sortition will be set to.
-        //  if the latest sortition wasn't successful, query the last one that was.
-        let latest_success = if latest_state.was_sortition {
-            latest_state
-        } else {
-            info!("Latest state wasn't a sortition: {latest_state:?}");
-            let last_sortition_ch = latest_state
-                .last_sortition_ch
-                .as_ref()
-                .ok_or_else(|| ClientError::NoSortitionOnChain)?;
-            client.get_sortition(last_sortition_ch)?
-        };
-
-        // now, figure out what `last_sortition` will be set to.
-        let last_sortition = latest_success
-            .last_sortition_ch
-            .as_ref()
-            .map(|ch| client.get_sortition(ch))
-            .transpose()?;
+        let CurrentAndLastSortition {
+            current_sortition,
+            last_sortition,
+        } = client.get_current_and_last_sortition()?;
 
-        let cur_sortition = SortitionState::try_from(latest_success)?;
+        let cur_sortition = SortitionState::try_from(current_sortition)?;
         let last_sortition = last_sortition
             .map(SortitionState::try_from)
             .transpose()
             .ok()
             .flatten();
 
-        let latest_consensus_hash = latest_ch;
-
         Ok(Self {
             cur_sortition,
             last_sortition,
-            latest_consensus_hash,
             config,
         })
     }

stacks-signer/src/client/stacks_client.rs

@@ -1,4 +1,3 @@
-use std::collections::VecDeque;
 // Copyright (C) 2013-2020 Blockstack PBC, a public benefit corporation
 // Copyright (C) 2020-2024 Stacks Open Internet Foundation
 //
@@ -14,6 +13,7 @@ use std::collections::VecDeque;
 //
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
+use std::collections::VecDeque;
 use std::net::SocketAddr;
 
 use blockstack_lib::burnchains::Txid;
@@ -88,6 +88,15 @@ struct GetStackersErrorResp {
     err_msg: String,
 }
 
+/// Result from fetching current and last sortition:
+///  two sortition infos
+pub struct CurrentAndLastSortition {
+    /// the latest winning sortition in the current burnchain fork
+    pub current_sortition: SortitionInfo,
+    /// the last winning sortition prior to `current_sortition`, if there was one
+    pub last_sortition: Option<SortitionInfo>,
+}
+
 impl From<&GlobalConfig> for StacksClient {
     fn from(config: &GlobalConfig) -> Self {
         Self {
@@ -484,10 +493,10 @@ impl StacksClient {
         Ok(tenures)
     }
 
-    /// Get the sortition information for the latest sortition
-    pub fn get_latest_sortition(&self) -> Result<SortitionInfo, ClientError> {
-        debug!("stacks_node_client: Getting latest sortition...");
-        let path = self.sortition_info_path();
+    /// Get the current winning sortition and the last winning sortition
+    pub fn get_current_and_last_sortition(&self) -> Result<CurrentAndLastSortition, ClientError> {
+        debug!("stacks_node_client: Getting current and prior sortition...");
+        let path = format!("{}/latest_and_last", self.sortition_info_path());
         let timer = crate::monitoring::new_rpc_call_timer(&path, &self.http_origin);
         let send_request = || {
             self.stacks_node_client.get(&path).send().map_err(|e| {
@@ -500,29 +509,29 @@ impl StacksClient {
         if !response.status().is_success() {
             return Err(ClientError::RequestFailure(response.status()));
         }
-        let sortition_info = response.json()?;
-        Ok(sortition_info)
-    }
-
-    /// Get the sortition information for a given sortition
-    pub fn get_sortition(&self, ch: &ConsensusHash) -> Result<SortitionInfo, ClientError> {
-        debug!("stacks_node_client: Getting sortition with consensus hash {ch}...");
-        let path = format!("{}/consensus/{}", self.sortition_info_path(), ch.to_hex());
-        let timer_label = format!("{}/consensus/:consensus_hash", self.sortition_info_path());
-        let timer = crate::monitoring::new_rpc_call_timer(&timer_label, &self.http_origin);
-        let send_request = || {
-            self.stacks_node_client.get(&path).send().map_err(|e| {
-                warn!("Signer failed to request sortition"; "consensus_hash" => %ch, "err" => ?e);
-                e
-            })
+        let mut info_list: VecDeque<SortitionInfo> = response.json()?;
+        let Some(current_sortition) = info_list.pop_front() else {
+            return Err(ClientError::UnexpectedResponseFormat(
+                "Empty SortitionInfo returned".into(),
+            ));
         };
-        let response = send_request()?;
-        timer.stop_and_record();
-        if !response.status().is_success() {
-            return Err(ClientError::RequestFailure(response.status()));
+        if !current_sortition.was_sortition {
+            return Err(ClientError::UnexpectedResponseFormat(
+                "'Current' SortitionInfo returned which was not a winning sortition".into(),
+            ));
         }
-        let sortition_info = response.json()?;
-        Ok(sortition_info)
+        let last_sortition = if current_sortition.last_sortition_ch.is_some() {
+            let Some(last_sortition) = info_list.pop_back() else {
+                return Err(ClientError::UnexpectedResponseFormat("'Current' SortitionInfo has `last_sortition_ch` field, but corresponding data not returned".into()));
+            };
+            Some(last_sortition)
+        } else {
+            None
+        };
+        Ok(CurrentAndLastSortition {
+            current_sortition,
+            last_sortition,
+        })
     }
 
     /// Get the current peer info data from the stacks node
@@ -590,9 +599,10 @@ impl StacksClient {
                 .map_err(|e| backoff::Error::transient(e.into()))?;
             let status = response.status();
             if status.is_success() {
-                return response
-                    .json()
-                    .map_err(|e| backoff::Error::permanent(e.into()));
+                return response.json().map_err(|e| {
+                    warn!("Failed to parse the GetStackers response: {e}");
+                    backoff::Error::permanent(e.into())
+                });
             }
             let error_data = response.json::<GetStackersErrorResp>().map_err(|e| {
                 warn!("Failed to parse the GetStackers error response: {e}");

stacks-signer/src/tests/chainstate.rs

@@ -82,7 +82,6 @@ fn setup_test_environment(
     });
 
     let view = SortitionsView {
-        latest_consensus_hash: cur_sortition.consensus_hash,
         cur_sortition,
         last_sortition,
         config: ProposalEvalConfig {

stackslib/src/chainstate/stacks/miner.rs

@@ -2228,10 +2228,10 @@ impl StacksBlockBuilder {
 
         debug!("Block transaction selection begins (parent height = {tip_height})");
         let result = {
-            let mut intermediate_result: Result<_, Error> = Ok(0);
+            let mut loop_result = Ok(());
             while block_limit_hit != BlockLimitFunction::LIMIT_REACHED {
                 let mut num_considered = 0;
-                intermediate_result = mempool.iterate_candidates(
+                let intermediate_result = mempool.iterate_candidates(
                     epoch_tx,
                     &mut tx_events,
                     mempool_settings.clone(),
@@ -2390,16 +2390,27 @@ impl StacksBlockBuilder {
                     let _ = mempool.drop_and_blacklist_txs(&to_drop_and_blacklist);
                 }
 
-                if intermediate_result.is_err() {
-                    break;
+                match intermediate_result {
+                    Err(e) => {
+                        loop_result = Err(e);
+                        break;
+                    }
+                    Ok((_txs_considered, stop_reason)) => {
+                        match stop_reason {
+                            MempoolIterationStopReason::NoMoreCandidates => break,
+                            MempoolIterationStopReason::DeadlineReached => break,
+                            // if the iterator function exited, let the loop tick: it checks the block limits
+                            MempoolIterationStopReason::IteratorExited => {}
+                        }
+                    }
                 }
 
                 if num_considered == 0 {
                     break;
                 }
             }
             debug!("Block transaction selection finished (parent height {}): {} transactions selected ({} considered)", &tip_height, num_txs, considered.len());
-            intermediate_result
+            loop_result
         };
 
         mempool.drop_txs(&invalidated_txs)?;

stackslib/src/chainstate/stacks/tests/block_construction.rs

@@ -5072,6 +5072,7 @@ fn paramaterized_mempool_walk_test(
                         },
                     )
                     .unwrap()
+                    .0
                     == 0
                 {
                     break;