refactor: use SteadyClock instead of SystemClock for duration measurements in Dash-specific code

[UdjinM6]

Issue being fixed or feature implemented

Extends the approach from bitcoin#27405 to Dash-specific code. All six call sites were purely measuring elapsed time for timeouts, throttling, or cache expiry — SteadyClock is monotonic and immune to NTP adjustments, making it the correct choice.

What was done?

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

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[thepastaclaw]

✅ Review complete (commit 01b8c79)

[coderabbitai]

Walkthrough

This PR migrates timing mechanisms across multiple components from epoch-based SystemClock (using TicksSinceEpoch with int64_t timestamps) to monotonic SteadyClock timing (using SteadyClock::time_point values). The changes affect cache entry timestamps in DKG session management, instant-send lock measurements, network connection linger timers, and phase-sleep intervals. Data structure members and local timing variables are updated to reflect the new time source, with comparisons adjusted to operate on time_point objects instead of integer tick values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 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%.	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 clearly and specifically describes the main change: refactoring code to use SteadyClock instead of SystemClock for duration measurements in Dash-specific code.
Description check	✅ Passed	The description explains the rationale (extending bitcoin#27405 to Dash code, SteadyClock is monotonic) and categorizes the changes as purely measuring elapsed time.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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]

🧹 Nitpick comments (1)

src/net.cpp (1)

3666-3677: LGTM — same pattern, correct here too.

Same reasoning as the ThreadSocketHandler change: the default-constructed SteadyClock::time_point{} initial value ensures the 100 ms gate opens on the first iteration, and using a monotonic clock makes the throttle immune to system-time jumps (previously a backward NTP step could extend the effective skip interval, a forward step could unexpectedly unblock it).

Minor note (not blocking): the unchanged condMsgProc.wait_until(...) call a few lines below at roughly line 3704 still spells out std::chrono::steady_clock::now() directly. Since SteadyClock is just std::chrono::steady_clock, it's functionally identical, but a follow-up to use the SteadyClock alias consistently here would be a nice readability nit.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/net.cpp` around lines 3666 - 3677, The condMsgProc.wait_until call still
uses std::chrono::steady_clock::now() directly while the surrounding code uses
the SteadyClock alias; update the wait_until invocation to use
SteadyClock::now() for consistency with the alias used around
nLastSendMessagesTimeMasternodes and the loop logic (refer to
condMsgProc.wait_until, SteadyClock, and nLastSendMessagesTimeMasternodes) so
the code consistently uses the SteadyClock typedef.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/net.cpp`:
- Around line 3666-3677: The condMsgProc.wait_until call still uses
std::chrono::steady_clock::now() directly while the surrounding code uses the
SteadyClock alias; update the wait_until invocation to use SteadyClock::now()
for consistency with the alias used around nLastSendMessagesTimeMasternodes and
the loop logic (refer to condMsgProc.wait_until, SteadyClock, and
nLastSendMessagesTimeMasternodes) so the code consistently uses the SteadyClock
typedef.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 17bb09fd-2752-4123-961a-e04d5d142063

📥 Commits

Reviewing files that changed from the base of the PR and between cfad414 and 01b8c79.

📒 Files selected for processing (7)

• src/active/dkgsessionhandler.cpp
• src/instantsend/instantsend.cpp
• src/instantsend/instantsend.h
• src/llmq/dkgsessionmgr.cpp
• src/llmq/dkgsessionmgr.h
• src/net.cpp
• src/net.h

[UdjinM6]

🧹 Nitpick comments (1)

src/net.cpp (1)> 3666-3677: LGTM — same pattern, correct here too.

Same reasoning as the ThreadSocketHandler change: the default-constructed SteadyClock::time_point{} initial value ensures the 100 ms gate opens on the first iteration, and using a monotonic clock makes the throttle immune to system-time jumps (previously a backward NTP step could extend the effective skip interval, a forward step could unexpectedly unblock it).
Minor note (not blocking): the unchanged condMsgProc.wait_until(...) call a few lines below at roughly line 3704 still spells out std::chrono::steady_clock::now() directly. Since SteadyClock is just std::chrono::steady_clock, it's functionally identical, but a follow-up to use the SteadyClock alias consistently here would be a nice readability nit.

@coderabbitai I believe this code is not Dash-specific and it shouldn't be touched here. Pls verify.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the ThreadMessageHandler function around lines 3650-3720 in src/net.cpp
sed -n '3640,3720p' src/net.cpp

Length of output: 2893

---

@UdjinM6, you are correct. Looking at the code:

• The nLastSendMessagesTimeMasternodes / fSkipSendMessagesForMasternodes block (lines 3669–3677) is Dash-specific masternode logic — properly touched in this PR.
• The condMsgProc.wait_until(lock, std::chrono::steady_clock::now() + std::chrono::milliseconds(100), ...) call is upstream Bitcoin's standard ThreadMessageHandler sleep/wake mechanism — not Dash-specific and should not be modified here.

My nitpick suggesting that line be updated to use the SteadyClock alias was incorrect and out of scope for this PR. Apologies for the noise.

(╥_╥)

[thepastaclaw]

Code Review

Clean SystemClock→SteadyClock refactor for Dash-specific duration-measurement call sites. No blocking issues or suggestions. Two nitpicks surfaced by Claude concerning (1) potential non-lock-free fallback of std::atomicSteadyClock::time_point on non-64-bit targets and (2) the default-constructed time_point sentinel idiom. Both are low-signal observations; Codex independently found nothing to flag.

Reviewed commit: 01b8c79

💬 2 nitpick(s)

[thepastaclaw]

💬 Nitpick: std::atomicSteadyClock::time_point lock-freedom is not asserted

std::atomic<SteadyClock::time_point> is well-formed and will be lock-free on all 64-bit targets Dash ships (same width as the previous atomic<int64_t>). On platforms where steady_clock::duration::rep is wider than the largest lock-free integer, libstdc++/libc++ fall back to an internal spinlock — a silent regression from the previous atomic<int64_t>. All official Dash builds are 64-bit so this is not a real concern today; a defensive static_assert(std::atomic<SteadyClock::time_point>::is_always_lock_free) would catch any future regression.

_{source: ['claude']}

[thepastaclaw]

💬 Nitpick: Default-constructed time_point used as sentinel

The == SteadyClock::time_point{} check preserves the prior == 0 sentinel semantics and is correct in practice (a running process's SteadyClock::now() is never the epoch). It is slightly less self-documenting than std::optional<SteadyClock::time_point> or a named kUnset constant. Low priority — a short comment at the declaration site would be enough to document the idiom.

_{source: ['claude']}

[thepastaclaw]

Code Review

Clean mechanical refactor converting six Dash-specific call sites from epoch-based SystemClock arithmetic to monotonic SteadyClock time_points and chrono durations. Each site measures elapsed time (DKG cache expiry, InstantSend timing, net linger and cleanup intervals, DKG phase sleep), so SteadyClock correctly insulates them from NTP step adjustments. Both Claude and Codex independently verified no correctness, consensus, or build regressions; CodeRabbit also returned zero inline findings.

Reviewed commit: 01b8c79

No findings to report.