fix: skip repeated authenticated Platform bans

[PastaPastaPasta]

Issue being fixed or feature implemented

Direct Platform ban messages repeat signature verification, handler logging and relay work even when the same authenticated context is already cached. Identical same-height ban updates also report a successful metadata transition. A sender needs a legitimate recent ban for an existing EvoNode to trigger this redundant work; no throughput loss or node outage is claimed.

What was done?

Check the existing authenticated-object cache before handler logging and verification, while preserving request bookkeeping. Make identical same-height bans idempotent and remember authenticated contexts even when a newer metadata watermark rejects the update, including after restart. Unknown contexts still pass all target, height, quorum and signature checks. Unsolicited first-hop ingress, historical quorum acceptance, newer-height watermark advancement and same-height ban precedence are preserved.

How Has This Been Tested?

On macOS arm64, p2p_platform_ban.py passed. A legitimate signed ban still propagates and affects the next quorum. Resending one identical direct message produces no new PLATFORMBAN -- hash: handler log, and a service update revives the EvoNode. This check would fail on develop, where the handler logs before any cache check.

No replay throughput benchmark, log-amplification measurement or outage demonstration was performed. The skipped BLS work follows from the early return being placed before verification.

Breaking Changes

None

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1ea88b29-cf64-4d97-ace6-c8188c1bafc4

📥 Commits

Reviewing files that changed from the base of the PR and between f35a24c and 6224875.

📒 Files selected for processing (7)

• src/Makefile.test.include
• src/masternode/meta.cpp
• src/masternode/meta.h
• src/net_processing.cpp
• src/test/masternode_meta_tests.cpp
• test/functional/p2p_platform_ban.py
• test/util/data/non-backported.txt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

---

Walkthrough

The change updates same-height platform-ban metadata rules and records received messages in the ban cache even when metadata does not change. Message processing now returns early for cached bans before logging or continuing. New unit tests cover metadata precedence, cache behavior, and serialization reloads. The functional test checks that resending a verified message produces no matching log entry.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 62248

This change skips repeated processing of already-authenticated Platform ban messages while keeping existing behavior for new ones. No merge-blocking risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: skipping repeated authenticated Platform bans.
Description check	✅ Passed	The description directly explains the repeated-ban issue, the implemented cache and idempotency changes, and the testing performed.

Full details: Docstring Coverage

Explanation

Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.}

[thepastaclaw]

✅ Final review complete — no blockers (commit 2039964)

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Verified the complete base-to-head diff at 6224875 against the supplied reviewer evidence; no actionable in-scope defects were found. The cache shortcut preserves request bookkeeping, uncached contexts retain target/height/quorum/signature validation, and the metadata changes preserve height precedence while suppressing duplicate transitions and remembering authenticated contexts after reload. This was a static review: regression tests and their registration were inspected but not run; the supplied CI snapshot shows successful container builds and formatting checks, two queued manifest jobs, and no completed unit or functional test results.

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, contained change to authenticated-ban deduplication and metadata idempotency, with focused regression tests and no changes to signature verification rules or deserialization.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 95% left, weekly 95% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

[thepastaclaw]

Re-review — Final validation — Phase 2 only (queue backlog)

Verified the complete diff at 2039964. The authenticated-cache fast path preserves request bookkeeping, and unknown contexts still undergo the existing validation checks; no blocking correctness issue was found. One regression-coverage suggestion remains. This was a static review: the supplied CI snapshot shows successful formatting and preliminary checks, but container builds remain queued and provides no completed build or functional-test result.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff is small and contained, adding an authenticated-cache early return, idempotent metadata updates, and a focused regression check without intricate changes to a critical surface.
• Phase 1 reviewers: not run (skipped for throughput: 16 PRs queued, above the 10 limit)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

🤖 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 `test/functional/p2p_platform_ban.py`:
- [SUGGESTION] test/functional/p2p_platform_ban.py:76-79: Cover cold-cache duplicate and superseded metadata updates
  This replay follows a successful first delivery that already populated the authenticated cache, so it returns before reaching either modified metadata method. It therefore covers the handler fast path but not same-height metadata idempotence or caching an authenticated ban rejected by a newer watermark—both explicit goals of this PR. Add cold-cache scenarios by restarting the receiver with persisted banned metadata, and separately by applying a newer service update to the targeted EvoNode before restarting. Keep the signed message within the accepted height window. For each scenario, verify that the first delivery reaches the handler without forwarding inventory or changing the persisted ban state/watermark, and that the second delivery produces no new handler log. The existing protx info metaInfo fields expose the state and watermark directly.

[thepastaclaw]

⏭️ Deferred (maintainer decision; not blocking) at 20399642; see the replies below.

🟡 Suggestion: Cover cold-cache duplicate and superseded metadata updates

This replay follows a successful first delivery that already populated the authenticated cache, so it returns before reaching either modified metadata method. It therefore covers the handler fast path but not same-height metadata idempotence or caching an authenticated ban rejected by a newer watermark—both explicit goals of this PR. Add cold-cache scenarios by restarting the receiver with persisted banned metadata, and separately by applying a newer service update to the targeted EvoNode before restarting. Keep the signed message within the accepted height window. For each scenario, verify that the first delivery reaches the handler without forwarding inventory or changing the persisted ban state/watermark, and that the second delivery produces no new handler log. The existing protx info metaInfo fields expose the state and watermark directly.

_{source: gpt-6.1-sol (phase2-reviewer: general)}

[PastaPastaPasta]

Declining for this PR. The two metadata changes (same-height SetPlatformBan returning false, and caching a ban superseded by a newer watermark) are each a one-line condition that is clear from the diff, and the maintainer preference here is to keep the PR minimal rather than add restart-based functional scenarios for them. The existing replay check covers the handler fast path, which is the behavior the fix is about.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[thepastaclaw]

Understood; I accept keeping this PR minimal and deferring the restart-based coverage, since the existing replay test covers the handler fast path and I have not identified a correctness defect in the two metadata changes.

Noted as intentionally deferred; I will not press it further here.

[thepastaclaw]

Re-review after discussion — commit 2039964

Standing review was COMMENTED; this re-review is APPROVE: no blocking findings remain.

Approved: the discussion withdrew or deferred every finding the full review of 20399642 (run 3937) raised, and the approval rests on that review.

Same commit as the standing review; the inline threads above carry the per-finding outcome. This follow-up exists only to correct the verdict.

Review provenance

Source: conversation lane gpt-6.1-sol (agent: conversation); no reviewer or verifier lanes ran for this follow-up

• Verdict moved because the inline discussion withdrew or deferred findings of the standing review of this commit; the code was not re-reviewed

[PastaPastaPasta]

approved