diff options
| -rw-r--r-- | src/init.cpp | 2 | ||||
| -rw-r--r-- | src/net_processing.cpp | 54 | ||||
| -rw-r--r-- | src/node/txreconciliation.cpp | 53 | ||||
| -rw-r--r-- | src/node/txreconciliation.h | 13 | ||||
| -rw-r--r-- | src/protocol.h | 4 | ||||
| -rw-r--r-- | src/test/txreconciliation_tests.cpp | 46 | ||||
| -rwxr-xr-x | test/functional/p2p_sendtxrcncl.py | 151 | ||||
| -rwxr-xr-x | test/functional/test_framework/messages.py | 12 |
8 files changed, 169 insertions, 166 deletions
diff --git a/src/init.cpp b/src/init.cpp index 84b4a81e28..2fdc2717c9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -477,6 +477,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-onlynet=<net>", "Make automatic outbound connections only to network <net> (" + Join(GetNetworkNames(), ", ") + "). Inbound and manual connections are not affected by this option. It can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); // TODO: remove the sentence "Nodes not using ... incoming connections." once the changes from // https://github.com/bitcoin/bitcoin/pull/23542 have become widespread. argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port>. Nodes not using the default ports (default: %u, testnet: %u, signet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); @@ -485,7 +486,6 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-seednode=<ip>", "Connect to a node to retrieve peer addresses, and disconnect. This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-networkactive", "Enable all P2P network activity (default: 1). Can be changed by the setnetworkactive RPC command", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-timeout=<n>", strprintf("Specify socket connection timeout in milliseconds. If an initial attempt to connect is unsuccessful after this amount of time, drop it (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-peertimeout=<n>", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 346600efd0..d961ac90b8 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3273,17 +3273,14 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (greatest_common_version >= WTXID_RELAY_VERSION && m_txreconciliation) { // Per BIP-330, we announce txreconciliation support if: - // - protocol version per the VERSION message supports WTXID_RELAY; - // - we intended to exchange transactions over this connection while establishing it - // and the peer indicated support for transaction relay in the VERSION message; + // - protocol version per the peer's VERSION message supports WTXID_RELAY; + // - transaction relay is supported per the peer's VERSION message (see m_relays_txs); + // - this is not a block-relay-only connection and not a feeler (see m_relays_txs); + // - this is not an addr fetch connection; // - we are not in -blocksonly mode. - if (pfrom.m_relays_txs && !m_ignore_incoming_txs) { + if (pfrom.m_relays_txs && !pfrom.IsAddrFetchConn() && !m_ignore_incoming_txs) { const uint64_t recon_salt = m_txreconciliation->PreRegisterPeer(pfrom.GetId()); - // We suggest our txreconciliation role (initiator/responder) based on - // the connection direction. m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDTXRCNCL, - !pfrom.IsInboundConn(), - pfrom.IsInboundConn(), TXRECONCILIATION_VERSION, recon_salt)); } } @@ -3500,41 +3497,44 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (pfrom.fSuccessfullyConnected) { - // Disconnect peers that send a SENDTXRCNCL message after VERACK. LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received after verack from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; } - if (!peer->GetTxRelay()) { - // Disconnect peers that send a SENDTXRCNCL message even though we indicated we don't - // support transaction relay. + // Peer must not offer us reconciliations if we specified no tx relay support in VERSION. + if (RejectIncomingTxs(pfrom)) { LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received from peer=%d to which we indicated no tx relay; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; } - bool is_peer_initiator, is_peer_responder; + // Peer must not offer us reconciliations if they specified no tx relay support in VERSION. + // This flag might also be false in other cases, but the RejectIncomingTxs check above + // eliminates them, so that this flag fully represents what we are looking for. + if (!pfrom.m_relays_txs) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl received from peer=%d which indicated no tx relay to us; disconnecting\n", pfrom.GetId()); + pfrom.fDisconnect = true; + return; + } + uint32_t peer_txreconcl_version; uint64_t remote_salt; - vRecv >> is_peer_initiator >> is_peer_responder >> peer_txreconcl_version >> remote_salt; + vRecv >> peer_txreconcl_version >> remote_salt; - if (m_txreconciliation->IsPeerRegistered(pfrom.GetId())) { - // A peer is already registered, meaning we already received SENDTXRCNCL from them. + const ReconciliationRegisterResult result = m_txreconciliation->RegisterPeer(pfrom.GetId(), pfrom.IsInboundConn(), + peer_txreconcl_version, remote_salt); + switch (result) { + case ReconciliationRegisterResult::NOT_FOUND: + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "Ignore unexpected txreconciliation signal from peer=%d\n", pfrom.GetId()); + break; + case ReconciliationRegisterResult::SUCCESS: + break; + case ReconciliationRegisterResult::ALREADY_REGISTERED: LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d (sendtxrcncl received from already registered peer); disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; - } - - const ReconciliationRegisterResult result = m_txreconciliation->RegisterPeer(pfrom.GetId(), pfrom.IsInboundConn(), - is_peer_initiator, is_peer_responder, - peer_txreconcl_version, - remote_salt); - - // If it's a protocol violation, disconnect. - // If the peer was not found (but something unexpected happened) or it was registered, - // nothing to be done. - if (result == ReconciliationRegisterResult::PROTOCOL_VIOLATION) { + case ReconciliationRegisterResult::PROTOCOL_VIOLATION: LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d; disconnecting\n", pfrom.GetId()); pfrom.fDisconnect = true; return; diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp index 974358fcda..ed04a78cec 100644 --- a/src/node/txreconciliation.cpp +++ b/src/node/txreconciliation.cpp @@ -39,7 +39,8 @@ public: * the following commits. * * Reconciliation protocol assumes using one role consistently: either a reconciliation - * initiator (requesting sketches), or responder (sending sketches). This defines our role. + * initiator (requesting sketches), or responder (sending sketches). This defines our role, + * based on the direction of the p2p connection. * */ bool m_we_initiate; @@ -81,31 +82,30 @@ public: { AssertLockNotHeld(m_txreconciliation_mutex); LOCK(m_txreconciliation_mutex); - // We do not support txreconciliation salt/version updates. - assert(m_states.find(peer_id) == m_states.end()); LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Pre-register peer=%d\n", peer_id); const uint64_t local_salt{GetRand(UINT64_MAX)}; // We do this exactly once per peer (which are unique by NodeId, see GetNewNodeId) so it's // safe to assume we don't have this record yet. - Assert(m_states.emplace(peer_id, local_salt).second); + Assume(m_states.emplace(peer_id, local_salt).second); return local_salt; } - ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, bool is_peer_recon_initiator, - bool is_peer_recon_responder, uint32_t peer_recon_version, - uint64_t remote_salt) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) + ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, uint32_t peer_recon_version, + uint64_t remote_salt) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) { AssertLockNotHeld(m_txreconciliation_mutex); LOCK(m_txreconciliation_mutex); auto recon_state = m_states.find(peer_id); - // A peer should be in the pre-registered state to proceed here. - if (recon_state == m_states.end()) return NOT_FOUND; - uint64_t* local_salt = std::get_if<uint64_t>(&recon_state->second); - // A peer is already registered. This should be checked by the caller. - Assume(local_salt); + if (recon_state == m_states.end()) return ReconciliationRegisterResult::NOT_FOUND; + + if (std::holds_alternative<TxReconciliationState>(recon_state->second)) { + return ReconciliationRegisterResult::ALREADY_REGISTERED; + } + + uint64_t local_salt = *std::get_if<uint64_t>(&recon_state->second); // If the peer supports the version which is lower than ours, we downgrade to the version // it supports. For now, this only guarantees that nodes with future reconciliation @@ -114,27 +114,14 @@ public: // satisfactory (e.g. too low). const uint32_t recon_version{std::min(peer_recon_version, m_recon_version)}; // v1 is the lowest version, so suggesting something below must be a protocol violation. - if (recon_version < 1) return PROTOCOL_VIOLATION; - - // Must match SENDTXRCNCL logic. - const bool they_initiate = is_peer_recon_initiator && is_peer_inbound; - const bool we_initiate = !is_peer_inbound && is_peer_recon_responder; - - // If we ever announce support for both requesting and responding, this will need - // tie-breaking. For now, this is mutually exclusive because both are based on the - // inbound flag. - assert(!(they_initiate && we_initiate)); - - // The peer set both flags to false, we treat it as a protocol violation. - if (!(they_initiate || we_initiate)) return PROTOCOL_VIOLATION; + if (recon_version < 1) return ReconciliationRegisterResult::PROTOCOL_VIOLATION; - LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Register peer=%d with the following params: " /* Continued */ - "we_initiate=%i, they_initiate=%i.\n", - peer_id, we_initiate, they_initiate); + LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Register peer=%d (inbound=%i)\n", + peer_id, is_peer_inbound); - const uint256 full_salt{ComputeSalt(*local_salt, remote_salt)}; - recon_state->second = TxReconciliationState(we_initiate, full_salt.GetUint64(0), full_salt.GetUint64(1)); - return SUCCESS; + const uint256 full_salt{ComputeSalt(local_salt, remote_salt)}; + recon_state->second = TxReconciliationState(!is_peer_inbound, full_salt.GetUint64(0), full_salt.GetUint64(1)); + return ReconciliationRegisterResult::SUCCESS; } void ForgetPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) @@ -166,11 +153,9 @@ uint64_t TxReconciliationTracker::PreRegisterPeer(NodeId peer_id) } ReconciliationRegisterResult TxReconciliationTracker::RegisterPeer(NodeId peer_id, bool is_peer_inbound, - bool is_peer_recon_initiator, bool is_peer_recon_responder, uint32_t peer_recon_version, uint64_t remote_salt) { - return m_impl->RegisterPeer(peer_id, is_peer_inbound, is_peer_recon_initiator, is_peer_recon_responder, - peer_recon_version, remote_salt); + return m_impl->RegisterPeer(peer_id, is_peer_inbound, peer_recon_version, remote_salt); } void TxReconciliationTracker::ForgetPeer(NodeId peer_id) diff --git a/src/node/txreconciliation.h b/src/node/txreconciliation.h index a4f0870914..4591dd5df7 100644 --- a/src/node/txreconciliation.h +++ b/src/node/txreconciliation.h @@ -16,10 +16,11 @@ static constexpr bool DEFAULT_TXRECONCILIATION_ENABLE{false}; /** Supported transaction reconciliation protocol version */ static constexpr uint32_t TXRECONCILIATION_VERSION{1}; -enum ReconciliationRegisterResult { - NOT_FOUND = 0, - SUCCESS = 1, - PROTOCOL_VIOLATION = 2, +enum class ReconciliationRegisterResult { + NOT_FOUND, + SUCCESS, + ALREADY_REGISTERED, + PROTOCOL_VIOLATION, }; /** @@ -72,8 +73,8 @@ public: * Step 0. Once the peer agreed to reconcile txs with us, generate the state required to track * ongoing reconciliations. Must be called only after pre-registering the peer and only once. */ - ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, bool is_peer_recon_initiator, - bool is_peer_recon_responder, uint32_t peer_recon_version, uint64_t remote_salt); + ReconciliationRegisterResult RegisterPeer(NodeId peer_id, bool is_peer_inbound, + uint32_t peer_recon_version, uint64_t remote_salt); /** * Attempts to forget txreconciliation-related state of the peer (if we previously stored any). diff --git a/src/protocol.h b/src/protocol.h index 17a363b1d3..51fabf8da0 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -259,9 +259,7 @@ extern const char* CFCHECKPT; */ extern const char* WTXIDRELAY; /** - * Contains 2 1-byte bools, a 4-byte version number and an 8-byte salt. - * The 2 booleans indicate that a node is willing to participate in transaction - * reconciliation, respectively as an initiator or as a receiver. + * Contains a 4-byte version number and an 8-byte salt. * The salt is used to compute short txids needed for efficient * txreconciliation, as described by BIP 330. */ diff --git a/src/test/txreconciliation_tests.cpp b/src/test/txreconciliation_tests.cpp index bd74998002..b018629e76 100644 --- a/src/test/txreconciliation_tests.cpp +++ b/src/test/txreconciliation_tests.cpp @@ -12,56 +12,54 @@ BOOST_FIXTURE_TEST_SUITE(txreconciliation_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(RegisterPeerTest) { - TxReconciliationTracker tracker(1); + TxReconciliationTracker tracker(TXRECONCILIATION_VERSION); const uint64_t salt = 0; // Prepare a peer for reconciliation. tracker.PreRegisterPeer(0); - // Both roles are false, don't register. - BOOST_CHECK(tracker.RegisterPeer(/*peer_id=*/0, /*is_peer_inbound=*/true, - /*is_peer_recon_initiator=*/false, - /*is_peer_recon_responder=*/false, - /*peer_recon_version=*/1, salt) == - ReconciliationRegisterResult::PROTOCOL_VIOLATION); - - // Invalid roles for the given connection direction. - BOOST_CHECK(tracker.RegisterPeer(0, true, false, true, 1, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION); - BOOST_CHECK(tracker.RegisterPeer(0, false, true, false, 1, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION); - // Invalid version. - BOOST_CHECK(tracker.RegisterPeer(0, true, true, false, 0, salt) == ReconciliationRegisterResult::PROTOCOL_VIOLATION); + BOOST_CHECK_EQUAL(tracker.RegisterPeer(/*peer_id=*/0, /*is_peer_inbound=*/true, + /*peer_recon_version=*/0, salt), + ReconciliationRegisterResult::PROTOCOL_VIOLATION); - // Valid registration. + // Valid registration (inbound and outbound peers). BOOST_REQUIRE(!tracker.IsPeerRegistered(0)); - BOOST_REQUIRE(tracker.RegisterPeer(0, true, true, false, 1, salt) == ReconciliationRegisterResult::SUCCESS); + BOOST_REQUIRE_EQUAL(tracker.RegisterPeer(0, true, 1, salt), ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(0)); - - // Reconciliation version is higher than ours, should be able to register. BOOST_REQUIRE(!tracker.IsPeerRegistered(1)); tracker.PreRegisterPeer(1); - BOOST_REQUIRE(tracker.RegisterPeer(1, true, true, false, 2, salt) == ReconciliationRegisterResult::SUCCESS); + BOOST_REQUIRE(tracker.RegisterPeer(1, false, 1, salt) == ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(1)); + // Reconciliation version is higher than ours, should be able to register. + BOOST_REQUIRE(!tracker.IsPeerRegistered(2)); + tracker.PreRegisterPeer(2); + BOOST_REQUIRE(tracker.RegisterPeer(2, true, 2, salt) == ReconciliationRegisterResult::SUCCESS); + BOOST_CHECK(tracker.IsPeerRegistered(2)); + + // Try registering for the second time. + BOOST_REQUIRE(tracker.RegisterPeer(1, false, 1, salt) == ReconciliationRegisterResult::ALREADY_REGISTERED); + // Do not register if there were no pre-registration for the peer. - BOOST_REQUIRE(tracker.RegisterPeer(100, true, true, false, 1, salt) == ReconciliationRegisterResult::NOT_FOUND); + BOOST_REQUIRE_EQUAL(tracker.RegisterPeer(100, true, 1, salt), ReconciliationRegisterResult::NOT_FOUND); BOOST_CHECK(!tracker.IsPeerRegistered(100)); } BOOST_AUTO_TEST_CASE(ForgetPeerTest) { - TxReconciliationTracker tracker(1); + TxReconciliationTracker tracker(TXRECONCILIATION_VERSION); NodeId peer_id0 = 0; // Removing peer after pre-registring works and does not let to register the peer. tracker.PreRegisterPeer(peer_id0); tracker.ForgetPeer(peer_id0); - BOOST_CHECK(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::NOT_FOUND); + BOOST_CHECK_EQUAL(tracker.RegisterPeer(peer_id0, true, 1, 1), ReconciliationRegisterResult::NOT_FOUND); // Removing peer after it is registered works. tracker.PreRegisterPeer(peer_id0); BOOST_REQUIRE(!tracker.IsPeerRegistered(peer_id0)); - BOOST_REQUIRE(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::SUCCESS); + BOOST_REQUIRE_EQUAL(tracker.RegisterPeer(peer_id0, true, 1, 1), ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(peer_id0)); tracker.ForgetPeer(peer_id0); BOOST_CHECK(!tracker.IsPeerRegistered(peer_id0)); @@ -69,14 +67,14 @@ BOOST_AUTO_TEST_CASE(ForgetPeerTest) BOOST_AUTO_TEST_CASE(IsPeerRegisteredTest) { - TxReconciliationTracker tracker(1); + TxReconciliationTracker tracker(TXRECONCILIATION_VERSION); NodeId peer_id0 = 0; BOOST_REQUIRE(!tracker.IsPeerRegistered(peer_id0)); tracker.PreRegisterPeer(peer_id0); BOOST_REQUIRE(!tracker.IsPeerRegistered(peer_id0)); - BOOST_REQUIRE(tracker.RegisterPeer(peer_id0, true, true, false, 1, 1) == ReconciliationRegisterResult::SUCCESS); + BOOST_REQUIRE_EQUAL(tracker.RegisterPeer(peer_id0, true, 1, 1), ReconciliationRegisterResult::SUCCESS); BOOST_CHECK(tracker.IsPeerRegistered(peer_id0)); tracker.ForgetPeer(peer_id0); diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index fed9832a7d..0e349ef70c 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -10,6 +10,7 @@ from test_framework.messages import ( msg_verack, msg_version, msg_wtxidrelay, + NODE_BLOOM, ) from test_framework.p2p import ( P2PInterface, @@ -54,10 +55,8 @@ class PeerTrackMsgOrder(P2PInterface): super().on_message(message) self.messages.append(message) -def create_sendtxrcncl_msg(initiator=True): +def create_sendtxrcncl_msg(): sendtxrcncl_msg = msg_sendtxrcncl() - sendtxrcncl_msg.initiator = initiator - sendtxrcncl_msg.responder = not initiator sendtxrcncl_msg.version = 1 sendtxrcncl_msg.salt = 2 return sendtxrcncl_msg @@ -68,11 +67,11 @@ class SendTxRcnclTest(BitcoinTestFramework): self.extra_args = [['-txreconciliation']] def run_test(self): + # Check everything concerning *sending* SENDTXRCNCL + # First, *sending* to *inbound*. self.log.info('SENDTXRCNCL sent to an inbound') peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) assert peer.sendtxrcncl_msg_received - assert not peer.sendtxrcncl_msg_received.initiator - assert peer.sendtxrcncl_msg_received.responder assert_equal(peer.sendtxrcncl_msg_received.version, 1) self.nodes[0].disconnect_p2ps() @@ -81,7 +80,7 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.wait_for_verack() verack_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'verack'][0] sendtxrcncl_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'sendtxrcncl'][0] - assert(sendtxrcncl_index < verack_index) + assert sendtxrcncl_index < verack_index self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL on pre-WTXID version should not be sent') @@ -108,30 +107,77 @@ class SendTxRcnclTest(BitcoinTestFramework): assert not peer.sendtxrcncl_msg_received self.nodes[0].disconnect_p2ps() + self.log.info('SENDTXRCNCL for fRelay=false should not be sent (with NODE_BLOOM offered)') + self.restart_node(0, ["-peerbloomfilters", "-txreconciliation"]) + peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=False, wait_for_verack=False) + no_txrelay_version_msg = msg_version() + no_txrelay_version_msg.nVersion = P2P_VERSION + no_txrelay_version_msg.strSubVer = P2P_SUBVERSION + no_txrelay_version_msg.nServices = P2P_SERVICES + no_txrelay_version_msg.relay = 0 + peer.send_message(no_txrelay_version_msg) + peer.wait_for_verack() + assert peer.nServices & NODE_BLOOM != 0 + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + # Now, *sending* to *outbound*. + self.log.info('SENDTXRCNCL sent to an outbound') + peer = self.nodes[0].add_outbound_p2p_connection( + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="outbound-full-relay") + assert peer.sendtxrcncl_msg_received + assert_equal(peer.sendtxrcncl_msg_received.version, 1) + self.nodes[0].disconnect_p2ps() + + self.log.info('SENDTXRCNCL should not be sent if block-relay-only') + peer = self.nodes[0].add_outbound_p2p_connection( + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="block-relay-only") + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + self.log.info("SENDTXRCNCL should not be sent if feeler") + peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=0, connection_type="feeler") + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + self.log.info("SENDTXRCNCL should not be sent if addrfetch") + peer = self.nodes[0].add_outbound_p2p_connection( + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="addr-fetch") + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + self.log.info('SENDTXRCNCL not sent if -txreconciliation flag is not set') + self.restart_node(0, []) + peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + self.log.info('SENDTXRCNCL not sent if blocksonly is set') + self.restart_node(0, ["-txreconciliation", "-blocksonly"]) + peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) + assert not peer.sendtxrcncl_msg_received + self.nodes[0].disconnect_p2ps() + + # Check everything concerning *receiving* SENDTXRCNCL + # First, receiving from *inbound*. + self.restart_node(0, ["-txreconciliation"]) self.log.info('valid SENDTXRCNCL received') peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) - peer.send_message(create_sendtxrcncl_msg()) - self.wait_until(lambda : "sendtxrcncl" in self.nodes[0].getpeerinfo()[-1]["bytesrecv_per_msg"]) + with self.nodes[0].assert_debug_log(["received: sendtxrcncl"]): + peer.send_message(create_sendtxrcncl_msg()) self.log.info('second SENDTXRCNCL triggers a disconnect') with self.nodes[0].assert_debug_log(["(sendtxrcncl received from already registered peer); disconnecting"]): peer.send_message(create_sendtxrcncl_msg()) peer.wait_for_disconnect() - self.log.info('SENDTXRCNCL with initiator=responder=0 triggers a disconnect') - sendtxrcncl_no_role = create_sendtxrcncl_msg() - sendtxrcncl_no_role.initiator = False - sendtxrcncl_no_role.responder = False + self.restart_node(0, []) + self.log.info('SENDTXRCNCL if no txreconciliation supported is ignored') peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) - with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]): - peer.send_message(sendtxrcncl_no_role) - peer.wait_for_disconnect() + with self.nodes[0].assert_debug_log(['ignored, as our node does not have txreconciliation enabled']): + peer.send_message(create_sendtxrcncl_msg()) + self.nodes[0].disconnect_p2ps() - self.log.info('SENDTXRCNCL with initiator=0 and responder=1 from inbound triggers a disconnect') - sendtxrcncl_wrong_role = create_sendtxrcncl_msg(initiator=False) - peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) - with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]): - peer.send_message(sendtxrcncl_wrong_role) - peer.wait_for_disconnect() + self.restart_node(0, ["-txreconciliation"]) self.log.info('SENDTXRCNCL with version=0 triggers a disconnect') sendtxrcncl_low_version = create_sendtxrcncl_msg() @@ -141,6 +187,26 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.send_message(sendtxrcncl_low_version) peer.wait_for_disconnect() + self.log.info('SENDTXRCNCL with version=2 is valid') + sendtxrcncl_higher_version = create_sendtxrcncl_msg() + sendtxrcncl_higher_version.version = 2 + peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) + with self.nodes[0].assert_debug_log(['Register peer=1']): + peer.send_message(sendtxrcncl_higher_version) + self.nodes[0].disconnect_p2ps() + + self.log.info('unexpected SENDTXRCNCL is ignored') + peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=False, wait_for_verack=False) + old_version_msg = msg_version() + old_version_msg.nVersion = 70015 + old_version_msg.strSubVer = P2P_SUBVERSION + old_version_msg.nServices = P2P_SERVICES + old_version_msg.relay = 1 + peer.send_message(old_version_msg) + with self.nodes[0].assert_debug_log(['Ignore unexpected txreconciliation signal']): + peer.send_message(create_sendtxrcncl_msg()) + self.nodes[0].disconnect_p2ps() + self.log.info('sending SENDTXRCNCL after sending VERACK triggers a disconnect') peer = self.nodes[0].add_p2p_connection(P2PInterface()) with self.nodes[0].assert_debug_log(["sendtxrcncl received after verack"]): @@ -154,53 +220,14 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.send_message(msg_verack()) self.nodes[0].disconnect_p2ps() - self.log.info('SENDTXRCNCL sent to an outbound') - peer = self.nodes[0].add_outbound_p2p_connection( - SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="outbound-full-relay") - assert peer.sendtxrcncl_msg_received - assert peer.sendtxrcncl_msg_received.initiator - assert not peer.sendtxrcncl_msg_received.responder - assert_equal(peer.sendtxrcncl_msg_received.version, 1) - self.nodes[0].disconnect_p2ps() - - self.log.info('SENDTXRCNCL should not be sent if block-relay-only') - peer = self.nodes[0].add_outbound_p2p_connection( - SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="block-relay-only") - assert not peer.sendtxrcncl_msg_received - self.nodes[0].disconnect_p2ps() - - self.log.info("SENDTXRCNCL should not be sent if feeler") - peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=0, connection_type="feeler") - assert not peer.sendtxrcncl_msg_received - self.nodes[0].disconnect_p2ps() - + # Now, *receiving* from *outbound*. 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=0, connection_type="block-relay-only") with self.nodes[0].assert_debug_log(["we indicated no tx relay; disconnecting"]): - peer.send_message(create_sendtxrcncl_msg(initiator=False)) - peer.wait_for_disconnect() - - self.log.info('SENDTXRCNCL with initiator=1 and responder=0 from outbound triggers a disconnect') - sendtxrcncl_wrong_role = create_sendtxrcncl_msg(initiator=True) - peer = self.nodes[0].add_outbound_p2p_connection( - PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="outbound-full-relay") - with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]): - peer.send_message(sendtxrcncl_wrong_role) + peer.send_message(create_sendtxrcncl_msg()) peer.wait_for_disconnect() - self.log.info('SENDTXRCNCL not sent if -txreconciliation flag is not set') - self.restart_node(0, []) - peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) - assert not peer.sendtxrcncl_msg_received - self.nodes[0].disconnect_p2ps() - - self.log.info('SENDTXRCNCL not sent if blocksonly is set') - self.restart_node(0, ["-txreconciliation", "-blocksonly"]) - peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True) - assert not peer.sendtxrcncl_msg_received - self.nodes[0].disconnect_p2ps() - if __name__ == '__main__': SendTxRcnclTest().main() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 252b49cc6d..7389b1624e 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1840,29 +1840,23 @@ class msg_cfcheckpt: self.filter_type, self.stop_hash) class msg_sendtxrcncl: - __slots__ = ("initiator", "responder", "version", "salt") + __slots__ = ("version", "salt") msgtype = b"sendtxrcncl" def __init__(self): - self.initiator = False - self.responder = False self.version = 0 self.salt = 0 def deserialize(self, f): - self.initiator = struct.unpack("<?", f.read(1))[0] - self.responder = struct.unpack("<?", f.read(1))[0] self.version = struct.unpack("<I", f.read(4))[0] self.salt = struct.unpack("<Q", f.read(8))[0] def serialize(self): r = b"" - r += struct.pack("<?", self.initiator) - r += struct.pack("<?", self.responder) r += struct.pack("<I", self.version) r += struct.pack("<Q", self.salt) return r def __repr__(self): - return "msg_sendtxrcncl(initiator=%i, responder=%i, version=%lu, salt=%lu)" %\ - (self.initiator, self.responder, self.version, self.salt) + return "msg_sendtxrcncl(version=%lu, salt=%lu)" %\ + (self.version, self.salt) |