Merge bitcoin/bitcoin#25454: p2p: Avoid multiple getheaders messages in flight to the same peer

99f4785cad Replace GetTime() with NodeClock in MaybeSendGetHeaders() (Suhas Daftuar)
abf5d16c24 Don't send getheaders message when another request is outstanding (Suhas Daftuar)
ffe87db247 Cleanup received_new_header calculation to use WITH_LOCK (Suhas Daftuar)
6d95cd3e74 Move peer state updates from headers message into separate function (Suhas Daftuar)
2b341db731 Move headers direct fetch to end of ProcessHeadersMessage (Suhas Daftuar)
29c4518522 Move headers-direct-fetch logic into own function (Suhas Daftuar)
bf8ea6df75 Move additional headers fetching to own function (Suhas Daftuar)
9492e93bf9 Add helper function for checking header continuity (Suhas Daftuar)
7f2450871b Move handling of unconnecting headers into own function (Suhas Daftuar)

Pull request description:

  Change `getheaders` messages so that we wait up to 2 minutes for a response to a prior `getheaders` message before issuing a new one.

  Also change the handling of the `getheaders` message sent in response to a block INV, so that we no longer use the hashstop variable (including the hash stop will just mean that if our peer's headers chain is longer, then we won't learn
  it, so there's no benefit to using hashstop).

  Also, now respond to a `getheaders` during IBD with an empty headers message (rather than nothing) -- this better conforms to the intent of the new logic that it's better to not ignore a peer's `getheaders` message, even if you have nothing to give. This also avoids a lot of functional tests breaking.

  This PR also reworks the headers processing logic to make it more readable.

ACKs for top commit:
  ajtowns:
    ACK 99f4785cad ; code review, check over new logic of when to send getheaders messages
  dergoegge:
    Code review ACK  99f4785cad
  mzumsande:
    Code Review ACK 99f4785cad
  sipa:
    utACK 99f4785cad
  w0xlt:
    tACK 99f4785cad Good improvement in the code.

Tree-SHA512: b8a63f6f71ac83e292edc0200def7835ad8b06b2955dd34e3ea6fac85980fa6962efd31d689ef5ea121ff5477ec14aafa4bbe2d0db134c05f4a31a57a8ced365

test/functional/feature_minchainwork.py

@@ -82,7 +82,7 @@ class MinimumChainWorkTest(BitcoinTestFramework):
         msg.hashstop = 0
         peer.send_and_ping(msg)
         time.sleep(5)
-        assert "headers" not in peer.last_message
+        assert ("headers" not in peer.last_message or len(peer.last_message["headers"].headers) == 0)
 
         self.log.info("Generating one more block")
         self.generate(self.nodes[0], 1)

test/functional/p2p_segwit.py

@@ -371,6 +371,10 @@ class SegWitTest(BitcoinTestFramework):
         block1 = self.build_next_block()
         block1.solve()
 
+        # Send an empty headers message, to clear out any prior getheaders
+        # messages that our peer may be waiting for us on.
+        self.test_node.send_message(msg_headers())
+
         self.test_node.announce_block_and_wait_for_getdata(block1, use_header=False)
         assert self.test_node.last_message["getdata"].inv[0].type == blocktype
         test_witness_block(self.nodes[0], self.test_node, block1, True)