wallet, rpc, doc: various legacy wallet removal cleanups in RPCs #32596

pull theStack wants to merge 3 commits into bitcoin:master from theStack:2025-wallet-rpc-related_legacy_wallet_cleanups changing 7 files +6 −37

theStack commented at 10:46 PM on May 22, 2025: contributor
This PR contains a few smaller wallet RPC cleanups based on that we only ever operate on descriptor wallets now:
- remove the now obsolete "keypoololdest" field from the getwalletinfo RPC and the corresponding CWallet/ScriptPubKeyMan methods
- in RPCs where potential fast wallet rescan is documented, remove the "descriptor wallet" mentions (back then introduced in commit ca48a4694f73e5be8f971ae482ebc2cce4caef44, PR #25957)
- for the createwallet RPC examples, remove the "descriptors" parameters that always have to be true now (proposed in #31250 (review); corresponds to 86de8c1668005304b2c630ca2ad4a8ca8e348e90, PR #32544 which did the same for functional tests)

wallet, rpc: remove obsolete "keypoololdest" result field/code

This `getwalletinfo()` result field was only ever returned for
legacy wallets and is hence not relevant anymore, so we can
delete it and the corresponding CWallet/ScriptPubKeyMan code
behind it.

db465a50e2

rpc: doc: drop descriptor wallet mentions in fast wallet rescan related RPCs

Now that we only ever operate on descriptor wallets, mentioning
that a faster rescan is only available for them is redundant and
can be removed.

These texts were originally introduced in commit
ca48a4694f73e5be8f971ae482ebc2cce4caef44 (PR #25957).

7a05f941bb

rpc: doc: remove redundant "descriptors" parameter in `createwallet` examples

This is the RPC example counterpart to commit
86de8c1668005304b2c630ca2ad4a8ca8e348e90 (PR #32544).
Since the recent legacy wallet removal this parameter *must* be
true, so providing it in the examples doesn't contain valuable
information anymore and it seems best to remove them.

e5cbea416b

DrahtBot commented at 10:46 PM on May 22, 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/32596.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK 1440000bytes, rkrux, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
1440000bytes commented at 12:21 AM on May 23, 2025: none

ACK https://github.com/bitcoin/bitcoin/pull/32596/commits/e5cbea416b2f63e5d99819052f3e69a6383336d6

<details> <summary>Signature</summary>

<pre> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 ACK e5cbea416b2f63e5d99819052f3e69a6383336d6 -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQSSUwYqT5LWNFkIXKYtIwUgISpZAQUCaC++7QAKCRAtIwUgISpZ AZIaAP9bxiLFrXtgDk7+yoGgHAUO2xP2AmG5qRcYtZDSxuqkmAEAotL4R3L9iDoX NZeq+w5EEr+4SO7zLYo+Cnb7krWYpQg= =pUtX -----END PGP SIGNATURE----- </pre>

</details>
laanwj added the label Tests on May 23, 2025
in src/wallet/rpc/wallet.cpp:49 in db465a50e2 outdated
```
  45 | @@ -46,7 +46,6 @@ static RPCHelpMan getwalletinfo()
  46 |                          {RPCResult::Type::STR_AMOUNT, "unconfirmed_balance", "DEPRECATED. Identical to getbalances().mine.untrusted_pending"},
  47 |                          {RPCResult::Type::STR_AMOUNT, "immature_balance", "DEPRECATED. Identical to getbalances().mine.immature"},
  48 |                          {RPCResult::Type::NUM, "txcount", "the total number of transactions in the wallet"},
  49 | -                        {RPCResult::Type::NUM_TIME, "keypoololdest", /*optional=*/true, "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only."},
```
rkrux commented at 2:55 PM on May 23, 2025:

I assume this is a breaking change but fine because it will go in the same release that contains the removal of legacy wallets as well.

theStack commented at 3:13 PM on May 23, 2025:

As this is an optional return field, I wouldn't consider its removal as a breaking change, considering that the condition for returning a value (i.e. executing the RPC on a legacy wallet) can't be fulfilled anymore. Curious about other opinions though; I guess one could argue that there is still a point in mentioning the removal in the release notes?

rkrux commented at 3:27 PM on May 23, 2025:

Ah, I note now that its presence was conditional on legacy wallets.

Type	Reviewers
ACK	1440000bytes, rkrux, achow101

in src/wallet/rpc/wallet.cpp:355 in e5cbea416b

 350 | @@ -351,8 +351,8 @@ static RPCHelpMan createwallet()
 351 |          RPCExamples{
 352 |              HelpExampleCli("createwallet", "\"testwallet\"")
 353 |              + HelpExampleRpc("createwallet", "\"testwallet\"")
 354 | -            + HelpExampleCliNamed("createwallet", {{"wallet_name", "descriptors"}, {"avoid_reuse", true}, {"descriptors", true}, {"load_on_startup", true}})
 355 | -            + HelpExampleRpcNamed("createwallet", {{"wallet_name", "descriptors"}, {"avoid_reuse", true}, {"descriptors", true}, {"load_on_startup", true}})
 356 | +            + HelpExampleCliNamed("createwallet", {{"wallet_name", "descriptors"}, {"avoid_reuse", true}, {"load_on_startup", true}})
 357 | +            + HelpExampleRpcNamed("createwallet", {{"wallet_name", "descriptors"}, {"avoid_reuse", true}, {"load_on_startup", true}})

rkrux commented at 2:58 PM on May 23, 2025:

Nice, thanks: #31250 (review)

theStack commented at 3:06 PM on May 23, 2025:

Ah interesting, wasn't aware that this was proposed already in an earlier PR. Added a link to your comment in the PR description.

rkrux approved
rkrux commented at 2:58 PM on May 23, 2025: contributor

ACK e5cbea416b2f63e5d99819052f3e69a6383336d6
achow101 commented at 7:20 PM on May 23, 2025: member

ACK e5cbea416b2f63e5d99819052f3e69a6383336d6
achow101 merged this on May 23, 2025
achow101 closed this on May 23, 2025
theStack deleted the branch on May 23, 2025

Contributors

Labels

Linked (view graph)

#25957 wallet: fast rescan with BIP157 block filters for descriptor wallets #31250 wallet: Disable creating and loading legacy wallets #32544 scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls

wallet, rpc, doc: various legacy wallet removal cleanups in RPCs #32596

Code Coverage & Benchmarks

Reviews