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 63f3e8fbbf..0fbd7a0042 100644 --- a/docs/rpc/openapi.yaml +++ b/docs/rpc/openapi.yaml @@ -1295,24 +1295,11 @@ 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: [] 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 +1310,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}: diff --git a/stacks-node/src/tests/nakamoto_integrations.rs b/stacks-node/src/tests/nakamoto_integrations.rs index f7e080e730..83c721833f 100644 --- a/stacks-node/src/tests/nakamoto_integrations.rs +++ b/stacks-node/src/tests/nakamoto_integrations.rs @@ -126,7 +126,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; @@ -2426,10 +2426,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; @@ -2657,6 +2658,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 6adbeea23a..1b5feab35b 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..55760e410c 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 @@ -42,57 +35,19 @@ 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, } -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 +80,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 +95,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 +104,30 @@ 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_neighbor_address, max_stacks_height_of_neighbors), node_stacks_tip_height) = + node.with_node_state(|network, _sortdb, _chainstate, _mempool, _rpc_args| { + ( + network + .highest_stacks_neighbor + .map(|(addr, height)| (Some(addr.to_string()), height)) + .unwrap_or((None, 0)), + 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, - ); - - // 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.", - ), - } - }) + }); + + // 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); + + let preamble = HttpResponsePreamble::ok_json(&preamble); + 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)?; + Ok((preamble, body)) } } @@ -263,15 +144,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..540ded6a60 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 addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 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,18 @@ 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 +70,69 @@ 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 + let peer_1_addr = SocketAddr::new( + IpAddr::V4(Ipv4Addr::LOCALHOST), + rpc_test.peer_1.config.http_port, ); - 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_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 request = StacksHttpRequest::new_gethealth(addr.into(), NeighborsScope::Initial); + 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); // --- 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" + "Mismatch in difference_from_max_peer" ); - 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." + health_response.max_stacks_neighbor_address, + Some(peer_1_addr.to_string()), + "Mismatch in max_stacks_neighbor_address" ); } #[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 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); // --- 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/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 c654e660f1..3653e02408 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 7315cc8758..315ae9de18 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; @@ -806,6 +806,26 @@ impl<'a> StacksNodeState<'a> { } }) } + + pub fn update_highest_stacks_neighbor( + &mut self, + new_address: &SocketAddr, + new_height: Option, + ) { + self.with_node_state(|network, _, _, _, _| { + 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)); + } + } + }); + } } pub const STACKS_PUBLIC_KEY_ENCODED_SIZE: u32 = 33; @@ -2207,27 +2227,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; @@ -3318,6 +3317,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(); @@ -3331,7 +3352,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, @@ -3462,7 +3483,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/p2p.rs b/stackslib/src/net/p2p.rs index 3a6e5501ce..ad9b64aa02 100644 --- a/stackslib/src/net/p2p.rs +++ b/stackslib/src/net/p2p.rs @@ -638,6 +638,10 @@ pub struct PeerNetwork { /// Thread handle for the async block proposal endpoint. block_proposal_thread: Option>, + + /// 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 { @@ -799,6 +803,8 @@ impl PeerNetwork { nakamoto_inv_generator: InvGenerator::new(), block_proposal_thread: None, + + highest_stacks_neighbor: None, }; network.init_block_downloader(); diff --git a/stackslib/src/net/rpc.rs b/stackslib/src/net/rpc.rs index 641a247c25..a02ffb98ea 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, @@ -535,6 +532,10 @@ 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_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. // (this _should_ be our pending_request handle) match self 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); -} diff --git a/stackslib/src/net/tests/mod.rs b/stackslib/src/net/tests/mod.rs index f69a415ea3..20437817f2 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; @@ -63,10 +64,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 +1807,47 @@ 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(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] is_update_accepted: bool, +) { + let peer_config = TestPeerConfig::new(function_name!(), 0, 0); + let mut peer = TestPeer::new(peer_config); + + 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(); + 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, + ); + + 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 { + prev_highest_neighbor + }; + + assert_eq!(peer.network.highest_stacks_neighbor, expected_highest_peer); +}