Merge #6107: refactor: make coinjoin a composite command

89ade3e340 rpc: disallow `coinjoin stop` if there's no CoinJoin session to stop (Kittywhiskers Van Gogh)
51b6b94fc0 rpc: make `coinjoin` a composite command (Kittywhiskers Van Gogh)

Pull request description:

  ## Breaking Changes

  - `coinjoin stop` will now return an error if there is no CoinJoin mixing session to stop

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    utACK 89ade3e340
  UdjinM6:
    utACK 89ade3e340
  PastaPastaPasta:
    utACK 89ade3e340

Tree-SHA512: cde350217dd0ad15e5908eaae63773cad79ec6043e4ff902e2a32b57248ae3f3261e3180c7e39d5385eef361b2c14098b308feb648171c689fef5a3e8467381a

doc/release-notes-6107.md

@@ -0,0 +1,4 @@
+RPC changes
+-----------
+
+- `coinjoin stop` will now return an error if there is no CoinJoin mixing session to stop

src/rpc/coinjoin.cpp

@@ -22,6 +22,23 @@
 #include <univalue.h>
 
 #ifdef ENABLE_WALLET
+namespace {
+void ValidateCoinJoinArguments()
+{
+    /* If CoinJoin is enabled, everything is working as expected, we can bail */
+    if (CCoinJoinClientOptions::IsEnabled())
+        return;
+
+    /* CoinJoin is on by default, unless a command line argument says otherwise */
+    if (!gArgs.GetBoolArg("-enablecoinjoin", true)) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing is disabled via -enablecoinjoin=0 command line option, remove it to enable mixing again");
+    }
+
+    /* Most likely something bad happened and we disabled it while running the wallet */
+    throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing is disabled due to an internal error");
+}
+} // anonymous namespace
+
 static RPCHelpMan coinjoin()
 {
     return RPCHelpMan{"coinjoin",
@@ -35,6 +52,25 @@ static RPCHelpMan coinjoin()
         RPCResults{},
         RPCExamples{""},
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    throw JSONRPCError(RPC_INVALID_PARAMETER, "Must be a valid command");
+},
+    };
+}
+
+static RPCHelpMan coinjoin_reset()
+{
+    return RPCHelpMan{"coinjoin reset",
+        "\nReset CoinJoin mixing\n",
+        {},
+        RPCResult{
+            RPCResult::Type::STR, "", "Status of request"
+        },
+        RPCExamples{
+            HelpExampleCli("coinjoin reset", "")
+          + HelpExampleRpc("coinjoin reset", "")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
     if (!wallet) return NullUniValue;
@@ -45,27 +81,55 @@ static RPCHelpMan coinjoin()
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Client-side mixing is not supported on masternodes");
     }
 
-    if (!CCoinJoinClientOptions::IsEnabled()) {
+    ValidateCoinJoinArguments();
-        if (!gArgs.GetBoolArg("-enablecoinjoin", true)) {
-            // otherwise it's on by default, unless cmd line option says otherwise
-            throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing is disabled via -enablecoinjoin=0 command line option, remove it to enable mixing again");
-        } else {
-            // not enablecoinjoin=false case,
-            // most likely something bad happened and we disabled it while running the wallet
-            throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing is disabled due to some internal error");
-        }
-    }
 
+    CHECK_NONFATAL(node.coinjoin_loader);
     auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName());
-    CHECK_NONFATAL(cj_clientman != nullptr);
 
-    if (request.params[0].get_str() == "start") {
+    CHECK_NONFATAL(cj_clientman);
+    cj_clientman->ResetPool();
+
+    return "Mixing was reset";
+},
+    };
+}
+
+static RPCHelpMan coinjoin_start()
+{
+    return RPCHelpMan{"coinjoin start",
+        "\nStart CoinJoin mixing\n"
+        "Wallet must be unlocked for mixing\n",
+        {},
+        RPCResult{
+            RPCResult::Type::STR, "", "Status of request"
+        },
+        RPCExamples{
+            HelpExampleCli("coinjoin start", "")
+          + HelpExampleRpc("coinjoin start", "")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    if (!wallet) return NullUniValue;
+
+    const NodeContext& node = EnsureAnyNodeContext(request.context);
+
+    if (node.mn_activeman) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Client-side mixing is not supported on masternodes");
+    }
+
+    ValidateCoinJoinArguments();
+
     {
         LOCK(wallet->cs_wallet);
         if (wallet->IsLocked(true))
             throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Please unlock wallet for mixing with walletpassphrase first.");
     }
 
+    CHECK_NONFATAL(node.coinjoin_loader);
+    auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName());
+
+    CHECK_NONFATAL(cj_clientman);
     if (!cj_clientman->StartMixing()) {
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing has been started already.");
     }
@@ -75,19 +139,45 @@ static RPCHelpMan coinjoin()
     CConnman& connman = EnsureConnman(node);
     bool result = cj_clientman->DoAutomaticDenominating(chainman.ActiveChainstate(), connman, mempool);
     return "Mixing " + (result ? "started successfully" : ("start failed: " + cj_clientman->GetStatuses().original + ", will retry"));
+},
+    };
+}
+
+static RPCHelpMan coinjoin_stop()
+{
+    return RPCHelpMan{"coinjoin stop",
+        "\nStop CoinJoin mixing\n",
+        {},
+        RPCResult{
+            RPCResult::Type::STR, "", "Status of request"
+        },
+        RPCExamples{
+            HelpExampleCli("coinjoin stop", "")
+          + HelpExampleRpc("coinjoin stop", "")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    if (!wallet) return NullUniValue;
+
+    const NodeContext& node = EnsureAnyNodeContext(request.context);
+
+    if (node.mn_activeman) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Client-side mixing is not supported on masternodes");
     }
 
-    if (request.params[0].get_str() == "stop") {
+    ValidateCoinJoinArguments();
+
+    CHECK_NONFATAL(node.coinjoin_loader);
+    auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName());
+
+    CHECK_NONFATAL(cj_clientman);
+    if (!cj_clientman->IsMixing()) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "No mix session to stop");
+    }
     cj_clientman->StopMixing();
+
     return "Mixing was stopped";
-    }
-
-    if (request.params[0].get_str() == "reset") {
-        cj_clientman->ResetPool();
-        return "Mixing was reset";
-    }
-
-    return "Unknown command, please see \"help coinjoin\"";
 },
     };
 }
@@ -192,15 +282,18 @@ void RegisterCoinJoinRPCCommands(CRPCTable &t)
 // clang-format off
 static const CRPCCommand commands[] =
     { //  category              name                          actor (function)         argNames
-        //  --------------------- ------------------------  ---------------------------------
+      //  ------------------------------------------------------------------------------------------------------
         { "dash",               "getpoolinfo",                &getpoolinfo,            {} },
         { "dash",               "getcoinjoininfo",            &getcoinjoininfo,        {} },
 #ifdef ENABLE_WALLET
         { "dash",               "coinjoin",                   &coinjoin,               {"command"} },
+        { "dash",               "coinjoin", "reset",          &coinjoin_reset,         {} },
+        { "dash",               "coinjoin", "start",          &coinjoin_start,         {} },
+        { "dash",               "coinjoin", "stop",           &coinjoin_stop,          {} },
 #endif // ENABLE_WALLET
 };
 // clang-format on
     for (const auto& command : commands) {
-        t.appendCommand(command.name, &command);
+        t.appendCommand(command.name, command.subname, &command);
     }
 }