I noted in review of a previous PR here: #32553#pullrequestreview-2853743852.
Relaying the same information in the RPC help that’s present in a code comment: https://github.com/bitcoin/bitcoin/blob/af65fd1a333011137dafd3df9a51704fd319feb4/src/wallet/wallet.h#L809-L810
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32603.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Stale ACK | jonatack |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
39@@ -40,7 +40,7 @@ static RPCHelpMan getwalletinfo()
40 {
41 {
42 {RPCResult::Type::STR, "walletname", "the wallet name"},
43- {RPCResult::Type::NUM, "walletversion", "the wallet version"},
44+ {RPCResult::Type::NUM, "walletversion", "the wallet version (the oldest client version guaranteed to understand this wallet)"},
“the wallet version” is redundant to the field name, whereas “current wallet format” provides a bit more context
0 {RPCResult::Type::NUM, "walletversion", "the current wallet format (the oldest client version guaranteed to understand this wallet)"},
ACK 21ddfbf888dabed1bb89b30481f1391727f6212
modulo nit suggestion, and PR/commit title s/wallet, rpc/rpc, doc/
(suggest not linking to https://github.com/bitcoin/bitcoin/pull/32553#pullrequestreview-2853743852 in the commit message, as every push here adds a GitHub notification in that pull)
I noted in review of a previous PR 32553.
Relaying the same information in the RPC help that's present
in a code comment: https://github.com/bitcoin/bitcoin/blob/af65fd1a333011137dafd3df9a51704fd319feb4/src/wallet/wallet.h#L809-L810
39@@ -40,7 +40,7 @@ static RPCHelpMan getwalletinfo()
40 {
41 {
42 {RPCResult::Type::STR, "walletname", "the wallet name"},
43- {RPCResult::Type::NUM, "walletversion", "the wallet version"},
44+ {RPCResult::Type::NUM, "walletversion", "the current wallet format (the oldest client version guaranteed to understand this wallet)"},
I don’t think this is right. The current wallet version is 169900 (for a freshly created descriptor wallet). However, I don’t think a Bitcoin Core 16.99 client can open descriptor wallets at all.
Generally, all modules in Bitcoin Core moved away from the client version to use a module-specific and module-internal version. (You can see this when you look at all other files written (fee estimates, mempool, addr …). Also the p2p version, but that again has been replaced by feature-messages.)
Hmm I see. Thanks for highlighting. Descriptor wallets were indeed introduced after v16.99.
I’m not sure then what does the “client version” here refers to. Maybe it is referring to a wallet client somehow because as mentioned above that versioning has moved to being module specific.
I will dig more to see how this can be made clearer.