[net] disallow sending messages before receiving verack + enable p2p_leak.py test #2639
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
coming from btc@c45b9fb54c5ca068a5e276c3bd6ebf4ae720f6f7
7a8c251 made this logic hard to follow. After that change, messages would not be sent to a peer via SendMessages() before the handshake was complete, but messages could still be sent as a response to an incoming message. For example, if a peer had not yet sent a verack, we wouldn't notify it about new blocks, but we would respond to a PING with a PONG. This change makes the behavior straightforward: until we've received a verack, never send any message other than version/verack/reject. The behavior until a VERACK is received has always been undefined, this change just tightens our policy. This also makes testing much easier, because we can now connect but not send version/verack, and anything sent to us is an error.
furszy
force-pushed
the
2021_net_prevent_prior_verack_messages
branch
from
872fbf0
to
41b9b2f
Compare
furszy
changed the title
Straight update to btc@fa4dfd21
furszy
force-pushed
the
2021_net_prevent_prior_verack_messages
branch
from
41b9b2f
to
2f5339c
Compare
furszy
changed the title
|
rebased on master after 2587 merge. Ready. |
Fuzzbawls
approved these changes
Dec 17, 2021
furszy
added a commit
that referenced
this pull request
Jan 19, 2022
…rk layer ac014fb RPC: document getpeerinfo quorum members values (furszy) b39f275 test: add coverage for regular peers disconnecting after receiving an authenticated connection. (furszy) 758e19f p2p: do not send MN connection flag and challenge in the version msg if the connection is not meant to be a DMN-to-DMN. (furszy) cb34b51 Define constants for MN metadata manager filename and magic string. (furszy) b1a9637 Speed up functional tests decreasing the regtest tierTwo chain sync check up to the minimum. (furszy) 693bff6 mn_net: store local DMN protxhash instead of access the activeMnManager object. (furszy) 6015d90 mnsync: decouple IsBlockchainSync into a publicly available g_tiertwo_sync_state class which only stores the final value. (furszy) 038191c net_mn: extract cs_main/Misbehaving from mnauth::ProcessMessage to net_processing. (furszy) a38fdf3 add -pushversion help message (furszy) de436d3 net_processing: add missing return to the process verack msg flow. (furszy) 747f838 net_processing: try to update iqr member field if "qsendrecsigs" is received. (furszy) a2b478f net_mn: don't try to connect to ourselves. (furszy) 6b4d616 Node stats: add m_masternode_iqr_connection and m_masternode_probe_connection. (furszy) 13440cc net_mn: use string connection instead of addr to bypass the single IP conn requirement. (furszy) 72ef11d Test: add MN-to-MN, MN-to-quorum, MN-to-relay_members and probe MN connection test coverage (furszy) 80d7d6b bump protocol version to 70925 (furszy) acefaa8 RPC: add verified DMN connection info to 'getpeerinfo' (furszy) 3897ae5 net_mn: cache MN quorum relay members + let them know that we are interested in plain LLMQ recovered signatures. (furszy) 1c16a2f protocol: introduce QSENDRECSIGS net message. (furszy) 01ba363 net_mn: decouple g_args from the MN networking sources. (furszy) 934dd8c net_mn: add single MN connection process. (furszy) 15d70e7 net_mn: connect MN probing connections functionality. (furszy) b5074b4 net_mn: add MN probe connection workflow. (furszy) a0ef04e rpc: use snake_case for 'protx_list' verbose response json keys. (furszy) 4aa7bfd rpc: add MN meta info to the 'protx_list' verbose response. (furszy) 12bd6a3 net_mn: add mn-only connections cleanup process (furszy) 026ec74 net: if we are a MN, don't accept incoming connections until the node is fully synced. (furszy) e7e44f1 net_mn: protect verified MN connections from eviction + don't try to connect to already connected MNs. (furszy) cfa0238 net: introduce and connect MN authentication workflow. (furszy) e7c8892 net_mn: implement and connect new tier two connections manager. (furszy) 26a3cf5 net: introduce masternode-only connections. (furszy) 711b874 Introduce masternode net metadata manager (furszy) c724137 Introduce generic flatdb implementation (furszy) Pull request description: Built on top of #2607, #2632 and #2639. Focused on implementing the Masternode quorums network communication layer. This layer is in charge of: 1) Open MN-to-MN, MN-to-Quorum, MN-to-MN_iqr (intra-quorum relay) and MN-to-MN-probe connections. 2) Implement a challenge-based connection authentication for MNs (new network message `mnauth` introduced). 3) Cache and store MN connections metadata such as the last successful connection time and the last connection attempt time. 4) Protect verified MN connections from eviction while they are part of the active quorum. 5) Send interest in plain LLMQ recovered sigs (`QSENDRECSIGS` net message). Plus, added a new functional test `p2p_quorum_connect.py` which basically verifies the proper functioning of most of this points (except the eviction process). And finally, bumped the protocol version to 70925 so peers can know to whom they can, or not, send the new p2p messages. ACKs for top commit: random-zebra: utACK ac014fb Fuzzbawls: ACK ac014fb Tree-SHA512: 16af1e43ee8f225968accbfa9c628eead9e61f7c9ff656b205f3c936ccc6e4df205940ff756c807ad507942605bb9ff0c5d97b458af798b12311b6bfd9f49c1a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
More updates, corrections and test coverage to the network layer for the LLMQ MNs connections work (deep rabbit hole..). Built on top of #2587.
PR starts in cd00e31, focused on:
verackbefore responding to anything else (cbfc5a6).p2p_leak.pyfunctional test (check commit, the functional test framework was too ahead of it).SENDADDRV2msg processing so it can be processed before a verack.