diff --git a/src/outputtype.cpp b/src/outputtype.cpp --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -53,12 +53,10 @@ OutputType type) { // Add script to keystore keystore.AddCScript(script); - ScriptHash sh(script); // Note that scripts over 520 bytes are not yet supported. switch (type) { case OutputType::LEGACY: - keystore.AddCScript(GetScriptForDestination(sh)); - return sh; + return ScriptHash(script); default: assert(false); } diff --git a/src/script/signingprovider.h b/src/script/signingprovider.h --- a/src/script/signingprovider.h +++ b/src/script/signingprovider.h @@ -78,7 +78,29 @@ using KeyMap = std::map; using ScriptMap = std::map; + /** + * Map of key id to unencrypted private keys known by the signing provider. + * Map may be empty if the provider has another source of keys, like an + * encrypted store. + */ KeyMap mapKeys GUARDED_BY(cs_KeyStore); + + /** + * (This comment has been elided for clarity since most of it detailed Core + * SegWit implementation details, see Core commit 005f8a9) + * + * The FillableSigningProvider::mapScripts script map should not be confused + * with LegacyScriptPubKeyMan::setWatchOnly script set. The two collections + * can hold the same scripts, but they serve different purposes. The + * setWatchOnly script set is intended to expand the set of outputs the + * wallet considers payments. Every output with a script it contains is + * considered to belong to the wallet, regardless of whether the script is + * solvable or signable. By contrast, the scripts in mapScripts are only + * used for solving, and to restrict which outputs are considered payments + * by the wallet. An output with a script in mapScripts, unlike + * setWatchOnly, is not automatically considered to belong to the wallet if + * it can't be solved and signed for. + */ ScriptMap mapScripts GUARDED_BY(cs_KeyStore); public: diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -68,6 +68,7 @@ ../wallet/test/init_tests.cpp ../wallet/test/ismine_tests.cpp ../wallet/test/psbt_wallet_tests.cpp + ../wallet/test/scriptpubkeyman_tests.cpp ../wallet/test/wallet_tests.cpp ../wallet/test/walletdb_tests.cpp ../wallet/test/wallet_crypto_tests.cpp diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -70,9 +70,19 @@ return true; } +//! Recursively solve script and return spendable/watchonly/invalid status. +//! +//! @param keystore legacy key and script store +//! @param script script to solve +//! @param sigversion script type (top-level / redeemscript / +//! witnessscript) +//! @param recurse_scripthash whether to recurse into nested p2sh and p2wsh +//! scripts or simply treat any script that has been +//! stored in the keystore as spendable IsMineResult IsMineInner(const LegacyScriptPubKeyMan &keystore, const CScript &scriptPubKey, - IsMineSigVersion sigversion) { + IsMineSigVersion sigversion, + bool recurse_scripthash = true) { IsMineResult ret = IsMineResult::NO; std::vector vSolutions; @@ -103,8 +113,10 @@ CScriptID scriptID = CScriptID(uint160(vSolutions[0])); CScript subscript; if (keystore.GetCScript(scriptID, subscript)) { - ret = std::max(ret, IsMineInner(keystore, subscript, - IsMineSigVersion::P2SH)); + ret = std::max(ret, recurse_scripthash + ? IsMineInner(keystore, subscript, + IsMineSigVersion::P2SH) + : IsMineResult::SPENDABLE); } break; } @@ -438,13 +450,13 @@ bool LegacyScriptPubKeyMan::CanProvide(const CScript &script, SignatureData &sigdata) { - if (IsMine(script) != ISMINE_NO) { - // If it IsMine, we can always provide in some way - return true; - } - if (HaveCScript(CScriptID(script))) { - // We can still provide some stuff if we have the script, but IsMine - // failed because we don't have keys + IsMineResult ismine = IsMineInner(*this, script, IsMineSigVersion::TOP, + /* recurse_scripthash= */ false); + if (ismine == IsMineResult::SPENDABLE || + ismine == IsMineResult::WATCH_ONLY) { + // If ismine, it means we recognize keys or script ids in the script, or + // are watching the script itself, and we can at least provide metadata + // or solving information, even if not able to sign fully. return true; } // If, given the stuff in sigdata, we could make a valid sigature, then diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp new file mode 100644 --- /dev/null +++ b/src/wallet/test/scriptpubkeyman_tests.cpp @@ -0,0 +1,45 @@ +// 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 +#include +#include