diff options
author | MacroFake <falke.marco@gmail.com> | 2022-11-02 08:06:57 +0100 |
---|---|---|
committer | MacroFake <falke.marco@gmail.com> | 2022-11-02 08:07:28 +0100 |
commit | 39f026b1ec17ef8100457ef46a1e4980767c0fe2 (patch) | |
tree | 2fb0b02e4e005576dc70243885749d516ed77aec | |
parent | bf0cb4399061c6827bd5d7428a146010102d7ab1 (diff) | |
parent | fa24239a1c2281f61ab70a62228e88f4c7e72701 (diff) | |
download | bitcoin-39f026b1ec17ef8100457ef46a1e4980767c0fe2.tar.xz |
Merge bitcoin/bitcoin#26396: net: Avoid SetTxRelay for feeler connections
fa24239a1c2281f61ab70a62228e88f4c7e72701 net: Avoid SetTxRelay for feeler connections (MacroFake)
Pull request description:
Seems odd to reserve memory for the struct (the heaviest member being `m_tx_inventory_known_filter`) when it is never used.
This also avoids sending out `msg_sendtxrcncl` before disconnecting. This shouldn't matter, as other messages, such as `msg_wtxidrelay`, `msg_sendaddrv2`, `msg_verack` or `msg_getaddr` are still sent. Though, it allows to test the changes here as a side-effect.
ACKs for top commit:
naumenkogs:
ACK fa24239a1c2281f61ab70a62228e88f4c7e72701
vasild:
ACK fa24239a1c2281f61ab70a62228e88f4c7e72701
jonatack:
ACK fa24239a1c2281f61ab70a62228e88f4c7e72701
mzumsande:
ACK fa24239a1c2281f61ab70a62228e88f4c7e72701
Tree-SHA512: d7604c7eb4df8f2de811e600bdd312440ee03e508d3a0f09ae79f7f2d3eeec663bfd47a2d079fa50b756d61e35dfa998de068a7b9afaf35378fa0e62a538263d
-rw-r--r-- | src/net.h | 6 | ||||
-rw-r--r-- | src/net_processing.cpp | 8 | ||||
-rwxr-xr-x | test/functional/p2p_sendtxrcncl.py | 11 |
3 files changed, 17 insertions, 8 deletions
@@ -489,10 +489,8 @@ public: /** Whether this peer provides all services that we want. Used for eviction decisions */ std::atomic_bool m_has_all_wanted_services{false}; - /** Whether we should relay transactions to this peer (their version - * message did not include fRelay=false and this is not a block-relay-only - * connection). This only changes from false to true. It will never change - * back to false. Used only in inbound eviction logic. */ + /** Whether we should relay transactions to this peer. This only changes + * from false to true. It will never change back to false. */ std::atomic_bool m_relays_txs{false}; /** Whether this peer has loaded a bloom filter. Used only in inbound diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6c310b3fcd..6aaacd5068 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -395,9 +395,7 @@ struct Peer { private: Mutex m_tx_relay_mutex; - /** Transaction relay data. Will be a nullptr if we're not relaying - * transactions with this peer (e.g. if it's a block-relay-only peer or - * the peer has sent us fRelay=false with bloom filters disabled). */ + /** Transaction relay data. May be a nullptr. */ std::unique_ptr<TxRelay> m_tx_relay GUARDED_BY(m_tx_relay_mutex); }; @@ -3256,12 +3254,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } peer->m_starting_height = starting_height; - // We only initialize the Peer::TxRelay m_relay_txs data structure if: + // Only initialize the Peer::TxRelay m_relay_txs data structure if: // - this isn't an outbound block-relay-only connection, and + // - this isn't an outbound feeler connection, and // - fRelay=true (the peer wishes to receive transaction announcements) // or we're offering NODE_BLOOM to this peer. NODE_BLOOM means that // the peer may turn on transaction relay later. if (!pfrom.IsBlockOnlyConn() && + !pfrom.IsFeelerConn() && (fRelay || (peer->m_our_services & NODE_BLOOM))) { auto* const tx_relay = peer->SetTxRelay(); { diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index 14cd815a30..50a5127f25 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -39,6 +39,12 @@ class SendTxrcnclReceiver(P2PInterface): def on_sendtxrcncl(self, message): self.sendtxrcncl_msg_received = message + +class P2PFeelerReceiver(SendTxrcnclReceiver): + def on_version(self, message): + pass # feeler connections can not send any message other than their own version + + class PeerTrackMsgOrder(P2PInterface): def __init__(self): super().__init__() @@ -163,6 +169,11 @@ class SendTxRcnclTest(BitcoinTestFramework): assert not peer.sendtxrcncl_msg_received peer.peer_disconnect() + self.log.info("SENDTXRCNCL should not be sent if feeler") + peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=2, connection_type="feeler") + assert not peer.sendtxrcncl_msg_received + peer.peer_disconnect() + self.log.info('SENDTXRCNCL if block-relay-only triggers a disconnect') peer = self.nodes[0].add_outbound_p2p_connection( PeerNoVerack(), wait_for_verack=False, p2p_idx=3, connection_type="block-relay-only") |