diff --git a/src/avalanche/processor.cpp b/src/avalanche/processor.cpp --- a/src/avalanche/processor.cpp +++ b/src/avalanche/processor.cpp @@ -275,7 +275,7 @@ void Processor::sendResponse(CNode *pfrom, Response response) const { connman->PushMessage( - pfrom, CNetMsgMaker(pfrom->GetSendVersion()) + pfrom, CNetMsgMaker(pfrom->GetCommonVersion()) .Make(NetMsgType::AVARESPONSE, TCPResponse(std::move(response), sessionKey))); } @@ -424,7 +424,7 @@ } } - connman->PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()) + connman->PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()) .Make(NetMsgType::AVAHELLO, Hello(peerData->delegation, sig))); @@ -569,7 +569,7 @@ // Send the query to the node. connman->PushMessage( - pnode, CNetMsgMaker(pnode->GetSendVersion()) + pnode, CNetMsgMaker(pnode->GetCommonVersion()) .Make(NetMsgType::AVAPOLL, Poll(current_round, std::move(invs)))); return true; diff --git a/src/avalanche/test/processor_tests.cpp b/src/avalanche/test/processor_tests.cpp --- a/src/avalanche/test/processor_tests.cpp +++ b/src/avalanche/test/processor_tests.cpp @@ -102,7 +102,7 @@ auto node = new CNode(id++, ServiceFlags(NODE_NETWORK), 0, INVALID_SOCKET, addr, 0, 0, 0, CAddress(), "", ConnectionType::OUTBOUND); - node->SetSendVersion(PROTOCOL_VERSION); + node->SetCommonVersion(PROTOCOL_VERSION); node->nServices = nServices; m_node.peerman->InitializeNode(config, node); node->nVersion = 1; diff --git a/src/net.h b/src/net.h --- a/src/net.h +++ b/src/net.h @@ -836,7 +836,6 @@ std::deque vRecvGetData; uint64_t nRecvBytes GUARDED_BY(cs_vRecv){0}; - std::atomic nRecvVersion{INIT_PROTO_VERSION}; std::atomic nLastSend{0}; std::atomic nLastRecv{0}; @@ -1035,6 +1034,7 @@ const uint64_t nLocalHostNonce; const uint64_t nLocalExtraEntropy; const ConnectionType m_conn_type; + std::atomic m_greatest_common_version{INIT_PROTO_VERSION}; //! Services offered to this peer. //! @@ -1054,7 +1054,6 @@ const ServiceFlags nLocalServices; const int nMyStartingHeight; - int nSendVersion{0}; NetPermissionFlags m_permissionFlags{PF_NONE}; // Used only by SocketHandler thread std::list vRecvMsg; @@ -1082,10 +1081,10 @@ bool ReceiveMsgBytes(const Config &config, const char *pch, uint32_t nBytes, bool &complete); - void SetRecvVersion(int nVersionIn) { nRecvVersion = nVersionIn; } - int GetRecvVersion() const { return nRecvVersion; } - void SetSendVersion(int nVersionIn); - int GetSendVersion() const; + void SetCommonVersion(int greatest_common_version) { + m_greatest_common_version = greatest_common_version; + } + int GetCommonVersion() const { return m_greatest_common_version; } CService GetAddrLocal() const; //! May not be called more than once diff --git a/src/net.cpp b/src/net.cpp --- a/src/net.cpp +++ b/src/net.cpp @@ -647,32 +647,6 @@ 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 Config &config, const char *pch, uint32_t nBytes) { // copy data to temporary parsing buffer @@ -1280,9 +1254,10 @@ 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; diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -810,7 +810,7 @@ [&connman, nCMPCTBLOCKVersion](CNode *pnodeStop) { AssertLockHeld(cs_main); connman.PushMessage( - pnodeStop, CNetMsgMaker(pnodeStop->GetSendVersion()) + pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()) .Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); @@ -818,7 +818,7 @@ }); lNodesAnnouncingHeaderAndIDs.pop_front(); } - connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()) + connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()) .Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/true, nCMPCTBLOCKVersion)); @@ -1605,7 +1605,8 @@ AssertLockHeld(cs_main); // TODO: Avoid the repeated-serialization here - if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) { + if (pnode->GetCommonVersion() < INVALID_CB_NO_BAN_VERSION || + pnode->fDisconnect) { return; } ProcessBlockAvailability(pnode->GetId()); @@ -1848,7 +1849,7 @@ __func__, pfrom.GetId()); } } - const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); + const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); // Disconnect node in case we have reached the outbound limit for serving // historical blocks. // Never disconnect whitelisted nodes. @@ -2017,7 +2018,7 @@ std::deque::iterator it = pfrom.vRecvGetData.begin(); std::vector vNotFound; - const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); + const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); // mempool entries added before this time have likely expired from mapRelay const std::chrono::seconds longlived_mempool_time = @@ -2105,7 +2106,7 @@ resp.txn[i] = block.vtx[req.indices[i]]; } LOCK(cs_main); - const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); + const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); int nSendFlags = 0; m_connman.PushMessage( &pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCKTXN, resp)); @@ -2114,7 +2115,7 @@ void PeerManager::ProcessHeadersMessage( const Config &config, CNode &pfrom, const std::vector &headers, bool via_compact_block) { - const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); + const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); size_t nCount = headers.size(); if (nCount == 0) { @@ -2531,7 +2532,7 @@ } for (const auto &filter : filters) { - CSerializedNetMsg msg = CNetMsgMaker(peer.GetSendVersion()) + CSerializedNetMsg msg = CNetMsgMaker(peer.GetCommonVersion()) .Make(NetMsgType::CFILTER, filter); connman.PushMessage(&peer, std::move(msg)); } @@ -2593,7 +2594,7 @@ } CSerializedNetMsg msg = - CNetMsgMaker(peer.GetSendVersion()) + CNetMsgMaker(peer.GetCommonVersion()) .Make(NetMsgType::CFHEADERS, filter_type_ser, stop_index->GetBlockHash(), prev_header, filter_hashes); connman.PushMessage(&peer, std::move(msg)); @@ -2647,7 +2648,7 @@ } } - CSerializedNetMsg msg = CNetMsgMaker(peer.GetSendVersion()) + CSerializedNetMsg msg = CNetMsgMaker(peer.GetCommonVersion()) .Make(NetMsgType::CFCHECKPT, filter_type_ser, stop_index->GetBlockHash(), headers); connman.PushMessage(&peer, std::move(msg)); @@ -2691,14 +2692,12 @@ uint64_t nServiceInt; ServiceFlags nServices; int nVersion; - int nSendVersion; std::string cleanSubVer; int nStartingHeight = -1; bool fRelay = true; uint64_t nExtraEntropy = 1; vRecv >> nVersion >> nServiceInt >> nTime >> addrMe; - nSendVersion = std::min(nVersion, PROTOCOL_VERSION); nServices = ServiceFlags(nServiceInt); if (!pfrom.IsInboundConn()) { m_connman.SetServices(pfrom.addr, nServices); @@ -2757,8 +2756,15 @@ PushNodeVersion(config, pfrom, m_connman, GetAdjustedTime()); } + // Change version + const int greatest_common_version = + std::min(nVersion, PROTOCOL_VERSION); + pfrom.SetCommonVersion(greatest_common_version); + pfrom.nVersion = nVersion; + m_connman.PushMessage( - &pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); + &pfrom, + CNetMsgMaker(greatest_common_version).Make(NetMsgType::VERACK)); pfrom.nServices = nServices; pfrom.SetAddrLocal(addrMe); @@ -2784,9 +2790,6 @@ pfrom.m_tx_relay->fRelayTxes = fRelay; } - // Change version - pfrom.SetSendVersion(nSendVersion); - pfrom.nVersion = nVersion; pfrom.nRemoteHostNonce = nNonce; pfrom.nRemoteExtraEntropy = nExtraEntropy; @@ -2817,8 +2820,8 @@ } // Get recent addresses - m_connman.PushMessage( - &pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); + m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version) + .Make(NetMsgType::GETADDR)); pfrom.fGetAddr = true; m_connman.MarkAddressGood(pfrom.addr); } @@ -2861,11 +2864,9 @@ } // At this point, the outgoing message serialization version can't change. - const CNetMsgMaker msgMaker(pfrom.GetSendVersion()); + const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); if (msg_type == NetMsgType::VERACK) { - pfrom.SetRecvVersion(std::min(pfrom.nVersion.load(), PROTOCOL_VERSION)); - if (!pfrom.IsInboundConn()) { // Mark this node as currently connected, so we update its timestamp // later. @@ -2880,7 +2881,7 @@ pfrom.m_tx_relay == nullptr ? "block-relay" : "full-relay"); } - if (pfrom.nVersion >= SENDHEADERS_VERSION) { + if (pfrom.GetCommonVersion() >= SENDHEADERS_VERSION) { // Tell our peer we prefer to receive headers rather than inv's // We send this to non-NODE NETWORK peers as well, because even // non-NODE NETWORK peers can announce blocks (such as pruning @@ -2889,7 +2890,7 @@ msgMaker.Make(NetMsgType::SENDHEADERS)); } - if (pfrom.nVersion >= SHORT_IDS_BLOCKS_VERSION) { + if (pfrom.GetCommonVersion() >= SHORT_IDS_BLOCKS_VERSION) { // Tell our peer we are willing to provide version 1 or 2 // cmpctblocks. However, we do not request new block announcements // using cmpctblock messages. We send this to non-NODE NETWORK peers @@ -4196,7 +4197,7 @@ } if (msg_type == NetMsgType::PING) { - if (pfrom.nVersion > BIP0031_VERSION) { + if (pfrom.GetCommonVersion() > BIP0031_VERSION) { uint64_t nonce = 0; vRecv >> nonce; // Echo the message back with the nonce. This allows for two useful @@ -4502,7 +4503,7 @@ } CNetMessage &msg(msgs.front()); - msg.SetVersion(pfrom->GetRecvVersion()); + msg.SetVersion(pfrom->GetCommonVersion()); // Check network magic if (!msg.m_valid_netmagic) { @@ -4570,7 +4571,7 @@ AssertLockHeld(cs_main); CNodeState &state = *State(pto.GetId()); - const CNetMsgMaker msgMaker(pto.GetSendVersion()); + const CNetMsgMaker msgMaker(pto.GetCommonVersion()); if (!state.m_chain_sync.m_protect && pto.IsOutboundOrBlockRelayConn() && state.fSyncStarted) { @@ -4795,7 +4796,7 @@ // If we get here, the outgoing message serialization version is set and // can't change. - const CNetMsgMaker msgMaker(pto->GetSendVersion()); + const CNetMsgMaker msgMaker(pto->GetCommonVersion()); // // Message: ping @@ -4817,7 +4818,7 @@ } pto->fPingQueued = false; pto->m_ping_start = GetTime(); - if (pto->nVersion > BIP0031_VERSION) { + if (pto->GetCommonVersion() > BIP0031_VERSION) { pto->nPingNonceSent = nonce; m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::PING, nonce)); } else { @@ -5468,7 +5469,8 @@ // // We don't want white listed peers to filter txs to us if we have // -whitelistforcerelay - if (pto->m_tx_relay != nullptr && pto->nVersion >= FEEFILTER_VERSION && + if (pto->m_tx_relay != nullptr && + pto->GetCommonVersion() >= FEEFILTER_VERSION && gArgs.GetBoolArg("-feefilter", DEFAULT_FEEFILTER) && !pto->HasPermission(PF_FORCERELAY)) { Amount currentFilter = diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -86,7 +86,7 @@ CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK), 0, INVALID_SOCKET, addr1, 0, 0, 0, CAddress(), "", ConnectionType::OUTBOUND); - dummyNode1.SetSendVersion(PROTOCOL_VERSION); + dummyNode1.SetCommonVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(config, &dummyNode1); dummyNode1.nVersion = 1; @@ -149,7 +149,7 @@ INVALID_SOCKET, addr, 0, 0, 0, CAddress(), "", ConnectionType::OUTBOUND)); CNode &node = *vNodes.back(); - node.SetSendVersion(PROTOCOL_VERSION); + node.SetCommonVersion(PROTOCOL_VERSION); peerLogic.InitializeNode(config, &node); node.nVersion = 1; @@ -250,7 +250,7 @@ CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 0, 0, 0, CAddress(), "", ConnectionType::INBOUND); - dummyNode1.SetSendVersion(PROTOCOL_VERSION); + dummyNode1.SetCommonVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(config, &dummyNode1); dummyNode1.nVersion = 1; dummyNode1.fSuccessfullyConnected = true; @@ -269,7 +269,7 @@ CAddress addr2(ip(0xa0b0c002), NODE_NONE); CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, 1, CAddress(), "", ConnectionType::INBOUND); - dummyNode2.SetSendVersion(PROTOCOL_VERSION); + dummyNode2.SetCommonVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(config, &dummyNode2); dummyNode2.nVersion = 1; dummyNode2.fSuccessfullyConnected = true; @@ -319,7 +319,7 @@ CAddress addr(ip(0xa0b0c001), NODE_NONE); CNode dummyNode(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr, 4, 4, 4, CAddress(), "", ConnectionType::INBOUND); - dummyNode.SetSendVersion(PROTOCOL_VERSION); + dummyNode.SetCommonVersion(PROTOCOL_VERSION); peerLogic->InitializeNode(config, &dummyNode); dummyNode.nVersion = 1; dummyNode.fSuccessfullyConnected = true; diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -56,7 +56,7 @@ ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH})}; while (fuzzed_data_provider.ConsumeBool()) { - switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 12)) { + switch (fuzzed_data_provider.ConsumeIntegralInRange(0, 11)) { case 0: { node.CloseSocketDisconnect(); break; @@ -67,7 +67,7 @@ break; } case 2: { - node.SetSendVersion( + node.SetCommonVersion( fuzzed_data_provider.ConsumeIntegral()); break; } @@ -83,22 +83,17 @@ break; } case 4: { - node.SetRecvVersion( - fuzzed_data_provider.ConsumeIntegral()); - break; - } - case 5: { const CNode *add_ref_node = node.AddRef(); assert(add_ref_node == &node); break; } - case 6: { + case 5: { if (node.GetRefCount() > 0) { node.Release(); } break; } - case 7: { + case 6: { if (node.m_addr_known == nullptr) { break; } @@ -110,7 +105,7 @@ node.AddAddressKnown(*addr_opt); break; } - case 8: { + case 7: { if (node.m_addr_known == nullptr) { break; } @@ -124,7 +119,7 @@ node.PushAddress(*addr_opt, fast_random_context); break; } - case 9: { + case 8: { const std::optional inv_opt = ConsumeDeserializable(fuzzed_data_provider); if (!inv_opt) { @@ -134,12 +129,12 @@ node.AddKnownTx(txid); break; } - case 10: { + case 9: { const TxId &txid = TxId(ConsumeUInt256(fuzzed_data_provider)); node.PushTxInventory(txid); break; } - case 11: { + case 10: { const std::optional service_opt = ConsumeDeserializable(fuzzed_data_provider); if (!service_opt) { @@ -148,7 +143,7 @@ node.SetAddrLocal(*service_opt); break; } - case 12: { + case 11: { const std::vector b = ConsumeRandomLengthByteVector(fuzzed_data_provider); bool complete; @@ -165,10 +160,9 @@ (void)node.GetLocalNonce(); (void)node.GetLocalServices(); (void)node.GetMyStartingHeight(); - (void)node.GetRecvVersion(); const int ref_count = node.GetRefCount(); assert(ref_count >= 0); - (void)node.GetSendVersion(); + (void)node.GetCommonVersion(); (void)node.IsAddrRelayPeer(); const NetPermissionFlags net_permission_flags = diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -81,7 +81,7 @@ .release(); p2p_node.fSuccessfullyConnected = true; p2p_node.nVersion = PROTOCOL_VERSION; - p2p_node.SetSendVersion(PROTOCOL_VERSION); + p2p_node.SetCommonVersion(PROTOCOL_VERSION); connman.AddTestNode(p2p_node); g_setup->m_node.peerman->InitializeNode(config, &p2p_node); try { diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -61,7 +61,7 @@ p2p_node.fSuccessfullyConnected = true; p2p_node.fPauseSend = false; p2p_node.nVersion = PROTOCOL_VERSION; - p2p_node.SetSendVersion(PROTOCOL_VERSION); + p2p_node.SetCommonVersion(PROTOCOL_VERSION); g_setup->m_node.peerman->InitializeNode(config, &p2p_node); connman.AddTestNode(p2p_node);