wallet: refactor ProcessDescriptorImport #33874
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33874. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
naiyoma
changed the title
DrahtBot
changed the title
naiyoma
force-pushed
the
2025_6/approach_2_validation
branch
from
6bbff43 to
9cc3309
Compare
naiyoma
force-pushed
the
2025_6/approach_2_validation
branch
from
9cc3309 to
04e6704
Compare
| for (const UniValue& request : requests.getValues()) { | ||
| DescriptorImportData validated = ValidateDescriptorImport(*pwallet, request); | ||
| UniValue result(UniValue::VOBJ); | ||
| result.pushKV("success", UniValue(validated.success)); |
There was a problem hiding this comment.
This is dangerous, we should never return success: true if the descriptor hasn't been fully processed yet.
Given how this RPC behaved previously, this can lead to users thinking that their descriptor has actually been imported when it was not.
|
Concept meh
But I do see how exiting early and doing nothing if something can't be imported could be useful, but this is pretty big semantic change and we need to be careful about return values to avoid any possible confusion. |
3 participants
Fixes: #33655
Currently, we validate and import descriptors in the same loop. If any descriptor fails validation, an error is added to the response array, but the complete response (including that error) is only returned to after a rescan completes—potentially wasting significant time when validation error could have been caught immediately.
This PR separates validation from processing: validate all descriptors first, and process in the second loop