feat: add deterministic time and entropy support - #917
niteshpurohit wants to merge 3 commits into
Conversation
- Introduced `clocks.cpp` and `clocks.hpp` for managing monotonic and realtime clocks. - Added `laghu_test_time_entropy` library for testing deterministic time and entropy. - Implemented `ManualClock` and `DeterministicEntropy` classes for controlled testing. - Created tests for clock operations and deterministic entropy to ensure reliability. - Updated CMake configuration to include new sources and test targets. closes: #79
There was a problem hiding this comment.
🟡 Changes recommended
Resolve the entropy test portability failure and complete error-aware cancellation support.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds POSIX clock abstractions and deterministic time/entropy test utilities.
Changes:
- Adds monotonic and realtime clock APIs and implementations.
- Adds deterministic test clock and entropy support.
- Integrates new sources and tests into CMake.
- Unresolved: a critical portability issue in the entropy test target and an incomplete error-aware cancellation API.
File summaries
| File | Summary |
|---|---|
tests/support/time_entropy.cpp |
Tests clock and entropy behavior. |
tests/support/laghu_test_time_entropy.hpp |
Declares deterministic test interfaces. |
tests/support/laghu_test_time_entropy.cpp |
Implements deterministic clock and entropy utilities. |
src/core/private/laghu/core/internal/clock_operations.hpp |
Declares internal clock helpers. |
src/core/contract/laghu/core/deadlines_cancellation.hpp |
Adds clock-operation deadline support. |
src/core/contract/laghu/core/clocks.hpp |
Defines public clock APIs. |
src/core/clocks.cpp |
Implements POSIX clocks and conversions. |
CMakeLists.txt |
Adds sources and test targets. |
cmake/LaghuBuildIdentity.cmake |
Updates production build inputs. |
cmake/LaghuApiBoundaries.cmake |
Adds clock visibility coverage. |
Review details
Suppressed comments (1)
src/core/contract/laghu/core/deadlines_cancellation.hpp:90
- These overloads introduce an error-returning
ClockOperationsseam only forDeadline; the existingCancellationToken::require_active(deadline, clock)andCancellationSource::cancel_if_expired(deadline, clock)below still accept onlyMonotonicClock, which requires a non-Resultnow(). Consequently callers using the new clock table cannot exercise deadline cancellation, and clock-read failures cannot be propagated through those APIs. Add matching error-aware cancellation overloads (or defer exposing this seam until the whole deadline/cancellation surface is covered).
[[nodiscard]] Result<void> require_not_expired(const ClockOperations& operations) const noexcept {
const auto is_expired = expired(operations);
if (!is_expired.has_value()) {
return std::unexpected{is_expired.error()};
}
if (*is_expired) {
return std::unexpected{deadline_error()};
}
return {};
}
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate issues remain involving production clock wiring and deterministic entropy integration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/core/private/laghu/core/internal/clock_operations.hpp:8
- The OS-backed table is only exposed from this private header and the repository has no production caller for
default_clock_operations(); the only use is the new test. Consequently, production deadline/cancellation code cannot obtain the adapter implemented inclocks.cppand must supply or reimplement its own clock callbacks, which leaves the production-clock part of the feature unused. Please wire the default table into the production path or expose a supported factory in the public clock contract.
[[nodiscard]] const ClockOperations& default_clock_operations() noexcept;
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
clocks.cppandclocks.hppfor managing monotonic and realtime clocks.laghu_test_time_entropylibrary for testing deterministic time and entropy.ManualClockandDeterministicEntropyclasses for controlled testing.closes: #79