Coinjoin cs adjustments

src/coinjoin/coinjoin-client-options.cpp

@@ -22,36 +22,31 @@ CCoinJoinClientOptions& CCoinJoinClientOptions::Get()
 void CCoinJoinClientOptions::SetEnabled(bool fEnabled)
 {
     CCoinJoinClientOptions& options = CCoinJoinClientOptions::Get();
-    LOCK(options.cs_cj_options);
     options.fEnableCoinJoin = fEnabled;
 }
 
 void CCoinJoinClientOptions::SetMultiSessionEnabled(bool fEnabled)
 {
     CCoinJoinClientOptions& options = CCoinJoinClientOptions::Get();
-    LOCK(options.cs_cj_options);
     options.fCoinJoinMultiSession = fEnabled;
 }
 
 void CCoinJoinClientOptions::SetRounds(int nRounds)
 {
     CCoinJoinClientOptions& options = CCoinJoinClientOptions::Get();
-    LOCK(options.cs_cj_options);
     options.nCoinJoinRounds = nRounds;
 }
 
 void CCoinJoinClientOptions::SetAmount(CAmount amount)
 {
     CCoinJoinClientOptions& options = CCoinJoinClientOptions::Get();
-    LOCK(options.cs_cj_options);
     options.nCoinJoinAmount = amount;
 }
 
 void CCoinJoinClientOptions::Init()
 {
     assert(!CCoinJoinClientOptions::_instance);
     static CCoinJoinClientOptions instance;
-    LOCK(instance.cs_cj_options);
     instance.fCoinJoinMultiSession = gArgs.GetBoolArg("-coinjoinmultisession", DEFAULT_COINJOIN_MULTISESSION);
     instance.nCoinJoinSessions = std::min(std::max((int)gArgs.GetArg("-coinjoinsessions", DEFAULT_COINJOIN_SESSIONS), MIN_COINJOIN_SESSIONS), MAX_COINJOIN_SESSIONS);
     instance.nCoinJoinRounds = std::min(std::max((int)gArgs.GetArg("-coinjoinrounds", DEFAULT_COINJOIN_ROUNDS), MIN_COINJOIN_ROUNDS), MAX_COINJOIN_ROUNDS);

src/coinjoin/coinjoin-client-options.h

@@ -6,7 +6,8 @@
 #define BITCOIN_COINJOIN_COINJOIN_CLIENT_OPTIONS_H
 
 #include <amount.h>
-#include <sync.h>
+#include <atomic>
+#include <mutex>
 
 class UniValue;
 
@@ -72,26 +73,16 @@ class CCoinJoinClientOptions
     static CCoinJoinClientOptions* _instance;
     static std::once_flag onceFlag;
 
-    CCriticalSection cs_cj_options;
-    int nCoinJoinSessions;
-    int nCoinJoinRounds;
-    int nCoinJoinRandomRounds;
-    int nCoinJoinAmount;
-    int nCoinJoinDenomsGoal;
-    int nCoinJoinDenomsHardCap;
-    bool fEnableCoinJoin;
-    bool fCoinJoinMultiSession;
-
-    CCoinJoinClientOptions() :
-            nCoinJoinRounds(DEFAULT_COINJOIN_ROUNDS),
-            nCoinJoinRandomRounds(COINJOIN_RANDOM_ROUNDS),
-            nCoinJoinAmount(DEFAULT_COINJOIN_AMOUNT),
-            nCoinJoinDenomsGoal(DEFAULT_COINJOIN_DENOMS_GOAL),
-            nCoinJoinDenomsHardCap(DEFAULT_COINJOIN_DENOMS_HARDCAP),
-            fEnableCoinJoin(false),
-            fCoinJoinMultiSession(DEFAULT_COINJOIN_MULTISESSION)
-    {
-    }
+    std::atomic<int> nCoinJoinSessions{DEFAULT_COINJOIN_SESSIONS};
+    std::atomic<int> nCoinJoinRounds{DEFAULT_COINJOIN_ROUNDS};
+    std::atomic<int> nCoinJoinRandomRounds{COINJOIN_RANDOM_ROUNDS};
+    std::atomic<int> nCoinJoinAmount{DEFAULT_COINJOIN_AMOUNT};
+    std::atomic<int> nCoinJoinDenomsGoal{DEFAULT_COINJOIN_DENOMS_GOAL};
+    std::atomic<int> nCoinJoinDenomsHardCap{DEFAULT_COINJOIN_DENOMS_HARDCAP};
+    std::atomic<bool> fEnableCoinJoin{false};
+    std::atomic<bool> fCoinJoinMultiSession{DEFAULT_COINJOIN_MULTISESSION};
+
+    CCoinJoinClientOptions() = default;
 
     CCoinJoinClientOptions(const CCoinJoinClientOptions& other) = delete;
     CCoinJoinClientOptions& operator=(const CCoinJoinClientOptions&) = delete;

src/coinjoin/coinjoin-client.cpp

@@ -265,9 +265,9 @@ void CCoinJoinClientSession::ResetPool()
 
 void CCoinJoinClientManager::ResetPool()
 {
-    LOCK(cs_deqsessions);
     nCachedLastSuccessBlock = 0;
     vecMasternodesUsed.clear();
+    LOCK(cs_deqsessions);
     for (auto& session : deqSessions) {
         session.ResetPool();
     }
@@ -346,10 +346,10 @@ std::string CCoinJoinClientSession::GetStatus(bool fWaitForBlock) const
 
 std::string CCoinJoinClientManager::GetStatuses()
 {
-    LOCK(cs_deqsessions);
     std::string strStatus;
     bool fWaitForBlock = WaitForAnotherBlock();
 
+    LOCK(cs_deqsessions);
     for (const auto& session : deqSessions) {
         strStatus += session.GetStatus(fWaitForBlock) + "; ";
     }
@@ -358,9 +358,9 @@ std::string CCoinJoinClientManager::GetStatuses()
 
 std::string CCoinJoinClientManager::GetSessionDenoms()
 {
-    LOCK(cs_deqsessions);
     std::string strSessionDenoms;
 
+    LOCK(cs_deqsessions);
     for (const auto& session : deqSessions) {
         strSessionDenoms += CCoinJoin::DenominationToString(session.nSessionDenom) + "; ";
     }
@@ -490,6 +490,7 @@ bool CCoinJoinClientSession::SendDenominate(const std::vector<std::pair<CTxDSIn,
     LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::SendDenominate -- Submitting partial tx %s", tx.ToString()); /* Continued */
 
     // store our entry for later use
+    LOCK(cs_coinjoin);
     vecEntries.emplace_back(vecTxDSInTmp, vecTxOutTmp, txMyCollateral);
     RelayIn(vecEntries.back(), connman);
     nTimeLastSuccessfulStep = GetTime();
@@ -557,6 +558,8 @@ bool CCoinJoinClientSession::SignFinalTransaction(const CTransaction& finalTrans
     if (fMasternodeMode || pnode == nullptr) return false;
     if (!mixingMasternode) return false;
 
+    LOCK(cs_coinjoin);
+
     finalMutableTransaction = CMutableTransaction{finalTransactionNew};
     LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- finalMutableTransaction=%s", __func__, finalMutableTransaction.ToString()); /* Continued */
 
@@ -588,11 +591,11 @@ bool CCoinJoinClientSession::SignFinalTransaction(const CTransaction& finalTrans
 
     std::vector<CTxIn> sigs;
 
-    for (const auto& entry : vecEntries) {
+    for (const auto &entry: vecEntries) {
         // Check that the final transaction has all our outputs
-        for (const auto& txout : entry.vecTxOut) {
+        for (const auto &txout: entry.vecTxOut) {
             bool fFound = false;
-            for (const auto& txoutFinal : finalMutableTransaction.vout) {
+            for (const auto &txoutFinal: finalMutableTransaction.vout) {
                 if (txoutFinal == txout) {
                     fFound = true;
                     break;
@@ -641,7 +644,7 @@ bool CCoinJoinClientSession::SignFinalTransaction(const CTransaction& finalTrans
 
             sigs.push_back(finalMutableTransaction.vin[nMyInputIndex]);
             LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- nMyInputIndex: %d, sigs.size(): %d, scriptSig=%s\n",
-                    __func__, nMyInputIndex, (int)sigs.size(), ScriptToAsmStr(finalMutableTransaction.vin[nMyInputIndex].scriptSig));
+                     __func__, nMyInputIndex, (int) sigs.size(),ScriptToAsmStr(finalMutableTransaction.vin[nMyInputIndex].scriptSig));
         }
     }
 
@@ -973,8 +976,8 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(CConnman& connman, bool fDr
         LogPrint(BCLog::COINJOIN, "  vecMasternodesUsed: new size: %d, threshold: %d\n", (int)vecMasternodesUsed.size(), nThreshold_high);
     }
 
-    LOCK(cs_deqsessions);
     bool fResult = true;
+    LOCK(cs_deqsessions);
     if ((int)deqSessions.size() < CCoinJoinClientOptions::GetSessions()) {
         deqSessions.emplace_back(mixingWallet);
     }
@@ -1865,11 +1868,11 @@ void CCoinJoinClientSession::GetJsonInfo(UniValue& obj) const
 
 void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const
 {
-    LOCK(cs_deqsessions);
     assert(obj.isObject());
     obj.pushKV("running",       IsMixing());
 
     UniValue arrSessions(UniValue::VARR);
+    LOCK(cs_deqsessions);
     for (const auto& session : deqSessions) {
         if (session.GetState() != POOL_STATE_IDLE) {
             UniValue objSession(UniValue::VOBJ);

src/coinjoin/coinjoin-client.h

@@ -174,9 +174,9 @@ class CCoinJoinClientManager
     // Keep track of the used Masternodes
     std::vector<COutPoint> vecMasternodesUsed;
 
-    // TODO: or map<denom, CCoinJoinClientSession> ??
-    std::deque<CCoinJoinClientSession> deqSessions;
     mutable CCriticalSection cs_deqsessions;
+    // TODO: or map<denom, CCoinJoinClientSession> ??
+    std::deque<CCoinJoinClientSession> deqSessions GUARDED_BY(cs_deqsessions);
 
     bool fMixing{false};
 

src/coinjoin/coinjoin-server.cpp

@@ -302,6 +302,8 @@ void CCoinJoinServer::CreateFinalTransaction(CConnman& connman)
 {
     LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateFinalTransaction -- FINALIZE TRANSACTIONS\n");
 
+    LOCK(cs_coinjoin);
+
     CMutableTransaction txNew;
 
     // make our new transaction
@@ -329,7 +331,7 @@ void CCoinJoinServer::CommitFinalTransaction(CConnman& connman)
 {
     if (!fMasternodeMode) return; // check and relay final tx only on masternode
 
-    CTransactionRef finalTransaction = MakeTransactionRef(finalMutableTransaction);
+    CTransactionRef finalTransaction = WITH_LOCK(cs_coinjoin, return MakeTransactionRef(finalMutableTransaction));
     uint256 hashTx = finalTransaction->GetHash();
 
     LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- finalTransaction=%s", finalTransaction->ToString()); /* Continued */
@@ -395,6 +397,7 @@ void CCoinJoinServer::ChargeFees(CConnman& connman) const
     std::vector<CTransactionRef> vecOffendersCollaterals;
 
     if (nState == POOL_STATE_ACCEPTING_ENTRIES) {
+        LOCK(cs_coinjoin);
         for (const auto& txCollateral : vecSessionCollaterals) {
             bool fFound = false;
             for (const auto& entry : vecEntries) {
@@ -414,6 +417,7 @@ void CCoinJoinServer::ChargeFees(CConnman& connman) const
 
     if (nState == POOL_STATE_SIGNING) {
         // who didn't sign?
+        LOCK(cs_coinjoin);
         for (const auto& entry : vecEntries) {
             for (const auto& txdsin : entry.vecTxDSIn) {
                 if (!txdsin.fHasSig) {
@@ -538,21 +542,23 @@ bool CCoinJoinServer::IsInputScriptSigValid(const CTxIn& txin) const
     int nTxInIndex = -1;
     CScript sigPubKey = CScript();
 
-    for (const auto& entry : vecEntries) {
-        for (const auto& txout : entry.vecTxOut) {
-            txNew.vout.push_back(txout);
-        }
-        for (const auto& txdsin : entry.vecTxDSIn) {
-            txNew.vin.push_back(txdsin);
+    {
+        LOCK(cs_coinjoin);
+        for (const auto &entry: vecEntries) {
+            for (const auto &txout: entry.vecTxOut) {
+                txNew.vout.push_back(txout);
+            }
+            for (const auto &txdsin: entry.vecTxDSIn) {
+                txNew.vin.push_back(txdsin);
 
-            if (txdsin.prevout == txin.prevout) {
-                nTxInIndex = i;
-                sigPubKey = txdsin.prevPubKey;
+                if (txdsin.prevout == txin.prevout) {
+                    nTxInIndex = i;
+                    sigPubKey = txdsin.prevPubKey;
+                }
+                i++;
             }
-            i++;
         }
     }
-
     if (nTxInIndex >= 0) { //might have to do this one input at a time?
         txNew.vin[nTxInIndex].scriptSig = txin.scriptSig;
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::IsInputScriptSigValid -- verifying scriptSig %s\n", ScriptToAsmStr(txin.scriptSig).substr(0, 24));
@@ -599,7 +605,7 @@ bool CCoinJoinServer::AddEntry(CConnman& connman, const CCoinJoinEntry& entry, P
     std::vector<CTxIn> vin;
     for (const auto& txin : entry.vecTxDSIn) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- txin=%s\n", __func__, txin.ToString());
-
+        LOCK(cs_coinjoin);
         for (const auto& inner_entry : vecEntries) {
             for (const auto& txdsin : inner_entry.vecTxDSIn) {
                 if (txdsin.prevout == txin.prevout) {
@@ -624,7 +630,7 @@ bool CCoinJoinServer::AddEntry(CConnman& connman, const CCoinJoinEntry& entry, P
         return false;
     }
 
-    vecEntries.push_back(entry);
+    WITH_LOCK(cs_coinjoin, vecEntries.push_back(entry));
 
     LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- adding entry %d of %d required\n", __func__, GetEntriesCount(), CCoinJoin::GetMaxPoolParticipants());
     nMessageIDRet = MSG_ENTRIES_ADDED;
@@ -636,6 +642,7 @@ bool CCoinJoinServer::AddScriptSig(const CTxIn& txinNew)
 {
     LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddScriptSig -- scriptSig=%s\n", ScriptToAsmStr(txinNew.scriptSig).substr(0, 24));
 
+    LOCK(cs_coinjoin);
     for (const auto& entry : vecEntries) {
         for (const auto& txdsin : entry.vecTxDSIn) {
             if (txdsin.scriptSig == txinNew.scriptSig) {
@@ -672,6 +679,7 @@ bool CCoinJoinServer::AddScriptSig(const CTxIn& txinNew)
 // Check to make sure everything is signed
 bool CCoinJoinServer::IsSignaturesComplete() const
 {
+    LOCK(cs_coinjoin);
     for (const auto& entry : vecEntries) {
         for (const auto& txdsin : entry.vecTxDSIn) {
             if (!txdsin.fHasSig) return false;
@@ -730,6 +738,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
         dsq.Sign();
         dsq.Relay(connman);
+        LOCK(cs_vecqueue);
         vecCoinJoinQueue.push_back(dsq);
     }
 
@@ -796,10 +805,11 @@ void CCoinJoinServer::RelayFinalTransaction(const CTransaction& txFinal, CConnma
         __func__, nSessionID, nSessionDenom, CCoinJoin::DenominationToString(nSessionDenom));
 
     // final mixing tx with empty signatures should be relayed to mixing participants only
+    LOCK(cs_coinjoin);
     for (const auto& entry : vecEntries) {
         bool fOk = connman.ForNode(entry.addr, [&txFinal, &connman, this](CNode* pnode) {
             CNetMsgMaker msgMaker(pnode->GetSendVersion());
-            connman.PushMessage(pnode, msgMaker.Make(NetMsgType::DSFINALTX, nSessionID, txFinal));
+            connman.PushMessage(pnode, msgMaker.Make(NetMsgType::DSFINALTX, nSessionID.load(), txFinal));
             return true;
         });
         if (!fOk) {
@@ -821,6 +831,7 @@ void CCoinJoinServer::RelayStatus(PoolStatusUpdate nStatusUpdate, CConnman& conn
 {
     unsigned int nDisconnected{};
     // status updates should be relayed to mixing participants only
+    LOCK(cs_coinjoin);
     for (const auto& entry : vecEntries) {
         // make sure everyone is still connected
         bool fOk = connman.ForNode(entry.addr, [&nStatusUpdate, &nMessageID, &connman, this](CNode* pnode) {
@@ -859,10 +870,11 @@ void CCoinJoinServer::RelayCompletedTransaction(PoolMessage nMessageID, CConnman
         __func__, nSessionID, nSessionDenom, CCoinJoin::DenominationToString(nSessionDenom));
 
     // final mixing tx with empty signatures should be relayed to mixing participants only
+    LOCK(cs_coinjoin);
     for (const auto& entry : vecEntries) {
         bool fOk = connman.ForNode(entry.addr, [&nMessageID, &connman, this](CNode* pnode) {
             CNetMsgMaker msgMaker(pnode->GetSendVersion());
-            connman.PushMessage(pnode, msgMaker.Make(NetMsgType::DSCOMPLETE, nSessionID, nMessageID));
+            connman.PushMessage(pnode, msgMaker.Make(NetMsgType::DSCOMPLETE, nSessionID.load(), nMessageID));
             return true;
         });
         if (!fOk) {

src/coinjoin/coinjoin-util.cpp

@@ -98,7 +98,6 @@ CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBui
 
 bool CTransactionBuilderOutput::UpdateAmount(const CAmount nNewAmount)
 {
-    LOCK(pTxBuilder->cs_outputs);
     if (nNewAmount <= 0 || nNewAmount - nAmount > pTxBuilder->GetAmountLeft()) {
         return false;
     }
@@ -200,8 +199,8 @@ bool CTransactionBuilder::CouldAddOutputs(const std::vector<CAmount>& vecOutputA
 
 CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
 {
-    LOCK(cs_outputs);
     if (CouldAddOutput(nAmountOutput)) {
+        LOCK(cs_outputs);
         vecOutputs.push_back(std::make_unique<CTransactionBuilderOutput>(this, pwallet, nAmountOutput));
         return vecOutputs.back().get();
     }
@@ -210,6 +209,7 @@ CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
 
 unsigned int CTransactionBuilder::GetBytesTotal() const
 {
+    LOCK(cs_outputs);
     // Adding other outputs can change the serialized size of the vout size hence + GetSizeOfCompactSizeDiff()
     return nBytesBase + vecOutputs.size() * nBytesOutput + ::GetSizeOfCompactSizeDiff(0, vecOutputs.size());
 }
@@ -222,6 +222,7 @@ CAmount CTransactionBuilder::GetAmountLeft(const CAmount nAmountInitial, const C
 CAmount CTransactionBuilder::GetAmountUsed() const
 {
     CAmount nAmountUsed{0};
+    LOCK(cs_outputs);
     for (const auto& out : vecOutputs) {
         nAmountUsed += out->GetAmount();
     }
@@ -243,7 +244,7 @@ CAmount CTransactionBuilder::GetFee(unsigned int nBytes) const
 
 int CTransactionBuilder::GetSizeOfCompactSizeDiff(size_t nAdd) const
 {
-    size_t nSize = vecOutputs.size();
+    size_t nSize = WITH_LOCK(cs_outputs, return vecOutputs.size());
     unsigned int ret = ::GetSizeOfCompactSizeDiff(nSize, nSize + nAdd);
     assert(ret <= INT_MAX);
     return (int)ret;