From d91e48b4c9b229034d5387f1a480a62c706366e4 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 31 Oct 2026 22:03:14 +0000 Subject: [PATCH 1/3] Merge bitcoin/bitcoin#27114: p2p: Allow whitelisting manual connections 0a533613fb44207053796fd01a9f4b523a3153d4 docs: add release notes for #27114 (brunoerg) e6b8f19de9a6d1c477d0bbda18d17794cd81a6f4 test: add coverage for whitelisting manual connections (brunoerg) c985eb854cc86deb747caea5283c17cf51b6a983 test: add option to speed up tx relay/mempool sync (brunoerg) 66bc6e2d1749f43d7b314aa2784a06af78440170 Accept "in" and "out" flags to -whitelist to allow whitelisting manual connections (Luke Dashjr) 8e06be347c5e14cbe75256eba170e0867f95f360 net_processing: Move extra service flag into InitializeNode (Luke Dashjr) 9133fd69a5cc9a0ab1a06a60d09f1b7e1039018e net: Move `NetPermissionFlags::Implicit` verification to `AddWhitelistPermissionFlags` (Luke Dashjr) 2863d7dddb62d987b3e1c3b8bfad7083f0f774b2 net: store `-whitelist{force}relay` values in `CConnman` (brunoerg) Pull request description: Revives #17167. It allows whitelisting manual connections. Fixes #9923 Since there are some PRs/issues around this topic, I'll list some motivations/comments for whitelisting outbound connections from them: - Speed-up tx relay/mempool sync for testing purposes (my personal motivation for this) - In #26970, theStack pointed out that we whitelist peers to speed up tx relay for fast mempool synchronization, however, since it applies only for inbound connections and considering the topology `node0 <--- node1 <---- node2 <--- ... <-- nodeN`, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long. - https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1865155764 - "Before enabling -v2transport by default (which I'd image may happen after https://github.com/bitcoin/bitcoin/pull/24748) we could consider a way to force manual connections to be only-v1 or even only-v2 (disabling reconnect-with-v1). A possibility could be through a net permission flag, if https://github.com/bitcoin/bitcoin/pull/27114 makes it in." - https://github.com/bitcoin/bitcoin/pull/17167#issuecomment-1168606032 - "This would allow us to use https://github.com/bitcoin/bitcoin/pull/25355 when making outgoing connections to all nodes, except to whitelisted ones for which we would use our persistent I2P address." - Force-relay/mempool permissions for a node you intentionally connected to. ACKs for top commit: achow101: ACK 0a533613fb44207053796fd01a9f4b523a3153d4 sr-gi: re-ACK [0a53361](https://github.com/bitcoin/bitcoin/pull/27114/commits/0a533613fb44207053796fd01a9f4b523a3153d4) pinheadmz: ACK 0a533613fb44207053796fd01a9f4b523a3153d4 Tree-SHA512: 97a79bb854110da04540897d2619eda409d829016aafdf1825ab5515334b0b42ef82f33cd41587af235b3af6ddcec3f2905ca038b5ab22e4c8a03d34f27aebe1 Dash adaptations: - Test hunks for p2p_feefilter.py, p2p_segwit.py, wallet_address_types.py, wallet_avoid_mixing_output_types.py and wallet_bumpfee.py are dropped; those tests don't exist in Dash. - Converted tests keep their Dash-specific arguments (-blockmaxsize, -maxorphantxsize, -usehd, -dip3params, ...). interface_zmq.py keeps its ipc:// to unix: replacement when restarting the node. - Dash's ConnectionDirection also has Verified bits; -whitelist parsing only ever sets In and Out. Co-Authored-By: Claude Opus 5.5 --- doc/release-notes-27114.md | 2 + src/init.cpp | 24 ++++++++---- src/net.cpp | 38 ++++++++++--------- src/net.h | 33 +++++++++++----- src/net_permissions.cpp | 28 ++++++++++++-- src/net_permissions.h | 8 +++- src/net_processing.cpp | 5 +++ src/test/fuzz/net_permissions.cpp | 4 +- src/test/netbase_tests.cpp | 18 +++++++-- test/functional/feature_cltv.py | 3 +- test/functional/feature_csv_activation.py | 3 +- test/functional/feature_dersig.py | 3 +- test/functional/feature_fee_estimation.py | 9 +++-- test/functional/interface_rest.py | 3 +- test/functional/interface_zmq.py | 8 ++-- test/functional/mempool_packages.py | 3 +- test/functional/p2p_filter.py | 3 +- test/functional/p2p_invalid_block.py | 3 +- test/functional/p2p_permissions.py | 7 ++++ test/functional/rpc_packages.py | 3 +- test/functional/rpc_rawtransaction.py | 5 +-- .../test_framework/test_framework.py | 5 +++ test/functional/wallet_abandonconflict.py | 3 +- test/functional/wallet_avoidreuse.py | 5 +-- test/functional/wallet_backup.py | 13 ++++--- test/functional/wallet_balance.py | 5 +-- test/functional/wallet_basic.py | 6 ++- test/functional/wallet_fundrawtransaction.py | 6 +-- test/functional/wallet_groups.py | 3 +- test/functional/wallet_hd.py | 3 +- test/functional/wallet_import_rescan.py | 4 +- test/functional/wallet_importdescriptors.py | 5 +-- test/functional/wallet_listreceivedby.py | 2 +- test/functional/wallet_listsinceblock.py | 2 +- test/functional/wallet_listtransactions.py | 5 +-- test/functional/wallet_send.py | 7 ++-- 36 files changed, 188 insertions(+), 99 deletions(-) create mode 100644 doc/release-notes-27114.md diff --git a/doc/release-notes-27114.md b/doc/release-notes-27114.md new file mode 100644 index 000000000000..980ffd78a44f --- /dev/null +++ b/doc/release-notes-27114.md @@ -0,0 +1,2 @@ +- Additional flags "in" and "out" have been added to `-whitelist` to control whether + permissions apply to incoming connections and/or manual (default: incoming only). \ No newline at end of file diff --git a/src/init.cpp b/src/init.cpp index 0a040044b4f3..0ae176913494 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -613,7 +613,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-blocknotify=", "Execute command when the best block changes (%s in cmd is replaced by block hash)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #endif argsman.AddArg("-blockreconstructionextratxn=", strprintf("Extra transactions to keep in memory for compact block reconstructions (default: %u)", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Automatic broadcast and rebroadcast of any transactions from inbound peers is disabled, unless the peer has the 'forcerelay' permission. RPC transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Disables automatic broadcast and rebroadcast of transactions, unless the source peer has the 'forcerelay' permission. RPC transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #if HAVE_SYSTEM argsman.AddArg("-chainlocknotify=", "Execute command when the best chainlock changes (%s in cmd is replaced by chainlocked block hash)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #endif @@ -722,9 +722,11 @@ void SetupServerArgs(ArgsManager& argsman) "Use [host]:port notation for IPv6. Allowed permissions: " + Join(NET_PERMISSIONS_DOC, ", ") + ". " "Specify multiple permissions separated by commas (default: download,noban,mempool,relay). Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-whitelist=<[permissions@]IP address or network>", "Add permission flags to the peers connecting from the given IP address (e.g. 1.2.3.4) or " + argsman.AddArg("-whitelist=<[permissions@]IP address or network>", "Add permission flags to the peers using the given IP address (e.g. 1.2.3.4) or " "CIDR-notated network (e.g. 1.2.3.0/24). Uses the same permissions as " - "-whitebind. Can be specified multiple times." , ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + "-whitebind. " + "Additional flags \"in\" and \"out\" control whether permissions apply to incoming connections and/or manual (default: incoming only). " + "Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); g_wallet_init_interface.AddWalletOptions(argsman); @@ -859,8 +861,8 @@ void SetupServerArgs(ArgsManager& argsman) OptionsCategory::NODE_RELAY); argsman.AddArg("-minrelaytxfee=", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_MIN_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); - argsman.AddArg("-whitelistforcerelay", strprintf("Add 'forcerelay' permission to whitelisted inbound peers with default permissions. This will relay transactions even if the transactions were already in the mempool. (default: %d)", DEFAULT_WHITELISTFORCERELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); - argsman.AddArg("-whitelistrelay", strprintf("Add 'relay' permission to whitelisted inbound peers with default permissions. This will accept relayed transactions even when not relaying transactions (default: %d)", DEFAULT_WHITELISTRELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); + argsman.AddArg("-whitelistforcerelay", strprintf("Add 'forcerelay' permission to whitelisted peers with default permissions. This will relay transactions even if the transactions were already in the mempool. (default: %d)", DEFAULT_WHITELISTFORCERELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); + argsman.AddArg("-whitelistrelay", strprintf("Add 'relay' permission to whitelisted peers with default permissions. This will accept relayed transactions even when not relaying transactions (default: %d)", DEFAULT_WHITELISTRELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); argsman.AddArg("-blockmaxsize=", strprintf("Set maximum block size in bytes (default: %d)", DEFAULT_BLOCK_MAX_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-blockmintxfee=", strprintf("Set lowest fee rate (in %s/kB) for transactions to be included in block creation. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_BLOCK_MIN_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); @@ -2638,6 +2640,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // wallets may mix with CoinJoin, which connects to the mixing masternode connOptions.m_masternode_connections = node.active_ctx != nullptr || quorums_watch || node.wallet_loader != nullptr; + connOptions.whitelist_forcerelay = args.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY); + connOptions.whitelist_relay = args.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY); // Port to bind to if `-bind=addr` is provided without a `:port` suffix. const uint16_t default_bind_port = @@ -2725,9 +2729,15 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) for (const auto& net : args.GetArgs("-whitelist")) { NetWhitelistPermissions subnet; + ConnectionDirection connection_direction; bilingual_str error; - if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(error); - connOptions.vWhitelistedRange.push_back(subnet); + if (!NetWhitelistPermissions::TryParse(net, subnet, connection_direction, error)) return InitError(error); + if (connection_direction & ConnectionDirection::In) { + connOptions.vWhitelistedRangeIncoming.push_back(subnet); + } + if (connection_direction & ConnectionDirection::Out) { + connOptions.vWhitelistedRangeOutgoing.push_back(subnet); + } } connOptions.vSeedNodes = args.GetArgs("-seednode"); diff --git a/src/net.cpp b/src/net.cpp index 2cc479de5281..3bd224322fcd 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -489,6 +489,10 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo return nullptr; } + NetPermissionFlags permission_flags = NetPermissionFlags::None; + std::vector whitelist_permissions = conn_type == ConnectionType::MANUAL ? vWhitelistedRangeOutgoing : std::vector{}; + AddWhitelistPermissionFlags(permission_flags, addrConnect, whitelist_permissions); + // Add node NodeId id = GetNewNodeId(); uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); @@ -505,6 +509,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo conn_type, /*inbound_onion=*/false, CNodeOptions{ + .permission_flags = permission_flags, .i2p_sam_session = std::move(i2p_transient_session), .recv_flood_size = nReceiveFloodSize, .use_v2transport = use_v2transport, @@ -547,9 +552,18 @@ void CNode::CloseSocketDisconnect(CConnman* connman) ::g_stats_client->inc("peers.disconnect", 1.0f); } -void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const { - for (const auto& subnet : vWhitelistedRange) { - if (subnet.m_subnet.Match(addr)) NetPermissions::AddFlag(flags, subnet.m_flags); +void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr, const std::vector& ranges) const { + for (const auto& subnet : ranges) { + if (subnet.m_subnet.Match(addr)) { + NetPermissions::AddFlag(flags, subnet.m_flags); + } + } + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Implicit)) { + NetPermissions::ClearFlag(flags, NetPermissionFlags::Implicit); + if (whitelist_forcerelay) NetPermissions::AddFlag(flags, NetPermissionFlags::ForceRelay); + if (whitelist_relay) NetPermissions::AddFlag(flags, NetPermissionFlags::Relay); + NetPermissions::AddFlag(flags, NetPermissionFlags::Mempool); + NetPermissions::AddFlag(flags, NetPermissionFlags::NoBan); } } @@ -1888,14 +1902,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, int nVerifiedInboundMasternodes = 0; int nMaxInbound = nMaxConnections - m_max_outbound; - AddWhitelistPermissionFlags(permission_flags, addr); - if (NetPermissions::HasFlag(permission_flags, NetPermissionFlags::Implicit)) { - NetPermissions::ClearFlag(permission_flags, NetPermissionFlags::Implicit); - if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permission_flags, NetPermissionFlags::ForceRelay); - if (gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) NetPermissions::AddFlag(permission_flags, NetPermissionFlags::Relay); - NetPermissions::AddFlag(permission_flags, NetPermissionFlags::Mempool); - NetPermissions::AddFlag(permission_flags, NetPermissionFlags::NoBan); - } + AddWhitelistPermissionFlags(permission_flags, addr, vWhitelistedRangeIncoming); { @@ -1975,15 +1982,10 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, NodeId id = GetNewNodeId(); uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); - ServiceFlags nodeServices = nLocalServices; - if (NetPermissions::HasFlag(permission_flags, NetPermissionFlags::BloomFilter)) { - nodeServices = static_cast(nodeServices | NODE_BLOOM); - } - const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end(); // The V2Transport transparently falls back to V1 behavior when an incoming V1 connection is // detected, so use it whenever we signal NODE_P2P_V2. - const bool use_v2transport(nodeServices & NODE_P2P_V2); + const bool use_v2transport(nLocalServices & NODE_P2P_V2); CNode* pnode = new CNode(id, std::move(sock), @@ -2001,7 +2003,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, .use_v2transport = use_v2transport, }); pnode->AddRef(); - m_msgproc->InitializeNode(*pnode, nodeServices); + m_msgproc->InitializeNode(*pnode, nLocalServices); { LOCK(pnode->m_sock_mutex); diff --git a/src/net.h b/src/net.h index c23545887849..29f044631ea2 100644 --- a/src/net.h +++ b/src/net.h @@ -63,11 +63,6 @@ class CScheduler; struct bilingual_str; struct NodeEvictionCandidate; -/** Default for -whitelistrelay. */ -static const bool DEFAULT_WHITELISTRELAY = true; -/** Default for -whitelistforcerelay. */ -static const bool DEFAULT_WHITELISTFORCERELAY = false; - /** Time after which to disconnect, after waiting for a ping response (or inactivity). */ static constexpr std::chrono::minutes TIMEOUT_INTERVAL{20}; /** Time to wait since m_connected before disconnecting a probe node. */ @@ -1238,7 +1233,8 @@ friend class CNode; uint64_t nMaxOutboundLimit = 0; int64_t m_peer_connect_timeout = DEFAULT_PEER_CONNECT_TIMEOUT; std::vector vSeedNodes; - std::vector vWhitelistedRange; + std::vector vWhitelistedRangeIncoming; + std::vector vWhitelistedRangeOutgoing; std::vector vWhiteBinds; std::vector vBinds; std::vector onion_binds; @@ -1254,6 +1250,8 @@ friend class CNode; //! Run the thread that opens connections to masternodes; only masternodes, quorum watchers and //! CoinJoin mixing ever request them. bool m_masternode_connections = true; + bool whitelist_forcerelay = DEFAULT_WHITELISTFORCERELAY; + bool whitelist_relay = DEFAULT_WHITELISTRELAY; }; void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_total_bytes_sent_mutex) @@ -1279,7 +1277,8 @@ friend class CNode; LOCK(m_total_bytes_sent_mutex); nMaxOutboundLimit = connOptions.nMaxOutboundLimit; } - vWhitelistedRange = connOptions.vWhitelistedRange; + vWhitelistedRangeIncoming = connOptions.vWhitelistedRangeIncoming; + vWhitelistedRangeOutgoing = connOptions.vWhitelistedRangeOutgoing; { LOCK(m_added_nodes_mutex); // Attempt v2 connection if we support v2 - we'll reconnect with v1 if our @@ -1293,6 +1292,8 @@ friend class CNode; m_onion_binds = connOptions.onion_binds; m_active_masternode = connOptions.m_active_masternode; m_masternode_connections = connOptions.m_masternode_connections; + whitelist_forcerelay = connOptions.whitelist_forcerelay; + whitelist_relay = connOptions.whitelist_relay; } CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, const NetGroupManager& netgroupman, @@ -1749,7 +1750,7 @@ friend class CNode; std::vector GetEvictionCandidates() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); bool AttemptToEvictConnection() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_unused_i2p_sessions_mutex); - void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr) const; + void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr, const std::vector& ranges) const; void DeleteNode(CNode* pnode); @@ -1807,7 +1808,9 @@ friend class CNode; // Whitelisted ranges. Any node connecting from these is automatically // whitelisted (as well as those connecting to whitelisted binds). - std::vector vWhitelistedRange; + std::vector vWhitelistedRangeIncoming; + // Whitelisted ranges for outgoing connections. + std::vector vWhitelistedRangeOutgoing; unsigned int nSendBufferMaxSize{0}; unsigned int nReceiveFloodSize{0}; @@ -1990,6 +1993,18 @@ friend class CNode; */ std::vector m_onion_binds; + /** + * flag for adding 'forcerelay' permission to whitelisted inbound + * and manual peers with default permissions. + */ + bool whitelist_forcerelay; + + /** + * flag for adding 'relay' permission to whitelisted inbound + * and manual peers with default permissions. + */ + bool whitelist_relay; + /** * Mutex protecting m_i2p_sam_sessions. */ diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index 7ec2e6dc8b79..9cb2e35ac705 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -21,9 +21,10 @@ const std::vector NET_PERMISSIONS_DOC{ namespace { // Parse the following format: "perm1,perm2@xxxxxx" -bool TryParsePermissionFlags(const std::string& str, NetPermissionFlags& output, size_t& readen, bilingual_str& error) +static bool TryParsePermissionFlags(const std::string& str, NetPermissionFlags& output, ConnectionDirection* output_connection_direction, size_t& readen, bilingual_str& error) { NetPermissionFlags flags = NetPermissionFlags::None; + ConnectionDirection connection_direction = ConnectionDirection::None; const auto atSeparator = str.find('@'); // if '@' is not found (ie, "xxxxx"), the caller should apply implicit permissions @@ -52,6 +53,15 @@ bool TryParsePermissionFlags(const std::string& str, NetPermissionFlags& output, else if (permission == "all") NetPermissions::AddFlag(flags, NetPermissionFlags::All); else if (permission == "relay") NetPermissions::AddFlag(flags, NetPermissionFlags::Relay); else if (permission == "addr") NetPermissions::AddFlag(flags, NetPermissionFlags::Addr); + else if (permission == "in") connection_direction |= ConnectionDirection::In; + else if (permission == "out") { + if (output_connection_direction == nullptr) { + // Only NetWhitebindPermissions() should pass a nullptr. + error = _("whitebind may only be used for incoming connections (\"out\" was passed)"); + return false; + } + connection_direction |= ConnectionDirection::Out; + } else if (permission.length() == 0); // Allow empty entries else { error = strprintf(_("Invalid P2P permission: '%s'"), permission); @@ -61,7 +71,16 @@ bool TryParsePermissionFlags(const std::string& str, NetPermissionFlags& output, readen++; } + // By default, whitelist only applies to incoming connections + if (connection_direction == ConnectionDirection::None) { + connection_direction = ConnectionDirection::In; + } else if (flags == NetPermissionFlags::None) { + error = strprintf(_("Only direction was set, no permissions: '%s'"), str); + return false; + } + output = flags; + if (output_connection_direction) *output_connection_direction = connection_direction; error = Untranslated(""); return true; } @@ -85,7 +104,7 @@ bool NetWhitebindPermissions::TryParse(const std::string& str, NetWhitebindPermi { NetPermissionFlags flags; size_t offset; - if (!TryParsePermissionFlags(str, flags, offset, error)) return false; + if (!TryParsePermissionFlags(str, flags, /*output_connection_direction=*/nullptr, offset, error)) return false; const std::string strBind = str.substr(offset); const std::optional addrBind{Lookup(strBind, 0, false)}; @@ -104,11 +123,12 @@ bool NetWhitebindPermissions::TryParse(const std::string& str, NetWhitebindPermi return true; } -bool NetWhitelistPermissions::TryParse(const std::string& str, NetWhitelistPermissions& output, bilingual_str& error) +bool NetWhitelistPermissions::TryParse(const std::string& str, NetWhitelistPermissions& output, ConnectionDirection& output_connection_direction, bilingual_str& error) { NetPermissionFlags flags; size_t offset; - if (!TryParsePermissionFlags(str, flags, offset, error)) return false; + // Only NetWhitebindPermissions should pass a nullptr for output_connection_direction. + if (!TryParsePermissionFlags(str, flags, &output_connection_direction, offset, error)) return false; const std::string net = str.substr(offset); const CSubNet subnet{LookupSubNet(net)}; diff --git a/src/net_permissions.h b/src/net_permissions.h index 0c5388de5986..f61b946f6d5d 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include @@ -15,6 +16,11 @@ struct bilingual_str; extern const std::vector NET_PERMISSIONS_DOC; +/** Default for -whitelistrelay. */ +constexpr bool DEFAULT_WHITELISTRELAY = true; +/** Default for -whitelistforcerelay. */ +constexpr bool DEFAULT_WHITELISTFORCERELAY = false; + enum class NetPermissionFlags : uint32_t { None = 0, // Can query bloomfilter even if -peerbloomfilters is false @@ -82,7 +88,7 @@ class NetWhitebindPermissions : public NetPermissions class NetWhitelistPermissions : public NetPermissions { public: - static bool TryParse(const std::string& str, NetWhitelistPermissions& output, bilingual_str& error); + static bool TryParse(const std::string& str, NetWhitelistPermissions& output, ConnectionDirection& output_connection_direction, bilingual_str& error); CSubNet m_subnet; }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c6fefb7c3e6b..4ba98bbd7087 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1775,6 +1775,11 @@ void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services) { m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(node.IsInboundConn())); assert(WITH_LOCK(m_object_request_mutex, return m_object_request.Count(nodeid)) == 0); } + + if (NetPermissions::HasFlag(node.m_permission_flags, NetPermissionFlags::BloomFilter)) { + our_services = static_cast(our_services | NODE_BLOOM); + } + PeerRef peer = std::make_shared(nodeid, our_services); { LOCK(m_peer_mutex); diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp index 6ea2139c4606..811c0de4b92c 100644 --- a/src/test/fuzz/net_permissions.cpp +++ b/src/test/fuzz/net_permissions.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -31,8 +32,9 @@ FUZZ_TARGET(net_permissions) } NetWhitelistPermissions net_whitelist_permissions; + ConnectionDirection connection_direction; bilingual_str error_net_whitelist_permissions; - if (NetWhitelistPermissions::TryParse(s, net_whitelist_permissions, error_net_whitelist_permissions)) { + if (NetWhitelistPermissions::TryParse(s, net_whitelist_permissions, connection_direction, error_net_whitelist_permissions)) { (void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags); (void)NetPermissions::AddFlag(net_whitelist_permissions.m_flags, net_permission_flags); assert(NetPermissions::HasFlag(net_whitelist_permissions.m_flags, net_permission_flags)); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 054046f5aaa5..ab954665e601 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -500,6 +500,7 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) bilingual_str error; NetWhitebindPermissions whitebindPermissions; NetWhitelistPermissions whitelistPermissions; + ConnectionDirection connection_direction; // Detect invalid white bind BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error)); @@ -569,24 +570,33 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) BOOST_CHECK(NetWhitebindPermissions::TryParse(",,@1.2.3.4:32", whitebindPermissions, error)); BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::None); + BOOST_CHECK(!NetWhitebindPermissions::TryParse("out,forcerelay@1.2.3.4:32", whitebindPermissions, error)); + BOOST_CHECK(error.original.find("whitebind may only be used for incoming connections (\"out\" was passed)") != std::string::npos); + // Detect invalid flag BOOST_CHECK(!NetWhitebindPermissions::TryParse("bloom,forcerelay,oopsie@1.2.3.4:32", whitebindPermissions, error)); BOOST_CHECK(error.original.find("Invalid P2P permission") != std::string::npos); // Check netmask error - BOOST_CHECK(!NetWhitelistPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitelistPermissions, error)); + BOOST_CHECK(!NetWhitelistPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitelistPermissions, connection_direction, error)); BOOST_CHECK(error.original.find("Invalid netmask specified in -whitelist") != std::string::npos); // Happy path for whitelist parsing - BOOST_CHECK(NetWhitelistPermissions::TryParse("noban@1.2.3.4", whitelistPermissions, error)); + BOOST_CHECK(NetWhitelistPermissions::TryParse("noban@1.2.3.4", whitelistPermissions, connection_direction, error)); BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, NetPermissionFlags::NoBan); BOOST_CHECK(NetPermissions::HasFlag(whitelistPermissions.m_flags, NetPermissionFlags::NoBan)); - BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay@1.2.3.4/32", whitelistPermissions, error)); + BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay@1.2.3.4/32", whitelistPermissions, connection_direction, error)); BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, NetPermissionFlags::BloomFilter | NetPermissionFlags::ForceRelay | NetPermissionFlags::NoBan | NetPermissionFlags::Relay); BOOST_CHECK(error.empty()); BOOST_CHECK_EQUAL(whitelistPermissions.m_subnet.ToString(), "1.2.3.4/32"); - BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay,mempool@1.2.3.4/32", whitelistPermissions, error)); + BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay,mempool@1.2.3.4/32", whitelistPermissions, connection_direction, error)); + BOOST_CHECK(NetWhitelistPermissions::TryParse("in,relay@1.2.3.4", whitelistPermissions, connection_direction, error)); + BOOST_CHECK_EQUAL(connection_direction, ConnectionDirection::In); + BOOST_CHECK(NetWhitelistPermissions::TryParse("out,bloom@1.2.3.4", whitelistPermissions, connection_direction, error)); + BOOST_CHECK_EQUAL(connection_direction, ConnectionDirection::Out); + BOOST_CHECK(NetWhitelistPermissions::TryParse("in,out,bloom@1.2.3.4", whitelistPermissions, connection_direction, error)); + BOOST_CHECK_EQUAL(connection_direction, ConnectionDirection::Both); const auto strings = NetPermissions::ToStrings(NetPermissionFlags::All); BOOST_CHECK_EQUAL(strings.size(), 7U); diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index ca725722632a..15b5fd799f6e 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -86,9 +86,10 @@ def cltv_validate(tx, height): class BIP65Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [[ f'-testactivationheight=cltv@{CLTV_HEIGHT}', - '-whitelist=noban@127.0.0.1', '-par=1', # Use only one script thread to get the exact reject reason for testing '-acceptnonstdtxn=1', # cltv_invalidate is nonstandard '-dip3params=9000:9000', diff --git a/test/functional/feature_csv_activation.py b/test/functional/feature_csv_activation.py index 9c7f6e6ca647..6c96175fe1a3 100755 --- a/test/functional/feature_csv_activation.py +++ b/test/functional/feature_csv_activation.py @@ -95,10 +95,11 @@ class BIP68_112_113Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True # Must set '-dip3params=2000:2000' to create pre-dip3 blocks only self.extra_args = [[ '-peertimeout=999999', # bump because mocktime might cause a disconnect otherwise - '-whitelist=noban@127.0.0.1', f'-testactivationheight=csv@{CSV_ACTIVATION_HEIGHT}', '-par=1', # Use only one script thread to get the exact reject reason for testing '-dip3params=2000:2000', diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index 44428a02fcb0..42fc282b5709 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -48,9 +48,10 @@ def unDERify(tx): class BIP66Test(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [[ f'-testactivationheight=dersig@{DERSIG_HEIGHT}', - '-whitelist=noban@127.0.0.1', '-par=1', # Use only one script thread to get the exact log msg for testing '-dip3params=9000:9000', '-testactivationheight=v20@9000', # due to changes in CbTx diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index a37668e89136..377430cc7e3f 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -124,11 +124,12 @@ def check_estimates(node, fees_seen): class EstimateFeeTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 3 - # Force fSendTrickle to true (via whitelist) + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [ - ["-whitelist=noban@127.0.0.1"], - ["-whitelist=noban@127.0.0.1", "-blockmaxsize=17000"], - ["-whitelist=noban@127.0.0.1", "-blockmaxsize=8000"] + [], + ["-blockmaxsize=17000"], + ["-blockmaxsize=8000"], ] def setup_network(self): diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index fd4564f79583..37e47ed9575c 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -52,8 +52,7 @@ def set_test_params(self): self.num_nodes = 2 self.extra_args = [["-rest", "-blockfilterindex=1"], []] # whitelist peers to speed up tx relay / mempool sync - for args in self.extra_args: - args.append("-whitelist=noban@127.0.0.1") + self.noban_tx_relay = True self.supports_cli = False def test_rest_request( diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index 69e1c6e8399f..a14089fda676 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -103,9 +103,8 @@ class ZMQTest (BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.disable_mocktime = True - # This test isn't testing txn relay/timing, so set whitelist on the - # peers for instant txn relay. This speeds up the test run time 2-3x. - self.extra_args = [["-whitelist=noban@127.0.0.1"]] * self.num_nodes + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.zmq_port_base = p2p_port(self.num_nodes + 1) def skip_test_if_missing_module(self): @@ -143,8 +142,7 @@ def setup_zmq_test(self, services, *, recv_timeout=60, sync_blocks=True, ipv6=Fa socket.setsockopt(zmq.IPV6, 1) subscribers.append(ZMQSubscriber(socket, topic.encode())) - self.restart_node(0, [f"-zmqpub{topic}={address.replace('ipc://', 'unix:')}" for topic, address in services] + - self.extra_args[0]) + self.restart_node(0, [f"-zmqpub{topic}={address.replace('ipc://', 'unix:')}" for topic, address in services]) for i, sub in enumerate(subscribers): sub.socket.connect(services[i][1]) diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index 029be2cdeff6..31d299b1e0b4 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -27,10 +27,11 @@ class MempoolPackagesTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [ [ "-maxorphantxsize=1000", - "-whitelist=noban@127.0.0.1", # immediate tx relay ], [ "-maxorphantxsize=1000", diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 02950e30cabb..61676b45d7e6 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -99,9 +99,10 @@ def merkleblock_received(self, value): class FilterTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [[ '-peerbloomfilters', - '-whitelist=noban@127.0.0.1', # immediate tx relay ]] def generatetoscriptpubkey(self, scriptpubkey): diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py index 1d8217b40289..cf5f108fe3c9 100755 --- a/test/functional/p2p_invalid_block.py +++ b/test/functional/p2p_invalid_block.py @@ -31,7 +31,8 @@ class InvalidBlockRequestTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True - self.extra_args = [["-whitelist=noban@127.0.0.1"]] + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True def run_test(self): # Add p2p connection to node0 diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index 2452e307c4de..5fa4f482bfa0 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -83,7 +83,14 @@ def run_test(self): ["-whitelist=all@127.0.0.1"], ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download", "addr"]) + for flag, permissions in [(["-whitelist=noban,out@127.0.0.1"], ["noban", "download"]), (["-whitelist=noban@127.0.0.1"], [])]: + self.restart_node(0, flag) + self.connect_nodes(0, 1) + peerinfo = self.nodes[0].getpeerinfo()[0] + assert_equal(peerinfo['permissions'], permissions) + self.stop_node(1) + self.nodes[1].assert_start_raises_init_error(["-whitelist=in,out@127.0.0.1"], "Only direction was set, no permissions", match=ErrorMatch.PARTIAL_REGEX) self.nodes[1].assert_start_raises_init_error(["-whitelist=oopsie@127.0.0.1"], "Invalid P2P permission", match=ErrorMatch.PARTIAL_REGEX) self.nodes[1].assert_start_raises_init_error(["-whitelist=noban@127.0.0.1:230"], "Invalid netmask specified in", match=ErrorMatch.PARTIAL_REGEX) self.nodes[1].assert_start_raises_init_error(["-whitebind=noban@127.0.0.1/10"], "Cannot resolve -whitebind address", match=ErrorMatch.PARTIAL_REGEX) diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index cdc189a5e81e..ff4fa01c931b 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -29,7 +29,8 @@ class RPCPackagesTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True - self.extra_args = [["-whitelist=noban@127.0.0.1"]] # noban speeds up tx relay + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True def assert_testres_equal(self, package_hex, testres_expected): """Shuffle package_hex and assert that the testmempoolaccept result matches testres_expected. This should only diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 748f154f49e7..d8ef05cdf656 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -72,9 +72,8 @@ def set_test_params(self): ["-txindex"], ["-notxindex", "-fastprune", "-prune=1"], ] - # whitelist all peers to speed up tx relay / mempool sync - for args in self.extra_args: - args.append("-whitelist=noban@127.0.0.1") + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.supports_cli = False def setup_network(self): diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index ee9bba3e7792..4d3ca339e29f 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -122,6 +122,7 @@ def __init__(self): self.chain: str = 'regtest' self.setup_clean_chain: bool = False self.disable_mocktime: bool = False + self.noban_tx_relay: bool = False self.nodes: List[TestNode] = [] self.extra_args = None self.network_thread = None @@ -547,6 +548,10 @@ def get_bin_from_version(version, bin_name, bin_default): extra_confs = [[]] * num_nodes if extra_args is None: extra_args = [[]] * num_nodes + # Whitelist peers to speed up tx relay / mempool sync. Don't use it if testing tx relay or timing. + if self.noban_tx_relay: + for i in range(len(extra_args)): + extra_args[i] = extra_args[i] + ["-whitelist=noban,in,out@127.0.0.1"] if versions is None: versions = [None] * num_nodes if binary is None: diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index f72f6d1f7fdb..8a589f1aefd8 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -28,8 +28,7 @@ def set_test_params(self): self.num_nodes = 2 self.extra_args = [["-minrelaytxfee=0.00001"], []] # whitelist peers to speed up tx relay / mempool sync - for args in self.extra_args: - args.append("-whitelist=noban@127.0.0.1") + self.noban_tx_relay = True def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_avoidreuse.py b/test/functional/wallet_avoidreuse.py index 1d8bb663f6e6..5dca235a24b8 100755 --- a/test/functional/wallet_avoidreuse.py +++ b/test/functional/wallet_avoidreuse.py @@ -68,9 +68,8 @@ def add_options(self, parser): def set_test_params(self): self.num_nodes = 2 - # This test isn't testing txn relay/timing, so set whitelist on the - # peers for instant txn relay. This speeds up the test run time 2-3x. - self.extra_args = [["-whitelist=noban@127.0.0.1"]] * self.num_nodes + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index a29c10e1bf2b..8f8c2da40ea5 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -46,13 +46,14 @@ def add_options(self, parser): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True - # nodes 1, 2,3 are spenders, let's give them a keypool=100 - # whitelist all peers to speed up tx relay / mempool sync + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True + # nodes 1, 2, 3 are spenders, let's give them a keypool=100 self.extra_args = [ - ["-whitelist=noban@127.0.0.1", "-keypool=100"], - ["-whitelist=noban@127.0.0.1", "-keypool=100"], - ["-whitelist=noban@127.0.0.1", "-keypool=100"], - ["-whitelist=noban@127.0.0.1"], + ["-keypool=100"], + ["-keypool=100"], + ["-keypool=100"], + [], ] self.rpc_timeout = 120 diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 8a9752bf158f..81252afffb9a 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -53,15 +53,14 @@ def add_options(self, parser): def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [ # Limit mempool descendants as a hack to have wallet txs rejected from the mempool. # Set walletrejectlongchains=0 so the wallet still creates the transactions. ['-limitdescendantcount=3', '-walletrejectlongchains=0'], [], ] - # whitelist peers to speed up tx relay / mempool sync - for args in self.extra_args: - args.append("-whitelist=noban@127.0.0.1") def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index bacec6d53ccb..c24ea9b2277f 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -35,13 +35,15 @@ def add_options(self, parser): def set_test_params(self): self.num_nodes = 4 + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True if self.options.descriptors: self.extra_args = [[ - "-dustrelayfee=0", "-walletrejectlongchains=0", "-whitelist=noban@127.0.0.1" + "-dustrelayfee=0", "-walletrejectlongchains=0" ] for i in range(self.num_nodes)] else: self.extra_args = [[ - "-dustrelayfee=0", "-walletrejectlongchains=0", "-whitelist=noban@127.0.0.1", + "-dustrelayfee=0", "-walletrejectlongchains=0", '-usehd={:d}'.format(i%2==0) ] for i in range(self.num_nodes)] self.setup_clean_chain = True diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 5afdf33a5153..f6b5eaf948e3 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -38,9 +38,9 @@ class RawTransactionsTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True - # This test isn't testing tx relay. Set whitelist on the peers for - # instant tx relay. - self.extra_args = [[f'-usehd={not self.options.nohd}', '-whitelist=noban@127.0.0.1']] * self.num_nodes + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True + self.extra_args = [[f'-usehd={not self.options.nohd}']] * self.num_nodes def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index f57d9a6d47e0..a4cb6e98b255 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -21,6 +21,8 @@ def add_options(self, parser): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 5 + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [ [], [], @@ -30,7 +32,6 @@ def set_test_params(self): ] for args in self.extra_args: - args.append("-whitelist=noban@127.0.0.1") # whitelist peers to speed up tx relay / mempool sync args.append(f"-paytxfee={20 * 1e3 / 1e8}") # apply feerate of 20 sats/vB across all nodes self.rpc_timeout = 480 diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 14edc52d944b..2d3d8d77ff89 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -23,8 +23,7 @@ def set_test_params(self): self.num_nodes = 2 self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=0']] # whitelist peers to speed up tx relay / mempool sync - for args in self.extra_args: - args.append("-whitelist=noban@127.0.0.1") + self.noban_tx_relay = True def setup_network(self): self.add_nodes(self.num_nodes, self.extra_args) diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index d469a679748a..e32b817efe8a 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -143,6 +143,8 @@ def set_test_params(self): self.num_nodes = 2 + len(IMPORT_NODES) self.supports_cli = False self.rpc_timeout = 120 + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -161,7 +163,7 @@ def setup_network(self): self.import_deterministic_coinbase_privkeys() self.stop_nodes() - self.start_nodes(extra_args=[["-whitelist=noban@127.0.0.1"]] * self.num_nodes) + self.start_nodes() for i in range(1, self.num_nodes): self.connect_nodes(i, 0) diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index bec0b98b9392..6de638f7140d 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -37,12 +37,11 @@ def add_options(self, parser): def set_test_params(self): self.num_nodes = 2 + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [[], ["-keypool=5"] ] - # whitelist peers to speed up tx relay / mempool sync - for args in self.extra_args: - args.append("-whitelist=noban@127.0.0.1") self.setup_clean_chain = True self.wallet_names = [] diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py index 825420c1b769..156db5d1abcc 100755 --- a/test/functional/wallet_listreceivedby.py +++ b/test/functional/wallet_listreceivedby.py @@ -22,7 +22,7 @@ def add_options(self, parser): def set_test_params(self): self.num_nodes = 2 # whitelist peers to speed up tx relay / mempool sync - self.extra_args = [["-whitelist=noban@127.0.0.1"], ["-whitelist=noban@127.0.0.1"]] + self.noban_tx_relay = True def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 8ca295cd8c52..9a263ed81d52 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -27,7 +27,7 @@ def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = True # whitelist peers to speed up tx relay / mempool sync - self.extra_args = [["-whitelist=noban@127.0.0.1"]] * self.num_nodes + self.noban_tx_relay = True def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 35e47231e91f..eb01057a893f 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -22,9 +22,8 @@ def add_options(self, parser): def set_test_params(self): self.num_nodes = 3 - # This test isn't testing txn relay/timing, so set whitelist on the - # peers for instant txn relay. This speeds up the test run time 2-3x. - self.extra_args = [["-whitelist=noban@127.0.0.1"]] * self.num_nodes + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True def skip_test_if_missing_module(self): self.skip_if_no_wallet() diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 30d413a9a789..047e555d4689 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -25,10 +25,11 @@ def add_options(self, parser): def set_test_params(self): self.num_nodes = 2 - # whitelist all peers to speed up tx relay / mempool sync + # whitelist peers to speed up tx relay / mempool sync + self.noban_tx_relay = True self.extra_args = [ - ["-whitelist=127.0.0.1"], - ["-whitelist=127.0.0.1"], + [], + [], ] getcontext().prec = 8 # Satoshi precision for Decimal From 3b4c5abba244f63adf67bb7fb2f12b6e3a5116f5 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 31 Oct 2026 22:03:25 +0000 Subject: [PATCH 2/3] Merge bitcoin/bitcoin#33395: net: do not apply whitelist permissions to onion inbounds f563ce90818d486d2a199439d2f6ba39cd106352 net: Do not apply whitelist permission to onion inbounds (Martin Zumsande) Pull request description: Tor inbound connections do not reveal the peer's actual network address. Do not apply whitelist permissions to them since address-based matching is ineffective. ACKs for top commit: darosior: ACK f563ce90818d486d2a199439d2f6ba39cd106352 furszy: ACK f563ce90818d486d2a199439d2f6ba39cd106352 vasild: ACK f563ce90818d486d2a199439d2f6ba39cd106352 Tree-SHA512: 49ae70e382fc2f78b7073553fe649a6843a41214b2986ea7f77e285d02b7bd00fe0320a1b71d1aaca08713808fb14af058f0b1f19f19adb3a77b97cb9d3449ce Co-Authored-By: Claude Opus 5.5 --- src/net.cpp | 11 +++++++---- src/net.h | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 3bd224322fcd..255933b51b35 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -552,9 +552,9 @@ void CNode::CloseSocketDisconnect(CConnman* connman) ::g_stats_client->inc("peers.disconnect", 1.0f); } -void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr, const std::vector& ranges) const { +void CConnman::AddWhitelistPermissionFlags(NetPermissionFlags& flags, std::optional addr, const std::vector& ranges) const { for (const auto& subnet : ranges) { - if (subnet.m_subnet.Match(addr)) { + if (addr.has_value() && subnet.m_subnet.Match(addr.value())) { NetPermissions::AddFlag(flags, subnet.m_flags); } } @@ -1902,7 +1902,11 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, int nVerifiedInboundMasternodes = 0; int nMaxInbound = nMaxConnections - m_max_outbound; - AddWhitelistPermissionFlags(permission_flags, addr, vWhitelistedRangeIncoming); + const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end(); + + // Tor inbound connections do not reveal the peer's actual network address. + // Therefore do not apply address-based whitelist permissions to them. + AddWhitelistPermissionFlags(permission_flags, inbound_onion ? std::optional{} : addr, vWhitelistedRangeIncoming); { @@ -1982,7 +1986,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr&& sock, NodeId id = GetNewNodeId(); uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); - const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end(); // The V2Transport transparently falls back to V1 behavior when an incoming V1 connection is // detected, so use it whenever we signal NODE_P2P_V2. const bool use_v2transport(nLocalServices & NODE_P2P_V2); diff --git a/src/net.h b/src/net.h index 29f044631ea2..59463b88af6e 100644 --- a/src/net.h +++ b/src/net.h @@ -1750,7 +1750,7 @@ friend class CNode; std::vector GetEvictionCandidates() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); bool AttemptToEvictConnection() EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fCountFailure, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex, !m_unused_i2p_sessions_mutex); - void AddWhitelistPermissionFlags(NetPermissionFlags& flags, const CNetAddr &addr, const std::vector& ranges) const; + void AddWhitelistPermissionFlags(NetPermissionFlags& flags, std::optional addr, const std::vector& ranges) const; void DeleteNode(CNode* pnode); From ddcfbfc6d3c6b1993b18db06b6a983fb29125f3e Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 31 Oct 2026 21:38:39 +0000 Subject: [PATCH 3/3] test: check that -whitelist does not apply to inbound onion connections A peer on an onion bind is forwarded by Tor and appears to come from the Tor daemon's address, so -whitelist=noban@127.0.0.1 must not grant it noban. Fails without the previous commit. Co-Authored-By: Claude Opus 5.5 --- test/functional/p2p_permissions.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index 5fa4f482bfa0..0bddec876e11 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -10,12 +10,16 @@ from test_framework.messages import ( SEQUENCE_FINAL, ) -from test_framework.p2p import P2PDataStore +from test_framework.p2p import ( + P2PDataStore, + P2PInterface, +) from test_framework.test_node import ErrorMatch from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, p2p_port, + tor_port, ) from test_framework.wallet import MiniWallet @@ -89,6 +93,13 @@ def run_test(self): peerinfo = self.nodes[0].getpeerinfo()[0] assert_equal(peerinfo['permissions'], permissions) + self.log.info("Check that -whitelist does not apply to inbound onion connections") + self.restart_node(1, ["-whitelist=noban@127.0.0.1", f"-bind=127.0.0.1:{tor_port(1)}=onion"]) + self.nodes[1].add_p2p_connection(P2PInterface()) + self.nodes[1].add_p2p_connection(P2PInterface(), dstport=tor_port(1)) + permissions = {peer["network"]: peer["permissions"] for peer in self.nodes[1].getpeerinfo()} + assert_equal(permissions, {"not_publicly_routable": ["noban", "download"], "onion": []}) + self.stop_node(1) self.nodes[1].assert_start_raises_init_error(["-whitelist=in,out@127.0.0.1"], "Only direction was set, no permissions", match=ErrorMatch.PARTIAL_REGEX) self.nodes[1].assert_start_raises_init_error(["-whitelist=oopsie@127.0.0.1"], "Invalid P2P permission", match=ErrorMatch.PARTIAL_REGEX)