rpc, test: fix “internal” label check in importdescriptors and extend test #33614

1. 
   waketraindev commented at 4:45 pm on October 13, 2025: contributor

   Fixes a logic issue in ProcessDescriptorImport() where descriptors with "internal": false and a label were incorrectly rejected. The check now uses internal == true so labels are only disallowed when internal is true.

   Tests updated to cover both cases

2. 
   DrahtBot commented at 4:45 pm on October 13, 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/33614.

   Reviews

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31514 (wallet: allow label for non-ranged external descriptor (if internal=false) & disallow label for ranged descriptors by scgbckbone)

   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.

3. waketraindev marked this as ready for review on Oct 13, 2025
4. in src/wallet/rpc/backup.cpp:210 in 9de7cf0859

   206@@ -207,7 +207,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c
   207         }
   208 
   209         // Internal addresses should not have a label either
   210-        if (internal && data.exists("label")) {
   211+        if (internal.value_or(false) && data.exists("label")) {
   

   ---

   ---

   

   janb84 commented at 6:48 pm on October 13, 2025:

   NIT: would rather see internal == true imho better readability.

   0        if (internal == true && data.exists("label")) {
   

5. in test/functional/wallet_importdescriptors.py:109 in 9de7cf0859

   115+            self.test_importdesc(import_request, success=True)
   116+            test_address(w1,
   117+                        key.p2pkh_addr,
   118+                        solvable=True,
   119+                        ismine=True,
   120+                        labels=[label])
   

   ---

   ---

   

   janb84 commented at 6:50 pm on October 13, 2025:

   NIT: restore this and use the pattern that is used for the rest of the tests. Change the import_request slightly e.g:

   0        self.log.info("Test can import a p2pkh descriptor with internal explicit set to false ")
   1        self.test_importdesc({**import_request, "internal": False}, success=True)
   

6. waketraindev force-pushed on Oct 13, 2025
7. 
   waketraindev commented at 7:18 pm on October 13, 2025: contributor

   Resolved nits, internal checks for true, test updated, typos and grammar.
8. rpc, test: fix "internal" label check in importdescriptors and extend test d9bd357189
9. waketraindev force-pushed on Oct 13, 2025
10. 
    waketraindev commented at 5:43 pm on October 14, 2025: contributor

    Fixes #32376, same underlying issue as #31514
    Happy to defer to #31514 if it’s preferred.
11. 
    rkrux commented at 10:31 am on October 17, 2025: contributor

    Thanks for raising this and highlighting the previous PR as well, which I have reviewed and ACKed. I’m inclining to let this bug be fixed in that PR as that fix seems comprehensive.
12. waketraindev closed this on Oct 17, 2025

    ---

13. waketraindev deleted the branch on Nov 19, 2025

Contributors
waketraindev DrahtBot janb84 rkrux