diff options
| -rw-r--r-- | src/net.cpp | 29 | ||||
| -rw-r--r-- | src/net.h | 28 | ||||
| -rw-r--r-- | src/net_processing.cpp | 18 | ||||
| -rw-r--r-- | src/net_processing.h | 2 | ||||
| -rw-r--r-- | src/test/fuzz/p2p_transport_serialization.cpp | 8 | ||||
| -rw-r--r-- | src/test/net_tests.cpp | 8 |
6 files changed, 40 insertions, 53 deletions
diff --git a/src/net.cpp b/src/net.cpp index 075a7d9839..e1772233b2 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -683,8 +683,8 @@ bool CNode::ReceiveMsgBytes(Span<const uint8_t> 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(); @@ -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<const std::byte> ent32, std::vector<uint8_t> 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<const std::byte> ent32, std::vector<uint8_t> 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<const uint8_t> 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) { @@ -3638,9 +3637,9 @@ ServiceFlags CConnman::GetLocalServices() const static std::unique_ptr<Transport> MakeTransport(NodeId id, bool use_v2transport, bool inbound) noexcept { if (use_v2transport) { - return std::make_unique<V2Transport>(id, /*initiating=*/!inbound, SER_NETWORK, INIT_PROTO_VERSION); + return std::make_unique<V2Transport>(id, /*initiating=*/!inbound); } else { - return std::make_unique<V1Transport>(id, SER_NETWORK, INIT_PROTO_VERSION); + return std::make_unique<V1Transport>(id); } } @@ -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<uint8_t> m_recv_aad GUARDED_BY(m_recv_mutex); /** Buffer to put decrypted contents in, for converting to CNetMessage. */ std::vector<uint8_t> 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<const std::byte> ent32, std::vector<uint8_t> garbage) noexcept; + V2Transport(NodeId nodeid, bool initiating, const CKey& key, Span<const std::byte> ent32, std::vector<uint8_t> 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 0cd20680b0..533346ac81 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -515,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<bool>& 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; @@ -1033,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. @@ -1044,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. @@ -1055,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. @@ -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; @@ -3159,7 +3159,7 @@ void PeerManagerImpl::ProcessGetCFilters(CNode& node,Peer& peer, CDataStream& vR } } -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; @@ -3201,7 +3201,7 @@ void PeerManagerImpl::ProcessGetCFHeaders(CNode& node, Peer& peer, CDataStream& filter_hashes); } -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; @@ -3342,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<bool>& interruptMsgProc) { @@ -5056,8 +5056,6 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& 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; diff --git a/src/net_processing.h b/src/net_processing.h index 80d07648a4..afdef00067 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<bool>& 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/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 6af6aa1d18..a205ce19f4 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()}; @@ -335,7 +335,7 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa std::unique_ptr<Transport> MakeV1Transport(NodeId nodeid) noexcept { - return std::make_unique<V1Transport>(nodeid, SER_NETWORK, INIT_PROTO_VERSION); + return std::make_unique<V1Transport>(nodeid); } template<typename RNG> @@ -369,7 +369,7 @@ std::unique_ptr<Transport> MakeV2Transport(NodeId nodeid, bool initiator, RNG& r .Write(garb.data(), garb.size()) .Finalize(UCharCast(ent.data())); - return std::make_unique<V2Transport>(nodeid, initiator, SER_NETWORK, INIT_PROTO_VERSION, key, ent, std::move(garb)); + return std::make_unique<V2Transport>(nodeid, initiator, key, ent, std::move(garb)); } } // namespace diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 3c9227cfc4..508c42cabc 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1046,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: * |