diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index dd3824fb1c5..404c9ecc4fd 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -81,7 +81,7 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements) } m_F = static_cast(m_N) * static_cast(m_params.m_M); - CVectorWriter stream(GCS_SER_VERSION, m_encoded, 0); + VectorWriter stream{m_encoded, 0}; WriteCompactSize(stream, m_N); @@ -89,7 +89,7 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements) return; } - BitStreamWriter bitwriter(stream); + BitStreamWriter bitwriter{stream}; uint64_t last_value = 0; for (uint64_t value : BuildHashedSet(elements)) { diff --git a/src/net.cpp b/src/net.cpp index 49b84597bc5..dc76fdfb445 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -683,8 +683,8 @@ bool CNode::ReceiveMsgBytes(Span msg_bytes, bool& complete) return true; } -V1Transport::V1Transport(const NodeId node_id, int nTypeIn, int nVersionIn) noexcept : - m_magic_bytes{Params().MessageStart()}, m_node_id(node_id), hdrbuf(nTypeIn, nVersionIn), vRecv(nTypeIn, nVersionIn) +V1Transport::V1Transport(const NodeId node_id) noexcept + : m_magic_bytes{Params().MessageStart()}, m_node_id{node_id} { LOCK(m_recv_mutex); Reset(); @@ -818,7 +818,7 @@ bool V1Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept // serialize header m_header_to_send.clear(); - CVectorWriter{INIT_PROTO_VERSION, m_header_to_send, 0, hdr}; + VectorWriter{m_header_to_send, 0, hdr}; // update state m_message_to_send = std::move(msg); @@ -968,12 +968,12 @@ void V2Transport::StartSendingHandshake() noexcept // We cannot wipe m_send_garbage as it will still be used as AAD later in the handshake. } -V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32, std::vector garbage) noexcept : - m_cipher{key, ent32}, m_initiating{initiating}, m_nodeid{nodeid}, - m_v1_fallback{nodeid, type_in, version_in}, m_recv_type{type_in}, m_recv_version{version_in}, - m_recv_state{initiating ? RecvState::KEY : RecvState::KEY_MAYBE_V1}, - m_send_garbage{std::move(garbage)}, - m_send_state{initiating ? SendState::AWAITING_KEY : SendState::MAYBE_V1} +V2Transport::V2Transport(NodeId nodeid, bool initiating, const CKey& key, Span ent32, std::vector garbage) noexcept + : m_cipher{key, ent32}, m_initiating{initiating}, m_nodeid{nodeid}, + m_v1_fallback{nodeid}, + m_recv_state{initiating ? RecvState::KEY : RecvState::KEY_MAYBE_V1}, + m_send_garbage{std::move(garbage)}, + m_send_state{initiating ? SendState::AWAITING_KEY : SendState::MAYBE_V1} { Assume(m_send_garbage.size() <= MAX_GARBAGE_LEN); // Start sending immediately if we're the initiator of the connection. @@ -983,9 +983,9 @@ V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int versio } } -V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept : - V2Transport{nodeid, initiating, type_in, version_in, GenerateRandomKey(), - MakeByteSpan(GetRandHash()), GenerateRandomGarbage()} { } +V2Transport::V2Transport(NodeId nodeid, bool initiating) noexcept + : V2Transport{nodeid, initiating, GenerateRandomKey(), + MakeByteSpan(GetRandHash()), GenerateRandomGarbage()} {} void V2Transport::SetReceiveState(RecvState recv_state) noexcept { @@ -1429,8 +1429,7 @@ CNetMessage V2Transport::GetReceivedMessage(std::chrono::microseconds time, bool Assume(m_recv_state == RecvState::APP_READY); Span contents{m_recv_decode_buffer}; auto msg_type = GetMessageType(contents); - CDataStream ret(m_recv_type, m_recv_version); - CNetMessage msg{std::move(ret)}; + CNetMessage msg{DataStream{}}; // Note that BIP324Cipher::EXPANSION also includes the length descriptor size. msg.m_raw_message_size = m_recv_decode_buffer.size() + BIP324Cipher::EXPANSION; if (msg_type) { @@ -3660,9 +3659,9 @@ ServiceFlags CConnman::GetLocalServices() const static std::unique_ptr MakeTransport(NodeId id, bool use_v2transport, bool inbound) noexcept { if (use_v2transport) { - return std::make_unique(id, /*initiating=*/!inbound, SER_NETWORK, INIT_PROTO_VERSION); + return std::make_unique(id, /*initiating=*/!inbound); } else { - return std::make_unique(id, SER_NETWORK, INIT_PROTO_VERSION); + return std::make_unique(id); } } diff --git a/src/net.h b/src/net.h index 36c6065b192..4347bf12ca1 100644 --- a/src/net.h +++ b/src/net.h @@ -232,15 +232,16 @@ public: * Ideally it should only contain receive time, payload, * type and size. */ -class CNetMessage { +class CNetMessage +{ public: - CDataStream m_recv; //!< received message data + DataStream m_recv; //!< received message data std::chrono::microseconds m_time{0}; //!< time of message receipt uint32_t m_message_size{0}; //!< size of the payload uint32_t m_raw_message_size{0}; //!< used wire size of the message (including header/checksum) std::string m_type; - CNetMessage(CDataStream&& recv_in) : m_recv(std::move(recv_in)) {} + explicit CNetMessage(DataStream&& recv_in) : m_recv(std::move(recv_in)) {} // Only one CNetMessage object will exist for the same message on either // the receive or processing queue. For performance reasons we therefore // delete the copy constructor and assignment operator to avoid the @@ -249,11 +250,6 @@ public: CNetMessage(const CNetMessage&) = delete; CNetMessage& operator=(CNetMessage&&) = default; CNetMessage& operator=(const CNetMessage&) = delete; - - void SetVersion(int nVersionIn) - { - m_recv.SetVersion(nVersionIn); - } }; /** The Transport converts one connection's sent messages to wire bytes, and received bytes back. */ @@ -379,9 +375,9 @@ private: mutable CHash256 hasher GUARDED_BY(m_recv_mutex); mutable uint256 data_hash GUARDED_BY(m_recv_mutex); bool in_data GUARDED_BY(m_recv_mutex); // parsing header (false) or data (true) - CDataStream hdrbuf GUARDED_BY(m_recv_mutex); // partially received header + DataStream hdrbuf GUARDED_BY(m_recv_mutex){}; // partially received header CMessageHeader hdr GUARDED_BY(m_recv_mutex); // complete header - CDataStream vRecv GUARDED_BY(m_recv_mutex); // received message data + DataStream vRecv GUARDED_BY(m_recv_mutex){}; // received message data unsigned int nHdrPos GUARDED_BY(m_recv_mutex); unsigned int nDataPos GUARDED_BY(m_recv_mutex); @@ -420,7 +416,7 @@ private: size_t m_bytes_sent GUARDED_BY(m_send_mutex) {0}; public: - V1Transport(const NodeId node_id, int nTypeIn, int nVersionIn) noexcept; + explicit V1Transport(const NodeId node_id) noexcept; bool ReceivedMessageComplete() const override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) { @@ -598,10 +594,6 @@ private: std::vector m_recv_aad GUARDED_BY(m_recv_mutex); /** Buffer to put decrypted contents in, for converting to CNetMessage. */ std::vector m_recv_decode_buffer GUARDED_BY(m_recv_mutex); - /** Deserialization type. */ - const int m_recv_type; - /** Deserialization version number. */ - const int m_recv_version; /** Current receiver state. */ RecvState m_recv_state GUARDED_BY(m_recv_mutex); @@ -647,13 +639,11 @@ public: * * @param[in] nodeid the node's NodeId (only for debug log output). * @param[in] initiating whether we are the initiator side. - * @param[in] type_in the serialization type of returned CNetMessages. - * @param[in] version_in the serialization version of returned CNetMessages. */ - V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept; + V2Transport(NodeId nodeid, bool initiating) noexcept; /** Construct a V2 transport with specified keys and garbage (test use only). */ - V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span ent32, std::vector garbage) noexcept; + V2Transport(NodeId nodeid, bool initiating, const CKey& key, Span ent32, std::vector garbage) noexcept; // Receive side functions. bool ReceivedMessageComplete() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1556dc7e675..533346ac811 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -51,6 +51,7 @@ #include #include #include +#include /** Headers download timeout. * Timeout = base + per_header * (expected number of headers) */ @@ -514,7 +515,7 @@ public: void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void SetBestHeight(int height) override { m_best_height = height; }; void UnitTestMisbehaving(NodeId peer_id, int howmuch) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), howmuch, ""); }; - void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, + void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override; @@ -669,6 +670,14 @@ private: void AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** Send a message to a peer */ + void PushMessage(CNode& node, CSerializedNetMsg&& msg) const { m_connman.PushMessage(&node, std::move(msg)); } + template + void MakeAndPushMessage(CNode& node, std::string msg_type, Args&&... args) const + { + m_connman.PushMessage(&node, NetMsg::Make(std::move(msg_type), std::forward(args)...)); + } + /** Send a version message to a peer */ void PushNodeVersion(CNode& pnode, const Peer& peer); @@ -1024,7 +1033,7 @@ private: * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFilters(CNode& node, Peer& peer, CDataStream& vRecv); + void ProcessGetCFilters(CNode& node, Peer& peer, DataStream& vRecv); /** * Handle a cfheaders request. @@ -1035,7 +1044,7 @@ private: * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFHeaders(CNode& node, Peer& peer, CDataStream& vRecv); + void ProcessGetCFHeaders(CNode& node, Peer& peer, DataStream& vRecv); /** * Handle a getcfcheckpt request. @@ -1046,7 +1055,7 @@ private: * @param[in] peer The peer that we received the request from * @param[in] vRecv The raw message received */ - void ProcessGetCFCheckPt(CNode& node, Peer& peer, CDataStream& vRecv); + void ProcessGetCFCheckPt(CNode& node, Peer& peer, DataStream& vRecv); /** Checks if address relay is permitted with peer. If needed, initializes * the m_addr_known bloom filter and sets m_addr_relay_enabled to true. @@ -1277,14 +1286,14 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) // As per BIP152, we only get 3 of our peers to announce // blocks using compact encodings. m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this](CNode* pnodeStop){ - m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); + MakeAndPushMessage(*pnodeStop, NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION); // save BIP152 bandwidth state: we select peer to be low-bandwidth pnodeStop->m_bip152_highbandwidth_to = false; return true; }); lNodesAnnouncingHeaderAndIDs.pop_front(); } - m_connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/true, /*version=*/CMPCTBLOCKS_VERSION)); + MakeAndPushMessage(*pfrom, NetMsgType::SENDCMPCT, /*high_bandwidth=*/true, /*version=*/CMPCTBLOCKS_VERSION); // save BIP152 bandwidth state: we select peer to be high-bandwidth pfrom->m_bip152_highbandwidth_to = true; lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId()); @@ -1487,10 +1496,10 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) uint64_t your_services{addr.nServices}; const bool tx_relay{!RejectIncomingTxs(pnode)}; - m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, my_services, nTime, + MakeAndPushMessage(pnode, NetMsgType::VERSION, PROTOCOL_VERSION, my_services, nTime, your_services, CNetAddr::V1(addr_you), // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime) my_services, CNetAddr::V1(CService{}), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime) - nonce, strSubVersion, nNodeStartingHeight, tx_relay)); + nonce, strSubVersion, nNodeStartingHeight, tx_relay); if (fLogIPs) { LogPrint(BCLog::NET, "send version message: version %d, blocks=%d, them=%s, txrelay=%d, peer=%d\n", PROTOCOL_VERSION, nNodeStartingHeight, addr_you.ToStringAddrPort(), tx_relay, nodeid); @@ -1861,8 +1870,7 @@ std::optional PeerManagerImpl::FetchBlock(NodeId peer_id, const CBl // Send block request message to the peer bool success = m_connman.ForNode(peer_id, [this, &invs](CNode* node) { - const CNetMsgMaker msgMaker(node->GetCommonVersion()); - this->m_connman.PushMessage(node, msgMaker.Make(NetMsgType::GETDATA, invs)); + this->MakeAndPushMessage(*node, NetMsgType::GETDATA, invs); return true; }); @@ -1985,7 +1993,6 @@ void PeerManagerImpl::BlockDisconnected(const std::shared_ptr &blo void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) { auto pcmpctblock = std::make_shared(*pblock); - const CNetMsgMaker msgMaker(PROTOCOL_VERSION); LOCK(cs_main); @@ -1997,7 +2004,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha uint256 hashBlock(pblock->GetHash()); const std::shared_future lazy_ser{ - std::async(std::launch::deferred, [&] { return msgMaker.Make(NetMsgType::CMPCTBLOCK, *pcmpctblock); })}; + std::async(std::launch::deferred, [&] { return NetMsg::Make(NetMsgType::CMPCTBLOCK, *pcmpctblock); })}; { auto most_recent_block_txs = std::make_unique>(); @@ -2028,7 +2035,7 @@ void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::sha hashBlock.ToString(), pnode->GetId()); const CSerializedNetMsg& ser_cmpctblock{lazy_ser.get()}; - m_connman.PushMessage(pnode, ser_cmpctblock.Copy()); + PushMessage(*pnode, ser_cmpctblock.Copy()); state.pindexBestHeaderSent = pindex; } }); @@ -2262,7 +2269,6 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& LogPrint(BCLog::NET, "%s: ignoring request from peer=%i for old block that isn't in the main chain\n", __func__, pfrom.GetId()); return; } - const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); // disconnect node in case we have reached the outbound limit for serving historical blocks if (m_connman.OutboundTargetReached(true) && (((m_chainman.m_best_header != nullptr) && (m_chainman.m_best_header->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && @@ -2296,7 +2302,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos())) { assert(!"cannot load block from disk"); } - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, Span{block_data})); + MakeAndPushMessage(pfrom, NetMsgType::BLOCK, Span{block_data}); // Don't set pblock as we've sent the block } else { // Send block from disk @@ -2308,9 +2314,9 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } if (pblock) { if (inv.IsMsgBlk()) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, TX_NO_WITNESS(*pblock))); + MakeAndPushMessage(pfrom, NetMsgType::BLOCK, TX_NO_WITNESS(*pblock)); } else if (inv.IsMsgWitnessBlk()) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, TX_WITH_WITNESS(*pblock))); + MakeAndPushMessage(pfrom, NetMsgType::BLOCK, TX_WITH_WITNESS(*pblock)); } else if (inv.IsMsgFilteredBlk()) { bool sendMerkleBlock = false; CMerkleBlock merkleBlock; @@ -2322,7 +2328,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } } if (sendMerkleBlock) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::MERKLEBLOCK, merkleBlock)); + MakeAndPushMessage(pfrom, NetMsgType::MERKLEBLOCK, merkleBlock); // CMerkleBlock just contains hashes, so also push any transactions in the block the client did not see // This avoids hurting performance by pointlessly requiring a round-trip // Note that there is currently no way for a node to request any single transactions we didn't send here - @@ -2331,7 +2337,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // however we MUST always provide at least what the remote peer needs typedef std::pair PairType; for (PairType& pair : merkleBlock.vMatchedTxn) - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::TX, TX_NO_WITNESS(*pblock->vtx[pair.first]))); + MakeAndPushMessage(pfrom, NetMsgType::TX, TX_NO_WITNESS(*pblock->vtx[pair.first])); } // else // no response @@ -2342,13 +2348,13 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // instead we respond with the full, non-compact block. if (CanDirectFetch() && pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_CMPCTBLOCK_DEPTH) { if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, *a_recent_compact_block)); + MakeAndPushMessage(pfrom, NetMsgType::CMPCTBLOCK, *a_recent_compact_block); } else { CBlockHeaderAndShortTxIDs cmpctblock{*pblock}; - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); + MakeAndPushMessage(pfrom, NetMsgType::CMPCTBLOCK, cmpctblock); } } else { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, TX_WITH_WITNESS(*pblock))); + MakeAndPushMessage(pfrom, NetMsgType::BLOCK, TX_WITH_WITNESS(*pblock)); } } } @@ -2362,7 +2368,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // wait for other stuff first. std::vector vInv; vInv.emplace_back(MSG_BLOCK, m_chainman.ActiveChain().Tip()->GetBlockHash()); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::INV, vInv)); + MakeAndPushMessage(pfrom, NetMsgType::INV, vInv); peer.m_continuation_block.SetNull(); } } @@ -2396,7 +2402,6 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic std::deque::iterator it = peer.m_getdata_requests.begin(); std::vector vNotFound; - const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); // Process as many TX items from the front of the getdata queue as // possible, since they're common and it's efficient to batch process @@ -2419,7 +2424,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic if (tx) { // WTX and WITNESS_TX imply we serialize with witness const auto maybe_with_witness = (inv.IsMsgTx() ? TX_NO_WITNESS : TX_WITH_WITNESS); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::TX, maybe_with_witness(*tx))); + MakeAndPushMessage(pfrom, NetMsgType::TX, maybe_with_witness(*tx)); m_mempool.RemoveUnbroadcastTx(tx->GetHash()); } else { vNotFound.push_back(inv); @@ -2454,7 +2459,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic // In normal operation, we often send NOTFOUND messages for parents of // transactions that we relay; if a peer is missing a parent, they may // assume we have them and request the parents from us. - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::NOTFOUND, vNotFound)); + MakeAndPushMessage(pfrom, NetMsgType::NOTFOUND, vNotFound); } } @@ -2478,8 +2483,7 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlo resp.txn[i] = block.vtx[req.indexes[i]]; } - const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCKTXN, resp)); + MakeAndPushMessage(pfrom, NetMsgType::BLOCKTXN, resp); } bool PeerManagerImpl::CheckHeadersPoW(const std::vector& headers, const Consensus::Params& consensusParams, Peer& peer) @@ -2707,14 +2711,12 @@ bool PeerManagerImpl::IsAncestorOfBestHeaderOrTip(const CBlockIndex* header) bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer) { - const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - const auto current_time = NodeClock::now(); // Only allow a new getheaders message to go out if we don't have a recent // one already in-flight if (current_time - peer.m_last_getheaders_timestamp > HEADERS_RESPONSE_TIME) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, locator, uint256())); + MakeAndPushMessage(pfrom, NetMsgType::GETHEADERS, locator, uint256()); peer.m_last_getheaders_timestamp = current_time; return true; } @@ -2728,8 +2730,6 @@ bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& loc */ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex& last_header) { - const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - LOCK(cs_main); CNodeState *nodestate = State(pfrom.GetId()); @@ -2782,7 +2782,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c // In any case, we want to download using a compact block, not a regular one vGetData[0] = CInv(MSG_CMPCT_BLOCK, vGetData[0].hash); } - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vGetData)); + MakeAndPushMessage(pfrom, NetMsgType::GETDATA, vGetData); } } } @@ -3130,7 +3130,7 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, return true; } -void PeerManagerImpl::ProcessGetCFilters(CNode& node,Peer& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFilters(CNode& node, Peer& peer, DataStream& vRecv) { uint8_t filter_type_ser; uint32_t start_height; @@ -3155,13 +3155,11 @@ void PeerManagerImpl::ProcessGetCFilters(CNode& node,Peer& peer, CDataStream& vR } for (const auto& filter : filters) { - CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) - .Make(NetMsgType::CFILTER, filter); - m_connman.PushMessage(&node, std::move(msg)); + MakeAndPushMessage(node, NetMsgType::CFILTER, filter); } } -void PeerManagerImpl::ProcessGetCFHeaders(CNode& node, Peer& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFHeaders(CNode& node, Peer& peer, DataStream& vRecv) { uint8_t filter_type_ser; uint32_t start_height; @@ -3196,16 +3194,14 @@ void PeerManagerImpl::ProcessGetCFHeaders(CNode& node, Peer& peer, CDataStream& return; } - CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) - .Make(NetMsgType::CFHEADERS, + MakeAndPushMessage(node, NetMsgType::CFHEADERS, filter_type_ser, stop_index->GetBlockHash(), prev_header, filter_hashes); - m_connman.PushMessage(&node, std::move(msg)); } -void PeerManagerImpl::ProcessGetCFCheckPt(CNode& node, Peer& peer, CDataStream& vRecv) +void PeerManagerImpl::ProcessGetCFCheckPt(CNode& node, Peer& peer, DataStream& vRecv) { uint8_t filter_type_ser; uint256 stop_hash; @@ -3237,12 +3233,10 @@ void PeerManagerImpl::ProcessGetCFCheckPt(CNode& node, Peer& peer, CDataStream& } } - CSerializedNetMsg msg = CNetMsgMaker(node.GetCommonVersion()) - .Make(NetMsgType::CFCHECKPT, + MakeAndPushMessage(node, NetMsgType::CFCHECKPT, filter_type_ser, stop_index->GetBlockHash(), headers); - m_connman.PushMessage(&node, std::move(msg)); } void PeerManagerImpl::ProcessBlock(CNode& node, const std::shared_ptr& block, bool force_processing, bool min_pow_checked) @@ -3266,7 +3260,6 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl { std::shared_ptr pblock = std::make_shared(); bool fBlockRead{false}; - const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); { LOCK(cs_main); @@ -3302,7 +3295,7 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl // Might have collided, fall back to getdata now :( std::vector invs; invs.emplace_back(MSG_BLOCK | GetFetchFlags(peer), block_transactions.blockhash); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, invs)); + MakeAndPushMessage(pfrom, NetMsgType::GETDATA, invs); } else { RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); LogPrint(BCLog::NET, "Peer %d sent us a compact block but it failed to reconstruct, waiting on first download to complete\n", pfrom.GetId()); @@ -3349,7 +3342,7 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl return; } -void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, +void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) { @@ -3441,10 +3434,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, pfrom.SetCommonVersion(greatest_common_version); pfrom.nVersion = nVersion; - const CNetMsgMaker msg_maker(greatest_common_version); - if (greatest_common_version >= WTXID_RELAY_VERSION) { - m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::WTXIDRELAY)); + MakeAndPushMessage(pfrom, NetMsgType::WTXIDRELAY); } // Signal ADDRv2 support (BIP155). @@ -3453,7 +3444,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // implementations reject messages they don't know. As a courtesy, don't send // it to nodes with a version before 70016, as no software is known to support // BIP155 that doesn't announce at least that protocol version number. - m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2)); + MakeAndPushMessage(pfrom, NetMsgType::SENDADDRV2); } pfrom.m_has_all_wanted_services = HasAllDesirableServiceFlags(nServices); @@ -3493,12 +3484,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (tx_relay && WITH_LOCK(tx_relay->m_bloom_filter_mutex, return tx_relay->m_relay_txs) && !pfrom.IsAddrFetchConn() && !m_opts.ignore_incoming_txs) { const uint64_t recon_salt = m_txreconciliation->PreRegisterPeer(pfrom.GetId()); - m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDTXRCNCL, - TXRECONCILIATION_VERSION, recon_salt)); + MakeAndPushMessage(pfrom, NetMsgType::SENDTXRCNCL, + TXRECONCILIATION_VERSION, recon_salt); } } - m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); + MakeAndPushMessage(pfrom, NetMsgType::VERACK); // Potentially mark this peer as a preferred download peer. { @@ -3522,7 +3513,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // We skip this for block-relay-only peers. We want to avoid // potentially leaking addr information and we do not want to // indicate to the peer that we will participate in addr relay. - m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make(NetMsgType::GETADDR)); + MakeAndPushMessage(pfrom, NetMsgType::GETADDR); peer->m_getaddr_sent = true; // When requesting a getaddr, accept an additional MAX_ADDR_TO_SEND addresses in response // (bypassing the MAX_ADDR_PROCESSING_TOKEN_BUCKET limit). @@ -3568,7 +3559,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // If the peer is old enough to have the old alert system, send it the final alert. if (greatest_common_version <= 70012) { const auto finalAlert{ParseHex("60010000000000000000000000ffffff7f00000000ffffff7ffeffff7f01ffffff7f00000000ffffff7f00ffffff7f002f555247454e543a20416c657274206b657920636f6d70726f6d697365642c2075706772616465207265717569726564004630440220653febd6410f470f6bae11cad19c48413becb1ac2c17f908fd0fd53bdc3abd5202206d0e9c96fe88d4a0f01ed9dedae2b6f9e00da94cad0fecaae66ecf689bf71b50")}; - m_connman.PushMessage(&pfrom, CNetMsgMaker(greatest_common_version).Make("alert", Span{finalAlert})); + MakeAndPushMessage(pfrom, "alert", Span{finalAlert}); } // Feeler connections exist only to verify if address is online. @@ -3585,9 +3576,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - // At this point, the outgoing message serialization version can't change. - const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - if (msg_type == NetMsgType::VERACK) { if (pfrom.fSuccessfullyConnected) { LogPrint(BCLog::NET, "ignoring redundant verack message from peer=%d\n", pfrom.GetId()); @@ -3612,7 +3600,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // cmpctblock messages. // We send this to non-NODE NETWORK peers as well, because // they may wish to request compact blocks from us - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); + MakeAndPushMessage(pfrom, NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION); } if (m_txreconciliation) { @@ -4119,7 +4107,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because active chain has too little work; sending empty response\n", pfrom.GetId()); // Just respond with an empty headers message, to tell the peer to // go away but not treat us as unresponsive. - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::HEADERS, std::vector())); + MakeAndPushMessage(pfrom, NetMsgType::HEADERS, std::vector()); return; } @@ -4169,7 +4157,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // will re-announce the new block via headers (or compact blocks again) // in the SendMessages logic. nodestate->pindexBestHeaderSent = pindex ? pindex : m_chainman.ActiveChain().Tip(); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::HEADERS, TX_WITH_WITNESS(vHeaders))); + MakeAndPushMessage(pfrom, NetMsgType::HEADERS, TX_WITH_WITNESS(vHeaders)); return; } @@ -4471,7 +4459,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // so we just grab the block via normal getdata std::vector vInv(1); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), blockhash); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); + MakeAndPushMessage(pfrom, NetMsgType::GETDATA, vInv); } return; } @@ -4508,7 +4496,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Duplicate txindexes, the block is now in-flight, so just request it std::vector vInv(1); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), blockhash); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); + MakeAndPushMessage(pfrom, NetMsgType::GETDATA, vInv); } else { // Give up for this peer and wait for other peer(s) RemoveBlockRequest(pindex->GetBlockHash(), pfrom.GetId()); @@ -4527,7 +4515,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // We will try to round-trip any compact blocks we get on failure, // as long as it's first... req.blockhash = pindex->GetBlockHash(); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETBLOCKTXN, req)); + MakeAndPushMessage(pfrom, NetMsgType::GETBLOCKTXN, req); } else if (pfrom.m_bip152_highbandwidth_to && (!pfrom.IsInboundConn() || IsBlockRequestedFromOutbound(blockhash) || @@ -4537,7 +4525,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // - we already have an outbound attempt in flight(so we'll take what we can get), or // - it's not the final parallel download slot (which we may reserve for first outbound) req.blockhash = pindex->GetBlockHash(); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETBLOCKTXN, req)); + MakeAndPushMessage(pfrom, NetMsgType::GETBLOCKTXN, req); } else { // Give up for this peer and wait for other peer(s) RemoveBlockRequest(pindex->GetBlockHash(), pfrom.GetId()); @@ -4566,7 +4554,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // mempool will probably be useless - request the block normally std::vector vInv(1); vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), blockhash); - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); + MakeAndPushMessage(pfrom, NetMsgType::GETDATA, vInv); return; } else { // If this was an announce-cmpctblock, we want the same treatment as a header message @@ -4796,7 +4784,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // it, if the remote node sends a ping once per second and this node takes 5 // seconds to respond to each, the 5th ping the remote sends would appear to // return very quickly. - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::PONG, nonce)); + MakeAndPushMessage(pfrom, NetMsgType::PONG, nonce); } return; } @@ -5068,8 +5056,6 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic& interrupt CaptureMessage(pfrom->addr, msg.m_type, MakeUCharSpan(msg.m_recv), /*is_incoming=*/true); } - msg.SetVersion(pfrom->GetCommonVersion()); - try { ProcessMessage(*pfrom, msg.m_type, msg.m_recv, msg.m_time, interruptMsgProc); if (interruptMsgProc) return false; @@ -5297,7 +5283,6 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic return; } - const CNetMsgMaker msgMaker(node_to.GetCommonVersion()); bool pingSend = false; if (peer.m_ping_queued) { @@ -5319,11 +5304,11 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic peer.m_ping_start = now; if (node_to.GetCommonVersion() > BIP0031_VERSION) { peer.m_ping_nonce_sent = nonce; - m_connman.PushMessage(&node_to, msgMaker.Make(NetMsgType::PING, nonce)); + MakeAndPushMessage(node_to, NetMsgType::PING, nonce); } else { // Peer is too old to support ping command with nonce, pong will never arrive. peer.m_ping_nonce_sent = 0; - m_connman.PushMessage(&node_to, msgMaker.Make(NetMsgType::PING)); + MakeAndPushMessage(node_to, NetMsgType::PING); } } } @@ -5377,11 +5362,10 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros // No addr messages to send if (peer.m_addrs_to_send.empty()) return; - CNetMsgMaker mm(node.GetCommonVersion()); if (peer.m_wants_addrv2) { - m_connman.PushMessage(&node, mm.Make(NetMsgType::ADDRV2, CAddress::V2_NETWORK(peer.m_addrs_to_send))); + MakeAndPushMessage(node, NetMsgType::ADDRV2, CAddress::V2_NETWORK(peer.m_addrs_to_send)); } else { - m_connman.PushMessage(&node, mm.Make(NetMsgType::ADDR, CAddress::V1_NETWORK(peer.m_addrs_to_send))); + MakeAndPushMessage(node, NetMsgType::ADDR, CAddress::V1_NETWORK(peer.m_addrs_to_send)); } peer.m_addrs_to_send.clear(); @@ -5406,7 +5390,7 @@ void PeerManagerImpl::MaybeSendSendHeaders(CNode& node, Peer& peer) // We send this to non-NODE NETWORK peers as well, because even // non-NODE NETWORK peers can announce blocks (such as pruning // nodes) - m_connman.PushMessage(&node, CNetMsgMaker(node.GetCommonVersion()).Make(NetMsgType::SENDHEADERS)); + MakeAndPushMessage(node, NetMsgType::SENDHEADERS); peer.m_sent_sendheaders = true; } } @@ -5441,7 +5425,7 @@ void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::mi // We always have a fee filter of at least the min relay fee filterToSend = std::max(filterToSend, m_mempool.m_min_relay_feerate.GetFeePerK()); if (filterToSend != peer.m_fee_filter_sent) { - m_connman.PushMessage(&pto, CNetMsgMaker(pto.GetCommonVersion()).Make(NetMsgType::FEEFILTER, filterToSend)); + MakeAndPushMessage(pto, NetMsgType::FEEFILTER, filterToSend); peer.m_fee_filter_sent = filterToSend; } peer.m_next_send_feefilter = GetExponentialRand(current_time, AVG_FEEFILTER_BROADCAST_INTERVAL); @@ -5518,9 +5502,6 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!pto->fSuccessfullyConnected || pto->fDisconnect) return true; - // If we get here, the outgoing message serialization version is set and can't change. - const CNetMsgMaker msgMaker(pto->GetCommonVersion()); - const auto current_time{GetTime()}; if (pto->IsAddrFetchConn() && current_time - pto->m_connected > 10 * AVG_ADDRESS_BROADCAST_INTERVAL) { @@ -5675,17 +5656,17 @@ bool PeerManagerImpl::SendMessages(CNode* pto) { LOCK(m_most_recent_block_mutex); if (m_most_recent_block_hash == pBestIndex->GetBlockHash()) { - cached_cmpctblock_msg = msgMaker.Make(NetMsgType::CMPCTBLOCK, *m_most_recent_compact_block); + cached_cmpctblock_msg = NetMsg::Make(NetMsgType::CMPCTBLOCK, *m_most_recent_compact_block); } } if (cached_cmpctblock_msg.has_value()) { - m_connman.PushMessage(pto, std::move(cached_cmpctblock_msg.value())); + PushMessage(*pto, std::move(cached_cmpctblock_msg.value())); } else { CBlock block; const bool ret{m_chainman.m_blockman.ReadBlockFromDisk(block, *pBestIndex)}; assert(ret); CBlockHeaderAndShortTxIDs cmpctblock{block}; - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock)); + MakeAndPushMessage(*pto, NetMsgType::CMPCTBLOCK, cmpctblock); } state.pindexBestHeaderSent = pBestIndex; } else if (peer->m_prefers_headers) { @@ -5698,7 +5679,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) LogPrint(BCLog::NET, "%s: sending header %s to peer=%d\n", __func__, vHeaders.front().GetHash().ToString(), pto->GetId()); } - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::HEADERS, TX_WITH_WITNESS(vHeaders))); + MakeAndPushMessage(*pto, NetMsgType::HEADERS, TX_WITH_WITNESS(vHeaders)); state.pindexBestHeaderSent = pBestIndex; } else fRevertToInv = true; @@ -5743,7 +5724,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) for (const uint256& hash : peer->m_blocks_for_inv_relay) { vInv.emplace_back(MSG_BLOCK, hash); if (vInv.size() == MAX_INV_SZ) { - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + MakeAndPushMessage(*pto, NetMsgType::INV, vInv); vInv.clear(); } } @@ -5796,7 +5777,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) tx_relay->m_tx_inventory_known_filter.insert(inv.hash); vInv.push_back(inv); if (vInv.size() == MAX_INV_SZ) { - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + MakeAndPushMessage(*pto, NetMsgType::INV, vInv); vInv.clear(); } } @@ -5848,7 +5829,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) vInv.push_back(inv); nRelayedTransactions++; if (vInv.size() == MAX_INV_SZ) { - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + MakeAndPushMessage(*pto, NetMsgType::INV, vInv); vInv.clear(); } tx_relay->m_tx_inventory_known_filter.insert(hash); @@ -5860,7 +5841,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } if (!vInv.empty()) - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); + MakeAndPushMessage(*pto, NetMsgType::INV, vInv); // Detect whether we're stalling auto stalling_timeout = m_block_stalling_timeout.load(); @@ -5980,7 +5961,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) gtxid.GetHash().ToString(), pto->GetId()); vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash()); if (vGetData.size() >= MAX_GETDATA_SZ) { - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); + MakeAndPushMessage(*pto, NetMsgType::GETDATA, vGetData); vGetData.clear(); } m_txrequest.RequestedTx(pto->GetId(), gtxid.GetHash(), current_time + GETDATA_TX_INTERVAL); @@ -5993,7 +5974,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!vGetData.empty()) - m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::GETDATA, vGetData)); + MakeAndPushMessage(*pto, NetMsgType::GETDATA, vGetData); } // release cs_main MaybeSendFeefilter(*pto, *peer, current_time); return true; diff --git a/src/net_processing.h b/src/net_processing.h index 80d07648a48..afdef000674 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -105,7 +105,7 @@ public: virtual void CheckForStaleTipAndEvictPeers() = 0; /** Process a single message from a peer. Public for fuzz testing */ - virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, + virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */ diff --git a/src/netmessagemaker.h b/src/netmessagemaker.h index a121183aabe..d04867e45e9 100644 --- a/src/netmessagemaker.h +++ b/src/netmessagemaker.h @@ -9,28 +9,15 @@ #include #include -class CNetMsgMaker -{ -public: - explicit CNetMsgMaker(int nVersionIn) : nVersion(nVersionIn){} - +namespace NetMsg { template - CSerializedNetMsg Make(int nFlags, std::string msg_type, Args&&... args) const + CSerializedNetMsg Make(std::string msg_type, Args&&... args) { CSerializedNetMsg msg; msg.m_type = std::move(msg_type); - CVectorWriter{nFlags | nVersion, msg.data, 0, std::forward(args)...}; + VectorWriter{msg.data, 0, std::forward(args)...}; return msg; } - - template - CSerializedNetMsg Make(std::string msg_type, Args&&... args) const - { - return Make(0, std::move(msg_type), std::forward(args)...); - } - -private: - const int nVersion; -}; +} // namespace NetMsg #endif // BITCOIN_NETMESSAGEMAKER_H diff --git a/src/psbt.h b/src/psbt.h index a14df038378..ac2d246a27a 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -316,7 +316,7 @@ struct PSBTInput const auto& [leaf_hashes, origin] = leaf_origin; SerializeToVector(s, PSBT_IN_TAP_BIP32_DERIVATION, xonly); std::vector value; - CVectorWriter s_value{s.GetVersion(), value, 0}; + VectorWriter s_value{value, 0}; s_value << leaf_hashes; SerializeKeyOrigin(s_value, origin); s << value; @@ -757,7 +757,7 @@ struct PSBTOutput if (!m_tap_tree.empty()) { SerializeToVector(s, PSBT_OUT_TAP_TREE); std::vector value; - CVectorWriter s_value{s.GetVersion(), value, 0}; + VectorWriter s_value{value, 0}; for (const auto& [depth, leaf_ver, script] : m_tap_tree) { s_value << depth; s_value << leaf_ver; @@ -771,7 +771,7 @@ struct PSBTOutput const auto& [leaf_hashes, origin] = leaf; SerializeToVector(s, PSBT_OUT_TAP_BIP32_DERIVATION, xonly); std::vector value; - CVectorWriter s_value{s.GetVersion(), value, 0}; + VectorWriter s_value{value, 0}; s_value << leaf_hashes; SerializeKeyOrigin(s_value, origin); s << value; diff --git a/src/signet.cpp b/src/signet.cpp index d6a3a44d913..ef9913218a1 100644 --- a/src/signet.cpp +++ b/src/signet.cpp @@ -110,7 +110,7 @@ std::optional SignetTxs::Create(const CBlock& block, const CScript& c uint256 signet_merkle = ComputeModifiedMerkleRoot(modified_cb, block); std::vector block_data; - CVectorWriter writer{INIT_PROTO_VERSION, block_data, 0}; + VectorWriter writer{block_data, 0}; writer << block.nVersion; writer << block.hashPrevBlock; writer << signet_merkle; diff --git a/src/streams.h b/src/streams.h index 068e2e8efd5..5fec289e34e 100644 --- a/src/streams.h +++ b/src/streams.h @@ -49,17 +49,15 @@ inline void Xor(Span write, Span key, size_t key_off * * The referenced vector will grow as necessary */ -class CVectorWriter +class VectorWriter { - public: - +public: /* - * @param[in] nVersionIn Serialization Version (including any flags) * @param[in] vchDataIn Referenced byte vector to overwrite/append * @param[in] nPosIn Starting position. Vector index where writes should start. The vector will initially * grow as necessary to max(nPosIn, vec.size()). So to append, use vec.size(). */ - CVectorWriter(int nVersionIn, std::vector& vchDataIn, size_t nPosIn) : nVersion{nVersionIn}, vchData{vchDataIn}, nPos{nPosIn} + VectorWriter(std::vector& vchDataIn, size_t nPosIn) : vchData{vchDataIn}, nPos{nPosIn} { if(nPos > vchData.size()) vchData.resize(nPos); @@ -69,7 +67,7 @@ class CVectorWriter * @param[in] args A list of items to serialize starting at nPosIn. */ template - CVectorWriter(int nVersionIn, std::vector& vchDataIn, size_t nPosIn, Args&&... args) : CVectorWriter{nVersionIn, vchDataIn, nPosIn} + VectorWriter(std::vector& vchDataIn, size_t nPosIn, Args&&... args) : VectorWriter{vchDataIn, nPosIn} { ::SerializeMany(*this, std::forward(args)...); } @@ -85,19 +83,14 @@ class CVectorWriter } nPos += src.size(); } - template - CVectorWriter& operator<<(const T& obj) + template + VectorWriter& operator<<(const T& obj) { ::Serialize(*this, obj); return (*this); } - int GetVersion() const - { - return nVersion; - } private: - const int nVersion; std::vector& vchData; size_t nPos; }; diff --git a/src/test/fuzz/golomb_rice.cpp b/src/test/fuzz/golomb_rice.cpp index f3073c5c973..8b395f3ea95 100644 --- a/src/test/fuzz/golomb_rice.cpp +++ b/src/test/fuzz/golomb_rice.cpp @@ -51,9 +51,9 @@ FUZZ_TARGET(golomb_rice) for (int i = 0; i < n; ++i) { elements.insert(ConsumeRandomLengthByteVector(fuzzed_data_provider, 16)); } - CVectorWriter stream{0, golomb_rice_data, 0}; + VectorWriter stream{golomb_rice_data, 0}; WriteCompactSize(stream, static_cast(elements.size())); - BitStreamWriter bitwriter(stream); + BitStreamWriter bitwriter{stream}; if (!elements.empty()) { uint64_t last_value = 0; for (const uint64_t value : BuildHashedSet(elements, static_cast(elements.size()) * static_cast(BASIC_FILTER_M))) { diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 21d8dab5360..a205ce19f4a 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -36,8 +36,8 @@ void initialize_p2p_transport_serialization() FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serialization) { // Construct transports for both sides, with dummy NodeIds. - V1Transport recv_transport{NodeId{0}, SER_NETWORK, INIT_PROTO_VERSION}; - V1Transport send_transport{NodeId{1}, SER_NETWORK, INIT_PROTO_VERSION}; + V1Transport recv_transport{NodeId{0}}; + V1Transport send_transport{NodeId{1}}; FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; @@ -88,7 +88,7 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial assert(msg.m_time == m_time); std::vector header; - auto msg2 = CNetMsgMaker{msg.m_recv.GetVersion()}.Make(msg.m_type, Span{msg.m_recv}); + auto msg2 = NetMsg::Make(msg.m_type, Span{msg.m_recv}); bool queued = send_transport.SetMessageToSend(msg2); assert(queued); std::optional known_more; @@ -335,7 +335,7 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa std::unique_ptr MakeV1Transport(NodeId nodeid) noexcept { - return std::make_unique(nodeid, SER_NETWORK, INIT_PROTO_VERSION); + return std::make_unique(nodeid); } template @@ -369,7 +369,7 @@ std::unique_ptr MakeV2Transport(NodeId nodeid, bool initiator, RNG& r .Write(garb.data(), garb.size()) .Finalize(UCharCast(ent.data())); - return std::make_unique(nodeid, initiator, SER_NETWORK, INIT_PROTO_VERSION, key, ent, std::move(garb)); + return std::make_unique(nodeid, initiator, key, ent, std::move(garb)); } } // namespace diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 48e0706a53b..508c42cabc9 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -845,7 +845,6 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) const uint64_t services{NODE_NETWORK | NODE_WITNESS}; const int64_t time{0}; - const CNetMsgMaker msg_maker{PROTOCOL_VERSION}; // Force ChainstateManager::IsInitialBlockDownload() to return false. // Otherwise PushAddress() isn't called by PeerManager::ProcessMessage(). @@ -858,13 +857,13 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) std::chrono::microseconds time_received_dummy{0}; const auto msg_version = - msg_maker.Make(NetMsgType::VERSION, PROTOCOL_VERSION, services, time, services, CAddress::V1_NETWORK(peer_us)); + NetMsg::Make(NetMsgType::VERSION, PROTOCOL_VERSION, services, time, services, CAddress::V1_NETWORK(peer_us)); CDataStream msg_version_stream{msg_version.data, SER_NETWORK, PROTOCOL_VERSION}; m_node.peerman->ProcessMessage( peer, NetMsgType::VERSION, msg_version_stream, time_received_dummy, interrupt_dummy); - const auto msg_verack = msg_maker.Make(NetMsgType::VERACK); + const auto msg_verack = NetMsg::Make(NetMsgType::VERACK); CDataStream msg_verack_stream{msg_verack.data, SER_NETWORK, PROTOCOL_VERSION}; // Will set peer.fSuccessfullyConnected to true (necessary in SendMessages()). @@ -1047,10 +1046,10 @@ class V2TransportTester public: /** Construct a tester object. test_initiator: whether the tested transport is initiator. */ - V2TransportTester(bool test_initiator) : - m_transport(0, test_initiator, SER_NETWORK, INIT_PROTO_VERSION), - m_cipher{GenerateRandomTestKey(), MakeByteSpan(InsecureRand256())}, - m_test_initiator(test_initiator) {} + explicit V2TransportTester(bool test_initiator) + : m_transport{0, test_initiator}, + m_cipher{GenerateRandomTestKey(), MakeByteSpan(InsecureRand256())}, + m_test_initiator(test_initiator) {} /** Data type returned by Interact: * diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index d8478ecf5f9..693247b8d4c 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -74,49 +74,49 @@ BOOST_AUTO_TEST_CASE(streams_vector_writer) // point should yield the same results, even if the first test grew the // vector. - CVectorWriter{INIT_PROTO_VERSION, vch, 0, a, b}; + VectorWriter{vch, 0, a, b}; BOOST_CHECK((vch == std::vector{{1, 2}})); - CVectorWriter{INIT_PROTO_VERSION, vch, 0, a, b}; + VectorWriter{vch, 0, a, b}; BOOST_CHECK((vch == std::vector{{1, 2}})); vch.clear(); - CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b}; + VectorWriter{vch, 2, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 1, 2}})); - CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b}; + VectorWriter{vch, 2, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 1, 2}})); vch.clear(); vch.resize(5, 0); - CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b}; + VectorWriter{vch, 2, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 1, 2, 0}})); - CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b}; + VectorWriter{vch, 2, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 1, 2, 0}})); vch.clear(); vch.resize(4, 0); - CVectorWriter{INIT_PROTO_VERSION, vch, 3, a, b}; + VectorWriter{vch, 3, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 0, 1, 2}})); - CVectorWriter{INIT_PROTO_VERSION, vch, 3, a, b}; + VectorWriter{vch, 3, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 0, 1, 2}})); vch.clear(); vch.resize(4, 0); - CVectorWriter{INIT_PROTO_VERSION, vch, 4, a, b}; + VectorWriter{vch, 4, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 0, 0, 1, 2}})); - CVectorWriter{INIT_PROTO_VERSION, vch, 4, a, b}; + VectorWriter{vch, 4, a, b}; BOOST_CHECK((vch == std::vector{{0, 0, 0, 0, 1, 2}})); vch.clear(); - CVectorWriter{INIT_PROTO_VERSION, vch, 0, bytes}; + VectorWriter{vch, 0, bytes}; BOOST_CHECK((vch == std::vector{{3, 4, 5, 6}})); - CVectorWriter{INIT_PROTO_VERSION, vch, 0, bytes}; + VectorWriter{vch, 0, bytes}; BOOST_CHECK((vch == std::vector{{3, 4, 5, 6}})); vch.clear(); vch.resize(4, 8); - CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, bytes, b}; + VectorWriter{vch, 2, a, bytes, b}; BOOST_CHECK((vch == std::vector{{8, 8, 1, 3, 4, 5, 6, 2}})); - CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, bytes, b}; + VectorWriter{vch, 2, a, bytes, b}; BOOST_CHECK((vch == std::vector{{8, 8, 1, 3, 4, 5, 6, 2}})); vch.clear(); } diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index e0404e33ed5..9257a4964ac 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -26,13 +26,12 @@ void ConnmanTestMsg::Handshake(CNode& node, { auto& peerman{static_cast(*m_msgproc)}; auto& connman{*this}; - const CNetMsgMaker mm{0}; peerman.InitializeNode(node, local_services); FlushSendBuffer(node); // Drop the version message added by InitializeNode. CSerializedNetMsg msg_version{ - mm.Make(NetMsgType::VERSION, + NetMsg::Make(NetMsgType::VERSION, version, // Using>(remote_services), // int64_t{}, // dummy time @@ -59,7 +58,7 @@ void ConnmanTestMsg::Handshake(CNode& node, assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn())); assert(statestats.their_services == remote_services); if (successfully_connected) { - CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)}; + CSerializedNetMsg msg_verack{NetMsg::Make(NetMsgType::VERACK)}; (void)connman.ReceiveMsgFrom(node, std::move(msg_verack)); node.fPauseSend = false; connman.ProcessMessagesOnce(node); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 63932c64307..6d7b9100b58 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -752,7 +752,7 @@ bool malformed_descriptor(std::ios_base::failure e) BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup) { std::vector malformed_record; - CVectorWriter vw{0, malformed_record, 0}; + VectorWriter vw{malformed_record, 0}; vw << std::string("notadescriptor"); vw << uint64_t{0}; vw << int32_t{0};