refactor: use CWallet const shared pointers in dump{privkey,wallet} #22805

pull theStack wants to merge 3 commits into bitcoin:master from theStack:202108-refactor-const_correctness_for_further_dump_methods changing 3 files +21 −8
  1. theStack commented at 1:33 PM on August 26, 2021: contributor

    This PR is based on #22787 ("refactor: actual immutable pointing"), which should be reviewed first. (merged by now)

    It aims to make the CWallet shared pointers actually immutable also for the dumpprivkey and dumpwallet RPC methods. For doing that, some more preparations are needed; we need a const-counterpart to the helper EnsureLegacyScriptPubKeyMan that accepts a const CWallet pointer and accordingly also returns a const LegacyScriptPubKeyMan instance. The metadata lookup in dumpwallet is changed to not need a mutable ScriptPubKeyMan instance by avoiding using the operator[] in its mapKeyMetadata map, which also avoids repeated lookups.

  2. fanquake added the label Wallet on Aug 26, 2021
  3. fanquake added the label Refactoring on Aug 26, 2021
  4. DrahtBot commented at 8:50 PM on August 26, 2021: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #11803 (Bugfix: RPC/Wallet: Include HD key metadata in dumpwallet by luke-jr)

    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.

  5. DrahtBot cross-referenced this on Aug 27, 2021 from issue Make bech32m the default for RPC, opt-in for GUI by Sjors
  6. DrahtBot cross-referenced this on Aug 27, 2021 from issue refactor: Clean up new wallet spend, receive files added #21207 by ryanofsky
  7. DrahtBot cross-referenced this on Aug 27, 2021 from issue refactor: Make CWalletTx sync state type-safe by ryanofsky
  8. DrahtBot cross-referenced this on Aug 27, 2021 from issue multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky
  9. DrahtBot cross-referenced this on Aug 27, 2021 from issue multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky
  10. DrahtBot cross-referenced this on Aug 27, 2021 from issue Bugfix: RPC/Wallet: Include HD key metadata in dumpwallet by luke-jr
  11. DrahtBot added the label Needs rebase on Sep 3, 2021
  12. theStack force-pushed on Sep 5, 2021
  13. DrahtBot removed the label Needs rebase on Sep 5, 2021
  14. DrahtBot cross-referenced this on Sep 9, 2021 from issue refactor: Remove `gArgs` from `wallet.h` and `wallet.cpp` (2) by kiminuo
  15. DrahtBot cross-referenced this on Sep 29, 2021 from issue Remove `-rescan` startup parameter by meshcollider
  16. DrahtBot added the label Needs rebase on Sep 30, 2021
  17. theStack force-pushed on Oct 2, 2021
  18. theStack commented at 9:14 PM on October 2, 2021: contributor

    Rebased on #22787 again (which in turn rebased on master recently).

  19. DrahtBot removed the label Needs rebase on Oct 2, 2021
  20. DrahtBot cross-referenced this on Oct 15, 2021 from issue tests: remove usage of LegacyScriptPubKeyMan from some wallet tests by achow101
  21. DrahtBot added the label Needs rebase on Oct 22, 2021
  22. theStack force-pushed on Oct 28, 2021
  23. theStack commented at 11:44 PM on October 28, 2021: contributor

    Rebased on #22787 again (which in turn rebased on master recently).

  24. DrahtBot removed the label Needs rebase on Oct 29, 2021
  25. laanwj commented at 4:08 PM on November 10, 2021: member

    Concept ACK, making const-wallet operations take const pointers is a good precaution.

    Code review ACK d150fe3ad5ce181511f2d2fd035a10530eaa4203

  26. refactor: avoid multiple key->metadata lookups in dumpwallet RPC
    This also enables working with a const ScriptPubKeyMan which was
previously not possible due to std::map::operator[] not being const.
    29905c092f
  27. refactor: use const `LegacyScriptPubKeyMan` references in dump{privkey,wallet} RPCs ec2792d1dc
  28. refactor: use `CWallet` const shared pointers in dump{privkey,wallet} RPCs d150fe3ad5
  29. MarcoFalke commented at 4:15 PM on November 10, 2021: member

    Maybe rebase once more for fun?

  30. theStack force-pushed on Nov 10, 2021
  31. theStack commented at 4:25 PM on November 10, 2021: contributor

    Maybe rebase once more for fun?

    Done :upside_down_face:

  32. laanwj merged this on Nov 10, 2021
  33. laanwj closed this on Nov 10, 2021
  34. sidhujag referenced this in commit a5de714ef0 on Nov 10, 2021
  35. bitcoin locked this on Nov 10, 2022
Contributors
theStackDrahtBotlaanwjMarcoFalke
Labels
RefactoringWallet
Linked (view graph)
#22787 refactor: actual immutable pointing
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-19 06:53 UTC