Skip to content

fix: reject duplicate session collateral in CoinJoin dsa acceptance #7507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 2 commits into from
Aug 3, 2026
Merged

fix: reject duplicate session collateral in CoinJoin dsa acceptance #7507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36dbbcd
fix: commit CoinJoin session state and its collateral atomically
PastaPastaPasta Aug 3, 2026
7af25a9
fix: reject duplicate session collateral in CoinJoin dsa acceptance
PastaPastaPasta Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 52 additions & 7 deletions src/coinjoin/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ void CCoinJoinServer::SetNull()
AssertLockHeld(cs_coinjoin);
// MN side
vecSessionCollaterals.clear();
setSessionCollateralPrevouts.clear();

CCoinJoinBaseSession::SetNull();
m_queueman.SetNull();
Expand Down Expand Up @@ -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;
Expand All @@ -758,12 +768,26 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
return false;
}

// start new session
nMessageIDRet = MSG_NOERR;
nSessionID = GetRand<int>(/*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<int>(/*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
Expand All @@ -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());

Expand Down Expand Up @@ -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;
}
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
// 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());
Expand Down
14 changes: 11 additions & 3 deletions src/coinjoin/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#include <net_processing.h>
#include <net_types.h>
#include <protocol.h>
#include <util/hasher.h>

#include <unordered_set>

class CActiveMasternodeManager;
class CConnman;
Expand Down Expand Up @@ -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<CTransactionRef> 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<COutPoint, SaltedOutpointHasher> setSessionCollateralPrevouts GUARDED_BY(cs_coinjoin);

bool fUnitTest;

Expand All @@ -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;

Expand All @@ -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);
Expand Down
Loading