Merge bitcoin#16226: Move ismine to the wallet module

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
dashpay · Dec 25, 2021 · 2a5427b · 2a5427b
1 parent f329625
commit 2a5427b
Show file tree

Hide file tree

Showing 14 changed files with 423 additions and 212 deletions.
diff --git a/src/Makefile.am b/src/Makefile.am
@@ -259,7 +259,6 @@ BITCOIN_CORE_H = \
  saltedhasher.h \
  scheduler.h \
  script/descriptor.h \
- script/ismine.h \
  script/sigcache.h \
  script/sign.h \
  script/standard.h \
@@ -312,6 +311,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 \
@@ -466,6 +466,7 @@ libdash_wallet_a_SOURCES = \
  wallet/crypter.cpp \
  wallet/db.cpp \
  wallet/fees.cpp \
+ wallet/ismine.cpp \
  wallet/load.cpp \
  wallet/psbtwallet.cpp \
  wallet/rpcdump.cpp \
@@ -633,7 +634,6 @@ libdash_common_a_SOURCES = \
  saltedhasher.cpp \
  scheduler.cpp \
  script/descriptor.cpp \
- script/ismine.cpp \
  script/sign.cpp \
  script/standard.cpp \
  versionbitsinfo.cpp \

diff --git a/src/Makefile.test.include b/src/Makefile.test.include
@@ -208,7 +208,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 \

diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
@@ -14,7 +14,6 @@
 #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>
@@ -23,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>

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

diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp
@@ -20,6 +20,7 @@
 #include <validation.h>
 #include <script/script.h>
 #include <util/system.h>
+#include <wallet/ismine.h>
 
 #include <stdint.h>
 #include <string>

diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp
@@ -11,6 +11,7 @@
 #include <interfaces/node.h>
 #include <timedata.h>
 #include <validation.h>
+#include <wallet/ismine.h>
 
 #include <stdint.h>
 

diff --git a/src/test/multisig_tests.cpp b/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/setup_common.h>

diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp
@@ -11,7 +11,6 @@
 #include <script/script.h>
 #include <script/script_error.h>
 #include <script/sign.h>
-#include <script/ismine.h>
 #include <test/setup_common.h>
 
 #include <vector>
@@ -98,7 +97,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++)
  {
@@ -195,7 +193,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++)
  {

diff --git a/src/test/script_standard_tests.cpp b/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/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()
diff --git a/src/script/ismine.cpp → src/wallet/ismine.cpp b/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 <wallet/wallet.h>
 typedef std::vector<unsigned char> valtype;
 
 namespace {
@@ -44,7 +42,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();
@@ -53,7 +51,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;
 
@@ -135,7 +133,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:
@@ -149,7 +147,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);