refactor: introduce coinjoin status for information on mix sessions

kwvg · kwvg · commit 48fccdd010b3 · 2025-02-24T21:26:11.000Z
This helps us get rid of having to report it through `coinjoin start`.

Additionally, we won't try to jump ahead of the scheduler by starting
denomination routines ourselves, let the scheduler run its course.
This matters because we don't plan on exposing that capability through
the interface as we are migrating to the interface in the next commit.
diff --git a/doc/release-notes-6594.md b/doc/release-notes-6594.md
@@ -0,0 +1,7 @@
+Updated RPCs
+------------
+
+* `coinjoin status` is a new RPC that reports the status message of all running mix
+  sessions.
+* `coinjoin start` will no longer report errors from mix sessions, users are recommended
+  to query the status of mix sessions using `coinjoin status` instead.
diff --git a/src/coinjoin/client.cpp b/src/coinjoin/client.cpp
@@ -350,17 +350,18 @@ bilingual_str CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const
     }
 }
 
-bilingual_str CCoinJoinClientManager::GetStatuses()
+std::vector<std::string> CCoinJoinClientManager::GetStatuses() const
 {
-    bilingual_str strStatus;
-    bool fWaitForBlock = WaitForAnotherBlock();
-
     AssertLockNotHeld(cs_deqsessions);
+
+    bool fWaitForBlock{WaitForAnotherBlock()};
+    std::vector<std::string> ret;
+
     LOCK(cs_deqsessions);
     for (const auto& session : deqSessions) {
-        strStatus = strStatus + session.GetStatus(fWaitForBlock) + Untranslated("; ");
+        ret.push_back(session.GetStatus(fWaitForBlock).original);
     }
-    return strStatus;
+    return ret;
 }
 
 std::string CCoinJoinClientManager::GetSessionDenoms()
diff --git a/src/coinjoin/client.h b/src/coinjoin/client.h
@@ -327,7 +327,7 @@ class CCoinJoinClientManager
     bool IsMixing() const;
     void ResetPool() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
 
-    bilingual_str GetStatuses() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
+    std::vector<std::string> GetStatuses() const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
     std::string GetSessionDenoms() EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
 
     bool GetMixingMasternodesInfo(std::vector<CDeterministicMNCPtr>& vecDmnsRet) const EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp
@@ -45,6 +45,7 @@ static RPCHelpMan coinjoin()
     return RPCHelpMan{"coinjoin",
         "\nAvailable commands:\n"
         "  start       - Start mixing\n"
+        "  status      - Get mixing status\n"
         "  stop        - Stop mixing\n"
         "  reset       - Reset mixing",
         {
@@ -129,11 +130,46 @@ static RPCHelpMan coinjoin_start()
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Mixing has been started already.");
     }
 
-    ChainstateManager& chainman = EnsureChainman(node);
-    CTxMemPool& mempool = EnsureMemPool(node);
-    CConnman& connman = EnsureConnman(node);
-    bool result = cj_clientman->DoAutomaticDenominating(chainman, connman, mempool);
-    return "Mixing " + (result ? "started successfully" : ("start failed: " + cj_clientman->GetStatuses().original + ", will retry"));
+    return "Mixing requested";
+},
+    };
+}
+
+static RPCHelpMan coinjoin_status()
+{
+    return RPCHelpMan{"coinjoin status",
+        "\nGet status on CoinJoin mixing sessions\n",
+        {},
+        RPCResult{
+            RPCResult::Type::ARR, "", "",
+            {{RPCResult::Type::STR, "", "Status of mixing session"}}},
+        RPCExamples{
+            HelpExampleCli("coinjoin status", "")
+          + HelpExampleRpc("coinjoin status", "")
+        },
+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
+{
+    const std::shared_ptr<const CWallet> 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();
+
+    auto cj_clientman = CHECK_NONFATAL(node.coinjoin_loader)->walletman().Get(wallet->GetName());
+    if (!CHECK_NONFATAL(cj_clientman)->IsMixing()) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "No ongoing mix session");
+    }
+
+    UniValue ret(UniValue::VARR);
+    for (auto str_status : cj_clientman->GetStatuses()) {
+        ret.push_back(str_status);
+    }
+    return ret;
 },
     };
 }
@@ -460,6 +496,7 @@ static const CRPCCommand commands[] =
     { "dash",                &coinjoin,               },
     { "dash",                &coinjoin_reset,         },
     { "dash",                &coinjoin_start,         },
+    { "dash",                &coinjoin_status,        },
     { "dash",                &coinjoin_stop,          },
     { "dash",                &coinjoinsalt,           },
     { "dash",                &coinjoinsalt_generate,  },
diff --git a/test/functional/rpc_coinjoin.py b/test/functional/rpc_coinjoin.py
@@ -66,6 +66,8 @@ def test_coinjoin_start_stop(self, node):
         assert_equal(cj_info['running'], True)
         # Repeated start should yield error
         assert_raises_rpc_error(-32603, 'Mixing has been started already.', node.coinjoin, 'start')
+        # Requesting status shouldn't complain
+        node.coinjoin('status')
 
         # Stop mix session and ensure it's reported
         node.coinjoin('stop')
@@ -74,6 +76,8 @@ def test_coinjoin_start_stop(self, node):
         assert_equal(cj_info['running'], False)
         # Repeated stop should yield error
         assert_raises_rpc_error(-32603, 'No mix session to stop', node.coinjoin, 'stop')
+        # Requesting status should tell us off
+        assert_raises_rpc_error(-32603, 'No ongoing mix session', node.coinjoin, 'status')
 
         # Reset mix session
         assert_equal(node.coinjoin('reset'), "Mixing was reset")

Original file line number	Diff line number	Diff line change
`@@ -350,17 +350,18 @@ bilingual_str CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const`
`350`	`350`	`}`
`351`	`351`	`}`
`352`	`352`
`353`		`-bilingual_str CCoinJoinClientManager::GetStatuses()`
	`353`	`+std::vector<std::string> CCoinJoinClientManager::GetStatuses() const`
`354`	`354`	`{`
`355`		`- bilingual_str strStatus;`
`356`		`- bool fWaitForBlock = WaitForAnotherBlock();`
`357`		`-`
`358`	`355`	`AssertLockNotHeld(cs_deqsessions);`
	`356`	`+`
	`357`	`+ bool fWaitForBlock{WaitForAnotherBlock()};`
	`358`	`+ std::vector<std::string> ret;`
	`359`	`+`
`359`	`360`	`LOCK(cs_deqsessions);`
`360`	`361`	`for (const auto& session : deqSessions) {`
`361`		`- strStatus = strStatus + session.GetStatus(fWaitForBlock) + Untranslated("; ");`
	`362`	`+ ret.push_back(session.GetStatus(fWaitForBlock).original);`
`362`	`363`	`}`
`363`		`- return strStatus;`
	`364`	`+ return ret;`
`364`	`365`	`}`
`365`	`366`
`366`	`367`	`std::string CCoinJoinClientManager::GetSessionDenoms()`