fix(qt): prevent banned masternodes from returning status=0 by UdjinM6 · Pull Request #7157 · dashpay/dash

UdjinM6 · 2026-02-19T19:24:15Z

Issue being fixed or feature implemented

Two code paths in MasternodeModel::data() could return 0 for banned nodes:

ban_height == m_current_height (banned this block)
ban_height unavailable (unknown ban time)

0 is also valid for an active node registered in the current block, so the "Hide banned" filter (status > 0) could silently fail to hide freshly-banned nodes.

#7147 follow-up

What was done?

Fix: add +1 offset to the banned-node calculation and change the fallback return from 0 to 1. The encoding contract is now unambiguous:

Banned → always ≥ 1 (age in blocks + 1)
Active → always ≤ 0 (0 = registered this block)

Relative sort order among banned nodes is preserved.

How Has This Been Tested?

Breaking Changes

n/a

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
I have made corresponding changes to the documentation
I have assigned this pull request to a milestone

github-actions · 2026-02-19T19:26:27Z

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

coderabbitai · 2026-02-19T19:28:01Z

No actionable comments were generated in the recent review. 🎉

Walkthrough

The change updates MasternodeModel::data behavior for STATUS EditRole. For banned masternodes with a positive poseBanHeight, the displayed/sort value is now computed as (current_height - ban_height + 1) instead of (current_height - ban_height). For banned entries with unknown ban height, the return value is changed from 0 to 1. No other status handling for active or unknown nodes was modified.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 directly addresses the main change: preventing banned masternodes from returning status=0, which is the core issue being fixed.
Description check	✅ Passed	The description clearly explains the bug being fixed, the solution implemented, and the new encoding contract, all directly related to the changeset.

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

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

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

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.

❤️ Share

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

kwvg

utACK 82ae05b

PastaPastaPasta · 2026-02-20T16:23:29Z

src/qt/masternodemodel.cpp

                    // Banned nodes use positive values
                    if (auto ban_height = entry->poseBanHeight(); ban_height && *ban_height > 0) {
-                        return m_current_height - *ban_height;
+                        return m_current_height - *ban_height + 1;  // +1: freshly banned → 1, never 0


Suggested change

return m_current_height - *ban_height + 1; // +1: freshly banned → 1, never 0

return m_current_height - *ban_height + 1; // +1: freshly banned → 1, never 0

Two code paths in MasternodeModel::data() could return 0 for banned nodes: - ban_height == m_current_height (banned this block) - ban_height unavailable (unknown ban time) 0 is also valid for an active node registered in the current block, so the "Hide banned" filter (status > 0) could silently fail to hide freshly-banned nodes. Fix: add +1 offset to the banned-node calculation and change the fallback return from 0 to 1. The encoding contract is now unambiguous: - Banned → always ≥ 1 (age in blocks + 1) - Active → always ≤ 0 (0 = registered this block) Relative sort order among banned nodes is preserved. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

kwvg

utACK 9705b9a

…g status=0 9705b9a fix(qt): prevent banned masternodes from returning status=0 (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Two code paths in `MasternodeModel::data()` could return `0` for banned nodes: - `ban_height` == `m_current_height` (banned this block) - `ban_height` unavailable (unknown ban time) `0` is also valid for an active node registered in the current block, so the "Hide banned" filter (`status > 0`) could silently fail to hide freshly-banned nodes. dashpay#7147 follow-up ## What was done? Fix: add `+1` offset to the banned-node calculation and change the fallback return from 0 to 1. The encoding contract is now unambiguous: - Banned → always ≥ 1 (age in blocks + 1) - Active → always ≤ 0 (0 = registered this block) Relative sort order among banned nodes is preserved. ## How Has This Been Tested? ## Breaking Changes n/a ## 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 - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone ACKs for top commit: kwvg: utACK 9705b9a Tree-SHA512: aa0aea49ad02ab23155dd5568baaac384bfcd30ff9fecbbd4ecd297c1b4f0207fd68399e858edde50daf36c6017b0bdd4d8a04be087e85d5337db3cab8079cfd

00f590d Merge #7180: qt: add Tahoe styled icons for macOS, runtime styling for each network type, update bundle icon, add mask-based tray icon, generation scripts (pasta) 60dda51 Merge #7176: perf: do linear lookup instead building 2 heavy Hash-Maps (pasta) df1ca87 Merge #7159: feat(qt): UI refresh (5/n, add proposal information widget to information, donut chart for proposal allocation) (pasta) 9061ad0 Merge #7118: feat(qt): UI refresh (4/n, introduce distinct widgets for Dash-specific reporting in debug window) (pasta) 64cc4f2 Merge #7160: feat(interfaces): consolidate masternode counts into one struct, expose chainlock, instantsend, credit pool, quorum statistics (pasta) 5d28a69 Merge #7157: fix(qt): prevent banned masternodes from returning status=0 (pasta) e0b7386 Merge #7146: feat(qt): introduce framework for sourcing and applying data, use for `{Masternode,Proposal}List`s (pasta) 8fd53cd Merge #7144: feat(qt): add support for reporting `OP_RETURN` payloads as Data Transactions (pasta) cc6f0bb Merge #7154: fix: MN update notifications had old_list/new_list swapped (pasta) 33f0138 Merge #7145: fix(qt): move labelError styling from proposalcreate.ui into general.css (pasta) 1bdbde6 Merge #7148: feat(qt): persist filter preferences in masternode list (pasta) 96bb601 Merge #7147: fix(qt): prevent overview page font double scaling, recalculate minimum width correctly, `SERVICE` and `STATUS` sorting, fix common types filtering (pasta) Pull request description: ## Backport PRs for v23.1.1 Cherry-picks the following 12 PRs (labeled `backport-candidate-23.1.x`) from `develop` onto `v23.1.x`, in merge order: | PR | Title | |---|---| | #7147 | fix(qt): prevent overview page font double scaling, recalculate minimum width correctly, `SERVICE` and `STATUS` sorting, fix common types filtering | | #7148 | feat(qt): persist filter preferences in masternode list | | #7145 | fix(qt): move labelError styling from proposalcreate.ui into general.css | | #7154 | fix: MN update notifications had old_list/new_list swapped | | #7144 | feat(qt): add support for reporting `OP_RETURN` payloads as Data Transactions | | #7146 | feat(qt): introduce framework for sourcing and applying data, use for `{Masternode,Proposal}List`s | | #7157 | fix(qt): prevent banned masternodes from returning status=0 | | #7160 | feat(interfaces): consolidate masternode counts into one struct, expose chainlock, instantsend, credit pool, quorum statistics | | #7118 | feat(qt): UI refresh (4/n, introduce distinct widgets for Dash-specific reporting in debug window) | | #7159 | feat(qt): UI refresh (5/n, add proposal information widget to information, donut chart for proposal allocation) | | #7176 | perf: do linear lookup instead building 2 heavy Hash-Maps | | #7180 | qt: add Tahoe styled icons for macOS, runtime styling for each network type, update bundle icon, add mask-based tray icon, generation scripts | All 12 cherry-picked cleanly (no conflicts). ## Notes - Used `git cherry-pick -m 1 <merge-sha>` for each (all were merge commits on develop) - Applied in chronological merge order to respect dependency chains - Version bump and release notes are separate steps per the release process ACKs for top commit: kwvg: utACK 00f590d UdjinM6: utACK 00f590d Tree-SHA512: 90d2a0660db8daa69b3e3b33a8a790fb0ea7d9a04656a2e27955575e76b6f4c9a379c435ef1c573ef6669c36cb6e205ba9701716d3dc303b01f19c719516b6d1

da16809 docs: add #7221 and #7222 to v23.1.2 release notes (PastaClaw) 8a93926 Merge #7222: fix: properly skip evodb repair on reindex (pasta) b74b549 Merge #7221: refactor: rename bitcoin-util manpage and test references to dash-util (pasta) 81464ac Merge #7211: fix: qt info tab layout (pasta) 7c27c2f Merge #7209: fix: serialize TrySignChainTip to prevent concurrent signing race (pasta) 81d5eb2 Merge #7208: fix: skip collecting block txids during IBD to prevent unbounded memory growth (pasta) d02243c ci: run check-skip on blacksmith with GitHub-hosted fallback (PastaClaw) 033b3fe chore: regenerate manpages for v23.1.2 (PastaClaw) ff965b5 chore: v23.1.2 release preparation (PastaClaw) 8d5936d chore: add #7191 and #7193 to v23.1.1 release notes (PastaClaw) 9f3662b chore: v23.1.1 release preparation (PastaClaw) 5dbfa98 chore: v23.1.1 release preparation (PastaClaw) 240a95f Merge #7193: fix: reject identity elements in deserialization and key generation (pasta) 444cbf2 Merge #7191: fix(qt): reseat quorum labels when new types are inserted (pasta) 00f590d Merge #7180: qt: add Tahoe styled icons for macOS, runtime styling for each network type, update bundle icon, add mask-based tray icon, generation scripts (pasta) 60dda51 Merge #7176: perf: do linear lookup instead building 2 heavy Hash-Maps (pasta) df1ca87 Merge #7159: feat(qt): UI refresh (5/n, add proposal information widget to information, donut chart for proposal allocation) (pasta) 9061ad0 Merge #7118: feat(qt): UI refresh (4/n, introduce distinct widgets for Dash-specific reporting in debug window) (pasta) 64cc4f2 Merge #7160: feat(interfaces): consolidate masternode counts into one struct, expose chainlock, instantsend, credit pool, quorum statistics (pasta) 5d28a69 Merge #7157: fix(qt): prevent banned masternodes from returning status=0 (pasta) e0b7386 Merge #7146: feat(qt): introduce framework for sourcing and applying data, use for `{Masternode,Proposal}List`s (pasta) 8fd53cd Merge #7144: feat(qt): add support for reporting `OP_RETURN` payloads as Data Transactions (pasta) cc6f0bb Merge #7154: fix: MN update notifications had old_list/new_list swapped (pasta) 33f0138 Merge #7145: fix(qt): move labelError styling from proposalcreate.ui into general.css (pasta) 1bdbde6 Merge #7148: feat(qt): persist filter preferences in masternode list (pasta) 96bb601 Merge #7147: fix(qt): prevent overview page font double scaling, recalculate minimum width correctly, `SERVICE` and `STATUS` sorting, fix common types filtering (pasta) da1e336 build: expand minimum Darwin version to macOS 11 (Big Sur) (Kittywhiskers Van Gogh) Pull request description: ## Issue being fixed or feature implemented Note: Skipping changes from #7149 which was for the v23.1.x only. ## What was done? ## How Has This Been Tested? ## Breaking Changes ## 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 - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: kwvg: utACK 36988f9 Tree-SHA512: f5bf8f0af11379bbcea606108ee90af08c16f588ebdbfac1fdd567adfcad14926b9c797c8fa6b398fb65fc3210c5f2c084015ea07718371df04e2412625f42d4

UdjinM6 added this to the 23.1.1 milestone Feb 19, 2026

UdjinM6 requested a review from kwvg February 19, 2026 19:24

UdjinM6 added the backport-candidate-23.1.x label Feb 19, 2026

kwvg previously approved these changes Feb 20, 2026

View reviewed changes

UdjinM6 requested review from PastaPastaPasta and knst February 20, 2026 09:31

PastaPastaPasta reviewed Feb 20, 2026

View reviewed changes

UdjinM6 dismissed kwvg’s stale review via 222ff42 February 20, 2026 16:29

UdjinM6 force-pushed the fix/masternode-status-ambiguous-zero branch from 82ae05b to 222ff42 Compare February 20, 2026 16:29

UdjinM6 force-pushed the fix/masternode-status-ambiguous-zero branch from 222ff42 to 9705b9a Compare February 20, 2026 16:29

kwvg approved these changes Feb 20, 2026

View reviewed changes

PastaPastaPasta merged commit 38bedd8 into dashpay:develop Feb 20, 2026
42 of 44 checks passed

thepastaclaw mentioned this pull request Feb 27, 2026

chore: backport 12 PRs to v23.1.x for v23.1.1 release #7182

Merged

UdjinM6 removed the backport-candidate-23.1.x label Mar 14, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(qt): prevent banned masternodes from returning status=0#7157

fix(qt): prevent banned masternodes from returning status=0#7157
PastaPastaPasta merged 1 commit intodashpay:developfrom
UdjinM6:fix/masternode-status-ambiguous-zero

UdjinM6 commented Feb 19, 2026

Uh oh!

github-actions bot commented Feb 19, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Feb 19, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

kwvg left a comment

Uh oh!

PastaPastaPasta Feb 20, 2026

Uh oh!

kwvg left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	return m_current_height - *ban_height + 1; // +1: freshly banned → 1, never 0
	return m_current_height - *ban_height + 1; // +1: freshly banned → 1, never 0

Conversation

UdjinM6 commented Feb 19, 2026

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

github-actions bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ No Merge Conflicts Detected

Uh oh!

coderabbitai bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

kwvg left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

kwvg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Feb 19, 2026 •

edited

Loading

coderabbitai bot commented Feb 19, 2026 •

edited

Loading