fix(qt): stable sort for equal items in Coin Control dialog#7156
Conversation
CCoinControlWidgetItem::operator< had no tiebreaker for equal numeric values (amount, date, confirmations, CoinJoin rounds), causing Qt's unstable sort to produce a different ordering on each updateView() call whenever coins with identical values were present. Add a secondary sort by outpoint (txid:vout), which is unique and stable across view rebuilds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughThe change modifies the sorting logic in the coin control dialog to ensure deterministic ordering of items. When primary sort keys (amount, date, confirmations, coinjoin rounds) are equal, the new logic introduces a stable tiebreaker by comparing items based on transaction ID and output index (txid:vout), then by output number (vout). This ensures consistent and reproducible sorting results across identical data. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/qt/coincontroldialog.cpp (1)
46-51: Optional: tiebreaker is inert for tree-mode parent nodes.In tree mode, parent (
itemWalletAddress) nodes haveCOLUMN_AMOUNTUserRole data (line 792) but never receiveTxHashRole/VOutRole(those are set only on child items at lines 764-767). When two parent nodes share the same total amount, the tiebreaker compares empty strings and then0 < 0, always returningfalse— so ordering among same-sum parents is still nondeterministic.This is a minor edge case (parents represent distinct addresses, so collisions are rare) and doesn't undermine the primary fix. If you ever want full stability, you could fall back to the wallet-address string for parent nodes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/qt/coincontroldialog.cpp` around lines 46 - 51, The current tiebreaker in coincontroldialog.cpp compares TxHashRole and VOutRole which are only set on child items, so parent nodes (itemWalletAddress) with equal amounts compare empty/zero and remain non-deterministic; update the comparison in the relevant compare/operator (the block using data(..., TxHashRole) and VOutRole) to detect missing/empty TxHashRole (or missing VOutRole) and, for those cases (i.e., parent nodes), fall back to comparing the address string from COLUMN_ADDRESS (or another stable identifier like the item text) to produce a deterministic ordering for parent nodes while keeping the existing child-item outpoint tiebreaker.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/qt/coincontroldialog.cpp`:
- Around line 46-51: The current tiebreaker in coincontroldialog.cpp compares
TxHashRole and VOutRole which are only set on child items, so parent nodes
(itemWalletAddress) with equal amounts compare empty/zero and remain
non-deterministic; update the comparison in the relevant compare/operator (the
block using data(..., TxHashRole) and VOutRole) to detect missing/empty
TxHashRole (or missing VOutRole) and, for those cases (i.e., parent nodes), fall
back to comparing the address string from COLUMN_ADDRESS (or another stable
identifier like the item text) to produce a deterministic ordering for parent
nodes while keeping the existing child-item outpoint tiebreaker.
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
Issue being fixed or feature implemented
CCoinControlWidgetItem::operator<had no tiebreaker for equal numeric values (amount, date, confirmations, CoinJoin rounds), causing Qt's unstable sort to produce a different ordering on each updateView() call whenever coins with identical values were present.Noticed while testing #7155 - clicking "(un)lock all" button shifted rows with the same amount when list was sorted by amounts.
What was done?
Add a secondary sort by outpoint (txid:vout), which is unique and stable across view rebuilds.
How Has This Been Tested?
Build it on top of #7155, run and click "(un)lock all" button in Coin Control dialog - should be stable now.
Breaking Changes
n/a
Checklist: