Skip to content

Commit

Permalink
refactor: rename Transport class receive functions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sipa committed Aug 23, 2023
1 parent 27f9ba2 commit 649a83c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 19 deletions.
8 changes: 4 additions & 4 deletions src/net.cpp
Expand Up @@ -681,16 +681,16 @@ bool CNode::ReceiveMsgBytes(Span<const uint8_t> 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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 13 additions & 12 deletions src/net.h
Expand Up @@ -261,14 +261,14 @@ class Transport {
// 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<const uint8_t>& 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<const uint8_t>& 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:

Expand Down Expand Up @@ -325,21 +325,21 @@ class V1Transport final : public Transport
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);
hdrbuf.SetVersion(nVersionIn);
vRecv.SetVersion(nVersionIn);
}

int Read(Span<const uint8_t>& msg_bytes) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex)
int ReceivedBytes(Span<const uint8_t>& msg_bytes) override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex)
{
AssertLockNotHeld(m_recv_mutex);
LOCK(m_recv_mutex);
Expand All @@ -351,7 +351,8 @@ class V1Transport final : public Transport
}
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<unsigned char>& header) const override;
};
Expand Down
6 changes: 3 additions & 3 deletions src/test/fuzz/p2p_transport_serialization.cpp
Expand Up @@ -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<const uint8_t> 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<int64_t>::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);
Expand Down

0 comments on commit 649a83c

Please sign in to comment.