From 952d8213a6d44b8eeff42f3a52defcaa1bc4a0dc Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 18 Apr 2018 14:41:32 -0700 Subject: [PATCH 1/9] Make CScript -> CScriptID conversion explicit --- src/script/standard.h | 2 +- src/wallet/rpcdump.cpp | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/script/standard.h b/src/script/standard.h index 3b2838a5b..4922b7236 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/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index b8533839a..b3a59e4b2 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 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"); From fb1dfbbec0e57d57188a47406c49487f2145549e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 18 Apr 2018 14:48:06 -0700 Subject: [PATCH 2/9] Remove unused IsMine overload --- src/script/ismine.cpp | 8 +------- src/script/ismine.h | 1 - 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp index b826bcfe2..60384670b 100644 --- a/src/script/ismine.cpp +++ b/src/script/ismine.cpp @@ -29,15 +29,9 @@ isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, SigVer } isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest, SigVersion sigversion) -{ - bool isInvalid = false; - return IsMine(keystore, dest, isInvalid, sigversion); -} - -isminetype IsMine(const CKeyStore &keystore, const CTxDestination& dest, bool& isInvalid, SigVersion sigversion) { CScript script = GetScriptForDestination(dest); - return IsMine(keystore, script, isInvalid, sigversion); + return IsMine(keystore, script, sigversion); } isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool& isInvalid, SigVersion sigversion) diff --git a/src/script/ismine.h b/src/script/ismine.h index f93a66e35..b46722ecf 100644 --- a/src/script/ismine.h +++ b/src/script/ismine.h @@ -35,7 +35,6 @@ typedef uint8_t isminefilter; */ 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); #endif // BITCOIN_SCRIPT_ISMINE_H From 19fc973097ce5d336836c7c6cf4569b4fbaec653 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 16 Apr 2018 16:10:21 -0700 Subject: [PATCH 3/9] Do not expose SigVersion argument to IsMine Only IsMine's internal code needs this, as part of a recursion into P2SH and P2WSH scripts. The exposed functions always operate on actual scriptPubKeys and not on redeemScripts or witness scripts. --- src/script/ismine.cpp | 37 +++++++++++++++++++++---------------- src/script/ismine.h | 6 +++--- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp index 60384670b..54468bc42 100644 --- a/src/script/ismine.cpp +++ b/src/script/ismine.cpp @@ -22,19 +22,7 @@ static bool HaveKeys(const std::vector& pubkeys, const CKeyStore& keyst return true; } -isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, SigVersion sigversion) -{ - bool isInvalid = false; - return IsMine(keystore, scriptPubKey, isInvalid, sigversion); -} - -isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest, SigVersion sigversion) -{ - CScript script = GetScriptForDestination(dest); - return IsMine(keystore, script, sigversion); -} - -isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool& isInvalid, SigVersion sigversion) +static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid, SigVersion sigversion) { isInvalid = false; @@ -70,7 +58,7 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool& // 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, SigVersion::WITNESS_V0); if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid)) return ret; break; @@ -92,7 +80,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, SigVersion::BASE); if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid)) return ret; } @@ -108,7 +96,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, SigVersion::WITNESS_V0); if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid)) return ret; } @@ -144,3 +132,20 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey, bool& } return ISMINE_NO; } + +isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid) +{ + return IsMineInner(keystore, scriptPubKey, isInvalid, SigVersion::BASE); +} + +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 b46722ecf..8573bdfbd 100644 --- a/src/script/ismine.h +++ b/src/script/ismine.h @@ -33,8 +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, 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 From ac6ec625227b9a7821ccd7c622b2b72982cd309c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 16 Apr 2018 16:15:02 -0700 Subject: [PATCH 4/9] Switch to a private version of SigVersion inside IsMine This will allow us to have the consensus code and IsMine code diverge. --- src/script/ismine.cpp | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp index 54468bc42..92d659569 100644 --- a/src/script/ismine.cpp +++ b/src/script/ismine.cpp @@ -13,6 +13,12 @@ typedef std::vector valtype; +enum class IsMineSigVersion +{ + BASE = 0, + WITNESS_V0 = 1 +}; + static bool HaveKeys(const std::vector& pubkeys, const CKeyStore& keystore) { for (const valtype& pubkey : pubkeys) { @@ -22,7 +28,7 @@ static bool HaveKeys(const std::vector& pubkeys, const CKeyStore& keyst return true; } -static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid, SigVersion sigversion) +static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid, IsMineSigVersion sigversion) { isInvalid = false; @@ -43,7 +49,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu break; case TX_PUBKEY: keyID = CPubKey(vSolutions[0]).GetID(); - if (sigversion != SigVersion::BASE && vSolutions[0].size() != 33) { + if (sigversion != IsMineSigVersion::BASE && vSolutions[0].size() != 33) { isInvalid = true; return ISMINE_NO; } @@ -58,14 +64,14 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu // This also applies to the P2WSH case. break; } - isminetype ret = IsMineInner(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 (sigversion != IsMineSigVersion::BASE) { CPubKey pubkey; if (keystore.GetPubKey(keyID, pubkey) && !pubkey.IsCompressed()) { isInvalid = true; @@ -80,7 +86,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu CScriptID scriptID = CScriptID(uint160(vSolutions[0])); CScript subscript; if (keystore.GetCScript(scriptID, subscript)) { - isminetype ret = IsMineInner(keystore, subscript, isInvalid, SigVersion::BASE); + isminetype ret = IsMineInner(keystore, subscript, isInvalid, IsMineSigVersion::BASE); if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid)) return ret; } @@ -96,7 +102,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu CScriptID scriptID = CScriptID(hash); CScript subscript; if (keystore.GetCScript(scriptID, subscript)) { - isminetype ret = IsMineInner(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; } @@ -111,7 +117,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu // them) enable spend-out-from-under-you attacks, especially // in shared-wallet situations. std::vector keys(vSolutions.begin()+1, vSolutions.begin()+vSolutions.size()-1); - if (sigversion != SigVersion::BASE) { + if (sigversion != IsMineSigVersion::BASE) { for (size_t i = 0; i < keys.size(); i++) { if (keys[i].size() != 33) { isInvalid = true; @@ -135,7 +141,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid) { - return IsMineInner(keystore, scriptPubKey, isInvalid, SigVersion::BASE); + return IsMineInner(keystore, scriptPubKey, isInvalid, IsMineSigVersion::BASE); } isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey) From 3619735b09c0cb1383d437527bfbd4dc023d9f76 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 16 Apr 2018 16:21:10 -0700 Subject: [PATCH 5/9] Track difference between scriptPubKey and P2SH execution in IsMine Inside IsMine we care about the distinction between scriptPubKey execution and P2SH redeemScript execution. The consensus code does not care about this distinction, and thus SigVersion does not have a field for P2SH. As the IsMine code will care, it uses a separate enum with more fields. --- src/script/ismine.cpp | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp index 92d659569..0ef4a59f1 100644 --- a/src/script/ismine.cpp +++ b/src/script/ismine.cpp @@ -13,12 +13,24 @@ typedef std::vector valtype; +/** + * 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 { - BASE = 0, - WITNESS_V0 = 1 + TOP = 0, //! scriptPubKey execution + P2SH = 1, //! P2SH redeemScript + WITNESS_V0 = 2 //! P2WSH witness script execution }; +static bool PermitsUncompressed(IsMineSigVersion sigversion) +{ + return sigversion == IsMineSigVersion::TOP || sigversion == IsMineSigVersion::P2SH; +} + static bool HaveKeys(const std::vector& pubkeys, const CKeyStore& keystore) { for (const valtype& pubkey : pubkeys) { @@ -49,7 +61,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu break; case TX_PUBKEY: keyID = CPubKey(vSolutions[0]).GetID(); - if (sigversion != IsMineSigVersion::BASE && vSolutions[0].size() != 33) { + if (!PermitsUncompressed(sigversion) && vSolutions[0].size() != 33) { isInvalid = true; return ISMINE_NO; } @@ -71,7 +83,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu } case TX_PUBKEYHASH: keyID = CKeyID(uint160(vSolutions[0])); - if (sigversion != IsMineSigVersion::BASE) { + if (!PermitsUncompressed(sigversion)) { CPubKey pubkey; if (keystore.GetPubKey(keyID, pubkey) && !pubkey.IsCompressed()) { isInvalid = true; @@ -86,7 +98,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu CScriptID scriptID = CScriptID(uint160(vSolutions[0])); CScript subscript; if (keystore.GetCScript(scriptID, subscript)) { - isminetype ret = IsMineInner(keystore, subscript, isInvalid, IsMineSigVersion::BASE); + isminetype ret = IsMineInner(keystore, subscript, isInvalid, IsMineSigVersion::P2SH); if (ret == ISMINE_SPENDABLE || ret == ISMINE_WATCH_SOLVABLE || (ret == ISMINE_NO && isInvalid)) return ret; } @@ -117,7 +129,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu // them) enable spend-out-from-under-you attacks, especially // in shared-wallet situations. std::vector keys(vSolutions.begin()+1, vSolutions.begin()+vSolutions.size()-1); - if (sigversion != IsMineSigVersion::BASE) { + if (!PermitsUncompressed(sigversion)) { for (size_t i = 0; i < keys.size(); i++) { if (keys[i].size() != 33) { isInvalid = true; @@ -141,7 +153,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid) { - return IsMineInner(keystore, scriptPubKey, isInvalid, IsMineSigVersion::BASE); + return IsMineInner(keystore, scriptPubKey, isInvalid, IsMineSigVersion::TOP); } isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey) From 08f322865429c307ea620a1e349855f9eee3af7e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 16 Apr 2018 16:36:17 -0700 Subject: [PATCH 6/9] Optimization: only test for witness scripts at top level Inside P2SH scripts we already know that the P2SH script version of witness keys/scripts are acceptable, so there is no need to test for it again. --- src/script/ismine.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp index 0ef4a59f1..eb3847d8e 100644 --- a/src/script/ismine.cpp +++ b/src/script/ismine.cpp @@ -70,7 +70,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu 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. @@ -106,7 +106,7 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu } 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; From 9c2a8b8d34c2ff6bd8611587ae883c1dc0091a45 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 16 Apr 2018 17:18:34 -0700 Subject: [PATCH 7/9] Do not treat bare multisig as IsMine Such outputs can still be watched, and signed for, but they aren't treated as valid payments. That means they won't cause transactions to appear in listtransactions, their outputs to be shown under listunspent, or affect balances. --- src/script/ismine.cpp | 3 +++ src/test/script_standard_tests.cpp | 9 ++++++++- test/functional/feature_segwit.py | 12 ++++++++---- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp index eb3847d8e..a987944f5 100644 --- a/src/script/ismine.cpp +++ b/src/script/ismine.cpp @@ -123,6 +123,9 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu 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 diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp index 767c5fdbd..ff0bf6c66 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/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index fa1732c4c..88c4d7fb2 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -311,8 +311,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: @@ -328,8 +330,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: From b61fb7113688cc25c02937f0684759dcaf247f86 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 16 Apr 2018 18:06:07 -0700 Subject: [PATCH 8/9] Mention removal of bare multisig IsMine in release notes --- doc/release-notes.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index 9e9c891de..cd1308552 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -96,6 +96,16 @@ Low-level RPC changes with any `-wallet=` options, there is no change in behavior, and the name of any wallet is just its `` 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"). From 7d0f80bbf4de8c17b6db9a035ba32698ad076e2e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 19 Apr 2018 21:06:47 -0700 Subject: [PATCH 9/9] Use anonymous namespace instead of static functions --- src/script/ismine.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp index a987944f5..fefa02fde 100644 --- a/src/script/ismine.cpp +++ b/src/script/ismine.cpp @@ -13,6 +13,8 @@ typedef std::vector valtype; +namespace { + /** * 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 @@ -26,12 +28,12 @@ enum class IsMineSigVersion WITNESS_V0 = 2 //! P2WSH witness script execution }; -static bool PermitsUncompressed(IsMineSigVersion sigversion) +bool PermitsUncompressed(IsMineSigVersion sigversion) { return sigversion == IsMineSigVersion::TOP || sigversion == IsMineSigVersion::P2SH; } -static bool HaveKeys(const std::vector& pubkeys, const CKeyStore& keystore) +bool HaveKeys(const std::vector& pubkeys, const CKeyStore& keystore) { for (const valtype& pubkey : pubkeys) { CKeyID keyID = CPubKey(pubkey).GetID(); @@ -40,7 +42,7 @@ static bool HaveKeys(const std::vector& pubkeys, const CKeyStore& keyst return true; } -static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid, IsMineSigVersion sigversion) +isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid, IsMineSigVersion sigversion) { isInvalid = false; @@ -154,6 +156,8 @@ static isminetype IsMineInner(const CKeyStore& keystore, const CScript& scriptPu return ISMINE_NO; } +} // namespace + isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid) { return IsMineInner(keystore, scriptPubKey, isInvalid, IsMineSigVersion::TOP);