IsMine: Set state to WATCH_ONLY if we can get the pubkey

[achow101]

This fixes a logic bug in watch only detection.

Split from #17261

[Sjors]

ACK 9972b28, trusting my previous self here, I remember understanding what this did :-) #16341 (comment)

[maflcko]

When was this bug introduced? Does it need backport? Would a regression test be appropriate?

[achow101]

Looking at this much more closely, I'm not entirely sure that this is really a bug fix or whether it is needed.

The reason this was included in the ScriptPubKeyMan PR was that because IsMine is relied upon more heavily in the ScriptPubKeyMan model, wallet_importmulti.py was failing. It failed in the specific case where a legacy address was fetched, and from its information, a wpkh() descriptor (so an output type that does not match the address) was imported into another wallet. Then getaddressinfo was called using the original legacy address but on the imported wallet and it failed to find any information. This change was introduced to make the test pass, but perhaps it would have been better to change the test to be consistent with the address types.

Furthermore, the wallet with the import does not recognize the legacy address as being watch only. getaddressinfo reports it as not watch only and coins sent to that address are not detected by the wallet. This PR would change that, furthering the "any pubkey we have" behavior of the ismine, which I don't think is actually desirable. But, at the same time, the current behavior is that we are able to get metadata about the legacy address (we return the pubkey and key origin info). After the rest of the ScriptPubKeyMan refactor, this would no longer be true; getaddressinfo will not return any information about the legacy address and treat it as any other third party address unrelated to the wallet even though we have the pubkey.

And lastly, the current behavior is that we do have this weird state where the legacy address is not watch only but is solvable. So we could add key origin information to a PSBT which has inputs spending from this address but the wallet wouldn't be tracking that address. ScriptPubKeyMan does change that behavior too (due to checking IsMine).

[instagibbs]

this might not be worth fixing. AFAIK it was only introduced with descriptor import, but the behavior was never a regression as such.

[Sjors]

Paging @sipa.