feat: descriptor wallets by default create spk for mobile derivation path for CJ

Author: knst

src/wallet/rpc/backup.cpp

@@ -1783,7 +1783,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c
             if (!w_desc.descriptor->GetOutputType()) {
                 warnings.push_back("Unknown output type, cannot set descriptor to active.");
             } else {
-                wallet.AddActiveScriptPubKeyMan(spk_manager->GetID(), internal);
+                wallet.AddActiveScriptPubKeyMan(spk_manager->GetID(), internal ? InternalKey::Internal : InternalKey::External);
             }
         } else {
             if (w_desc.descriptor->GetOutputType()) {
@@ -1976,6 +1976,7 @@ RPCHelpMan listdescriptors()
                     {RPCResult::Type::NUM, "timestamp", "The creation time of the descriptor"},
                     {RPCResult::Type::BOOL, "active", "Whether this descriptor is currently used to generate new addresses"},
                     {RPCResult::Type::BOOL, "internal", /*optional=*/true, "True if this descriptor is used to generate change addresses. False if this descriptor is used to generate receiving addresses; defined only for active descriptors"},
+                    {RPCResult::Type::BOOL, "coinjoin", /*optional=*/true, "True if this descriptor is used to generate CoinJoin addresses. False if this descriptor is used to generate receiving addresses; defined only for active descriptors"},
                     {RPCResult::Type::ARR_FIXED, "range", /*optional=*/true, "Defined only for ranged descriptors", {
                         {RPCResult::Type::NUM, "", "Range start inclusive"},
                         {RPCResult::Type::NUM, "", "Range end inclusive"},
@@ -2034,7 +2035,10 @@ RPCHelpMan listdescriptors()
         spk.pushKV("active", active);
         const auto& type = wallet_descriptor.descriptor->GetOutputType();
         if (active && type != std::nullopt) {
-            spk.pushKV("internal", wallet->GetScriptPubKeyMan(true) == desc_spk_man);
+            spk.pushKV("internal", wallet->GetScriptPubKeyMan(InternalKey::Internal) == desc_spk_man);
+        }
+        if (active && type != std::nullopt) {
+            spk.pushKV("coinjoin", wallet->GetScriptPubKeyMan(InternalKey::CoinJoin) == desc_spk_man);
         }
         if (wallet_descriptor.descriptor->IsRange()) {
             UniValue range(UniValue::VARR);

src/wallet/rpc/wallet.cpp

@@ -168,7 +168,7 @@ static RPCHelpMan getwalletinfo()
                             {RPCResult::Type::NUM_TIME, "timefirstkey", "the " + UNIX_EPOCH_TIME + " of the oldest known key in the wallet"},
                             {RPCResult::Type::NUM_TIME, "keypoololdest", /* optional */ true, "the " + UNIX_EPOCH_TIME + " of the oldest pre-generated key in the key pool. Legacy wallets only"},
                             {RPCResult::Type::NUM, "keypoolsize", "how many new keys are pre-generated (only counts external keys)"},
-                            {RPCResult::Type::NUM, "keypoolsize_hd_internal", /* optional */ true, "how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)"},
+                            {RPCResult::Type::NUM, "keypoolsize_hd_internal", /* optional */ true, "how many new keys are pre-generated for internal use (used for change outputs and mobile coinjoin, only appears if the wallet is using this feature, otherwise external keys are used)"},
                             {RPCResult::Type::NUM, "keys_left", "how many new keys are left since last automatic backup"},
                             {RPCResult::Type::NUM_TIME, "unlocked_until", /* optional */ true, "the " + UNIX_EPOCH_TIME + " until which the wallet is unlocked for transfers, or 0 if the wallet is locked (only present for passphrase-encrypted wallets)"},
                             {RPCResult::Type::STR_AMOUNT, "paytxfee", "the transaction fee configuration, set in " + CURRENCY_UNIT + "/kB"},

src/wallet/scriptpubkeyman.cpp

@@ -2074,7 +2074,7 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const
     }
 }
 
-bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, const SecureString& secure_mnemonic, const SecureString& secure_mnemonic_passphrase, bool internal)
+bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, const SecureString& secure_mnemonic, const SecureString& secure_mnemonic_passphrase, InternalKey internal)
 {
     LOCK(cs_desc_man);
     assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
@@ -2099,10 +2099,10 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_
     std::string xpub = EncodeExtPubKey(master_key.Neuter());
 
     // Build descriptor string
-    std::string desc_prefix = strprintf("pkh(%s/44'/%d'", xpub, Params().ExtCoinType());
+    std::string desc_prefix = strprintf("pkh(%s/%d'/%d'", xpub, internal == InternalKey::CoinJoin ? 9 : 44, Params().ExtCoinType());
     std::string desc_suffix = "/*)";
 
-    std::string internal_path = internal ? "/1" : "/0";
+    std::string internal_path = (internal == InternalKey::Internal) ? "/1" : "/0";
     std::string desc_str = desc_prefix + "/0'" + internal_path + desc_suffix;
 
     // Make the descriptor

src/wallet/scriptpubkeyman.h

@@ -147,6 +147,13 @@ class CKeyPool
     }
 };
 
+enum class InternalKey
+{
+    External,
+    Internal,
+    CoinJoin,
+};
+
 /*
  * A class implementing ScriptPubKeyMan manages some (or all) scriptPubKeys used in a wallet.
  * It contains the scripts and keys related to the scriptPubKeys it manages.
@@ -575,7 +582,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan
     bool IsHDEnabled() const override;
 
     //! Setup descriptors based on the given CExtkey
-    bool SetupDescriptorGeneration(const CExtKey& master_key, const SecureString& secure_mnemonic, const SecureString& secure_mnemonic_passphrase, bool internal);
+    bool SetupDescriptorGeneration(const CExtKey& master_key, const SecureString& secure_mnemonic, const SecureString& secure_mnemonic_passphrase, InternalKey internal);
 
     bool HavePrivateKeys() const override;
 

src/wallet/wallet.cpp

@@ -1477,7 +1477,7 @@ bool CWallet::CanGetAddresses(bool internal) const
 {
     LOCK(cs_wallet);
     if (m_spk_managers.empty()) return false;
-    auto spk_man = GetScriptPubKeyMan(internal);
+    auto spk_man = GetScriptPubKeyMan(internal ? InternalKey::Internal : InternalKey::External);
     if (spk_man && spk_man->CanGetAddresses(internal)) {
         return true;
     }
@@ -2354,7 +2354,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
 util::Result<CTxDestination> CWallet::GetNewDestination(const std::string label)
 {
     LOCK(cs_wallet);
-    auto spk_man = GetScriptPubKeyMan(false /* internal */);
+    auto spk_man = GetScriptPubKeyMan(InternalKey::External);
     if (!spk_man) {
         return util::Error{_("Error: No addresses available.")};
     }
@@ -2447,7 +2447,7 @@ std::set<std::string> CWallet::ListAddrBookLabels(const std::string& purpose) co
 
 util::Result<CTxDestination> ReserveDestination::GetReservedDestination(bool fInternalIn)
 {
-    m_spk_man = pwallet->GetScriptPubKeyMan(fInternalIn);
+    m_spk_man = pwallet->GetScriptPubKeyMan(fInternalIn ? InternalKey::Internal : InternalKey::External);
     if (!m_spk_man) {
         return util::Error{_("Error: No addresses available.")};
     }
@@ -3638,7 +3638,7 @@ bool CWallet::Unlock(const CKeyingMaterial& vMasterKeyIn, bool fForMixingOnly, b
 std::set<ScriptPubKeyMan*> CWallet::GetActiveScriptPubKeyMans() const
 {
     std::set<ScriptPubKeyMan*> spk_mans;
-    for (bool internal : {false, true}) {
+    for (auto internal : {InternalKey::Internal, InternalKey::External, InternalKey::CoinJoin}) {
         auto spk_man = GetScriptPubKeyMan(internal);
         if (spk_man) {
             spk_mans.insert(spk_man);
@@ -3656,13 +3656,18 @@ std::set<ScriptPubKeyMan*> CWallet::GetAllScriptPubKeyMans() const
     return spk_mans;
 }
 
-ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(bool internal) const
+ScriptPubKeyMan* CWallet::GetScriptPubKeyMan(InternalKey internal) const
 {
-    const auto spk_manager = internal ? m_internal_spk_managers : m_external_spk_managers;
-    if (spk_manager == nullptr) {
-        return nullptr;
-    }
-    return spk_manager;
+    switch (internal)
+    {
+    case InternalKey::Internal:
+        return m_internal_spk_managers;
+    case InternalKey::External:
+        return m_external_spk_managers;
+    case InternalKey::CoinJoin:
+        return m_coinjoin_spk_managers;
+    } // no default to let compiler warn us
+    return nullptr;
 }
 
 std::set<ScriptPubKeyMan*> CWallet::GetScriptPubKeyMans(const CScript& script, SignatureData& sigdata) const
@@ -3787,7 +3792,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const SecureString& mnemonic_arg,
     CExtKey master_key;
     master_key.SetSeed(MakeByteSpan(seed_key));
 
-    for (bool internal : {false, true}) {
+    for (auto internal : {InternalKey::External, InternalKey::Internal, InternalKey::CoinJoin}) {
         { // OUTPUT_TYPE is only one: LEGACY
             auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this));
             if (IsCrypted()) {
@@ -3806,7 +3811,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const SecureString& mnemonic_arg,
     }
 }
 
-void CWallet::AddActiveScriptPubKeyMan(uint256 id, bool internal)
+void CWallet::AddActiveScriptPubKeyMan(uint256 id, InternalKey internal)
 {
     WalletBatch batch(GetDatabase());
     if (!batch.WriteActiveScriptPubKeyMan(id, internal)) {
@@ -3815,29 +3820,48 @@ void CWallet::AddActiveScriptPubKeyMan(uint256 id, bool internal)
     LoadActiveScriptPubKeyMan(id, internal);
 }
 
-void CWallet::LoadActiveScriptPubKeyMan(uint256 id, bool internal)
+void CWallet::LoadActiveScriptPubKeyMan(uint256 id, InternalKey internal)
 {
     // Activating ScriptPubKeyManager for a given output and change type is incompatible with legacy wallets.
     // Legacy wallets have only one ScriptPubKeyManager and it's active for all output and change types.
     Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
 
-    WalletLogPrintf("Setting spkMan to active: id = %s, type = %s, internal = %s\n", id.ToString(), FormatOutputType(OutputType::LEGACY), internal ? "true" : "false");
-    auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers;
-    auto& spk_mans_other = internal ? m_external_spk_managers : m_internal_spk_managers;
+    WalletLogPrintf("Setting spkMan to active: id = %s, type = %s, internal = %s\n", id.ToString(), FormatOutputType(OutputType::LEGACY), internal == InternalKey::Internal ? "true" : "false");
+
     auto spk_man = m_spk_managers.at(id).get();
-    spk_mans = spk_man;
+    switch (internal) {
+    case InternalKey::Internal:
+        m_internal_spk_managers = spk_man;
+        break;
 
-    if (spk_mans_other == spk_man) {
-        spk_mans_other = nullptr;
+    case InternalKey::External:
+        m_external_spk_managers = spk_man;
+        break;
+    case InternalKey::CoinJoin:
+        m_coinjoin_spk_managers = spk_man;
+        break;
+    }
+
+    // no default case to let compiler hint it
+    if (internal != InternalKey::Internal && m_internal_spk_managers == spk_man) {
+        m_internal_spk_managers = nullptr;
+    }
+
+    if (internal != InternalKey::External && m_external_spk_managers == spk_man) {
+        m_external_spk_managers = nullptr;
+    }
+    if (internal != InternalKey::CoinJoin && m_coinjoin_spk_managers == spk_man) {
+        m_coinjoin_spk_managers = nullptr;
     }
 
     NotifyCanGetAddressesChanged();
 
 }
 
+// TODO: probably need to support InternalKey here
 void CWallet::DeactivateScriptPubKeyMan(uint256 id, bool internal)
 {
-    auto spk_man = GetScriptPubKeyMan(internal);
+    auto spk_man = GetScriptPubKeyMan(internal ? InternalKey::Internal : InternalKey::External);
     if (spk_man != nullptr && spk_man->GetID() == id) {
         WalletLogPrintf("Deactivate spkMan: id = %s, type = %s, internal = %s\n", id.ToString(), FormatOutputType(OutputType::LEGACY), internal ? "true" : "false");
         WalletBatch batch(GetDatabase());

src/wallet/wallet.h

@@ -394,6 +394,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
 
     ScriptPubKeyMan* m_external_spk_managers{nullptr};
     ScriptPubKeyMan* m_internal_spk_managers{nullptr};
+    ScriptPubKeyMan* m_coinjoin_spk_managers{nullptr};
 
     // Indexed by a unique identifier produced by each ScriptPubKeyMan using
     // ScriptPubKeyMan::GetID. In many cases it will be the hash of an internal structure
@@ -984,7 +985,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     std::set<ScriptPubKeyMan*> GetAllScriptPubKeyMans() const;
 
     //! Get the ScriptPubKeyMan for internal/external chain.
-    ScriptPubKeyMan* GetScriptPubKeyMan(bool internal) const;
+    ScriptPubKeyMan* GetScriptPubKeyMan(InternalKey internal) const;
 
     //! Get the ScriptPubKeyMan for a script
     ScriptPubKeyMan* GetScriptPubKeyMan(const CScript& script) const;
@@ -1039,12 +1040,12 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     //! Adds the active ScriptPubKeyMan for the specified type and internal. Writes it to the wallet file
     //! @param[in] id The unique id for the ScriptPubKeyMan
     //! @param[in] internal Whether this ScriptPubKeyMan provides change addresses
-    void AddActiveScriptPubKeyMan(uint256 id, bool internal);
+    void AddActiveScriptPubKeyMan(uint256 id, InternalKey internal);
 
     //! Loads an active ScriptPubKeyMan for the specified type and internal. (used by LoadWallet)
     //! @param[in] id The unique id for the ScriptPubKeyMan
     //! @param[in] internal Whether this ScriptPubKeyMan provides change addresses
-    void LoadActiveScriptPubKeyMan(uint256 id, bool internal);
+    void LoadActiveScriptPubKeyMan(uint256 id, InternalKey internal);
 
     //! Remove specified ScriptPubKeyMan from set of active SPK managers. Writes the change to the wallet file.
     //! @param[in] id The unique id for the ScriptPubKeyMan

src/wallet/walletdb.cpp

@@ -34,6 +34,7 @@ namespace DBKeys {
 const std::string ACENTRY{"acentry"};
 const std::string ACTIVEEXTERNALSPK{"activeexternalspk"};
 const std::string ACTIVEINTERNALSPK{"activeinternalspk"};
+const std::string ACTIVECOINJOINSPK{"activecoinjoinspk"};
 const std::string BESTBLOCK_NOMERKLE{"bestblock_nomerkle"};
 const std::string BESTBLOCK{"bestblock"};
 const std::string CRYPTED_KEY{"ckey"};
@@ -230,9 +231,23 @@ bool WalletBatch::WriteGovernanceObject(const Governance::Object& obj)
     return WriteIC(std::make_pair(DBKeys::G_OBJECT, obj.GetHash()), obj, false);
 }
 
-bool WalletBatch::WriteActiveScriptPubKeyMan(const uint256& id, bool internal)
+bool WalletBatch::WriteActiveScriptPubKeyMan(const uint256& id, InternalKey internal)
 {
-    std::string key = internal ? DBKeys::ACTIVEINTERNALSPK : DBKeys::ACTIVEEXTERNALSPK;
+    std::string key;
+    switch (internal) {
+    case InternalKey::Internal:
+        key = DBKeys::ACTIVEINTERNALSPK;
+        break;
+    case InternalKey::External:
+        key = DBKeys::ACTIVEEXTERNALSPK;
+        break;
+    case InternalKey::CoinJoin:
+        key = DBKeys::ACTIVECOINJOINSPK;
+        break;
+    }
+    // no default to get a hint from a compiler
+    assert(!key.empty());
+
     return WriteIC(key, id);
 }
 
@@ -333,6 +348,7 @@ class CWalletScanState {
     std::vector<uint256> vWalletUpgrade;
     std::map<OutputType, uint256> m_active_external_spks;
     std::map<OutputType, uint256> m_active_internal_spks;
+    std::map<OutputType, uint256> m_active_coinjoin_spks;
     std::map<uint256, DescriptorCache> m_descriptor_caches;
     std::map<std::pair<uint256, CKeyID>, CKey> m_descriptor_keys;
     std::map<std::pair<uint256, CKeyID>, std::pair<CPubKey, std::vector<unsigned char>>> m_descriptor_crypt_keys;
@@ -614,12 +630,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
         } else if (strType == DBKeys::OLD_KEY) {
             strErr = "Found unsupported 'wkey' record, try loading with version 0.17";
             return false;
-        } else if (strType == DBKeys::ACTIVEEXTERNALSPK || strType == DBKeys::ACTIVEINTERNALSPK) {
+        } else if (strType == DBKeys::ACTIVEEXTERNALSPK || strType == DBKeys::ACTIVEINTERNALSPK || strType == DBKeys::ACTIVEEXTERNALSPK) {
             uint256 id;
             ssValue >> id;
 
             bool internal = strType == DBKeys::ACTIVEINTERNALSPK;
-            auto& spk_mans = internal ? wss.m_active_internal_spks : wss.m_active_external_spks;
+            bool coinjoin = strType == DBKeys::ACTIVECOINJOINSPK;
+            auto& spk_mans = internal ? wss.m_active_internal_spks : (coinjoin ? wss.m_active_coinjoin_spks : wss.m_active_external_spks);
             const OutputType type = OutputType::LEGACY;
             if (spk_mans.count(static_cast<OutputType>(type)) > 0) {
                 strErr = "Multiple ScriptPubKeyMans specified for a single type";
@@ -878,10 +895,13 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
 
     // Set the active ScriptPubKeyMans
     for (auto spk_man : wss.m_active_external_spks) {
-        pwallet->LoadActiveScriptPubKeyMan(spk_man.second, /* internal */ false);
+        pwallet->LoadActiveScriptPubKeyMan(spk_man.second, InternalKey::External);
     }
     for (auto spk_man : wss.m_active_internal_spks) {
-        pwallet->LoadActiveScriptPubKeyMan(spk_man.second, /* internal */ true);
+        pwallet->LoadActiveScriptPubKeyMan(spk_man.second, InternalKey::Internal);
+    }
+    for (auto spk_man : wss.m_active_coinjoin_spks) {
+        pwallet->LoadActiveScriptPubKeyMan(spk_man.second, InternalKey::CoinJoin);
     }
 
     // Set the descriptor caches

src/wallet/walletdb.h

@@ -31,7 +31,7 @@ class CMasterKey;
 class CWallet;
 class CWalletTx;
 struct WalletContext;
-
+enum class InternalKey;
 /**
  * Overview of wallet database classes:
  *
@@ -61,6 +61,7 @@ namespace DBKeys {
 extern const std::string ACENTRY;
 extern const std::string ACTIVEEXTERNALSPK;
 extern const std::string ACTIVEINTERNALSPK;
+extern const std::string ACTIVECOINJOINSPK;
 extern const std::string BESTBLOCK;
 extern const std::string BESTBLOCK_NOMERKLE;
 extern const std::string CRYPTED_HDCHAIN;
@@ -229,7 +230,7 @@ class WalletBatch
     /// Erase destination data tuple from wallet database
     bool EraseDestData(const std::string &address, const std::string &key);
 
-    bool WriteActiveScriptPubKeyMan(const uint256& id, bool internal);
+    bool WriteActiveScriptPubKeyMan(const uint256& id, InternalKey internal);
     bool EraseActiveScriptPubKeyMan(bool internal);
 
     DBErrors LoadWallet(CWallet* pwallet);

test/functional/tool_wallet.py

@@ -266,7 +266,8 @@ def test_tool_wallet_create_on_existing_wallet(self):
         shasum_before = self.wallet_shasum()
         timestamp_before = self.wallet_timestamp()
         self.log.debug('Wallet file timestamp before calling create: {}'.format(timestamp_before))
-        out = "Topping up keypool...\n" + self.get_expected_info_output(name="foo", keypool=2000)
+        keypool_size = 3000 if self.options.descriptors else 2000
+        out = "Topping up keypool...\n" + self.get_expected_info_output(name="foo", keypool=keypool_size)
         self.assert_tool_output(out, '-wallet=foo', 'create')
         shasum_after = self.wallet_shasum()
         timestamp_after = self.wallet_timestamp()

test/functional/wallet_createwallet.py

@@ -152,9 +152,10 @@ def run_test(self):
         # There should only be 1 key for legacy, 1 for descriptors (dash has only one type of addresses)
         walletinfo = w6.getwalletinfo()
         keys = 1 if self.options.descriptors else 1
+        cj_keys = 1 if self.options.descriptors else 0
         assert_equal(walletinfo['keypoolsize'], keys)
         # hd_internals are not refilled by default for descriptor wallets atm
-        assert_equal(walletinfo['keypoolsize_hd_internal'], keys)
+        assert_equal(walletinfo['keypoolsize_hd_internal'], keys + cj_keys)
         # Allow empty passphrase, but there should be a warning
         resp = self.nodes[0].createwallet(wallet_name='w7', disable_private_keys=False, blank=False, passphrase='')
         assert 'Empty string given as passphrase, wallet will not be encrypted.' in resp['warning']