Skip to content

fix(buzz-cli): stop aged projects becoming permanently unmodifiable - #5111

Open
PJalgotrader wants to merge 1 commit into
block:mainfrom
PJalgotrader:fix/cli-timestamp-and-repos-delete
Open

fix(buzz-cli): stop aged projects becoming permanently unmodifiable#5111
PJalgotrader wants to merge 1 commit into
block:mainfrom
PJalgotrader:fix/cli-timestamp-and-repos-delete

Conversation

@PJalgotrader

Copy link
Copy Markdown

Problem

buzz projects mutations sign created_at = head.created_at + 1 and never consult the wall clock:

// crates/buzz-cli/src/commands/projects.rs
fn next_timestamp(head: &Event) -> Result<Timestamp, CliError> {
    head.created_at.as_secs().checked_add(1).map(Timestamp::from)
        .ok_or_else(|| CliError::Other("project timestamp cannot be advanced".into()))
}

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:

relay error 400: invalid: event timestamp too far from server time

The rejection is permanent, not transient — the stored head only ages further, so update, add-repo, remove-repo and delete are 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:

write at wall clock signed created_at stale by result
1786042474 1786042049 425s accepted
1786043482 1786042049 1433s rejected

The 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_clock test covers and which still passes unchanged — while tracking the wall clock in the ordinary case.

Trade-off, stated plainly

head + 1 was 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 delete

A kind:30617 announcement could be replaced but never retracted — buzz repos had no delete at all, so a published announcement was permanent. Added buzz repos delete --id <repo-id>, mirroring buzz projects delete: fetch the caller's own head, emit a NIP-09 kind:5 addressable tombstone for 30617:<self>:<id>, then re-query to confirm no head survived. 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}.

Testing

cargo fmt -p buzz-cli -- --check                                     exit 0
cargo test -p buzz-cli                                               exit 0  (329 passed, 0 failed)
cargo clippy -p buzz-cli --all-targets --all-features -- -D warnings exit 0

New unit tests: aged head, current head, future head, and u64::MAX overflow for both next_timestamp and next_announcement_timestamp, plus a test asserting the tombstone is kind:5 carrying exactly the 30617:<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 (backing repos bind and repos protect) still carries the same + 1 pattern 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.

`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>
@PJalgotrader
PJalgotrader requested a review from a team as a code owner August 6, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant