diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index bc0dd30133df..6f7494043f0e 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -283,6 +283,7 @@ void CCoinJoinServer::SetNull() AssertLockHeld(cs_coinjoin); // MN side vecSessionCollaterals.clear(); + setSessionCollateralPrevouts.clear(); CCoinJoinBaseSession::SetNull(); m_queueman.SetNull(); @@ -743,6 +744,15 @@ bool CCoinJoinServer::IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& n return true; } +void CCoinJoinServer::CommitSessionCollateral(const CMutableTransaction& txCollateral) +{ + AssertLockHeld(cs_coinjoin); + vecSessionCollaterals.push_back(MakeTransactionRef(txCollateral)); + for (const auto& txin : txCollateral.vin) { + setSessionCollateralPrevouts.insert(txin.prevout); + } +} + bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) { if (nSessionID != 0) return false; @@ -758,12 +768,26 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& return false; } - // start new session - nMessageIDRet = MSG_NOERR; - nSessionID = GetRand(/*nMax=*/999999) + 1; - nSessionDenom = dsa.nDenom; + { + LOCK(cs_coinjoin); + + // A scheduler-thread timeout can reset the session via SetNull() between the checks + // above and taking cs_coinjoin, so revalidate: the session state and the collateral + // that opened it have to be committed as one unit. + if (nSessionID != 0 || nState != POOL_STATE_IDLE) { + nMessageIDRet = ERR_MODE; + return false; + } + + // start new session + nMessageIDRet = MSG_NOERR; + nSessionID = GetRand(/*nMax=*/999999) + 1; + nSessionDenom = dsa.nDenom; - SetState(POOL_STATE_QUEUE); + SetState(POOL_STATE_QUEUE); + + CommitSessionCollateral(dsa.txCollateral); + } if (!fUnitTest) { //broadcast that I'm accepting entries, only if it's the first entry through @@ -775,7 +799,6 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& m_queueman.AddQueue(std::move(dsq)); } - vecSessionCollaterals.push_back(MakeTransactionRef(dsa.txCollateral)); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- new session created, nSessionID: %d nSessionDenom: %d (%s) vecSessionCollaterals.size(): %d CoinJoin::GetMaxPoolParticipants(): %d\n", nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), vecSessionCollaterals.size(), CoinJoin::GetMaxPoolParticipants()); @@ -804,10 +827,32 @@ bool CCoinJoinServer::AddUserToExistingSession(const CCoinJoinAccept& dsa, PoolM return false; } + LOCK(cs_coinjoin); + + // A scheduler-thread timeout can reset the session via SetNull() between the checks above + // and taking cs_coinjoin, so revalidate: a collateral must never be committed to a session + // that no longer exists. + if (nSessionID == 0 || nState != POOL_STATE_QUEUE) { + nMessageIDRet = ERR_MODE; + return false; + } + + // Session collaterals are only ever test-accepted, never added to the mempool, so nothing + // pins their identity: the same UTXO can be re-signed into arbitrarily many distinct txids. + // Match on input prevouts so a resent or replayed dsa cannot be counted as a new participant. + for (const auto& txin : dsa.txCollateral.vin) { + if (setSessionCollateralPrevouts.contains(txin.prevout)) { + LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddUserToExistingSession -- collateral %s spends prevout %s already committed to this session\n", + dsa.txCollateral.GetHash().ToString(), txin.prevout.ToStringShort()); + nMessageIDRet = ERR_ALREADY_HAVE; + return false; + } + } + // count new user as accepted to an existing session nMessageIDRet = MSG_NOERR; - vecSessionCollaterals.push_back(MakeTransactionRef(dsa.txCollateral)); + CommitSessionCollateral(dsa.txCollateral); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::AddUserToExistingSession -- new user accepted, nSessionID: %d nSessionDenom: %d (%s) vecSessionCollaterals.size(): %d CoinJoin::GetMaxPoolParticipants(): %d\n", nSessionID, nSessionDenom, CoinJoin::DenominationToString(nSessionDenom), vecSessionCollaterals.size(), CoinJoin::GetMaxPoolParticipants()); diff --git a/src/coinjoin/server.h b/src/coinjoin/server.h index 3e7443dbc6f0..a04eb4993bd4 100644 --- a/src/coinjoin/server.h +++ b/src/coinjoin/server.h @@ -10,6 +10,9 @@ #include #include #include +#include + +#include class CActiveMasternodeManager; class CConnman; @@ -43,6 +46,9 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler // Mixing uses collateral transactions to trust parties entering the pool // to behave honestly. If they don't it takes their money. std::vector vecSessionCollaterals; + // Input prevouts of every transaction in vecSessionCollaterals, so a dsa whose collateral + // reuses one of them can be rejected without rescanning them all. + std::unordered_set setSessionCollateralPrevouts GUARDED_BY(cs_coinjoin); bool fUnitTest; @@ -66,8 +72,10 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler /// Is this nDenom and txCollateral acceptable? bool IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) const; - bool CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet); - bool AddUserToExistingSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet); + /// Record an accepted collateral and index its input prevouts + void CommitSessionCollateral(const CMutableTransaction& txCollateral) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); + bool CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); + bool AddUserToExistingSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); /// Do we have enough users to take entries? bool IsSessionReady() const; @@ -85,7 +93,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler void RelayStatus(PoolStatusUpdate nStatusUpdate, PoolMessage nMessageID = MSG_NOERR) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin); void RelayCompletedTransaction(PoolMessage nMessageID) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); - void ProcessDSACCEPT(CNode& peer, CDataStream& vRecv); + void ProcessDSACCEPT(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); void ProcessDSQUEUE(NodeId from, CDataStream& vRecv); void ProcessDSVIN(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin); void ProcessDSSIGNFINALTX(CNode& peer, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin);