fix(autotls): renewal never fires and renewBufferTime is applied twice

[rlve]

Summary

Fixes two independent defects in AutoTLS certificate renewal. Neither had test coverage.

Certificate expiry was compared against the wrong clock. asMoment built a Moment out of Unix epoch seconds, but Moment is monotonic (chronos defaults to asyncTimer = "mono", and its docs say a Moment's value has no direct meaning). So cert.expiry - Moment.now evaluated to the epoch value minus the machine's uptime, which is around 56 years on any real host. The renewal branch was never taken and certificates expired in place. asMoment is deleted. ACMECertificateResponse.certificateExpiry is already a DateTime, so the service now stores it unchanged and manageCert subtracts wall-clock now.

renewBufferTime was applied twice. The old code computed waitTime = timeUntilExpiry - renewBufferTime and then tested waitTime <= renewBufferTime, which reduces to timeUntilExpiry <= 2 * renewBufferTime. Renewal began two hours before expiry with the default one hour buffer, while the comment directly above promised one hour. The expression collapses to if timeUntilExpiry <= self.config.renewBufferTime and the comment goes, since the code now states it.

Affected Areas

• Other
  AutoTLS certificate service (libp2p/autotls/). Certificate renewal scheduling only. No transport, muxer or protocol code is touched.

Compatibility & Downstream Validation

No downstream validation was run. AutoTLS is opt-in through .withAutotls(), and AutotlsCert is not re-exported through libp2p.nim, so only projects that import libp2p/autotls/service directly can see the type change.

Impact on Library Users

AutotlsCert.expiry is now a times.DateTime instead of a chronos.Moment, and AutotlsCert.new takes a DateTime. Five call sites exist in this repo and all are updated here.

Renewal behavior fires at all now, where before it never did, and it starts at renewBufferTime before expiry rather than at twice that.

Risk Assessment

The renewal path has never run in production, so treat it as new code even though it is not. Nodes that previously served a certificate until it expired will now place real ACME orders against a rate-limited CA.

Additional Notes

Tests are in tests/libp2p/autotls/test_service.nim, with a new tests/stubs/acme_api_stub.nim that refuses every ACME request and records the URI. Each issuance attempt fails on its first request, so a recorded request is an attempted renewal.

The integration test's renewal half passed for the wrong reason. It overwrote expiry by hand before waiting, which stepped over the clock bug, and its follow-up certAfter.expiry > Moment.now was always true because the value was an epoch number. That assertion is meaningful now.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.42%. Comparing base (3e0ab20) to head (9e19807).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2933      +/-   ##
==========================================
+ Coverage   82.19%   82.42%   +0.23%     
==========================================
  Files         177      177              
  Lines       32369    32368       -1     
  Branches       12       11       -1     
==========================================
+ Hits        26606    26680      +74     
+ Misses       5763     5688      -75     

Files with missing lines	Coverage Δ
libp2p/autotls/mockservice.nim	99.84% <100.00%> (+0.16%)	⬆️
libp2p/autotls/service.nim	54.32% <100.00%> (+39.68%)	⬆️
libp2p/autotls/utils.nim	91.30% <ø> (+26.59%)	⬆️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR fixes AutoTLS certificate renewal scheduling so that renewal triggers correctly using wall-clock time (instead of monotonic Moment), and ensures renewBufferTime is applied exactly once. It also updates the AutoTLS certificate expiry representation from chronos.Moment to times.DateTime and adds targeted tests to cover the previously untested renewal behavior.

Changes:

• Replace monotonic-clock-based expiry handling with wall-clock DateTime and compute timeUntilExpiry against times.now().
• Fix renewal buffer logic so renewal starts at renewBufferTime before expiry (not 2 * renewBufferTime).
• Add a stub ACME API and new unit tests for renewal/no-renewal behavior; update integration test assertions accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/stubs/acme_api_stub.nim	Adds an ACME API stub that refuses requests while recording URIs to detect renewal attempts in tests.
tests/libp2p/autotls/test_service.nim	Adds unit tests verifying renewal fires near expiry and does not fire outside the buffer window.
tests/integration/test_autotls_integration.nim	Updates integration test to use DateTime expiry and wall-clock comparisons.
libp2p/autotls/utils.nim	Removes asMoment helper that incorrectly converted wall-clock DateTime into monotonic Moment.
libp2p/autotls/service.nim	Changes AutotlsCert.expiry to DateTime and fixes renewal scheduling logic.
libp2p/autotls/mockservice.nim	Updates mock certificate creation to use times.now() for DateTime expiry.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.