feat(ui): group Masternodes and Voting under a Governance menu - #924
Merged
Conversation
Main menu: "Voting" becomes "Governance", a submenu holding Masternodes (moved out of Tools) and Voting. Gating changes deliberately. The old Voting row was `#if DASHPAY` plus `VotingPrefs.votingEnabled`; Governance carries neither, because Masternodes was never DashPay-gated and gating the parent would make it unreachable in the dashwallet scheme. Governance is therefore always present, and its contents adapt: Masternodes always, Voting only under DASHPAY when the preference is on. With voting off, Governance holds Masternodes alone. Masternodes moves rather than being duplicated — one path to the screen. "Masternode Keychain" stays in Tools; that is key material, not governance. The hosting wrapper (UIHostingController + NavigationStack for row drill-downs) and the UIKit push for UsernameVotingViewController move verbatim, so behaviour is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Registrations still on the network stay expanded at the top; retired ones move to a DisclosureGroup at the bottom, collapsed by default with a count on its label. Only `retired` (collateral spent / revoked / expired) moves down. `inactive` (PoSe-banned) and `unknown` (masternode list not yet synced) stay in the top section: a banned node is a problem worth surfacing, and unknown is a transient sync state, so burying either would hide something actionable. Both sections render only when non-empty, so a wallet with no retired nodes sees no stray disclosure row, and the existing empty state is untouched. Row construction is shared between the two sections rather than duplicated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds a Governance submenu, moves Masternodes into it, conditionally exposes username voting, and separates current and retired masternode registrations. ChangesGovernance menu
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MainMenuViewController
participant GovernanceMenuScreen
participant MasternodesScreen
MainMenuViewController->>GovernanceMenuScreen: Present Governance
GovernanceMenuScreen->>MasternodesScreen: Push Masternodes
MasternodesScreen-->>GovernanceMenuScreen: Navigate back
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DashWallet/Sources/UI/Menu/Governance/GovernanceMenuViewModel.swift`:
- Around line 17-28: Add an explicit Combine import alongside Foundation in
GovernanceMenuViewModel.swift so ObservableObject and `@Published` resolve
directly from their defining module.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a1432aa-5267-4e57-a103-352ecd37635b
📒 Files selected for processing (9)
DashWallet.xcodeproj/project.pbxprojDashWallet/Sources/UI/Menu/Governance/GovernanceMenuScreen.swiftDashWallet/Sources/UI/Menu/Governance/GovernanceMenuViewModel.swiftDashWallet/Sources/UI/Menu/Main/MainMenuViewController.swiftDashWallet/Sources/UI/Menu/Main/MainMenuViewModel.swiftDashWallet/Sources/UI/Menu/Tools/MasternodesScreen.swiftDashWallet/Sources/UI/Menu/Tools/ToolsMenuScreen.swiftDashWallet/Sources/UI/Menu/Tools/ToolsMenuViewModel.swiftDashWallet/en.lproj/Localizable.strings
💤 Files with no reviewable changes (2)
- DashWallet/Sources/UI/Menu/Tools/ToolsMenuScreen.swift
- DashWallet/Sources/UI/Menu/Tools/ToolsMenuViewModel.swift
`ObservableObject` and `@Published` are Combine symbols; the file imported only Foundation and relied on transitively picking them up. It compiled either way, but every sibling menu view model (MainMenuViewModel, ToolsMenuViewModel, SettingsMenuViewModel) imports Combine explicitly — this was the outlier. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 7, 2026
QuantumExplorer
added a commit
that referenced
this pull request
Aug 7, 2026
develop does not compile: #923 and #924 collided after both merged green independently. #923 replaced UsernameVotingViewController with the SwiftUI UsernameVotingScreen and updated the showVoting() that lived in MainMenuViewController when that PR was written. #924 had since moved the Voting entry point into GovernanceMenuScreen, which #923 never saw. On the merged tip the live entry point (Governance) still called the deleted controller — a hard build error — while the updated copy in MainMenuViewController was orphaned, since #924 removed its caller. Points Governance at UsernameVotingScreen (exactly what #923 wrote, in the location that is actually reachable) and deletes the dead copy. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two menu changes, one commit each.
1. Governance submenu. The main menu's "Voting" row becomes Governance, a new submenu holding Masternodes (moved out of Tools) and Voting.
2. Retired masternodes collapse. The Masternodes list splits into two sections: registrations still on the network stay expanded at the top, retired ones move to a
DisclosureGroupat the bottom, collapsed by default with a count on its label.Decisions worth reviewing
Governance is not DashPay-gated, unlike the row it replaces. The old Voting row required
#if DASHPAYandVotingPrefs.votingEnabled. Masternodes was never gated at all, so gating the parent would have made it unreachable in thedashwalletscheme — a regression. Governance is therefore always present and its contents adapt: Masternodes always, Voting only under DASHPAY when the preference is on. With voting disabled, Governance holds Masternodes alone.Masternodes moves rather than being duplicated, keeping one path to the screen. "Masternode Keychain" stays in Tools — that is key material, not governance. Say the word if you want a Tools shortcut kept as well.
Only
retiredcollapses.MasternodeStatushas four cases;inactive(PoSe-banned) andunknown(masternode list not yet synced) stay in the top section alongsideactive. A banned node is a problem worth surfacing andunknownis a transient sync state, so burying either would hide something actionable. Trivial to change if you want the top section to be strictlyactive.Notes
UIHostingController+NavigationStack, needed for row drill-downs) and the UIKit push forUsernameVotingViewControllermove verbatim — behaviour unchanged.GovernanceMenuScreen+@MainActor GovernanceMenuViewModel, matching theToolsMenuScreenidiom.Governance);Retiredalready existed.Not included
A PoSe score next to the status ("Active — With Score 5") was requested and is not implementable from our data, so nothing was built. The score (
nPoSePenalty) is not in the DML wire format that an SPV client receives —MasternodeListEntrycarries onlyis_valid, and that boolean is already what drives the existing "Inactive" label.pose_penaltyexists in rust-dashcore solely underrpc-json, i.e. the full-nodeprotxRPC surface. Surfacing a real score would mean querying a Core node, which is a separate architectural decision for a screen that is currently a pure local snapshot.Verification
dashpayscheme builds clean (arm64 simulator) fromdevelopwith no dependency on #922, and was installed to the simulator. Not visually confirmed on-device: the simulator is PIN-gated.Summary by CodeRabbit
New Features
Improvements