feat(qt): UI refresh (3/n, update "Masternode" tab layout, status icons, description dialogs, filtering options)

[kwvg]

Additional Information

• Depends on refactor(qt): UI refresh (1/n, runtime show/hide of "Governance" and "Masternode" tabs, split out proposal model, additional bounds checking, better QTextEdit styling) #7112

v23.0.2 (cdc5a63)	This PR (WIP code)
Build doesn't have have decoration icons
Build cannot filter by node type

Breaking Changes

None expected.

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 (note: N/A)
• I have made corresponding changes to the documentation (note: N/A)
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• feat: tidy-up mnemonic dialog by removing useless code and reducing exposure of sensitive data #7127 - feat: tidy-up mnemonic dialog by removing useless code and reducing exposure of sensitive data

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

[github-actions]

This pull request has conflicts, please rebase.

[kwvg]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

Refactors the masternode list UI from a QTableWidget to a Qt Model-View design. Adds MasternodeModel and MasternodeEntry to represent masternode data, a MasternodeListSortFilterProxyModel to handle type/owned/hidden-banned/text filtering, and updates MasternodeList to populate and reconcile the model on timer-driven updates. UI changes include switching to QTableView, adding a type combo box, owned and hide-banned checkboxes, and filter-by-address actions. New MOC/public header entries and CSS color rules are included.

Sequence Diagram

sequenceDiagram
    participant Timer as Timer
    participant List as MasternodeList
    participant Client as Client Interface
    participant Model as MasternodeModel
    participant Proxy as MasternodeListSortFilterProxyModel
    participant UI as QTableView

    Timer->>List: triggerUpdate()
    List->>Client: request masternode list (MnList)
    Client-->>List: return MnList
    List->>List: build MasternodeEntryList (compute next payments)
    List->>Model: reconcile(entries)
    Model->>Model: diff / update / append / remove entries
    Model->>Proxy: emit dataChanged / layoutChanged
    Proxy->>Proxy: apply filters (Type, Owned, HideBanned, Text)
    Proxy->>UI: update visible rows
    UI->>UI: repaint / refresh display

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.35% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main changes: a UI refresh for the Masternode tab with updated layout, status icons, description dialogs, and filtering options across multiple UI components.
Description check	✅ Passed	The description includes visual comparisons with screenshots, dependency information, breaking changes statement, and a completed checklist, all relevant to the changeset's UI improvements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/qt/masternodelist.cpp (1)

1-6: ⚠️ Potential issue | 🟡 Minor

CI failure: clang-format differences detected — must be resolved before merge.

The pipeline reports formatting violations in this file. Run ./contrib/devtools/clang-format-diff.py -p1 to auto-fix.

src/qt/masternodelist.h (1)

1-1: ⚠️ Potential issue | 🟡 Minor

Fix clang-format differences flagged by CI.

The pipeline reports clang-format violations in this file. Run ./contrib/devtools/clang-format-diff.py -p1 to resolve them before merging.

🧹 Nitpick comments (7)

src/qt/forms/masternodelist.ui (1)

42-61: Combo box item order is implicitly coupled to the TypeFilter enum.

The items (All=0, Regular=1, Evo=2) are cast directly to the enum in on_comboBoxType_currentIndexChanged. This works because the handler validates the range, but adding or reordering items here without updating the enum would silently break filtering. A brief comment in the .ui or the handler noting the coupling would help maintainability.

src/qt/masternodemodel.cpp (3)

41-112: Constructor performs JSON round-trip for fields not exposed by the MnEntry interface.

The constructor extracts some fields directly from MnEntry (lines 42–55) and others by parsing the JSON representation (lines 82–111). This works but means every entry construction calls toJson() + write(2) followed by multiple find_value lookups.

For ~500+ masternodes this runs on every reconciliation cycle. If the MnEntry interface could expose collateralHash, consecutivePayments, PoSeBanHeight, etc., the JSON serialization/parsing overhead could be eliminated. Worth considering in a follow-up if profiling shows this is a bottleneck.

---

348-405: Reconcile algorithm is correct — the offset-adjustment logic for dataChanged is subtle but verified.

I traced multiple scenarios (interleaved changes/removals) and the index adjustment is correct: the removed vector holds original pre-removal indices, and subtracting the count of removed indices below each changed index gives the correct post-removal position.

One subtlety worth noting: in-place data updates (m_data[idx] = std::move(entry) on line 363) occur before beginRemoveRows/endRemoveRows for other rows, and before dataChanged is emitted (line 392). This is safe because all operations are synchronous on the GUI thread, but a future refactor introducing background processing would break this assumption. A brief inline comment on the signal ordering would help future maintainers.

---

212-228: Tooltip shows "0 day(s)" for nodes banned/active less than one full day.

Integer division (m_current_height - height) / blocks_per_day truncates, so anything under ~576 blocks shows "0 day(s)". Consider using a "less than 1 day" message or showing hours for the sub-day case if UX precision matters.

src/qt/masternodemodel.h (1)

82-85: toTie() subset is well-chosen for mutable fields — consider a brief comment.

The tie includes only fields that change over time (m_banned, heights, penalty, service, reward). Immutable fields (addresses, hashes) are correctly excluded. A one-line comment explaining this would help reviewers understand the intent.

src/qt/masternodelist.cpp (2)

122-125: Explicit updateFilteredCount() calls are redundant given the signal connections.

updateFilteredCount is connected to rowsInserted, rowsRemoved, modelReset, and layoutChanged on the proxy. setFilterRegularExpression (line 291) internally calls invalidateFilter(), which emits layoutChanged, already triggering the connected slot. The explicit calls in on_filterText_textChanged (line 293), on_comboBoxType_currentIndexChanged (line 305), on_checkBoxOwned_stateChanged (line 319), on_checkBoxHideBanned_stateChanged (line 328), and updateDIP3List (line 254) are harmless but redundant.

Not a bug — just extra setText calls that will be superseded by the signal-driven ones.

Also applies to: 284-287, 289-293

---

308-320: Consider guarding against re-fetching the masternode list when the "Owned" checkbox is re-checked without changes.

Each time the checkbox is checked, clientModel->getMasternodeList() is called and the owned hashes are recomputed. If the user toggles rapidly, this causes repeated (potentially expensive) list fetches. This is bounded by the GUI thread serializing calls, but a dirty flag or timestamp guard could avoid redundant work.

[PastaPastaPasta]

utACK