Merge #19107: p2p: Move all header verification into the network layer, extend logging

deb52711a1 Remove header checks out of net_processing (Troy Giorshev)
52d4ae46ab Give V1TransportDeserializer CChainParams& member (Troy Giorshev)
5bceef6b12 Change CMessageHeader Constructor (Troy Giorshev)
1ca20c1af8 Add doxygen comment for ReceiveMsgBytes (Troy Giorshev)
890b1d7c2b Move checksum check from net_processing to net (Troy Giorshev)
2716647ebf Give V1TransportDeserializer an m_node_id member (Troy Giorshev)

Pull request description:

  Inspired by #15206 and #15197, this PR moves all message header verification from the message processing layer and into the network/transport layer.

  In the previous PRs there is a change in behavior, where we would disconnect from peers upon a single failed checksum check.  In various discussions there was concern over whether this was the right choice, and some expressed a desire to see how this would look if it was made to be a pure refactor.

  For more context, see https://bitcoincore.reviews/15206.html#l-81.

  This PR improves the separation between the p2p layers, helping improvements like [BIP324](https://github.com/bitcoin/bitcoin/pull/18242) and #18989.

ACKs for top commit:
  ryanofsky:
    Code review ACK deb52711a1 just rebase due to conflict on adjacent line
  jnewbery:
    Code review ACK deb52711a1.

Tree-SHA512: 1a3b7ae883b020cfee1bef968813e04df651ffdad9dd961a826bd80654f2c98676ce7f4721038a1b78d8790e4cebe8060419e3d8affc97ce2b9b4e4b72e6fa9f
This commit is contained in:
fanquake
2020-09-29 15:50:03 +08:00
8 changed files with 99 additions and 109 deletions

View File

@@ -10,12 +10,14 @@
#include <addrman.h>
#include <amount.h>
#include <bloom.h>
#include <chainparams.h>
#include <compat.h>
#include <crypto/siphash.h>
#include <hash.h>
#include <limitedmap.h>
#include <netaddress.h>
#include <net_permissions.h>
#include <netaddress.h>
#include <optional.h>
#include <policy/feerate.h>
#include <protocol.h>
#include <random.h>
@@ -712,11 +714,8 @@ class CNetMessage {
public:
CDataStream m_recv; //!< received message data
std::chrono::microseconds m_time{0}; //!< time of message receipt
bool m_valid_netmagic = false;
bool m_valid_header = false;
bool m_valid_checksum = false;
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)
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_command;
CNetMessage(CDataStream&& recv_in) : m_recv(std::move(recv_in)) {}
@@ -740,13 +739,15 @@ public:
// read and deserialize data
virtual int Read(const char *data, unsigned int bytes) = 0;
// decomposes a message from the context
virtual CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, std::chrono::microseconds time) = 0;
virtual Optional<CNetMessage> GetMessage(std::chrono::microseconds time, uint32_t& out_err) = 0;
virtual ~TransportDeserializer() {}
};
class V1TransportDeserializer final : public TransportDeserializer
{
private:
const CChainParams& m_chain_params;
const NodeId m_node_id; // Only for logging
mutable CHash256 hasher;
mutable uint256 data_hash;
bool in_data; // parsing header (false) or data (true)
@@ -772,8 +773,12 @@ private:
}
public:
V1TransportDeserializer(const CMessageHeader::MessageStartChars& pchMessageStartIn, int nTypeIn, int nVersionIn) : hdrbuf(nTypeIn, nVersionIn), hdr(pchMessageStartIn), vRecv(nTypeIn, nVersionIn) {
V1TransportDeserializer(const CChainParams& chain_params, const NodeId node_id, int nTypeIn, int nVersionIn)
: m_chain_params(chain_params),
m_node_id(node_id),
hdrbuf(nTypeIn, nVersionIn),
vRecv(nTypeIn, nVersionIn)
{
Reset();
}
@@ -793,7 +798,7 @@ public:
if (ret < 0) Reset();
return ret;
}
CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, std::chrono::microseconds time) override;
Optional<CNetMessage> GetMessage(std::chrono::microseconds time, uint32_t& out_err_raw_size) override;
};
/** The TransportSerializer prepares messages for the network transport