feat: allow manipulation of CoinJoin salt using RPC

doc/release-notes-6093.md

@@ -0,0 +1,4 @@
+New functionality
+-----------
+
+- A new RPC command, `coinjoinsalt`, allows for manipulating a CoinJoin salt stored in a wallet. `coinjoinsalt get` will fetch an existing salt, `coinjoinsalt set` will allow setting a custom salt and `coinjoinsalt generate` will set a random hash as the new salt.

src/rpc/coinjoin.cpp

@@ -181,6 +181,193 @@ static RPCHelpMan coinjoin_stop()
 },
     };
 }
+
+static RPCHelpMan coinjoinsalt()
+{
+    return RPCHelpMan{"coinjoinsalt",
+        "\nAvailable commands:\n"
+        "  generate  - Generate new CoinJoin salt\n"
+        "  get       - Fetch existing CoinJoin salt\n"
+        "  set       - Set new CoinJoin salt\n",
+        {
+            {"command", RPCArg::Type::STR, RPCArg::Optional::NO, "The command to execute"},
+        },
+        RPCResults{},
+        RPCExamples{""},
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    throw JSONRPCError(RPC_INVALID_PARAMETER, "Must be a valid command");
+},
+    };
+}
+
+static RPCHelpMan coinjoinsalt_generate()
+{
+    return RPCHelpMan{"coinjoinsalt generate",
+        "\nGenerate new CoinJoin salt and store it in the wallet database\n"
+        "Cannot generate new salt if CoinJoin mixing is in process or wallet has private keys disabled.\n",
+        {
+            {"overwrite", RPCArg::Type::BOOL, /* default */ "false", "Generate new salt even if there is an existing salt and/or there is CoinJoin balance"},
+        },
+        RPCResult{
+            RPCResult::Type::BOOL, "", "Status of CoinJoin salt generation and commitment"
+        },
+        RPCExamples{
+            HelpExampleCli("coinjoinsalt generate", "")
+          + HelpExampleRpc("coinjoinsalt generate", "")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    if (!wallet) return NullUniValue;
+
+    const auto str_wallet = wallet->GetName();
+    if (wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
+        throw JSONRPCError(RPC_INVALID_REQUEST,
+                           strprintf("Wallet \"%s\" has private keys disabled, cannot perform CoinJoin!", str_wallet));
+    }
+
+    bool enable_overwrite{false}; // Default value
+    if (!request.params[0].isNull()) {
+        enable_overwrite = ParseBoolV(request.params[0], "overwrite");
+    }
+
+    if (!enable_overwrite && !wallet->GetCoinJoinSalt().IsNull()) {
+        throw JSONRPCError(RPC_INVALID_REQUEST,
+                           strprintf("Wallet \"%s\" already has set CoinJoin salt!", str_wallet));
+    }
+
+    const NodeContext& node = EnsureAnyNodeContext(request.context);
+    if (node.coinjoin_loader != nullptr) {
+        auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName());
+        if (cj_clientman != nullptr && cj_clientman->IsMixing()) {
+            throw JSONRPCError(RPC_WALLET_ERROR,
+                               strprintf("Wallet \"%s\" is currently mixing, cannot change salt!", str_wallet));
+        }
+    }
+
+    const auto wallet_balance{wallet->GetBalance()};
+    const bool has_balance{(wallet_balance.m_anonymized
+                          + wallet_balance.m_denominated_trusted
+                          + wallet_balance.m_denominated_untrusted_pending) > 0};
+    if (!enable_overwrite && has_balance) {
+        throw JSONRPCError(RPC_WALLET_ERROR,
+                           strprintf("Wallet \"%s\" has CoinJoin balance, cannot continue!", str_wallet));
+    }
+
+    if (!wallet->SetCoinJoinSalt(GetRandHash())) {
+        throw JSONRPCError(RPC_INVALID_REQUEST,
+                           strprintf("Unable to set new CoinJoin salt for wallet \"%s\"!", str_wallet));
+    }
+
+    wallet->ClearCoinJoinRoundsCache();
+
+    return true;
+},
+    };
+}
+
+static RPCHelpMan coinjoinsalt_get()
+{
+    return RPCHelpMan{"coinjoinsalt get",
+        "\nFetch existing CoinJoin salt\n"
+        "Cannot fetch salt if wallet has private keys disabled.\n",
+        {},
+        RPCResult{
+            RPCResult::Type::STR_HEX, "", "CoinJoin salt"
+        },
+        RPCExamples{
+            HelpExampleCli("coinjoinsalt get", "")
+          + HelpExampleRpc("coinjoinsalt get", "")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    if (!wallet) return NullUniValue;
+
+    const auto str_wallet = wallet->GetName();
+    if (wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
+        throw JSONRPCError(RPC_INVALID_REQUEST,
+                           strprintf("Wallet \"%s\" has private keys disabled, cannot perform CoinJoin!", str_wallet));
+    }
+
+    const auto salt{wallet->GetCoinJoinSalt()};
+    if (salt.IsNull()) {
+        throw JSONRPCError(RPC_WALLET_ERROR,
+                           strprintf("Wallet \"%s\" has no CoinJoin salt!", str_wallet));
+    }
+    return salt.GetHex();
+},
+    };
+}
+
+static RPCHelpMan coinjoinsalt_set()
+{
+    return RPCHelpMan{"coinjoinsalt set",
+        "\nSet new CoinJoin salt\n"
+        "Cannot set salt if CoinJoin mixing is in process or wallet has private keys disabled.\n"
+        "Will overwrite existing salt. The presence of a CoinJoin balance will cause the wallet to rescan.\n",
+        {
+            {"salt", RPCArg::Type::STR, RPCArg::Optional::NO, "Desired CoinJoin salt value for the wallet"},
+            {"overwrite", RPCArg::Type::BOOL, /* default */ "false", "Overwrite salt even if CoinJoin balance present"},
+        },
+        RPCResult{
+            RPCResult::Type::BOOL, "", "Status of CoinJoin salt change request"
+        },
+        RPCExamples{
+            HelpExampleCli("coinjoinsalt set", "f4184fc596403b9d638783cf57adfe4c75c605f6356fbc91338530e9831e9e16")
+          + HelpExampleRpc("coinjoinsalt set", "f4184fc596403b9d638783cf57adfe4c75c605f6356fbc91338530e9831e9e16")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    if (!wallet) return NullUniValue;
+
+    const auto salt{ParseHashV(request.params[0], "salt")};
+    if (salt == uint256::ZERO) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid CoinJoin salt value");
+    }
+
+    bool enable_overwrite{false}; // Default value
+    if (!request.params[1].isNull()) {
+        enable_overwrite = ParseBoolV(request.params[1], "overwrite");
+    }
+
+    const auto str_wallet = wallet->GetName();
+    if (wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
+        throw JSONRPCError(RPC_INVALID_REQUEST,
+                           strprintf("Wallet \"%s\" has private keys disabled, cannot perform CoinJoin!", str_wallet));
+    }
+
+    const NodeContext& node = EnsureAnyNodeContext(request.context);
+    if (node.coinjoin_loader != nullptr) {
+        auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName());
+        if (cj_clientman != nullptr && cj_clientman->IsMixing()) {
+            throw JSONRPCError(RPC_WALLET_ERROR,
+                               strprintf("Wallet \"%s\" is currently mixing, cannot change salt!", str_wallet));
+        }
+    }
+
+    const auto wallet_balance{wallet->GetBalance()};
+    const bool has_balance{(wallet_balance.m_anonymized
+                          + wallet_balance.m_denominated_trusted
+                          + wallet_balance.m_denominated_untrusted_pending) > 0};
+    if (has_balance && !enable_overwrite) {
+        throw JSONRPCError(RPC_WALLET_ERROR,
+                           strprintf("Wallet \"%s\" has CoinJoin balance, cannot continue!", str_wallet));
+    }
+
+    if (!wallet->SetCoinJoinSalt(salt)) {
+        throw JSONRPCError(RPC_WALLET_ERROR,
+                           strprintf("Unable to set new CoinJoin salt for wallet \"%s\"!", str_wallet));
+    }
+
+    wallet->ClearCoinJoinRoundsCache();
+
+    return true;
+},
+    };
+}
 #endif // ENABLE_WALLET
 
 static RPCHelpMan getpoolinfo()
@@ -295,6 +482,10 @@ static const CRPCCommand commands[] =
     { "dash",                &coinjoin_reset,         },
     { "dash",                &coinjoin_start,         },
     { "dash",                &coinjoin_stop,          },
+    { "dash",                &coinjoinsalt,           },
+    { "dash",                &coinjoinsalt_generate,  },
+    { "dash",                &coinjoinsalt_get,       },
+    { "dash",                &coinjoinsalt_set,       },
 #endif // ENABLE_WALLET
 };
 // clang-format on

src/wallet/wallet.cpp

@@ -1448,6 +1448,15 @@ int CWallet::GetCappedOutpointCoinJoinRounds(const COutPoint& outpoint) const
     return realCoinJoinRounds > CCoinJoinClientOptions::GetRounds() ? CCoinJoinClientOptions::GetRounds() : realCoinJoinRounds;
 }
 
+void CWallet::ClearCoinJoinRoundsCache()
+{
+    LOCK(cs_wallet);
+    mapOutpointRoundsCache.clear();
+    MarkDirty();
+    // Notify UI
+    NotifyTransactionChanged(uint256::ONE, CT_UPDATED);
+}
+
 bool CWallet::IsDenominated(const COutPoint& outpoint) const
 {
     LOCK(cs_wallet);
@@ -2757,21 +2766,34 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out
     return ptx->vout[n];
 }
 
-void CWallet::InitCoinJoinSalt()
+const uint256& CWallet::GetCoinJoinSalt()
+{
+    if (nCoinJoinSalt.IsNull()) {
+        InitCJSaltFromDb();
+    }
+    return nCoinJoinSalt;
+}
+
+void CWallet::InitCJSaltFromDb()
 {
-    // Avoid fetching it multiple times
     assert(nCoinJoinSalt.IsNull());
 
     WalletBatch batch(GetDatabase());
     if (!batch.ReadCoinJoinSalt(nCoinJoinSalt) && batch.ReadCoinJoinSalt(nCoinJoinSalt, true)) {
+        // Migrate salt stored with legacy key
         batch.WriteCoinJoinSalt(nCoinJoinSalt);
     }
+}
 
-    while (nCoinJoinSalt.IsNull()) {
-        // We never generated/saved it
-        nCoinJoinSalt = GetRandHash();
-        batch.WriteCoinJoinSalt(nCoinJoinSalt);
+bool CWallet::SetCoinJoinSalt(const uint256& cj_salt)
+{
+    WalletBatch batch(GetDatabase());
+    // Only store new salt in CWallet if database write is successful
+    if (batch.WriteCoinJoinSalt(cj_salt)) {
+        nCoinJoinSalt = cj_salt;
+        return true;
     }
+    return false;
 }
 
 struct CompareByPriority
@@ -3942,11 +3964,14 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
         }
     }
 
-    InitCoinJoinSalt();
-
     if (nLoadWalletRet != DBErrors::LOAD_OK)
         return nLoadWalletRet;
 
+    /* If the CoinJoin salt is not set, try to set a new random hash as the salt */
+    if (GetCoinJoinSalt().IsNull() && !SetCoinJoinSalt(GetRandHash())) {
+        return DBErrors::LOAD_FAIL;
+    }
+
     return DBErrors::LOAD_OK;
 }
 

src/wallet/wallet.h

@@ -747,7 +747,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     void AddToSpends(const uint256& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     std::set<COutPoint> setWalletUTXO;
-    mutable std::map<COutPoint, int> mapOutpointRoundsCache;
+    mutable std::map<COutPoint, int> mapOutpointRoundsCache GUARDED_BY(cs_wallet);
 
     /**
      * Add a transaction to the wallet, or update it.  pIndex and posInBlock should
@@ -817,16 +817,16 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
      */
     uint256 m_last_block_processed GUARDED_BY(cs_wallet);
 
-    /** Pulled from wallet DB ("ps_salt") and used when mixing a random number of rounds.
+    /** Pulled from wallet DB ("cj_salt") and used when mixing a random number of rounds.
      *  This salt is needed to prevent an attacker from learning how many extra times
      *  the input was mixed based only on information in the blockchain.
      */
     uint256 nCoinJoinSalt;
 
     /**
-     * Fetches CoinJoin salt from database or generates and saves a new one if no salt was found in the db
+     * Populates nCoinJoinSalt with value from database (and migrates salt stored with legacy key).
      */
-    void InitCoinJoinSalt();
+    void InitCJSaltFromDb();
 
     /** Height of last block processed is used by wallet to know depth of transactions
      * without relying on Chain interface beyond asynchronous updates. For safety, we
@@ -872,6 +872,19 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
      */
     const std::string& GetName() const { return m_name; }
 
+    /**
+     * Get an existing CoinJoin salt. Will attempt to read database (and migrate legacy salts) if
+     * nCoinJoinSalt is empty but will skip database read if nCoinJoinSalt is populated.
+     **/
+    const uint256& GetCoinJoinSalt();
+
+    /**
+     * Write a new CoinJoin salt. This will directly write the new salt value into the wallet database.
+     * Ensuring that undesirable behaviour like overwriting the salt of a wallet that already uses CoinJoin
+     * is the responsibility of the caller.
+     **/
+    bool SetCoinJoinSalt(const uint256& cj_salt);
+
     // Map from governance object hash to governance object, they are added by gobject_prepare.
     std::map<uint256, Governance::Object> m_gobjects;
 
@@ -982,6 +995,8 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
     int GetRealOutpointCoinJoinRounds(const COutPoint& outpoint, int nRounds = 0) const;
     // respect current settings
     int GetCappedOutpointCoinJoinRounds(const COutPoint& outpoint) const;
+    // drop the internal cache to let Get...Rounds recalculate CJ balance from scratch and notify UI
+    void ClearCoinJoinRoundsCache();
 
     bool IsDenominated(const COutPoint& outpoint) const;
     bool IsFullyMixed(const COutPoint& outpoint) const;