wallet: rpc: Deprecate `removeprunedfunds` RPC #35605

davidgumberg commented at 10:59 PM on June 25, 2026: contributor

Originally added in #7558 as a companion to importprunedfunds, this RPC has no known helpful use while being both dangerous and a maintenance burden.

Despite what the name says, it allows the deletion of arbitrary transactions, and importprunedfunds does not allow the importing of transactions not belonging to the user, and listtransactions does not list transactions not belonging to the wallet, so this RPC can only be used to delete transactions actually belonging to the wallet, and in the unlikely event that transactions not belonging to the wallet are present, they cause no harm except for occupying a few bytes on the users disk.

DrahtBot added the label Wallet on Jun 25, 2026

DrahtBot commented at 10:59 PM on June 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/35605.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	pablomartin4btc
Concept ACK	achow101, polespinasa

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

Conflicts

No conflicts as of last run.

davidgumberg force-pushed on Jun 25, 2026

DrahtBot added the label CI failed on Jun 25, 2026

DrahtBot commented at 11:12 PM on June 25, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/28205807916/job/83556135158</sub> <sub>LLM reason (✨ experimental): CI failed due to ruff Python lint errors (unused imports, F401) in test/functional/wallet_resendwallettransactions.py.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on Jun 26, 2026

in doc/release-notes-removeprunedfunds.md:1 in 661a4dbf68

   0 | @@ -0,0 +1,12 @@
   1 | +RPC

pablomartin4btc commented at 6:10 PM on June 26, 2026:

nit (if you have to retouch)

Updated RPCs

davidgumberg commented at 12:36 AM on June 27, 2026:

Thanks, fixed.

in doc/release-notes-removeprunedfunds.md:10 in 661a4dbf68

   5 | +
   6 | +The `removeprunedfunds` RPC has been deprecated and will be removed in the next
   7 | +major release.
   8 | +
   9 | +In order to continue using it, `bitcoind` must be started with the
  10 | +`deprecatedrpc=removeprunedfunds

pablomartin4btc commented at 6:16 PM on June 26, 2026:

`deprecatedrpc=removeprunedfunds`.

davidgumberg commented at 12:36 AM on June 27, 2026:

Thanks, fixed.

in doc/release-notes-removeprunedfunds.md:12 in 661a4dbf68

   7 | +major release.
   8 | +
   9 | +In order to continue using it, `bitcoind` must be started with the
  10 | +`deprecatedrpc=removeprunedfunds
  11 | +
  12 | +---

pablomartin4btc commented at 6:20 PM on June 26, 2026:

davidgumberg commented at 12:36 AM on June 27, 2026:

I thought maybe this was necessary since i was using two lines for one change, looking at past release notes people always use one line / top level bullet point, so I just switched to doing that, thanks.

in doc/release-notes-removeprunedfunds.md:5 in 661a4dbf68

   0 | @@ -0,0 +1,12 @@
   1 | +RPC
   2 | +------------
   3 | +
   4 | +---
   5 | +

pablomartin4btc commented at 6:20 PM on June 26, 2026:

in src/wallet/rpc/backup.cpp:98 in 661a4dbf68

  94 | @@ -95,6 +95,7 @@ RPCMethod removeprunedfunds()
  95 |  {
  96 |      return RPCMethod{
  97 |          "removeprunedfunds",
  98 | +        "DEPRECATION WARNING: This feature will be removed in the next major release. Start bitcoind with the `-deprecatedrpc=removeprunedfunds` option in order to use this.\n"

pablomartin4btc commented at 9:07 PM on June 26, 2026:

nit (please check https://github.com/bitcoin/bitcoin/pull/31278/changes/bf194c920cf768d1339d41aef1441a78e2f5fcbe):

        "(DEPRECATED) This feature will be removed in the next major release. Start bitcoind with the `-deprecatedrpc=removeprunedfunds` option in order to use this.\n"

davidgumberg commented at 12:40 AM on June 27, 2026:

Thanks, taken.

in test/functional/wallet_deprecated_removeprunedfunds.py:31 in 661a4dbf68

  26 | +        node.createwallet("super_secure_wallet")
  27 | +        wallet = node.get_wallet_rpc("super_secure_wallet")
  28 | +        self.generatetoaddress(node, nblocks=COINBASE_MATURITY + 1, address=wallet.getnewaddress(), sync_fun=self.no_op)
  29 | +        tx_to_remove = wallet.listunspent()[0]["txid"]
  30 | +
  31 | +        print(f"tx to remove: {tx_to_remove}")

pablomartin4btc commented at 9:15 PM on June 26, 2026:

was this part of a test?

davidgumberg commented at 12:40 AM on June 27, 2026:

Oops, thanks for catching!

in test/functional/wallet_deprecated_removeprunedfunds.py:43 in 661a4dbf68 outdated

  38 | +        self.log.info("Test calling removeprunedfunds with -deprecatedrpc=removeprunedfunds")
  39 | +        self.restart_node(0, extra_args=["-deprecatedrpc=removeprunedfunds"])
  40 | +        node.loadwallet("super_secure_wallet")
  41 | +        wallet = node.get_wallet_rpc("super_secure_wallet")
  42 | +        wallet.removeprunedfunds(tx_to_remove)
  43 | +

pablomartin4btc commented at 9:19 PM on June 26, 2026:

Perhaps you can add an assert_raises_rpc_error on wallet.gettransaction(tx_to_remove) after the call to confirm the RPC actually does what it says.

davidgumberg commented at 12:40 AM on June 27, 2026:

Ah, I tried to think of something here that was very simple and short but couldn't think of that, (I went in the direction of balances and decided it wasn't worth it) thanks for the suggestion! I've taken it.

pablomartin4btc commented at 9:20 PM on June 26, 2026: member

ACK 661a4dbf689f65a7f10fb3d1753ffc8c58be3563

I think the removeprunedfunds RPC was designed to remove non-wallet transactions imported via importprunedfunds, but since importprunedfunds only imports wallet transactions, the only thing removeprunedfunds can actually delete is your own wallet transactions. So it's dangerous with no real use case.

Perhaps the description should reference the original PR #7558 / v0.13.0, when the RPCs were added.

nit: typo in commit 2 body: "delte" → "delete".

non blocker: I thought release notes file should have the PR number just to make it easier to identify it but it seems it's not mandatory as the consolidation process is done manually.

achow101 commented at 10:40 PM on June 26, 2026: member

Concept ACK

davidgumberg force-pushed on Jun 27, 2026

test: wallet: resend: avoid internal behavior via removeprunedfunds

Currently this test makes assumptions about `listtransaction` exposing a
wallet-internal data structure `mapWallet` That is not guaranteed or
enforced anywhere. Ideally, this would live in a unit test instead of a
functional test, but as a half-measure to simplify the test, just check
the behavior 10 times, if there are any dependencies on random ordering
inside of a wallet data structure, this is likely to catch them.

e5b7785447

davidgumberg force-pushed on Jun 27, 2026

DrahtBot added the label CI failed on Jun 27, 2026

davidgumberg force-pushed on Jun 27, 2026

wallet: rpc: deprecate removeprunedfunds

This RPC has no helpful use while being both dangerous and a maintenance
burden.

Despite what the name says, it allows the deletion of arbitrary
transactions, and `importprunedfunds` does not allow the importing of
transactions not belonging to the user, and `listtransactions` does not
list transactions not belonging to the wallet, so this RPC can only be
used to delete transactions actually belonging to the wallet, and in the
unlikely event that transactions not belonging to the wallet are
present, they cause no harm except for occupying a few bytes on the
users disk.

44c3268c76

davidgumberg force-pushed on Jun 27, 2026

davidgumberg commented at 12:43 AM on June 27, 2026: contributor

(sorry for all the pushes :O )

Pushed to address @pablomartin4btc's feedback.

DrahtBot removed the label CI failed on Jun 27, 2026

pablomartin4btc commented at 9:41 PM on June 27, 2026: member

ACK 44c3268c7639be052cc54461cde66ec89a3d118a

Thanks @davidgumberg for considering the suggestions!

DrahtBot requested review from achow101 on Jun 27, 2026

polespinasa commented at 11:09 AM on July 3, 2026: member

concept ACK

There is no need to create wallet_deprecated_removeprunedfunds.py, that test should be moved into rpc_deprecated.py

in test/functional/wallet_deprecated_removeprunedfunds.py:45 in 44c3268c76

  40 | +
  41 | +        wallet.gettransaction(tx_to_remove)
  42 | +        wallet.removeprunedfunds(tx_to_remove)
  43 | +        assert_raises_rpc_error(
  44 | +            -5, "Invalid or non-wallet transaction id",
  45 | +            wallet.gettransaction, tx_to_remove)

polespinasa commented at 11:11 AM on July 3, 2026:

I don't think this tests belongs here. Aren't we testing this in some other functional test?

Ideally rpc_decrepated.py should only test the deprecation error if -deprecatedrpc is not used. The other logic should be tested outside it updating the rest of the functional tests adding the -deprecatedrpc flag.

davidgumberg commented at 8:46 PM on July 8, 2026:

I think that seems reasonable, I added this because it was only a single line change to check it, and it seemed reasonable to add it here just so a reader can see here in one place:

You can call the RPC without it throwing an error and
It actually calls the RPC instead of doing nothing.

That seemed reasonable although I agree that it is a bit duplicate with the other test since that one passes the arg.

I can see good points either way, just pushing back a little with the motivation for why I added it.

cc: @pablomartin4btc

pablomartin4btc commented at 9:38 PM on July 8, 2026:

I agree with @polespinasa — the gettransaction assertion verifying the removal actually happened belongs in wallet_importprunedfunds.py rather than the deprecation test. Since wallet_importprunedfunds.py already has -deprecatedrpc=removeprunedfunds in its extra_args, it's a small addition. And since there are only concept ACKs, I'd suggest doing the move within this PR: remove the -deprecatedrpc functional test block from the deprecation test and move the gettransaction assertion to wallet_importprunedfunds.py. A follow-up PR just for this wouldn't be worth it.

in doc/release-notes-removeprunedfunds.md:4 in 44c3268c76

   0 | @@ -0,0 +1,6 @@
   1 | +Updated RPCs
   2 | +------------
   3 | +
   4 | +- The `removeprunedfunds` RPC has been deprecated and will be removed in the

polespinasa commented at 11:14 AM on July 3, 2026:

feel-free-to-ignore-nit: I would say "it is planned to be removed", but not secure it. We never know, maybe the removal PR doesn't get merged :)

davidgumberg commented at 8:38 PM on July 8, 2026:

I disagree, I think it is implicit that when a release note or some documentation says "X will be removed in the future." that this is only stating the point of view today, and that it's subject to change in the future.

pablomartin4btc commented at 9:19 PM on July 8, 2026:

Here is another example https://github.com/bitcoin/bitcoin/pull/35433/. For what it's worth, #35337 recently added a deprecation guide to developer-notes, just in case we need to update it.

in src/wallet/rpc/backup.cpp:98 in 44c3268c76

  94 | @@ -95,6 +95,7 @@ RPCMethod removeprunedfunds()
  95 |  {
  96 |      return RPCMethod{
  97 |          "removeprunedfunds",
  98 | +        "(DEPRECATED) This feature will be removed in the next major release. Start bitcoind with the `-deprecatedrpc=removeprunedfunds` option in order to use this.\n"

polespinasa commented at 11:14 AM on July 3, 2026:

Same as my comment in the release note

davidgumberg commented at 8:39 PM on July 8, 2026:

Marking as resolved just so we can discuss in the other comment.

in src/wallet/rpc/backup.cpp:114 in 44c3268c76

 110 | @@ -110,6 +111,10 @@ RPCMethod removeprunedfunds()
 111 |      std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request);
 112 |      if (!pwallet) return UniValue::VNULL;
 113 |  
 114 | +    if (!pwallet->chain().rpcEnableDeprecated("removeprunedfunds")) {

polespinasa commented at 11:14 AM on July 3, 2026:

Same as my comment in the release note

davidgumberg commented at 8:39 PM on July 8, 2026:

Marking as resolved just so we can discuss in the other comment.