rpc, doc: clarify wallet version in getwalletinfo help #32603

pull rkrux wants to merge 1 commits into bitcoin:master from rkrux:wallet-version changing 1 files +1 −1
  1. rkrux commented at 3:19 PM on May 23, 2025: contributor

    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

    <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md -->

    <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. -->

    <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. -->

  2. DrahtBot commented at 3:19 PM on May 23, 2025: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32603.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    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.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #32977 (wallet: Remove wallet version and several legacy related functions by w0xlt)

    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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  3. in src/wallet/rpc/wallet.cpp:43 in 21ddfbf888 outdated 
      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)"},
    jonatack commented at 5:45 PM on May 23, 2025:

    "the wallet version" is redundant to the field name, whereas "current wallet format" provides a bit more context

                            {RPCResult::Type::NUM, "walletversion", "the current wallet format (the oldest client version guaranteed to understand this wallet)"},
    rkrux commented at 7:42 AM on May 24, 2025:

    Makes sense, done.

  4. jonatack commented at 5:47 PM on May 23, 2025: member

    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)

  5. rpc, doc: clarify wallet version in getwalletinfo help
    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
    058537331c
  6. rkrux force-pushed on May 24, 2025
  7. rkrux renamed this:
    wallet, rpc: clarify wallet version in getwalletinfo help
    rpc, doc: clarify wallet version in getwalletinfo help
    on May 24, 2025
  8. in src/wallet/rpc/wallet.cpp:43 in 058537331c 
      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)"},
    maflcko commented at 11:02 AM on May 24, 2025:

    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.)

    rkrux commented at 12:05 PM on May 24, 2025:

    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.

    maflcko commented at 6:27 AM on August 18, 2025:

    Are you still working on this, or can this be closed?

    rkrux commented at 8:21 AM on August 18, 2025:

    Closing this for now.

  9. rkrux marked this as a draft on May 26, 2025
  10. DrahtBot commented at 12:42 AM on August 16, 2025: contributor

    <!--cf906140f33d8803c4a75a2196329ecb-->

    🐙 This pull request conflicts with the target branch and needs rebase.

  11. DrahtBot added the label Needs rebase on Aug 16, 2025
  12. rkrux closed this on Aug 18, 2025
Contributors
rkruxDrahtBotjonatackmaflcko
Labels
Needs rebase
Linked (view graph)
#32553 wallet: Fix logging of wallet version
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-01 03:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me