Merge bitcoin/bitcoin#32073: net: Block v2->v1 transport downgrade if !fNetworkActive

6869fb417096b43ba7f74bf767ca3e41b9894899 net: Block v2->v1 transport downgrade if !CConnman::fNetworkActive (Hodlinator)

Pull request description:

  We might have just set `CNode::fDisconnect` in the first loop because of `!CConnman::fNetworkActive`.

  Attempting to reconnect using v1 transport just because `fNetworkActive` was set to `false` at the "right" stage in the v2 handshake does not make sense.

  Issue [discovered](https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1930908304) by davidgumberg.

ACKs for top commit:
  davidgumberg:
    Tested and Reviewed ACK 6869fb417096b43ba7f7
  mabu44:
    ACK 6869fb417096b43ba7f74bf767ca3e41b9894899
  stratospher:
    ACK 6869fb4. I've reviewed the code but don't have strong preference for this branch vs master since only functional change is just a single log not being printed in a low probability scenario (we happen to be attempting v2 connection when P2P network activity is being turned off).
  vasild:
    ACK 6869fb417096b43ba7f74bf767ca3e41b9894899

Tree-SHA512: 54f596e54c5a6546f2c3fec2609aa8d10dec3adcf1001ca16666d8b374b8d79d64397f46c90d9b3915b4e91a5041b6ced3044fd2a5b4fb4aa7282eb51f61296a
This commit is contained in:
Ryan Ofsky 2025-03-24 16:23:13 -04:00
commit a0d737cd7a
No known key found for this signature in database
GPG Key ID: 46800E30FC748A66

View File

@ -1912,7 +1912,8 @@ void CConnman::DisconnectNodes()
{
LOCK(m_nodes_mutex);
if (!fNetworkActive) {
const bool network_active{fNetworkActive};
if (!network_active) {
// Disconnect any connected nodes
for (CNode* pnode : m_nodes) {
if (!pnode->fDisconnect) {
@ -1934,7 +1935,7 @@ void CConnman::DisconnectNodes()
// Add to reconnection list if appropriate. We don't reconnect right here, because
// the creation of a connection is a blocking operation (up to several seconds),
// and we don't want to hold up the socket handler thread for that long.
if (pnode->m_transport->ShouldReconnectV1()) {
if (network_active && pnode->m_transport->ShouldReconnectV1()) {
reconnections_to_add.push_back({
.addr_connect = pnode->addr,
.grant = std::move(pnode->grantOutbound),