diff options
author | fanquake <fanquake@gmail.com> | 2020-09-29 15:50:03 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2020-09-29 16:14:40 +0800 |
commit | 6af9b31bfc6dd6086d245c00eb7cff762fc34a1e (patch) | |
tree | e210292fa0ea40a5ef4827a22361f2eaaa8997f2 /src/net.h | |
parent | e36aa351a31cde0f95ce957b2ff593a97f91eb6d (diff) | |
parent | deb52711a17236d0fca302701b5af585341ab42a (diff) | |
download | bitcoin-6af9b31bfc6dd6086d245c00eb7cff762fc34a1e.tar.xz |
Merge #19107: p2p: Move all header verification into the network layer, extend logging
deb52711a17236d0fca302701b5af585341ab42a Remove header checks out of net_processing (Troy Giorshev)
52d4ae46ab822d0f54e246a6f2364415cda149bd Give V1TransportDeserializer CChainParams& member (Troy Giorshev)
5bceef6b12fa16d20287693be377dace3dfec3e5 Change CMessageHeader Constructor (Troy Giorshev)
1ca20c1af8f08f07c407c3183c37b467ddf0f413 Add doxygen comment for ReceiveMsgBytes (Troy Giorshev)
890b1d7c2b8312d41d048d2db124586c5dbc8a49 Move checksum check from net_processing to net (Troy Giorshev)
2716647ebf60cea05fc9edce6a18dcce4e7727ad 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 deb52711a17236d0fca302701b5af585341ab42a just rebase due to conflict on adjacent line
jnewbery:
Code review ACK deb52711a17236d0fca302701b5af585341ab42a.
Tree-SHA512: 1a3b7ae883b020cfee1bef968813e04df651ffdad9dd961a826bd80654f2c98676ce7f4721038a1b78d8790e4cebe8060419e3d8affc97ce2b9b4e4b72e6fa9f
Diffstat (limited to 'src/net.h')
-rw-r--r-- | src/net.h | 25 |
1 files changed, 15 insertions, 10 deletions
@@ -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 |