Page MenuHomePhabricator
Files F13115305

D9115.diff
No OneTemporary
Actions

D9115.diff
View Options

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<CKeyID, CKey>;
using ScriptMap = std::map<CScriptID, CScript>;
+ /**
+ * 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<valtype> 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 <chainparams.h>
+#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, Params());
+ 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()

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 1, 10:46 (11 h, 39 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5187391
Default Alt Text
D9115.diff (7 KB)

Event Timeline