fix: only sign EHF signals inside the deployment's start/timeout window

[PastaPastaPasta]

Issue being fixed or feature implemented

Masternodes create and mine MnEHF (DIP-0023) signals for deployments whose start/timeout window is not open, including deployments that are NEVER_ACTIVE.

CEHFSignalsHandler::UpdatedBlockTip() asks the mnhf quorum to sign a signal for every useEHF deployment whose bit has not been mined yet, as long as CChainParams::IsValidMNActivation() returns true. That function is the validation check, and it is deliberately permissive:

• A NEVER_ACTIVE start time is -2, so timePast < nStartTime is never true and the deployment counts as in range.
• A deployment outside of its window is skipped with continue, and then the "unknown MnEHF fork bit" fallback returns true. Validation needs that fallback so that old signals on a reused bit stay valid on reindex (mnhf_bit_reuse_reindex_tests).

So the producer signs regardless of the window. On mainnet today V24 is on bit 13 with nStartTime = NEVER_ACTIVE, so as soon as enough mainnet masternodes run this code they will sign and mine a bit-13 signal. Setting a future mainnet start time would not help either: that case goes through the fallback and is signed too.

This has happened before. 1be3ac2 ("fix: move DEPLOYMENT_V24 to a fresh version bit") moved V24 off bit 12 because bit 12 "already has mined MNHF signals on mainnet and testnet from before V24 had a start time". That commit moved the bit but left the producer unchanged, so bit 13 would go the same way.

Why this is a real problem

Code path at c14104b (unchanged on current develop):

• The producer only checks that the signal is missing and IsValidMNActivation(): src/llmq/ehf_signals.cpp#L45
• IsValidMNActivation() skips out-of-window deployments and then accepts unknown bits: src/chainparams.cpp#L109-L131
• Mainnet V24 is bit 13, NEVER_ACTIVE: src/chainparams.cpp#L212-L213
• Validation uses the same check, so the signal is accepted into the mempool and blocks: src/evo/mnhftx.cpp#L124-L126, src/evo/mnhftx.cpp#L237-L240
• A mined signal only expires if its MTP is below a positive nStartTime, so nodes with NEVER_ACTIVE keep it forever and reject any later signal for that bit as bad-mnhf-duplicate: src/evo/mnhftx.cpp#L80, src/evo/mnhftx.cpp#L232-L235

Reproduction: the updated feature_mnehf.py from this PR, run against a dashd without the src/llmq/ehf_signals.cpp change. The test starts the testdummy deployment (bit 28) as NEVER_ACTIVE (-vbparams=testdummy:-2:...), mines two blocks and checks the masternode's log. The unpatched masternode tries to sign it on every new tip:

feature_mnehf.py against unpatched dashd

TestFramework (INFO): Check that masternodes do not sign testdummy while it is NEVER_ACTIVE
AssertionError: [node 1] Unexpected message "bit=28 at height" partially matches log:
 - ... [trySignEHFSignal] [ehf] CEHFSignalsHandler::trySignEHFSignal: already signed bit=28 at height=175 id=a0eee872d7d3170dd20d5c5e8380c92b3aa887da5f63d8033289fafa35a90691

Why it matters

If v24.0.0 ships like this, upgraded mainnet masternodes will mine a bit-13 signal long before V24 has a start time. Nodes whose V24 start time is still NEVER_ACTIVE (v24.0.0 itself) never expire that signal, so when the real signal is mined after a later release sets the start time, those nodes reject that block as bad-mnhf-duplicate. If the start time were instead set at or before the early signal, the early signal would count toward activation. In both cases the bit cannot be used cleanly any more, which is what already happened to bit 12. Nothing outside the masternode set can trigger this. It happens on its own once the quorum runs the code.

This should also go to v24.0.x: v24.0.0-rc.1 and rc.2 carry V24 on bit 13 with mainnet NEVER_ACTIVE and the same producer code.

What was done?

In CEHFSignalsHandler::UpdatedBlockTip(), a deployment is now skipped unless its own signalling window is open at the tip MTP: nStartTime != NEVER_ACTIVE && nStartTime <= MTP && MTP <= nTimeout. The existing "not mined yet" and IsValidMNActivation() conditions are kept as they were.

feature_mnehf.py gets a new first step, test_no_signing_outside_window(), which runs right after the first quorum is mined and before the existing testdummy activation flow:

• testdummy starts as NEVER_ACTIVE, then the nodes are restarted with a window that has already timed out (0:1) and one that starts 10 minutes in the future. In each case two blocks are mined and the test checks that the masternode logged no signing attempt for bit 28 and that its mempool has no bit-28 signal.
• Mocktime is then moved past the start time and the test waits until the masternode signs testdummy, so the negative checks are not passing just because the masternode cannot sign.
• The rest of the test is unchanged. It uses testdummy rather than v24, so it keeps working after v24 is buried.

Why this is the correct minimal fix

The wrong decision is made in the producer, so the fix goes there. IsValidMNActivation() cannot be tightened instead. It is the validation check, and its fallback for out-of-window and unknown bits is what keeps old signals valid on reindex after a bit is reused (mnhf_bit_reuse_reindex_tests). Changing it would make nodes disagree about blocks they already accepted. CheckMNHFTx() and CMNHFManager::ProcessBlock() are not touched, so consensus is unchanged: a signal mined by an old node is still accepted exactly as before.

The NEVER_ACTIVE check has to be explicit. It is the mainnet case, and the plain range check treats -2 as an already-passed start time. Deployments with ALWAYS_ACTIVE (-1) or a past start and NO_TIMEOUT still pass, so regtest/devnet deployments (start 0) and testnet V24 (its window opened 2026-09-24) are signed as before.

Checking the tip MTP is enough. The signal is judged later by the MTP of the block that includes it, and that MTP is greater than or equal to the tip's. I did not add a re-check in HandleNewRecoveredSig(). It only submits signals for request ids that trySignEHFSignal() registered, and those are only registered while the window is open.

How Has This Been Tested?

macOS arm64, --enable-debug build.

• test/functional/feature_mnehf.py against a dashd without the src/llmq/ehf_signals.cpp change: fails at the NEVER_ACTIVE check (output above).
• test/functional/feature_mnehf.py with this PR: passes.
• Each condition of the new check was removed in turn, and every variant fails in the step meant to catch it:
  • Without the NEVER_ACTIVE check, the "while it is NEVER_ACTIVE" step fails.
  • Without MTP <= nTimeout, the "after its timeout" step fails.
  • Without nStartTime <= MTP, the "before its start time" step fails.
• test/functional/feature_asset_locks.py with this PR: passes. It activates regtest v24 (start 0) through masternode-signed EHF.
• src/test/test_dash --run_test=evo_mnhf_tests (includes mnhf_bit_reuse_reindex_tests): passes.
• test/lint/lint-python.py, test/lint/lint-whitespace.py and contrib/devtools/clang-format-diff.py on the diff: clean.

Breaking Changes

None. Consensus validation is unchanged. Masternodes no longer create signals for deployments whose window is not open.

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 (for repository code-owners and collaborators only)

🤖 Generated with Claude Code

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (1)

doc/developer-notes.md — auto-discovered

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: aee2c3dc-34b0-46a8-95e7-f471f74f444b

📥 Commits

Reviewing files that changed from the base of the PR and between 3789ec0 and dcff069.

📒 Files selected for processing (2)

• src/llmq/ehf_signals.cpp
• test/functional/feature_mnehf.py

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

---

Walkthrough

The EHF signal handler now signs only when median-time-past is within the deployment’s inclusive start-time-to-timeout window and the start time is not NEVER_ACTIVE. Functional tests check that no signal is produced outside the window and that a signal is produced after the start time.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to dcff0

The change appears mergeable after normal checks. No actionable issue is established for the current head.

Security Architecture Review

Security architecture risk: 🔵 Low · up to dcff0

The change restricts when activation signals are requested without changing existing acceptance rules. No introduced security weakness was established, but delayed signing completion and recovery near deployment boundaries remain incompletely validated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The relevant exposure is network-wide deployment signaling for a version bit, not merely one node's local state. An accepted mined signal enters chain-derived activation state. The PR reduces automatic request eligibility but does not replace the quorum-signature authority required for acceptance.

Security Findings and Attack Paths

• inferred — The changed request path is a restriction of the base path: it adds a condition without removing existing checks or accepting a new caller-controlled parameter. The inspected change does not establish a new route for an unauthenticated actor to obtain a valid activation signature.

Trust Boundaries and Controls

• observed — Signing requires an active masternode identity and valid membership in the selected quorum. Transaction validation independently checks payload bounds, quorum provenance on the applicable chain, and the signature over the quorum type, quorum hash, version-bit request ID, and transaction hash. Permissive time validation alone cannot authenticate a fabricated signal.

Resilience and Maintainability Implications

• observed — Existing controls suppress already-recovered requests and reject duplicate mined bits. Signal eligibility is derived from the relevant chain history, including expiration when a bit is reused with a newer start time. These controls remain distinct from the unresolved automatic re-delivery behavior after interruption or reorganization.

Hardening Proposals

• proposed — Specify whether window eligibility is authoritative at request initiation or at signature completion, then exercise an in-flight request across timeout and reorganization. Any additional producer-side policy should preserve historical consensus acceptance rather than silently turning this fix into a validation-rule change.

🚥 Pre-merge checks | ✅ 4 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the change to limit EHF signing to the deployment window.
Description check	✅ Passed	The description explains the signing-window issue, the producer-side fix, and the added tests.
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.

• 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]

⚠️ Automated review could not complete (commit dcff069)
Reason: phase2/general lane failed twice: [infra] lane exit 1: API Error: 400 unknown provider for model gpt-6.1-sol

[github-actions]

This pull request has conflicts, please rebase.

[knst]

Should not use v24 in the functional tests to test forks. After activation this fork will be buried and test will require rewritting

[PastaPastaPasta]

Agreed. dcff069 drops the v24 override. The test now runs the out-of-window checks on testdummy (bit 28): it starts as NEVER_ACTIVE, then gets a window that has already timed out and one that has not started yet, and finally the test checks that the masternode does sign once the start time is reached.

---

🤖 Posted autonomously by Codex on behalf of pasta.

[knst]

This test sounds be done at the very beginning but not the last scenario

[PastaPastaPasta]

Done in dcff069. The checks now run right after the first quorum is mined, before the existing activation flow, instead of as the last scenario. The two extra quorums per round and the 14-day mocktime bumps are gone.

---

🤖 Posted autonomously by Codex on behalf of pasta.

[thepastaclaw]

Final validation — Phase 1 + Phase 2

At head 7809670, the producer-side guard correctly excludes NEVER_ACTIVE deployments and enforces the inclusive start/timeout window without changing consensus validation. The branch has a verified merge conflict with develop, and its title fails the configured scope validation. This was a static review: the supplied CI snapshot shows successful container builds and queued manifest jobs, but does not establish completion of the unit or functional tests.

🔴 1 blocking | 💬 1 nitpick(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

💬 Nitpick: Remove the unsupported llmq scope from the PR title
.github/workflows/semantic-pull-request.yml:38-52

The PR title uses fix(llmq), but the workflow's allowed scopes do not include llmq. The failed Validate PR title job explicitly reports Unknown scope "llmq" for this title. Retitle the PR to "fix: only sign EHF signals inside the deployment's start/timeout window"; no workflow change is needed.

_{source: glm-5.3-flash (phase1-reviewer: general)}

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 adds a small, straightforward deployment-window guard in CEHFSignalsHandler::UpdatedBlockTip() and focused functional coverage without changing consensus validation or signature mechanics.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 99% left, weekly 68% 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

🤖 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/feature_mnehf.py`:
- [BLOCKING] test/functional/feature_mnehf.py:33-34: Resolve the functional-test conflict with develop's single-member-quorum conversion
  Merging this exact head with the supplied develop base, 88195077594b838b3abd5724bda9bd4ea1677a80, produces a content conflict in set_test_params(): this PR adds the v24 override to the four-node setup, while #7778 replaces that setup with two nodes, one masternode, and set_dash_llmq_test_params(1, 1). The check_merge job independently reports the same file conflict. Rebase and resolve the test adaptation while preserving develop's single-member-quorum changes, including its create_mnehf() and activation-boundary fixes; adapt the new outside-window rounds to the single-member mining helper as well. The current branch cannot merge into its target.

In `.github/workflows/semantic-pull-request.yml`:
- [NITPICK] .github/workflows/semantic-pull-request.yml:38-52: Remove the unsupported llmq scope from the PR title
  The PR title uses fix(llmq), but the workflow's allowed scopes do not include llmq. The failed Validate PR title job explicitly reports Unknown scope "llmq" for this title. Retitle the PR to "fix: only sign EHF signals inside the deployment's start/timeout window"; no workflow change is needed.

[thepastaclaw]

🔴 Blocking: Resolve the functional-test conflict with develop's single-member-quorum conversion

Merging this exact head with the supplied develop base, 8819507, produces a content conflict in set_test_params(): this PR adds the v24 override to the four-node setup, while #7778 replaces that setup with two nodes, one masternode, and set_dash_llmq_test_params(1, 1). The check_merge job independently reports the same file conflict. Rebase and resolve the test adaptation while preserving develop's single-member-quorum changes, including its create_mnehf() and activation-boundary fixes; adapt the new outside-window rounds to the single-member mining helper as well. The current branch cannot merge into its target.

_{source: glm-5.3-flash (phase1-reviewer: general)}

[PastaPastaPasta]

Rebased onto develop (3789ec0) in dcff069. The test is rewritten on top of the single-member quorum setup from #7778 and uses mine_quorum_single_member(). The PR title no longer has the llmq scope.

---

🤖 Posted autonomously by Codex on behalf of pasta.

[knst]

utACK dcff069