wallet: unfriend LegacyDataSPKM and DescriptorScriptPubKeyMan #35192

1. 
   rkrux commented at 10:39 AM on May 1, 2026: contributor

   After #28333, LegacyDataSPKM doesn't need to use the private or protected members of DescriptorScriptPubKeyMan class such as AddDescriptorKeyWithDB and TopUpWithDB. Moreover, these two SPKMs are siblings that inherit from the common ScriptPubKeyMan that have non intersecting use cases semantically. It seems reasonable to me that they are unfriended so that private members of one are not exposed to another unnecessarily.

2. DrahtBot added the label Wallet on May 1, 2026
3. 
   DrahtBot commented at 10:39 AM on May 1, 2026: 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/35192.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. 
   rkrux commented at 10:40 AM on May 1, 2026: contributor

   The CI should pass after #28333 is merged, will rebase and undraft then.

5. DrahtBot added the label CI failed on May 1, 2026
6. 
   DrahtBot commented at 11:15 AM on May 1, 2026: contributor

   <!--85328a0da195eb286784d51f73fa0af9-->

   🚧 At least one of the CI tasks failed. _{Task macOS-cross to arm64: https://github.com/bitcoin/bitcoin/actions/runs/25211457547/job/73922821804} _{LLM reason (✨ experimental): CI failed because the build didn’t compile: scriptpubkeyman.cpp tried to call DescriptorScriptPubKeyMan’s private/protected methods (AddDescriptorKeyWithDB, TopUpWithDB).}

   <details><summary>Hints</summary>

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

   </details>

7. rkrux renamed this:
   wallet: unfriend LegacyDataSPKM and DescriptorScriptPubKeyMan classes
   wallet: unfriend LegacyDataSPKM and DescriptorScriptPubKeyMan
   on May 11, 2026
8. wallet: unfriend LegacyDataSPKM and DescriptorScriptPubKeyMan classes

   After PR 28333, `LegacyDataSPKM` doesn't need to use the private or
   protected members of `DescriptorScriptPubKeyMan` class such as
   `AddDescriptorKeyWithDB` and `TopUpWithDB`. Moreover, these two SPKMs
   are siblings that inherit from the common `ScriptPubKeyMan`.
   
   It seems reasonable to me that they are unfriended so that private
   members of one are not exposed to another unnecessarily.

   6c525c2ec1
9. rkrux force-pushed on May 29, 2026
10. 
    rkrux commented at 10:00 AM on May 29, 2026: contributor

    Rebased over master to incorporate changes from #28333.

11. rkrux marked this as ready for review on May 29, 2026
12. fanquake requested review from polespinasa on May 29, 2026
13. fanquake requested review from davidgumberg on May 29, 2026
14. DrahtBot removed the label CI failed on May 29, 2026

Contributors

rkruxDrahtBot

Review Requested
davidgumbergpolespinasa