chore: enforce ban for re-propagation of stale QFCOMMITs

[kwvg]

Additional Information

Spun off from dash#7066 due to breaking change, enforces policy introduced in dash#5145.

Breaking changes

Broadcasting stale QFCOMMITs will result in the offending node getting banned.

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas (note: N/A)
• 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]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

The changes version-gate the punishment of old block propagation in the LLMQ block processor. The PROTOCOL_VERSION is incremented from 70238 to 70239, and a new constant QFCOMMIT_STALE_REPROP_BAN_VERSION is introduced with value 70239. In CQuorumBlockProcessor::ProcessMessage, handling of blocks older than the DKG interval now conditionally applies a 100-point misbehavior penalty only when the peer's protocol version is at least 70239; older protocol versions are not punished for this condition.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

• Verify the version constant value (70239) is consistent and appropriate for the feature gate
• Confirm the conditional version check correctly guards the misbehavior penalty assignment
• Ensure the early-return behavior for old blocks is preserved in both code paths

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is related to the changeset, explaining the breaking change context and policy enforcement related to stale QFCOMMIT handling.
Title check	✅ Passed	The title 'chore: enforce ban for re-propagation of stale QFCOMMITs' directly and clearly summarizes the main change: adding version-gated punishment for peers re-propagating stale QFCOMMIT messages.

✨ 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: 1

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0455eb8 and 858e9cb.

📒 Files selected for processing (2)

• src/llmq/blockprocessor.cpp (1 hunks)
• src/version.h (2 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Files:

• src/llmq/blockprocessor.cpp
• src/version.h

src/{masternode,evo,llmq,governance,coinjoin}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Use Dash-specific database implementations: CFlatDB for persistent storage (MasternodeMetaStore, GovernanceStore, SporkStore, NetFulfilledRequestStore) and CDBWrapper extensions for Evolution/DKG/InstantSend/Quorum/RecoveredSigs data

Files:

• src/llmq/blockprocessor.cpp

src/{masternode,llmq}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

BLS integration must be used for cryptographic foundation of advanced masternode features

Files:

• src/llmq/blockprocessor.cpp

src/llmq/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

src/llmq/**/*.{cpp,h}: LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)
InstantSend implementation must provide distributed key generation for secure transaction locking with quorum consensus
ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Files:

• src/llmq/blockprocessor.cpp

src/{masternode,llmq,evo,coinjoin,governance}/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

Use unordered_lru_cache for efficient caching with LRU eviction in Dash-specific data structures

Files:

• src/llmq/blockprocessor.cpp

🧠 Learnings (4)

📓 Common learnings

Learnt from: UdjinM6
Repo: dashpay/dash PR: 6933
File: src/llmq/utils.cpp:284-298
Timestamp: 2025-11-04T18:24:27.241Z
Learning: In consensus-critical code (such as quorum formation, block validation, or deployment activation logic), do not suggest changes to the logic itself even if the implementation appears theoretically incorrect or off-by-one. Consensus rules, once deployed on the Dash network, must be preserved exactly to avoid network forks. Refactoring PRs should maintain perfect behavioral equivalence. Only suggest logic changes if explicitly accompanied by a DIP (Dash Improvement Proposal) or if the maintainer indicates the consensus rule needs to be changed with appropriate activation logic.

Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.

Learnt from: kwvg
Repo: dashpay/dash PR: 6761
File: src/chainlock/signing.cpp:247-250
Timestamp: 2025-07-29T14:32:48.369Z
Learning: In PR #6761, kwvg acknowledged a null pointer check issue in ChainLockSigner::Cleanup() method but deferred it to follow-up, consistent with the pattern of avoiding scope creep in refactoring PRs.

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : LLMQ quorums must support multiple configurations (50/60, 400/60, 400/85) for different services (ChainLocks, InstantSend, governance voting)

📚 Learning: 2025-08-19T14:57:31.801Z

Learnt from: knst
Repo: dashpay/dash PR: 6692
File: src/llmq/blockprocessor.cpp:217-224
Timestamp: 2025-08-19T14:57:31.801Z
Learning: In PR #6692, knst acknowledged a null pointer dereference issue in ProcessBlock() method where LookupBlockIndex may return nullptr but is passed to gsl::not_null, and created follow-up PR #6789 to address it, consistent with avoiding scope creep in performance-focused PRs.

Applied to files:

• src/llmq/blockprocessor.cpp

📚 Learning: 2025-11-24T16:41:22.457Z

Learnt from: CR
Repo: dashpay/dash PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:41:22.457Z
Learning: Applies to src/llmq/**/*.{cpp,h} : ChainLocks implementation must prevent reorganizations and provide block finality through 51% attack prevention

Applied to files:

• src/llmq/blockprocessor.cpp

📚 Learning: 2025-06-06T11:53:09.094Z

Learnt from: kwvg
Repo: dashpay/dash PR: 6665
File: src/evo/providertx.h:82-82
Timestamp: 2025-06-06T11:53:09.094Z
Learning: In ProTx serialization code (SERIALIZE_METHODS), version checks should use hardcoded maximum flags (/*is_basic_scheme_active=*/true, /*is_extended_addr=*/true) rather than deployment-based flags. This is because serialization code should be able to deserialize any structurally valid ProTx up to the maximum version the code knows how to handle, regardless of current consensus validity. Validation code, not serialization code, is responsible for checking whether a ProTx version is consensus-valid based on deployment status.

Applied to files:

• src/version.h

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: linux64_multiprocess-build / Build source
• GitHub Check: mac-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: linux64_nowallet-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64_ubsan-build / Build source

🔇 Additional comments (2)

src/version.h (1)

13-13: LGTM! Protocol version bump is appropriate.

The version increment from 70238 to 70239 is correct for introducing the breaking P2P behavior change that bans re-propagation of stale QFCOMMIT messages.

src/llmq/blockprocessor.cpp (1)

134-136: LGTM! Version-gated punishment correctly implements the breaking change.

The conditional banning logic properly enforces the policy while maintaining backward compatibility. Peers running protocol version 70239 or higher will be banned for re-propagating stale QFCOMMIT messages (older than the DKG interval), while older peers remain unpunished during the network upgrade transition. The 100-point misbehavior penalty is appropriate for this violation.

[PastaPastaPasta]

utACK 8e44990; since this is protocol version guarded; not a breaking change.

[UdjinM6]

utACK 8e44990