From 24e36fac0a86aa371046470dc4f8dfed7a868fb2 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Mon, 19 Sep 2022 13:07:31 +0300 Subject: [PATCH 1/9] log: Add tx reconciliation log category --- src/logging.cpp | 3 +++ src/logging.h | 1 + 2 files changed, 4 insertions(+) diff --git a/src/logging.cpp b/src/logging.cpp index a3f1d39be5a..c95c0b7e377 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -180,6 +180,7 @@ const CLogCategoryDesc LogCategories[] = #endif {BCLog::UTIL, "util"}, {BCLog::BLOCKSTORE, "blockstorage"}, + {BCLog::TXRECONCILIATION, "txreconciliation"}, {BCLog::ALL, "1"}, {BCLog::ALL, "all"}, }; @@ -280,6 +281,8 @@ std::string LogCategoryToStr(BCLog::LogFlags category) return "util"; case BCLog::LogFlags::BLOCKSTORE: return "blockstorage"; + case BCLog::LogFlags::TXRECONCILIATION: + return "txreconciliation"; case BCLog::LogFlags::ALL: return "all"; } diff --git a/src/logging.h b/src/logging.h index fe91ee43a58..5ee6665c763 100644 --- a/src/logging.h +++ b/src/logging.h @@ -66,6 +66,7 @@ namespace BCLog { #endif UTIL = (1 << 25), BLOCKSTORE = (1 << 26), + TXRECONCILIATION = (1 << 27), ALL = ~(uint32_t)0, }; enum class Level { From 3fcf78ee6ab59d1a539fdf54bbae21b5d29caea9 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Mon, 19 Sep 2022 17:31:01 +0300 Subject: [PATCH 2/9] p2p: Announce reconciliation support If we're connecting to the peer which might support transaction reconciliation, we announce we want to reconcile with them. We store the reconciliation salt so that when the peer responds with their salt, we are able to compute the full reconciliation salt. This behavior is enabled with a CLI flag. --- src/Makefile.am | 2 + src/init.cpp | 2 + src/net_processing.cpp | 28 ++++++++++++- src/node/txreconciliation.cpp | 66 +++++++++++++++++++++++++++++++ src/node/txreconciliation.h | 66 +++++++++++++++++++++++++++++++ src/protocol.cpp | 2 + src/protocol.h | 8 ++++ src/test/fuzz/process_message.cpp | 1 + 8 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 src/node/txreconciliation.cpp create mode 100644 src/node/txreconciliation.h diff --git a/src/Makefile.am b/src/Makefile.am index 88c62d51779..5347aa5ddef 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -209,6 +209,7 @@ BITCOIN_CORE_H = \ node/minisketchwrapper.h \ node/psbt.h \ node/transaction.h \ + node/txreconciliation.h \ node/utxo_snapshot.h \ node/validation_cache_args.h \ noui.h \ @@ -396,6 +397,7 @@ libbitcoin_node_a_SOURCES = \ node/minisketchwrapper.cpp \ node/psbt.cpp \ node/transaction.cpp \ + node/txreconciliation.cpp \ node/utxo_snapshot.cpp \ node/validation_cache_args.cpp \ noui.cpp \ diff --git a/src/init.cpp b/src/init.cpp index 8ffab646226..1b3162bf398 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -484,6 +485,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-seednode=", "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=", 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=", 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=:", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torpassword=", "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 75537a2d98b..a10b6bfc471 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -703,6 +704,7 @@ private: ChainstateManager& m_chainman; CTxMemPool& m_mempool; TxRequestTracker m_txrequest GUARDED_BY(::cs_main); + std::unique_ptr m_txreconciliation; /** The height of the best chain */ std::atomic m_best_height{-1}; @@ -1776,6 +1778,11 @@ PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman, m_mempool(pool), m_ignore_incoming_txs(ignore_incoming_txs) { + // While Erlay support is incomplete, it must be enabled explicitly via -txreconciliation. + // This argument can go away after Erlay support is complete. + if (gArgs.GetBoolArg("-txreconciliation", DEFAULT_TXRECONCILIATION_ENABLE)) { + m_txreconciliation = std::make_unique(); + } } void PeerManagerImpl::StartScheduledTasks(CScheduler& scheduler) @@ -3236,8 +3243,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2)); } - m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); - pfrom.m_has_all_wanted_services = HasAllDesirableServiceFlags(nServices); peer->m_their_services = nServices; pfrom.SetAddrLocal(addrMe); @@ -3262,6 +3267,25 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (fRelay) pfrom.m_relays_txs = true; } + 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; + // - we are not in -blocksonly mode. + if (pfrom.m_relays_txs && !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)); + } + } + + m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); + // Potentially mark this peer as a preferred download peer. { LOCK(cs_main); diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp new file mode 100644 index 00000000000..dfac2ad82ed --- /dev/null +++ b/src/node/txreconciliation.cpp @@ -0,0 +1,66 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include + +#include +#include + + +namespace { + +/** + * Keeps track of txreconciliation-related per-peer state. + */ +class TxReconciliationState +{ +}; + +} // namespace + +/** Actual implementation for TxReconciliationTracker's data structure. */ +class TxReconciliationTracker::Impl +{ +private: + mutable Mutex m_txreconciliation_mutex; + + /** + * Keeps track of txreconciliation states of eligible peers. + * For pre-registered peers, the locally generated salt is stored. + * For registered peers, the locally generated salt is forgotten, and the state (including + * "full" salt) is stored instead. + */ + std::unordered_map> m_states GUARDED_BY(m_txreconciliation_mutex); + +public: + explicit Impl() {} + + uint64_t PreRegisterPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) + { + 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); + return local_salt; + } +}; + +TxReconciliationTracker::TxReconciliationTracker() : m_impl{std::make_unique()} {} + +TxReconciliationTracker::~TxReconciliationTracker() = default; + +uint64_t TxReconciliationTracker::PreRegisterPeer(NodeId peer_id) +{ + return m_impl->PreRegisterPeer(peer_id); +} diff --git a/src/node/txreconciliation.h b/src/node/txreconciliation.h new file mode 100644 index 00000000000..6e94a51cf24 --- /dev/null +++ b/src/node/txreconciliation.h @@ -0,0 +1,66 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_TXRECONCILIATION_H +#define BITCOIN_NODE_TXRECONCILIATION_H + +#include +#include + +#include +#include + +/** Whether transaction reconciliation protocol should be enabled by default. */ +static constexpr bool DEFAULT_TXRECONCILIATION_ENABLE{false}; +/** Supported transaction reconciliation protocol version */ +static constexpr uint32_t TXRECONCILIATION_VERSION{1}; + +/** + * Transaction reconciliation is a way for nodes to efficiently announce transactions. + * This object keeps track of all txreconciliation-related communications with the peers. + * The high-level protocol is: + * 0. Txreconciliation protocol handshake. + * 1. Once we receive a new transaction, add it to the set instead of announcing immediately. + * 2. At regular intervals, a txreconciliation initiator requests a sketch from a peer, where a + * sketch is a compressed representation of short form IDs of the transactions in their set. + * 3. Once the initiator received a sketch from the peer, the initiator computes a local sketch, + * and combines the two sketches to attempt finding the difference in *sets*. + * 4a. If the difference was not larger than estimated, see SUCCESS below. + * 4b. If the difference was larger than estimated, initial txreconciliation fails. The initiator + * requests a larger sketch via an extension round (allowed only once). + * - If extension succeeds (a larger sketch is sufficient), see SUCCESS below. + * - If extension fails (a larger sketch is insufficient), see FAILURE below. + * + * SUCCESS. The initiator knows full symmetrical difference and can request what the initiator is + * missing and announce to the peer what the peer is missing. + * + * FAILURE. The initiator notifies the peer about the failure and announces all transactions from + * the corresponding set. Once the peer received the failure notification, the peer + * announces all transactions from their set. + + * This is a modification of the Erlay protocol (https://arxiv.org/abs/1905.10518) with two + * changes (sketch extensions instead of bisections, and an extra INV exchange round), both + * are motivated in BIP-330. + */ +class TxReconciliationTracker +{ +private: + class Impl; + const std::unique_ptr m_impl; + +public: + explicit TxReconciliationTracker(); + ~TxReconciliationTracker(); + + /** + * Step 0. Generates initial part of the state (salt) required to reconcile txs with the peer. + * The salt is used for short ID computation required for txreconciliation. + * The function returns the salt. + * A peer can't participate in future txreconciliations without this call. + * This function must be called only once per peer. + */ + uint64_t PreRegisterPeer(NodeId peer_id); +}; + +#endif // BITCOIN_NODE_TXRECONCILIATION_H diff --git a/src/protocol.cpp b/src/protocol.cpp index bdd1cc2aff2..23c68b335bf 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -44,6 +44,7 @@ const char *CFHEADERS="cfheaders"; const char *GETCFCHECKPT="getcfcheckpt"; const char *CFCHECKPT="cfcheckpt"; const char *WTXIDRELAY="wtxidrelay"; +const char *SENDTXRCNCL="sendtxrcncl"; } // namespace NetMsgType /** All known message types. Keep this in the same order as the list of @@ -84,6 +85,7 @@ const static std::string allNetMessageTypes[] = { NetMsgType::GETCFCHECKPT, NetMsgType::CFCHECKPT, NetMsgType::WTXIDRELAY, + NetMsgType::SENDTXRCNCL, }; const static std::vector allNetMessageTypesVec(std::begin(allNetMessageTypes), std::end(allNetMessageTypes)); diff --git a/src/protocol.h b/src/protocol.h index b85dc0d8201..17a363b1d32 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -258,6 +258,14 @@ extern const char* CFCHECKPT; * @since protocol version 70016 as described by BIP 339. */ 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. + * The salt is used to compute short txids needed for efficient + * txreconciliation, as described by BIP 330. + */ +extern const char* SENDTXRCNCL; }; // namespace NetMsgType /* Get a vector of all valid message types (see above) */ diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index f6a35da4fc0..5a4df735da7 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -130,6 +130,7 @@ FUZZ_TARGET_MSG(pong); FUZZ_TARGET_MSG(sendaddrv2); FUZZ_TARGET_MSG(sendcmpct); FUZZ_TARGET_MSG(sendheaders); +FUZZ_TARGET_MSG(sendtxrcncl); FUZZ_TARGET_MSG(tx); FUZZ_TARGET_MSG(verack); FUZZ_TARGET_MSG(version); From 4470acf076289831ac60bcbafb6b3afa0e98e99d Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Sat, 20 Mar 2021 12:50:02 +0200 Subject: [PATCH 3/9] p2p: Forget peer's reconciliation state on disconnect --- src/net_processing.cpp | 1 + src/node/txreconciliation.cpp | 14 ++++++++++++++ src/node/txreconciliation.h | 6 ++++++ 3 files changed, 21 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a10b6bfc471..af8afe235e3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1494,6 +1494,7 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) } WITH_LOCK(g_cs_orphans, m_orphanage.EraseForPeer(nodeid)); m_txrequest.DisconnectedPeer(nodeid); + if (m_txreconciliation) m_txreconciliation->ForgetPeer(nodeid); m_num_preferred_download_peers -= state->fPreferredDownload; m_peers_downloading_from -= (state->nBlocksInFlight != 0); assert(m_peers_downloading_from >= 0); diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp index dfac2ad82ed..d8cb1564759 100644 --- a/src/node/txreconciliation.cpp +++ b/src/node/txreconciliation.cpp @@ -54,6 +54,15 @@ public: Assert(m_states.emplace(peer_id, local_salt).second); return local_salt; } + + void ForgetPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) + { + AssertLockNotHeld(m_txreconciliation_mutex); + LOCK(m_txreconciliation_mutex); + if (m_states.erase(peer_id)) { + LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Forget txreconciliation state of peer=%d\n", peer_id); + } + } }; TxReconciliationTracker::TxReconciliationTracker() : m_impl{std::make_unique()} {} @@ -64,3 +73,8 @@ uint64_t TxReconciliationTracker::PreRegisterPeer(NodeId peer_id) { return m_impl->PreRegisterPeer(peer_id); } + +void TxReconciliationTracker::ForgetPeer(NodeId peer_id) +{ + m_impl->ForgetPeer(peer_id); +} diff --git a/src/node/txreconciliation.h b/src/node/txreconciliation.h index 6e94a51cf24..b8e9b649c08 100644 --- a/src/node/txreconciliation.h +++ b/src/node/txreconciliation.h @@ -61,6 +61,12 @@ public: * This function must be called only once per peer. */ uint64_t PreRegisterPeer(NodeId peer_id); + + /** + * Attempts to forget txreconciliation-related state of the peer (if we previously stored any). + * After this, we won't be able to reconcile transactions with the peer. + */ + void ForgetPeer(NodeId peer_id); }; #endif // BITCOIN_NODE_TXRECONCILIATION_H From 36cf6bf2168f9f154a652c91bbb96480c2e1d044 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Tue, 11 Jan 2022 09:34:19 +0200 Subject: [PATCH 4/9] Add helper to see if a peer is registered for reconciliations --- src/node/txreconciliation.cpp | 14 ++++++++++++++ src/node/txreconciliation.h | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp index d8cb1564759..f35beae28b0 100644 --- a/src/node/txreconciliation.cpp +++ b/src/node/txreconciliation.cpp @@ -63,6 +63,15 @@ public: LogPrintLevel(BCLog::TXRECONCILIATION, BCLog::Level::Debug, "Forget txreconciliation state of peer=%d\n", peer_id); } } + + bool IsPeerRegistered(NodeId peer_id) const EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) + { + AssertLockNotHeld(m_txreconciliation_mutex); + LOCK(m_txreconciliation_mutex); + auto recon_state = m_states.find(peer_id); + return (recon_state != m_states.end() && + std::holds_alternative(recon_state->second)); + } }; TxReconciliationTracker::TxReconciliationTracker() : m_impl{std::make_unique()} {} @@ -78,3 +87,8 @@ void TxReconciliationTracker::ForgetPeer(NodeId peer_id) { m_impl->ForgetPeer(peer_id); } + +bool TxReconciliationTracker::IsPeerRegistered(NodeId peer_id) const +{ + return m_impl->IsPeerRegistered(peer_id); +} diff --git a/src/node/txreconciliation.h b/src/node/txreconciliation.h index b8e9b649c08..1bffbea9dbb 100644 --- a/src/node/txreconciliation.h +++ b/src/node/txreconciliation.h @@ -67,6 +67,11 @@ public: * After this, we won't be able to reconcile transactions with the peer. */ void ForgetPeer(NodeId peer_id); + + /** + * Check if a peer is registered to reconcile transactions with us. + */ + bool IsPeerRegistered(NodeId peer_id) const; }; #endif // BITCOIN_NODE_TXRECONCILIATION_H From 88d326c8e3796b9685c141fa50628615d3e43a38 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Fri, 19 Mar 2021 13:07:15 +0200 Subject: [PATCH 5/9] p2p: Finish negotiating reconciliation support Once we received a reconciliation announcement support message from a peer and it doesn't violate our protocol, we store the negotiated parameters which will be used for future reconciliations. --- src/net_processing.cpp | 54 +++++++++++++++++++- src/node/txreconciliation.cpp | 94 ++++++++++++++++++++++++++++++++++- src/node/txreconciliation.h | 15 +++++- 3 files changed, 159 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index af8afe235e3..350187bdda3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1782,7 +1782,7 @@ PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman, // While Erlay support is incomplete, it must be enabled explicitly via -txreconciliation. // This argument can go away after Erlay support is complete. if (gArgs.GetBoolArg("-txreconciliation", DEFAULT_TXRECONCILIATION_ENABLE)) { - m_txreconciliation = std::make_unique(); + m_txreconciliation = std::make_unique(TXRECONCILIATION_VERSION); } } @@ -3477,6 +3477,58 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } + // Received from a peer demonstrating readiness to announce transactions via reconciliations. + // This feature negotiation must happen between VERSION and VERACK to avoid relay problems + // from switching announcement protocols after the connection is up. + if (msg_type == NetMsgType::SENDTXRCNCL) { + if (!m_txreconciliation) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "sendtxrcncl from peer=%d ignored, as our node does not have txreconciliation enabled\n", pfrom.GetId()); + return; + } + + 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. + 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; + uint32_t peer_txreconcl_version; + uint64_t remote_salt; + vRecv >> is_peer_initiator >> is_peer_responder >> peer_txreconcl_version >> remote_salt; + + if (m_txreconciliation->IsPeerRegistered(pfrom.GetId())) { + // A peer is already registered, meaning we already received SENDTXRCNCL from them. + 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) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "txreconciliation protocol violation from peer=%d; disconnecting\n", pfrom.GetId()); + pfrom.fDisconnect = true; + return; + } + return; + } + if (!pfrom.fSuccessfullyConnected) { LogPrint(BCLog::NET, "Unsupported message \"%s\" prior to verack from peer=%d\n", SanitizeString(msg_type), pfrom.GetId()); return; diff --git a/src/node/txreconciliation.cpp b/src/node/txreconciliation.cpp index f35beae28b0..974358fcda0 100644 --- a/src/node/txreconciliation.cpp +++ b/src/node/txreconciliation.cpp @@ -13,11 +13,46 @@ namespace { +/** Static salt component used to compute short txids for sketch construction, see BIP-330. */ +const std::string RECON_STATIC_SALT = "Tx Relay Salting"; +const HashWriter RECON_SALT_HASHER = TaggedHash(RECON_STATIC_SALT); + +/** + * Salt (specified by BIP-330) constructed from contributions from both peers. It is used + * to compute transaction short IDs, which are then used to construct a sketch representing a set + * of transactions we want to announce to the peer. + */ +uint256 ComputeSalt(uint64_t salt1, uint64_t salt2) +{ + // According to BIP-330, salts should be combined in ascending order. + return (HashWriter(RECON_SALT_HASHER) << std::min(salt1, salt2) << std::max(salt1, salt2)).GetSHA256(); +} + /** * Keeps track of txreconciliation-related per-peer state. */ class TxReconciliationState { +public: + /** + * TODO: This field is public to ignore -Wunused-private-field. Make private once used in + * the following commits. + * + * Reconciliation protocol assumes using one role consistently: either a reconciliation + * initiator (requesting sketches), or responder (sending sketches). This defines our role. + * + */ + bool m_we_initiate; + + /** + * TODO: These fields are public to ignore -Wunused-private-field. Make private once used in + * the following commits. + * + * These values are used to salt short IDs, which is necessary for transaction reconciliations. + */ + uint64_t m_k0, m_k1; + + TxReconciliationState(bool we_initiate, uint64_t k0, uint64_t k1) : m_we_initiate(we_initiate), m_k0(k0), m_k1(k1) {} }; } // namespace @@ -28,6 +63,9 @@ class TxReconciliationTracker::Impl private: mutable Mutex m_txreconciliation_mutex; + // Local protocol version + uint32_t m_recon_version; + /** * Keeps track of txreconciliation states of eligible peers. * For pre-registered peers, the locally generated salt is stored. @@ -37,7 +75,7 @@ private: std::unordered_map> m_states GUARDED_BY(m_txreconciliation_mutex); public: - explicit Impl() {} + explicit Impl(uint32_t recon_version) : m_recon_version(recon_version) {} uint64_t PreRegisterPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) { @@ -55,6 +93,50 @@ public: 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) + { + 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(&recon_state->second); + // A peer is already registered. This should be checked by the caller. + Assume(local_salt); + + // 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 + // versions have the choice of reconciling with this current version. However, they also + // have the choice to refuse supporting reconciliations if the common version is not + // 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; + + 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); + + 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; + } + void ForgetPeer(NodeId peer_id) EXCLUSIVE_LOCKS_REQUIRED(!m_txreconciliation_mutex) { AssertLockNotHeld(m_txreconciliation_mutex); @@ -74,7 +156,7 @@ public: } }; -TxReconciliationTracker::TxReconciliationTracker() : m_impl{std::make_unique()} {} +TxReconciliationTracker::TxReconciliationTracker(uint32_t recon_version) : m_impl{std::make_unique(recon_version)} {} TxReconciliationTracker::~TxReconciliationTracker() = default; @@ -83,6 +165,14 @@ uint64_t TxReconciliationTracker::PreRegisterPeer(NodeId peer_id) return m_impl->PreRegisterPeer(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); +} + void TxReconciliationTracker::ForgetPeer(NodeId peer_id) { m_impl->ForgetPeer(peer_id); diff --git a/src/node/txreconciliation.h b/src/node/txreconciliation.h index 1bffbea9dbb..a4f08709148 100644 --- a/src/node/txreconciliation.h +++ b/src/node/txreconciliation.h @@ -16,6 +16,12 @@ 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, +}; + /** * Transaction reconciliation is a way for nodes to efficiently announce transactions. * This object keeps track of all txreconciliation-related communications with the peers. @@ -50,7 +56,7 @@ private: const std::unique_ptr m_impl; public: - explicit TxReconciliationTracker(); + explicit TxReconciliationTracker(uint32_t recon_version); ~TxReconciliationTracker(); /** @@ -62,6 +68,13 @@ public: */ uint64_t PreRegisterPeer(NodeId peer_id); + /** + * 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); + /** * Attempts to forget txreconciliation-related state of the peer (if we previously stored any). * After this, we won't be able to reconcile transactions with the peer. From f63f1d3f4bdef2923a8395473ca59e4763769bd7 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Fri, 17 Dec 2021 13:32:16 +0200 Subject: [PATCH 6/9] p2p: clear txreconciliation state for non-wtxid peers We optimistically pre-register a peer for txreconciliations upon sending txreconciliation support announcement. But if, at VERACK, we realize that the peer never sent WTXIDRELAY message, we should unregister the peer from txreconciliations, because txreconciliations rely on wtxids. --- src/net_processing.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 350187bdda3..c87b0e7cd2b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3414,6 +3414,16 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // they may wish to request compact blocks from us m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); } + + if (m_txreconciliation) { + if (!peer->m_wtxid_relay || !m_txreconciliation->IsPeerRegistered(pfrom.GetId())) { + // We could have optimistically pre-registered/registered the peer. In that case, + // we should forget about the reconciliation state here if this wasn't followed + // by WTXIDRELAY (since WTXIDRELAY can't be announced later). + m_txreconciliation->ForgetPeer(pfrom.GetId()); + } + } + pfrom.fSuccessfullyConnected = true; return; } From b99ee9d22d53777f501d0926d7ca6b0b45b237a1 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Fri, 5 Nov 2021 10:40:55 +0200 Subject: [PATCH 7/9] test: Add unit tests for reconciliation negotiation --- src/Makefile.test.include | 1 + src/test/txreconciliation_tests.cpp | 86 +++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 src/test/txreconciliation_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index b21e9906a39..fc60359efde 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -148,6 +148,7 @@ BITCOIN_TESTS =\ test/transaction_tests.cpp \ test/txindex_tests.cpp \ test/txpackage_tests.cpp \ + test/txreconciliation_tests.cpp \ test/txrequest_tests.cpp \ test/txvalidation_tests.cpp \ test/txvalidationcache_tests.cpp \ diff --git a/src/test/txreconciliation_tests.cpp b/src/test/txreconciliation_tests.cpp new file mode 100644 index 00000000000..bd74998002c --- /dev/null +++ b/src/test/txreconciliation_tests.cpp @@ -0,0 +1,86 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include + +#include + +BOOST_FIXTURE_TEST_SUITE(txreconciliation_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(RegisterPeerTest) +{ + TxReconciliationTracker tracker(1); + 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); + + // Valid registration. + BOOST_REQUIRE(!tracker.IsPeerRegistered(0)); + BOOST_REQUIRE(tracker.RegisterPeer(0, true, true, false, 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_CHECK(tracker.IsPeerRegistered(1)); + + // 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_CHECK(!tracker.IsPeerRegistered(100)); +} + +BOOST_AUTO_TEST_CASE(ForgetPeerTest) +{ + TxReconciliationTracker tracker(1); + 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); + + // 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_CHECK(tracker.IsPeerRegistered(peer_id0)); + tracker.ForgetPeer(peer_id0); + BOOST_CHECK(!tracker.IsPeerRegistered(peer_id0)); +} + +BOOST_AUTO_TEST_CASE(IsPeerRegisteredTest) +{ + TxReconciliationTracker tracker(1); + 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_CHECK(tracker.IsPeerRegistered(peer_id0)); + + tracker.ForgetPeer(peer_id0); + BOOST_CHECK(!tracker.IsPeerRegistered(peer_id0)); +} + +BOOST_AUTO_TEST_SUITE_END() From cfcef60779e62bcb81243e0bb8ffc8abb5b5baf5 Mon Sep 17 00:00:00 2001 From: Gleb Naumenko Date: Wed, 30 Mar 2022 17:21:56 +0300 Subject: [PATCH 8/9] test: Add functional tests for sendtxrcncl from inbound --- test/functional/p2p_sendtxrcncl.py | 163 +++++++++++++++++++++ test/functional/test_framework/messages.py | 28 ++++ test/functional/test_framework/p2p.py | 3 + test/functional/test_runner.py | 1 + 4 files changed, 195 insertions(+) create mode 100755 test/functional/p2p_sendtxrcncl.py diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py new file mode 100755 index 00000000000..3b0162b44df --- /dev/null +++ b/test/functional/p2p_sendtxrcncl.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test SENDTXRCNCL message +""" + +from test_framework.messages import ( + msg_sendtxrcncl, + msg_verack, + msg_version, + msg_wtxidrelay, +) +from test_framework.p2p import ( + P2PInterface, + P2P_SERVICES, + P2P_SUBVERSION, + P2P_VERSION, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +class PeerNoVerack(P2PInterface): + def __init__(self, wtxidrelay=True): + super().__init__(wtxidrelay=wtxidrelay) + + def on_version(self, message): + # Avoid sending verack in response to version. + # When calling add_p2p_connection, wait_for_verack=False must be set (see + # comment in add_p2p_connection). + if message.nVersion >= 70016 and self.wtxidrelay: + self.send_message(msg_wtxidrelay()) + +class SendTxrcnclReceiver(P2PInterface): + def __init__(self): + super().__init__() + self.sendtxrcncl_msg_received = None + + def on_sendtxrcncl(self, message): + self.sendtxrcncl_msg_received = message + +class PeerTrackMsgOrder(P2PInterface): + def __init__(self): + super().__init__() + self.messages = [] + + def on_message(self, message): + super().on_message(message) + self.messages.append(message) + +def create_sendtxrcncl_msg(initiator=True): + sendtxrcncl_msg = msg_sendtxrcncl() + sendtxrcncl_msg.initiator = initiator + sendtxrcncl_msg.responder = not initiator + sendtxrcncl_msg.version = 1 + sendtxrcncl_msg.salt = 2 + return sendtxrcncl_msg + +class SendTxRcnclTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [['-txreconciliation']] + + def run_test(self): + 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) + peer.peer_disconnect() + + self.log.info('SENDTXRCNCL should be sent before VERACK') + peer = self.nodes[0].add_p2p_connection(PeerTrackMsgOrder(), send_version=True, wait_for_verack=True) + 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) + peer.peer_disconnect() + + self.log.info('SENDTXRCNCL on pre-WTXID version should not be sent') + peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=False, wait_for_verack=False) + pre_wtxid_version_msg = msg_version() + pre_wtxid_version_msg.nVersion = 70015 + pre_wtxid_version_msg.strSubVer = P2P_SUBVERSION + pre_wtxid_version_msg.nServices = P2P_SERVICES + pre_wtxid_version_msg.relay = 1 + peer.send_message(pre_wtxid_version_msg) + peer.wait_for_verack() + assert not peer.sendtxrcncl_msg_received + peer.peer_disconnect() + + self.log.info('SENDTXRCNCL for fRelay=false should not be sent') + 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 not peer.sendtxrcncl_msg_received + peer.peer_disconnect() + + 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"]) + self.log.info('second SENDTXRCNCL triggers a disconnect') + 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 + peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) + peer.send_message(sendtxrcncl_no_role) + peer.wait_for_disconnect() + + 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) + peer.send_message(sendtxrcncl_wrong_role) + peer.wait_for_disconnect() + + self.log.info('SENDTXRCNCL with version=0 triggers a disconnect') + sendtxrcncl_low_version = create_sendtxrcncl_msg() + sendtxrcncl_low_version.version = 0 + peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) + peer.send_message(sendtxrcncl_low_version) + peer.wait_for_disconnect() + + self.log.info('sending SENDTXRCNCL after sending VERACK triggers a disconnect') + # We use PeerNoVerack even though verack is sent right after, to make sure it was actually + # sent before sendtxrcncl is sent. + peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) + peer.send_and_ping(msg_verack()) + peer.send_message(create_sendtxrcncl_msg()) + peer.wait_for_disconnect() + + self.log.info('SENDTXRCNCL without WTXIDRELAY is ignored (recon state is erased after VERACK)') + peer = self.nodes[0].add_p2p_connection(PeerNoVerack(wtxidrelay=False), send_version=True, wait_for_verack=False) + with self.nodes[0].assert_debug_log(['Forget txreconciliation state of peer']): + peer.send_message(create_sendtxrcncl_msg()) + peer.send_message(msg_verack()) + peer.peer_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 + peer.peer_disconnect() + + 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 + peer.peer_disconnect() + + +if __name__ == '__main__': + SendTxRcnclTest().main() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 8a928a1e506..252b49cc6d6 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1838,3 +1838,31 @@ class msg_cfcheckpt: def __repr__(self): return "msg_cfcheckpt(filter_type={:#x}, stop_hash={:x})".format( self.filter_type, self.stop_hash) + +class msg_sendtxrcncl: + __slots__ = ("initiator", "responder", "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(" Date: Mon, 19 Sep 2022 17:11:32 +0300 Subject: [PATCH 9/9] test: Add functional tests for sendtxrcncl message from outbound --- test/functional/p2p_sendtxrcncl.py | 28 +++++++++++++++++++++ test/functional/test_framework/test_node.py | 7 +++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index 3b0162b44df..f4c5dd4586e 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -146,6 +146,34 @@ class SendTxRcnclTest(BitcoinTestFramework): peer.send_message(msg_verack()) peer.peer_disconnect() + self.log.info('SENDTXRCNCL sent to an outbound') + peer = self.nodes[0].add_outbound_p2p_connection( + SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=1, 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) + peer.peer_disconnect() + + 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=2, connection_type="block-relay-only") + 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") + 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( + P2PInterface(), wait_for_verack=False, p2p_idx=4, connection_type="outbound-full-relay") + peer.send_message(sendtxrcncl_wrong_role) + 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) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index e35cae006f3..2367a9a8fa4 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -618,7 +618,7 @@ class TestNode(): return p2p_conn - def add_outbound_p2p_connection(self, p2p_conn, *, p2p_idx, connection_type="outbound-full-relay", **kwargs): + def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", **kwargs): """Add an outbound p2p connection from node. Must be an "outbound-full-relay", "block-relay-only", "addr-fetch" or "feeler" connection. @@ -640,8 +640,9 @@ class TestNode(): p2p_conn.wait_for_connect() self.p2ps.append(p2p_conn) - p2p_conn.wait_for_verack() - p2p_conn.sync_with_ping() + if wait_for_verack: + p2p_conn.wait_for_verack() + p2p_conn.sync_with_ping() return p2p_conn