node: change a tx-relay on/off flag to enum
What changed, and why it matters
This commit is a straightforward code cleanup: it replaces a true/false 'relay' flag with a named enum so the code is easier to read and future-proof. It does not change what the program actually does—transactions are still added to the mempool and/or broadcast exactly as before. There is no security fix or vulnerability here.
No security action needed. Treat as normal refactoring/code-quality change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors BroadcastTransaction() and related callers to use a new node::TxBroadcast enum (MEMPOOL_AND_BROADCAST_TO_ALL, MEMPOOL_NO_BROADCAST) instead of a bool relay parameter. All call sites are mechanically updated with equivalent values: true becomes MEMPOOL_AND_BROADCAST_TO_ALL and false becomes MEMPOOL_NO_BROADCAST. The control flow in BroadcastTransaction() is restructured with switch statements but remains semantically identical. A new header node/types.h gains the enum definition. No behavior changes, no bug fixes, and no security boundary changes are present in the diff.
Changed components
src/node/transaction.cppsrc/node/transaction.hsrc/node/types.hsrc/interfaces/chain.hsrc/node/interfaces.cppsrc/rpc/mempool.cppsrc/wallet/wallet.cppsrc/wallet/wallet.hsrc/wallet/rpc/backup.cppsrc/wallet/test/wallet_tests.cppInspect captured patch +116 / −44
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index 34ab3631..2b1d953e 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -7,6 +7,7 @@
#include <blockfilter.h>
#include <common/settings.h>
+#include <node/types.h>
#include <primitives/transaction.h>
#include <util/result.h>
@@ -206,13 +207,19 @@ public:
//! Check if transaction has descendants in mempool.
virtual bool hasDescendantsInMempool(const Txid& txid) = 0;
- //! Transaction is added to memory pool, if the transaction fee is below the
- //! amount specified by max_tx_fee, and broadcast to all peers if relay is set to true.
- //! Return false if the transaction could not be added due to the fee or for another reason.
+ //! Process a local transaction, optionally adding it to the mempool and
+ //! optionally broadcasting it to the network.
+ //! @param[in] tx Transaction to process.
+ //! @param[in] max_tx_fee Don't add the transaction to the mempool or
+ //! broadcast it if its fee is higher than this.
+ //! @param[in] broadcast_method Whether to add the transaction to the
+ //! mempool and how/whether to broadcast it.
+ //! @param[out] err_string Set if an error occurs.
+ //! @return False if the transaction could not be added due to the fee or for another reason.
virtual bool broadcastTransaction(const CTransactionRef& tx,
- const CAmount& max_tx_fee,
- bool relay,
- std::string& err_string) = 0;
+ const CAmount& max_tx_fee,
+ node::TxBroadcast broadcast_method,
+ std::string& err_string) = 0;
//! Calculate mempool ancestor and descendant counts for the given transaction.
virtual void getTransactionAncestry(const Txid& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) = 0;
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index fd3fa226..6e5a3896 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -363,7 +363,12 @@ public:
}
TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, std::string& err_string) override
{
- return BroadcastTransaction(*m_context, std::move(tx), err_string, max_tx_fee, /*relay=*/ true, /*wait_callback=*/ false);
+ return BroadcastTransaction(*m_context,
+ std::move(tx),
+ err_string,
+ max_tx_fee,
+ TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL,
+ /*wait_callback=*/false);
}
WalletLoader& walletLoader() override
{
@@ -672,10 +677,10 @@ public:
}
bool broadcastTransaction(const CTransactionRef& tx,
const CAmount& max_tx_fee,
- bool relay,
+ TxBroadcast broadcast_method,
std::string& err_string) override
{
- const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, relay, /*wait_callback=*/false);
+ const TransactionError err = BroadcastTransaction(m_node, tx, err_string, max_tx_fee, broadcast_method, /*wait_callback=*/false);
// Chain clients only care about failures to accept the tx to the mempool. Disregard non-mempool related failures.
// Note: this will need to be updated if BroadcastTransactions() is updated to return other non-mempool failures
// that Chain clients do not need to know about.
diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
index 7bb92872..34ac01aa 100644
--- a/src/node/transaction.cpp
+++ b/src/node/transaction.cpp
@@ -31,7 +31,12 @@ static TransactionError HandleATMPError(const TxValidationState& state, std::str
}
}
-TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback)
+TransactionError BroadcastTransaction(NodeContext& node,
+ const CTransactionRef tx,
+ std::string& err_string,
+ const CAmount& max_tx_fee,
+ TxBroadcast broadcast_method,
+ bool wait_callback)
{
// BroadcastTransaction can be called by RPC or by the wallet.
// chainman, mempool and peerman are initialized before the RPC server and wallet are started
@@ -62,7 +67,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
// There's already a transaction in the mempool with this txid. Don't
// try to submit this transaction to the mempool (since it'll be
// rejected as a TX_CONFLICT), but do attempt to reannounce the mempool
- // transaction if relay=true.
+ // transaction if broadcast_method is not TxBroadcast::MEMPOOL_NO_BROADCAST.
//
// The mempool transaction may have the same or different witness (and
// wtxid) as this transaction. Use the mempool's wtxid for reannouncement.
@@ -79,18 +84,26 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
return TransactionError::MAX_FEE_EXCEEDED;
}
}
- // Try to submit the transaction to the mempool.
- const MempoolAcceptResult result = node.chainman->ProcessTransaction(tx, /*test_accept=*/ false);
- if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
- return HandleATMPError(result.m_state, err_string);
- }
- // Transaction was accepted to the mempool.
+ switch (broadcast_method) {
+ case TxBroadcast::MEMPOOL_NO_BROADCAST:
+ case TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL:
+ // Try to submit the transaction to the mempool.
+ {
+ const MempoolAcceptResult result =
+ node.chainman->ProcessTransaction(tx, /*test_accept=*/false);
+ if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
+ return HandleATMPError(result.m_state, err_string);
+ }
+ }
+ // Transaction was accepted to the mempool.
- if (relay) {
- // the mempool tracks locally submitted transactions to make a
- // best-effort of initial broadcast
- node.mempool->AddUnbroadcastTx(txid);
+ if (broadcast_method == TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL) {
+ // the mempool tracks locally submitted transactions to make a
+ // best-effort of initial broadcast
+ node.mempool->AddUnbroadcastTx(txid);
+ }
+ break;
}
if (wait_callback && node.validation_signals) {
@@ -116,8 +129,12 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
promise.get_future().wait();
}
- if (relay) {
+ switch (broadcast_method) {
+ case TxBroadcast::MEMPOOL_NO_BROADCAST:
+ break;
+ case TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL:
node.peerman->RelayTransaction(txid, wtxid);
+ break;
}
return TransactionError::OK;
diff --git a/src/node/transaction.h b/src/node/transaction.h
index 2a411581..df30d7f1 100644
--- a/src/node/transaction.h
+++ b/src/node/transaction.h
@@ -6,6 +6,7 @@
#define BITCOIN_NODE_TRANSACTION_H
#include <common/messages.h>
+#include <node/types.h>
#include <policy/feerate.h>
#include <primitives/transaction.h>
@@ -45,11 +46,16 @@ static const CAmount DEFAULT_MAX_BURN_AMOUNT{0};
* @param[in] tx the transaction to broadcast
* @param[out] err_string reference to std::string to fill with error string if available
* @param[in] max_tx_fee reject txs with fees higher than this (if 0, accept any fee)
- * @param[in] relay flag if both mempool insertion and p2p relay are requested
+ * @param[in] broadcast_method whether to add the transaction to the mempool and how to broadcast it
* @param[in] wait_callback wait until callbacks have been processed to avoid stale result due to a sequentially RPC.
* return error
*/
-[[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node, CTransactionRef tx, std::string& err_string, const CAmount& max_tx_fee, bool relay, bool wait_callback);
+[[nodiscard]] TransactionError BroadcastTransaction(NodeContext& node,
+ CTransactionRef tx,
+ std::string& err_string,
+ const CAmount& max_tx_fee,
+ TxBroadcast broadcast_method,
+ bool wait_callback);
/**
* Return transaction with a given hash.
diff --git a/src/node/types.h b/src/node/types.h
index 547d6448..6c268762 100644
--- a/src/node/types.h
+++ b/src/node/types.h
@@ -15,6 +15,7 @@
#include <consensus/amount.h>
#include <cstddef>
+#include <cstdint>
#include <policy/policy.h>
#include <script/script.h>
#include <uint256.h>
@@ -97,6 +98,18 @@ struct BlockCheckOptions {
*/
bool check_pow{true};
};
+
+/**
+ * How to broadcast a local transaction.
+ * Used to influence `BroadcastTransaction()` and its callers.
+ */
+enum class TxBroadcast : uint8_t {
+ /// Add the transaction to the mempool and broadcast to all peers for which tx relay is enabled.
+ MEMPOOL_AND_BROADCAST_TO_ALL,
+ /// Add the transaction to the mempool, but don't broadcast to anybody.
+ MEMPOOL_NO_BROADCAST,
+};
+
} // namespace node
#endif // BITCOIN_NODE_TYPES_H
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 147af369..408b5cde 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -97,7 +97,12 @@ static RPCHelpMan sendrawtransaction()
std::string err_string;
AssertLockNotHeld(cs_main);
NodeContext& node = EnsureAnyNodeContext(request.context);
- const TransactionError err = BroadcastTransaction(node, tx, err_string, max_raw_tx_fee, /*relay=*/true, /*wait_callback=*/true);
+ const TransactionError err = BroadcastTransaction(node,
+ tx,
+ err_string,
+ max_raw_tx_fee,
+ node::TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL,
+ /*wait_callback=*/true);
if (TransactionError::OK != err) {
throw JSONRPCTransactionError(err, err_string);
}
@@ -1066,7 +1071,12 @@ static RPCHelpMan submitpackage()
// We do not expect an error here; we are only broadcasting things already/still in mempool
std::string err_string;
- const auto err = BroadcastTransaction(node, tx, err_string, /*max_tx_fee=*/0, /*relay=*/true, /*wait_callback=*/true);
+ const auto err = BroadcastTransaction(node,
+ tx,
+ err_string,
+ /*max_tx_fee=*/0,
+ node::TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL,
+ /*wait_callback=*/true);
if (err != TransactionError::OK) {
throw JSONRPCTransactionError(err,
strprintf("transaction broadcast failed: %s (%d transactions were broadcast successfully)",
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index e8109214..9edbeafb 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -9,6 +9,7 @@
#include <interfaces/chain.h>
#include <key_io.h>
#include <merkleblock.h>
+#include <node/types.h>
#include <rpc/util.h>
#include <script/descriptor.h>
#include <script/script.h>
@@ -400,7 +401,7 @@ RPCHelpMan importdescriptors()
// Rescan the blockchain using the lowest timestamp
if (rescan) {
int64_t scanned_time = pwallet->RescanFromTime(lowest_timestamp, reserver, /*update=*/true);
- pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true);
+ pwallet->ResubmitWalletTransactions(node::TxBroadcast::MEMPOOL_NO_BROADCAST, /*force=*/true);
if (pwallet->IsAbortingRescan()) {
throw JSONRPCError(RPC_MISC_ERROR, "Rescan aborted by user.");
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index bcff2593..a4cbb1e1 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -13,6 +13,7 @@
#include <interfaces/chain.h>
#include <key_io.h>
#include <node/blockstorage.h>
+#include <node/types.h>
#include <policy/policy.h>
#include <rpc/server.h>
#include <script/solver.h>
@@ -618,7 +619,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
auto block_tx = TestSimpleSpend(*m_coinbase_txns[0], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
auto mempool_tx = TestSimpleSpend(*m_coinbase_txns[1], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
- BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
+ BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, node::TxBroadcast::MEMPOOL_NO_BROADCAST, error));
// Reload wallet and make sure new transactions are detected despite events
@@ -660,7 +661,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup)
block_tx = TestSimpleSpend(*m_coinbase_txns[2], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
mempool_tx = TestSimpleSpend(*m_coinbase_txns[3], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
- BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
+ BOOST_CHECK(m_node.chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, node::TxBroadcast::MEMPOOL_NO_BROADCAST, error));
m_node.validation_signals->SyncWithValidationInterfaceQueue();
});
wallet = TestLoadWallet(context);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index d08d6782..86474a45 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1959,7 +1959,9 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
return result;
}
-bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
+bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx,
+ std::string& err_string,
+ node::TxBroadcast broadcast_method) const
{
AssertLockHeld(cs_wallet);
@@ -1973,8 +1975,16 @@ bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string
// Don't try to submit conflicted or confirmed transactions.
if (GetTxDepthInMainChain(wtx) != 0) return false;
- // Submit transaction to mempool for relay
- WalletLogPrintf("Submitting wtx %s to mempool for relay\n", wtx.GetHash().ToString());
+ const char* what{""};
+ switch (broadcast_method) {
+ case node::TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL:
+ what = "to mempool and for broadcast to peers";
+ break;
+ case node::TxBroadcast::MEMPOOL_NO_BROADCAST:
+ what = "to mempool without broadcast";
+ break;
+ }
+ WalletLogPrintf("Submitting wtx %s %s\n", wtx.GetHash().ToString(), what);
// We must set TxStateInMempool here. Even though it will also be set later by the
// entered-mempool callback, if we did not there would be a race where a
// user could call sendmoney in a loop and hit spurious out of funds errors
@@ -1984,7 +1994,7 @@ bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string
// If broadcast fails for any reason, trying to set wtx.m_state here would be incorrect.
// If transaction was previously in the mempool, it should be updated when
// TransactionRemovedFromMempool fires.
- bool ret = chain().broadcastTransaction(wtx.tx, m_default_max_tx_fee, relay, err_string);
+ bool ret = chain().broadcastTransaction(wtx.tx, m_default_max_tx_fee, broadcast_method, err_string);
if (ret) wtx.m_state = TxStateInMempool{};
return ret;
}
@@ -2039,10 +2049,11 @@ NodeClock::time_point CWallet::GetDefaultNextResend() { return FastRandomContext
//
// The `force` option results in all unconfirmed transactions being submitted to
// the mempool. This does not necessarily result in those transactions being relayed,
-// that depends on the `relay` option. Periodic rebroadcast uses the pattern
-// relay=true force=false, while loading into the mempool
-// (on start, or after import) uses relay=false force=true.
-void CWallet::ResubmitWalletTransactions(bool relay, bool force)
+// that depends on the `broadcast_method` option. Periodic rebroadcast uses the pattern
+// broadcast_method=TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL force=false, while loading into
+// the mempool (on start, or after import) uses
+// broadcast_method=TxBroadcast::MEMPOOL_NO_BROADCAST force=true.
+void CWallet::ResubmitWalletTransactions(node::TxBroadcast broadcast_method, bool force)
{
// Don't attempt to resubmit if the wallet is configured to not broadcast,
// even if forcing.
@@ -2068,7 +2079,7 @@ void CWallet::ResubmitWalletTransactions(bool relay, bool force)
// Now try submitting the transactions to the memory pool and (optionally) relay them.
for (auto wtx : to_submit) {
std::string unused_err_string;
- if (SubmitTxMemoryPoolAndRelay(*wtx, unused_err_string, relay)) ++submitted_tx_count;
+ if (SubmitTxMemoryPoolAndRelay(*wtx, unused_err_string, broadcast_method)) ++submitted_tx_count;
}
} // cs_wallet
@@ -2083,7 +2094,7 @@ void MaybeResendWalletTxs(WalletContext& context)
{
for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) {
if (!pwallet->ShouldResend()) continue;
- pwallet->ResubmitWalletTransactions(/*relay=*/true, /*force=*/false);
+ pwallet->ResubmitWalletTransactions(node::TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL, /*force=*/false);
pwallet->SetNextResend();
}
}
@@ -2284,7 +2295,7 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
}
std::string err_string;
- if (!SubmitTxMemoryPoolAndRelay(*wtx, err_string, true)) {
+ if (!SubmitTxMemoryPoolAndRelay(*wtx, err_string, node::TxBroadcast::MEMPOOL_AND_BROADCAST_TO_ALL)) {
WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string);
// TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
}
@@ -3233,7 +3244,7 @@ void CWallet::postInitProcess()
{
// Add wallet transactions that aren't already in a block to mempool
// Do this here as mempool requires genesis block to be loaded
- ResubmitWalletTransactions(/*relay=*/false, /*force=*/true);
+ ResubmitWalletTransactions(node::TxBroadcast::MEMPOOL_NO_BROADCAST, /*force=*/true);
// Update wallet transactions with current mempool transactions.
WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this));
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index b341ac6d..275d4ead 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -12,6 +12,7 @@
#include <interfaces/handler.h>
#include <kernel/cs_main.h>
#include <logging.h>
+#include <node/types.h>
#include <outputtype.h>
#include <policy/feerate.h>
#include <primitives/transaction.h>
@@ -654,7 +655,7 @@ public:
void SetNextResend() { m_next_resend = GetDefaultNextResend(); }
/** Return true if all conditions for periodically resending transactions are met. */
bool ShouldResend() const;
- void ResubmitWalletTransactions(bool relay, bool force);
+ void ResubmitWalletTransactions(node::TxBroadcast broadcast_method, bool force);
OutputType TransactionChangeType(const std::optional<OutputType>& change_type, const std::vector<CRecipient>& vecSend) const;
@@ -698,8 +699,8 @@ public:
*/
void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm);
- /** Pass this transaction to node for mempool insertion and relay to peers if flag set to true */
- bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
+ /** Pass this transaction to node for optional mempool insertion and relay to peers. */
+ bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, node::TxBroadcast broadcast_method) const
EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/** Updates wallet birth time if 'time' is below it */
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.