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/test | |
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/test')
-rw-r--r-- | src/test/fuzz/deserialize.cpp | 5 | ||||
-rw-r--r-- | src/test/fuzz/p2p_transport_deserializer.cpp | 20 |
2 files changed, 11 insertions, 14 deletions
diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 54793c890f..b799d3b43b 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -189,10 +189,9 @@ void test_one_input(const std::vector<uint8_t>& buffer) DeserializeFromFuzzingInput(buffer, s); AssertEqualAfterSerializeDeserialize(s); #elif MESSAGEHEADER_DESERIALIZE - const CMessageHeader::MessageStartChars pchMessageStart = {0x00, 0x00, 0x00, 0x00}; - CMessageHeader mh(pchMessageStart); + CMessageHeader mh; DeserializeFromFuzzingInput(buffer, mh); - (void)mh.IsValid(pchMessageStart); + (void)mh.IsCommandValid(); #elif ADDRESS_DESERIALIZE CAddress a; DeserializeFromFuzzingInput(buffer, a); diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp index 6fba2bfaba..7e216e16fe 100644 --- a/src/test/fuzz/p2p_transport_deserializer.cpp +++ b/src/test/fuzz/p2p_transport_deserializer.cpp @@ -19,7 +19,8 @@ void initialize() void test_one_input(const std::vector<uint8_t>& buffer) { - V1TransportDeserializer deserializer{Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION}; + // Construct deserializer, with a dummy NodeId + V1TransportDeserializer deserializer{Params(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION}; const char* pch = (const char*)buffer.data(); size_t n_bytes = buffer.size(); while (n_bytes > 0) { @@ -31,16 +32,13 @@ void test_one_input(const std::vector<uint8_t>& buffer) n_bytes -= handled; if (deserializer.Complete()) { const std::chrono::microseconds m_time{std::numeric_limits<int64_t>::max()}; - const CNetMessage msg = deserializer.GetMessage(Params().MessageStart(), m_time); - assert(msg.m_command.size() <= CMessageHeader::COMMAND_SIZE); - assert(msg.m_raw_message_size <= buffer.size()); - assert(msg.m_raw_message_size == CMessageHeader::HEADER_SIZE + msg.m_message_size); - assert(msg.m_time == m_time); - if (msg.m_valid_header) { - assert(msg.m_valid_netmagic); - } - if (!msg.m_valid_netmagic) { - assert(!msg.m_valid_header); + uint32_t out_err_raw_size{0}; + Optional<CNetMessage> result{deserializer.GetMessage(m_time, out_err_raw_size)}; + if (result) { + assert(result->m_command.size() <= CMessageHeader::COMMAND_SIZE); + assert(result->m_raw_message_size <= buffer.size()); + assert(result->m_raw_message_size == CMessageHeader::HEADER_SIZE + result->m_message_size); + assert(result->m_time == m_time); } } } |