net_processing: randomize start time of private broadcast

src/init.cpp

@@ -679,6 +679,11 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
                    DEFAULT_PRIVATE_BROADCAST),
                    ArgsManager::ALLOW_ANY,
                    OptionsCategory::NODE_RELAY);
+    argsman.AddArg("-privatebroadcastdelay=<n>",
+                   strprintf("Maximum random delay in milliseconds before starting private broadcast for transactions submitted via sendrawtransaction (default: %u). Set to 0 to disable.",
+                             Ticks<std::chrono::milliseconds>(DEFAULT_PRIVATE_BROADCAST_DELAY_MAX)),
+                   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);
 

src/net.h

@@ -87,6 +87,8 @@ static const bool DEFAULT_BLOCKSONLY = false;
 static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60;
 /** Default for -privatebroadcast. */
 static constexpr bool DEFAULT_PRIVATE_BROADCAST{false};
+/** Default for -privatebroadcastdelay. */
+static constexpr auto DEFAULT_PRIVATE_BROADCAST_DELAY_MAX{5s};
 /** Number of file descriptors required for message capture **/
 static const int NUM_FDS_MESSAGE_CAPTURE = 1;
 /** Interval for ASMap Health Check **/

src/net_processing.cpp

@@ -1089,6 +1089,8 @@ class PeerManagerImpl final : public PeerManager
 
     void LogBlockHeader(const CBlockIndex& index, const CNode& peer, bool via_compact_block);
 
+    CScheduler* m_scheduler{nullptr};
+
     /// The transactions to be broadcast privately.
     PrivateBroadcast m_tx_for_private_broadcast;
 };
@@ -1996,6 +1998,8 @@ PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman,
 
 void PeerManagerImpl::StartScheduledTasks(CScheduler& scheduler)
 {
+    m_scheduler = &scheduler;
+
     // Stale tip checking and peer eviction are on two different timers, but we
     // don't want them to get out of sync due to drift in the scheduler, so we
     // combine them in one function and schedule at the quicker (peer-eviction)
@@ -2239,8 +2243,22 @@ void PeerManagerImpl::InitiateTxBroadcastPrivate(const CTransactionRef& tx)
 {
     const auto txstr{strprintf("txid=%s, wtxid=%s", tx->GetHash().ToString(), tx->GetWitnessHash().ToString())};
     if (m_tx_for_private_broadcast.Add(tx)) {
-        LogDebug(BCLog::PRIVBROADCAST, "Requesting %d new connections due to %s", NUM_PRIVATE_BROADCAST_PER_TX, txstr);
-        m_connman.m_private_broadcast.NumToOpenAdd(NUM_PRIVATE_BROADCAST_PER_TX);
+        if (m_opts.private_broadcast_delay_max > 0ms && m_scheduler != nullptr) {
+            const auto delay{FastRandomContext{m_opts.deterministic_rng}.randrange<std::chrono::milliseconds>(m_opts.private_broadcast_delay_max + 1ms)};
+            LogDebug(BCLog::PRIVBROADCAST,
+                     "Scheduling %d new connections in %dms due to %s",
+                     NUM_PRIVATE_BROADCAST_PER_TX, count_milliseconds(delay), txstr);
+            m_scheduler->scheduleFromNow(
+                [this] {
+                    if (m_tx_for_private_broadcast.HavePendingTransactions()) {
+                        m_connman.m_private_broadcast.NumToOpenAdd(NUM_PRIVATE_BROADCAST_PER_TX);
+                    }
+                },
+                delay);
+        } else {
+            LogDebug(BCLog::PRIVBROADCAST, "Requesting %d new connections due to %s", NUM_PRIVATE_BROADCAST_PER_TX, txstr);
+            m_connman.m_private_broadcast.NumToOpenAdd(NUM_PRIVATE_BROADCAST_PER_TX);
+        }
     } else {
         LogDebug(BCLog::PRIVBROADCAST, "Ignoring unnecessary request to schedule an already scheduled transaction: %s", txstr);
     }

src/net_processing.h

@@ -92,6 +92,8 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
         uint32_t max_headers_result{MAX_HEADERS_RESULTS};
         //! Whether private broadcast is used for sending transactions.
         bool private_broadcast{DEFAULT_PRIVATE_BROADCAST};
+        //! Maximum random delay before starting private broadcast for transactions submitted via sendrawtransaction.
+        std::chrono::milliseconds private_broadcast_delay_max{DEFAULT_PRIVATE_BROADCAST_DELAY_MAX};
     };
 
     static std::unique_ptr<PeerManager> make(CConnman& connman, AddrMan& addrman,

src/node/peerman_args.cpp

@@ -25,7 +25,10 @@ void ApplyArgsManOptions(const ArgsManager& argsman, PeerManager::Options& optio
     if (auto value{argsman.GetBoolArg("-blocksonly")}) options.ignore_incoming_txs = *value;
 
     if (auto value{argsman.GetBoolArg("-privatebroadcast")}) options.private_broadcast = *value;
+
+    if (auto value{argsman.GetIntArg("-privatebroadcastdelay")}) {
+        options.private_broadcast_delay_max = std::chrono::milliseconds{std::max<int64_t>(*value, 0)};
+    }
 }
 
 } // namespace node
-

src/test/peerman_tests.cpp

@@ -6,11 +6,15 @@
 #include <node/miner.h>
 #include <net_processing.h>
 #include <pow.h>
+#include <primitives/transaction.h>
+#include <scheduler.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
 
 #include <boost/test/unit_test.hpp>
 
+#include <thread>
+
 BOOST_FIXTURE_TEST_SUITE(peerman_tests, RegTestingSetup)
 
 /** Window, in blocks, for connecting to NODE_NETWORK_LIMITED peers */
@@ -73,4 +77,31 @@ BOOST_AUTO_TEST_CASE(connections_desirable_service_flags)
     BOOST_CHECK(peerman->GetDesirableServiceFlags(peer_flags) == ServiceFlags(NODE_NETWORK | NODE_WITNESS));
 }
 
+BOOST_AUTO_TEST_CASE(private_broadcast_delays_connection_requests)
+{
+    PeerManager::Options opts;
+    opts.deterministic_rng = true;
+    opts.private_broadcast = false;
+    opts.private_broadcast_delay_max = std::chrono::milliseconds{5000};
+
+    std::unique_ptr<PeerManager> peerman = PeerManager::make(*m_node.connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, *m_node.warnings, opts);
+
+    CScheduler scheduler;
+    peerman->StartScheduledTasks(scheduler);
+
+    const CTransactionRef tx{MakeTransactionRef(CMutableTransaction{})};
+
+    BOOST_CHECK_EQUAL(m_node.connman->m_private_broadcast.NumToOpen(), 0U);
+    peerman->InitiateTxBroadcastPrivate(tx);
+    BOOST_CHECK_EQUAL(m_node.connman->m_private_broadcast.NumToOpen(), 0U);
+
+    std::thread scheduler_thread([&] { scheduler.serviceQueue(); });
+    scheduler.MockForward(std::chrono::duration_cast<std::chrono::seconds>(opts.private_broadcast_delay_max));
+    // Ensure the scheduler has time to process all tasks queued before now.
+    scheduler.scheduleFromNow([&scheduler] { scheduler.stop(); }, std::chrono::milliseconds{1});
+    scheduler_thread.join();
+
+    BOOST_CHECK_EQUAL(m_node.connman->m_private_broadcast.NumToOpen(), 3U);
+}
+
 BOOST_AUTO_TEST_SUITE_END()

test/functional/p2p_private_broadcast.py

@@ -246,6 +246,7 @@ def on_listen_done(addr, port):
                 "-v2transport=0",
                 "-test=addrman",
                 "-privatebroadcast",
+                "-privatebroadcastdelay=0",
                 f"-proxy={socks5_server_config.addr[0]}:{socks5_server_config.addr[1]}",
                 # To increase coverage, make it think that the I2P network is reachable so that it
                 # selects such addresses as well. Pick a proxy address where nobody is listening