wallet: mark bip125-replaceable RPC key and walletrbf startup option as deprecated #34917

rkrux commented at 9:53 AM on March 25, 2026: contributor

Partially fixes #32661.

This patch set is in line with the deprecation of outdated BIP 125 opt-in RBF signalling and fullrbf in wallet transaction RPCs and startup options.

DrahtBot added the label Wallet on Mar 25, 2026

DrahtBot commented at 9:53 AM on March 25, 2026: 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/34917.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	w0xlt, polespinasa, achow101

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33034 (wallet: Store transactions in a separate sqlite table 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:

Using this options emits -> Using this option emits [“options” should be singular to match the referenced startup option.]

in src/wallet/init.cpp:77 in 8c0a7d6f14 outdated

  73 | @@ -74,8 +74,6 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
  74 |  #if HAVE_SYSTEM
  75 |      argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
  76 |  #endif
  77 | -    argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);

w0xlt commented at 5:48 PM on March 25, 2026:

Removing the -walletrbf registration introduces an upgrade regression: existing configs or service arguments that still set it will now prevent the node from starting. Shouldn't it remain accepted as a hidden/deprecated no-op rather than being removed outright ?

rkrux commented at 3:07 PM on May 11, 2026:

Hmm yeah, I think making the -walletrbf argument go through the deprecatedrpc cycle could be an alternative like we did for the related rbf fields in #34911.

polespinasa commented at 9:28 AM on May 14, 2026:

You can see https://github.com/bitcoin/bitcoin/pull/31278/changes/bf194c920cf768d1339d41aef1441a78e2f5fcbe#diff-f807f97d1de845ebccc36c08e68869abcfe37478871194d5d4beb9a29047145aR69 how other wallet args where deprecated in the past.

rkrux commented at 1:27 PM on May 20, 2026:

Deprecated the walletrbf option instead.

DrahtBot added the label Needs rebase on Apr 23, 2026

rkrux force-pushed on May 11, 2026

rkrux commented at 3:02 PM on May 11, 2026: contributor

Rebased over master to incorporate changes from #21283 and #33671.

in src/wallet/wallet.h:130 in dc578445de

 126 | @@ -127,7 +127,6 @@ static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true;
 127 |  static const bool DEFAULT_WALLET_REJECT_LONG_CHAINS{true};
 128 |  //! -txconfirmtarget default
 129 |  static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6;
 130 | -//! -walletrbf default
 131 |  static const bool DEFAULT_WALLET_RBF = true;

optout21 commented at 3:21 PM on May 11, 2026:

Are these still needed: m_signal_rbf, DEFAULT_WALLET_RBF, m_signal_bip125_rbf ?

rkrux commented at 1:27 PM on May 20, 2026:

Yes, as per the new approach #34917 (review).

DrahtBot removed the label Needs rebase on May 11, 2026

DrahtBot added the label CI failed on May 11, 2026

polespinasa commented at 9:28 AM on May 14, 2026: member

Concept ACK

Will review soon. This will need a release note indicating all the deprecated fields and startup options.

DrahtBot added the label Needs rebase on May 19, 2026

rkrux force-pushed on May 20, 2026

rkrux commented at 1:26 PM on May 20, 2026: contributor

Rebased over master to incorporate changes from #35279 and changed the approach to deprecate the walletrbf startup option instead of removing it outright as highlighted in #34917 (review).

rkrux renamed this:
~~wallet: mark bip125-replaceable deprecated, remove walletrbf argument~~
wallet: mark bip125-replaceable RPC key and walletrbf startup option as deprecated
on May 20, 2026

rkrux force-pushed on May 20, 2026

DrahtBot removed the label Needs rebase on May 20, 2026

rkrux commented at 2:42 PM on May 20, 2026: contributor

Interesting compilation error that happens in non-mac envs, that's why worked in my local: https://github.com/bitcoin/bitcoin/actions/runs/26166359698/job/76975610926?pr=34917

wallet: mark `bip125-replaceable` key as deprecated in transaction RPCs

Transactions are replaceable by default since v28 and the corresponding
tweaking argument has been removed since v29. The related key from the
mempool RPC has been marked deprecated as well since v29, this patch
does the same for the wallet transaction RPCs such as listtransactions,
listsinceblock, and gettransaction.

Users can pass the -deprecatedrpc=bip125 startup argument to retrieve
this key in the responses of above mentioned RPCs.

c4a7613e6a

wallet: mark -walletrbf startup option as deprecated

All transactions are by default replaceable since v28, the wallet need not have
a configuration option to opt into RBF signalling because it seems redundant
now. Emit a warning if this option is used.

97f7cc0233

rkrux force-pushed on May 21, 2026

DrahtBot removed the label CI failed on May 21, 2026

wallet: remove "RPC Only" from -walletrbf option help description

Effective post https://github.com/bitcoin-core/gui/pull/936.

Co-authored-by: Pol Espinasa <pol.espinasa@uab.cat>

aba24a9b62

doc: add release notes for deprecation of wallet rbf & bip125 fields 5faf2ad880

in src/wallet/init.cpp:77 in 751d4c124e

  73 | @@ -74,7 +74,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
  74 |  #if HAVE_SYSTEM
  75 |      argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
  76 |  #endif
  77 | -    argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
  78 | +    argsman.AddArg("-walletrbf", strprintf("(DEPRECATED) Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);

polespinasa commented at 11:44 AM on May 22, 2026:

With https://github.com/bitcoin-core/gui/pull/936 merged, this is no longer (RPC only). Probably worth to update here as it is being touched anyway and it is RBF related.

rkrux commented at 2:49 PM on May 22, 2026:

Done, added you as co-author.

rkrux force-pushed on May 22, 2026

w0xlt commented at 9:04 AM on May 23, 2026: contributor

ACK 5faf2ad88029fe70d7fc8d3e21ba93739a2e250a

DrahtBot requested review from polespinasa on May 23, 2026

in doc/release-notes-34917.md:8 in 5faf2ad880

   0 | @@ -0,0 +1,9 @@
   1 | +RPC and Startup Option
   2 | +------------
   3 | +The `bip125-replaceable` key in the wallet transaction RPCs such
   4 | +as `listtransactions`, `listsinceblock`, and `gettransaction` is
   5 | +marked as deprecated. Users still have the option to retrieve this
   6 | +key by passing the `-deprecatedrpc=bip125` startup option. Also,
   7 | +the `-walletrbf` startup option has been marked as deprecated and
   8 | +will be fully removed in the next release. Using this options emits

polespinasa commented at 2:59 PM on May 25, 2026:

feel-free-to-ignore-nit: Maybe worth explicitly saying the next release number. I guess we can get this merged for v32.0. So removed by v33.0?

maflcko commented at 8:48 AM on May 26, 2026:

from the llm:

Using this options emits -> Using this option emits [“options” should be singular to match the referenced startup option.]

polespinasa commented at 3:02 PM on May 25, 2026: member

code reviewed ACK 5faf2ad88029fe70d7fc8d3e21ba93739a2e250a

Tested it manually locally and the behavior is the expected.

I think if we are deprecating this for v32.0 we be consistent and do the same with all other RPCs. We have time though, if you open a PR deprecating the others feel free to ping me to review.

polespinasa commented at 3:12 PM on May 25, 2026: member

I was just thinking (post-acking the PR) if keeping ONLY the information fields is useful. For monitoring and analysis on the network it might be useful. @0xB10C would removing this bip125-replaceable affect any of your monitoring on transactions signaling RBF?

0xB10C commented at 8:00 PM on May 25, 2026: contributor

No, I don't think I/we care about bip125-replaceable anymore at this point.

achow101 commented at 12:42 AM on May 26, 2026: member

ACK 5faf2ad88029fe70d7fc8d3e21ba93739a2e250a

achow101 merged this on May 26, 2026

achow101 closed this on May 26, 2026

in test/functional/wallet_send.py:34 in 5faf2ad880

  30 | @@ -31,10 +31,7 @@ def set_test_params(self):
  31 |          # whitelist peers to speed up tx relay / mempool sync
  32 |          self.noban_tx_relay = True
  33 |          self.supports_cli = False
  34 | -        self.extra_args = [
  35 | -            ["-walletrbf=1", "-datacarriersize=16"],
  36 | -            ["-walletrbf=1", "-datacarriersize=16"]
  37 | -        ]
  38 | +        self.extra_args = [["-walletrbf=1", "-datacarriersize=16", "-deprecatedrpc=bip125"]] * self.num_nodes

maflcko commented at 8:58 AM on May 26, 2026:

I think this is the wrong fix. There should be a minimal single test to check the -deprecatedrpc=bip125 behavior in test/functional/rpc_deprecated.py, as explained in that test.

This test itself should be updated to not rely on the deprecatedrpc=bip125 anymore, because this will need to happen anyway before removal.

Same for -walletrbf

rkrux commented at 1:07 PM on May 26, 2026:

I had to update the documentation of rpc_deprecated.py a bit, but I've the follow-up PR here: #35381.

in test/functional/wallet_migration.py:47 in 5faf2ad880

  43 | @@ -44,7 +44,7 @@ def set_test_params(self):
  44 |          self.setup_clean_chain = True
  45 |          self.num_nodes = 2
  46 |          self.supports_cli = False
  47 | -        self.extra_args = [[], ["-deprecatedrpc=create_bdb"]]
  48 | +        self.extra_args = [["-deprecatedrpc=bip125"], ["-deprecatedrpc=create_bdb"]]

maflcko commented at 8:59 AM on May 26, 2026:

Same here, and in all other tests.

in test/functional/wallet_backwards_compatibility.py:353 in 5faf2ad880

 349 | @@ -349,9 +350,7 @@ def get_flags(conn):
 350 |  
 351 |              # Restore the wallet to master
 352 |              load_res = node_master.restorewallet(wallet_name, backup_path)
 353 | -
 354 | -            # There should be no warnings
 355 | -            assert "warnings" not in load_res
 356 | +            assert_equal(load_res["warnings"], [WALLETRBF_DEPRECATION_WARNING[9:]])

maflcko commented at 9:01 AM on May 26, 2026:

This seems the wrong diff. The follow-up pull request in the next release, will likely remove this line, in which case the overall diff will be that the assert "warnings" not in load_res will be removed, weakening the test.

Generally, I think it is better to fix up the tests to check the relevant behavior (of current master/the next release), instead of first updating all tests to check the wrong/deprecated behavior, and then updating them again some time in the future to check the relevant behavior.

rkrux commented at 10:31 AM on May 26, 2026:

Let me see what I can do with these tests in a follow-up PR.

in src/wallet/wallet.cpp:3106 in 5faf2ad880

3101 | @@ -3103,7 +3102,11 @@ bool CWallet::LoadWalletArgs(std::shared_ptr<CWallet> wallet, const WalletContex
3102 |  
3103 |      wallet->m_confirm_target = args.GetIntArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET);
3104 |      wallet->m_spend_zero_conf_change = args.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE);
3105 | -    wallet->m_signal_rbf = args.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF);
3106 | +    wallet->m_signal_rbf = DEFAULT_WALLET_RBF;
3107 | +    if (args.IsArgSet("-walletrbf")) {

maflcko commented at 9:04 AM on May 26, 2026:

style nit: I think the general pattern is to call it only once: if (auto value{args.GetBoolArg("-walletrbf")}) { ... m_signal_rbf=*value;}, to avoid the two IsArgSet/GetBoolArg calls.

in src/wallet/wallet.cpp:3105 in 5faf2ad880

3101 | @@ -3103,7 +3102,11 @@ bool CWallet::LoadWalletArgs(std::shared_ptr<CWallet> wallet, const WalletContex
3102 |  
3103 |      wallet->m_confirm_target = args.GetIntArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET);
3104 |      wallet->m_spend_zero_conf_change = args.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE);
3105 | -    wallet->m_signal_rbf = args.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF);
3106 | +    wallet->m_signal_rbf = DEFAULT_WALLET_RBF;

maflcko commented at 9:07 AM on May 26, 2026:

I think this is the wrong default.

The general idea seems to avoid bip125-replacement, because it is deprecated. So using it, seems backwards. See also the related fixup #31953, which moved toward fullrbf, away from bip125-rbf.

polespinasa commented at 9:20 AM on May 26, 2026:

Maybe this is something we can debate on a next PR when removing the options or opening an issue to discuss what the correct path is, but I see some benefits on keep signaling bip125-replacement.

Wallet fingerprinting, currently ~80% of the transactions are signaling it. data. Following what the majority do if it's harmless seems a good choice for privacy.
I guess we want old nodes that still look for bip125-replacement signaling to also replace transactions. Signaling it on every transaction would act as a fullrbf.

Maybe I am missing some downside, but I cannot come with any.

rkrux commented at 10:32 AM on May 26, 2026:

This PR was focussed on marking the fields as deprecated, it was intentional on my part to not change the default, which I think is best done in a separate PR.

maflcko commented at 11:14 AM on May 27, 2026:

Wallet fingerprinting, currently ~80% of the transactions are signaling it. data. Following what the majority do if it's harmless seems a good choice for privacy.

I think the graph shows that the 90-day moving average was always below 50% up to 3 years ago (the year prior to when #30592 was opened and merged). Not sure which self-custodial wallets or which custodial wallets are responsible for the recent increase, but given that the increase was recent, it seems that those wallets are likely easier to upgrade to a new behavior than those wallets that haven't switched to bip125 signalling in a decade.

So I think for those that care about privacy, it would be beneficial to not use bip125 signalling and quickly (hopefully less than ~3 years) converge again toward an anonymity set where the majority is not signalling.

I guess we want old nodes that still look for bip125-replacement signaling to also replace transactions. Signaling it on every transaction would act as a fullrbf.

#25353 with fullrbf was merged 4 years ago, and I don't think one can meaningfully run a node older than that in a safe way.

polespinasa commented at 8:23 AM on May 28, 2026:

Not sure which self-custodial wallets or which custodial wallets are responsible for the recent increase

I think I can get some data for that, will check and write it here just for curiosity :) I mostly agree with the other things you said, I was just pointing some ideas that came to my mind.

rkrux commented at 2:24 PM on May 28, 2026:

The general idea seems to avoid bip125-replacement, because it is deprecated.

Raised the PR #35405 here that does this.

polespinasa commented at 8:30 PM on May 29, 2026:

For context, some of my colleges at uni are doing research on wallet fingerprinting and they shared with me some data about bip125: <img width="2094" height="70" alt="image" src="https://github.com/user-attachments/assets/aee9ae06-e964-4395-ae48-2c19133a6586" /> <img width="2100" height="68" alt="image" src="https://github.com/user-attachments/assets/7f1ee4cb-7b0b-49ed-aeda-71cd6fabec1a" />

Which matches Ishaana's work results

maflcko commented at 9:09 AM on May 26, 2026: member

concept ack. I had a branch doing this last year, but I forgot to create a pull for this.

Haven't checked everything, but I think the approach may be moving in the wrong direction.

rkrux referenced this in commit 695b1fe2e8 on May 26, 2026

rkrux referenced this in commit ddda0c3b72 on May 27, 2026

rkrux referenced this in commit efada4d341 on May 27, 2026

rkrux referenced this in commit a723947a6a on May 27, 2026

rkrux referenced this in commit f701cd159a on May 28, 2026

achow101 referenced this in commit d0a54dd8e0 on May 28, 2026