fix: recheck live CoinJoin side coverage before finalizing timeout session

src/coinjoin/server.cpp

@@ -373,11 +373,23 @@ void CCoinJoinServer::CreateFinalTransaction(int session_id)
     LOCK(cs_coinjoin);
 
     // The decision to finalize came from a snapshot taken before this lock, so make sure it
-    // still describes the live session - it may have timed out and been replaced in between.
+    // still describes an eligible live session. An entry can finish validation and commit while
+    // the timeout path charges fees, changing a covered side from empty to a lone participant.
+    // Check the live entries under the same lock used to build the transaction so that entry
+    // admission cannot invalidate the decision before the state moves to signing.
     if (!IsCurrentSession(session_id)) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- session changed, not finalizing\n");
         return;
     }
+    const auto sides = GetMixSideCountsLocked();
+    if (vecEntries.size() < static_cast<size_t>(CoinJoin::GetMinPoolParticipants()) || !sides.IsCovered()) {
+        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- session no longer eligible, entries=%d, sides=%d/%d\n",
+                 vecEntries.size(), sides.inputs, sides.outputs);
+        // Deliberately no timer refresh: on the timeout path the session stays timed out and
+        // the scheduler's next CheckTimeout() resets it. The missing side already had the full
+        // entry window, so waiting another one would only keep everyone else's coins locked.
+        return;
+    }
 
     CMutableTransaction txNew;
 

src/coinjoin/server.h

@@ -76,6 +76,8 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler
     bool AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessageIDRet) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin);
     /// Record an accepted collateral and index its input prevouts
     void CommitSessionCollateral(const CMutableTransaction& txCollateral) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
+    /// Build and relay the final transaction if the live session is still eligible
+    void CreateFinalTransaction(int session_id) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin);
 
 private:
     bool fUnitTest;
@@ -114,8 +116,6 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler
     /// Check for process
     void CheckPool();
 
-    /// Build and relay the final transaction, unless session_id is no longer the live session
-    void CreateFinalTransaction(int session_id) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin);
     void CommitFinalTransaction() EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin);
 
     /// Is this nDenom and txCollateral acceptable?

src/test/coinjoin_inouts_tests.cpp

@@ -181,8 +181,9 @@ BOOST_AUTO_TEST_CASE(entry_addscriptsig_matches_and_rejects)
 class TestableCoinJoinServer : public CCoinJoinServer
 {
 public:
-    using CCoinJoinServer::CCoinJoinServer;
     using CCoinJoinServer::AddEntry;
+    using CCoinJoinServer::CCoinJoinServer;
+    using CCoinJoinServer::CreateFinalTransaction;
 
     // A live session always carries a non-zero id, and AddEntry rejects entries that don't
     // belong to one, so seed an id along with the state.
@@ -427,6 +428,44 @@ BOOST_AUTO_TEST_CASE(server_addentry_rejects_entries_once_the_session_finalized)
     BOOST_CHECK_EQUAL(server.GetEntriesCount(), 0);
 }
 
+BOOST_AUTO_TEST_CASE(server_finalization_rechecks_live_side_coverage)
+{
+    CActiveMasternodeManager mn_activeman(*Assert(m_node.connman), *Assert(m_node.dmnman), MakeSecretKey());
+    TestableCoinJoinServer server(m_node.peerman.get(), *Assert(m_node.chainman), *Assert(m_node.connman),
+                                  *Assert(m_node.dmnman), *Assert(m_node.dstxman), *Assert(m_node.mn_metaman),
+                                  *Assert(m_node.mempool), mn_activeman, *Assert(m_node.mn_sync),
+                                  *Assert(m_node.isman));
+
+    const auto make_entry = [](CoinJoin::MixShape shape, uint32_t tag) {
+        const size_t input_count{shape == CoinJoin::MixShape::PROMOTION ? size_t{CoinJoin::PROMOTION_RATIO} : 1};
+        const size_t output_count{shape == CoinJoin::MixShape::DEMOTION ? size_t{CoinJoin::PROMOTION_RATIO} : 1};
+        std::vector<CTxDSIn> inputs;
+        std::vector<CTxOut> outputs;
+        for (size_t i{0}; i < input_count; ++i) {
+            inputs.emplace_back(CTxIn{COutPoint{uint256::ONE, tag + static_cast<uint32_t>(i)}}, P2PKHScript(), 0);
+        }
+        for (size_t i{0}; i < output_count; ++i) {
+            outputs.emplace_back(CoinJoin::GetSmallestDenomination(), P2PKHScript(static_cast<uint8_t>(tag + i)));
+        }
+        return CCoinJoinEntry{inputs, outputs, CTransaction{CMutableTransaction{}}};
+    };
+
+    server.SeedEntry(make_entry(CoinJoin::MixShape::DEMOTION, 0));
+    server.SeedEntry(make_entry(CoinJoin::MixShape::DEMOTION, 10));
+    server.SeedEntry(make_entry(CoinJoin::MixShape::DEMOTION, 20));
+    server.SeedEntry(make_entry(CoinJoin::MixShape::PROMOTION, 30));
+    server.EnterAcceptingEntriesState();
+
+    // A timeout snapshot could have observed only the three demotions as covered (0/3), then
+    // this first promotion could commit while ChargeFees() ran. Finalization must use the live
+    // 1/3 side counts and refuse to build the uncovered transaction, staying out of
+    // POOL_STATE_SIGNING; the still-timed-out session is then reset by the scheduler's
+    // regular CheckTimeout() pass instead of leaking a lone promoter on-chain.
+    server.CreateFinalTransaction(/*session_id=*/1);
+    BOOST_CHECK_EQUAL(server.GetState(), int{POOL_STATE_ACCEPTING_ENTRIES});
+    BOOST_CHECK_EQUAL(server.GetEntriesCount(), 4);
+}
+
 BOOST_AUTO_TEST_CASE(entry_deserializes_vectors_through_wire_cap)
 {
     const size_t wire_cap{CoinJoin::GetMaxPoolInputOutputCount()};