docs: Improve `keypoolrefill` RPC docs #32429

pull w0xlt wants to merge 1 commits into bitcoin:master from w0xlt:keypoolrefill_docs changing 1 files +2 −1

w0xlt commented at 7:15 PM on May 6, 2025: contributor

Update keypoolrefill RPC docs to make it clear that descriptor wallets have 4 ScriptPubKeyManagers by default and each of them is updated in this command, as suggested #29924 (comment)

Closes https://github.com/bitcoin/bitcoin/issues/29924
DrahtBot commented at 7:15 PM on May 6, 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/32429.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK brunoerg, achow101

Stale ACK rkrux

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Docs on May 6, 2025
in src/wallet/rpc/addresses.cpp:340 in 9e20f6ba68 outdated
```
 335 | @@ -336,7 +336,9 @@ RPCHelpMan addmultisigaddress()
 336 |  RPCHelpMan keypoolrefill()
 337 |  {
 338 |      return RPCHelpMan{"keypoolrefill",
 339 | -                "\nFills the keypool."+
 340 | +                "\nRefills each `ScriptPubKeyManager` keypool in the wallet up to the specified number of new keys.\n"
 341 | +                "By default, descriptor wallets have four `ScriptPubKeyManagers` (`wpkh`, `tr`, `pkh`, and `sh-wpkh`), each with 1000 entries.\n"
```
maflcko commented at 8:43 PM on May 6, 2025:

Should probably use DEFAULT_KEYPOOL_SIZE instead of hardcoded 1000? Same for the OUTPUT_TYPES? Also, ScriptPubKeyManagers was never used in user-facing docs, except for one error message. Maybe just use "active ranged descriptor" (or similar)?

w0xlt commented at 11:37 PM on May 6, 2025:

Done (for DEFAULT_KEYPOOL_SIZE and ScriptPubKeyManagers). Regarding OUTPUT_TYPES, I think it may need to be refactored to include it in the RPC documentation due to static initialization. But for now, I left keypoolrefill exactly as getnewaddress.

w0xlt commented at 3:17 AM on May 7, 2025:

OUTPUT_TYPES handled in https://github.com/bitcoin/bitcoin/pull/32432

Type	Reviewers
ACK	brunoerg, achow101
Stale ACK	rkrux

in src/wallet/rpc/addresses.cpp:341 in 9e20f6ba68 outdated

 335 | @@ -336,7 +336,9 @@ RPCHelpMan addmultisigaddress()
 336 |  RPCHelpMan keypoolrefill()
 337 |  {
 338 |      return RPCHelpMan{"keypoolrefill",
 339 | -                "\nFills the keypool."+
 340 | +                "\nRefills each `ScriptPubKeyManager` keypool in the wallet up to the specified number of new keys.\n"
 341 | +                "By default, descriptor wallets have four `ScriptPubKeyManagers` (`wpkh`, `tr`, `pkh`, and `sh-wpkh`), each with 1000 entries.\n"
 342 | +                "Legacy wallet typically uses a single `LegacyScriptPubKeyMan` (which manages both external and internal keypools)\n"+

maflcko commented at 9:07 PM on May 6, 2025:

also, now that bdb was removed, no need to add docs for it?

brunoerg commented at 10:22 PM on May 6, 2025:

Agreed. Otherwise, #28710 would have to drop it.

w0xlt commented at 11:37 PM on May 6, 2025:

Done. Thanks.

w0xlt force-pushed on May 6, 2025
w0xlt force-pushed on May 6, 2025

in src/wallet/rpc/addresses.cpp:339 in cd00fb85e9 outdated

 335 | @@ -336,7 +336,8 @@ RPCHelpMan addmultisigaddress()
 336 |  RPCHelpMan keypoolrefill()
 337 |  {
 338 |      return RPCHelpMan{"keypoolrefill",
 339 | -                "\nFills the keypool."+
 340 | +                "\nRefills each descriptor keypool in the wallet up to the specified number of new keys.\n"

maflcko commented at 7:46 AM on May 7, 2025:

                "Refills each descriptor keypool in the wallet up to the specified number of new keys.\n"

nit: This is trimmed anyway.

w0xlt commented at 5:45 PM on May 7, 2025:

Done in https://github.com/bitcoin/bitcoin/pull/32429/commits/ff35a4b021e12ae33e28c01ffeeb2c1916f7a487

in src/wallet/rpc/addresses.cpp:340 in cd00fb85e9 outdated

 335 | @@ -336,7 +336,8 @@ RPCHelpMan addmultisigaddress()
 336 |  RPCHelpMan keypoolrefill()
 337 |  {
 338 |      return RPCHelpMan{"keypoolrefill",
 339 | -                "\nFills the keypool."+
 340 | +                "\nRefills each descriptor keypool in the wallet up to the specified number of new keys.\n"
 341 | +                "By default, descriptor wallets have 4 active ranged descriptors (\"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"), each with " + std::to_string(DEFAULT_KEYPOOL_SIZE) + " entries.\n" +

maflcko commented at 8:02 AM on May 7, 2025:

I don't think to_string is allowed, see #32434

Could use ToString?

w0xlt commented at 5:45 PM on May 7, 2025:

Done in https://github.com/bitcoin/bitcoin/pull/32429/commits/ff35a4b021e12ae33e28c01ffeeb2c1916f7a487

rkrux approved
rkrux commented at 10:49 AM on May 7, 2025: contributor

ACK cd00fb85e9e0997120715e68374462cff0dad7eb given the suggestions are addressed.

I checked that by default there are 4 scriptpubkeymans setup on the first run of the wallet inside SetupDescriptorScriptPubKeyMans. Also, ack because the lack of this information has caused an issue to be created.
docs: Improve `keypoolrefill` RPC docs ff35a4b021
w0xlt force-pushed on May 7, 2025
brunoerg approved
brunoerg commented at 5:48 PM on May 7, 2025: contributor

code review ACK ff35a4b021e12ae33e28c01ffeeb2c1916f7a487
DrahtBot requested review from rkrux on May 7, 2025
achow101 commented at 9:44 PM on May 7, 2025: member

ACK ff35a4b021e12ae33e28c01ffeeb2c1916f7a487
achow101 merged this on May 7, 2025
achow101 closed this on May 7, 2025
stickies-v referenced this in commit 772a33e052 on May 23, 2025
sedited referenced this in commit cb3cd9b8b9 on May 29, 2025
luke-jr referenced this in commit 3ca2b01605 on Jun 6, 2025
achow101 referenced this in commit f33154c464 on Jul 1, 2025
yuvicc referenced this in commit 069643f094 on Jul 6, 2025
bug-castercv502 referenced this in commit 2858098810 on Sep 28, 2025
Kino1994 referenced this in commit a4c7b958f2 on Jun 28, 2026
Kino1994 referenced this in commit ed9fc2d410 on Jun 28, 2026
BigcoinBGC referenced this in commit facb48ac8e on Jun 30, 2026
BigcoinBGC referenced this in commit 7211f3fe3b on Jun 30, 2026
bitcoin locked this on Jul 30, 2026

Contributors

Review Requested
rkrux

Labels

Docs

Linked (view graph)

#29924 `keypoolrefill` doesn't fill keypool to specified parameter #32432 wallet, rpc: Use `OUTPUT_TYPES` to describe the output types instead of hardcoding them

docs: Improve `keypoolrefill` RPC docs #32429

Code Coverage & Benchmarks

Reviews