fix(buzz-cli): stop aged projects becoming permanently unmodifiable - #5111
Open
PJalgotrader wants to merge 1 commit into
Open
fix(buzz-cli): stop aged projects becoming permanently unmodifiable#5111PJalgotrader wants to merge 1 commit into
PJalgotrader wants to merge 1 commit into
Conversation
`next_timestamp` advanced `created_at` off the observed head only
(`head.created_at + 1`) and never consulted the wall clock. The signed
timestamp therefore stayed frozen near the project's creation and drifted
further from real time as the project aged. Once it fell outside the relay's
accepted timestamp window, every mutation was rejected with
relay error 400: invalid: event timestamp too far from server time
and the rejection was permanent rather than transient: the stored head only
ages further, so `update`, `add-repo`, `remove-repo` and `delete` were all
refused alike. An affected project could no longer be modified *or* removed
by anyone, including its author.
Advance to `max(now, head.created_at + 1)` instead. The result is still
strictly greater than the observed head — including when the head sits in the
future, which the existing ordering test covers — while tracking the wall
clock in the ordinary case.
Trade-off, recorded in the doc comment: `head + 1` also gave free lost-update
detection, since a writer working from a stale read landed at or below an
intervening writer and was refused as dominated. Tracking wall clock removes
that backstop. Closing it properly needs a compare-and-swap on the observed
head rather than a timestamp rule.
Also add `buzz repos delete --id <repo-id>`, which had no equivalent at all —
a kind:30617 announcement could be replaced but never retracted. It fetches
the caller's own head, emits a NIP-09 kind:5 addressable tombstone for
`30617:<self>:<id>`, and re-queries to confirm no head survived, mirroring
`buzz projects delete`. Scoped to signer-self, so another author's
announcement at the same `d` tag is never touched.
Both delete commands now include the tombstone `event_id` in their output,
per the documented CLI contract that all writes return
`{event_id, accepted, message}`.
Note for reviewers: `repos bind` and `repos protect` still carry the same
`+1` timestamp pattern in `build_updated_repo_announcement`. Left untouched
here to keep this change reviewable; happy to follow up.
Co-authored-by: Fizz (Buzz agent) <fizz@buzz.invalid>
Signed-off-by: Pedram Jahangiry <pjalgotrader@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
buzz projectsmutations signcreated_at = head.created_at + 1and never consult the wall clock:The signed timestamp stays frozen near the project's creation and drifts further from real time as the project ages. Once it falls outside the relay's accepted timestamp window, the write is rejected with:
The rejection is permanent, not transient — the stored head only ages further, so
update,add-repo,remove-repoanddeleteare all refused from then on. An affected project can no longer be modified or removed by anyone, including its author.This is not theoretical. Measured against a live relay:
created_at1786042474178604204917860434821786042049The error message points at the clock, which sends people hunting for NTP drift. The local clock was verified correct to within one second in both cases.
Fix
Advance to
max(now, head.created_at + 1).The result is still strictly greater than the observed head — including when the head sits in the future, which the existing
next_timestamp_returns_head_plus_one_when_head_is_ahead_of_wall_clocktest covers and which still passes unchanged — while tracking the wall clock in the ordinary case.Trade-off, stated plainly
head + 1was also providing free lost-update detection: a writer working from a stale read landed at or below an intervening writer's timestamp and was refused as dominated. Tracking wall clock removes that backstop — a delayed writer now lands above the intervening write and silently replaces it.Closing that properly needs a compare-and-swap on the observed head, not a timestamp rule. This PR takes the trade because the current failure is a certainty (aged projects permanently bricked) and the race is not, but it is a real trade and it is recorded in the doc comment rather than glossed over. Happy to discuss if maintainers would rather see the CAS first.
Also:
buzz repos deleteA kind:30617 announcement could be replaced but never retracted —
buzz reposhad no delete at all, so a published announcement was permanent. Addedbuzz repos delete --id <repo-id>, mirroringbuzz projects delete: fetch the caller's own head, emit a NIP-09 kind:5 addressable tombstone for30617:<self>:<id>, then re-query to confirm no head survived. Scoped to signer-self, so another author's announcement at the samedtag is never touched.Both delete commands now include the tombstone
event_idin their output, per the documented CLI contract that all writes return{event_id, accepted, message}.Testing
New unit tests: aged head, current head, future head, and
u64::MAXoverflow for bothnext_timestampandnext_announcement_timestamp, plus a test asserting the tombstone is kind:5 carrying exactly the30617:<pubkey>:<d>coordinate. Two existing stability tests (subcommand_names_are_stable,subcommand_counts_are_stable) updated for the new subcommand.Verified end to end against a live relay. Three events that had been stuck and unremovable — one kind:30621 project aged well past the window, two kind:30617 announcements — were deleted with the patched binary, and all three coordinates then returned no live head. The aged project is the exact case the old code could not recover from.
Not included
build_updated_repo_announcement(backingrepos bindandrepos protect) still carries the same+ 1pattern and has the same latent failure. Left out to keep this change reviewable — glad to follow up in a separate PR.Retracting an announcement removes the announcement only; any relay-hosted bare repository storage behind it is an operator concern and is not reclaimed by a kind:5.