Skip to content

fix: return removed code for m_dsq to relay Dsq queue #7227

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

Closed
knst wants to merge 1 commit into from
Closed

fix: return removed code for m_dsq to relay Dsq queue #7227

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
3 changes: 3 additions & 0 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3643,6 +3643,9 @@ void PeerManagerImpl::PostProcessMessage(MessageProcessingResult&& result, NodeI
for (const auto& inv : result.m_inventory) {
RelayInv(inv);
}
for (const auto& dsq : result.m_dsq) {
RelayDSQ(dsq);
}
Comment on lines +3646 to +3648
Copy link

@thepastaclaw thepastaclaw Mar 16, 2026

Choose a reason for hiding this comment

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

🟡 Suggestion: No test covers the client-side m_dsq relay path, risking repeat regression

The DSQUEUE → m_dsq.push_back() → PostProcessMessage → RelayDSQ flow has no unit or functional test coverage. This exact path was silently broken by #7070 without any test catching it. A regression test exercising CCoinJoinClientQueueManager::ProcessMessage and verifying that result.m_dsq is non-empty (or that RelayDSQ is called) would prevent this from recurring. Neither src/test/ nor test/functional/ reference m_dsq, DSQUEUE, or RelayDSQ.

source: ['claude', 'codex']

🤖 Fix this with AI agents 
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/net_processing.cpp`:
- [SUGGESTION] lines 3646-3648: No test covers the client-side m_dsq relay path, risking repeat regression
  The DSQUEUE → `m_dsq.push_back()` → `PostProcessMessage` → `RelayDSQ` flow has no unit or functional test coverage. This exact path was silently broken by #7070 without any test catching it. A regression test exercising `CCoinJoinClientQueueManager::ProcessMessage` and verifying that `result.m_dsq` is non-empty (or that `RelayDSQ` is called) would prevent this from recurring. Neither `src/test/` nor `test/functional/` reference `m_dsq`, `DSQUEUE`, or `RelayDSQ`.

Comment on lines +3646 to +3648
Copy link

@thepastaclaw thepastaclaw Mar 16, 2026

Choose a reason for hiding this comment

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

💬 Nitpick: A comment noting the sole caller would help prevent re-deletion

Since the server side now calls PeerRelayDSQ directly, only CCoinJoinClientQueueManager::ProcessMessage populates m_dsq. A brief comment here would help future developers understand why this loop exists and avoid accidentally removing it again.

Suggested change
for (const auto& dsq : result.m_dsq) {
RelayDSQ(dsq);
}
for (const auto& dsq : result.m_dsq) {
// Relays DSQ messages populated by CCoinJoinClientQueueManager::ProcessMessage (client-side)
RelayDSQ(dsq);
}

source: ['claude']

}

MessageProcessingResult PeerManagerImpl::ProcessPlatformBanMessage(NodeId node, std::string_view msg_type, CDataStream& vRecv)
Expand Down
Loading