Skip to content

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Nov 27, 2025

Issue being fixed or feature implemented

Having duplicated recipients is ok on protocol level, we could be less restrictive in GUI.

Inspired by #7012

What was done?

Handle DuplicateAddress as a warning status instead of an error. Show a confirmation dialog when duplicates are detected, allowing the transaction to proceed if the user confirms.

Screenshot 2025-11-27 at 22 34 56

How Has This Been Tested?

Run dash-qt, send a tx

Breaking Changes

n/a

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@UdjinM6 UdjinM6 added this to the 23.1 milestone Nov 27, 2025
@github-actions
Copy link

github-actions bot commented Nov 27, 2025

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

Please coordinate with the authors of these PRs to avoid merge conflicts.

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Walkthrough

The PR changes duplicate-address handling during send operations. In walletmodel.cpp the duplicate-address check is moved to occur after fee computation/absurd-fee validation rather than before balance/fee checks. In sendcoinsdialog.cpp, when a DuplicateAddress prepare status is returned the UI now shows a confirmation dialog and only aborts the send if the user declines; other non-OK prepare statuses still abort immediately.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Review walletmodel.cpp: confirm reordering of duplicate-address check doesn't alter error precedence or expose fee/balance checks to unexpected inputs.
  • Review sendcoinsdialog.cpp: verify the confirmation dialog blocks and the abort/continue paths are correct and thread-safe.
  • Ensure processSendCoinsReturn change (removing user-facing DuplicateAddress message) aligns with UX requirements and that compiler-silencing placement is correct.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(qt): warn when sending to duplicate recipients' clearly and concisely describes the main change: converting duplicate recipient handling from an error to a warning in the Qt GUI.
Description check ✅ Passed The description is related to the changeset, explaining the rationale (protocol allows duplicates), what was done (warning dialog instead of error), and how it was tested (run dash-qt and send tx).
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f2a3fc and e14aaa0.

📒 Files selected for processing (2)
  • src/qt/sendcoinsdialog.cpp (2 hunks)
  • src/qt/walletmodel.cpp (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/qt/sendcoinsdialog.cpp
  • src/qt/walletmodel.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: linux64_multiprocess-build / Build source
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: x86_64-apple-darwin / Build depends
  • GitHub Check: x86_64-w64-mingw32 / Build depends
  • GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
  • GitHub Check: arm-linux-gnueabihf / Build depends
  • GitHub Check: x86_64-pc-linux-gnu / Build depends

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@PastaPastaPasta
Copy link
Member

Handle DuplicateAddress as a warning status instead of an error.
Show a confirmation dialog when duplicates are detected, allowing
the transaction to proceed if the user confirms.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link

@nmarley nmarley left a comment

Choose a reason for hiding this comment

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

LGTM 👍. I like it better than #7012

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK e14aaa0

@UdjinM6 UdjinM6 requested a review from knst December 2, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants