test: p2p: check that peer’s announced starting height is remembered #33990

1. 
   theStack commented at 0:25 am on December 2, 2025: contributor

   Note that the announced starting height of a peer is neither verified nor used in any other logic, so reporting a bogus value [1] as done in the test doesn’t have any consequences on the connection – it’s only used for inspection via the getpeerinfo RPC and in some debug messages (see also e.g. https://github.com/bitcoin-dot-org/Bitcoin.org/issues/1387#issuecomment-252934859). Admittedly this is not terribly important, but I’d still think having test coverage for this simple reporting functionality is better than not having it.

   [1] any signed 32-bit integer is fine, i.e. in the range [-2^31,2^31-1]

2. test: check that peer's announced starting height is remembered

   Note that the announced starting height is neither verified nor
   used in any other logic, so reporting a bogus value doesn't
   have any consequences -- it's only used for inspection via the
   `getpeerinfo` RPC and in some debug messages.

   52f96cc235
3. DrahtBot added the label Tests on Dec 2, 2025
4. 
   DrahtBot commented at 0:25 am on December 2, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33990.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

5. theStack renamed this:
   test: check that peer's announced starting height is remembered
   test: p2p: check that peer's announced starting height is remembered
   on Dec 2, 2025
6. 
   rkrux commented at 11:17 am on December 2, 2025: contributor

   Concept ~0 52f96cc235d309d9156eb742036c859984b9a467

   I’m on the fence regarding whether this case testing just one field is warranted. I don’t see much coverage gained in the corecheck. Though there is a similar test_desirable_service_flags case as well but in it the connection is disconnected conditionally, so one can argue that there is more to test over there.

7. 
   brunoerg commented at 11:20 am on December 2, 2025: contributor

   Can be tested with the following diff:

    0diff --git a/src/net_processing.cpp b/src/net_processing.cpp
    1index 5dac413319..ec417b97ac 100644
    2--- a/src/net_processing.cpp
    3+++ b/src/net_processing.cpp
    4@@ -3535,7 +3535,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
    5             LOCK(pfrom.m_subver_mutex);
    6             pfrom.cleanSubVer = cleanSubVer;
    7         }
    8-        peer->m_starting_height = starting_height;
    9+        peer->m_starting_height = starting_height + 1;
   10
   11         // Only initialize the Peer::TxRelay m_relay_txs data structure if:
   12         // - this isn't an outbound block-relay-only connection, and
   

Contributors
theStack DrahtBot rkrux brunoerg

Labels
Tests