watch only multisig scripts require pubkeys to be imported to flag them solvable #14415

Use case: decouple keys from the wallet (via importing scripts and fundrawtransaction with watch-only).

Problem: multisig inputs are flagged as non-spendable even if importing both the multisig P2SH scriptPubKey and the redeemScript (via importmulti).

The current isSolveable() logic calls GetPubKey() and only flags them as solvable if the pubkeys are in the wallet.

Expected behaviour:

• multisig Inputs should be flagged as solvable (and therefore usable via fundrawtransaction) when importing the P2(W)SH scriptPubKey and the redeemScripts

Current workaround:

• Call importpubkey for all pubkeys used in the multisig (redeemScript). importpubkey derives all coming scripts (including a raw P2PK script). The P2PK scripts make the wallet store the pubkey and flag the multisig script solvable.

IRC discussion:

[12:06:00]  <jonasschnelli>	The only way how to import a watch only multisig is by importing the outer script, the readme-script and all it's pubkeys (via importpubkey)
[12:06:30]  <sipa>	you don't need the pubkeys for multisig
[12:06:34]  <sipa>	only for p2pkh
[12:06:51]  <jonasschnelli>	sipa:If you don't import the pubkey they are non-solvable and not useable in fundraw
[12:07:06]  <jonasschnelli>	So you need to import via importpubkey (and import all possible scripts!)
[12:07:28]  <sipa>	jonasschnelli: that makes no sense
[12:07:31]  <jonasschnelli>	GetScriptForRawPubKey will then lead to extract the pubKey and place them mapWatchKeys
[12:07:43]  <jonasschnelli>	Agree it makes no sense...
[12:07:55]  <sipa>	importing a pubkey is so that the signing code can find the pubkey based on the hash of the key
[12:08:03]  <sipa>	in a multisig there are no key hashes involved
[12:08:13]  <sipa>	just the redeemscript should be enough for multisig
[12:08:39]  <sipa>	plus importaddress of the toplevel thing
[12:08:43]  <jonasschnelli>	Yes. But following isSolvable, it will require to find the pubkey in oder to flag it solveable
[12:09:00]  <sipa>	it has the pubkey; it's in the redeemscript
[12:09:06]  <sipa>	i'm very skeptical :)
[12:09:11]  <jonasschnelli>	but it won't find it...
[12:09:19]  <sipa>	have you tried this?
[12:09:29]  <jonasschnelli>	Yes
[12:09:32]  <sipa>	i find this very hard to believe
[12:09:37]  <jonasschnelli>	mapWatchKeys gets only touched by AddWatchOnly
[12:09:44]  <jonasschnelli>	Try yourself... with the following steps:
[12:09:49]  <jonasschnelli>	1. createwallet "dummy" true
[12:09:53]  <sipa>	what do you need mapWatchKeys for in multisig?
[12:09:54] 	escrivner (~user@cpe-76-175-74-114.socal.res.rr.com) left IRC (Ping timeout: 252 seconds)
[12:10:37]  <jonasschnelli>	sipa: pure watching no,.. but if you want to use them for watchOnly coinselection via fundraw, yes, you need it with the corrent code
[12:10:47]  <sipa>	that makes no sense :)
[12:10:55]  <sipa>	if you say so, i believe you
[12:11:01]  <sipa>	bit i don't know what that is the case
[12:11:03]  <jonasschnelli>	Try it youself. :)
[12:11:09]  <sipa>	i can't right now
[12:11:16]  <jonasschnelli>	Sure. No hurry...
[12:11:24]  <jonasschnelli>	the use case: decouple the keys from the wallet
[12:11:44]  <jonasschnelli>	(via importing scripts and using fundrawtransaction)
[12:11:44]  <sipa>	yes not claiming this isn't important!
[12:11:57]  <sipa>	i'm just not understanding why you need to import the pubkeys
[12:12:06]  <sipa>	they are in the redeemscript
[12:12:28]  <sipa>	the signing code doesn't need to look up any pubkeys
[12:12:45]  <jonasschnelli>	Because: fundraw takes only solvable imputs. isSolveable requires to find the pubKey in the keystore.
[12:12:53]  <jonasschnelli>	But it's fixable IMO
[12:12:54]  <sipa>	why?
[12:13:23]  <jonasschnelli>	isSolveable _currently_ requires to find the pubKey... its just coded that way...
[12:13:50]  <sipa>	can you point me to the code?
[12:14:01]  <jonasschnelli>	1s..
[12:14:49]  <jonasschnelli>	code part 1: https://github.com/bitcoin/bitcoin/blob/master/src/script/sign.cpp#L141
[12:15:36]  <sipa>	sure, but that doesn't need the pubkey imported, only the redeemscript
[12:16:25]  <jonasschnelli>	code part 2: https://github.com/bitcoin/bitcoin/blob/aa39ca764578a9017e03796b554955796022eb0d/src/script/sign.cpp#L84
[12:16:31]  <jonasschnelli>	(GetPubKey(provider, sigdata, keyid, pubkey);)
[12:16:46]  <jonasschnelli>	Goes into: bool CCryptoKeyStore::GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const
[12:16:51]  <sipa>	oh
[12:16:55]  <sipa>	you're right
[12:16:56] 	phwalkr (~phwalkr@194.210.91.110) left IRC (Remote host closed the connection)
[12:17:03]  <sipa>	that's stupid :)
[12:17:08]  <jonasschnelli>	yes... it's a mess
[12:17:30] sipa	quietly mentions descriptors again
[12:17:44]  <jonasschnelli>	Indeed!
[12:17:47] 	escrivner (~user@cpe-76-175-74-114.socal.res.rr.com) joined the channel
[12:18:23]  <jonasschnelli>	You can bypass that problem by importpubkey all the multisigs pubkeys because it will generate a RawPubKeyScript where the pubkeys gets extracted and added to the keystore. :/
[12:18:36]  <jonasschnelli>	(with an overhead of a couple of unused scripts)
[12:18:48]  <sipa>	yeah
[12:18:56]  <sipa>	but that's pretty ridiculous

Seems to be a 0.17 regression (just tested on 0.16.3 and seems that the inputs there are marked as solvable).

@jonasschnelli this is fixed, right?

Since #14424 was merged, this can be closed now.