Page MenuHomePhabricator

[backport#18067] wallet: Improve LegacyScriptPubKeyMan::CanProvide script recognition
ClosedPublic

Authored by majcosta on Jan 31 2021, 07:46.

Details

Summary

a304a3632f0437f4d0f04589a2200e2da91624a7 Revert "Store p2sh scripts in AddAndGetDestinationForScript" (Russell Yanofsky)
eb7d8a5b07e89133a5fb465ad1b793362e7439f7 [test] check for addmultisigaddress regression (Sjors Provoost)
005f8a92ccb5bc10c8daa106d75e1c21390461d3 wallet: Improve LegacyScriptPubKeyMan::CanProvide script recognition (Russell Yanofsky)

Pull request description:

Make `LegacyScriptPubKeyMan::CanProvide` method able to recognize p2sh scripts when the redeem script is present in the `mapScripts` map without the p2sh script also having to be added to the `mapScripts` map. This restores behavior prior to #17261, which I think broke backwards compatibility with old wallet files by no longer treating addresses created by `addmultisigaddress` calls before #17261 as solvable.

The reason why tests didn't fail with the CanProvide implementation in #17261 is because of a workaround added in 4a7e43e8460127a40a7895519587399feff3b682 "Store p2sh scripts in AddAndGetDestinationForScript", which masked the problem for new `addmultisigaddress` RPC calls without fixing it for multisig addresses already created in old wallet files.

This change adds a lot of comments and allows reverting commit 4a7e43e8460127a40a7895519587399feff3b682 "Store p2sh scripts in AddAndGetDestinationForScript", so the `AddAndGetDestinationForScript()` function, `CanProvide()` method, and `mapScripts` map should all be more comprehensible

Backport of Core PR18067

Test Plan
ninja all check check-functional

Event Timeline

PiRK requested changes to this revision.Feb 1 2021, 06:27
PiRK added a subscriber: PiRK.
PiRK added inline comments.
src/script/signingprovider.h
91 ↗(On Diff #27436)

This entire paragraph seems related to SegWit.

src/wallet/scriptpubkeyman.cpp
78 ↗(On Diff #27436)

indent

79 ↗(On Diff #27436)

I would remove "and p2wsh".

This revision now requires changes to proceed.Feb 1 2021, 06:27

elided comment of SegWit information and left a trail of crumbs for archival reasons

This revision is now accepted and ready to land.Feb 2 2021, 07:38