From 649a83c7f73db2ee115f5dce3df16622e318aeba Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 14 Aug 2023 16:37:05 -0400 Subject: [PATCH] refactor: rename Transport class receive functions Now that the Transport class deals with both the sending and receiving side of things, make the receive side have function names that clearly indicate they're about receiving. * Transport::Read() -> Transport::ReceivedBytes() * Transport::Complete() -> Transport::ReceivedMessageComplete() * Transport::GetMessage() -> Transport::GetReceivedMessage() * Transport::SetVersion() -> Transport::SetReceiveVersion() Further, also update the comments on these functions to (among others) remove the "deserialization" terminology. That term is better reserved for just the serialization/deserialization between objects and bytes (see serialize.h), and not the conversion from/to wire bytes as performed by the Transport. --- src/net.cpp | 8 +++--- src/net.h | 25 ++++++++++--------- src/test/fuzz/p2p_transport_serialization.cpp | 6 ++--- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index b350c58c615..338831bb48b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -681,16 +681,16 @@ bool CNode::ReceiveMsgBytes(Span msg_bytes, bool& complete) nRecvBytes += msg_bytes.size(); while (msg_bytes.size() > 0) { // absorb network data - int handled = m_transport->Read(msg_bytes); + int handled = m_transport->ReceivedBytes(msg_bytes); if (handled < 0) { // Serious header problem, disconnect from the peer. return false; } - if (m_transport->Complete()) { + if (m_transport->ReceivedMessageComplete()) { // decompose a transport agnostic CNetMessage from the deserializer bool reject_message{false}; - CNetMessage msg = m_transport->GetMessage(time, reject_message); + CNetMessage msg = m_transport->GetReceivedMessage(time, reject_message); if (reject_message) { // Message deserialization failed. Drop the message but don't disconnect the peer. // store the size of the corrupt message @@ -785,7 +785,7 @@ const uint256& V1Transport::GetMessageHash() const return data_hash; } -CNetMessage V1Transport::GetMessage(const std::chrono::microseconds time, bool& reject_message) +CNetMessage V1Transport::GetReceivedMessage(const std::chrono::microseconds time, bool& reject_message) { AssertLockNotHeld(m_recv_mutex); // Initialize out parameter diff --git a/src/net.h b/src/net.h index e34ea590cc7..a17ca36652a 100644 --- a/src/net.h +++ b/src/net.h @@ -261,14 +261,14 @@ public: // 1. Receiver side functions, for decoding bytes received on the wire into transport protocol // agnostic CNetMessage (message type & payload) objects. - // returns true if the current deserialization is complete - virtual bool Complete() const = 0; - // set the deserialization context version - virtual void SetVersion(int version) = 0; - /** read and deserialize data, advances msg_bytes data pointer */ - virtual int Read(Span& msg_bytes) = 0; - // decomposes a message from the context - virtual CNetMessage GetMessage(std::chrono::microseconds time, bool& reject_message) = 0; + /** Returns true if the current message is complete (so GetReceivedMessage can be called). */ + virtual bool ReceivedMessageComplete() const = 0; + /** Set the deserialization context version for objects returned by GetReceivedMessage. */ + virtual void SetReceiveVersion(int version) = 0; + /** Feed wire bytes to the transport; chops off consumed bytes off front of msg_bytes. */ + virtual int ReceivedBytes(Span& msg_bytes) = 0; + /** Retrieve a completed message from transport (only when ReceivedMessageComplete). */ + virtual CNetMessage GetReceivedMessage(std::chrono::microseconds time, bool& reject_message) = 0; // 2. Sending side functions: @@ -325,13 +325,13 @@ public: Reset(); } - bool Complete() const override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) + bool ReceivedMessageComplete() const override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) { AssertLockNotHeld(m_recv_mutex); return WITH_LOCK(m_recv_mutex, return CompleteInternal()); } - void SetVersion(int nVersionIn) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) + void SetReceiveVersion(int nVersionIn) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) { AssertLockNotHeld(m_recv_mutex); LOCK(m_recv_mutex); @@ -339,7 +339,7 @@ public: vRecv.SetVersion(nVersionIn); } - int Read(Span& msg_bytes) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) + int ReceivedBytes(Span& msg_bytes) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex) { AssertLockNotHeld(m_recv_mutex); LOCK(m_recv_mutex); @@ -351,7 +351,8 @@ public: } return ret; } - CNetMessage GetMessage(std::chrono::microseconds time, bool& reject_message) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); + + CNetMessage GetReceivedMessage(std::chrono::microseconds time, bool& reject_message) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex); void prepareForTransport(CSerializedNetMsg& msg, std::vector& header) const override; }; diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 5e44421f1d7..dcf7529918c 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -64,14 +64,14 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial mutable_msg_bytes.insert(mutable_msg_bytes.end(), payload_bytes.begin(), payload_bytes.end()); Span msg_bytes{mutable_msg_bytes}; while (msg_bytes.size() > 0) { - const int handled = recv_transport.Read(msg_bytes); + const int handled = recv_transport.ReceivedBytes(msg_bytes); if (handled < 0) { break; } - if (recv_transport.Complete()) { + if (recv_transport.ReceivedMessageComplete()) { const std::chrono::microseconds m_time{std::numeric_limits::max()}; bool reject_message{false}; - CNetMessage msg = recv_transport.GetMessage(m_time, reject_message); + CNetMessage msg = recv_transport.GetReceivedMessage(m_time, reject_message); assert(msg.m_type.size() <= CMessageHeader::COMMAND_SIZE); assert(msg.m_raw_message_size <= mutable_msg_bytes.size()); assert(msg.m_raw_message_size == CMessageHeader::HEADER_SIZE + msg.m_message_size);