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_processing.cpp | |
| 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_processing.cpp')
| -rw-r--r-- | src/net_processing.cpp | 32 |
1 files changed, 1 insertions, 31 deletions
diff --git a/src/net_processing.cpp b/src/net_processing.cpp index d2a76550ea..f784a9b754 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3815,14 +3815,6 @@ bool PeerManager::MaybeDiscourageAndDisconnect(CNode& pnode) bool PeerManager::ProcessMessages(CNode* pfrom, std::atomic<bool>& interruptMsgProc) { - // - // Message format - // (4) message start - // (12) command - // (4) size - // (4) checksum - // (x) data - // bool fMoreWork = false; if (!pfrom->vRecvGetData.empty()) @@ -3863,35 +3855,13 @@ bool PeerManager::ProcessMessages(CNode* pfrom, std::atomic<bool>& interruptMsgP CNetMessage& msg(msgs.front()); msg.SetVersion(pfrom->GetCommonVersion()); - // Check network magic - if (!msg.m_valid_netmagic) { - LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.m_command), pfrom->GetId()); - pfrom->fDisconnect = true; - return false; - } - - // Check header - if (!msg.m_valid_header) - { - LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(msg.m_command), pfrom->GetId()); - return fMoreWork; - } const std::string& msg_type = msg.m_command; // Message size unsigned int nMessageSize = msg.m_message_size; - // Checksum - CDataStream& vRecv = msg.m_recv; - if (!msg.m_valid_checksum) - { - LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR peer=%d\n", __func__, - SanitizeString(msg_type), nMessageSize, pfrom->GetId()); - return fMoreWork; - } - try { - ProcessMessage(*pfrom, msg_type, vRecv, msg.m_time, interruptMsgProc); + ProcessMessage(*pfrom, msg_type, msg.m_recv, msg.m_time, interruptMsgProc); if (interruptMsgProc) return false; if (!pfrom->vRecvGetData.empty()) |