From 51b6b94fc0fc659133329adb70f5c44ddb1c2f1d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 11 Jul 2024 16:16:41 +0000 Subject: [PATCH 1/2] rpc: make `coinjoin` a composite command --- src/rpc/coinjoin.cpp | 188 ++++++++++++++++++++++++++++++++----------- 1 file changed, 139 insertions(+), 49 deletions(-) diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 6bf4d8f9b3..1e239edd5f 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -22,19 +22,55 @@ #include #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", - "\nAvailable commands:\n" - " start - Start mixing\n" - " stop - Stop mixing\n" - " reset - Reset mixing", - { - {"command", RPCArg::Type::STR, RPCArg::Optional::NO, "The command to execute"}, - }, - RPCResults{}, - RPCExamples{""}, - [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + return RPCHelpMan{"coinjoin", + "\nAvailable commands:\n" + " start - Start mixing\n" + " stop - Stop mixing\n" + " reset - Reset mixing", + { + {"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 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 const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; @@ -45,49 +81,100 @@ static RPCHelpMan coinjoin() throw JSONRPCError(RPC_INTERNAL_ERROR, "Client-side mixing is not supported on masternodes"); } - if (!CCoinJoinClientOptions::IsEnabled()) { - 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"); - } - } + ValidateCoinJoinArguments(); + 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") { - { - 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(cj_clientman); + cj_clientman->ResetPool(); - if (!cj_clientman->StartMixing()) { - throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing has been started already."); - } + return "Mixing was reset"; +}, + }; +} - const ChainstateManager& chainman = EnsureChainman(node); - CTxMemPool& mempool = EnsureMemPool(node); - 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_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 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") { - cj_clientman->StopMixing(); - return "Mixing was stopped"; + ValidateCoinJoinArguments(); + + { + LOCK(wallet->cs_wallet); + if (wallet->IsLocked(true)) + throw JSONRPCError(RPC_WALLET_UNLOCK_NEEDED, "Error: Please unlock wallet for mixing with walletpassphrase first."); } - if (request.params[0].get_str() == "reset") { - cj_clientman->ResetPool(); - return "Mixing was reset"; + 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."); } - return "Unknown command, please see \"help coinjoin\""; + const ChainstateManager& chainman = EnsureChainman(node); + CTxMemPool& mempool = EnsureMemPool(node); + 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 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(); + + CHECK_NONFATAL(node.coinjoin_loader); + auto cj_clientman = node.coinjoin_loader->walletman().Get(wallet->GetName()); + + CHECK_NONFATAL(cj_clientman); + cj_clientman->StopMixing(); + + return "Mixing was stopped"; }, }; } @@ -191,16 +278,19 @@ void RegisterCoinJoinRPCCommands(CRPCTable &t) { // clang-format off static const CRPCCommand commands[] = - { // category name actor (function) argNames - // --------------------- ------------------------ --------------------------------- - { "dash", "getpoolinfo", &getpoolinfo, {} }, - { "dash", "getcoinjoininfo", &getcoinjoininfo, {} }, + { // category name actor (function) argNames + // ------------------------------------------------------------------------------------------------------ + { "dash", "getpoolinfo", &getpoolinfo, {} }, + { "dash", "getcoinjoininfo", &getcoinjoininfo, {} }, #ifdef ENABLE_WALLET - { "dash", "coinjoin", &coinjoin, {"command"} }, + { "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); } } From 89ade3e340225ccaa612eea652fd5916fbd80447 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 11 Jul 2024 11:17:18 +0000 Subject: [PATCH 2/2] rpc: disallow `coinjoin stop` if there's no CoinJoin session to stop --- doc/release-notes-6107.md | 4 ++++ src/rpc/coinjoin.cpp | 3 +++ 2 files changed, 7 insertions(+) create mode 100644 doc/release-notes-6107.md diff --git a/doc/release-notes-6107.md b/doc/release-notes-6107.md new file mode 100644 index 0000000000..d8299b2ee4 --- /dev/null +++ b/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 diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index 1e239edd5f..29cf8fbb41 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -172,6 +172,9 @@ static RPCHelpMan coinjoin_stop() 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";