Page MenuHomePhabricator

Reintroduce IsSolvable
ClosedPublic

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

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markblundeberg created this revision.Sun, Jan 12, 12:48
Herald added a reviewer: Restricted Project. · View Herald TranscriptSun, Jan 12, 12:48
markblundeberg added inline comments.Sun, Jan 12, 12:50
src/test/descriptor_tests.cpp
108 ↗(On Diff #15360)

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.

deadalnix accepted this revision.Sun, Jan 12, 14:01
deadalnix added inline comments.
src/test/descriptor_tests.cpp
108 ↗(On Diff #15360)

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.Sun, Jan 12, 14:01
markblundeberg added inline comments.Mon, Jan 13, 00:39
src/test/descriptor_tests.cpp
108 ↗(On Diff #15360)

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