node: change a tx-relay on/off flag to enum

vasild commented at 3:11 pm on October 7, 2025: contributor

Previously the bool relay argument to BroadcastTransaction() designated:

0relay=true: add to the mempool and broadcast to all peers
1relay=false: add to the mempool

Change this to an enum, so it is more readable and easier to extend with a 3rd option. Consider these example call sites:

0Paint(true);
1// Or
2Paint(/*is_red=*/true);

vs

0Paint(RED);

The idea for putting TxBroadcastMethod into node/types.h by Ryan.

This is part of #29415 Broadcast own transactions only via short-lived Tor or I2P connections. Putting it in its own PR to reduce the size of #29415 and because it does not logically depend on the other commits from there.

node: change a tx-relay on/off flag to enum

Previously the `bool relay` argument to `BroadcastTransaction()`
designated:

```
relay=true: add to the mempool and broadcast to all peers
relay=false: add to the mempool
```

Change this to an `enum`, so it is more readable and easier to extend
with a 3rd option. Consider these example call sites:

```cpp
Paint(true);
// Or
Paint(/*is_red=*/true);
```

vs

```cpp
Paint(RED);
```

The idea for putting `TxBroadcastMethod` into `node/types.h` by Ryan.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

1197919eab

DrahtBot commented at 3:11 pm on October 7, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33567.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33565 (net_processing: rename RelayTransaction to better describe what it does by vasild)
#29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
#29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)
#27865 (wallet: Track no-longer-spendable TXOs separately by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

and/if -> if/how [clarifies intent: “and/if” is not valid phrasing; “if/how” or “and, if so, how” conveys whether to add to mempool and, if so, how to broadcast]

drahtbot_id_5_m

glozow added the label Refactoring on Oct 7, 2025

in src/interfaces/chain.h:216 in 1197919eab

214+    //! optionally broadcasting it to the network.
215+    //! @param[in] tx Transaction to process.
216+    //! @param[in] max_tx_fee Don't add the transaction to the mempool or
217+    //! broadcast it if its fee is higher than this.
218+    //! @param[in] broadcast_method Whether to add the transaction to the
219+    //! mempool and how/whether to broadcast it.

andrewtoth commented at 4:11 pm on October 9, 2025:

This parameter does not allow the caller to decide whether to add the transaction to the mempool (yet).

in src/interfaces/chain.h:210 in 1197919eab

206@@ -206,13 +207,19 @@ class Chain
207     //! Check if transaction has descendants in mempool.
208     virtual bool hasDescendantsInMempool(const Txid& txid) = 0;
209 
210-    //! Transaction is added to memory pool, if the transaction fee is below the
211-    //! amount specified by max_tx_fee, and broadcast to all peers if relay is set to true.
212-    //! Return false if the transaction could not be added due to the fee or for another reason.
213+    //! Consume a local transaction, optionally adding it to the mempool and

andrewtoth commented at 4:17 pm on October 9, 2025:

The transaction is passed by const ref, so it’s not really “consuming” it. Perhaps “Process a local transaction”?

in src/wallet/wallet.h:702 in 1197919eab

698@@ -698,8 +699,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
699      */
700     void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm);
701 
702-    /** Pass this transaction to node for mempool insertion and relay to peers if flag set to true */
703-    bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const
704+    /** Pass this transaction to node for optional mempool insertion and relay to peers. */

andrewtoth commented at 5:03 pm on October 9, 2025:

0    /** Pass this transaction to node for mempool insertion and optional relay to peers. */

in src/node/transaction.h:49 in 1197919eab

45@@ -45,11 +46,16 @@ static const CAmount DEFAULT_MAX_BURN_AMOUNT{0};
46  * @param[in]  tx the transaction to broadcast
47  * @param[out] err_string reference to std::string to fill with error string if available
48  * @param[in]  max_tx_fee reject txs with fees higher than this (if 0, accept any fee)
49- * @param[in]  relay flag if both mempool insertion and p2p relay are requested
50+ * @param[in]  broadcast_method whether to add the transaction to the mempool and/if how to broadcast it

andrewtoth commented at 5:05 pm on October 9, 2025:

0 * [@param](/bitcoin-bitcoin/contributor/param/)[in]  broadcast_method whether broadcast the transaction after inserting it into the mempool

andrewtoth commented at 5:06 pm on October 9, 2025: contributor

Not sure we should add comments (yet) that suggest adding to the mempool is optional. With this change it is still always added unless it already exists. In the change for private broadcast there are a few comments that can be updated.

in src/node/types.h:106 in 1197919eab

 97@@ -97,6 +98,18 @@ struct BlockCheckOptions {
 98      */
 99     bool check_pow{true};
100 };
101+
102+/**
103+ * Methods to broadcast a local transaction.
104+ * Used to influence `BroadcastTransaction()` and its callers.
105+ */
106+enum TxBroadcastMethod : uint8_t {

luke-jr commented at 2:17 am on October 10, 2025:

Probably should use class enums for new things?

bitcoin deleted a comment on Oct 10, 2025

node: change a tx-relay on/off flag to enum #33567

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)