Skip to content

fix(coinjoin): push finalized DSTX directly to participants before DSCOMPLETE #7478

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

Open
PastaPastaPasta wants to merge 1 commit into
base: develop
Choose a base branch
from
+28 −0
Open

fix(coinjoin): push finalized DSTX directly to participants before DSCOMPLETE #7478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 27 additions & 0 deletions src/coinjoin/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,14 @@ void CCoinJoinServer::CommitFinalTransaction()

LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- TRANSMITTING DSTX\n");

// Push the fully signed transaction directly to every mixing participant first:
// the inv-based relay below is subject to delayed trickling, so without this a
// participant could process DSCOMPLETE (which is sent directly) and release its
// inputs for reuse before ever seeing the transaction that spends them.
if (const auto dstx = m_dstxman.GetDSTX(hashTx); dstx) {
WITH_LOCK(cs_coinjoin, RelayDSTXToParticipants(dstx));
Comment on lines +395 to +396

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep session inputs locked until the wallet observes the DSTX

When a participant receives this direct DSTX, ProcessTransaction only enqueues the wallet's TransactionAddedToMempool callback; it does not synchronously mark the wallet input spent. The following directly queued DSCOMPLETE is handled immediately and CompletedTransaction() calls UnlockCoins(), so if the validation callback queue has not drained (a normal scheduling race, or a backlog), a -coinjoinmultisession=1 session can still select and sign the same input. Thus sending the messages in socket order does not establish the claimed wallet-observation ordering; retain the lock until the wallet-side callback/observed spend is processed.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

Comment on lines +391 to +396

@thepastaclaw thepastaclaw Jul 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Direct DSTX ordering does not prevent wallet input reuse

Processing DSTX first only adds the transaction to the mempool and enqueues TransactionAddedToMempool; validationinterface.cpp dispatches that notification asynchronously, so CWallet::transactionAddedToMempool() may not yet have called SyncTransaction() and AddToSpends() when the next DSCOMPLETE is processed. DSCOMPLETE then calls CompletedTransaction() and unlocks the session inputs, while AvailableCoins() still sees them as unspent because it relies on the wallet's lock and mapTxSpends state. This is reachable with -coinjoinmultisession=1: if the scheduler dequeues a due CoinJoin maintenance task before the newly queued validation callback and blocks on cs_wallet_manager_map while DSCOMPLETE holds it, the maintenance task resumes immediately after completion unlocks the inputs and can select them before the same scheduler thread processes the wallet callback. Synchronize preceding validation callbacks before dispatching a validated matching DSCOMPLETE without holding CoinJoin locks, or retain the input locks until wallet observation is confirmed. Add a regression test that delays the validation callback and verifies that successful completion cannot make the spent inputs selectable.

source: ['codex']

@PastaPastaPasta PastaPastaPasta Jul 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree; I'll work on another PR. I want this one just doing the send ordering fix

}

CInv inv(MSG_DSTX, hashTx);
m_peer_manager->PeerRelayInv(inv);

Expand Down Expand Up @@ -852,6 +860,25 @@ void CCoinJoinServer::RelayFinalTransaction(const CTransaction& txFinal)
}
}

void CCoinJoinServer::RelayDSTXToParticipants(const CCoinJoinBroadcastTx& dstx)
{
AssertLockHeld(cs_coinjoin);
// the fully signed mixing tx is pushed to mixing participants only, everyone else
// gets it via the regular inv-based relay
for (const auto& entry : vecEntries) {
bool fOk = connman.ForNode(entry.addr, [&dstx, this](CNode* pnode) {
CNetMsgMaker msgMaker(pnode->GetCommonVersion());
connman.PushMessage(pnode, msgMaker.Make(NetMsgType::DSTX, dstx));
return true;
});
if (!fOk) {
// best-effort only: the participant will still get the tx via regular inv relay
LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- failed to push DSTX to participant %s\n", __func__,
entry.addr.ToStringAddrPort());
}
}
}

void CCoinJoinServer::PushStatus(CNode& peer, PoolStatusUpdate nStatusUpdate, PoolMessage nMessageID) const
{
CCoinJoinStatusUpdate psssup(nSessionID, nState, 0, nStatusUpdate, nMessageID);
Expand Down
1 change: 1 addition & 0 deletions src/coinjoin/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public NetHandler

/// Relay mixing Messages
void RelayFinalTransaction(const CTransaction& txFinal) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
void RelayDSTXToParticipants(const CCoinJoinBroadcastTx& dstx) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
void PushStatus(CNode& peer, PoolStatusUpdate nStatusUpdate, PoolMessage nMessageID) const;
void RelayStatus(PoolStatusUpdate nStatusUpdate, PoolMessage nMessageID = MSG_NOERR) EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
void RelayCompletedTransaction(PoolMessage nMessageID) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin);
Expand Down
Loading