refactor: bdb removals #32511

1. 
   maflcko commented at 1:00 pm on May 15, 2025: member

   remove dead code
2. remove unused Import* function signatures faecf158d9
3. remove unused AddrToPubKey fa91d57de3
4. remove unused GetAllDestinationsForKey fac72fef27
5. remove unused GetDestinationForKey

   It is only used in test. There it is problematic, because it sometimes
   relies on m_default_address_type. If the default were changed to
   BECH32M, those tests would fail the assert(false).
   
   So just use PKHash{} in all tests and remove GetDestinationForKey.

   fafee85358
6. 
   DrahtBot commented at 1:00 pm on May 15, 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/32511.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	davidgumberg, rkrux, achow101

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #bitcoin-core/gui/872 (gui: Menu action to export a watchonly wallet by achow101)
   • #32489 (wallet: Add exportwatchonlywallet RPC to export a watchonly version of a wallet by achow101)
   • #32475 (wallet: Use util::Error throughout AddWalletDescriptor instead of returning nullptr for some errors by achow101)
   • #32432 (wallet, rpc: Change OutputType items from string into compile-time constants string_view 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.

7. DrahtBot added the label Refactoring on May 15, 2025
8. fanquake requested review from rkrux on May 15, 2025
9. fanquake removed review request from rkrux on May 15, 2025
10. fanquake requested review from Sjors on May 15, 2025
11. fanquake requested review from rkrux on May 15, 2025
12. 
    davidgumberg commented at 6:39 am on May 16, 2025: contributor

    crACK https://github.com/bitcoin/bitcoin/pull/32511/commits/fafee85358397289aa4c6b799d2603a5d89e83a2
13. 
    rkrux commented at 8:54 am on May 16, 2025: contributor

    crACK fafee85358397289aa4c6b799d2603a5d89e83a2

    The functions in the last 3 commits do look like good helper functions but I don’t mind their removal as they are not used besides tests, making it easier to navigate the codebase that I prefer. An alternative could be to move some of them under tests directory. But fine with current state of the PR too.

14. 
    maflcko commented at 9:03 am on May 16, 2025: member

    The functions in the last 3 commits do look like good helper functions but I don’t mind their removal as they are not used besides tests, making it easier to navigate the codebase that I prefer. An alternative could be to move some of them under tests directory. But fine with current state of the PR too.

    It is hard for me to see a use case for the functions, even in tests. The tests only used them to generate dummy values, but those are trivially created inline. Generally, when it comes to providing helper functions for tests, my preference would be to focus on porting the real and user-exposed (wallet) RPCs to similarly-named or similarly-featured helper functions to be used in tests.

15. 
    rkrux commented at 10:48 am on May 16, 2025: contributor

    Generally, when it comes to providing helper functions for tests, my preference would be to focus on porting the real and user-exposed (wallet) RPCs to similarly-named or similarly-featured helper functions to be used in tests.

    Interesting, something like this is done in this PR #32452 for importprivkey.

16. 
    maflcko commented at 11:33 am on May 16, 2025: member

    The pull here only touches C++ code, so I meant to have such helpers in C++ code. The python side can just use the RPCs directly (or use one of the pre-existing helpers)
17. 
    rkrux commented at 1:47 pm on May 16, 2025: contributor

    Oh yeah, for a second there, I got engrossed in that wrapper/helper function over there and forgot it’s in the functional test framework.
18. 
    achow101 commented at 8:17 pm on May 16, 2025: member

    ACK fafee85358397289aa4c6b799d2603a5d89e83a2
19. achow101 merged this on May 16, 2025
20. achow101 closed this on May 16, 2025

    ---

21. maflcko deleted the branch on May 17, 2025

Contributors
maflcko DrahtBot davidgumberg rkrux achow101

Review Requested
Sjors

Labels
Refactoring