aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--doc/release-notes.md10
-rw-r--r--src/script/ismine.cpp78
-rw-r--r--src/script/ismine.h7
-rw-r--r--src/script/standard.h2
-rw-r--r--src/test/script_standard_tests.cpp9
-rw-r--r--src/wallet/rpcdump.cpp16
-rwxr-xr-xtest/functional/feature_segwit.py12
7 files changed, 90 insertions, 44 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md
index e190c78609..1554272195 100644
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -105,6 +105,16 @@ Low-level RPC changes
with any `-wallet=<path>` options, there is no change in behavior, and the
name of any wallet is just its `<path>` string.
+- Bare multisig outputs to our keys are no longer automatically treated as
+ incoming payments. As this feature was only available for multisig outputs for
+ which you had all private keys in your wallet, there was generally no use for
+ them compared to single-key schemes. Furthermore, no address format for such
+ outputs is defined, and wallet software can't easily send to it. These outputs
+ will no longer show up in `listtransactions`, `listunspent`, or contribute to
+ your balance, unless they are explicitly watched (using `importaddress` or
+ `importmulti` with hex script argument). `signrawtransaction*` also still
+ works for them.
+
### Logging
- The log timestamp format is now ISO 8601 (e.g. "2018-02-28T12:34:56Z").
diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp
index b826bcfe20..fefa02fdef 100644
--- a/src/script/ismine.cpp
+++ b/src/script/ismine.cpp
@@ -13,34 +13,36 @@
typedef std::vector<unsigned char> valtype;
-static bool HaveKeys(const std::vector<valtype>& pubkeys, const CKeyStore& keystore)
-{
- for (const valtype& pubkey : pubkeys) {
- CKeyID keyID = CPubKey(pubkey).GetID();
- if (!keystore.HaveKey(keyID)) return false;
- }
- return true;
-}
+namespace {
-isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, SigVersion sigversion)
+/**
+ * This is an enum that tracks the execution context of a script, similar to
+ * SigVersion in script/interpreter. It is separate however because we want to
+ * distinguish between top-level scriptPubKey execution and P2SH redeemScript
+ * execution (a distinction that has no impact on consensus rules).
+ */
+enum class IsMineSigVersion
{
- bool isInvalid = false;
- return IsMine(keystore, scriptPubKey, isInvalid, sigversion);
-}
+ TOP = 0, //! scriptPubKey execution
+ P2SH = 1, //! P2SH redeemScript
+ WITNESS_V0 = 2 //! P2WSH witness script execution
+};
-isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest, SigVersion sigversion)
+bool PermitsUncompressed(IsMineSigVersion sigversion)
{
- bool isInvalid = false;
- return IsMine(keystore, dest, isInvalid, sigversion);
+ return sigversion == IsMineSigVersion::TOP || sigversion == IsMineSigVersion::P2SH;
}
-isminetype IsMine(const CKeyStore &keystore, const CTxDestination& dest, bool& isInvalid, SigVersion sigversion)
+bool HaveKeys(const std::vector<valtype>& pubkeys, const CKeyStore& keystore)
{
- CScript script = GetScriptForDestination(dest);
- return IsMine(keystore, script, isInvalid, sigversion);
+ for (const valtype& pubkey : pubkeys) {
+ CKeyID keyID = CPubKey(pubkey).GetID();
+ if (!keystore.HaveKey(keyID)) return false;
+ }
+ return true;
}
-isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool& isInvalid, SigVersion sigversion)
+isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid, IsMineSigVersion sigversion)
{
isInvalid = false;
@@ -61,7 +63,7 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool&
break;
case TX_PUBKEY:
keyID = CPubKey(vSolutions[0]).GetID();
- if (sigversion != SigVersion::BASE && vSolutions[0].size() != 33) {
+ if (!PermitsUncompressed(sigversion) && vSolutions[0].size() != 33) {
isInvalid = true;
return ISMINE_NO;
}
@@ -70,20 +72,20 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool&
break;
case TX_WITNESS_V0_KEYHASH:
{
- if (!keystore.HaveCScript(CScriptID(CScript() << OP_0 << vSolutions[0]))) {
+ if (sigversion == IsMineSigVersion::TOP && !keystore.HaveCScript(CScriptID(CScript() << OP_0 << vSolutions[0]))) {
// We do not support bare witness outputs unless the P2SH version of it would be
// acceptable as well. This protects against matching before segwit activates.
// This also applies to the P2WSH case.
break;
}
- isminetype ret = ::IsMine(keystore, GetScriptForDestination(CKeyID(uint160(vSolutions[0]))), isInvalid, SigVersion::WITNESS_V0);
+ isminetype ret = IsMineInner(keystore, GetScriptForDestination(CKeyID(uint160(vSolutions[0]))), isInvalid, IsMineSigVersion::WITNESS_V0);
if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid))
return ret;
break;
}
case TX_PUBKEYHASH:
keyID = CKeyID(uint160(vSolutions[0]));
- if (sigversion != SigVersion::BASE) {
+ if (!PermitsUncompressed(sigversion)) {
CPubKey pubkey;
if (keystore.GetPubKey(keyID, pubkey) && !pubkey.IsCompressed()) {
isInvalid = true;
@@ -98,7 +100,7 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool&
CScriptID scriptID = CScriptID(uint160(vSolutions[0]));
CScript subscript;
if (keystore.GetCScript(scriptID, subscript)) {
- isminetype ret = IsMine(keystore, subscript, isInvalid);
+ isminetype ret = IsMineInner(keystore, subscript, isInvalid, IsMineSigVersion::P2SH);
if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid))
return ret;
}
@@ -106,7 +108,7 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool&
}
case TX_WITNESS_V0_SCRIPTHASH:
{
- if (!keystore.HaveCScript(CScriptID(CScript() << OP_0 << vSolutions[0]))) {
+ if (sigversion == IsMineSigVersion::TOP && !keystore.HaveCScript(CScriptID(CScript() << OP_0 << vSolutions[0]))) {
break;
}
uint160 hash;
@@ -114,7 +116,7 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool&
CScriptID scriptID = CScriptID(hash);
CScript subscript;
if (keystore.GetCScript(scriptID, subscript)) {
- isminetype ret = IsMine(keystore, subscript, isInvalid, SigVersion::WITNESS_V0);
+ isminetype ret = IsMineInner(keystore, subscript, isInvalid, IsMineSigVersion::WITNESS_V0);
if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid))
return ret;
}
@@ -123,13 +125,16 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool&
case TX_MULTISIG:
{
+ // Never treat bare multisig outputs as ours (they can still be made watchonly-though)
+ if (sigversion == IsMineSigVersion::TOP) break;
+
// Only consider transactions "mine" if we own ALL the
// keys involved. Multi-signature transactions that are
// partially owned (somebody else has a key that can spend
// them) enable spend-out-from-under-you attacks, especially
// in shared-wallet situations.
std::vector<valtype> keys(vSolutions.begin()+1, vSolutions.begin()+vSolutions.size()-1);
- if (sigversion != SigVersion::BASE) {
+ if (!PermitsUncompressed(sigversion)) {
for (size_t i = 0; i < keys.size(); i++) {
if (keys[i].size() != 33) {
isInvalid = true;
@@ -150,3 +155,22 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool&
}
return ISMINE_NO;
}
+
+} // namespace
+
+isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid)
+{
+ return IsMineInner(keystore, scriptPubKey, isInvalid, IsMineSigVersion::TOP);
+}
+
+isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey)
+{
+ bool isInvalid = false;
+ return IsMine(keystore, scriptPubKey, isInvalid);
+}
+
+isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest)
+{
+ CScript script = GetScriptForDestination(dest);
+ return IsMine(keystore, script);
+}
diff --git a/src/script/ismine.h b/src/script/ismine.h
index f93a66e35a..8573bdfbd2 100644
--- a/src/script/ismine.h
+++ b/src/script/ismine.h
@@ -33,9 +33,8 @@ typedef uint8_t isminefilter;
* different SIGVERSION may have different network rules. Currently the only use of isInvalid is indicate uncompressed
* keys in SigVersion::WITNESS_V0 script, but could also be used in similar cases in the future
*/
-isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid, SigVersion = SigVersion::BASE);
-isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, SigVersion = SigVersion::BASE);
-isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest, bool& isInvalid, SigVersion = SigVersion::BASE);
-isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest, SigVersion = SigVersion::BASE);
+isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid);
+isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey);
+isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest);
#endif // BITCOIN_SCRIPT_ISMINE_H
diff --git a/src/script/standard.h b/src/script/standard.h
index 3b2838a5bb..4922b7236b 100644
--- a/src/script/standard.h
+++ b/src/script/standard.h
@@ -23,7 +23,7 @@ class CScriptID : public uint160
{
public:
CScriptID() : uint160() {}
- CScriptID(const CScript& in);
+ explicit CScriptID(const CScript& in);
CScriptID(const uint160& in) : uint160(in) {}
};
diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp
index 767c5fdbd2..ff0bf6c66d 100644
--- a/src/test/script_standard_tests.cpp
+++ b/src/test/script_standard_tests.cpp
@@ -561,7 +561,14 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
keystore.AddKey(keys[1]);
result = IsMine(keystore, scriptPubKey, isInvalid);
- BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
+ BOOST_CHECK_EQUAL(result, ISMINE_NO);
+ BOOST_CHECK(!isInvalid);
+
+ // Keystore has 2/2 keys and the script
+ keystore.AddCScript(scriptPubKey);
+
+ result = IsMine(keystore, scriptPubKey, isInvalid);
+ BOOST_CHECK_EQUAL(result, ISMINE_NO);
BOOST_CHECK(!isInvalid);
}
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index b8533839a0..b3a59e4b2a 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -224,10 +224,11 @@ void ImportScript(CWallet* const pwallet, const CScript& script, const std::stri
}
if (isRedeemScript) {
- if (!pwallet->HaveCScript(script) && !pwallet->AddCScript(script)) {
+ const CScriptID id(script);
+ if (!pwallet->HaveCScript(id) && !pwallet->AddCScript(script)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2sh redeemScript to wallet");
}
- ImportAddress(pwallet, CScriptID(script), strLabel);
+ ImportAddress(pwallet, id, strLabel);
} else {
CTxDestination destination;
if (ExtractDestination(script, destination)) {
@@ -602,7 +603,8 @@ UniValue importwallet(const JSONRPCRequest& request)
} else if(IsHex(vstr[0])) {
std::vector<unsigned char> vData(ParseHex(vstr[0]));
CScript script = CScript(vData.begin(), vData.end());
- if (pwallet->HaveCScript(script)) {
+ CScriptID id(script);
+ if (pwallet->HaveCScript(id)) {
LogPrintf("Skipping import of %s (script already present)\n", vstr[0]);
continue;
}
@@ -613,7 +615,7 @@ UniValue importwallet(const JSONRPCRequest& request)
}
int64_t birth_time = DecodeDumpTime(vstr[1]);
if (birth_time > 0) {
- pwallet->m_script_metadata[CScriptID(script)].nCreateTime = birth_time;
+ pwallet->m_script_metadata[id].nCreateTime = birth_time;
nTimeBegin = std::min(nTimeBegin, birth_time);
}
}
@@ -899,12 +901,12 @@ UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, const int6
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet");
}
- if (!pwallet->HaveCScript(redeemScript) && !pwallet->AddCScript(redeemScript)) {
+ CScriptID redeem_id(redeemScript);
+ if (!pwallet->HaveCScript(redeem_id) && !pwallet->AddCScript(redeemScript)) {
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding p2sh redeemScript to wallet");
}
- CTxDestination redeem_dest = CScriptID(redeemScript);
- CScript redeemDestination = GetScriptForDestination(redeem_dest);
+ CScript redeemDestination = GetScriptForDestination(redeem_id);
if (::IsMine(*pwallet, redeemDestination) == ISMINE_SPENDABLE) {
throw JSONRPCError(RPC_WALLET_ERROR, "The wallet already contains the private key for this address or script");
diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py
index e835b9d777..a47c42829a 100755
--- a/test/functional/feature_segwit.py
+++ b/test/functional/feature_segwit.py
@@ -303,8 +303,10 @@ class SegWitTest(BitcoinTestFramework):
v = self.nodes[0].getaddressinfo(i)
if (v['isscript']):
[bare, p2sh, p2wsh, p2sh_p2wsh] = self.p2sh_address_to_script(v)
- # bare and p2sh multisig with compressed keys should always be spendable
- spendable_anytime.extend([bare, p2sh])
+ # p2sh multisig with compressed keys should always be spendable
+ spendable_anytime.extend([p2sh])
+ # bare multisig can be watched and signed, but is not treated as ours
+ solvable_after_importaddress.extend([bare])
# P2WSH and P2SH(P2WSH) multisig with compressed keys are spendable after direct importaddress
spendable_after_importaddress.extend([p2wsh, p2sh_p2wsh])
else:
@@ -320,8 +322,10 @@ class SegWitTest(BitcoinTestFramework):
v = self.nodes[0].getaddressinfo(i)
if (v['isscript']):
[bare, p2sh, p2wsh, p2sh_p2wsh] = self.p2sh_address_to_script(v)
- # bare and p2sh multisig with uncompressed keys should always be spendable
- spendable_anytime.extend([bare, p2sh])
+ # p2sh multisig with uncompressed keys should always be spendable
+ spendable_anytime.extend([p2sh])
+ # bare multisig can be watched and signed, but is not treated as ours
+ solvable_after_importaddress.extend([bare])
# P2WSH and P2SH(P2WSH) multisig with uncompressed keys are never seen
unseen_anytime.extend([p2wsh, p2sh_p2wsh])
else:
generated by cgit v1.2.3 (git 2.26.2) at 2024-07-20 17:27:23 +0000