Merge #20291: [net] Consolidate logic around calling CAddrMan::Connected()

0bfce9dc46234b196a8b3679c21d6f8455962495 [addrman] Fix Connected() comment (John Newbery)
eefe19471868ef0cdc9d32473d0b57015e7647ee [net] Consolidate logic around calling CAddrMan::Connected() (John Newbery)

Pull request description:

  Currently, the logic around whether we called CAddrMan::Connected() for
  a peer is spread between verack processing (where we discard inbound
  peers) and FinalizeNode (where we discard misbehaving and
  block-relay-only peers). Consolidate that logic to a single place.

  Also remove the CNode.fCurrentlyConnected bool, which is now
  redundant. We can rely on CNode.fSuccessfullyConnected, since the two
  bools were only ever flipped to true in the same place.

ACKs for top commit:
  mzumsande:
    Code review ACK 0bfce9dc46234b196a8b3679c21d6f8455962495
  amitiuttarwar:
    code review ACK 0bfce9dc46. nice tidy, and bonus that we get to remove an unnecessary call to `cs_main`

Tree-SHA512: 1ab74dae3bc12a6846da57c853033e546bb4f91caa39f4c50bf0cf7eca59cb917bdb2ef795da55363e7e9f70949cb28bb3be004cb3afa4389f970d2fe097d932

1 files changed, 3 insertions, 8 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index c649cf7757..e9915a3091 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -290,8 +290,6 @@ namespace {
 struct CNodeState {
     //! The peer's address
     const CService address;
-    //! Whether we have a fully established connection.
-    bool fCurrentlyConnected;
     //! The best known block we know this peer has announced.
     const CBlockIndex *pindexBestKnownBlock;
     //! The hash of the last unknown block this peer has announced.
@@ -390,7 +388,6 @@ struct CNodeState {
     CNodeState(CAddress addrIn, bool is_inbound, bool is_manual)
         : address(addrIn), m_is_inbound(is_inbound), m_is_manual_connection(is_manual)
     {
-        fCurrentlyConnected = false;
         pindexBestKnownBlock = nullptr;
         hashLastUnknownBlock.SetNull();
         pindexLastCommonBlock = nullptr;
@@ -857,8 +854,9 @@ void PeerManager::FinalizeNode(const CNode& node, bool& fUpdateConnectionTime) {
     if (state->fSyncStarted)
         nSyncStarted--;
 
-    if (misbehavior == 0 && state->fCurrentlyConnected && !node.IsBlockOnlyConn()) {
-        // Note: we avoid changing visible addrman state for block-relay-only peers
+    if (node.fSuccessfullyConnected && misbehavior == 0 &&
+        !node.IsBlockOnlyConn() && !node.IsInboundConn()) {
+        // Only change visible addrman state for outbound, full-relay peers
         fUpdateConnectionTime = true;
     }
 
@@ -2488,9 +2486,6 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
         if (pfrom.fSuccessfullyConnected) return;
 
         if (!pfrom.IsInboundConn()) {
-            // Mark this node as currently connected, so we update its timestamp later.
-            LOCK(cs_main);
-            State(pfrom.GetId())->fCurrentlyConnected = true;
             LogPrintf("New outbound peer connected: version: %d, blocks=%d, peer=%d%s (%s)\n",
                       pfrom.nVersion.load(), pfrom.nStartingHeight,
                       pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToString()) : ""),