wallet: Improve LegacyScriptPubKeyMan::CanProvide script recognition

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
https://github.com/bitcoin/bitcoin/pull/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 4a7e43e846
"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
4a7e43e846 "Store p2sh scripts in
AddAndGetDestinationForScript", so the AddAndGetDestinationForScript() function,
CanProvide() method, and mapScripts map should all be more comprehensible
This commit is contained in:
Russell Yanofsky
2020-02-04 17:24:48 -05:00
parent 2bdc476d4d
commit 005f8a92cc
4 changed files with 107 additions and 9 deletions

View File

@@ -0,0 +1,43 @@
// Copyright (c) 2020 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <key.h>
#include <script/standard.h>
#include <test/util/setup_common.h>
#include <wallet/scriptpubkeyman.h>
#include <wallet/wallet.h>
#include <boost/test/unit_test.hpp>
BOOST_FIXTURE_TEST_SUITE(scriptpubkeyman_tests, BasicTestingSetup)
// Test LegacyScriptPubKeyMan::CanProvide behavior, making sure it returns true
// for recognized scripts even when keys may not be available for signing.
BOOST_AUTO_TEST_CASE(CanProvide)
{
// Set up wallet and keyman variables.
NodeContext node;
std::unique_ptr<interfaces::Chain> chain = interfaces::MakeChain(node);
CWallet wallet(chain.get(), WalletLocation(), WalletDatabase::CreateDummy());
LegacyScriptPubKeyMan& keyman = *wallet.GetOrCreateLegacyScriptPubKeyMan();
// Make a 1 of 2 multisig script
std::vector<CKey> keys(2);
std::vector<CPubKey> pubkeys;
for (CKey& key : keys) {
key.MakeNewKey(true);
pubkeys.emplace_back(key.GetPubKey());
}
CScript multisig_script = GetScriptForMultisig(1, pubkeys);
CScript p2sh_script = GetScriptForDestination(ScriptHash(multisig_script));
SignatureData data;
// Verify the p2sh(multisig) script is not recognized until the multisig
// script is added to the keystore to make it solvable
BOOST_CHECK(!keyman.CanProvide(p2sh_script, data));
keyman.AddCScript(multisig_script);
BOOST_CHECK(keyman.CanProvide(p2sh_script, data));
}
BOOST_AUTO_TEST_SUITE_END()