Closes #31728
This PR updates the DescriptorScriptPubKeyMan to skip range checks for non-ranged descriptors, which previously caused errors when updating a non-ranged descriptor with the range [0,0]
A unit test was added to test the new behaviour
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32344.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | achow101, BrandonOdiwuor |
| Concept ACK | w0xlt, shahsb |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/41113093377
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.
Concept ACK.
Functional tests might be necessary.
Non-range desc are always added to the wallet with the range [0,0]. After the descriptor is added, the wallet will TopUp the keypool. For non-range descriptors, this process updates the desc range to [0,1].
Any attempts to update this non-range descriptor with a [0,0] range will result in an error because the range checks rejects new ranges not included in the old range.
Since this is a non-range desc, the range information should be disregarded and AddWalletDescriptor should always succeed regardless of provided range information
Functional tests might be necessary.
The importdescriptors RPC sets the range for all non-ranged descriptors to [0,1] in every request so this error cannot be triggered using the RPC.
In #31728 @JeremyRubin explains that this error may be unexpectedly triggered when " experimenting with non-ranged descriptors in custom wallet workflows."
86+ FlatSigningProvider provider;
87+ std::string error;
88+ auto descs{Parse(desc_str, provider, error, /* require_checksum=*/ false)};
89+ auto& desc{descs.at(0)};
90+ WalletDescriptor w_desc{std::move(desc), 0, 0, 0, 0};
91+ auto spk_manager{*Assert(wallet.AddWalletDescriptor(w_desc, provider, "", false))};
DescriptorScriptPubKeyMan::CanUpdateToWalletDescriptor() being made in this test case?
