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 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

1 files changed, 43 insertions, 0 deletions

diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp
new file mode 100644
index 0000000000..757865ea37
--- /dev/null
+++ b/src/wallet/test/scriptpubkeyman_tests.cpp
@@ -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()