Allow to use unsolveable P2PKH watch-only in fundrawtransaction #9965

pull jonasschnelli wants to merge 1 commits into bitcoin:master from jonasschnelli:2017/03/watch_solve changing 5 files +21 −4

jonasschnelli commented at 1:59 PM on March 9, 2017: contributor

I'm not entirely sure if this is a good thing. But, I don't see a reason why I can't use an imported P2PKH watch-only-address in conjunction with fundrawtransaction when enabling includeWatching.

This marks P2PKH watch only scripts as ISMINE_WATCH_SOLVABLE. The second change makes the DummySignatureCreator() no longer verifying the produced signature. The DummySignatureCreator is only used for calculating the transaction size and therefore the verification of the script should not matter.
jonasschnelli added the label Wallet on Mar 9, 2017
jonasschnelli force-pushed on Mar 9, 2017
paveljanik commented at 7:35 AM on March 10, 2017: contributor

segwit.py fails on two builds.
Allow to use unsolveable P2PKH watch-only in fundrawtransaction ca003b407b
jonasschnelli force-pushed on Mar 10, 2017
jonasschnelli commented at 10:03 AM on March 10, 2017: contributor

Fixed the segwit.py test.

in src/script/sign.cpp:None in ca003b407b

 183 | @@ -184,7 +184,7 @@ bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& fromPu
 184 |      sigdata.scriptSig = PushAll(result);
 185 |  
 186 |      // Test solution
 187 | -    return solved && VerifyScript(sigdata.scriptSig, fromPubKey, &sigdata.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker());
 188 | +    return solved && (!creator.ShouldVerifySig() || VerifyScript(sigdata.scriptSig, fromPubKey, &sigdata.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker()));

TheBlueMatt commented at 10:10 PM on March 10, 2017:

Is this neccessary? it looks like the Checker here should just always return true if its the DummySignatureChecker from the DummySignatureCreator passed in by IsMine().

NicolasDorier commented at 2:43 AM on March 12, 2017:

Not necessary, the DummySignatureCreator already bypass signature checking.

NicolasDorier commented at 2:41 AM on March 12, 2017: contributor

Unsolveable P2PKH watch-only is already possible but you need to import the public key. Also, your ShouldVerifySig is not needed, DummySignatureCreator already bypass the sig.

The reason you got unsolvable is because the wallet does not know the pubkey of the p2pkh.

I think the current behavior make sense.
sipa commented at 4:41 AM on March 12, 2017: member

I'm not sure this is a good idea. If you want it to be solvable, you need to import the public key. Without it, you don't know whether the public key is compressed or not, so you can't actually accurately estimate the input size, so it's unsolvable for a reason.
jonasschnelli commented at 7:07 PM on March 26, 2017: contributor

Thanks @sipa. I kinda falsely ignored the fact that the pubkey could be uncompressed. Your right. Closing.
jonasschnelli closed this on Mar 26, 2017
DrahtBot locked this on Sep 8, 2021

Contributors

Labels