Reintroduce IsSolvable
ClosedPublic
Actions

Authored by markblundeberg on Jan 12 2020, 12:48.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rSTAGING9dda12ee9286: Reintroduce IsSolvable
rABC9dda12ee9286: Reintroduce IsSolvable

Summary

IsSolvable was added temporarily from D3121 .. D3136. At the time it looked
like it was a witness-only thing but it gets used in some other ways in Core
codebase now and so it's useful for backports. The basic concept of
solvability is 'Do I have all the pubkeys for this in my wallet, so that
if I had the private keys, then I would be able to fully sign this input'.
It was introduced in 2015 in d3354c52d7c0c6446cad4074c1d0e04bb1b3d84e .

This reintroduces IsSolvable in a form that includes the skipped changes from
already-done backports:
D3888 (PR12714)
D3889 (PR12803)
D4736 (PR13534)
D4420 (PR13697)

Test Plan

ninja check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

issolvable

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 8877
Build 15729: Default Diff Build & Tests
Build 15728: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Jan 12 2020, 12:48

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 12 2020, 12:48

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Diff Testing started.

markblundeberg added inline comments.Jan 12 2020, 12:50

src/test/descriptor_tests.cpp
108	Note that both in ABC and Core, no tests actually have UNSOLVABLE set, so currently this is always equivalent to `BOOST_CHECK(IsSolvable(Merge(key_provider, script_provider), spks[n]))` ... but this is still an important check to do.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8877: Diff 15360.Jan 12 2020, 12:54

markblundeberg added a child revision: D4913: Merge #13142: Separate IsMine from solvability.Jan 12 2020, 12:55

deadalnix accepted this revision.Jan 12 2020, 14:01

deadalnix added inline comments.

src/test/descriptor_tests.cpp
108	Yes, this is hy it was removed, I don't think this is possible. Or is it?

This revision is now accepted and ready to land.Jan 12 2020, 14:01

markblundeberg added inline comments.Jan 13 2020, 00:39

src/test/descriptor_tests.cpp
108	The distinction is pretty subtle and almost nothing anywhere actually uses the concept of 'solvable'. There is really only one path: solvability gets reported by `listunspent` RPC wallet_importmulti.py is the only test that actually examines solvability. From there we can see the distinction as far as it matters for non-segwit: If we import a P2SH address but not the redeemscript, then we get an unsolvable watched address. If we import a P2SH address along with the redeemscript, then we get a solvable watched address. I guess the same thing applies to p2pkh (only importing address, vs. importing pubkey)? Still, I'm not really sure why this concept exists. Anyway, the solvability concept definitely gets probed more in future, see https://github.com/bitcoin/bitcoin/pull/14565