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, 113 insertions, 136 deletions

diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp
index 7ab0978228..7d4734986a 100644
--- a/src/test/script_standard_tests.cpp
+++ b/src/test/script_standard_tests.cpp
@@ -398,106 +398,149 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
 
     CScript scriptPubKey;
     isminetype result;
-    bool isInvalid;
 
     // P2PK compressed
     {
         CBasicKeyStore keystore;
-        scriptPubKey.clear();
-        scriptPubKey << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
+        scriptPubKey = GetScriptForRawPubKey(pubkeys[0]);
 
         // Keystore does not have key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key
         keystore.AddKey(keys[0]);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2PK uncompressed
     {
         CBasicKeyStore keystore;
-        scriptPubKey.clear();
-        scriptPubKey << ToByteVector(uncompressedPubkey) << OP_CHECKSIG;
+        scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey);
 
         // Keystore does not have key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key
         keystore.AddKey(uncompressedKey);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2PKH compressed
     {
         CBasicKeyStore keystore;
-        scriptPubKey.clear();
-        scriptPubKey << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+        scriptPubKey = GetScriptForDestination(pubkeys[0].GetID());
 
         // Keystore does not have key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key
         keystore.AddKey(keys[0]);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2PKH uncompressed
     {
         CBasicKeyStore keystore;
-        scriptPubKey.clear();
-        scriptPubKey << OP_DUP << OP_HASH160 << ToByteVector(uncompressedPubkey.GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
+        scriptPubKey = GetScriptForDestination(uncompressedPubkey.GetID());
 
         // Keystore does not have key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key
         keystore.AddKey(uncompressedKey);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2SH
     {
         CBasicKeyStore keystore;
 
-        CScript redeemScript;
-        redeemScript << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
-
-        scriptPubKey.clear();
-        scriptPubKey << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+        CScript redeemScript = GetScriptForDestination(pubkeys[0].GetID());
+        scriptPubKey = GetScriptForDestination(CScriptID(redeemScript));
 
         // Keystore does not have redeemScript or key
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has redeemScript but no key
         keystore.AddCScript(redeemScript);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has redeemScript and key
         keystore.AddKey(keys[0]);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
+    }
+
+    // (P2PKH inside) P2SH inside P2SH (invalid)
+    {
+        CBasicKeyStore keystore;
+
+        CScript redeemscript_inner = GetScriptForDestination(pubkeys[0].GetID());
+        CScript redeemscript = GetScriptForDestination(CScriptID(redeemscript_inner));
+        scriptPubKey = GetScriptForDestination(CScriptID(redeemscript));
+
+        keystore.AddCScript(redeemscript);
+        keystore.AddCScript(redeemscript_inner);
+        keystore.AddCScript(scriptPubKey);
+        keystore.AddKey(keys[0]);
+        result = IsMine(keystore, scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+    }
+
+    // (P2PKH inside) P2SH inside P2WSH (invalid)
+    {
+        CBasicKeyStore keystore;
+
+        CScript redeemscript = GetScriptForDestination(pubkeys[0].GetID());
+        CScript witnessscript = GetScriptForDestination(CScriptID(redeemscript));
+        scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessscript));
+
+        keystore.AddCScript(witnessscript);
+        keystore.AddCScript(redeemscript);
+        keystore.AddCScript(scriptPubKey);
+        keystore.AddKey(keys[0]);
+        result = IsMine(keystore, scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+    }
+
+    // P2WPKH inside P2WSH (invalid)
+    {
+        CBasicKeyStore keystore;
+
+        CScript witnessscript = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0].GetID()));
+        scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessscript));
+
+        keystore.AddCScript(witnessscript);
+        keystore.AddCScript(scriptPubKey);
+        keystore.AddKey(keys[0]);
+        result = IsMine(keystore, scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
+    }
+
+    // (P2PKH inside) P2WSH inside P2WSH (invalid)
+    {
+        CBasicKeyStore keystore;
+
+        CScript witnessscript_inner = GetScriptForDestination(pubkeys[0].GetID());
+        CScript witnessscript = GetScriptForDestination(WitnessV0ScriptHash(witnessscript_inner));
+        scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessscript));
+
+        keystore.AddCScript(witnessscript_inner);
+        keystore.AddCScript(witnessscript);
+        keystore.AddCScript(scriptPubKey);
+        keystore.AddKey(keys[0]);
+        result = IsMine(keystore, scriptPubKey);
+        BOOST_CHECK_EQUAL(result, ISMINE_NO);
     }
 
     // P2WPKH compressed
@@ -505,14 +548,12 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         CBasicKeyStore keystore;
         keystore.AddKey(keys[0]);
 
-        scriptPubKey.clear();
-        scriptPubKey << OP_0 << ToByteVector(pubkeys[0].GetID());
+        scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0].GetID()));
 
         // Keystore implicitly has key and P2SH redeemScript
         keystore.AddCScript(scriptPubKey);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2WPKH uncompressed
@@ -520,56 +561,45 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         CBasicKeyStore keystore;
         keystore.AddKey(uncompressedKey);
 
-        scriptPubKey.clear();
-        scriptPubKey << OP_0 << ToByteVector(uncompressedPubkey.GetID());
+        scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(uncompressedPubkey.GetID()));
 
         // Keystore has key, but no P2SH redeemScript
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has key and P2SH redeemScript
         keystore.AddCScript(scriptPubKey);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(isInvalid);
     }
 
     // scriptPubKey multisig
     {
         CBasicKeyStore keystore;
 
-        scriptPubKey.clear();
-        scriptPubKey << OP_2 <<
-            ToByteVector(uncompressedPubkey) <<
-            ToByteVector(pubkeys[1]) <<
-            OP_2 << OP_CHECKMULTISIG;
+        scriptPubKey = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
 
         // Keystore does not have any keys
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has 1/2 keys
         keystore.AddKey(uncompressedKey);
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has 2/2 keys
         keystore.AddKey(keys[1]);
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         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);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2SH multisig
@@ -578,25 +608,17 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         keystore.AddKey(uncompressedKey);
         keystore.AddKey(keys[1]);
 
-        CScript redeemScript;
-        redeemScript << OP_2 <<
-            ToByteVector(uncompressedPubkey) <<
-            ToByteVector(pubkeys[1]) <<
-            OP_2 << OP_CHECKMULTISIG;
-
-        scriptPubKey.clear();
-        scriptPubKey << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+        CScript redeemScript = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
+        scriptPubKey = GetScriptForDestination(CScriptID(redeemScript));
 
         // Keystore has no redeemScript
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has redeemScript
         keystore.AddCScript(redeemScript);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2WSH multisig with compressed keys
@@ -605,35 +627,22 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         keystore.AddKey(keys[0]);
         keystore.AddKey(keys[1]);
 
-        CScript witnessScript;
-        witnessScript << OP_2 <<
-            ToByteVector(pubkeys[0]) <<
-            ToByteVector(pubkeys[1]) <<
-            OP_2 << OP_CHECKMULTISIG;
-
-        uint256 scriptHash;
-        CSHA256().Write(&witnessScript[0], witnessScript.size())
-            .Finalize(scriptHash.begin());
-
-        scriptPubKey.clear();
-        scriptPubKey << OP_0 << ToByteVector(scriptHash);
+        CScript witnessScript = GetScriptForMultisig(2, {pubkeys[0], pubkeys[1]});
+        scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessScript));
 
         // Keystore has keys, but no witnessScript or P2SH redeemScript
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has keys and witnessScript, but no P2SH redeemScript
         keystore.AddCScript(witnessScript);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has keys, witnessScript, P2SH redeemScript
         keystore.AddCScript(scriptPubKey);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // P2WSH multisig with uncompressed key
@@ -642,75 +651,47 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         keystore.AddKey(uncompressedKey);
         keystore.AddKey(keys[1]);
 
-        CScript witnessScript;
-        witnessScript << OP_2 <<
-            ToByteVector(uncompressedPubkey) <<
-            ToByteVector(pubkeys[1]) <<
-            OP_2 << OP_CHECKMULTISIG;
-
-        uint256 scriptHash;
-        CSHA256().Write(&witnessScript[0], witnessScript.size())
-            .Finalize(scriptHash.begin());
-
-        scriptPubKey.clear();
-        scriptPubKey << OP_0 << ToByteVector(scriptHash);
+        CScript witnessScript = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
+        scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessScript));
 
         // Keystore has keys, but no witnessScript or P2SH redeemScript
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has keys and witnessScript, but no P2SH redeemScript
         keystore.AddCScript(witnessScript);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has keys, witnessScript, P2SH redeemScript
         keystore.AddCScript(scriptPubKey);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(isInvalid);
     }
 
     // P2WSH multisig wrapped in P2SH
     {
         CBasicKeyStore keystore;
 
-        CScript witnessScript;
-        witnessScript << OP_2 <<
-            ToByteVector(pubkeys[0]) <<
-            ToByteVector(pubkeys[1]) <<
-            OP_2 << OP_CHECKMULTISIG;
-
-        uint256 scriptHash;
-        CSHA256().Write(&witnessScript[0], witnessScript.size())
-            .Finalize(scriptHash.begin());
-
-        CScript redeemScript;
-        redeemScript << OP_0 << ToByteVector(scriptHash);
-
-        scriptPubKey.clear();
-        scriptPubKey << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL;
+        CScript witnessScript = GetScriptForMultisig(2, {pubkeys[0], pubkeys[1]});
+        CScript redeemScript = GetScriptForDestination(WitnessV0ScriptHash(witnessScript));
+        scriptPubKey = GetScriptForDestination(CScriptID(redeemScript));
 
         // Keystore has no witnessScript, P2SH redeemScript, or keys
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has witnessScript and P2SH redeemScript, but no keys
         keystore.AddCScript(redeemScript);
         keystore.AddCScript(witnessScript);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
 
         // Keystore has keys, witnessScript, P2SH redeemScript
         keystore.AddKey(keys[0]);
         keystore.AddKey(keys[1]);
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
-        BOOST_CHECK(!isInvalid);
     }
 
     // OP_RETURN
@@ -721,9 +702,8 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         scriptPubKey.clear();
         scriptPubKey << OP_RETURN << ToByteVector(pubkeys[0]);
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
     }
 
     // witness unspendable
@@ -734,9 +714,8 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         scriptPubKey.clear();
         scriptPubKey << OP_0 << ToByteVector(ParseHex("aabb"));
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
     }
 
     // witness unknown
@@ -747,9 +726,8 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         scriptPubKey.clear();
         scriptPubKey << OP_16 << ToByteVector(ParseHex("aabb"));
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
     }
 
     // Nonstandard
@@ -760,9 +738,8 @@ BOOST_AUTO_TEST_CASE(script_standard_IsMine)
         scriptPubKey.clear();
         scriptPubKey << OP_9 << OP_ADD << OP_11 << OP_EQUAL;
 
-        result = IsMine(keystore, scriptPubKey, isInvalid);
+        result = IsMine(keystore, scriptPubKey);
         BOOST_CHECK_EQUAL(result, ISMINE_NO);
-        BOOST_CHECK(!isInvalid);
     }
 }