perf: faster calculation for versionbits

[knst]

Issue being fixed or feature implemented

::cs_main mutex is kept after Dash-Qt starts until chain state is loaded.
Loading of changestate includes a step with calculations of BIP9-warnings about unknown bits.
It takes just 0.3s [far from instant, but reasonable fast] per bit in my environment. But knowing that VERSIONBITS_NUM_BITS=29 it makes Dash Core start for 5-8seconds slower.
Main window is shown on screen, UI is responsible during this calculations; but many RPC such as getblockchaininfo can not be called yet; sync of new blocks and governance objects is delayed for all time that ::cs_main is held.

This slow calculation for bip-9 warnings doesn't affect further consecutive blocks validation, because cache is used there and it's fulfilled after 1st time. Issue is relevant only for cold load of chainstate.

What was done?

This PR makes calculations of AbstractThresholdConditionChecker::GetStateFor for case of ThresholdState::STARTED faster by early exit from internal loop over 2016 [nPeriod] blocks.

This PR makes Dash Core (dash-qt, dashd) to sync 5-10 seconds faster after start.

How Has This Been Tested?

I added extra logs and made a guix build with all optimizations:

diff --git a/src/validation.cpp b/src/validation.cpp
index 7686968416..9b8bf4e7a8 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2996,6 +2996,7 @@ void CChainState::UpdateTip(const CBlockIndex* pindexNew)
     bilingual_str warning_messages;
     if (!this->IsInitialBlockDownload())
     {
+        LogPrintf("knst Check for warning bits...\n");
         const CBlockIndex* pindex = pindexNew;
         for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) {
             WarningBitsConditionChecker checker(bit);
@@ -3009,6 +3010,7 @@ void CChainState::UpdateTip(const CBlockIndex* pindexNew)
                 }
             }
         }
+        LogPrintf("knst Checked\n");
     }
     UpdateTipLog(coins_tip, pindexNew, m_params, m_evoDb, __func__, "", warning_messages.original);
 }

Current develop:

2025-12-11T16:29:08Z knst Check for warning bits... 
2025-12-11T16:29:13Z knst Checked

With changes from PR:

2025-12-13T19:46:05Z knst Check for warning bits...
2025-12-13T19:46:06Z knst Checked

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 (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

Precomputes the attempt count and threshold used during STARTED-state processing in src/versionbits.cpp. Adds an early-return: if pindexPrev->nHeight is below MinBIP9WarningHeight, the state is marked DEFINED immediately. In the STARTED path, nAttempt and the threshold are computed once and reused inside the counting loop and the final comparison, replacing per-iteration threshold recalculation. FAILED determination based on median time past is unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

• Verify correctness of the early-termination condition using MinBIP9WarningHeight.
• Confirm the precomputed nAttempt and threshold preserve original state-transition semantics across all window boundaries.
• Validate LOCKED_IN transition and final comparison against the precomputed threshold for edge cases near activation windows.
• Review the modified loop for off-by-one risks and maintainability (clarity of invariants).

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
Title check	✅ Passed	The title 'perf: faster calculation for versionbits' accurately describes the main optimization: improving performance of version bits calculation.
Description check	✅ Passed	The description clearly relates to the changeset, explaining the problem (slow BIP9 warning calculations during startup), the solution (early exit optimization in GetStateFor), and provides testing evidence.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff742c6 and 617059f.

📒 Files selected for processing (1)

• src/versionbits.cpp (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

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/versionbits.cpp

🧠 Learnings (1)

📓 Common learnings

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.

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: 6718
File: test/functional/test_framework/test_framework.py:2102-2102
Timestamp: 2025-06-09T16:43:20.996Z
Learning: In the test framework consolidation PR (#6718), user kwvg prefers to limit functional changes to those directly related to MasternodeInfo, avoiding scope creep even for minor improvements like error handling consistency.

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: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.

🧬 Code graph analysis (1)

src/versionbits.cpp (2)

src/test/fuzz/versionbits.cpp (13)

• pindexPrev (53-53)
• pindexPrev (53-53)
• pindexPrev (54-54)
• pindexPrev (54-54)
• pindexPrev (55-55)
• pindexPrev (55-55)
• params (46-46)
• params (46-46)
• params (48-48)
• params (48-48)
• params (49-49)
• params (49-49)
• params (50-50)

src/validation.cpp (8)

• params (2205-2205)
• params (2205-2205)
• params (2206-2206)
• params (2206-2206)
• params (2208-2208)
• params (2208-2208)
• params (2209-2209)
• params (2209-2209)

⏰ 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_nowallet-build / Build source
• GitHub Check: linux64_tsan-build / Build source
• GitHub Check: win64-build / Build source
• GitHub Check: linux64_fuzz-build / Build source
• GitHub Check: linux64_sqlite-build / Build source
• GitHub Check: linux64_ubsan-build / Build source
• GitHub Check: linux64-build / Build source
• GitHub Check: mac-build / Build source
• GitHub Check: arm-linux-build / Build source
• GitHub Check: Lint / Run linters

🔇 Additional comments (4)

src/versionbits.cpp (4)

105-106: LGTM! Effective precomputation optimization.

Computing nAttempt and threshold once before the loop eliminates redundant calculations. Since both values remain constant throughout the period window, this is a correct optimization with no behavioral change.

---

107-107: LGTM! Correct early-exit optimization.

The loop condition count + (nPeriod - i) >= threshold enables early termination when the maximum possible count (current count plus remaining blocks) falls below the threshold. This optimization is the core performance improvement described in the PR, avoiding full 2016-block scans when the outcome is predetermined.

The logic is sound: the loop exits when reaching the threshold becomes impossible, preserving behavioral equivalence while eliminating unnecessary iterations.

---

114-114: LGTM! Reuses precomputed threshold efficiently.

The comparison now uses the precomputed threshold from line 106, eliminating a redundant Threshold(params, nAttempt) call. This is a straightforward optimization with no behavioral change.

---

64-68: The early exit optimization is safe and preserves consensus semantics. Verification confirms that MinBIP9WarningHeight is explicitly intended for warning suppression (documented in consensus/params.h as "Don't warn about unknown BIP 9 activations below this height"), and WarningBitsConditionChecker::Condition() already enforces the height check independently. Blocks below this threshold would never satisfy the warning condition anyway. Setting DEFINED for these blocks is semantically correct, consistent with the existing nTimeStart optimization at lines 69-73, and does not affect consensus rule activation logic.

---

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

[UdjinM6]

Pls consider 1bb965e instead. CI: https://github.com/UdjinM6/dash/actions/runs/20215172691.

[knst]

pindexPrev->nHeight < params.MinBIP9WarningHeight

It changes logic and that's a breaking change.

Bip9warning is used only for warnings, but not for real for calculations of fork so far.

But yeah, it works. What do you think?

[UdjinM6]

pindexPrev->nHeight < params.MinBIP9WarningHeight

It changes logic and that's a breaking change.

Not really. All known old deployments are below MinBIP9WarningHeight and they are buried and new deployments never reach this part because they are handled by pindexPrev->GetMedianTimePast() < nTimeStart condition above.

[knst]

I moved check pindexPrev->nHeight < params.MinBIP9WarningHeight above pindexPrev->GetMedianTimePast() < nTimeStart because it is faster than GetMedianTimePast() calculation.

2025-12-15T16:53:23.141231Z knst Check for warning bits...
2025-12-15T16:53:23.154798Z knst Checked

5seconds -> 0.013s. 300 times faster! We are safe until we reach 200M blocks :D

[UdjinM6]

utACK 617059f