Merge #13491: Improve handling of INVALID in IsMine

bb582a59c Add P2WSH destination helper and use it instead of manual hashing (Pieter Wuille)
eaba1c111 Add additional unit tests for invalid IsMine combinations (Pieter Wuille)
e6b9730c4 Do not expose invalidity from IsMine (Pieter Wuille)

Pull request description:

  This improves the handling of INVALID in IsMine:
  * Extra INVALID conditions were added to `IsMine` (following https://github.com/bitcoin/bitcoin/pull/13142/files#r185349057), but these were untested. Add unit tests for them.
  * In https://github.com/bitcoin/bitcoin/pull/13142#issuecomment-386396975 it was suggested to merge `isInvalid` into the return status. This PR takes a different approach, and removes the `isInvalid` entirely. It was only ever used inside tests, as normal users of IsMine don't care about the reason for non-mine-ness, only whether it is or not. As the unit tests are extensive enough, it seems sufficient to have a black box text (with tests for both compressed and uncompressed keys).

  Some addition code simplification is done as well.

Tree-SHA512: 3267f8846f3fa4e994f57504b155b0e1bbdf13808c4c04dab7c6886c2c0b88716169cee9c5b350513297e0ca2a00812e3401acf30ac9cde5d892f9fb59ad7fef

1 files changed, 2 insertions, 10 deletions

diff --git a/src/script/ismine.cpp b/src/script/ismine.cpp
index 43dd9e582..8c2686648 100644
--- a/src/script/ismine.cpp
+++ b/src/script/ismine.cpp
@@ -38,7 +38,7 @@ enum class IsMineResult
     NO = 0,          //! Not ours
     WATCH_ONLY = 1,  //! Included in watch-only balance
     SPENDABLE = 2,   //! Included in all balances
-    INVALID = 3,     //! Not spendable by anyone
+    INVALID = 3,     //! Not spendable by anyone (uncompressed pubkey in segwit, P2SH inside P2SH or witness, witness inside witness)
 };
 
 bool PermitsUncompressed(IsMineSigVersion sigversion)
@@ -173,12 +173,10 @@ IsMineResult IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey,
 
 } // namespace
 
-isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool& isInvalid)
+isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey)
 {
-    isInvalid = false;
     switch (IsMineInner(keystore, scriptPubKey, IsMineSigVersion::TOP)) {
     case IsMineResult::INVALID:
-        isInvalid = true;
     case IsMineResult::NO:
         return ISMINE_NO;
     case IsMineResult::WATCH_ONLY:
@@ -189,12 +187,6 @@ isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey, bool&
     assert(false);
 }
 
-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);