Merge branch 'develop' into chore/update-docs_1721318026

Author: kantai

.github/workflows/bitcoin-tests.yml

@@ -32,7 +32,6 @@ jobs:
           - tests::bitcoin_regtest::bitcoind_integration_test
           - tests::integrations::integration_test_get_info
           - tests::neon_integrations::antientropy_integration_test
-          - tests::neon_integrations::bad_microblock_pubkey
           - tests::neon_integrations::bitcoind_forking_test
           - tests::neon_integrations::bitcoind_integration_test
           - tests::neon_integrations::block_large_tx_integration_test
@@ -43,21 +42,26 @@ jobs:
           - tests::neon_integrations::fuzzed_median_fee_rate_estimation_test_window10
           - tests::neon_integrations::fuzzed_median_fee_rate_estimation_test_window5
           - tests::neon_integrations::liquid_ustx_integration
-          - tests::neon_integrations::microblock_fork_poison_integration_test
-          - tests::neon_integrations::microblock_integration_test
+          # Microblock tests that are no longer needed on every CI run
+          #  (microblocks are unsupported starting in Epoch 2.5)
+          # - tests::neon_integrations::bad_microblock_pubkey
+          # - tests::neon_integrations::microblock_fork_poison_integration_test
+          # - tests::neon_integrations::microblock_integration_test
+          # - tests::neon_integrations::microblock_limit_hit_integration_test
+          # - tests::neon_integrations::test_problematic_microblocks_are_not_mined
+          # - tests::neon_integrations::test_problematic_microblocks_are_not_relayed_or_stored
+          # - tests::neon_integrations::size_overflow_unconfirmed_invalid_stream_microblocks_integration_test
+          # - tests::neon_integrations::size_overflow_unconfirmed_microblocks_integration_test
+          # - tests::neon_integrations::size_overflow_unconfirmed_stream_microblocks_integration_test
+          # - tests::neon_integrations::runtime_overflow_unconfirmed_microblocks_integration_test
           # Disable this flaky test. Microblocks are no longer supported anyways.
           # - tests::neon_integrations::microblock_large_tx_integration_test_FLAKY
-          - tests::neon_integrations::microblock_limit_hit_integration_test
           - tests::neon_integrations::miner_submit_twice
           - tests::neon_integrations::mining_events_integration_test
           - tests::neon_integrations::pox_integration_test
           - tests::neon_integrations::push_boot_receipts
-          - tests::neon_integrations::runtime_overflow_unconfirmed_microblocks_integration_test
           - tests::neon_integrations::should_fix_2771
           - tests::neon_integrations::size_check_integration_test
-          - tests::neon_integrations::size_overflow_unconfirmed_invalid_stream_microblocks_integration_test
-          - tests::neon_integrations::size_overflow_unconfirmed_microblocks_integration_test
-          - tests::neon_integrations::size_overflow_unconfirmed_stream_microblocks_integration_test
           - tests::neon_integrations::stx_delegate_btc_integration_test
           - tests::neon_integrations::stx_transfer_btc_integration_test
           - tests::neon_integrations::stack_stx_burn_op_test
@@ -66,8 +70,6 @@ jobs:
           - tests::neon_integrations::test_flash_block_skip_tenure
           - tests::neon_integrations::test_problematic_blocks_are_not_mined
           - tests::neon_integrations::test_problematic_blocks_are_not_relayed_or_stored
-          - tests::neon_integrations::test_problematic_microblocks_are_not_mined
-          - tests::neon_integrations::test_problematic_microblocks_are_not_relayed_or_stored
           - tests::neon_integrations::test_problematic_txs_are_not_stored
           - tests::neon_integrations::use_latest_tip_integration_test
           - tests::neon_integrations::confirm_unparsed_ongoing_ops
@@ -81,7 +83,7 @@ jobs:
           - tests::epoch_25::microblocks_disabled
           - tests::should_succeed_handling_malformed_and_valid_txs
           - tests::nakamoto_integrations::simple_neon_integration
-          - tests::nakamoto_integrations::simple_neon_integration_with_flash_blocks_on_epoch_3
+          - tests::nakamoto_integrations::flash_blocks_on_epoch_3
           - tests::nakamoto_integrations::mine_multiple_per_tenure_integration
           - tests::nakamoto_integrations::block_proposal_api_endpoint
           - tests::nakamoto_integrations::miner_writes_proposed_block_to_stackerdb
@@ -90,6 +92,7 @@ jobs:
           - tests::nakamoto_integrations::follower_bootup
           - tests::nakamoto_integrations::forked_tenure_is_ignored
           - tests::nakamoto_integrations::nakamoto_attempt_time
+          - tests::nakamoto_integrations::skip_mining_long_tx
           - tests::signer::v0::block_proposal_rejection
           - tests::signer::v0::miner_gather_signatures
           - tests::signer::v0::end_of_tenure

stacks-signer/src/chainstate.rs

@@ -187,13 +187,15 @@ impl SortitionsView {
         block: &NakamotoBlock,
         block_pk: &StacksPublicKey,
         reward_cycle: u64,
+        reset_view_if_wrong_consensus_hash: bool,
     ) -> Result<bool, SignerChainstateError> {
         if self
             .cur_sortition
             .is_timed_out(self.config.block_proposal_timeout, signer_db)?
         {
             info!(
                 "Current miner timed out, marking as invalid.";
+                "block_height" => block.header.chain_length,
                 "current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
             );
             self.cur_sortition.miner_status = SortitionMinerStatus::InvalidatedBeforeFirstBlock;
@@ -202,6 +204,7 @@ impl SortitionsView {
             if last_sortition.is_timed_out(self.config.block_proposal_timeout, signer_db)? {
                 info!(
                     "Last miner timed out, marking as invalid.";
+                    "block_height" => block.header.chain_length,
                     "last_sortition_consensus_hash" => ?last_sortition.consensus_hash,
                 );
                 last_sortition.miner_status = SortitionMinerStatus::InvalidatedBeforeFirstBlock;
@@ -236,6 +239,23 @@ impl SortitionsView {
                 })
             })
         else {
+            if reset_view_if_wrong_consensus_hash {
+                info!(
+                    "Miner block proposal has consensus hash that is neither the current or last sortition. Resetting view.";
+                    "proposed_block_consensus_hash" => %block.header.consensus_hash,
+                    "current_sortition_consensus_hash" => ?self.cur_sortition.consensus_hash,
+                    "last_sortition_consensus_hash" => ?self.last_sortition.as_ref().map(|x| x.consensus_hash),
+                );
+                self.reset_view(client)?;
+                return self.check_proposal(
+                    client,
+                    signer_db,
+                    block,
+                    block_pk,
+                    reward_cycle,
+                    false,
+                );
+            }
             warn!(
                 "Miner block proposal has consensus hash that is neither the current or last sortition. Considering invalid.";
                 "proposed_block_consensus_hash" => %block.header.consensus_hash,
@@ -347,6 +367,7 @@ impl SortitionsView {
             "sortition_state.consensus_hash" => %sortition_state.consensus_hash,
             "sortition_state.prior_sortition" => %sortition_state.prior_sortition,
             "sortition_state.parent_tenure_id" => %sortition_state.parent_tenure_id,
+            "block_height" => block.header.chain_length,
         );
 
         let tenures_reorged = client.get_tenure_forking_info(
@@ -406,6 +427,7 @@ impl SortitionsView {
                             "Miner is not building off of most recent tenure. A tenure they reorg has already mined blocks, but the block was poorly timed, allowing the reorg.";
                             "proposed_block_consensus_hash" => %block.header.consensus_hash,
                             "proposed_block_signer_sighash" => %block.header.signer_signature_hash(),
+                            "proposed_block_height" => block.header.chain_length,
                             "parent_tenure" => %sortition_state.parent_tenure_id,
                             "last_sortition" => %sortition_state.prior_sortition,
                             "violating_tenure_id" => %tenure.consensus_hash,
@@ -578,6 +600,7 @@ impl SortitionsView {
                 "Have no accepted blocks in the tenure, assuming block confirmation is correct";
                 "proposed_block_consensus_hash" => %block.header.consensus_hash,
                 "proposed_block_signer_sighash" => %block.header.signer_signature_hash(),
+                "proposed_block_height" => block.header.chain_length,
             );
             return Ok(true);
         };
@@ -624,4 +647,23 @@ impl SortitionsView {
             config,
         })
     }
+
+    /// Reset the view to the current sortition and last sortition
+    pub fn reset_view(&mut self, client: &StacksClient) -> Result<(), ClientError> {
+        let CurrentAndLastSortition {
+            current_sortition,
+            last_sortition,
+        } = client.get_current_and_last_sortition()?;
+
+        let cur_sortition = SortitionState::try_from(current_sortition)?;
+        let last_sortition = last_sortition
+            .map(SortitionState::try_from)
+            .transpose()
+            .ok()
+            .flatten();
+
+        self.cur_sortition = cur_sortition;
+        self.last_sortition = last_sortition;
+        Ok(())
+    }
 }

stacks-signer/src/client/mod.rs

@@ -89,9 +89,6 @@ pub enum ClientError {
     /// Invalid response from the stacks node
     #[error("Invalid response from the stacks node: {0}")]
     InvalidResponse(String),
-    /// A successful sortition has not occurred yet
-    #[error("The Stacks chain has not processed any successful sortitions yet")]
-    NoSortitionOnChain,
     /// A successful sortition's info response should be parseable into a SortitionState
     #[error("A successful sortition's info response should be parseable into a SortitionState")]
     UnexpectedSortitionInfo,

stacks-signer/src/client/stacks_client.rs

@@ -14,6 +14,8 @@
 // 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::{HashMap, VecDeque};
+use std::fmt::Display;
+use std::time::{Duration, Instant};
 
 use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
 use blockstack_lib::chainstate::stacks::boot::{NakamotoSignerEntry, SIGNERS_NAME};
@@ -30,7 +32,7 @@ use blockstack_lib::net::api::get_tenures_fork_info::{
 use blockstack_lib::net::api::getaccount::AccountEntryResponse;
 use blockstack_lib::net::api::getpoxinfo::RPCPoxInfoData;
 use blockstack_lib::net::api::getsortition::{SortitionInfo, RPC_SORTITION_INFO_PATH};
-use blockstack_lib::net::api::getstackers::{GetStackersErrors, GetStackersResponse};
+use blockstack_lib::net::api::getstackers::GetStackersResponse;
 use blockstack_lib::net::api::postblock::StacksBlockAcceptedData;
 use blockstack_lib::net::api::postblock_proposal::NakamotoBlockProposal;
 use blockstack_lib::net::api::postblock_v3;
@@ -78,7 +80,6 @@ pub struct StacksClient {
 
 #[derive(Deserialize)]
 struct GetStackersErrorResp {
-    err_type: String,
     err_msg: String,
 }
 
@@ -375,10 +376,12 @@ impl StacksClient {
             "last_sortition" => %last_sortition,
         );
         let path = self.tenure_forking_info_path(chosen_parent, last_sortition);
-        let timer = crate::monitoring::new_rpc_call_timer(
-            "/v3/tenures/fork_info/:start/:stop",
-            &self.http_origin,
+        // Use a separate metrics path to allow the same metric for different start and stop hashes
+        let metrics_path = format!(
+            "{}{RPC_TENURE_FORKING_INFO_PATH}/:start/:stop",
+            self.http_origin
         );
+        let timer = crate::monitoring::new_rpc_call_timer(&metrics_path, &self.http_origin);
         let send_request = || {
             self.stacks_node_client
                 .get(&path)
@@ -483,14 +486,11 @@ impl StacksClient {
                 warn!("Failed to parse the GetStackers error response: {e}");
                 backoff::Error::permanent(e.into())
             })?;
-            if error_data.err_type == GetStackersErrors::NOT_AVAILABLE_ERR_TYPE {
-                Err(backoff::Error::permanent(ClientError::NoSortitionOnChain))
-            } else {
-                warn!("Got error response ({status}): {}", error_data.err_msg);
-                Err(backoff::Error::permanent(ClientError::RequestFailure(
-                    status,
-                )))
-            }
+
+            warn!("Got error response ({status}): {}", error_data.err_msg);
+            Err(backoff::Error::permanent(ClientError::RequestFailure(
+                status,
+            )))
         };
         let stackers_response =
             retry_with_exponential_backoff::<_, ClientError, GetStackersResponse>(send_request)?;
@@ -564,6 +564,31 @@ impl StacksClient {
         Ok(account_entry)
     }
 
+    /// Post a block to the stacks-node, retry forever on errors.
+    ///
+    /// In tests, this panics if the retry takes longer than 30 seconds.
+    pub fn post_block_until_ok<F: Display>(&self, log_fmt: &F, block: &NakamotoBlock) -> bool {
+        let start_time = Instant::now();
+        loop {
+            match self.post_block(block) {
+                Ok(block_push_result) => {
+                    debug!("{log_fmt}: Block pushed to stacks node: {block_push_result:?}");
+                    return block_push_result;
+                }
+                Err(e) => {
+                    if cfg!(any(test, feature = "testing"))
+                        && start_time.elapsed() > Duration::from_secs(30)
+                    {
+                        panic!(
+                            "{log_fmt}: Timed out in test while pushing block to stacks node: {e}"
+                        );
+                    }
+                    warn!("{log_fmt}: Failed to push block to stacks node: {e}. Retrying...");
+                }
+            };
+        }
+    }
+
     /// Try to post a completed nakamoto block to our connected stacks-node
     /// Returns `true` if the block was accepted or `false` if the block
     ///   was rejected.

stacks-signer/src/monitoring/mod.rs

@@ -92,13 +92,22 @@ pub fn update_signer_nonce(nonce: u64) {
     prometheus::SIGNER_NONCE.set(nonce as i64);
 }
 
+// Allow dead code because this is only used in the `monitoring_prom` feature
+// but we want to run it in a test
+#[allow(dead_code)]
+/// Remove the origin from the full path to avoid duplicate metrics for different origins
+fn remove_origin_from_path(full_path: &str, origin: &str) -> String {
+    let path = full_path.replace(origin, "");
+    path
+}
+
 /// Start a new RPC call timer.
 /// The `origin` parameter is the base path of the RPC call, e.g. `http://node.com`.
 /// The `origin` parameter is removed from `full_path` when storing in prometheus.
 #[cfg(feature = "monitoring_prom")]
 pub fn new_rpc_call_timer(full_path: &str, origin: &str) -> HistogramTimer {
-    let path = &full_path[origin.len()..];
-    let histogram = prometheus::SIGNER_RPC_CALL_LATENCIES_HISTOGRAM.with_label_values(&[path]);
+    let path = remove_origin_from_path(full_path, origin);
+    let histogram = prometheus::SIGNER_RPC_CALL_LATENCIES_HISTOGRAM.with_label_values(&[&path]);
     histogram.start_timer()
 }
 
@@ -140,3 +149,16 @@ pub fn start_serving_monitoring_metrics(config: GlobalConfig) -> Result<(), Stri
     }
     Ok(())
 }
+
+#[test]
+fn test_remove_origin_from_path() {
+    let full_path = "http://localhost:20443/v2/info";
+    let origin = "http://localhost:20443";
+    let path = remove_origin_from_path(full_path, origin);
+    assert_eq!(path, "/v2/info");
+
+    let full_path = "/v2/info";
+    let origin = "http://localhost:20443";
+    let path = remove_origin_from_path(full_path, origin);
+    assert_eq!(path, "/v2/info");
+}