Merge bitcoin/bitcoin#22147: p2p: Protect last outbound HB compact block peer

30aee2dfe671b347438c1c327c6f79edfacff1ce tests: Add test for compact block HB selection (Pieter Wuille)
6efbcec4ded6116a42d2783c96c60ef0f255a1b2 Protect last outbound HB compact block peer (Suhas Daftuar)

Pull request description:

  If all our high-bandwidth compact block serving peers (BIP 152) stall block
  download, then we can be denied a block for (potentially) a long time. As
  inbound connections are much more likely to be adversarial than outbound
  connections, mitigate this risk by never removing our last outbound HB peer if
  it would be replaced by an inbound.

ACKs for top commit:
  achow101:
    ACK 30aee2dfe671b347438c1c327c6f79edfacff1ce
  ariard:
    Code ACK 30aee2dfe
  jonatack:
    ACK 30aee2dfe671b347438c1c327c6f79edfacff1ce

Tree-SHA512: 5c6c9326e3667b97e0864c371ae2174d2be9054dad479f4366127b9cd3ac60ffa01ec9707b16ef29cac122db6916cf56fd9985733390017134ace483278921d5

1 files changed, 15 insertions, 0 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index b2112cfd2e..9bb3dfdcf3 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -836,12 +836,27 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid)
         return;
     }
     if (nodestate->fProvidesHeaderAndIDs) {
+        int num_outbound_hb_peers = 0;
         for (std::list<NodeId>::iterator it = lNodesAnnouncingHeaderAndIDs.begin(); it != lNodesAnnouncingHeaderAndIDs.end(); it++) {
             if (*it == nodeid) {
                 lNodesAnnouncingHeaderAndIDs.erase(it);
                 lNodesAnnouncingHeaderAndIDs.push_back(nodeid);
                 return;
             }
+            CNodeState *state = State(*it);
+            if (state != nullptr && !state->m_is_inbound) ++num_outbound_hb_peers;
+        }
+        if (nodestate->m_is_inbound) {
+            // If we're adding an inbound HB peer, make sure we're not removing
+            // our last outbound HB peer in the process.
+            if (lNodesAnnouncingHeaderAndIDs.size() >= 3 && num_outbound_hb_peers == 1) {
+                CNodeState *remove_node = State(lNodesAnnouncingHeaderAndIDs.front());
+                if (remove_node != nullptr && !remove_node->m_is_inbound) {
+                    // Put the HB outbound peer in the second slot, so that it
+                    // doesn't get removed.
+                    std::swap(lNodesAnnouncingHeaderAndIDs.front(), *std::next(lNodesAnnouncingHeaderAndIDs.begin()));
+                }
+            }
         }
         m_connman.ForNode(nodeid, [this](CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
             AssertLockHeld(::cs_main);