fix(cli): stamp repo announcement updates with max(head+1, now) - #4509
fix(cli): stamp repo announcement updates with max(head+1, now)#4509redirwin wants to merge 1 commit into
Conversation
build_updated_repo_announcement stamps replacement announcements at the observed head's created_at + 1 to stay monotonic over the head it was derived from. The relay's ingest gate rejects events more than ±15 minutes from server time, so once an announcement is older than that window every update built from it — repos protect set, protect remove, and bind — is rejected with 'event timestamp too far from server time', making protection rules permanently unchangeable. Stamp updates with max(head + 1, now) instead: still strictly after the observed head (a peer-clock-ahead head within the drift window keeps winning by +1), but never in the past, so the relay accepts it. Adjusts the two tests that pinned the old head+1 behavior on stale heads and adds a regression test for the future-head monotonicity case. Fixes block#2876 Fixes block#4431 Fixes block#4432 Signed-off-by: David Irwin <david@redirwin.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 699773eae2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .as_secs() | ||
| .checked_add(1) | ||
| .ok_or_else(|| CliError::Other("repository timestamp cannot be advanced".into()))?; | ||
| let next_created_at = bumped_head.max(Timestamp::now().as_secs()); |
There was a problem hiding this comment.
When a CLI process builds from a stale announcement—such as after a delayed query response, replica lag, or a concurrent update—using its later wall-clock time makes the reconstructed event newer than the intervening head. replace_parameterized_event only compares (created_at, id), so the relay accepts this event instead of returning the conflict handled by validate_write_response; because the tags were copied from the stale event, this can silently erase newly added protection rules or restore an obsolete channel binding. Refreshing an old timestamp therefore needs a server-side conditional/CAS mechanism rather than allowing now to leapfrog an unobserved head.
Useful? React with 👍 / 👎.
|
max(head+1, now) is the right stamp for the relay drift window — the fresh-head monotonic test covers the leapfrog case well. |
2 participants
Summary
build_updated_repo_announcementstamps replacement announcements at the observed head'screated_at + 1(an anti-leapfrog guard), while the relay's ingest gate rejects any event more than ±15 minutes from server time (buzz-relay/src/handlers/ingest.rs). Once an announcement is older than that window, every update built from it —repos protect set,repos protect remove, andrepos bind— is rejected withevent timestamp too far from server time, so branch protection becomes permanently unchangeable 15 minutes after a repo is announced.This stamps updates with
max(head + 1, now)instead: still strictly after the head the update was derived from (a peer-clock-ahead head within the drift window keeps winning by +1), but never in the past, so the relay accepts it.Reproduced live today on a 0.4.x relay: a byte-fresh
protect setagainst a day-old announcement was refused; re-announcing viarepos create(which stamps wall clock) and re-runningprotect setwithin the window succeeded — consistent with the diagnosis in #2876.Related issue
Fixes #2876
Fixes #4431
Fixes #4432
No existing fix PR found (searched open PRs for "timestamp announcement" / "created_at repos").
Testing
head + 1behavior on stale heads (created_at100) to assert wall-clock stamping.update_of_fresh_head_stays_monotonic_over_it: a head 100 s in the future still yieldshead + 1, preserving the monotonicity guarantee the original comment protects.cargo test -p buzz-cliwas not runnable on the authoring machine (Windows, no Rust toolchain; hermit shims are POSIX-only) — relying on CI for the run. The change is confined to one timestamp expression plus tests; flagging this explicitly rather than claiming a local pass.