fix(qt): drop zero fraction from collateral amounts in masternode wizard - #7640
fix(qt): drop zero fraction from collateral amounts in masternode wizard#7640thephez wants to merge 1 commit into
Conversation
Collateral amounts are whole coins in every display unit, so the padded fraction ("1 000.00000000 tDASH") adds noise without information. Format them without the decimal part when the amount is a whole number of the display unit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
✅ Final review complete — no blockers (commit c6c3e28) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe masternode wizard adds Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change only adjusts how whole-number collateral amounts are displayed in the masternode wizard; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Should have Ss's :D but ok |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The formatter correctly trims only zero fractional portions for units with decimals, preserves separators and unit suffixes, and leaves duffs unchanged. The behavior is the PR's primary change but has no automated regression coverage despite existing Qt wizard tests that can exercise each display unit.
Source: codex-general (reviewer backend identifier; exact model ID was not supplied in the evidence) and grok-4.5 (final verifier backend). Orchestration-only, not reviewer evidence: openclaw-agent/cliproxy/gpt-5.6-sol.
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
🟡 1 suggestion(s)
🤖 Prompt for all review comments 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/qt/masternodewizard.cpp`:
- [SUGGESTION] src/qt/masternodewizard.cpp:113-117: Add automated coverage for collateral formatting across display units
No automated test asserts the new trimming behavior. Extend the existing `MasternodeWidgetTests` coverage to construct the wizard with DASH, mDASH, uDASH, and duffs selected, then verify the regular and EvoNode collateral labels. This directly protects the unit-dependent `decimals()` and `factor()` logic, including the zero-decimal path, instead of relying solely on a visual check of the default testnet unit.
| QString str{BitcoinUnits::format(unit, amount, /*plussign=*/false, BitcoinUnits::SeparatorStyle::ALWAYS)}; | ||
| const int decimals{BitcoinUnits::decimals(unit)}; | ||
| if (decimals > 0 && amount % BitcoinUnits::factor(unit) == 0) { | ||
| str.chop(decimals + 1); | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: Add automated coverage for collateral formatting across display units
No automated test asserts the new trimming behavior. Extend the existing MasternodeWidgetTests coverage to construct the wizard with DASH, mDASH, uDASH, and duffs selected, then verify the regular and EvoNode collateral labels. This directly protects the unit-dependent decimals() and factor() logic, including the zero-decimal path, instead of relying solely on a visual check of the default testnet unit.
source: ['codex']
There was a problem hiding this comment.
@PastaPastaPasta do you want tests for this?
|
After: See #7618 for a before screenshot |
3 participants
Issue being fixed or feature implemented
Collateral amounts shown in the Register Masternode wizard are always a whole number of coins (e.g. 1000 DASH for a masternode, 4000 DASH for an EvoNode), so the full-precision fraction padding ("1 000.00000000 tDASH") added noise without conveying any information.
What was done?
Renamed the wizard's local
FormatAmounthelper toFormatCollateralAmountand made it drop the decimal part when the amount is a whole number of the current display unit. The trim is based onBitcoinUnits::decimals()/factor(), so it works correctly for any display unit the user has selected.How Has This Been Tested?
Verified visually by opening the Register Masternode wizard in dash-qt on testnet and checking the collateral amounts on the type-selection page.
Breaking Changes
None. Display-only change scoped to the masternode registration wizard.
Checklist: