did:webvh: author whole-second versionTimes; created/updated selector contract (#127)

[moisesja]

Fixes #127

Revised after review rounds 1 and 2. Round 1 corrected future-time authoring and the false-success Deactivate path. Round 2 (5ef07b2) replaces the per-delay check with one aggregate monotonic deadline and corrects the documented failure threshold.

What

Implements issue #127's Option B + A for the created/updated truncation residual disclosed in PR #126:

• Whole-second authoring, no future time. Create truncates the current UTC instant. Update and Deactivate use the current whole second when it is strictly later than the supplied log head; otherwise they wait for the next authorable whole second to actually arrive. No authored versionTime is later than the sampled authoring clock.
• One aggregate two-second authoring budget. A single timestamp from the injected TimeProvider bounds the complete wait across every retry. Frozen or backward UTC cannot restart the budget, and an overslept timer cannot return success after the deadline. Eligibility at exactly two seconds remains allowed.
• Honest failure. If the next strictly increasing whole-second timestamp cannot be reached within the remaining aggregate budget, Update and Deactivate throw ArgumentException with a retry-after-the-clock-advances contract. Appending past a future-dated head without authoring future time is impossible; a maximum-timestamp head is permanently unappendable. Read-side future-skew enforcement remains did:webvh: resolver does not enforce the spec's future-skew bound on versionTime (MUST fail beyond tolerance) #131.
• Selector contract. DidDocumentMetadata.Created/Updated remain DID Core whole-second informational values. VersionTime/VersionId are the version selectors for imported logs with fractional timestamps.

For NetDid-authored logs, created/updated/versionTime coincide exactly, and serialized updated round-trips as ?versionTime= to the same versionId. Fractional imported logs still parse, validate, select, and reserialize at full precision.

Behavior changes

• Authored versionTimes no longer carry fractional seconds.
• Same-second Update/Deactivate calls throttle to approximately one write per second.
• Update and Deactivate fail when the next authorable whole second cannot be reached within one aggregate two-second monotonic budget, including stalled/backward UTC and timer oversleep.
• No public clock configuration was added; the injected clock remains an internal test seam.

Regression coverage

Test	Pins
Issue127_Update_WaitsForNextWholeSecond_NeverAuthorsFutureTime	exact boundary: eligibility at two seconds succeeds; authored timestamp is not later than the clock
Issue127_Update_FailsHonestly_JustBeyondBoundedWait	three-second initial wait is rejected
Issue127_WriteOperations_FailHonestly_WhenNextTimestampExceedsWait	Update and Deactivate reject a +2-minute head without waiting toward it
Issue127_FrozenUtcClock_EnforcesAggregateWaitBudget	frozen UTC cannot restart the aggregate budget; Update and Deactivate
Issue127_BackwardUtcStep_EnforcesAggregateWaitBudget	a backward UTC step spends the same remaining monotonic budget; Update and Deactivate
Issue127_OversleptTimer_EnforcesAggregateWaitBudget	eligibility cannot bypass the deadline after a three-second timer oversleep; Update and Deactivate
Issue127_Wait_PropagatesCallerCancellation	caller cancellation interrupts a non-firing wait
Existing Issue127/Issue117 authoring and imported-log tests	whole-second authoring, selector round-trip, exact fractional imported-log selection

Fail-first evidence: against d5dddf0, frozen UTC hit the guard instead of the promised domain failure and a backward step succeeded after 2.5 virtual seconds. Against the first aggregate-deadline implementation, both overslept-timer cases returned success after three monotonic seconds. All fail quickly on assertions and pass after the final fix.

Verification after adversarial fixes

• Release build: 0 warnings, 0 errors
• Tests: Core 418; Key 52; Peer 48; WebVh 437; Ethr 490; DI 20; W3C 233/233; Ethr integration 11 passed / 7 expected real-EVM skips — 1,709 passed total
• All five offline samples exit 0; git diff --check clean; W3C report timestamp churn restored
• WebVh suite: ~51 seconds before the virtual-clock cleanup, ~6 seconds after; the high-frequency 108-test class is ~256 ms

Adversarial review

Two independent round-2 lenses attacked deadline math and test/documentation fidelity. Confirmed findings were: eligibility-before-elapsed allowed success after an overslept timer, the docs described absolute target distance instead of reachability within the remaining budget, and stalled/backward loop coverage did not directly exercise Deactivate. All three were fixed and the full gate was rerun. Final verdict: clean.

Out of scope

• Read-side enforcement of did:webvh's future-skew rejection rule remains did:webvh: resolver does not enforce the spec's future-skew bound on versionTime (MUST fail beyond tolerance) #131.
• Timestamp serialization and fractional imported-log reading are unchanged.
• Other DID methods are unchanged; the full suite is the cross-method guard.

[moisesja]

Blocking review — not ready to merge

CI is green and all 429 WebVh tests pass locally, but the new tests validate NetDid against itself and miss the normative failure.

1. The implementation knowingly authors future timestamps (DidWebVhMethod.cs:763). A same-second update at 12:00:00.100Z after a 12:00:00Z head signs 12:00:01Z; bursts push that almost a minute ahead. The v1.0 Update algorithm requires the timestamp to be the retrieval time or earlier. Resolver clock-skew tolerance does not authorize the writer to manufacture future time. If whole-second identity is required, wait/throttle or fail until the next whole second exists; otherwise use the documentation-only option.

2. The deactivation “success” is false assurance (DidWebVhMethod.cs:690). For the test's +6-minute head, this code emits a deactivation at roughly +6m01s and returns Success = true. A conforming resolver must reject entries beyond its future tolerance under the Read rules, and witnesses must validate the candidate before approval. The test passes only because NetDid's validator omits that MUST-level check—the exact open defect tracked by #131. Once #131 is fixed, validation at the start of DeactivateCoreAsync rejects the input before this exemption is reached. A maximal representable timestamp also still blocks deactivation. Remove the “can never block” claim and define an honest failure/queued-retry contract; strict monotonicity means immediate effective revocation after an arbitrarily future head is impossible.

3. The tolerance argument is backwards. “No more than 5 minutes” is a recommended maximum, not a minimum every conforming resolver grants. A conforming resolver with a 30-second tolerance rejects NetDid's allowed +31-second head. The code comments, PRD, README, CHANGELOG, error message, and PR description all overclaim universal interoperability.

4. The 1-minute security fix is not regression-pinned (LogChainValidatorTimestampTests.cs:208). The rejection test uses a +6-minute head, which also fails under the old 5-minute budget; the burst test reaches only about +3 seconds. Reverting the constant from 1 minute to 5 minutes leaves every new test green. Add a stable between-budget case (for example +2 minutes), preferably with an injected clock and exact boundary tests, plus an assertion that authored timestamps are not later than observation/retrieval time.

The scope is coherent and the commit messages are clear. Test quantity is not the problem; the oracle is wrong. Please fix the timestamp design and reconcile it with #131 before merging.

[moisesja]

Thanks for the review — all four findings accepted; the design flaw was real. Fixed in 2942278 (verification/record in d5dddf0); PR body rewritten to match.

1. Future-time authoring — fixed at the root. You're right that resolver-side skew tolerance is leniency for reading, not writer permission; the Update algorithm's retrieval-time-or-before rule governs. GetNextVersionTime is now GetNextVersionTimeAsync: when the current whole second isn't strictly later than the head, it waits (bounded, 2 s max — covers same-second writes with margin) for the next whole second after the head to actually arrive, then stamps it. No authored versionTime is ever later than the authoring clock; a same-second burst throttles to ~1 write/second. The 12:00:00.100Z-after-12:00:00Z case now blocks ~900 ms and signs 12:00:01Z once 12:00:01Z exists.

2. Deactivation false assurance — exemption removed, honest contract instead. Agreed the exemption produced a "success" that conforming resolvers and witnesses reject, and that it only looked viable because of the read-side gap (#131). Update and Deactivate now fail with ArgumentException and an explicit retry-after-the-clock-catches-up contract when the head is ahead of the clock beyond the bounded wait; the "can never block" claim is removed everywhere, and the docs now state plainly that immediate revocation past an arbitrarily future head is impossible under strict monotonicity + no-future-authoring, and that a maximal-timestamp head is permanently un-appendable. This stays consistent with #131's end state: once read-side validation rejects future-dated logs, DeactivateCoreAsync fails at chain validation instead — same honest outcome, earlier boundary.

3. Tolerance argument — retracted. The "≥4-minute margin under every conforming resolver" reasoning inverted the SHOULD-maximum, as you said. All margin/budget claims are removed from the code comments, exception text, PRD, README, CHANGELOG, and PR body. The design no longer references resolver tolerance at all, because nothing future is ever authored.

4. Pinning — new oracle, injected clock, between-budget case. Authoring now reads time through an internal TimeProvider seam; tests use a deterministic auto-advancing clock. Added: exact boundary pair (head T+1s with clock at T → waits to exactly T+2s and succeeds; head T+2s → refuses), the +2-minute case for both Update and Deactivate (it sat inside the old 5-minute budget, so it is red under any revert to a minutes-scale budget), and per-entry authored ≤ observation time assertions in the burst/Create/boundary tests. Mutation check: reverting the wait to immediate future-stamping turns exactly the 5 new-oracle tests red — the previous suite stayed green under that revert, which was your point about the oracle. That lesson (my adversarial agents also validated the writer against NetDid's own reader, which lacks the #131 MUST) is recorded in tasks/lessons.md.

One operational note: WebVh suite wall time grew to ~51 s because real-clock tests' same-second writes now genuinely wait — the throttle working as designed; the boundary tests use the injected clock and stay instant.

[moisesja]

Re-validation round 2 — original findings closed, one new blocker

I re-validated d5dddf0 / substantive fix 2942278. All four original findings are genuinely closed:

• Update and Deactivate no longer stamp future time.
• Deactivate no longer returns false success for a far-future head.
• The backwards five-minute-tolerance argument is gone.
• The deterministic boundary, +2-minute, and authored-at-or-before-observation tests now kill the old design.

Focused tests pass 9/9, the complete WebVh suite passes 430/430, and current CI is green.

There is still one blocking defect in the replacement design:

Medium — the advertised two-second wait is not actually bounded

GetNextVersionTimeAsync checks each newly computed UTC-derived wait against two seconds, but it never tracks aggregate monotonic elapsed time. After every timer fires it starts the same budget again.

Concrete reproduction: valid head = T; TimeProvider.GetUtcNow() remains frozen at T while its timers fire normally. The helper waits one second, wakes, recomputes the same one-second wait, and repeats forever when the caller uses the default non-cancelable token. I ran this against the current private helper; a 2.5-second caller cancellation—not the documented two-second bound—was what finally stopped it. A backward UTC clock adjustment causes the same class of overrun.

This contradicts the code/README/PRD/CHANGELOG promises of “bounded at 2 seconds” / “each write may block up to 2 s.” Use one monotonic deadline established before the loop (TimeProvider.GetTimestamp / elapsed time), enforce its remaining aggregate budget on every iteration, and add frozen-clock, backward-step, and cancellation regression tests.

Documentation mismatch

The implementation bounds time until the next strictly increasing whole-second timestamp, not how far the head is ahead of the clock. The current wording says failure occurs when the head is ahead “by more than” two seconds. Its own boundary test disproves that: at clock T, head T+2s leads by exactly two seconds but is rejected because the next valid timestamp is T+3s, three seconds away. Reword the code comments, exception contract, README, PRD, CHANGELOG, and PR body around distance to the next authorable timestamp.

Non-blocking cleanup: the full unit suite now takes ~51 seconds because existing tests use real waits. The clock seam should be propagated through those fixtures so the authoring throttle does not impose a permanent 50-second test tax.

Verdict: not clean yet. The original design flaw is fixed; make the new wait truly bounded and correct the contract wording.

[moisesja]

Round-2 blocker fixed in 5ef07b2.

Specific changes:

• GetNextVersionTimeAsync now captures one monotonic start timestamp and enforces one aggregate two-second budget across the entire retry loop. Frozen or backward UTC cannot reset it.
• Elapsed time is checked before the eligible-success return. A timer that resumes after three monotonic seconds now fails even if UTC reached the target; eligibility at exactly two seconds still succeeds.
• Frozen UTC, backward UTC, and timer-oversleep regressions directly cover both Update and Deactivate. Caller cancellation during a non-firing wait is pinned separately.
• README, PRD, CHANGELOG, code comments, and the exception contract now say “cannot be reached within the aggregate budget,” not the inaccurate “head/target is more than two seconds away.”
• The high-frequency WebVh fixture now uses exact-second virtual time: the full WebVh suite dropped from ~51 seconds to ~6 seconds (437/437); its 108-test main class is ~256 ms.

Fail-first evidence was real, fast, and assertion-based: the pre-fix frozen clock escaped to the guard, the backward step succeeded after 2.5 virtual seconds, and the first aggregate implementation returned success after a three-second timer oversleep for both operations.

Post-adversarial gate: Release build 0 warnings/errors; 1,709 tests passed (W3C 233/233; 7 expected opt-in EVM skips); all five samples exit 0; git diff --check clean. Two independent adversarial passes found the oversleep ordering bug and wording/Deactivate-coverage gaps; all confirmed findings are fixed. Final verdict: clean. Please re-review the latest head.