From 856144829e9dc32e7a817cd5ab85eca8ea172c1f Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Tue, 5 Aug 2025 11:13:13 +0100 Subject: [PATCH 01/10] update /v3/health to use highest_stacks_height_of_neighbors --- .../src/tests/nakamoto_integrations.rs | 18 +- stacks-node/src/tests/neon_integrations.rs | 13 + stackslib/src/net/api/gethealth.rs | 176 ++------- stackslib/src/net/api/tests/gethealth.rs | 370 ++---------------- stackslib/src/net/mod.rs | 12 +- stackslib/src/net/p2p.rs | 5 + stackslib/src/net/rpc.rs | 3 + stackslib/src/net/tests/mod.rs | 39 +- 8 files changed, 140 insertions(+), 496 deletions(-) diff --git a/stacks-node/src/tests/nakamoto_integrations.rs b/stacks-node/src/tests/nakamoto_integrations.rs index ba73bc7f83..2c9a9087d4 100644 --- a/stacks-node/src/tests/nakamoto_integrations.rs +++ b/stacks-node/src/tests/nakamoto_integrations.rs @@ -125,7 +125,7 @@ use crate::operations::BurnchainOpSigner; use crate::run_loop::boot_nakamoto; use crate::tests::neon_integrations::{ call_read_only, get_account, get_account_result, get_chain_info_opt, get_chain_info_result, - get_neighbors, get_pox_info, get_sortition_info, next_block_and_wait, + get_neighbors, get_node_health, get_pox_info, get_sortition_info, next_block_and_wait, run_until_burnchain_height, submit_tx, submit_tx_fallible, test_observer, wait_for_runloop, }; use crate::tests::signer::SignerTest; @@ -2413,10 +2413,11 @@ fn mine_multiple_per_tenure_integration() { /// It starts in Epoch 2.0, mines with `neon_node` to Epoch 3.0, and then switches /// to Nakamoto operation (activating pox-4 by submitting a stack-stx tx). The BootLoop /// struct handles the epoch-2/3 tear-down and spin-up. -/// This test makes three assertions: +/// This test makes four assertions: /// * 15 tenures are mined after 3.0 starts /// * Each tenure has 6 blocks (the coinbase block and 5 interim blocks) /// * Both nodes see the same chainstate at the end of the test +/// * Both nodes have the same `PeerNetwork::highest_stacks_height_of_neighbors` fn multiple_miners() { if env::var("BITCOIND_TEST") != Ok("1".into()) { return; @@ -2641,6 +2642,19 @@ fn multiple_miners() { info!("Peer height information"; "peer_1" => peer_1_height, "peer_2" => peer_2_height); assert_eq!(peer_1_height, peer_2_height); + // check that the `ConversationHttp::chat` was called and updated + // `PeerNetwork::highest_stacks_height_of_neighbors` + wait_for(20, || { + let health_node_1 = get_node_health(&naka_conf); + let health_node_2 = get_node_health(&conf_node_2); + info!("Peer health information"; "peer_1" => ?health_node_1, "peer_2" => ?health_node_2); + Ok( + health_node_1.max_stacks_height_of_neighbors == peer_2_height + && health_node_2.max_stacks_height_of_neighbors == peer_1_height, + ) + }) + .unwrap(); + assert!(tip.anchored_header.as_stacks_nakamoto().is_some()); assert_eq!( tip.stacks_block_height, diff --git a/stacks-node/src/tests/neon_integrations.rs b/stacks-node/src/tests/neon_integrations.rs index 2557ef6534..9bc6f376e3 100644 --- a/stacks-node/src/tests/neon_integrations.rs +++ b/stacks-node/src/tests/neon_integrations.rs @@ -55,6 +55,7 @@ use stacks::core::{ }; use stacks::net::api::getaccount::AccountEntryResponse; use stacks::net::api::getcontractsrc::ContractSrcResponse; +use stacks::net::api::gethealth::RPCGetHealthResponse; use stacks::net::api::getinfo::RPCPeerInfoData; use stacks::net::api::getpoxinfo::RPCPoxInfoData; use stacks::net::api::getsortition::SortitionInfo; @@ -1010,6 +1011,18 @@ pub fn get_block(http_origin: &str, block_id: &StacksBlockId) -> Option RPCGetHealthResponse { + let http_origin = format!("http://{}", &conf.node.rpc_bind); + let client = reqwest::blocking::Client::new(); + let path = format!("{http_origin}/v3/health"); + client + .get(&path) + .send() + .unwrap() + .json::() + .unwrap() +} + pub fn get_chain_info_result(conf: &Config) -> Result { let http_origin = format!("http://{}", &conf.node.rpc_bind); let client = reqwest::blocking::Client::new(); diff --git a/stackslib/src/net/api/gethealth.rs b/stackslib/src/net/api/gethealth.rs index d17fe14229..36961bbb14 100644 --- a/stackslib/src/net/api/gethealth.rs +++ b/stackslib/src/net/api/gethealth.rs @@ -14,22 +14,15 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use std::fmt; -use std::str::FromStr; - use regex::{Captures, Regex}; use stacks_common::types::net::PeerHost; -use stacks_common::types::StacksEpochId; -use crate::net::db::PeerDB; use crate::net::http::{ parse_json, Error, HttpRequest, HttpRequestContents, HttpRequestPreamble, HttpResponse, HttpResponseContents, HttpResponsePayload, HttpResponsePreamble, HttpServerError, }; use crate::net::httpcore::{RPCRequestHandler, StacksHttpRequest, StacksHttpResponse}; -use crate::net::{ - infer_initial_burnchain_block_download, Error as NetError, NeighborAddress, StacksNodeState, -}; +use crate::net::{Error as NetError, StacksNodeState}; /// The response for the GET /v3/health endpoint /// This endpoint returns the difference in height between the node and its most advanced neighbor @@ -46,53 +39,13 @@ pub struct RPCGetHealthResponse { pub node_stacks_tip_height: u64, } -const NEIGHBORS_SCOPE_PARAM_NAME: &str = "neighbors"; - -#[derive(Clone, Debug, PartialEq)] -pub enum NeighborsScope { - Initial, - All, -} - -impl FromStr for NeighborsScope { - type Err = crate::net::http::Error; - - fn from_str(s: &str) -> Result { - match s { - "initial" => Ok(NeighborsScope::Initial), - "all" => Ok(NeighborsScope::All), - _ => Err(crate::net::http::Error::Http( - 400, - format!( - "Invalid `neighbors` query parameter: `{}`, allowed values are `initial` or `all`", - s - ), - )), - } - } -} - -impl fmt::Display for NeighborsScope { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let s = match self { - NeighborsScope::Initial => "initial", - NeighborsScope::All => "all", - }; - write!(f, "{s}") - } -} - #[derive(Clone)] /// Empty request handler for the GET /v3/health endpoint -pub struct RPCGetHealthRequestHandler { - neighbors_scope: Option, -} +pub struct RPCGetHealthRequestHandler {} impl RPCGetHealthRequestHandler { pub fn new() -> Self { - Self { - neighbors_scope: None, - } + Self {} } } @@ -125,12 +78,7 @@ impl HttpRequest for RPCGetHealthRequestHandler { )); } - let req_contents = HttpRequestContents::new().query_string(query); - if let Some(scope) = req_contents.get_query_arg(NEIGHBORS_SCOPE_PARAM_NAME) { - self.neighbors_scope = Some(scope.parse()?); - } - - Ok(req_contents) + Ok(HttpRequestContents::new().query_string(query)) } } @@ -145,9 +93,7 @@ fn create_error_response( impl RPCRequestHandler for RPCGetHealthRequestHandler { /// Reset internal state - fn restart(&mut self) { - self.neighbors_scope = None; - } + fn restart(&mut self) {} /// Make the response fn try_handle_request( @@ -156,97 +102,26 @@ impl RPCRequestHandler for RPCGetHealthRequestHandler { _contents: HttpRequestContents, node: &mut StacksNodeState, ) -> Result<(HttpResponsePreamble, HttpResponseContents), NetError> { - let neighbors_scope = self - .neighbors_scope - .take() - .unwrap_or(NeighborsScope::Initial); - let use_all_neighbors = neighbors_scope == NeighborsScope::All; - - node.with_node_state(|network, _sortdb, _chainstate, _mempool, _rpc_args| { - let current_epoch = network.get_current_epoch(); - - let neighbors_arg = if use_all_neighbors { - None - } else { - let initial_neighbors = PeerDB::get_valid_initial_neighbors( - network.peerdb.conn(), - network.local_peer.network_id, - current_epoch.network_epoch, - network.peer_version, - network.chain_view.burn_block_height, + let (max_stacks_height_of_neighbors, node_stacks_tip_height) = + node.with_node_state(|network, _sortdb, _chainstate, _mempool, _rpc_args| { + ( + network.highest_stacks_height_of_neighbors, + network.stacks_tip.height, ) - .map_err(NetError::from)?; - - if initial_neighbors.is_empty() { - return create_error_response( - &preamble, - "No viable bootstrap peers found, unable to determine health", - ); - } - Some(initial_neighbors) - }; + }); - let peer_max_stacks_height_opt = { - if current_epoch.epoch_id < StacksEpochId::Epoch30 { - // When the node enters Epoch 3.0, ibd is not accurate. In nakamoto it's always set to false. - // See the implementation of `RunLoop::start` in `stacks-node/src/run_loop/nakamoto.rs`, - // specifically the section and comment where `let ibd = false`, for details. - let ibd = infer_initial_burnchain_block_download( - &network.burnchain, - network.burnchain_tip.block_height, - network.chain_view.burn_block_height, - ); + // There could be a edge case where our node is ahead of all peers. + let difference_from_max_peer = + max_stacks_height_of_neighbors.saturating_sub(node_stacks_tip_height); - // get max block height amongst bootstrap nodes - match network.inv_state.as_ref() { - Some(inv_state) => { - inv_state.get_max_stacks_height_of_neighbors(neighbors_arg.as_deref(), ibd) - } - None => { - return create_error_response( - &preamble, - "Peer inventory state (Epoch 2.x) not found, unable to determine health.", - ); - } - } - } else { - let neighbors_arg: Option> = neighbors_arg.as_ref().map(|v| v.iter().map(NeighborAddress::from_neighbor).collect()); - match network.block_downloader_nakamoto.as_ref() { - Some(block_downloader_nakamoto) => { - block_downloader_nakamoto.get_max_stacks_height_of_neighbors(neighbors_arg.as_deref()) - } - None => { - return create_error_response( - &preamble, - "Nakamoto block downloader not found (Epoch 3.0+), unable to determine health.", - ); - } - } - } - }; - - match peer_max_stacks_height_opt { - Some(max_stacks_height_of_neighbors) => { - // There could be a edge case where our node is ahead of all peers. - let node_stacks_tip_height = network.stacks_tip.height; - let difference_from_max_peer = - max_stacks_height_of_neighbors.saturating_sub(node_stacks_tip_height); - - let preamble = HttpResponsePreamble::ok_json(&preamble); - let data = RPCGetHealthResponse { - difference_from_max_peer, - max_stacks_height_of_neighbors, - node_stacks_tip_height, - }; - let body = HttpResponseContents::try_from_json(&data)?; - Ok((preamble, body)) - } - None => create_error_response( - &preamble, - "Couldn't obtain stats on any bootstrap peers, unable to determine health.", - ), - } - }) + let preamble = HttpResponsePreamble::ok_json(&preamble); + let data = RPCGetHealthResponse { + difference_from_max_peer, + max_stacks_height_of_neighbors, + node_stacks_tip_height, + }; + let body = HttpResponseContents::try_from_json(&data)?; + Ok((preamble, body)) } } @@ -263,15 +138,12 @@ impl HttpResponse for RPCGetHealthRequestHandler { } impl StacksHttpRequest { - pub fn new_gethealth(host: PeerHost, neighbors_scope: NeighborsScope) -> StacksHttpRequest { + pub fn new_gethealth(host: PeerHost) -> StacksHttpRequest { StacksHttpRequest::new_for_peer( host, "GET".into(), "/v3/health".into(), - HttpRequestContents::new().query_arg( - NEIGHBORS_SCOPE_PARAM_NAME.into(), - neighbors_scope.to_string(), - ), + HttpRequestContents::new(), ) .expect("FATAL: failed to construct request from infallible data") } diff --git a/stackslib/src/net/api/tests/gethealth.rs b/stackslib/src/net/api/tests/gethealth.rs index 7efcfcce5d..f1428495fa 100644 --- a/stackslib/src/net/api/tests/gethealth.rs +++ b/stackslib/src/net/api/tests/gethealth.rs @@ -15,32 +15,19 @@ // along with this program. If not, see . use std::net::{IpAddr, Ipv4Addr, SocketAddr}; -use clarity::types::chainstate::StacksBlockId; -use clarity::types::StacksEpochId; - use super::TestRPC; -use crate::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader}; -use crate::chainstate::stacks::boot::RewardSet; -use crate::net::api::gethealth::{ - NeighborsScope, RPCGetHealthRequestHandler, RPCGetHealthResponse, -}; -use crate::net::api::gettenureinfo::RPCGetTenureInfo; +use crate::net::api::gethealth::{RPCGetHealthRequestHandler, RPCGetHealthResponse}; use crate::net::connection::ConnectionOptions; -use crate::net::download::nakamoto::{ - NakamotoDownloadState, NakamotoDownloadStateMachine, NakamotoTenureDownloader, - NakamotoUnconfirmedTenureDownloader, -}; -use crate::net::http::HttpRequestContents; -use crate::net::httpcore::{StacksHttp, StacksHttpRequest}; +use crate::net::httpcore::{HttpPreambleExtensions as _, StacksHttp, StacksHttpRequest}; use crate::net::test::TestEventObserver; -use crate::net::{NeighborAddress, ProtocolFamily}; +use crate::net::ProtocolFamily; #[test] fn test_try_parse_request() { let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); let mut http = StacksHttp::new(addr.clone(), &ConnectionOptions::default()); - let request = StacksHttpRequest::new_gethealth(addr.into(), NeighborsScope::Initial); + let request = StacksHttpRequest::new_gethealth(addr.into()); let bytes = request.try_serialize().unwrap(); let (parsed_preamble, offset) = http.read_preamble(&bytes).unwrap(); @@ -60,17 +47,17 @@ fn test_try_parse_request() { assert_eq!(&preamble, request.preamble()); } -// Helper function for Nakamoto health test scenarios -fn setup_and_run_nakamoto_health_test( - test_function_name_suffix: &str, - peer_1_height_relative_to_node: i64, // How many blocks peer_1 is ahead (positive) or behind (negative) the node. - expected_difference_from_max_peer: u64, - nakamoto_download_state: NakamotoDownloadState, +#[rstest] +#[case::node_behind(100, 100)] +#[case::same_height(0, 0)] +#[case::node_ahead(-10, 0)] +fn test_get_health( + #[case] peer_1_height_relative_to_node: i64, // How many blocks peer_1 is ahead (positive) or behind (negative) the node. + #[case] expected_difference_from_max_peer: u64, ) { // `rpc_test` will have peer_1 (client) and peer_2 (server/node) let test_observer = TestEventObserver::new(); - let rpc_test_name = format!("{}{}", function_name!(), test_function_name_suffix); - let mut rpc_test = TestRPC::setup_nakamoto(&rpc_test_name, &test_observer); + let mut rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); // The node being tested is peer_2 (server role in TestRPC) rpc_test.peer_2.refresh_burnchain_view(); @@ -82,349 +69,58 @@ fn setup_and_run_nakamoto_health_test( } else { node_stacks_tip_height + (peer_1_height_relative_to_node as u64) }; - - let peer_1_address = NeighborAddress::from_neighbor(&rpc_test.peer_1.config.to_neighbor()); - - // Setup peer_1's tenure information to reflect its calculated height - let peer_1_tenure_tip = RPCGetTenureInfo { - consensus_hash: rpc_test.peer_1.network.stacks_tip.consensus_hash.clone(), - tenure_start_block_id: rpc_test.peer_1.network.tenure_start_block_id.clone(), - parent_consensus_hash: rpc_test - .peer_1 - .network - .parent_stacks_tip - .consensus_hash - .clone(), - parent_tenure_start_block_id: StacksBlockId::new( - &rpc_test.peer_1.network.parent_stacks_tip.consensus_hash, - &rpc_test.peer_1.network.parent_stacks_tip.block_hash, - ), - tip_block_id: StacksBlockId::new( - &rpc_test.peer_1.network.stacks_tip.consensus_hash, // We are not changing the tip block id for this height adjustment - &rpc_test.peer_1.network.stacks_tip.block_hash, - ), - tip_height: peer_1_actual_height, - reward_cycle: rpc_test - .peer_1 - .network - .burnchain - .block_height_to_reward_cycle(rpc_test.peer_1.network.burnchain_tip.block_height) - .expect("FATAL: burnchain tip before system start"), - }; - - // Initialize the downloader state for peer_2 (the node) - let epoch = rpc_test - .peer_1 - .network - .get_epoch_by_epoch_id(StacksEpochId::Epoch30); - let mut downloader = NakamotoDownloadStateMachine::new( - epoch.start_height, - rpc_test.peer_1.network.stacks_tip.block_id(), // Initial tip for the downloader state machine - ); - match nakamoto_download_state { - NakamotoDownloadState::Confirmed => { - let mut confirmed_tenure = NakamotoTenureDownloader::new( - peer_1_tenure_tip.consensus_hash.clone(), - peer_1_tenure_tip.consensus_hash.clone(), - peer_1_tenure_tip.parent_tenure_start_block_id.clone(), - peer_1_tenure_tip.consensus_hash.clone(), - peer_1_tenure_tip.tip_block_id.clone(), - peer_1_address.clone(), - RewardSet::empty(), - RewardSet::empty(), - false, - ); - - let mut header = NakamotoBlockHeader::empty(); - header.chain_length = peer_1_actual_height - 1; - header.consensus_hash = peer_1_tenure_tip.consensus_hash.clone(); - header.parent_block_id = peer_1_tenure_tip.parent_tenure_start_block_id.clone(); - let nakamoto_block = NakamotoBlock { - header, - txs: vec![], - }; - confirmed_tenure.tenure_end_block = Some(nakamoto_block); - downloader - .tenure_downloads - .downloaders - .push(Some(confirmed_tenure)); // Add peer_1's state to peer_2's downloader - downloader.state = NakamotoDownloadState::Confirmed; - } - NakamotoDownloadState::Unconfirmed => { - let mut unconfirmed_tenure = NakamotoUnconfirmedTenureDownloader::new( - peer_1_address.clone(), - Some(peer_1_tenure_tip.tip_block_id.clone()), - ); - unconfirmed_tenure.tenure_tip = Some(peer_1_tenure_tip); - downloader - .unconfirmed_tenure_downloads - .insert(peer_1_address, unconfirmed_tenure); // Add peer_1's state to peer_2's downloader - downloader.state = NakamotoDownloadState::Unconfirmed; - } - } - rpc_test.peer_2.network.block_downloader_nakamoto = Some(downloader); + rpc_test.peer_2.network.highest_stacks_height_of_neighbors = peer_1_actual_height; // --- Invoke the Handler --- let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); - let request = StacksHttpRequest::new_gethealth(addr.into(), NeighborsScope::Initial); + let request = StacksHttpRequest::new_gethealth(addr.into()); let mut responses = rpc_test.run(vec![request]); let response = responses.remove(0); // --- Assertions --- let (http_resp_preamble, contents) = response.destruct(); - assert_eq!( - http_resp_preamble.status_code, 200, - "Expected HTTP 200 OK for test case: {}", - test_function_name_suffix - ); + assert_eq!(http_resp_preamble.status_code, 200, "Expected HTTP 200 OK"); - let response_json_val: serde_json::Value = contents.try_into().unwrap_or_else(|e| { - panic!( - "Failed to parse JSON for test case {}: {}", - test_function_name_suffix, e - ) - }); + let response_json_val: serde_json::Value = contents + .try_into() + .unwrap_or_else(|e| panic!("Failed to parse JSON: {e}")); let health_response: RPCGetHealthResponse = serde_json::from_value(response_json_val) - .unwrap_or_else(|e| { - panic!( - "Failed to deserialize RPCGetHealthResponse for test case {}: {}", - test_function_name_suffix, e - ) - }); + .unwrap_or_else(|e| panic!("Failed to deserialize RPCGetHealthResponse: {e}")); assert_eq!( health_response.node_stacks_tip_height, node_stacks_tip_height, - "Mismatch in node_stacks_tip_height for test case: {}", - test_function_name_suffix + "Mismatch in node_stacks_tip_height" ); - // In these scenarios, peer_1 is the only peer configured with stats in the downloader. assert_eq!( health_response.max_stacks_height_of_neighbors, peer_1_actual_height, - "Mismatch in max_stacks_height_of_neighbors for test case: {}", - test_function_name_suffix + "Mismatch in max_stacks_height_of_neighbors" ); assert_eq!( health_response.difference_from_max_peer, expected_difference_from_max_peer, - "Mismatch in difference_from_max_peer for test case: {}", - test_function_name_suffix - ); -} - -#[test] -fn test_get_health_node_behind_of_peers_unconfirmed() { - // This test simulates peer_2 (node) being behind peer_1. - // So, peer_1's height is greater than peer_2's height. - setup_and_run_nakamoto_health_test( - "node_behind", - 100, // peer_1 is 100 blocks *ahead* of the node (node's height + 100) - 100, // Expected difference: node is 100 blocks behind max peer height - NakamotoDownloadState::Unconfirmed, - ); -} - -#[test] -fn test_get_health_same_height_as_peers_unconfirmed() { - // Test when node is at the same height as its most advanced peer (peer_1) - setup_and_run_nakamoto_health_test( - "same_height", - 0, // peer_1 is at the same height as the node (node's height + 0) - 0, // Expected difference: node is at the same height as max peer - NakamotoDownloadState::Unconfirmed, - ); -} - -#[test] -fn test_get_health_node_ahead_of_peers_unconfirmed() { - // Test when node (peer_2) is ahead of its peer (peer_1) - // So, peer_1's height is less than peer_2's height. - setup_and_run_nakamoto_health_test( - "node_ahead", - -10, // peer_1 is 10 blocks *behind* the node (node's height - 10) - 0, // Expected difference: 0, because difference is node_height.saturating_sub(peer_height) - // when the node is ahead, this results in 0 if peer_height < node_height. - NakamotoDownloadState::Unconfirmed, - ); -} - -#[test] -fn test_get_health_node_behind_of_peers_confirmed() { - // Test when node (peer_2) is behind its peer (peer_1) - // So, peer_1's height is greater than peer_2's height. - setup_and_run_nakamoto_health_test( - "node_behind", - 100, // peer_1 is 100 blocks *ahead* of the node (node's height + 100) - 100, // Expected difference: node is 100 blocks behind max peer height - NakamotoDownloadState::Confirmed, - ); -} - -#[test] -fn test_get_health_same_height_as_peers_confirmed() { - // Test when node (peer_2) is at the same height as its peer (peer_1) - // So, peer_1's height is equal to peer_2's height. - setup_and_run_nakamoto_health_test( - "same_height", - 0, // peer_1 is at the same height as the node (node's height + 0) - 0, // Expected difference: node is at the same height as max peer - NakamotoDownloadState::Confirmed, - ); -} - -#[test] -fn test_get_health_node_ahead_of_peers_confirmed() { - // Test when node (peer_2) is ahead of its peer (peer_1) - // So, peer_1's height is less than peer_2's height. - setup_and_run_nakamoto_health_test( - "node_ahead", - -10, // peer_1 is 10 blocks *behind* the node (node's height - 10) - 0, // Expected difference: 0, because difference is node_height.saturating_sub(peer_height) - // when the node is ahead, this results in 0 if peer_height < node_height. - NakamotoDownloadState::Confirmed, - ); -} - -#[test] -fn test_get_health_400_invalid_neighbors_param() { - let test_observer = TestEventObserver::new(); - let rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); - let request = StacksHttpRequest::new_for_peer( - SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333).into(), - "GET".into(), - "/v3/health".into(), - HttpRequestContents::new().query_string(Some("neighbors=invalid")), - ) - .expect("FATAL: failed to construct request from infallible data"); - - let mut responses = rpc_test.run(vec![request]); - let response = responses.remove(0); - - let (http_resp_preamble, contents) = response.destruct(); - let error_message: String = contents.try_into().unwrap(); - assert_eq!( - error_message, - "Invalid `neighbors` query parameter: `invalid`, allowed values are `initial` or `all`" - ); - assert_eq!( - http_resp_preamble.status_code, 400, - "Expected HTTP 400 Bad Request for invalid neighbors parameter" - ); -} - -#[test] -fn test_get_health_500_no_initial_neighbors() { - // Test error handling when no initial neighbors are found - let test_observer = TestEventObserver::new(); - let mut rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); - rpc_test.peer_2.refresh_burnchain_view(); - rpc_test.peer_2.network.init_nakamoto_block_downloader(); - - // Mock the PeerDB::get_valid_initial_neighbors to return empty vec by - // clearing all peers from the peer DB - rpc_test - .peer_2 - .network - .peerdb - .conn() - .execute("DELETE FROM frontier", []) - .unwrap(); - - // --- Invoke the Handler --- - let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); - let request = StacksHttpRequest::new_gethealth(addr.into(), NeighborsScope::Initial); - let mut responses = rpc_test.run(vec![request]); - let response = responses.remove(0); - - // --- Assertions --- - let (http_resp_preamble, contents) = response.destruct(); - assert_eq!( - http_resp_preamble.status_code, 500, - "Expected HTTP 500 Internal Server Error" - ); - let error_message: String = contents - .try_into() - .expect("Failed to parse JSON from HttpResponseContents"); - assert_eq!( - error_message, - "No viable bootstrap peers found, unable to determine health" - ); -} - -#[test] -fn test_get_health_500_no_inv_state_pre_nakamoto() { - // Test when inv_state is None in pre-Nakamoto epochs - let test_observer = TestEventObserver::new(); - let mut rpc_test = TestRPC::setup(function_name!()); - - // Reset inv_state to None - rpc_test.peer_2.network.inv_state = None; - - // --- Invoke the Handler --- - let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); - let request = StacksHttpRequest::new_gethealth(addr.into(), NeighborsScope::Initial); - let mut responses = rpc_test.run(vec![request]); - let response = responses.remove(0); - - // --- Assertions --- - let (http_resp_preamble, contents) = response.destruct(); - assert_eq!( - http_resp_preamble.status_code, 500, - "Expected HTTP 500 Internal Server Error" - ); - let error_message: String = contents - .try_into() - .expect("Failed to parse JSON from HttpResponseContents"); - assert_eq!( - error_message, - "Peer inventory state (Epoch 2.x) not found, unable to determine health." + "Mismatch in difference_from_max_peer" ); } #[test] -fn test_get_health_500_no_download_state() { +fn test_get_health_no_peers_stats() { let test_observer = TestEventObserver::new(); - // by default, setup_nakamoto doesn't intialize the network.block_downloader_nakamoto let rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); // --- Invoke the Handler --- let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); - let request = StacksHttpRequest::new_gethealth(addr.into(), NeighborsScope::Initial); + let request = StacksHttpRequest::new_gethealth(addr.into()); let mut responses = rpc_test.run(vec![request]); let response = responses.remove(0); // --- Assertions --- let (http_resp_preamble, contents) = response.destruct(); + assert_eq!(http_resp_preamble.status_code, 200, "Expected HTTP 200 OK"); + let response_json_val: serde_json::Value = contents.try_into().unwrap(); + let health_response: RPCGetHealthResponse = serde_json::from_value(response_json_val).unwrap(); + assert_eq!(health_response.max_stacks_height_of_neighbors, 0); assert_eq!( - http_resp_preamble.status_code, 500, - "Expected HTTP 500 Internal Server Error" - ); - let error_message: String = contents - .try_into() - .expect("Failed to parse JSON from HttpResponseContents"); - assert_eq!( - error_message, - "Nakamoto block downloader not found (Epoch 3.0+), unable to determine health." - ); -} - -#[test] -fn test_get_health_500_no_peers_stats() { - let test_observer = TestEventObserver::new(); - let mut rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); - rpc_test.peer_2.network.init_nakamoto_block_downloader(); - // --- Invoke the Handler --- - let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); - let request = StacksHttpRequest::new_gethealth(addr.into(), NeighborsScope::Initial); - let mut responses = rpc_test.run(vec![request]); - let response = responses.remove(0); - // --- Assertions --- - let (http_resp_preamble, contents) = response.destruct(); - assert_eq!( - http_resp_preamble.status_code, 500, - "Expected HTTP 500 Internal Server Error" - ); - let error_message: String = contents - .try_into() - .expect("Failed to parse JSON from HttpResponseContents"); - assert_eq!( - error_message, - "Couldn't obtain stats on any bootstrap peers, unable to determine health." + health_response.node_stacks_tip_height, + http_resp_preamble + .get_canonical_stacks_tip_height() + .unwrap() ); + assert_eq!(health_response.difference_from_max_peer, 0); } diff --git a/stackslib/src/net/mod.rs b/stackslib/src/net/mod.rs index 15edb36b92..f477dcdc13 100644 --- a/stackslib/src/net/mod.rs +++ b/stackslib/src/net/mod.rs @@ -694,9 +694,7 @@ impl<'a> StacksNodeState<'a> { } pub fn canonical_stacks_tip_height(&mut self) -> u64 { - self.with_node_state(|network, _, _, _, _| { - network.burnchain_tip.canonical_stacks_tip_height - }) + self.with_node_state(|network, _, _, _, _| network.stacks_tip.height) } pub fn set_relay_message(&mut self, msg: StacksMessageType) { @@ -808,6 +806,14 @@ impl<'a> StacksNodeState<'a> { } }) } + + pub fn update_highest_stacks_height_of_neighbors(&mut self, new_height: Option) { + self.with_node_state(|network, _, _, _, _| { + network.highest_stacks_height_of_neighbors = network + .highest_stacks_height_of_neighbors + .max(new_height.unwrap_or(0)); + }); + } } pub const STACKS_PUBLIC_KEY_ENCODED_SIZE: u32 = 33; diff --git a/stackslib/src/net/p2p.rs b/stackslib/src/net/p2p.rs index 3a6e5501ce..d3f94027a1 100644 --- a/stackslib/src/net/p2p.rs +++ b/stackslib/src/net/p2p.rs @@ -638,6 +638,9 @@ pub struct PeerNetwork { /// Thread handle for the async block proposal endpoint. block_proposal_thread: Option>, + + /// Highest stacks block height received from RPC responses from neighbors + pub highest_stacks_height_of_neighbors: u64, } impl PeerNetwork { @@ -799,6 +802,8 @@ impl PeerNetwork { nakamoto_inv_generator: InvGenerator::new(), block_proposal_thread: None, + + highest_stacks_height_of_neighbors: 0, }; network.init_block_downloader(); diff --git a/stackslib/src/net/rpc.rs b/stackslib/src/net/rpc.rs index 641a247c25..a40b5f1205 100644 --- a/stackslib/src/net/rpc.rs +++ b/stackslib/src/net/rpc.rs @@ -535,6 +535,9 @@ impl ConversationHttp { info!("Handled StacksHTTPRequest Error"; "path" => %path, "processing_time_ms" => start_time.elapsed().as_millis(), "conn_id" => self.conn_id, "peer_addr" => &self.peer_addr); } StacksHttpMessage::Response(resp) => { + node.update_highest_stacks_height_of_neighbors( + resp.preamble().get_canonical_stacks_tip_height(), + ); // Is there someone else waiting for this message? If so, pass it along. // (this _should_ be our pending_request handle) match self diff --git a/stackslib/src/net/tests/mod.rs b/stackslib/src/net/tests/mod.rs index f69a415ea3..752b460af0 100644 --- a/stackslib/src/net/tests/mod.rs +++ b/stackslib/src/net/tests/mod.rs @@ -63,10 +63,10 @@ use crate::chainstate::stacks::{ use crate::clarity::vm::types::StacksAddressExtensions; use crate::core::{StacksEpoch, StacksEpochExtension}; use crate::net::relay::Relayer; -use crate::net::test::{TestEventObserver, TestPeer, TestPeerConfig}; +use crate::net::test::{RPCHandlerArgsType, TestEventObserver, TestPeer, TestPeerConfig}; use crate::net::{ BlocksData, BlocksDatum, MicroblocksData, NakamotoBlocksData, NeighborKey, NetworkResult, - PingData, StackerDBPushChunkData, StacksMessage, StacksMessageType, + PingData, StackerDBPushChunkData, StacksMessage, StacksMessageType, StacksNodeState, }; use crate::util_lib::boot::boot_code_id; use crate::util_lib::signed_structured_data::pox4::make_pox_4_signer_key_signature; @@ -1806,3 +1806,38 @@ fn test_network_result_update() { assert_eq!(updated_uploaded.uploaded_nakamoto_blocks.len(), 1); assert_eq!(updated_uploaded.uploaded_nakamoto_blocks[0], nblk1); } + +#[rstest] +#[case::zero_to_none(0, None, 0)] +#[case::zero_to_some(0, Some(10), 10)] +#[case::some_to_none(10, None, 10)] +#[case::new_height_higher(10, Some(20), 20)] +#[case::new_height_lower(20, Some(10), 20)] +fn test_update_highest_stacks_height_of_neighbors( + #[case] old_height: u64, + #[case] new_height: Option, + #[case] expected_height: u64, +) { + let peer_config = TestPeerConfig::new(function_name!(), 0, 0); + let mut peer = TestPeer::new(peer_config); + peer.network.highest_stacks_height_of_neighbors = old_height; + let peer_sortdb = peer.sortdb.take().unwrap(); + let mut peer_stacks_node = peer.stacks_node.take().unwrap(); + let mut peer_mempool = peer.mempool.take().unwrap(); + let rpc_args = RPCHandlerArgsType::make_default(); + let mut node_state = StacksNodeState::new( + &mut peer.network, + &peer_sortdb, + &mut peer_stacks_node.chainstate, + &mut peer_mempool, + &rpc_args, + false, + false, + ); + + node_state.update_highest_stacks_height_of_neighbors(new_height); + assert_eq!( + peer.network.highest_stacks_height_of_neighbors, + expected_height + ); +} From 3d7087a0832aa793fb2238a8f9dee7fad826517d Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Tue, 5 Aug 2025 11:13:32 +0100 Subject: [PATCH 02/10] remove unused canonical_stacks_tip_height --- stackslib/src/net/rpc.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/stackslib/src/net/rpc.rs b/stackslib/src/net/rpc.rs index a40b5f1205..c8db1b2553 100644 --- a/stackslib/src/net/rpc.rs +++ b/stackslib/src/net/rpc.rs @@ -61,8 +61,6 @@ pub struct ConversationHttp { last_response_timestamp: u64, /// absolute time when this conversation was instantiated connection_time: u64, - /// stacks canonical chain tip that this peer reported - canonical_stacks_tip_height: Option, /// Ongoing replies reply_streams: VecDeque<(ReplyHandleHttp, HttpResponseContents, bool)>, /// outstanding request @@ -115,7 +113,6 @@ impl ConversationHttp { peer_addr, outbound_url, peer_host, - canonical_stacks_tip_height: None, pending_request: None, pending_response: None, keep_alive: true, From faab928ee97e5a30a8e5555439c1ecb7820ce2a1 Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Tue, 5 Aug 2025 11:58:31 +0100 Subject: [PATCH 03/10] remove code and tests relative to the old health implementation --- stackslib/src/net/db.rs | 36 --- .../nakamoto/download_state_machine.rs | 65 +---- stackslib/src/net/inv/epoch2x.rs | 48 ---- stackslib/src/net/mod.rs | 47 +-- stackslib/src/net/tests/inv/epoch2x.rs | 269 ------------------ 5 files changed, 27 insertions(+), 438 deletions(-) diff --git a/stackslib/src/net/db.rs b/stackslib/src/net/db.rs index 0325970b7f..74406db49a 100644 --- a/stackslib/src/net/db.rs +++ b/stackslib/src/net/db.rs @@ -1773,42 +1773,6 @@ impl PeerDB { ) } - pub fn get_valid_initial_neighbors( - conn: &DBConn, - network_id: u32, - network_epoch: u8, - peer_version: u32, - burn_block_height: u64, - ) -> Result, db_error> { - // UTC time - let now_secs = util::get_epoch_time_secs(); - // Extract the epoch from the peer_version. The epoch is stored in the last byte. - let node_peer_version = peer_version & 0x000000ff; - - // the peer_version check mirrors the check in `has_acceptable_epoch`: - // (my_epoch <= peer_epoch) OR (curr_epoch <= peer_epoch) - let query = r#" - SELECT * - FROM frontier - WHERE initial = 1 - AND (allowed < 0 OR ?1 < allowed) - AND network_id = ?2 - AND denied < ?3 - AND ?4 < expire_block_height - AND (?5 <= (peer_version & 0x000000ff) OR ?6 <= (peer_version & 0x000000ff))"#; - - let args = params![ - u64_to_sql(now_secs)?, - network_id, - u64_to_sql(now_secs)?, - u64_to_sql(burn_block_height)?, - node_peer_version, - network_epoch, - ]; - - Self::query_peers(conn, query, args) - } - /// Get a randomized set of peers for walking the peer graph. /// -- selects peers at random even if not allowed /// -- may include private IPs diff --git a/stackslib/src/net/download/nakamoto/download_state_machine.rs b/stackslib/src/net/download/nakamoto/download_state_machine.rs index 566f17f66e..0450971d9e 100644 --- a/stackslib/src/net/download/nakamoto/download_state_machine.rs +++ b/stackslib/src/net/download/nakamoto/download_state_machine.rs @@ -73,7 +73,7 @@ pub struct NakamotoDownloadStateMachine { /// Last burnchain tip we've seen last_sort_tip: Option, /// Download behavior we're in - pub(crate) state: NakamotoDownloadState, + state: NakamotoDownloadState, /// Map a tenure ID to its tenure start-block and end-block for each of our neighbors' invs tenure_block_ids: HashMap, /// Who can serve a given tenure @@ -83,10 +83,9 @@ pub struct NakamotoDownloadStateMachine { /// Unconfirmed tenure download schedule unconfirmed_tenure_download_schedule: VecDeque, /// Ongoing unconfirmed tenure downloads, prioritized in who announces the latest block - pub(crate) unconfirmed_tenure_downloads: - HashMap, + unconfirmed_tenure_downloads: HashMap, /// Ongoing confirmed tenure downloads for when we know the start and end block hashes. - pub(crate) tenure_downloads: NakamotoTenureDownloaderSet, + tenure_downloads: NakamotoTenureDownloaderSet, /// comms to remote neighbors pub(super) neighbor_rpc: NeighborRPC, /// Nakamoto chain tip @@ -676,64 +675,6 @@ impl NakamotoDownloadStateMachine { schedule.into_iter().map(|(_count, ch)| ch).collect() } - /// Returns the highest Stacks tip height from available sources. - /// - /// In IBD mode (Confirmed state), this queries confirmed tenure downloaders for their - /// tenure end block heights. In steady-state mode (Unconfirmed state), this checks - /// the unconfirmed tenure downloads for the given neighbors to find their tip heights. - /// - /// # Arguments - /// - /// * `neighbors` - Optional slice of `NeighborAddress` structs to check. - /// If `None`, all neighbors are considered. - /// - /// # Returns - /// - /// * `Some(u64)` - The maximum height found, or None if no heights are available. - pub(crate) fn get_max_stacks_height_of_neighbors( - &self, - neighbors: Option<&[NeighborAddress]>, - ) -> Option { - match self.state { - // Still in IBD mode, so we can only get the max height from the confirmed tenure downloads - NakamotoDownloadState::Confirmed => self - .tenure_downloads - .downloaders - .iter() - .flatten() - .filter(|d| neighbors.map_or(true, |n| n.contains(&d.naddr))) - .filter_map(|d| { - d.tenure_end_block - .as_ref() - .map(|end_block| end_block.header.chain_length + 1) - .or_else(|| { - d.tenure_start_block - .as_ref() - .map(|start_block| start_block.header.chain_length + 1) - }) - }) - .max(), - // In steady-state mode, we can get the max height from the unconfirmed tenure downloads - NakamotoDownloadState::Unconfirmed => match neighbors { - None => self - .unconfirmed_tenure_downloads - .values() - .filter_map(|d| d.tenure_tip.as_ref().map(|tip| tip.tip_height)) - .max(), - Some(addrs) => addrs - .iter() - .filter_map(|addr| { - self.unconfirmed_tenure_downloads - .get(addr)? - .tenure_tip - .as_ref() - .map(|tip| tip.tip_height) - }) - .max(), - }, - } - } - /// How many neighbors can we contact still, given the map of tenures to neighbors which can /// serve it? fn count_available_tenure_neighbors( diff --git a/stackslib/src/net/inv/epoch2x.rs b/stackslib/src/net/inv/epoch2x.rs index 06e21e416f..36f6a1ac15 100644 --- a/stackslib/src/net/inv/epoch2x.rs +++ b/stackslib/src/net/inv/epoch2x.rs @@ -1134,54 +1134,6 @@ impl InvState { list } - /// Returns the highest Stacks tip height reported by the given neighbors. - /// - /// This function iterates through the provided neighbors, checks their block stats, - /// and determines the maximum block height. The status of the neighbor (Online or Diverged during IBD, - /// or Online when not in IBD) is considered. - /// - /// # Arguments - /// - /// * `neighbors` - Optional slice of `Neighbor` structs to check. If `None`, all neighbors are considered. - /// * `ibd` - A boolean indicating if the node is in Initial Block Download (IBD) mode. - /// - /// # Returns - /// - /// * `Some(u64)` if at least one neighbor has a tip height according to its status. - /// * `None` if no tip heights are found. - pub fn get_max_stacks_height_of_neighbors( - &self, - neighbors: Option<&[Neighbor]>, - ibd: bool, - ) -> Option { - let filter_and_extract = |stats: &NeighborBlockStats| -> Option { - let status_valid = if ibd { - stats.status == NodeStatus::Online || stats.status == NodeStatus::Diverged - } else { - stats.status == NodeStatus::Online - }; - - if status_valid { - Some(stats.inv.get_block_height()) - } else { - None - } - }; - - match neighbors { - Some(n) => n - .iter() - .filter_map(|neighbor| self.block_stats.get(&neighbor.addr)) - .filter_map(filter_and_extract) - .max(), - None => self - .block_stats - .values() - .filter_map(filter_and_extract) - .max(), - } - } - /// Get the list of dead pub fn get_dead_peers(&self) -> Vec { let mut list = vec![]; diff --git a/stackslib/src/net/mod.rs b/stackslib/src/net/mod.rs index f477dcdc13..6a4c902ed4 100644 --- a/stackslib/src/net/mod.rs +++ b/stackslib/src/net/mod.rs @@ -2215,27 +2215,6 @@ pub trait Requestable: std::fmt::Display { fn make_request_type(&self, peer_host: PeerHost) -> StacksHttpRequest; } -// TODO: DRY up from PoxSyncWatchdog -pub fn infer_initial_burnchain_block_download( - burnchain: &Burnchain, - last_processed_height: u64, - burnchain_height: u64, -) -> bool { - let ibd = last_processed_height + (burnchain.stable_confirmations as u64) < burnchain_height; - if ibd { - debug!( - "PoX watchdog: {} + {} < {}, so initial block download", - last_processed_height, burnchain.stable_confirmations, burnchain_height - ); - } else { - debug!( - "PoX watchdog: {} + {} >= {}, so steady-state", - last_processed_height, burnchain.stable_confirmations, burnchain_height - ); - } - ibd -} - #[cfg(test)] pub mod test { use std::collections::HashMap; @@ -3326,6 +3305,28 @@ pub mod test { tx.commit().unwrap(); } + // TODO: DRY up from PoxSyncWatchdog + pub fn infer_initial_burnchain_block_download( + burnchain: &Burnchain, + last_processed_height: u64, + burnchain_height: u64, + ) -> bool { + let ibd = + last_processed_height + (burnchain.stable_confirmations as u64) < burnchain_height; + if ibd { + debug!( + "PoX watchdog: {} + {} < {}, so initial block download", + last_processed_height, burnchain.stable_confirmations, burnchain_height + ); + } else { + debug!( + "PoX watchdog: {} + {} >= {}, so steady-state", + last_processed_height, burnchain.stable_confirmations, burnchain_height + ); + } + ibd + } + pub fn step(&mut self) -> Result { let sortdb = self.sortdb.take().unwrap(); let stacks_node = self.stacks_node.take().unwrap(); @@ -3339,7 +3340,7 @@ pub mod test { .unwrap() .map(|hdr| hdr.anchored_header.height()) .unwrap_or(0); - let ibd = infer_initial_burnchain_block_download( + let ibd = TestPeer::infer_initial_burnchain_block_download( &self.config.burnchain, stacks_tip_height, burn_tip_height, @@ -3470,7 +3471,7 @@ pub mod test { .unwrap() .map(|hdr| hdr.anchored_header.height()) .unwrap_or(0); - let ibd = infer_initial_burnchain_block_download( + let ibd = TestPeer::infer_initial_burnchain_block_download( &self.config.burnchain, stacks_tip_height, burn_tip_height, diff --git a/stackslib/src/net/tests/inv/epoch2x.rs b/stackslib/src/net/tests/inv/epoch2x.rs index e9d994e703..222a6a3546 100644 --- a/stackslib/src/net/tests/inv/epoch2x.rs +++ b/stackslib/src/net/tests/inv/epoch2x.rs @@ -17,9 +17,6 @@ use std::collections::HashMap; use stacks_common::deps_common::bitcoin::network::serialize::BitcoinHash; -use stacks_common::types::net::PeerAddress; -use stacks_common::util::hash::Hash160; -use stacks_common::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use crate::burnchains::bitcoin::indexer::BitcoinIndexer; use crate::burnchains::db::BurnchainHeaderReader; @@ -2016,269 +2013,3 @@ fn test_sync_inv_2_peers_different_pox_vectors() { assert!(!peer_2_inv.has_ith_microblock_stream(num_blocks - 2 * reward_cycle_length)); }) } - -/// Helper function to create a test neighbor without binding to a port -fn create_test_neighbor(port: u16) -> Neighbor { - let private_key = Secp256k1PrivateKey::random(); - let public_key = Secp256k1PublicKey::from_private(&private_key); - let public_key_hash = Hash160::from_node_public_key(&public_key); - - let neighbor_address = NeighborAddress { - addrbytes: PeerAddress::from_ipv4(127, 0, 0, 1), - port, - public_key_hash, - }; - - let neighbor_key = NeighborKey::from_neighbor_address( - 0x18000000, // peer_version - 0x80000000, // network_id - &neighbor_address, - ); - - Neighbor::empty(&neighbor_key, &public_key, 9999999) -} - -/// Specification for a test peer -struct TestPeerSpec { - port: u16, - status: NodeStatus, - num_sortitions: u64, -} - -impl TestPeerSpec { - fn new(port: u16, status: NodeStatus, num_sortitions: u64) -> Self { - Self { - port, - status, - num_sortitions, - } - } -} - -/// Helper function to setup multiple peers at once -fn setup_test_inv_state(specs: &[TestPeerSpec]) -> (InvState, Vec) { - // first_block_height=100, so heights = 100 + num_sortitions - let mut inv_state = InvState::new(100, 60, 3); - let mut neighbors = Vec::new(); - - for spec in specs { - let neighbor = create_test_neighbor(spec.port); - let neighbor_key = neighbor.addr.clone(); - - inv_state.add_peer(neighbor_key.clone(), false); - if let Some(stats) = inv_state.get_stats_mut(&neighbor_key) { - stats.status = spec.status; - stats.inv.num_sortitions = spec.num_sortitions; - } - - neighbors.push(neighbor); - } - - (inv_state, neighbors) -} - -/// Helper function to create neighbors without adding them to inv_state -fn create_test_neighbors(ports: &[u16]) -> Vec { - ports - .iter() - .map(|&port| create_test_neighbor(port)) - .collect() -} - -/// Helper function to assert the expected heights for both IBD and non-IBD modes -fn assert_max_heights( - inv_state: &InvState, - neighbors: Option<&[Neighbor]>, - expected_non_ibd: Option, - expected_ibd: Option, -) { - assert_eq!( - inv_state.get_max_stacks_height_of_neighbors(neighbors, false), - expected_non_ibd, - "Non-IBD mode assertion failed" - ); - assert_eq!( - inv_state.get_max_stacks_height_of_neighbors(neighbors, true), - expected_ibd, - "IBD mode assertion failed" - ); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors() { - // Test empty neighbors list - let (inv_state, neighbors) = setup_test_inv_state(&[]); - assert_max_heights(&inv_state, Some(&neighbors), None, None); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_no_stats() { - // Test neighbors without any stats in the inv_state - let inv_state = InvState::new(100, 60, 3); - let neighbors = create_test_neighbors(&[8080]); - - // Should return None since no stats exist for the neighbor - assert_max_heights(&inv_state, Some(&neighbors), None, None); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_single_online_peer() { - let (inv_state, neighbors) = - setup_test_inv_state(&[TestPeerSpec::new(8080, NodeStatus::Online, 150)]); - - // Online peers should be accepted in both modes (height = 100 + 150 = 250) - assert_max_heights(&inv_state, Some(&neighbors), Some(250), Some(250)); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_single_diverged_peer() { - let (inv_state, neighbors) = - setup_test_inv_state(&[TestPeerSpec::new(8080, NodeStatus::Diverged, 200)]); - - // Diverged peers accepted only in IBD mode (height = 100 + 200 = 300) - assert_max_heights(&inv_state, Some(&neighbors), None, Some(300)); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_single_broken_peer() { - let (inv_state, neighbors) = - setup_test_inv_state(&[TestPeerSpec::new(8080, NodeStatus::Broken, 180)]); - - // Broken peers never accepted - assert_max_heights(&inv_state, Some(&neighbors), None, None); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_single_stale_peer() { - let (inv_state, neighbors) = - setup_test_inv_state(&[TestPeerSpec::new(8080, NodeStatus::Stale, 120)]); - - // Stale peers never accepted - assert_max_heights(&inv_state, Some(&neighbors), None, None); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_single_dead_peer() { - let (inv_state, neighbors) = - setup_test_inv_state(&[TestPeerSpec::new(8080, NodeStatus::Dead, 190)]); - - // Dead peers never accepted - assert_max_heights(&inv_state, Some(&neighbors), None, None); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_multiple_online_peers() { - let specs = [ - TestPeerSpec::new(8080, NodeStatus::Online, 100), // height = 200 - TestPeerSpec::new(8081, NodeStatus::Online, 250), // height = 350 (max) - TestPeerSpec::new(8082, NodeStatus::Online, 180), // height = 280 - ]; - - let (inv_state, neighbors) = setup_test_inv_state(&specs); - - // Should return max height among all online peers (350) - assert_max_heights(&inv_state, Some(&neighbors), Some(350), Some(350)); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_mixed_statuses_non_ibd() { - let specs = [ - TestPeerSpec::new(8080, NodeStatus::Online, 150), // height = 250 - TestPeerSpec::new(8081, NodeStatus::Diverged, 300), // height = 400 (ignored in non-IBD) - TestPeerSpec::new(8082, NodeStatus::Online, 200), // height = 300 (max among Online) - TestPeerSpec::new(8083, NodeStatus::Broken, 350), // ignored - ]; - - let (inv_state, neighbors) = setup_test_inv_state(&specs); - - // Non-IBD: only Online peers considered, max = 300 - assert_max_heights(&inv_state, Some(&neighbors), Some(300), Some(400)); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_mixed_statuses_ibd() { - let specs = [ - TestPeerSpec::new(8080, NodeStatus::Online, 150), // height = 250 - TestPeerSpec::new(8081, NodeStatus::Diverged, 300), // height = 400 (max in IBD) - TestPeerSpec::new(8082, NodeStatus::Online, 200), // height = 300 - TestPeerSpec::new(8083, NodeStatus::Broken, 350), // ignored even in IBD - ]; - - let (inv_state, neighbors) = setup_test_inv_state(&specs); - - // IBD: both Online and Diverged considered, max = 400 - assert_max_heights(&inv_state, Some(&neighbors), Some(300), Some(400)); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_minimum_height() { - let (inv_state, neighbors) = - setup_test_inv_state(&[TestPeerSpec::new(8080, NodeStatus::Online, 0)]); - - // Should handle minimum height correctly (height = 100 + 0 = 100) - assert_max_heights(&inv_state, Some(&neighbors), Some(100), Some(100)); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_all_invalid_statuses() { - let specs = [ - TestPeerSpec::new(8080, NodeStatus::Broken, 150), - TestPeerSpec::new(8081, NodeStatus::Dead, 200), - ]; - - let (inv_state, neighbors) = setup_test_inv_state(&specs); - - // All invalid statuses should return None - assert_max_heights(&inv_state, Some(&neighbors), None, None); -} - -#[test] -fn test_get_max_stacks_height_of_neighbors_diverged_only_non_ibd() { - let specs = [ - TestPeerSpec::new(8080, NodeStatus::Diverged, 150), // height = 250 - TestPeerSpec::new(8081, NodeStatus::Diverged, 200), // height = 300 (max) - ]; - - let (inv_state, neighbors) = setup_test_inv_state(&specs); - - // Non-IBD: Diverged ignored, IBD: Diverged accepted - assert_max_heights(&inv_state, Some(&neighbors), None, Some(300)); -} - -#[test] -fn test_get_max_stacks_height_of_all_neighbors_mixed_statuses() { - let specs = [ - TestPeerSpec::new(8080, NodeStatus::Online, 150), // height = 250 - TestPeerSpec::new(8081, NodeStatus::Diverged, 300), // height = 400 - TestPeerSpec::new(8082, NodeStatus::Online, 200), // height = 300 - TestPeerSpec::new(8083, NodeStatus::Broken, 350), // ignored - ]; - - let (inv_state, _neighbors) = setup_test_inv_state(&specs); - - // When neighbors is None, it should scan all peers in block_stats. - // Non-IBD: only Online peers considered, max height = 300. - // IBD: both Online and Diverged considered, max height = 400. - assert_max_heights(&inv_state, None, Some(300), Some(400)); -} - -#[test] -fn test_get_max_stacks_height_of_all_neighbors_empty_stats() { - // inv_state with no block_stats - let inv_state = InvState::new(100, 60, 3); - assert_max_heights(&inv_state, None, None, None); -} - -#[test] -fn test_get_max_stacks_height_of_all_neighbors_no_valid_peers() { - let specs = [ - TestPeerSpec::new(8080, NodeStatus::Broken, 150), - TestPeerSpec::new(8081, NodeStatus::Dead, 200), - TestPeerSpec::new(8082, NodeStatus::Stale, 250), - ]; - - let (inv_state, _neighbors) = setup_test_inv_state(&specs); - - // No peers with Online or Diverged status - assert_max_heights(&inv_state, None, None, None); -} From 8509b5a62bb701e805e23a8d4579e2d7ec089c67 Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Tue, 5 Aug 2025 13:23:37 +0100 Subject: [PATCH 04/10] update openapi spec --- docs/rpc/openapi.yaml | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/docs/rpc/openapi.yaml b/docs/rpc/openapi.yaml index 63f3e8fbbf..754d0ce895 100644 --- a/docs/rpc/openapi.yaml +++ b/docs/rpc/openapi.yaml @@ -1299,20 +1299,6 @@ paths: - Info security: [] operationId: getNodeHealth - parameters: - - in: query - name: neighbors - description: | - Specifies the set of peers to use for health checks. - - `initial` (default): Use only the initial bootstrap peers. - - `all`: Use all connected peers. - required: false - schema: - type: string - enum: - - initial - - all - default: initial responses: "200": description: Success @@ -1323,11 +1309,8 @@ paths: example: $ref: "./components/examples/node-health.example.json" "400": - description: Bad request, such as an invalid `neighbors` query parameter. $ref: "#/components/responses/BadRequest" "500": - description: | - Failed to query for health (e.g., no data or no valid peers to query from). $ref: "#/components/responses/InternalServerError" /v2/attachments/{hash}: From 87f9bba6b429a86c747000e5266a9b2014bce5ad Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Tue, 5 Aug 2025 13:23:51 +0100 Subject: [PATCH 05/10] remove unused import --- stackslib/src/net/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stackslib/src/net/mod.rs b/stackslib/src/net/mod.rs index 6a4c902ed4..b796b270cd 100644 --- a/stackslib/src/net/mod.rs +++ b/stackslib/src/net/mod.rs @@ -38,7 +38,7 @@ use {rusqlite, url}; use self::dns::*; use crate::burnchains::affirmation::AffirmationMap; -use crate::burnchains::{Burnchain, Error as burnchain_error, Txid}; +use crate::burnchains::{Error as burnchain_error, Txid}; use crate::chainstate::burn::db::sortdb::SortitionDB; use crate::chainstate::burn::ConsensusHash; use crate::chainstate::coordinator::comm::CoordinatorChannels; From 2ad62bc7b936272ad23b446720c6223d8df85447 Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Wed, 13 Aug 2025 14:57:07 +0100 Subject: [PATCH 06/10] track addr of highest peer --- stackslib/src/net/api/gethealth.rs | 9 ++++++- stackslib/src/net/api/tests/gethealth.rs | 10 +++++++- stackslib/src/net/mod.rs | 20 ++++++++++++--- stackslib/src/net/p2p.rs | 7 +++--- stackslib/src/net/rpc.rs | 3 ++- stackslib/src/net/tests/mod.rs | 32 ++++++++++++++++-------- 6 files changed, 60 insertions(+), 21 deletions(-) diff --git a/stackslib/src/net/api/gethealth.rs b/stackslib/src/net/api/gethealth.rs index 36961bbb14..791a0fc22c 100644 --- a/stackslib/src/net/api/gethealth.rs +++ b/stackslib/src/net/api/gethealth.rs @@ -35,6 +35,8 @@ pub struct RPCGetHealthResponse { pub difference_from_max_peer: u64, /// the max height of the node's most advanced neighbor pub max_stacks_height_of_neighbors: u64, + /// the address of the node's most advanced neighbor + pub max_stacks_neighbor_address: Option, /// the height of this node pub node_stacks_tip_height: u64, } @@ -105,11 +107,15 @@ impl RPCRequestHandler for RPCGetHealthRequestHandler { let (max_stacks_height_of_neighbors, node_stacks_tip_height) = node.with_node_state(|network, _sortdb, _chainstate, _mempool, _rpc_args| { ( - network.highest_stacks_height_of_neighbors, + network.highest_stacks_neighbor, network.stacks_tip.height, ) }); + let (max_stacks_neighbor_address, max_stacks_height_of_neighbors) = match max_stacks_height_of_neighbors { + Some((addr, height)) => (Some(addr.to_string()), height), + None => (None, 0), + }; // There could be a edge case where our node is ahead of all peers. let difference_from_max_peer = max_stacks_height_of_neighbors.saturating_sub(node_stacks_tip_height); @@ -118,6 +124,7 @@ impl RPCRequestHandler for RPCGetHealthRequestHandler { let data = RPCGetHealthResponse { difference_from_max_peer, max_stacks_height_of_neighbors, + max_stacks_neighbor_address, node_stacks_tip_height, }; let body = HttpResponseContents::try_from_json(&data)?; diff --git a/stackslib/src/net/api/tests/gethealth.rs b/stackslib/src/net/api/tests/gethealth.rs index f1428495fa..e287b81655 100644 --- a/stackslib/src/net/api/tests/gethealth.rs +++ b/stackslib/src/net/api/tests/gethealth.rs @@ -56,6 +56,7 @@ fn test_get_health( #[case] expected_difference_from_max_peer: u64, ) { // `rpc_test` will have peer_1 (client) and peer_2 (server/node) + let test_observer = TestEventObserver::new(); let mut rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); @@ -69,7 +70,8 @@ fn test_get_health( } else { node_stacks_tip_height + (peer_1_height_relative_to_node as u64) }; - rpc_test.peer_2.network.highest_stacks_height_of_neighbors = peer_1_actual_height; + let peer_1_addr: SocketAddr = rpc_test.peer_1.config.data_url.parse().unwrap(); + rpc_test.peer_2.network.highest_stacks_neighbor = Some((peer_1_addr.clone(), peer_1_actual_height)); // --- Invoke the Handler --- let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); @@ -99,6 +101,12 @@ fn test_get_health( health_response.difference_from_max_peer, expected_difference_from_max_peer, "Mismatch in difference_from_max_peer" ); + + assert_eq!( + health_response.max_stacks_neighbor_address, + Some(peer_1_addr.to_string()), + "Mismatch in max_stacks_neighbor_address" + ); } #[test] diff --git a/stackslib/src/net/mod.rs b/stackslib/src/net/mod.rs index b796b270cd..e1fb2ad521 100644 --- a/stackslib/src/net/mod.rs +++ b/stackslib/src/net/mod.rs @@ -807,11 +807,23 @@ impl<'a> StacksNodeState<'a> { }) } - pub fn update_highest_stacks_height_of_neighbors(&mut self, new_height: Option) { + pub fn update_highest_stacks_neighbor( + &mut self, + new_address: &SocketAddr, + new_height: Option, + ) { self.with_node_state(|network, _, _, _, _| { - network.highest_stacks_height_of_neighbors = network - .highest_stacks_height_of_neighbors - .max(new_height.unwrap_or(0)); + if let Some(new_height) = new_height { + let current_height = network + .highest_stacks_neighbor + .as_ref() + .map(|(_addr, height)| *height) + .unwrap_or(0); + + if new_height > current_height { + network.highest_stacks_neighbor = Some((new_address.clone(), new_height)); + } + } }); } } diff --git a/stackslib/src/net/p2p.rs b/stackslib/src/net/p2p.rs index d3f94027a1..ad9b64aa02 100644 --- a/stackslib/src/net/p2p.rs +++ b/stackslib/src/net/p2p.rs @@ -639,8 +639,9 @@ pub struct PeerNetwork { /// Thread handle for the async block proposal endpoint. block_proposal_thread: Option>, - /// Highest stacks block height received from RPC responses from neighbors - pub highest_stacks_height_of_neighbors: u64, + /// Address and height of the neighbor that reported the highest Stacks block height + /// via RPC responses + pub highest_stacks_neighbor: Option<(SocketAddr, u64)>, } impl PeerNetwork { @@ -803,7 +804,7 @@ impl PeerNetwork { block_proposal_thread: None, - highest_stacks_height_of_neighbors: 0, + highest_stacks_neighbor: None, }; network.init_block_downloader(); diff --git a/stackslib/src/net/rpc.rs b/stackslib/src/net/rpc.rs index c8db1b2553..a02ffb98ea 100644 --- a/stackslib/src/net/rpc.rs +++ b/stackslib/src/net/rpc.rs @@ -532,7 +532,8 @@ impl ConversationHttp { info!("Handled StacksHTTPRequest Error"; "path" => %path, "processing_time_ms" => start_time.elapsed().as_millis(), "conn_id" => self.conn_id, "peer_addr" => &self.peer_addr); } StacksHttpMessage::Response(resp) => { - node.update_highest_stacks_height_of_neighbors( + node.update_highest_stacks_neighbor( + &self.get_peer_addr(), resp.preamble().get_canonical_stacks_tip_height(), ); // Is there someone else waiting for this message? If so, pass it along. diff --git a/stackslib/src/net/tests/mod.rs b/stackslib/src/net/tests/mod.rs index 752b460af0..793d8600aa 100644 --- a/stackslib/src/net/tests/mod.rs +++ b/stackslib/src/net/tests/mod.rs @@ -23,6 +23,7 @@ pub mod neighbors; pub mod relay; use std::collections::{HashMap, HashSet}; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use clarity::vm::types::{PrincipalData, QualifiedContractIdentifier}; use libstackerdb::StackerDBChunkData; @@ -1808,19 +1809,23 @@ fn test_network_result_update() { } #[rstest] -#[case::zero_to_none(0, None, 0)] -#[case::zero_to_some(0, Some(10), 10)] -#[case::some_to_none(10, None, 10)] -#[case::new_height_higher(10, Some(20), 20)] -#[case::new_height_lower(20, Some(10), 20)] +#[case::none_to_none(None, None, 0)] +#[case::none_to_some(None, Some(10), 10)] +#[case::some_to_none(Some(10), None, 10)] +#[case::new_height_higher(Some(10), Some(20), 20)] +#[case::new_height_lower(Some(20), Some(10), 20)] fn test_update_highest_stacks_height_of_neighbors( - #[case] old_height: u64, + #[case] old_height: Option, #[case] new_height: Option, #[case] expected_height: u64, ) { let peer_config = TestPeerConfig::new(function_name!(), 0, 0); let mut peer = TestPeer::new(peer_config); - peer.network.highest_stacks_height_of_neighbors = old_height; + if let Some(old_height) = old_height { + peer.network.highest_stacks_neighbor = Some((SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080), old_height)); + } else { + peer.network.highest_stacks_neighbor = None; + } let peer_sortdb = peer.sortdb.take().unwrap(); let mut peer_stacks_node = peer.stacks_node.take().unwrap(); let mut peer_mempool = peer.mempool.take().unwrap(); @@ -1835,9 +1840,14 @@ fn test_update_highest_stacks_height_of_neighbors( false, ); - node_state.update_highest_stacks_height_of_neighbors(new_height); + let new_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8081); + node_state.update_highest_stacks_neighbor(&new_addr, new_height); + if let Some(new_height) = new_height { assert_eq!( - peer.network.highest_stacks_height_of_neighbors, - expected_height - ); + peer.network.highest_stacks_neighbor, + Some((new_addr, expected_height)) + ); + } else { + assert_eq!(peer.network.highest_stacks_neighbor, None); + } } From d97f7abb17dbc560edea8d356d5ffc6eaeceadb3 Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Wed, 13 Aug 2025 15:12:26 +0100 Subject: [PATCH 07/10] update openapi spec --- docs/rpc/components/examples/node-health.example.json | 3 ++- docs/rpc/components/schemas/get-health.schema.yaml | 3 +++ docs/rpc/openapi.yaml | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/rpc/components/examples/node-health.example.json b/docs/rpc/components/examples/node-health.example.json index 472b1e383c..285f5eb9ff 100644 --- a/docs/rpc/components/examples/node-health.example.json +++ b/docs/rpc/components/examples/node-health.example.json @@ -1,5 +1,6 @@ { "difference_from_max_peer": 0, + "node_stacks_tip_height": 12345, "max_stacks_height_of_neighbors": 12345, - "node_stacks_tip_height": 12345 + "max_stacks_neighbor_address": "127.0.0.1:8080" } diff --git a/docs/rpc/components/schemas/get-health.schema.yaml b/docs/rpc/components/schemas/get-health.schema.yaml index 259a2cc280..743c8792e6 100644 --- a/docs/rpc/components/schemas/get-health.schema.yaml +++ b/docs/rpc/components/schemas/get-health.schema.yaml @@ -16,4 +16,7 @@ properties: type: integer minimum: 0 description: The current Stacks tip height of this node + max_stacks_neighbor_address: + type: [string, "null"] + description: The address of the most advanced peer description: Health information about the node's synchronization status diff --git a/docs/rpc/openapi.yaml b/docs/rpc/openapi.yaml index 754d0ce895..0fbd7a0042 100644 --- a/docs/rpc/openapi.yaml +++ b/docs/rpc/openapi.yaml @@ -1295,6 +1295,7 @@ paths: - `difference_from_max_peer`: The difference in Stacks height between this node and its most advanced peer. - `max_stacks_height_of_neighbors`: The maximum Stacks height observed among the node"s connected peers. - `node_stacks_tip_height`: The current Stacks tip height of this node. + - `max_stacks_neighbor_address`: The address of the most advanced peer. Null if no peer data is available. tags: - Info security: [] From c9dbe315adb2ee185e6448562b702e4817f3e045 Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Wed, 13 Aug 2025 15:12:59 +0100 Subject: [PATCH 08/10] cargo fmt --- stackslib/src/net/api/gethealth.rs | 14 ++++++-------- stackslib/src/net/api/tests/gethealth.rs | 3 ++- stackslib/src/net/tests/mod.rs | 9 ++++++--- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/stackslib/src/net/api/gethealth.rs b/stackslib/src/net/api/gethealth.rs index 791a0fc22c..7a45fa5a17 100644 --- a/stackslib/src/net/api/gethealth.rs +++ b/stackslib/src/net/api/gethealth.rs @@ -106,16 +106,14 @@ impl RPCRequestHandler for RPCGetHealthRequestHandler { ) -> Result<(HttpResponsePreamble, HttpResponseContents), NetError> { let (max_stacks_height_of_neighbors, node_stacks_tip_height) = node.with_node_state(|network, _sortdb, _chainstate, _mempool, _rpc_args| { - ( - network.highest_stacks_neighbor, - network.stacks_tip.height, - ) + (network.highest_stacks_neighbor, network.stacks_tip.height) }); - let (max_stacks_neighbor_address, max_stacks_height_of_neighbors) = match max_stacks_height_of_neighbors { - Some((addr, height)) => (Some(addr.to_string()), height), - None => (None, 0), - }; + let (max_stacks_neighbor_address, max_stacks_height_of_neighbors) = + match max_stacks_height_of_neighbors { + Some((addr, height)) => (Some(addr.to_string()), height), + None => (None, 0), + }; // There could be a edge case where our node is ahead of all peers. let difference_from_max_peer = max_stacks_height_of_neighbors.saturating_sub(node_stacks_tip_height); diff --git a/stackslib/src/net/api/tests/gethealth.rs b/stackslib/src/net/api/tests/gethealth.rs index e287b81655..da012cc2e4 100644 --- a/stackslib/src/net/api/tests/gethealth.rs +++ b/stackslib/src/net/api/tests/gethealth.rs @@ -71,7 +71,8 @@ fn test_get_health( node_stacks_tip_height + (peer_1_height_relative_to_node as u64) }; let peer_1_addr: SocketAddr = rpc_test.peer_1.config.data_url.parse().unwrap(); - rpc_test.peer_2.network.highest_stacks_neighbor = Some((peer_1_addr.clone(), peer_1_actual_height)); + rpc_test.peer_2.network.highest_stacks_neighbor = + Some((peer_1_addr.clone(), peer_1_actual_height)); // --- Invoke the Handler --- let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); diff --git a/stackslib/src/net/tests/mod.rs b/stackslib/src/net/tests/mod.rs index 793d8600aa..f534634330 100644 --- a/stackslib/src/net/tests/mod.rs +++ b/stackslib/src/net/tests/mod.rs @@ -1822,7 +1822,10 @@ fn test_update_highest_stacks_height_of_neighbors( let peer_config = TestPeerConfig::new(function_name!(), 0, 0); let mut peer = TestPeer::new(peer_config); if let Some(old_height) = old_height { - peer.network.highest_stacks_neighbor = Some((SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080), old_height)); + peer.network.highest_stacks_neighbor = Some(( + SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080), + old_height, + )); } else { peer.network.highest_stacks_neighbor = None; } @@ -1843,8 +1846,8 @@ fn test_update_highest_stacks_height_of_neighbors( let new_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8081); node_state.update_highest_stacks_neighbor(&new_addr, new_height); if let Some(new_height) = new_height { - assert_eq!( - peer.network.highest_stacks_neighbor, + assert_eq!( + peer.network.highest_stacks_neighbor, Some((new_addr, expected_height)) ); } else { From 4b07fd7cc6d0db01cdab4eaab1b2979ada96dec0 Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Thu, 14 Aug 2025 11:34:59 +0100 Subject: [PATCH 09/10] fix gethealth test --- stackslib/src/net/api/tests/gethealth.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/stackslib/src/net/api/tests/gethealth.rs b/stackslib/src/net/api/tests/gethealth.rs index da012cc2e4..540ded6a60 100644 --- a/stackslib/src/net/api/tests/gethealth.rs +++ b/stackslib/src/net/api/tests/gethealth.rs @@ -24,7 +24,7 @@ use crate::net::ProtocolFamily; #[test] fn test_try_parse_request() { - let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); + let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 33333); let mut http = StacksHttp::new(addr.clone(), &ConnectionOptions::default()); let request = StacksHttpRequest::new_gethealth(addr.into()); @@ -70,12 +70,15 @@ fn test_get_health( } else { node_stacks_tip_height + (peer_1_height_relative_to_node as u64) }; - let peer_1_addr: SocketAddr = rpc_test.peer_1.config.data_url.parse().unwrap(); + let peer_1_addr = SocketAddr::new( + IpAddr::V4(Ipv4Addr::LOCALHOST), + rpc_test.peer_1.config.http_port, + ); rpc_test.peer_2.network.highest_stacks_neighbor = Some((peer_1_addr.clone(), peer_1_actual_height)); // --- Invoke the Handler --- - let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); + let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 33333); let request = StacksHttpRequest::new_gethealth(addr.into()); let mut responses = rpc_test.run(vec![request]); let response = responses.remove(0); @@ -115,7 +118,7 @@ fn test_get_health_no_peers_stats() { let test_observer = TestEventObserver::new(); let rpc_test = TestRPC::setup_nakamoto(function_name!(), &test_observer); // --- Invoke the Handler --- - let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333); + let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 33333); let request = StacksHttpRequest::new_gethealth(addr.into()); let mut responses = rpc_test.run(vec![request]); let response = responses.remove(0); From 8076ce1a568f1d769583c689cf0ac6828478f375 Mon Sep 17 00:00:00 2001 From: Francesco Leacche Date: Thu, 14 Aug 2025 15:10:05 +0100 Subject: [PATCH 10/10] fix test_update_highest_stacks_height_of_neighbors --- stackslib/src/net/tests/mod.rs | 43 ++++++++++++++++------------------ 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/stackslib/src/net/tests/mod.rs b/stackslib/src/net/tests/mod.rs index f534634330..20437817f2 100644 --- a/stackslib/src/net/tests/mod.rs +++ b/stackslib/src/net/tests/mod.rs @@ -1809,26 +1809,23 @@ fn test_network_result_update() { } #[rstest] -#[case::none_to_none(None, None, 0)] -#[case::none_to_some(None, Some(10), 10)] -#[case::some_to_none(Some(10), None, 10)] -#[case::new_height_higher(Some(10), Some(20), 20)] -#[case::new_height_lower(Some(20), Some(10), 20)] +#[case(None, None, false)] +#[case(None, Some(10), true)] +#[case(Some(10), None, false)] +#[case(Some(10), Some(20), true)] +#[case(Some(20), Some(10), false)] fn test_update_highest_stacks_height_of_neighbors( #[case] old_height: Option, #[case] new_height: Option, - #[case] expected_height: u64, + #[case] is_update_accepted: bool, ) { let peer_config = TestPeerConfig::new(function_name!(), 0, 0); let mut peer = TestPeer::new(peer_config); - if let Some(old_height) = old_height { - peer.network.highest_stacks_neighbor = Some(( - SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080), - old_height, - )); - } else { - peer.network.highest_stacks_neighbor = None; - } + + let prev_highest_neighbor = + old_height.map(|h| (SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 8080), h)); + peer.network.highest_stacks_neighbor = prev_highest_neighbor.clone(); + let peer_sortdb = peer.sortdb.take().unwrap(); let mut peer_stacks_node = peer.stacks_node.take().unwrap(); let mut peer_mempool = peer.mempool.take().unwrap(); @@ -1843,14 +1840,14 @@ fn test_update_highest_stacks_height_of_neighbors( false, ); - let new_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8081); - node_state.update_highest_stacks_neighbor(&new_addr, new_height); - if let Some(new_height) = new_height { - assert_eq!( - peer.network.highest_stacks_neighbor, - Some((new_addr, expected_height)) - ); + let new_peer_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 8081); + node_state.update_highest_stacks_neighbor(&new_peer_addr, new_height); + + let expected_highest_peer = if is_update_accepted { + Some((new_peer_addr, new_height.unwrap())) } else { - assert_eq!(peer.network.highest_stacks_neighbor, None); - } + prev_highest_neighbor + }; + + assert_eq!(peer.network.highest_stacks_neighbor, expected_highest_peer); }