Merge bitcoin#16226: Move ismine to the wallet module (dashpay#4640)

e61de63 Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e2 Move ismine to wallet module (Andrew Chow)

Pull request description:

  `IsMine` isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves `IsMine` into the wallet module and for it to take a `CWallet` instead of `CKeyStore`. The test that used `IsMine` is also moved to the wallet tests.

  This is first [prerequisites](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#ismine) for the wallet structure changes.

ACKs for commit e61de6:
  MarcoFalke:
    re-ACK e61de63 (only change is rebase with git auto-merge)
  meshcollider:
    Very light code review ACK bitcoin@e61de63

Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341

Co-authored-by: MeshCollider <dobsonsa68@gmail.com>

src/Makefile.am

@@ -273,7 +273,6 @@ BITCOIN_CORE_H = \
  saltedhasher.h \
  scheduler.h \
  script/descriptor.h \
- script/ismine.h \
  script/sigcache.h \
  script/sign.h \
  script/standard.h \
@@ -330,6 +329,7 @@ BITCOIN_CORE_H = \
  wallet/crypter.h \
  wallet/db.h \
  wallet/fees.h \
+ wallet/ismine.h \
  wallet/load.h \
  wallet/psbtwallet.h \
  wallet/rpcwallet.h \
@@ -493,6 +493,7 @@ libcosanta_wallet_a_SOURCES = \
  wallet/crypter.cpp \
  wallet/db.cpp \
  wallet/fees.cpp \
+ wallet/ismine.cpp \
  wallet/load.cpp \
  wallet/psbtwallet.cpp \
  wallet/rpcdump.cpp \
@@ -682,7 +683,6 @@ libcosanta_common_a_SOURCES = \
  saltedhasher.cpp \
  scheduler.cpp \
  script/descriptor.cpp \
- script/ismine.cpp \
  script/sign.cpp \
  script/standard.cpp \
  versionbitsinfo.cpp \

src/Makefile.test.include

@@ -216,7 +216,8 @@ BITCOIN_TESTS += \
  wallet/test/wallet_tests.cpp \
  wallet/test/wallet_crypto_tests.cpp \
  wallet/test/coinselector_tests.cpp \
- wallet/test/init_tests.cpp
+ wallet/test/init_tests.cpp \
+ wallet/test/ismine_tests.cpp
 
 BITCOIN_TEST_SUITE += \
  wallet/test/wallet_test_fixture.cpp \

src/interfaces/wallet.cpp

@@ -14,9 +14,6 @@
 #include <policy/fees.h>
 #include <policy/policy.h>
 #include <primitives/transaction.h>
-#include <rpc/server.h>
-#include <scheduler.h>
-#include <script/ismine.h>
 #include <script/standard.h>
 #include <support/allocators/secure.h>
 #include <sync.h>
@@ -25,6 +22,7 @@
 #include <util/system.h>
 #include <validation.h>
 #include <wallet/fees.h>
+#include <wallet/ismine.h>
 #include <wallet/rpcwallet.h>
 #include <wallet/load.h>
 #include <wallet/wallet.h>

src/interfaces/wallet.h

@@ -7,8 +7,7 @@
 
 #include <amount.h> // For CAmount
 #include <fs.h> // For fs::path
-#include <pubkey.h> // For CTxDestination (CKeyID and CScriptID)
-#include <script/ismine.h> // For isminefilter, isminetype
+#include <pubkey.h> // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation)
 #include <script/standard.h> // For CTxDestination
 #include <support/allocators/secure.h> // For SecureString
 #include <ui_interface.h> // For ChangeType
@@ -26,7 +25,9 @@ class CCoinControl;
 class CFeeRate;
 class CKey;
 class CWallet;
+enum isminetype : unsigned int;
 enum class FeeReason;
+typedef uint8_t isminefilter;
 struct CRecipient;
 
 namespace interfaces {

src/qt/transactiondesc.cpp

@@ -21,6 +21,7 @@
 #include <script/script.h>
 #include <timedata.h>
 #include <util/system.h>
+#include <wallet/ismine.h>
 
 #include <stdint.h>
 #include <string>

src/qt/transactionrecord.cpp

@@ -12,6 +12,8 @@
 #include <timedata.h>
 #include <validation.h>
 
+#include <wallet/ismine.h>
+
 #include <stdint.h>
 
 #include <QDateTime>

src/test/multisig_tests.cpp

@@ -9,7 +9,6 @@
 #include <script/script_error.h>
 #include <script/interpreter.h>
 #include <script/sign.h>
-#include <script/ismine.h>
 #include <tinyformat.h>
 #include <uint256.h>
 #include <test/util/setup_common.h>

src/test/script_p2sh_tests.cpp

@@ -12,7 +12,6 @@
 #include <script/script_error.h>
 #include <policy/settings.h>
 #include <script/sign.h>
-#include <script/ismine.h>
 #include <test/util/setup_common.h>
 
 #include <vector>
@@ -99,7 +98,6 @@ BOOST_AUTO_TEST_CASE(sign)
  txTo[i].vin[0].prevout.n = i;
  txTo[i].vin[0].prevout.hash = txFrom.GetHash();
  txTo[i].vout[0].nValue = 1;
- BOOST_CHECK_MESSAGE(IsMine(keystore, txFrom.vout[i].scriptPubKey), strprintf("IsMine %d", i));
  }
  for (int i = 0; i < 8; i++)
  {
@@ -196,7 +194,6 @@ BOOST_AUTO_TEST_CASE(set)
  txTo[i].vin[0].prevout.hash = txFrom.GetHash();
  txTo[i].vout[0].nValue = 1*CENT;
  txTo[i].vout[0].scriptPubKey = inner[i];
- BOOST_CHECK_MESSAGE(IsMine(keystore, txFrom.vout[i].scriptPubKey), strprintf("IsMine %d", i));
  }
  for (int i = 0; i < 4; i++)
  {

src/test/script_standard_tests.cpp

@@ -4,7 +4,6 @@
 
 #include <key.h>
 #include <keystore.h>
-#include <script/ismine.h>
 #include <script/script.h>
 #include <script/standard.h>
 #include <test/util/setup_common.h>
@@ -295,191 +294,4 @@ BOOST_AUTO_TEST_CASE(script_standard_GetScriptFor_)
  BOOST_CHECK(result == expected);
 }
 
-BOOST_AUTO_TEST_CASE(script_standard_IsMine)
-{
- CKey keys[2];
- CPubKey pubkeys[2];
- for (int i = 0; i < 2; i++) {
- keys[i].MakeNewKey(true);
- pubkeys[i] = keys[i].GetPubKey();
- }
-
- CKey uncompressedKey;
- uncompressedKey.MakeNewKey(false);
- CPubKey uncompressedPubkey = uncompressedKey.GetPubKey();
-
- CScript scriptPubKey;
- isminetype result;
-
- // P2PK compressed
- {
- CBasicKeyStore keystore;
- scriptPubKey = GetScriptForRawPubKey(pubkeys[0]);
-
- // Keystore does not have key
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has key
- BOOST_CHECK(keystore.AddKey(keys[0]));
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
- }
-
- // P2PK uncompressed
- {
- CBasicKeyStore keystore;
- scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey);
-
- // Keystore does not have key
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has key
- BOOST_CHECK(keystore.AddKey(uncompressedKey));
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
- }
-
- // P2PKH compressed
- {
- CBasicKeyStore keystore;
- scriptPubKey = GetScriptForDestination(pubkeys[0].GetID());
-
- // Keystore does not have key
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has key
- BOOST_CHECK(keystore.AddKey(keys[0]));
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
- }
-
- // P2PKH uncompressed
- {
- CBasicKeyStore keystore;
- scriptPubKey = GetScriptForDestination(uncompressedPubkey.GetID());
-
- // Keystore does not have key
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has key
- BOOST_CHECK(keystore.AddKey(uncompressedKey));
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
- }
-
- // P2SH
- {
- CBasicKeyStore keystore;
-
- CScript redeemScript = GetScriptForDestination(pubkeys[0].GetID());
- scriptPubKey = GetScriptForDestination(CScriptID(redeemScript));
-
- // Keystore does not have redeemScript or key
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has redeemScript but no key
- BOOST_CHECK(keystore.AddCScript(redeemScript));
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has redeemScript and key
- BOOST_CHECK(keystore.AddKey(keys[0]));
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
- }
-
- // (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));
-
- BOOST_CHECK(keystore.AddCScript(redeemscript));
- BOOST_CHECK(keystore.AddCScript(redeemscript_inner));
- BOOST_CHECK(keystore.AddCScript(scriptPubKey));
- BOOST_CHECK(keystore.AddKey(keys[0]));
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
- }
-
- // scriptPubKey multisig
- {
- CBasicKeyStore keystore;
-
- scriptPubKey = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
-
- // Keystore does not have any keys
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has 1/2 keys
- BOOST_CHECK(keystore.AddKey(uncompressedKey));
-
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has 2/2 keys
- BOOST_CHECK(keystore.AddKey(keys[1]));
-
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has 2/2 keys and the script
- BOOST_CHECK(keystore.AddCScript(scriptPubKey));
-
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
- }
-
- // P2SH multisig
- {
- CBasicKeyStore keystore;
- BOOST_CHECK(keystore.AddKey(uncompressedKey));
- BOOST_CHECK(keystore.AddKey(keys[1]));
-
- CScript redeemScript = GetScriptForMultisig(2, {uncompressedPubkey, pubkeys[1]});
- scriptPubKey = GetScriptForDestination(CScriptID(redeemScript));
-
- // Keystore has no redeemScript
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
-
- // Keystore has redeemScript
- BOOST_CHECK(keystore.AddCScript(redeemScript));
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE);
- }
-
- // OP_RETURN
- {
- CBasicKeyStore keystore;
- BOOST_CHECK(keystore.AddKey(keys[0]));
-
- scriptPubKey.clear();
- scriptPubKey << OP_RETURN << ToByteVector(pubkeys[0]);
-
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
- }
-
- // Nonstandard
- {
- CBasicKeyStore keystore;
- BOOST_CHECK(keystore.AddKey(keys[0]));
-
- scriptPubKey.clear();
- scriptPubKey << OP_9 << OP_ADD << OP_11 << OP_EQUAL;
-
- result = IsMine(keystore, scriptPubKey);
- BOOST_CHECK_EQUAL(result, ISMINE_NO);
- }
-}
-
 BOOST_AUTO_TEST_SUITE_END()

src/script/ismine.cpp → src/wallet/ismine.cpp

@@ -3,13 +3,11 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <script/ismine.h>
+#include <wallet/ismine.h>
 
 #include <key.h>
-#include <keystore.h>
 #include <script/script.h>
-#include <script/sign.h>
-
+#include <wallet/wallet.h>
 
 typedef std::vector<unsigned char> valtype;
 
@@ -45,7 +43,7 @@ bool PermitsUncompressed(IsMineSigVersion sigversion)
  return sigversion == IsMineSigVersion::TOP || sigversion == IsMineSigVersion::P2SH;
 }
 
-bool HaveKeys(const std::vector<valtype>& pubkeys, const CKeyStore& keystore)
+bool HaveKeys(const std::vector<valtype>& pubkeys, const CWallet& keystore)
 {
  for (const valtype& pubkey : pubkeys) {
  CKeyID keyID = CPubKey(pubkey).GetID();
@@ -54,7 +52,7 @@ bool HaveKeys(const std::vector<valtype>& pubkeys, const CKeyStore& keystore)
  return true;
 }
 
-IsMineResult IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey, IsMineSigVersion sigversion)
+IsMineResult IsMineInner(const CWallet& keystore, const CScript& scriptPubKey, IsMineSigVersion sigversion)
 {
  IsMineResult ret = IsMineResult::NO;
 
@@ -136,7 +134,7 @@ IsMineResult IsMineInner(const CKeyStore& keystore, const CScript& scriptPubKey,
 
 } // namespace
 
-isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey)
+isminetype IsMine(const CWallet& keystore, const CScript& scriptPubKey)
 {
  switch (IsMineInner(keystore, scriptPubKey, IsMineSigVersion::TOP)) {
  case IsMineResult::INVALID:
@@ -150,7 +148,7 @@ isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey)
  assert(false);
 }
 
-isminetype IsMine(const CKeyStore& keystore, const CTxDestination& dest)
+isminetype IsMine(const CWallet& keystore, const CTxDestination& dest)
 {
  CScript script = GetScriptForDestination(dest);
  return IsMine(keystore, script);