qt: Add dust attack protection feature #7095
Conversation
Add optional dust attack protection that: - Detects external receives with small amounts (≤ threshold) - Auto-locks qualifying UTXOs to prevent accidental spending - Displays as "Dust Receive" type in transaction history - Shows only receiving address (hides sender info) - Excludes from overview recent transactions - Adds dedicated filter in transaction view - Right-click context menu to unlock dust UTXOs - GUI-configurable threshold (default: 10,000 duffs) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughAdds a dust-protection UI and wiring in the Qt wallet UI and model: new options controls (enable + threshold) are stored in OptionsModel. Transaction decomposition and listing are updated to classify small external incoming outputs as DustReceive when enabled. TransactionTableModel, TransactionView, WalletModel, and Wallet interactions support automatic locking of detected dust UTXOs, a context action to unlock them, and refreshes when the dust-protection setting changes. Sequence DiagramsequenceDiagram
participant User
participant GUI as OptionsDialog/TransactionView
participant OM as OptionsModel
participant TTM as TransactionTableModel
participant TR as TransactionRecord
participant WM as WalletModel
participant Wallet
Note over User,GUI: Configure dust protection
User->>GUI: Enable dust protection & set threshold
GUI->>OM: setOption(DustProtection, threshold)
OM->>OM: store fDustProtection / nDustProtectionThreshold
OM->>TTM: emit dustProtectionChanged
Note over Wallet,WM: New transaction seen
Wallet->>WM: NotifyTransactionChanged(CT_NEW, txhash)
WM->>WM: checkAndLockDustOutputs(txhash)
WM->>Wallet: lockCoin(outpoint) for dust outputs
Note over TTM,TR: Display/update transactions
TTM->>OM: getDustProtection() / getDustProtectionThreshold()
TTM->>TR: decomposeTransaction(wtx, dustProtection, dustThreshold)
TR->>TR: mark small external receipts as DustReceive if ≤ threshold
TR-->>TTM: return records
TTM->>GUI: refresh view (filters include/exclude DustReceive)
Note over User,GUI: Manual unlock flow
User->>GUI: Select "Unlock dust UTXO"
GUI->>WM: wallet().unlockCoin(outpoint)
WM->>Wallet: unlockCoin(outpoint)
GUI->>TTM: refresh view
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/qt/forms/optionsdialog.ui (1)
370-422: Consider increasing the maximum dust threshold.The maximum value is currently set to 1,000,000 duffs (0.01 DASH). Depending on the use case, users might want to set higher thresholds to protect against more sophisticated dust attacks. Consider whether this maximum is sufficient or if it should be increased (e.g., to 10,000,000 duffs or 0.1 DASH).
src/qt/optionsmodel.cpp (2)
360-368: Consider defining a constant for the default dust threshold.The default threshold value
10000is hardcoded in both the settings creation and the fallback value (lines 367 and 368). Consider defining a named constant to improve maintainability.♻️ Suggested refactor
In
src/qt/optionsmodel.h, add near other constants:+static constexpr qint64 DEFAULT_DUST_PROTECTION_THRESHOLD = 10000;Then in
src/qt/optionsmodel.cpp:// Dust protection if (!settings.contains("fDustProtection")) settings.setValue("fDustProtection", false); fDustProtection = settings.value("fDustProtection", false).toBool(); if (!settings.contains("nDustProtectionThreshold")) - settings.setValue("nDustProtectionThreshold", (qlonglong)10000); - nDustProtectionThreshold = settings.value("nDustProtectionThreshold", (qlonglong)10000).toLongLong(); + settings.setValue("nDustProtectionThreshold", (qlonglong)DEFAULT_DUST_PROTECTION_THRESHOLD); + nDustProtectionThreshold = settings.value("nDustProtectionThreshold", (qlonglong)DEFAULT_DUST_PROTECTION_THRESHOLD).toLongLong();
1008-1012: Consider adding validation for the dust protection threshold.The threshold value is accepted without validation. Consider adding bounds checking to prevent negative values or unreasonably large values that could cause unexpected behavior.
♻️ Suggested validation
case DustProtectionThreshold: + // Validate threshold is positive and reasonable + if (value.toLongLong() <= 0 || value.toLongLong() > MAX_MONEY) { + return false; + } nDustProtectionThreshold = value.toLongLong(); settings.setValue("nDustProtectionThreshold", qlonglong(nDustProtectionThreshold)); Q_EMIT dustProtectionChanged(); break;Note: You'll need to include the appropriate header for
MAX_MONEY(likely<amount.h>).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/qt/forms/optionsdialog.uisrc/qt/optionsdialog.cppsrc/qt/optionsmodel.cppsrc/qt/optionsmodel.hsrc/qt/overviewpage.cppsrc/qt/transactionrecord.cppsrc/qt/transactionrecord.hsrc/qt/transactiontablemodel.cppsrc/qt/transactiontablemodel.hsrc/qt/transactionview.cppsrc/qt/transactionview.hsrc/qt/walletmodel.cppsrc/qt/walletmodel.h
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,h,hpp,cc}
📄 CodeRabbit inference engine (CLAUDE.md)
Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Files:
src/qt/transactiontablemodel.hsrc/qt/optionsdialog.cppsrc/qt/optionsmodel.hsrc/qt/transactionview.hsrc/qt/walletmodel.hsrc/qt/overviewpage.cppsrc/qt/transactiontablemodel.cppsrc/qt/transactionrecord.cppsrc/qt/walletmodel.cppsrc/qt/optionsmodel.cppsrc/qt/transactionview.cppsrc/qt/transactionrecord.h
src/qt/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
GUI implementation in src/qt/ must use Qt 5
Files:
src/qt/transactiontablemodel.hsrc/qt/optionsdialog.cppsrc/qt/optionsmodel.hsrc/qt/transactionview.hsrc/qt/walletmodel.hsrc/qt/overviewpage.cppsrc/qt/transactiontablemodel.cppsrc/qt/transactionrecord.cppsrc/qt/walletmodel.cppsrc/qt/optionsmodel.cppsrc/qt/transactionview.cppsrc/qt/transactionrecord.h
🧠 Learnings (8)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.
Applied to files:
src/qt/transactiontablemodel.hsrc/qt/optionsmodel.hsrc/qt/transactionview.hsrc/qt/walletmodel.hsrc/qt/transactionrecord.h
📚 Learning: 2025-12-17T13:58:19.813Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:19.813Z
Learning: In Dash Qt wallet code, prefer using existing Qt signals/slots (e.g., ShowProgress signals) for long-running operations instead of adding explicit QProgressDialog dialogs. This minimizes upstream deviations and simplifies backports. Apply this pattern to wallet-related Qt components (e.g., operations like rescanning) and rely on signal-based progress updates rather than bespoke dialogs.
Applied to files:
src/qt/optionsdialog.cppsrc/qt/overviewpage.cppsrc/qt/transactiontablemodel.cppsrc/qt/transactionrecord.cppsrc/qt/walletmodel.cppsrc/qt/optionsmodel.cppsrc/qt/transactionview.cpp
📚 Learning: 2025-12-17T13:58:26.891Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.
Applied to files:
src/qt/transactionview.hsrc/qt/forms/optionsdialog.ui
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/coinjoin/**/*.{cpp,h} : CoinJoin implementation must use masternode-coordinated mixing sessions with uniform denomination outputs
Applied to files:
src/qt/transactionview.hsrc/qt/transactionrecord.cppsrc/qt/transactionrecord.h
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/qt/**/*.{cpp,h} : GUI implementation in src/qt/ must use Qt 5
Applied to files:
src/qt/overviewpage.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/wallet/**/*.{cpp,h} : Wallet implementation must use Berkeley DB and SQLite
Applied to files:
src/qt/overviewpage.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/**/*.{cpp,h} : Special transactions use payload serialization routines defined in src/evo/specialtx.h and must include appropriate special transaction types (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx)
Applied to files:
src/qt/overviewpage.cpp
🧬 Code graph analysis (7)
src/qt/optionsmodel.h (1)
src/qt/optionsmodel.cpp (2)
dustProtectionChanged(1006-1006)dustProtectionChanged(1011-1011)
src/qt/transactionview.h (1)
src/qt/transactionview.cpp (2)
unlockDust(465-489)unlockDust(465-465)
src/qt/walletmodel.h (1)
src/qt/walletmodel.cpp (2)
checkAndLockDustOutputs(152-200)checkAndLockDustOutputs(152-152)
src/qt/transactiontablemodel.cpp (2)
src/qt/transactionrecord.cpp (2)
decomposeTransaction(32-283)decomposeTransaction(32-33)src/qt/optionsmodel.cpp (2)
dustProtectionChanged(1006-1006)dustProtectionChanged(1011-1011)
src/qt/walletmodel.cpp (3)
src/qt/transactionview.cpp (1)
outpoint(484-484)src/wallet/wallet.cpp (1)
outpoint(1093-1093)src/wallet/spend.cpp (1)
outpoint(150-150)
src/qt/transactionview.cpp (3)
src/qt/transactionview.h (1)
unlockDustAction(80-127)src/qt/transactiontablemodel.cpp (1)
hash(104-104)src/qt/walletmodel.cpp (1)
outpoint(196-196)
src/qt/transactionrecord.h (1)
src/qt/transactionrecord.cpp (2)
decomposeTransaction(32-283)decomposeTransaction(32-33)
🪛 Cppcheck (2.19.0)
src/qt/transactionview.cpp
[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
⏰ 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). (10)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (25)
src/qt/transactionview.h (1)
80-80: LGTM! New dust unlock action and slot added.The new
unlockDustActionpointer andunlockDust()slot are properly declared for handling manual unlock of dust UTXOs via context menu.Also applies to: 106-106
src/qt/transactiontablemodel.h (1)
78-79: LGTM! New role properly added for UTXO identification.The
OutputIndexRolefollows the existing pattern and is correctly documented.src/qt/optionsdialog.cpp (2)
76-78: LGTM! Follows established UI pattern.The dust protection threshold initialization and connection follow the same pattern as the pruning feature (lines 73-74), which is consistent and correct.
351-352: Mapper entries are correct and wallet refresh is properly triggered.The
OptionsModel::dustProtectionChanged()signal is connected intransactiontablemodel.cppto callrefreshWallet(true), so dust protection changes will correctly refresh the wallet. The signal-based mechanism is already in place and functioning as intended.src/qt/overviewpage.cpp (2)
16-16: LGTM! Include added for TransactionRecord::DustReceive.The include is necessary for the new dust receive filtering logic below.
763-764: The TYPE() macro correctly handles enum-to-bitflag conversion for the bitwise exclusion operation.The
TYPE()macro (defined asstatic quint32 TYPE(int type) { return 1<<type; }intransactionfilterproxy.h) properly converts enum values to bit flags using left-bit-shift. The code correctly excludesDustReceivetransactions by inverting the flag and applying bitwise AND withALL_TYPES.src/qt/walletmodel.h (1)
244-245: No bounds safety issue — txout_is_mine size is guaranteed to match vout size by design.The vector
txout_is_mineis populated insrc/wallet/interfaces.cpp(lines 78–82) by reserving towtx.tx->vout.size()and then iterating through each output, adding one element per output. The loop inwalletmodel.cppcorrectly iterates throughwtx.tx->voutand accesses the corresponding index, which is safe because the vectors are guaranteed to have matching sizes. This pattern is used consistently throughout the codebase (e.g.,transactiondesc.cpp).Likely an incorrect or invalid review comment.
src/qt/transactionview.cpp (5)
22-22: LGTM!The include is necessary for
COutPointused in theunlockDust()method at line 484.
99-99: LGTM!The "Dust Receive" filter correctly corresponds to the new
TransactionRecord::DustReceivetype introduced in this PR.
167-167: LGTM!The context menu action is properly created and connected to the
unlockDust()slot.
425-427: LGTM!Correctly shows the unlock action only for
DustReceivetransactions, maintaining a clean context menu for other transaction types.
465-489: No issues identified with the implementation.OutputIndexRoleis properly defined inTransactionTableModel::RoleIndexenum at line 79 of transactiontablemodel.h with the documentation "Output index within transaction (for UTXO identification)". The method correctly retrieves and uses this role, and the UI refresh pattern viarefreshWallet(true)is consistent with how lock/unlock operations work elsewhere in the codebase (e.g., CoinControlDialog), where visual updates are provided rather than explicit user feedback messages.src/qt/transactionrecord.h (2)
89-89: LGTM!The new
DustReceivetransaction type is properly added to the enum and will be used to classify external small-value receives when dust protection is enabled.
120-121: LGTM - Backward compatible API extension.The signature change adds optional parameters with sensible defaults (
dustProtectionEnabled = false,dustThreshold = 0), maintaining full backward compatibility with existing callers.src/qt/walletmodel.cpp (2)
152-200: LGTM - Well-structured dust protection logic.The implementation correctly:
- Guards against disabled protection or invalid thresholds
- Identifies external receives by checking
isFromMe- Only locks outputs that belong to the wallet and are at or below the threshold
- Persists locks with
write_to_db = trueThe defensive checks ensure the feature only activates when properly configured.
512-518: LGTM - Thread-safe async invocation.The integration with
NotifyTransactionChangedcorrectly:
- Triggers only for new transactions (
CT_NEW)- Uses
Qt::QueuedConnectionfor thread safety- Passes the transaction hash as a QString parameter
This pattern ensures dust output locking happens asynchronously without blocking the notification thread.
src/qt/transactionrecord.cpp (3)
32-33: LGTM - Backward compatible signature update.The function signature correctly adds optional parameters with defaults, maintaining compatibility with existing callers while enabling dust protection functionality.
44-51: LGTM - Correct wallet ownership detection.The
isFromMecheck correctly identifies transactions originating from this wallet by examining whether any inputs belong to the wallet. This is essential for distinguishing external dust receives from internal transactions.
94-101: LGTM - Comprehensive dust detection logic.The dust classification correctly:
- Only applies when dust protection is enabled
- Excludes transactions from this wallet (
!isFromMe)- Excludes special transaction types (coinbase, platform transfers)
- Validates the amount is positive and within the threshold
- Only overrides standard receive types (
RecvWithAddress,RecvFromOther)This ensures dust protection targets the right transactions without interfering with normal wallet operations.
src/qt/optionsmodel.h (1)
98-99: LGTM! Dust protection options properly integrated.The dust protection feature is correctly integrated into the OptionsModel:
- Enum values added following existing pattern
- Getters follow naming conventions
- Private members use appropriate types (bool, qint64)
- Signal follows existing signal patterns
Also applies to: 135-136, 167-168, 192-192
src/qt/optionsmodel.cpp (1)
717-720: Implementation follows existing patterns correctly.The getOption and setOption implementations for dust protection settings are consistent with other Qt-only settings in the codebase. Signal emission properly triggers UI refresh.
Also applies to: 1003-1012
src/qt/transactiontablemodel.cpp (4)
116-117: Dust protection settings correctly propagated to transaction decomposition.The dust protection settings are properly retrieved from OptionsModel and passed to
decomposeTransactionin both refresh and update paths, ensuring consistent dust transaction classification.Also applies to: 120-120, 179-180, 182-182
285-286: Force refresh on dust protection change may impact performance with large transaction histories.The lambda calls
refreshWallet(true), which reprocesses all transactions. This is necessary to reclassify transactions based on new dust protection settings, but could be slow for wallets with thousands of transactions.Consider adding a progress indicator or deferring the refresh if this becomes a UX issue during testing.
The performance impact should be acceptable for most users, but it's worth monitoring during testing with wallets containing large transaction histories.
438-439: DustReceive transaction type consistently integrated into UI formatting.The
DustReceivetype is properly added to all relevant formatting functions:
formatTxType: Returns user-friendly "Dust Receive" labelformatTxToAddress: Shows address with label (consistent with other receive types)addressColor: Uses BAREADDRESS color (consistent with other special transaction types)amountColor: Uses ORANGE color (consistent with warning/informational transaction types like CoinJoin operations)Also applies to: 484-484, 511-511, 563-563
751-752: OutputIndexRole added for transaction output indexing.The new role exposes
rec->idx, which appears to be needed for identifying specific outputs when manually unlocking dust UTXOs via the context menu.
src/qt/walletmodel.cpp
Outdated
| // Check if any inputs belong to this wallet (isFromMe check) | ||
| bool isFromMe = false; | ||
| for (const auto& mine : wtx.txin_is_mine) { | ||
| if (mine) { | ||
| isFromMe = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // If this transaction has our inputs, it's not external dust | ||
| if (isFromMe) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
can simplify this part a bit:
| // Check if any inputs belong to this wallet (isFromMe check) | |
| bool isFromMe = false; | |
| for (const auto& mine : wtx.txin_is_mine) { | |
| if (mine) { | |
| isFromMe = true; | |
| break; | |
| } | |
| } | |
| // If this transaction has our inputs, it's not external dust | |
| if (isFromMe) { | |
| return; | |
| } | |
| // If this transaction has our inputs, it's not external dust | |
| for (const auto& mine : wtx.txin_is_mine) { | |
| if (mine) { | |
| return; | |
| } | |
| } |
or maybe even use std::anyof(...): if (std::anyof(...) return;
| // Check for dust attack: external receive with small amount | ||
| // Only override if not already a special type (coinbase, platform transfer) | ||
| if (dustProtectionEnabled && !isFromMe && !wtx.is_coinbase && !wtx.is_platform_transfer && | ||
| sub.credit > 0 && sub.credit <= dustThreshold && |
There was a problem hiding this comment.
nit: consider refactoring: move sub.credit > 0 && sub.credit <= dustThreshold && and !isFromMe to the helper and use it here in TransactionRecord::decomposeTransaction and in checkAndLockDustOutputs
| DustProtection, // bool | ||
| DustProtectionThreshold, // CAmount (in duffs) |
There was a problem hiding this comment.
nit: consider DustProtection to be CAmount;
| DustProtection, // bool | |
| DustProtectionThreshold, // CAmount (in duffs) | |
| DustProtection, // CAmount (in duffs, disabled if equals to 0) |
- Define DEFAULT_DUST_PROTECTION_THRESHOLD constant (10000 duffs) - Add default initializers to fDustProtection and nDustProtectionThreshold - Use constant in optionsmodel.cpp instead of magic numbers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Check that TxHashRole and OutputIndexRole return valid QVariant values before using them to prevent undefined behavior when data is unavailable. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use getTx() instead of getWalletTx() for lighter wallet access - Skip coinbase and special transactions (not dust attacks) - Use txinIsMine()/txoutIsMine() with early exit on first match - Check threshold before ownership (cheaper check first) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add new method to scan and lock existing dust UTXOs: - Called on wallet startup if dust protection is enabled - Called when dust protection settings change - Uses listCoins() to iterate UTXOs (more efficient than all txs) - Skips coinbase, special transactions, and self-sends This ensures dust received while wallet was closed or existing when the feature is enabled gets properly locked. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Use separate label for unit instead of spinbox suffix - Set unit text dynamically (duffs/tduffs based on network) - Move dust protection to last position in wallet options 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
pls consider commits from https://github.com/UdjinM6/dash/commits/pr-7095-review/ |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/qt/forms/optionsdialog.ui:
- Around line 446-453: Remove the hardcoded "duffs" string from the
optionsdialog.ui widget (dustProtectionThresholdUnitLabel) and instead set that
label at runtime in src/qt/optionsdialog.cpp using the network-aware unit name;
replace the current call that uses BitcoinUnits::name(BitcoinUnits::Unit::duffs)
with the network-aware variant (use the BitcoinUnits::name overload that derives
the unit name from the active network or obtain the unit string from chain
params), and assign it to dustProtectionThresholdUnitLabel->setText(...) in the
OptionsDialog initialization so the label shows "duffs" or "tduffs" depending on
the active network.
🧹 Nitpick comments (1)
src/qt/optionsmodel.cpp (1)
1003-1012: Consider adding threshold value validation.The implementation correctly stores the dust protection settings and emits the
dustProtectionChanged()signal. However, consider adding validation fornDustProtectionThresholdto ensure it's within reasonable bounds (e.g., > 0 and <= some maximum), even though the UI spinbox enforces these limits. This would provide defense-in-depth if settings are modified outside the UI.♻️ Optional: Add threshold validation
case DustProtectionThreshold: + if (value.toLongLong() <= 0) { + return false; + } nDustProtectionThreshold = value.toLongLong(); settings.setValue("nDustProtectionThreshold", qlonglong(nDustProtectionThreshold)); Q_EMIT dustProtectionChanged(); break;
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/qt/forms/optionsdialog.uisrc/qt/optionsdialog.cppsrc/qt/optionsmodel.cppsrc/qt/optionsmodel.hsrc/qt/transactionview.cppsrc/qt/walletmodel.cppsrc/qt/walletmodel.h
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{cpp,h,hpp,cc}
📄 CodeRabbit inference engine (CLAUDE.md)
Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1
Files:
src/qt/walletmodel.hsrc/qt/optionsdialog.cppsrc/qt/walletmodel.cppsrc/qt/optionsmodel.hsrc/qt/optionsmodel.cppsrc/qt/transactionview.cpp
src/qt/**/*.{cpp,h}
📄 CodeRabbit inference engine (CLAUDE.md)
GUI implementation in src/qt/ must use Qt 5
Files:
src/qt/walletmodel.hsrc/qt/optionsdialog.cppsrc/qt/walletmodel.cppsrc/qt/optionsmodel.hsrc/qt/optionsmodel.cppsrc/qt/transactionview.cpp
🧠 Learnings (8)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.
Learnt from: PastaPastaPasta
Repo: dashpay/dash PR: 6804
File: src/qt/proposalwizard.cpp:40-42
Timestamp: 2025-08-11T17:16:36.654Z
Learning: In the Dash repository, when a PR adds new files that are not from Bitcoin backports, these files must be added to the list in test/util/data/non-backported.txt. This applies to newly created files like qt/proposalwizard.{h,cpp} and forms/proposalwizard.ui. Limited exemptions may exist for subtrees and similar cases.
📚 Learning: 2025-12-17T13:58:26.891Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:26.891Z
Learning: In Dash Qt wallet code, when leveraging upstream Bitcoin Core wallet capabilities (especially for operations like rescanning), prefer using the inherited signal-based mechanisms (e.g., ShowProgress signals) over adding explicit Qt progress dialogs to minimize upstream deviation and simplify future backports.
Applied to files:
src/qt/walletmodel.hsrc/qt/forms/optionsdialog.ui
📚 Learning: 2025-12-22T15:42:48.595Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:48.595Z
Learning: In C++/Qt codebases, use fail-fast asserts in setters to enforce invariants (e.g., ensuring internal maps contain necessary keys). Prefer assert() for programmer errors that should be caught in development and debugging, rather than defensive runtime checks with fallbacks. This helps catch invariant violations early during development. Apply to header and source files where invariant-driven state mutations occur, especially in setters like SetWeightBold/SetWeightNormal that assume established relationships or preconditions.
Applied to files:
src/qt/walletmodel.hsrc/qt/optionsmodel.h
📚 Learning: 2025-12-17T13:58:19.813Z
Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:19.813Z
Learning: In Dash Qt wallet code, prefer using existing Qt signals/slots (e.g., ShowProgress signals) for long-running operations instead of adding explicit QProgressDialog dialogs. This minimizes upstream deviations and simplifies backports. Apply this pattern to wallet-related Qt components (e.g., operations like rescanning) and rely on signal-based progress updates rather than bespoke dialogs.
Applied to files:
src/qt/optionsdialog.cppsrc/qt/walletmodel.cppsrc/qt/optionsmodel.cppsrc/qt/transactionview.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,txmempool}/**/*.{cpp,h} : Block validation and mempool handling must use extensions to Bitcoin Core mechanisms for special transaction validation and enhanced transaction relay
Applied to files:
src/qt/walletmodel.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/evo/**/*.{cpp,h} : Special transactions use payload serialization routines defined in src/evo/specialtx.h and must include appropriate special transaction types (ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx)
Applied to files:
src/qt/walletmodel.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{masternode,llmq}/**/*.{cpp,h} : BLS integration must be used for cryptographic foundation of advanced masternode features
Applied to files:
src/qt/walletmodel.cpp
📚 Learning: 2025-11-24T16:41:22.457Z
Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/{validation,consensus,net_processing}/**/*.{cpp,h} : ValidationInterface callbacks must be used for event-driven architecture to coordinate subsystems during block/transaction processing
Applied to files:
src/qt/walletmodel.cpp
🧬 Code graph analysis (4)
src/qt/walletmodel.h (1)
src/qt/walletmodel.cpp (4)
checkAndLockDustOutputs(160-203)checkAndLockDustOutputs(160-160)lockExistingDustOutputs(205-246)lockExistingDustOutputs(205-205)
src/qt/optionsmodel.h (1)
src/qt/optionsmodel.cpp (2)
dustProtectionChanged(1006-1006)dustProtectionChanged(1011-1011)
src/qt/optionsmodel.cpp (1)
src/qt/optionsmodel.h (1)
nDustProtectionThreshold(171-196)
src/qt/transactionview.cpp (1)
src/qt/transactionview.h (1)
unlockDustAction(80-127)
🪛 Cppcheck (2.19.0)
src/qt/transactionview.cpp
[error] 22-22: failed to evaluate #if condition, undefined function-like macro invocation
(syntaxError)
⏰ 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). (10)
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (21)
src/qt/walletmodel.h (1)
244-247: LGTM! Clean slot declarations for dust protection.The two new public slots are well-positioned alongside existing update-related slots and integrate cleanly with the dust protection workflow implemented in walletmodel.cpp.
src/qt/optionsdialog.cpp (2)
76-79: LGTM! Clean UI initialization for dust protection.The code properly disables the threshold input by default and uses
BitcoinUnits::name()for network-aware unit labeling (duffs/tduffs). The checkbox-controlled input field pattern is standard Qt practice.
352-353: LGTM! Standard mapper integration.The mapper bindings follow the established pattern in this file and correctly propagate the UI elements to the OptionsModel.
src/qt/transactionview.cpp (5)
22-22: LGTM! Necessary include for COutPoint.The include is required for the COutPoint type used in the unlockDust implementation. The static analysis error about macro evaluation is a false positive related to build system configuration.
99-99: LGTM! Dust filter addition is consistent.The "Dust Receive" filter follows the established pattern for transaction type filters and is positioned appropriately in the dropdown.
167-167: LGTM! Context menu action is well-formed.The unlock dust action follows the established pattern for context menu actions and includes an appropriate keyboard mnemonic.
425-427: LGTM! Proper conditional visibility for unlock dust action.The action is correctly shown only for DustReceive transactions, preventing confusion when right-clicking on other transaction types.
465-496: Excellent validation, but consider error feedback.The thorough QVariant validation prevents undefined behavior and follows the improvements mentioned in the commit messages. The early-exit pattern keeps the code clean.
One consideration: the
unlockCoin()call at Line 492 doesn't check for success/failure. If unlocking fails for any reason, the user receives no feedback. Consider whether error handling or status feedback would improve the user experience.src/qt/walletmodel.cpp (5)
26-26: LGTM! Necessary include for dust protection types.The include provides COutPoint and CTransactionRef types required by the new dust protection functions.
68-74: Startup dust locking is correctly wired.The signal connection enables dynamic response to settings changes, and the startup call to
lockExistingDustOutputs()implements the intended behavior. The function has early exits when dust protection is disabled (lines 207-208), so the performance impact should be minimal when the feature is off.For large wallets with many UTXOs and dust protection enabled, this synchronous call in the constructor could delay wallet loading. However, the optimization to use
listCoins()(mentioned in PR objectives) should keep this reasonably fast. Consider monitoring startup performance with the feature enabled if users report delays.Based on learnings, the pattern of performing work synchronously during initialization is acceptable in Qt wallet code when the work is bounded and necessary for correct state.
160-203: Excellent implementation with smart optimizations.The function demonstrates several performance optimizations mentioned in the commit messages:
- Uses
getTx()instead of the heaviergetWalletTx()(Line 176)- Checks
txinIsMine()with early exit on first match (Lines 188-192)- Validates threshold before checking ownership (Line 197) — cheap check before expensive
- Skips coinbase and special transactions that can't be dust attacks (Lines 182-183)
The
write_to_db=trueparameter ensures locked status persists across restarts, which is correct for this feature.
205-246: Well-optimized UTXO iteration for dust locking.The function efficiently scans existing UTXOs with multiple optimization layers:
- Iterates only UTXOs via
listCoins()rather than all transactions (Line 217) — much smaller set- Skips already-locked coins immediately (Line 220)
- Skips above-threshold outputs before loading transaction (Line 223)
- Uses early-continue pattern to minimize expensive operations
The logic correctly identifies external dust by verifying no inputs belong to the wallet (Lines 232-240), preventing self-sent transactions from being locked.
557-564: Proper integration with transaction notification flow.The modification correctly:
- Triggers only for
CT_NEWtransactions to lock newly received dust (Line 559)- Uses
Qt::QueuedConnectionfor thread-safe invocation from core signals (Line 561)- Follows the established pattern used by other notification handlers in this file
This ensures automatic dust locking happens asynchronously when new external receives are detected.
src/qt/optionsmodel.h (5)
27-28: LGTM! Reasonable default threshold for dust protection.The default of 10,000 duffs (0.0001 DASH) is a sensible threshold for identifying dust transactions. The
static constexprdeclaration is appropriate for a compile-time constant, andqint64correctly matches theCAmounttype used throughout the codebase.
101-102: LGTM! Clean enum additions for dust protection options.The new option identifiers follow the established pattern with clear type annotations and are correctly positioned before the
OptionIDRowCountsentinel.
138-139: LGTM! Standard getter declarations.The getters follow the established pattern in this section with appropriate return types and const correctness.
170-171: LGTM! Proper default initialization of dust protection state.The member variables use modern C++ default initialization and follow the Hungarian notation conventions established in this class. Defaulting
fDustProtectiontofalseensures the feature is opt-in, which is appropriate for a new protection mechanism.
195-195: LGTM! Standard signal declaration for dust protection changes.The signal follows the established pattern for settings-change notifications and will be emitted by
setOptionwhen dust protection configuration changes, enabling connected components to respond dynamically.src/qt/forms/optionsdialog.ui (1)
410-419: LGTM!The checkbox widget is properly configured with a clear tooltip and keyboard shortcut.
src/qt/optionsmodel.cpp (2)
717-720: LGTM!The
getOptioncases follow the established pattern and correctly return the dust protection settings with appropriate type conversion.
360-368: DEFAULT_DUST_PROTECTION_THRESHOLD is properly defined.The constant is declared as
static constexpr qint64 DEFAULT_DUST_PROTECTION_THRESHOLD = 10000;insrc/qt/optionsmodel.hat line 28, and the type is appropriate for theqlonglongcasts used in the code.
| <widget class="QLabel" name="dustProtectionThresholdUnitLabel"> | ||
| <property name="text"> | ||
| <string>duffs</string> | ||
| </property> | ||
| <property name="textFormat"> | ||
| <enum>Qt::PlainText</enum> | ||
| </property> | ||
| </widget> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -nC3 'dustProtectionThresholdUnitLabel' --type=cppRepository: dashpay/dash
Length of output: 551
Fix hardcoded unit label to be network-aware.
The unit label is hardcoded to "duffs" in both the UI file (line 446-453) and at runtime in src/qt/optionsdialog.cpp:78 via BitcoinUnits::name(BitcoinUnits::Unit::duffs). This conflicts with the PR objective to use a network-aware label (duffs/tduffs). On testnet, the label should display "tduffs" instead of "duffs".
🤖 Prompt for AI Agents
In @src/qt/forms/optionsdialog.ui around lines 446 - 453, Remove the hardcoded
"duffs" string from the optionsdialog.ui widget
(dustProtectionThresholdUnitLabel) and instead set that label at runtime in
src/qt/optionsdialog.cpp using the network-aware unit name; replace the current
call that uses BitcoinUnits::name(BitcoinUnits::Unit::duffs) with the
network-aware variant (use the BitcoinUnits::name overload that derives the unit
name from the active network or obtain the unit string from chain params), and
assign it to dustProtectionThresholdUnitLabel->setText(...) in the OptionsDialog
initialization so the label shows "duffs" or "tduffs" depending on the active
network.
3 participants
Summary
Test plan
Note: This is a minimally tested implementation. The historical transaction detection has been validated, but the auto-locking component has not yet been tested.
🤖 Generated with Claude Code