Stop requiring imported pubkey to sign non-PKH schemes #14424

pull sipa wants to merge 1 commits into bitcoin:master from sipa:201810_importpubkeylol changing 1 files +10 −7

sipa commented at 2:12 PM on October 7, 2018: member

...

This is a fix for #14415
promag commented at 10:01 PM on October 7, 2018: member

Test failing https://github.com/bitcoin/bitcoin/blob/a23e50fcd175a038690bd196cf0573fd98435e47/src/wallet/test/psbt_wallet_tests.cpp#L71
meshcollider commented at 12:56 AM on October 8, 2018: contributor

This is a fix for #14415 right?
sipa commented at 12:58 AM on October 8, 2018: member

@MeshCollider Hopefully.
MarcoFalke added the label Wallet on Oct 8, 2018
Stop requiring imported pubkey to sign non-PKH schemes 2f6b466aeb
in src/script/sign.cpp:84 in a23e50fcd1 outdated
```
  81 |      if (it != sigdata.signatures.end()) {
  82 |          sig_out = it->second.second;
  83 |          return true;
  84 |      }
  85 | -    CPubKey pubkey;
  86 | -    GetPubKey(provider, sigdata, keyid, pubkey);
```
achow101 commented at 4:15 AM on October 8, 2018:

Removing this is also breaking the PSBT updating test. Without it, we don't know which pubkeys are needed for signing in order to properly update a PSBT with the pubkeys.

instagibbs commented at 5:04 AM on October 8, 2018:

sorry can you point to where this breaks stuff exactly, non-obvious to me

achow101 commented at 5:11 AM on October 8, 2018:

PSBT relies on the GetPubKey helper function to find out which public keys are used and to add the public key to a SignatureData object. The public keys are taken from the sigdata after ProduceSignature is done and moved into the PSBT. Without this call to GetPubKey, the pubkeys that were needed during signing are not added to the sigdata and thus not added to the PSBT. This causes the test to fail (and further breaks PSBT updating) as public keys that are watch only are not being added to the PSBT.

sipa commented at 5:19 AM on October 8, 2018:

Possibly fixed.
sipa force-pushed on Oct 8, 2018
instagibbs commented at 6:48 AM on October 8, 2018: member

one build was failing to finish building(?), kicked
achow101 commented at 12:42 AM on October 9, 2018: member

utACK 2f6b466aeb6d4c88ab2e0e8b2a402be0743608b5
instagibbs commented at 1:48 AM on October 9, 2018: member

utACK https://github.com/bitcoin/bitcoin/pull/14424/commits/2f6b466aeb6d4c88ab2e0e8b2a402be0743608b5
sipa added this to the milestone 0.17.1 on Oct 9, 2018
sipa commented at 2:00 AM on October 9, 2018: member

Marked as 0.17.1.
meshcollider commented at 4:56 AM on October 9, 2018: contributor

utACK https://github.com/bitcoin/bitcoin/pull/14424/commits/2f6b466aeb6d4c88ab2e0e8b2a402be0743608b5
sipa added the label Needs backport on Oct 12, 2018
jonasschnelli commented at 10:48 AM on October 15, 2018: contributor

Tested ACK 2f6b466aeb6d4c88ab2e0e8b2a402be0743608b5, fixes #14415
jonasschnelli merged this on Oct 15, 2018
jonasschnelli closed this on Oct 15, 2018
jonasschnelli referenced this in commit 2a2cac7873 on Oct 15, 2018
MarcoFalke referenced this in commit a4bf7bed70 on Oct 24, 2018
laanwj referenced this in commit c6e4a1fca5 on Dec 6, 2018
fanquake commented at 1:41 PM on December 9, 2018: member

Backported in #14889.
fanquake removed the label Needs backport on Dec 9, 2018
deadalnix referenced this in commit d8f89cfc89 on May 11, 2020
ftrader referenced this in commit 08af019481 on Aug 17, 2020
Munkybooty referenced this in commit 985ea09da1 on Jul 21, 2021
Munkybooty referenced this in commit 4f3f89fe55 on Jul 22, 2021
linuxsh2 referenced this in commit b0e7af7b72 on Jul 29, 2021
kittywhiskers referenced this in commit 833a68f8eb on Aug 5, 2021
kittywhiskers referenced this in commit 230df0b53e on Aug 5, 2021
kittywhiskers referenced this in commit 1099ae7f6c on Aug 5, 2021
kittywhiskers referenced this in commit a233e751a5 on Aug 9, 2021
UdjinM6 referenced this in commit 7aebf156e9 on Aug 10, 2021
5tefan referenced this in commit 73ceedbc31 on Aug 12, 2021
DrahtBot locked this on Sep 8, 2021