Merge #17785: p2p: Unify Send and Receive protocol versions

ddefb5c0b759950942ac03f28c43b548af7b4033 p2p: Use the greatest common version in peer logic (Hennadii Stepanov)
e084d45562b94827b3a7873895882fcaae9f4d48 p2p: Remove SetCommonVersion() from VERACK handler (Hennadii Stepanov)
8d2026796a6f7add0c2cda9806e759817d1eae6f refactor: Rename local variable nSendVersion (Hennadii Stepanov)
e9a6d8b13b0558b17cdafbd32fd2663b4138ff11 p2p: Unify Send and Receive protocol versions (Hennadii Stepanov)

Pull request description:

  On master (6fef85bfa3cd7f76e83b8b57f9e4acd63eb664ec) `CNode` has two members to keep protocol version:
  - `nRecvVersion` for received messages
  - `nSendVersion` for messages to send

  After exchanging with `VERSION` and `VERACK` messages via protocol version `INIT_PROTO_VERSION`, both nodes set `nRecvVersion` _and_ `nSendVersion` to _the same_ value which is the greatest common protocol version.

  This PR:
  - replaces two `CNode` members, `nRecvVersion` `nSendVersion`, with `m_greatest_common_version`
  - removes duplicated getter and setter

  There is no change in behavior on the P2P network.

ACKs for top commit:
  jnewbery:
    ACK ddefb5c0b759950942ac03f28c43b548af7b4033
  naumenkogs:
    ACK ddefb5c0b759950942ac03f28c43b548af7b4033
  fjahr:
    Code review ACK ddefb5c0b759950942ac03f28c43b548af7b4033
  amitiuttarwar:
    code review but untested ACK ddefb5c0b7
  benthecarman:
    utACK `ddefb5c`

Tree-SHA512: 5305538dbaa5426b923b0afd20bdef4f248d310855d1d78427210c00716c67b7cb691515c421716b6157913e453076e293b10ff5fd2cd26a8e5375d42da7809d

1 files changed, 1 insertions, 27 deletions

diff --git a/src/net.cpp b/src/net.cpp
index 0fd95dfe3f..e7d3a146ff 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -622,32 +622,6 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete
     return true;
 }
 
-void CNode::SetSendVersion(int nVersionIn)
-{
-    // Send version may only be changed in the version message, and
-    // only one version message is allowed per session. We can therefore
-    // treat this value as const and even atomic as long as it's only used
-    // once a version message has been successfully processed. Any attempt to
-    // set this twice is an error.
-    if (nSendVersion != 0) {
-        error("Send version already set for node: %i. Refusing to change from %i to %i", id, nSendVersion, nVersionIn);
-    } else {
-        nSendVersion = nVersionIn;
-    }
-}
-
-int CNode::GetSendVersion() const
-{
-    // The send version should always be explicitly set to
-    // INIT_PROTO_VERSION rather than using this value until SetSendVersion
-    // has been called.
-    if (nSendVersion == 0) {
-        error("Requesting unset send version for node: %i. Using %i", id, INIT_PROTO_VERSION);
-        return INIT_PROTO_VERSION;
-    }
-    return nSendVersion;
-}
-
 int V1TransportDeserializer::readHeader(const char *pch, unsigned int nBytes)
 {
     // copy data to temporary parsing buffer
@@ -1194,7 +1168,7 @@ void CConnman::InactivityCheck(CNode *pnode)
             LogPrintf("socket sending timeout: %is\n", nTime - pnode->nLastSend);
             pnode->fDisconnect = true;
         }
-        else if (nTime - pnode->nLastRecv > (pnode->nVersion > BIP0031_VERSION ? TIMEOUT_INTERVAL : 90*60))
+        else if (nTime - pnode->nLastRecv > (pnode->GetCommonVersion() > BIP0031_VERSION ? TIMEOUT_INTERVAL : 90*60))
         {
             LogPrintf("socket receive timeout: %is\n", nTime - pnode->nLastRecv);
             pnode->fDisconnect = true;