IsSolvable() using descriptors
#25664
Now that we have descriptors there is no need to try to sign for a scriptPubKey using dummy signatures, and using a mocked verification of this witness against the interpreter, just to make sure we know how to spend such a Script. Just try to infer a solvable descriptor: any scriptPubKey that we can sign for can be inferred as such.
This came up in #24149 but i think it’s worth it on its own.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, 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.
577@@ -578,7 +578,7 @@ RPCHelpMan getaddressinfo()
578
579 if (provider) {
580 auto inferred = InferDescriptor(scriptPubKey, *provider);
581- bool solvable = inferred->IsSolvable() || IsSolvable(*provider, scriptPubKey);
582+ bool solvable = inferred->IsSolvable();
nit:
0 bool solvable = inferred && inferred->IsSolvable();
In that sense, then might be better to add an explicit assertion so we don’t rely on a nullptr crash?
0auto inferred = Assert(InferDescriptor(scriptPubKey, *provider));
(np on doing it here or not)
CHECK_NONFATAL instead because it happens to be in RPC code.. But it’s not nonfatal, currently it is. Anyhow, it’s not related to this PR, so i just pushed back the previous state of this branch without the assertion.
186@@ -187,7 +187,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
187
188 std::unique_ptr<SigningProvider> provider = wallet.GetSolvingProvider(output.scriptPubKey);
189
190- bool solvable = provider ? IsSolvable(*provider, output.scriptPubKey) : false;
191+ bool solvable = provider ? IsSolvable(output.scriptPubKey, *provider) : false;
nit:
0 bool solvable = provider && IsSolvable(output.scriptPubKey, *provider)
Needs rebase but ACK 3693771b
Nice that we don’t longer need to produce so many dummy signatures. Should get us a pretty decent speedup in AvailableCoins for big wallets.
It's already checked by its (only) caller, AddAndGetMultisigDestination.
42@@ -43,4 +43,10 @@ WalletFeature GetClosestWalletFeature(int version)
43 }
44 return static_cast<WalletFeature>(0);
45 }
46+
47+bool IsSolvable(const CScript& script, const SigningProvider& provider) {
IsSolvable, with swapped arguments, is clearer than ripping out the old one at the same time. Last two commits can be same commit imo
IsSolvable helper now.
It was used back when we didn't have a concept of descriptor. Now we
can check for solvability using descriptors.
IsSolvable helper function removal.