mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-12 01:07:46 +00:00
main
2831
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Claude
|
db6b81eb04 |
feat(nip56): declare pointer hints on report, chat, classifieds and channel events
Quartz's PubKeyHintProvider / EventHintProvider / AddressHintProvider are the kind-agnostic answer to "what does this event point at" — they let a caller walk an event's references without knowing which tag name a given NIP chose (`p` vs `P` vs `member` vs `moderator`). Measured against the 248k-event corpus in commonTest, 84 of 403 event classes implement one, covering ~95% of all pointer edges. This closes the four largest remaining gaps. ReportEvent (1984) carried the most undeclared edges of any kind — 14,244 — and they are the negative trust signal that a web-of-trust projection most needs. Its tag classes also predate the modern layout, so they are brought up to the structure used by e.g. NIP-88 polls: - ReportedAuthorTag now implements PubKeyReferenceTag, ReportedEventTag implements GenericETag, and all three tags carry a relay hint. - Adds parseKey / parseId / parseAddressId / parseAsHint companions. Fixes a latent bug while doing so. NIP-56 predates the convention that slot 2 of a pointer tag is a relay hint — it put the report type there — so both layouts are in the wild. The old reader passed slot 2 straight to ReportType.parseOrNull, which despite its name never returns null and maps anything unrecognized to OTHER. A modern `["p", <pubkey>, "wss://relay/"]` tag therefore became an OTHER report and masked the event-level default. The new shared ReportTagLayout disambiguates by shape (a slot that parses as a relay URL is a hint, never a type) and falls back to the event-level default when a tag names no type of its own. Emitted tags are unchanged: assemble() still writes the legacy `[name, id, type]` form unless a relay hint is supplied, since many clients still read the report type out of slot 2. Also renames ReportedAuthorTag.pubkey to pubKey to satisfy PubKeyReferenceTag, updating the four call sites. Coverage over the corpus goes from ~95.3% to ~98.5% of pointer edges. What remains is GiftWrapEvent's recipient p-tag (deliberate — it is the store owner key and handled separately) and PrivateDmEvent's e-tags. |
||
 Vitor Pamplona
|
7041980996 |
negentropy: let a caller decline an id before the download REQ
negentropySync names every id the relay has that the caller lacks, then fetches all of them. There is no point between those two steps where a caller can say "not that one" — onEvent is the first hook, and by then the body has already crossed the wire. That is a real cost for a mirror. A store keeping only the newest version of a replaceable event refuses every relay's older copy, and negentropy re-offers those copies on every sync because they are genuinely absent from the local id set. Measured on a downstream mirror against relay.damus.io, nos.lol and relay.primal.net: three passes over kinds 0/3/10002 produced 5, 18 and 29 REPLACED rejections, the same events re-downloaded each time. Adds an optional `wantId: ((HexKey) -> Boolean)?` to negentropySync, negentropySyncOrFetch and negentropySyncFanOut, consulted for each id before the REQ, plus a `skipped` count on the three result types. Default null keeps every existing call byte-identical. Three decisions worth stating: - The gate runs on a whole reconcile round's ids, BEFORE they are chunked into fetch batches. Gating after the chunking keeps the batch count and shrinks every batch instead — at the density this exists for, a fetchBatch of 500 becomes a hundred REQs of five ids each, turning a bandwidth saving into a latency regression. - `skipped` is reported apart from both `downloaded` and `needCount`. needCount stays the honest protocol diff whether or not the caller fetched it, and without a separate number an operator cannot tell a predicate that does nothing from one that eats everything — both are silent. - keep() returns an empty list, never null. A nullable return invites `gate?.keep(ids) ?: ids`, which reads as "no gate, keep everything" and means "everything was declined, so send everything". That elvis turned the fully-declining case into a full download during development; the non-null contract removes the trap rather than documenting it. The gate does not cover a window handed to onUnreconcilableWindow: that is drained over REQ, which names no ids before streaming bodies. Documented on both the base function and the combinator. |
||
  Vitor PamplonaandGitHub
|
b259382652 |
Merge pull request #3888 from vitorpamplona/fix/concord-followups
feat(concord): close the CORD-05/06 invite lifecycle — Invite List, re-mint on Refounding, revocation |
||
|
Vitor PamplonaandClaude Opus 5
|
70c53a8fcd |
fix(concord): make the invite-list writes actually durable
A high-effort audit of the branch found that the durability guarantees the
previous commits claimed were not the guarantees the code provided. Three of
these are in the code written to close the last review, and they defeat
exactly what those commits set out to fix.
`INostrClient.publish` returns Unit — it queues an event and never reports
acceptance; `publishAndConfirm` is the confirming variant. So every
`runCatching { publish(...); true }` was true whenever local signing worked.
That made minting's "record the link before handing out the URL" gate
decorative, and made revoke worse than decorative: it reported success for a
tombstone no relay stored, then recorded the kind-13303 tombstone, whose
merge drops the entry — destroying the only `signer_sk` that could ever
retire the link while the link stayed live. Both paths, and the Refounding
re-mint, now confirm.
`fetchAll` returns an empty list on cannot-connect / CLOSED / idle-timeout,
so "a relay served us and had nothing" and "nobody answered" were the same
observation. Reading the second as "no list yet" reintroduced, one layer
below, the wipe the null-vs-empty work existed to prevent. `fetchAllWithHooks`
gains a `doneOut` of per-relay terminal reasons plus `anyRelayServed()`, and
both clients now only treat an empty read as an empty list when a relay
actually reached EOSE.
The rest:
- `drainConcordRekeys` discarded the entry `adoptConcordRoot` now returns, so
only the account that *launched* a rotation re-minted its links. An admin
who was merely re-keyed left every link they had handed out on the dead
root, and anyone stranded behind one could never recover — which is the
branch's headline goal, holding only for the rotator.
- The join-time ban gate fetched the Control Plane from `bundle.relays`
alone (stale metadata refuses a community we can plainly reach) with a
single un-paged REQ (truncated at the relay's filter cap, so a missing
older ban edition fails the gate OPEN, re-admitting the account it exists
to refuse). Now unions in the relays that just served the bundle, and pages.
- `decodeOrNull` failed the whole document for one structurally incompatible
entry. Since null now means "refuse to write", that converted the old
silent data loss into a permanent write lock on a coordinate that never
ages out. Unreadable entries are carried verbatim instead, so they neither
block the account nor get dropped on re-encode.
- The list read took the newest event of any kind and then cast, so one stray
event at the coordinate read as "unreadable" forever. Filters by kind first.
- The Refounding refresh did one full round trip per link, serially, inside a
user-visible rotation. One pooled REQ over every link signer, then
concurrent confirmed re-mints, classified per coordinate so one link's
tombstone cannot decide another's status.
Verified on a tablet: mint, list, revoke and the cross-client refusal still
work end to end — and with the community relay killed, revoke now reports
"The link couldn't be revoked" and leaves the entry intact, where before it
would have claimed success and destroyed the key.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Vitor PamplonaandClaude Opus 5
|
fc3f181184 |
feat(cli): add amy concord revoke — retire an invite link (CORD-05)
The reading half of revocation already existed: `classify` has always resolved a `vsk=9` tombstone to `Revoked`, and Amethyst's join honours it. Nothing anywhere could *produce* one, so a leaked link could only be outrun by a Refounding — rotating the whole community to retire one URL. `ConcordInviteBundle.buildRevocation` emits the grave the spec describes and Armada's `buildRevocationEvent` already publishes: kind 33301 at the link's own `["d",""]` coordinate, empty content, `["vsk","9"]`, signed by the `link_signer` secret. Empty content is the interop contract, not an omission — there is nothing to encrypt when the point is that no bundle key opens anything. `amy concord revoke COMMUNITY TOKEN|URL` takes either the shareable URL a creator actually has to hand or the bare token. It publishes the wire tombstone FIRST and records the kind-13303 tombstone second, which is the inverse of minting and deliberate: the list entry holds the only copy of the `signer_sk` the publish needs, and a merge drops a tombstoned token's entry terminally. Recording first and then failing to publish would leave the link live with its signer gone and no way left to retire it. A failed list write is recoverable by comparison and is reported rather than swallowed. Also fixes a revocation bypass in amy's own `join`, found while testing this: it opened the first wrap that decrypted instead of classifying the coordinate, so a relay still serving a stale copy alongside the grave would have handed out a revoked link. It now resolves per CORD-05 §2 like Amethyst does, and can say which of revoked/expired/unreadable/absent it hit instead of reporting everything as `not_found`. Verified end to end against a local relay: revoking flips the coordinate to vsk=9 with empty content, the link is refused from that moment on, a second revoke reports `already_revoked`, and a Refounding afterwards moves the surviving link while leaving the grave alone — the first real proof of the tombstone-skip in the refresh path, which until now had only unit coverage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vitor PamplonaandClaude Opus 5
|
eb8c812690 |
fix(concord): close ten review findings in the CORD-05 invite path
A high-effort review of the invite work found ten correctness bugs, nine confirmed and one plausible. All are fixed here; the on-device pass on a tablet proved the three that are observable through the UI. The load-bearing one: the kind-13303 Invite List is replaceable, and both clients merged a patch onto a base that silently degraded to EMPTY whenever the read failed — an unanswered relay or a bunker signer declining one decrypt was enough. Republishing that destroys every `signer_sk` it could not read, and those secrets cannot be regenerated, so every outstanding link is orphaned at a dead epoch. `decode` is now `decodeOrNull` and `decrypt` returns null, so "I could not read it" is distinguishable from "it is empty", and the write aborts rather than overwriting. The rest: - `join` is now ban-gated on both clients. A Refounding re-mints every outstanding link onto the new root, and an ex-member keeps the URL and its token forever, so the rotation meant to expel them handed them the new keys instead. Fails closed on an unreadable plane. - Android's Refounding re-read the entry from `liveCommunities` straight after adopting the new root, but that flow decrypts asynchronously, so every link was re-minted onto the epoch just left. `adoptConcordRoot` now returns the entry it wrote. - amy's refound folded the fresh bans locally and then never used them, re-draining from relays instead; a relay slow to echo them back would produce a new epoch whose roster never banned anyone. - Link refresh rebuilt the bundle from scratch, stripping expiry, channel grants, icon and label; it now moves the link's own current bundle and changes only the epoch's key material. - Refresh also re-posted over revocation tombstones, silently un-revoking a retired link. - The 13303 coordinate is (13303, me, "") — one list per account — but was read and written on per-community relays, forking it into divergent versions that newest-wins then collapsed. Now account-outbox only. - Minting returned the URL even when recording the link failed, handing out a link that could never be refreshed. It now fails closed. - amy's refound had no equivalent of Amethyst's recipient cap, leaving the attacker-writable half of the union unbounded. - amy's store dropped the banked epoch's `controlRoot` on the round-trip, losing the staff write key that rebuilds the anti-rollback floor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Vitor PamplonaandClaude Opus 5
|
293ffd0bc5 |
feat(concord): implement the CORD-05 Invite List (kind 13303), wire-compatible with Armada
The previous commit kept link secrets in amy's local store, which made link
refresh work but only for that one client. The spec already defines where
they belong, and Armada implements it, so this replaces the local field with
the real cross-client document.
Kind 13303, replaceable, NIP-44-encrypted to self — the creator's private
bookkeeping:
{ "entries": [ { "token", "signer_sk", "community_id", "url",
"label?", "created_at", "expires_at?" } ],
"tombstones": [ { "token", "community_id" } ] }
`token` is both the link's unlock secret and the merge key; `signer_sk` is
what lets any of the creator's clients re-sign at that link's addressable
coordinate. Armada types both the entry and the tombstone as
`[k: string]: unknown`, so unknown keys are contract: the codec preserves
entry-, tombstone- and document-level residue, and re-encoding never deletes
another client's data.
Merge is by token, read-merge-write rather than overwrite — the list is
replaceable and per-creator, so two devices minting concurrently would
otherwise destroy each other's `signer_sk`, which is unrecoverable. A token
tombstoned on either side stays dropped, so a stale device cannot resurrect
a retired link.
Registers 13303 in EventFactory (without it the kind deserializes as a plain
Event and every typed read fails), and points amy's mint and Refounding
refresh at the list instead of its own store.
Semantics were taken from the spec and confirmed against Armada's observable
behaviour — read for semantics only, never copied: Armada is AGPLv3 and
Amethyst is MIT.
Verified against a loopback geode: `concord invite` publishes an encrypted,
untagged 13303; a Refounding reads it back, refreshes the live links, and a
member with no role who never posted — unfindable by any rotation — recovers
epoch 0 → 1 through the link he already held.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Claude
|
7ca4fbab33 |
Cover the dense-second step-past the new guard sits in front of
The step-past path had no test, and it is the one thing the ignored-cursor guard could plausibly break: both cases reach the same `delivered == 0` branch. They are told apart by WHERE the events landed — a dense boundary second returns them AT the boundary, so `aboveBoundary` stays 0 while `received` is 1 and the guard holds its fire; only a relay answering ABOVE the boundary is not paging at all. Scripted end to end: a second the relay's page cap can only ever return the head of, the step strictly past it, and the empty EOSEd page below. Also proves the documented cost is still paid rather than silently changed — the unreachable tail of that second is lost, and `downloaded` says so. `event()` grows a nonce so two events can share one `created_at`; the id was derived from the timestamp alone, which collapsed them into one event and made a dense second impossible to script. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPSzniNdvJxkhRsCe1QcyT |
||
|
Claude
|
3b7ffcd06c |
Make a paged walk terminate: floor the cursor at 0, stop when a relay ignores it
`fetchAllPages` could not end against a relay that does not honour `until`.
Found in production on purplepag.es, which holds twelve `kind 10002` events
stamped `created_at = 0` and treats `until <= 0` as *no* `until` — so the page
below them comes back with its five hundred NEWEST events. None of those
matches the filter's own `until`, so the page delivers nothing, which read as
"the boundary second is too dense to page", stepped one second lower, and
asked the identical unanswerable question again.
Measured against the live relay: ~5.5 pages a second, 500 events fetched and
discarded on each, an EOSE on every single page, `until` marching one second
further negative every time, for as long as the process ran. A cold walk pulled
1,490,010 real events in ~10.8 minutes and then never returned, so the caller's
coverage was never recorded and the next boot re-walked all of it. Not a rate
limit: ~1,000 consecutive pages drew no NOTICE, no CLOSED and no throttling.
Two guards, both at the points where the cursor moves:
- The cursor floors at zero. `created_at` is unsigned, so nothing can exist
below epoch 0: a cursor that would step under it has reached the bottom of
the time axis and the walk is DRAINED. `until = 0` is still asked — it is a
legal query and the boundary re-fetch for epoch-stamped events — only going
BELOW it ends the walk. This also keeps a negative `until` off the wire,
which relays disagree violently about: measured across five, one CLOSEs the
subscription with a parse error, three answer a NOTICE and then never EOSE,
and one drops the bound and serves its newest events. The floor is applied
on the advance path too, not just the step: `pageMinTs` is an event's own
`created_at`, so one relay serving a negative timestamp is enough to drive
the cursor under zero, and clamping rather than stopping would not help —
such an event never equals the boundary, so it dodges the dedup and returns
on every page.
- A relay that ignores the cursor is UNPAGEABLE. When a page delivers nothing
and every event it received was NEWER than the `until` it asked for, the
relay is not paging at all and stepping one second lower just repeats the
question. `aboveBoundary == received` is what tells this apart from a
genuinely dense boundary second, whose events are AT the boundary rather
than above it. UNPAGEABLE is deliberate and conservative: it proves nothing
about what the relay holds, so no coverage claim can be built on a page the
relay never really answered.
This second guard is the structural fix. Giving the filter a `since` does not
substitute for it: the step path decrements `until` without regard to `since`,
so a cursor-ignoring relay still walks from the window floor down to 0 — up to
~1.5 billion pages. A `since` only helps when the relay honours it, and then
only because the empty page arrives as a drain.
Three scripted tests cover both guards, and CursorTerminationProbe dials the
five relays that found this (opt-in, `-PprodRelayBench=1`, asserts nothing).
Against the live relays after the change: purplepag.es ends UNPAGEABLE in one
page and 2.4s with the cursor never going under 0, where it previously ran 244
pages to `until = -243` without ending; the other four still DRAIN unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HPSzniNdvJxkhRsCe1QcyT
|
||
|
Vitor PamplonaandGitHub
|
2ecbb66cdb |
Merge pull request #3885 from vitorpamplona/claude/concord-soft-ban-vulnerability-rbjjet
fix(concord): close the soft-ban authority holes (audit A1–A4, B1, B2, B4) |
||
|
Claude
|
f1241e891b |
fix(quartz): compaction must carry the authority-gated head, not the chain head
Findings from an independent review of this branch (a different model, per CONTRIBUTING-WITH-AI.md). Two were real, and the first is a regression this branch introduced. compactControlPlane picked its per-entity head with a bare structural chain walk, which is worse than the raw max-version it replaced. With no floor, foldEntity anchors at the lowest-version edition carrying no `prev` — and after a PRIOR compaction the genuine head's `prev` dangles into a trimmed epoch by design. So a forged `version = 1, prev = null` decoy outranks a real v50→v52 chain, and because nothing in this path checks a signature it became the entity's entire carried-forward state. A forged empty banlist would have erased every ban at the next Refounding. Reproduced, then fixed by selecting the owner-rooted authority-gated head — the same edition ConcordCommunityState.fold would seat, so the new epoch starts where the old one left off, and an unprivileged author cannot influence the choice at all. recoverStrandedConcordCommunities derived its new ban gate with `?.isBanned(..) == true`, which reads "not banned" when the session does not exist yet or its first fold has not landed. The sweep runs on the revision tick, so a banned member's own client would have hit that window on cold start and recovered itself — the exact bypass the gate exists to stop. It now fails closed and retries on the next sweep. Also from the review: resolve() now warns when the ban fixpoint exhausts its pass cap without settling, instead of silently returning a roster folded under a mask that no longer matches its banlist; and banGate stops lowercasing the same author three times. Two review findings are accepted rather than fixed, and recorded on the PR: the anchor tie-break picks the lowest rumor id before testing whether that candidate connects (pre-existing, and changing it is consensus-affecting), and non-owner moderators now need a resolved roster before a verb succeeds, which is the intended fail-closed trade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Claude
|
9cc19c60ca |
fix(concord): three defects found auditing this branch's own changes
Self-review of the diff before merge. One of these is a real correctness bug in the B2 fix as shipped. The resolver stopped after two passes, which left the mask a pass resolved UNDER disagreeing with the banlist that pass produced — and the disagreement is not cosmetic. A moderator whose only ban came from an admin the owner banned concurrently is released by pass 2, correctly; but pass 2 had already dropped her editions, because she was on pass 1's list. The fold then reported her as a moderator in good standing whose promotions had silently vanished, and did so deterministically, so she never got them back. resolve() now iterates until the mask and the resulting banlist agree. The mask cannot simply be assumed to shrink, which is why this is bounded rather than proven monotone: masking an author can strip a THIRD member's role, which drops their rank to roleless, which lets a junior BAN holder who previously could not reach them ban them after all. The loop keeps its last pass if it does not settle within the cap — still better than the two-pass answer, and it always terminates. Real communities settle on the first or second pass, and the skip-if-no-banned-author guard means most never enter the loop at all. boundRecipients could exceed its own budget while reporting that it had capped at it, because the roster was added with filterTo before the budget loop ran. The roster now goes in whole deliberately — it is owner-rooted and cannot be padded from outside, and dropping an admin to make room for a stranger inverts the point — and the log reports what was actually kept and dropped. mintConcordInvite started requiring a session, which the owner's own invite button would not have on a cold start, since sessions are built asynchronously off the joined list. The owner is proven by the community id, so they are read off the entry; everyone else still needs the folded roster. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Claude
|
54d3bfc84a |
perf(quartz): skip the resolver's second pass when it cannot change anything
The two-pass ban-aware fold doubles resolve(), which runs once per held epoch in controlFloorsLocked plus once in fold, on a client that re-folds the whole buffer from scratch on every Control Plane change. So the second pass is now skipped unless a banned member actually authored a Control edition — not merely when the banlist is empty. Bans overwhelmingly land on plain members who hold no role and write nothing, and for those pass B is provably identical to pass A. Armada's fold checks the same condition. Measured over ConcordCommunityState.fold (throwaway benchmark, not committed; 226 and 2059 editions, 200 reps after warmup). Pass A is byte-for-byte the old algorithm, so the single-pass rows are the before-numbers: 226 eds, no bans 1457 us 226 eds, 20 bans, none authored 994 us 226 eds, 20 bans, one authored -> pass B 1881 us 2059 eds, no bans 2194 us 2059 eds, 50 bans, none authored 2001 us 2059 eds, 50 bans, one authored -> pass B 5697 us 2059 eds, with floors (B1's arm) 2015 us So the common case is free, and B1's chain-first compaction arm is not measurable — the floored fold matches the unfloored one. A banned staffer costs ~2-3x, which is the price of the fix and is paid only under the attack. The audit records this, plus the standing opportunity it surfaced: we have no fold memoization where Armada does, which predates this work and would absorb the pass-B cost too. Not done here — that is a change to make on its own merits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Claude
|
4e99aafb59 |
fix(quartz): honor the banlist against the Control Plane itself
B2 in docs/concord-soft-ban-audit.md, plus B4's bound and the audit's status pass. hasPermission was ban-aware; the resolver's own ROLE/GRANT/BANLIST gates were not, and could not be as written — the roles/grants fixpoint settled before `banned` was computed at all. So half the Control Plane honored a ban and half was blind to it, and a banned staffer still holding control_root kept the whole roster: banning everyone beneath them, revoking the surviving moderators, retiring the roles under them, and minting a fresh un-banned npub that passed every ban-aware gate and finished the job. resolve() is now a bounded two-pass where authority only ever shrinks. Pass A resolves as before and yields a candidate banlist; pass B re-resolves with every author on it treated as holding no authority. Two passes always, so it terminates by construction, and mutual bans cannot oscillate because the rank rule makes them unreachable — only someone who strictly outranks you may ban you, and you cannot outrank them back. A chain-local rule would not have worked: forking the banlist at genesis means no parent ever mentions the ban and §4's re-heal union carries it in regardless, so the rule is a whole-pass mask rather than a per-edition check. This cascades, deliberately: every edition a banned member ever authored is dropped, grants included, so banning an admin also demotes everyone that admin promoted. That is the literal reading of CORD-04 §4 and it is what kills the sockpuppet, but a legitimate promotion by a later-banned admin vanishes with it and has to be re-issued. Both the cascade and its blast radius are pinned, and the trade-off is written up in the Armada report as the answer to its own open row 3 — which also widens the divergence recorded there: we now drop editions they honor wherever a privileged member was banned. B4: the Refounding recipient set is capped. allMembers() is the Guestbook ∪ observedAuthors ∪ the roster, and the first two are unbounded and attacker-writable, so each throwaway npub someone posts from became one more mandatory blob in the next Refounding — the attack inflating the cost of its own remedy. The owner-rooted roster is kept first and anything dropped is logged, never silently truncated, because a dropped member is stranded. The nine escalation reproductions now assert the fixed behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Claude
|
54c412da7a |
fix(quartz): stop a stray edition from pinning a Control entity forever
B1 in docs/concord-soft-ban-audit.md, the worst item on the list: one edition at version = Long.MAX_VALUE permanently pinned its entity to the author's content, for every client holding a floor for it, with no way back. The floor rose to MAX_VALUE, no honest edition could exceed it, and a Refounding that dropped the poison fell back to EntityFloor.known — the poison. Authored in the tests by a current, legitimately granted moderator: no ban, no sockpuppet, one ordinary permission bit. The chain walk was never the weakness; it advances only to head.version + 1 citing the head's hash, so a fresh joiner was untouched. The compaction arm was: it trades contiguity for cross-epoch tolerance, which left VERSION as the only contest an edition had to win. Two changes. The arm now tries the floor-anchored chain first and falls back to the raw-version bootstrap only when nothing connects, so a stray never wins a fold where the honest chain is present. And the bootstrap will not follow a jump of more than MAX_COMPACTION_VERSION_JUMP above the floor — a compacted head is legitimately ahead by a chain's worth, not by 2^63 — so the version space cannot be exhausted in a step. A new test pins the tolerance the arm exists for, so the bound cannot later be tightened into breaking CORD-06 §3. compactControlPlane picked its per-entity head by raw highest version too, which made an honest rotator the delivery mechanism: a disconnected stray never joins the chain but won that comparison, and was re-wrapped into the new epoch as the entity's whole history, where fresh joiners anchor on it. It now picks the chain head, keeping foldEntity's fresh-joiner fallback for the dangling `prev` a prior compaction leaves behind. The three reproductions now assert the fixed behaviour. The banlist's escape hatch (a floor-less chain walk plus the re-heal union) is kept and still pinned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Claude
|
3a53292993 |
fix(concord): close the soft-ban holes reachable from the shipping app
Part A of docs/concord-soft-ban-audit.md — the ones a banned user reaches by tapping a button, no custom tooling involved. A1. mintConcordInvite checked that the account was writeable and that we held the community, and nothing else, while its button was the one control on the screen with no gate at all. A member banned a minute ago could hand out a working link to the community they were removed from, and every account they invited arrived as a fresh un-banned npub. Now gated on CREATE_INVITE, both in the verb and on the button. Worth noting the bit was not enforced anywhere else: the fold gates the INVITE_* Control entities on it, but a link's bundle is a standalone kind-33301 published outside the Control Plane, so this check is the only one that exists. A3. Every moderation verb checked isWriteable() plus the Control write key — which is a spam gate, never authority (CORD-02 §5) — and left the real decision to whichever composable drew the button. Those gates then tested effectivePermissions, which ignores the banlist, so a banned staffer kept seeing the controls; the editions were dropped by everyone's fold, making them silently no-op, which this codebase elsewhere calls out as worse than absent. Ban and Remove survived only because a second, unrelated condition happened to route through the ban-aware canActOn. Authority now lives in the action layer behind isAuthorizedFor(), so a caller from desktop, amy or a future screen inherits it, and every authorization test uses hasPermission. refoundConcordCommunity's own guard was ban-blind outright and now rank-checks each removed member too. A2. The recovery sweep merges us onto any higher-epoch bundle found at our stored invite_ref, and an ex-member keeps that link's unlock token forever — so our own background timer walked a removed member back into the epoch a Refounding had rotated them out of. isStranded/mergeForward now take bannedAtCurrentEpoch as a required argument rather than leaving it to callers, because a caller that forgets it inverts the mechanism. The liveness half of that finding (nothing re-mints at a stable coordinate, so legitimate recovery never fires either) needs a spec answer and is untouched here. A4. Typing heartbeats are filtered on both ends, so a banned member stops announcing that they are typing messages nobody will see. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Vitor PamplonaandClaude Opus 5
|
5136a97b16 |
Return the walk's outcome instead of signalling a drain by callback
`onDrained` was the wrong shape. It reported the one ending a coverage caller happens to need and threw the rest away, so a CLOSED and an idle timeout still arrived indistinguishable from a clean finish — the very conflation this branch set out to remove, just moved one step along. `fetchAllPages` now returns `PagedFetchResult(downloaded, end)`, where `end` names every way the loop can stop: DRAINED, LIMIT_REACHED, IDLE, CLOSED, CANNOT_CONNECT, UNPAGEABLE. `drained` stays as a shorthand on the result so the meaning lives in one place. A caller can no longer ignore the reason by accident, and the two failure endings are now reportable rather than silently swallowed. I argued for the callback on the grounds that ~25 call sites use the `Int`. That was overstated: most call it as a statement and never touch the return. Six needed a `.downloaded`, all mechanical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016TNy5BsU9NErXYa3UNGTeJ |
||
|
Vitor PamplonaandClaude Opus 5
|
37d715830b |
Let a drained paged walk close the leg below it
A paged band records the events it SAW, never the range it asked for, so `legs()` can only ever say "walked this far" and keeps re-asking the leg below the floor. Against a relay whose corpus for one kind simply starts later than the others' that leg is unclosable: it comes back empty every cycle, an empty fetch earns no band, so the floor never moves. Measured on a live mirror of five NIP-65 indexers, three were in that state — kind 10002 re-walked from the beginning of time to Feb 2023 forever, because relay lists did not exist before then. The missing fact is why a page ended. `fetchAllPages` treated all three terminal signals as one bare `Unit`, so an empty page could not be told apart from silence or a CLOSED. It now carries a PageEnd, and reports `onDrained` only for the one ending that proves absence: an EOSE on a page that returned nothing, with no filter capped by its `limit` and no `search` filter in play (both stop the walk short of the corpus). An idle timeout is silence, not an answer, and recording it would durably claim coverage the relay never served. A callback rather than a richer return type: ~25 call sites across quartz, geode and downstream use the `Int`, and none should have to change to learn a fact they do not want. It follows `onNewPage`'s shape. `SyncCoverage.record` takes `drained` and marks the kinds that produced evidence complete — which required completeness to move from Band onto Span. It could not stay on the band: once kinds diverge, `legs()` hands each group its own ask, so a walk that drained `kinds: [10002]` proves nothing about kind 0, and a band-level flag set from that leg would claim both. That is the same over-claim per-kind spans exist to prevent, one level up. `Band.complete` stays as a DERIVED all-kinds-complete, so both state files keep writing the flag a pre-per-kind reader expects, and read it back as every span's default. A kind the walk never saw at all still earns nothing: there is no interval to anchor a claim to, and inventing one would be the over-claim again. Tests: five in NostrClientFetchAllPagesDrainTest pinning EOSE-empty vs silence vs CLOSED vs cannot-connect vs a fulfilled limit, and five in SyncCoverageTest for per-kind completeness, widening, and the deeper-floor escape hatch that a drain must not defeat. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016TNy5BsU9NErXYa3UNGTeJ |
||
|
Claude
|
1e6cda712d |
docs(concord): audit the soft-ban and Control Plane attack surface
Collects the findings from this branch into docs/concord-soft-ban-audit.md, each marked Verified (a test reproduces it, named) or Read (follows from the code, untested), with a suggested order of attack. Adds the reproduction for the one finding that was still unverified, and it did not hold up the way it was first described. Version inflation does not poison the anti-rollback floor through the chain walk — that walk advances only to head.version + 1 citing the head's hash, so a fresh joiner is untouched. It goes through the COMPACTION ARM: once a client holds a floor and the entity is in the epoch snapshot, the head comes from bootstrapHead, which is highest-version at or above the floor with no prev, no hash and no contiguity. Version is then the whole contest and Long.MAX_VALUE wins it permanently — the floor rises to MAX_VALUE, no honest edition can exceed it, and a Refounding that drops the poison falls back to EntityFloor.known, which is the poison. That makes it the worst item on the list: unrecoverable, and authored in the tests by a current, legitimately granted moderator — no ban, no sockpuppet, one ordinary permission bit. compactControlPlane picks per entity by raw max version too, so honest rotators carry it into every future epoch. The banlist escapes only because AuthorityResolver folds it on a floor-less chain walk and re-heals the union, so an honest ban still lands. That accident is all that separates this from a permanently unmoderatable community, so it is pinned by its own test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Claude
|
41a035034b |
test(quartz): pin the hand-crafted routes out of a Concord ban
The ban/unban verb is not the threat model — a malicious client writes editions directly, so what matters is which routes the FOLD refuses. Three more, all of them ones the UI would never author. Two are refused, and it is worth pinning why, because neither is refused by the rule you would expect. Removing yourself from the banlist is caught by the delta rule's strict outranking (nobody outranks themselves), so the sharper attempt does not remove anything: it forks the banlist at genesis, or builds a private chain, that simply never mentions him, at a version high enough to win the head fold. There is then nothing to remove and the rank rule never fires. What catches it is CORD-04 §4's re-heal — the owner's edition is not on the forged head's back-chain, so it is unioned back in as a concurrent ban. The union is load-bearing security here, not just convergence. The third works. A §3 compaction re-wraps one edition per entity and the ROTATOR picks it, so a rotator can decline to carry the banlist forward; every edition it serves is genuine and no signature check can see the omission. A banned member cannot rotate — drainConcordRekeys gates the rotator on the ban-aware hasPermission — but the puppet from the previous commit is not banned and can. EntityFloor is the entire defense, so the community splits: clients that already folded the ban refuse the rollback, fresh joiners have no floor and see no ban. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Claude
|
f2160f6264 |
test(quartz): pin what a soft-banned Concord staffer can still do
CORD-04 §4 row 3 of docs/concord-banlist-rank-conformance.md was left open as "a genuine fixpoint-ordering question". This reproduces what that gap costs. ConcordCommunityState.fold gates METADATA/CHANNEL/INVITE through authority.hasPermission (`!isBanned && ..`), but ROLE, GRANT and BANLIST are gated inside AuthorityResolver.resolve by holdsManageRoles / bitsOf / effectivePermissionsOf, none of which consult the banlist — and none of which can, as written, since the roles/grants fixpoint settles before `banned` is computed. So half the Control Plane honors a ban and half is blind to it. A banned member who still holds control_root therefore keeps the roster: they revoke the surviving moderators, retire the roles beneath them, ban everyone they outrank, and — since a role edition they author is honored — mint a fresh, unbanned npub at the next position down. That npub passes every ban-aware gate, so it tombstones the channels (terminal ids), rewrites the metadata, and, being a non-banned BAN holder, is accepted as a rotator by drainConcordRekeys. The tests assert the CURRENT, VULNERABLE behaviour so it cannot regress silently; each ESCALATION assertion is to be inverted, not deleted, when the ordering rule lands. Two companions pin what the fix must preserve: self-unban and puppet-unban both stay refused, closed already by the delta rank rule. Also records why a chain-local fix is insufficient — forking the banlist at genesis dodges any "was the author banned by this edition's parent" rule, and §4's re-heal union carries the rogue bans in anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj |
||
|
Claude
|
4b49032e52 |
fix(quartz): merge probe verdicts into the record they replace
Follow-up to #3882, which made RelayReachabilityStore edit a relay's kind:30166 rather than rebuild it. toDiscoveryEventTemplate was the remaining co-writer: it builds from the verdict alone, so a consumer following its own KDoc — sign with the monitor key, insert — wipes whatever else is on that address, undoing the merge for exactly the writer #3882 set out to protect. It now takes the current record and carries across every tag the verdict did not measure, on the same rules: - Ownership is per writer, and this one measures more than the store does. A write probe determines `pow` from the OK message, so `R pow` is its own finding and must not be re-dated from an older record. Without a ReadWriteVerdict it never exercised the write path, so the same tag is somebody else's and is carried across untouched — hence the hasReadWrite flag rather than a fixed set. - Both polarities of each requirement are owned, so an update cannot leave the record asserting `pow` and `!pow` at once. - created_at is max(requested, current + 1): a store enforcing replaceable semantics rejects anything not strictly newer, and the probe would be lost with nothing to show for the round trip. The parameter defaults to null, so every existing caller keeps today's behaviour and the change is additive. Test plan: ./gradlew :quartz:jvmTest — 4,081 tests, all passing. Three new cases in RelayProberFlowTest: a foreign tag and an unmeasured `R pow` surviving a probe without a write verdict, a stale `R pow` being replaced when the write path DID run, and the stamp landing past the record it replaces. |
||
|
Claude
|
56eec420e4 |
fix(quartz): correct the merge's ownership, guard and timestamp rules
Second review pass on the merge itself. Three of these reverse choices
made in the previous commit; the reasoning there was wrong.
The created_at cap is gone. Capping the bump to a window past `now`
looked prudent and was worse: a record already further ahead than the cap
can then never be replaced, because every stamp we are willing to write
is older than what is stored, so the relay's live/dead verdict freezes
until the wall clock catches up — 24h in the test that shipped asserting
that behaviour as correct. It did not even buy the freshness it claimed:
snapshot() selects on `since` alone, so a future-stamped record sits
inside the window either way. A record ahead of the clock is a defect in
whatever produced it; this class's job is to keep updating it.
Ownership is now the full liveness set on every write — `n`, all three
rtt types, and both polarities of `R auth` — rather than the narrower
per-path sets. A 30166 carries ONE created_at, so a tag carried across is
re-dated as a current measurement: keeping a rtt-read from an earlier
observation beside a fresh rtt-open republishes a stale latency as
today's, which aggregators rank on, and RelayObserver documents exactly
how wrong a queued rtt can be. Carrying `R auth` forward was worse still
— only an observation can clear it and that needs the connection the flag
discourages, so it became permanent, a regression against the rebuild
this PR replaced. Owning only the positive auth form also let `R !auth`
survive while `requirement("auth")` appended the opposite, publishing a
record asserting both.
The per-relay guard no longer swallows. It caught Exception, which
includes CancellationException, so a shutdown flush wrapped in
withTimeout — the pattern RelayMonitor.close() prescribes — could not
abort and would grind through every remaining relay. And a caught failure
went nowhere: collectUnreported() has already cleared the observation
flags by then, so the measurement is lost for good while the run reports
success. Cancellation now propagates, every relay is still attempted, and
the first real failure is rethrown once the loop finishes.
Also corrected a comment: the 16,507-relay figure is measured in
RelayObserver, not RelayProber, and the SQLite ceiling is verified here
rather than quoted — 32,765 `d` values pass, 32,766 fails.
Test plan: ./gradlew :quartz:jvmTest — 4,078 tests, all passing. Four new
cases: a future-stamped record still updatable, a stale rtt-read not
re-dated, an auth wall not outliving its observation, and a run whose
writes all fail reporting failure instead of success.
|
||
|
Claude
|
a81c43e1d4 |
fix(quartz): address code-review findings on the record merge
Five issues from a review pass on the previous commit, all in the new merge path. currentRecords() bound one SQL host parameter per relay with no chunking. Callers pass the whole relay universe — RelayProber's own measurement puts that at 16,507 — and a bundled SQLite refuses past 32,766 variables. The throw lands BEFORE anything is written, so an entire probe run's records are lost rather than one relay's. Chunked at 500, in the same range as the author chunking elsewhere. The created_at bump had no ceiling, so a stamp that once landed in the future was sticky: every later edit derived from the bad value and never re-anchored to now. Such a record never ages out of snapshot()'s TTL window (an isKnownDead verdict that can never expire) and relays enforcing future-timestamp limits reject every publish for it. Capped at 60s past now — a pathological record now costs the updates made while the clock catches up, and heals itself. writeOne owned all three rtt names but only ever measures rtt-open, so the reachable path deleted rtt-read/rtt-write taken by an observation — the exact silent loss this change exists to stop. It now owns rtt-open alone; only the dead path clears them all, which liveness semantics require. writeObserved owned the whole R tag name but can only prove `auth`, so it erased `R pow` and friends written by RelayProber. Ownership is now per VALUE, which is why edit() takes a predicate rather than a set of names. The read-modify-write spans a store round trip and IEventStore exposes no read inside a transaction, so a concurrent writer to the same address can still win the race and get our stale insert rejected. That cannot be closed at this layer; it is now isolated per relay so one loser does not end the loop and silently drop every relay after it. Test plan: ./gradlew :quartz:jvmTest — 4,075 tests, all passing. Four new cases, one per fixable finding: a flush wider than one chunk writing every relay, a far-future record not being pushed further ahead, a reachable update keeping latencies it never measured, and an observation clearing only `auth`. |
||
|
Claude
|
9ce0fb9559 |
fix(quartz): update NIP-66 relay records instead of rebuilding them
A kind:30166 is addressable, so RelayReachabilityStore keeps exactly one record per (monitor, relay) — but it is not necessarily the only thing writing per-relay knowledge under that identity. Both write paths built the record from their own tags and inserted it, so every update deleted whatever else was in that slot. Observed while adding a "this url is an alias of that one" tag alongside the monitor: `[d, n, rtt-open]` became `[d, redirect]` on our write, and the monitor's next observation turned it back into `[d, n, rtt-open]`. Nothing looks wrong at any point — the event still signs, still parses, still reads as a valid NIP-66 record. It just says less than it did, and the reader downstream cannot tell. Writing is now an edit: read this monitor's current record, carry across every tag the writer does not own — including tags this version of quartz has never heard of — and replace only what it measured. `n` and the three `rtt-*` types are owned by both paths, so a dead update still clears a stale rtt and liveness keeps meaning what it meant. `R` is owned only by the observation path, which is the one that learns whether a relay challenged us; writeOne leaves it alone rather than deleting what it cannot re-measure. Only OUR records are merged. Folding another monitor's tags into a document signed with this key would republish their claims as ours. The timestamp is now `max(now, current + 1)` rather than `now`. A store enforcing replaceable semantics REJECTS a record that is not strictly newer than the one it replaces, and two writers inside the same second — or a peer whose clock runs ahead — are ordinary. That is not theoretical: it silently swallowed a repair pass in the caller that found this bug, which reported success having written nothing. The reads are batched per call rather than per relay, so a flush over N relays costs one extra query, not N. Test plan: ./gradlew :quartz:jvmTest — 4,071 tests, all passing, including four new cases in RelayReachabilityStoreTest covering a foreign tag surviving an update, an update against a record stamped an hour ahead, a dead update clearing its rtt, and another monitor's record not being merged. ./gradlew :quartz:spotlessApply clean. |
||
|
Vitor PamplonaandGitHub
|
33e30423a2 |
Merge pull request #3873 from alexgleason/cord2
feat(concord): staff-held control_root write-gates the Control Plane (CORD-02 §2) |
||
 davotoula
|
c8e3573812 |
feat(resourceusage): relay churn and traffic attribution counters
The ledger could say how much relay data the app moved, but not why. It
counted completed connections and a single undifferentiated byte total,
so "1.65 GB/day across 6,600 connects" could not be broken down further,
and relay.connfails was being read as a dial-failure count when it also
fires for mid-session drops of successful connections.
Adds, all as counters with no behaviour change:
relay.dials / relay.disc real dial and disconnect counts
relay.life.<bucket> connection-lifetime histogram, bucketed to
straddle STABLE_CONNECTION_IN_SECS
relay.verb.up/down.<verb> the byte totals split by protocol verb
relay.purpose.<p>.* REQ bytes, inbound bytes and frames by the
SubPurpose that asked, read off the
ExplainedFilter that already travels on the
filter
relay.subs.* REQs sent, closed, replayed after connect,
and re-sent for an already-open subscription
relay.events.* inbound EVENT frames and how many carried an
event already delivered
relay.notice.<reason> NOTICE frames by an allowlisted reason
relay.hs / relay.gap the transport's own handshake timing, and
everything before the request went out
relay.trigger.<cause> which decision asked for a reconnect
|
||
|
Claude
|
dc03209bb5 |
fix: silence Kotlin override-parameter-name and redundant-!! warnings
LocalCache implements both Dao and ICacheProvider, which disagreed on the parameter names of getOrCreateUser (hex vs pubkey) and getOrCreateAddressableNote (address vs key), so every override warned about named-argument mismatches. Align both interfaces on pubkey/address and update the implementations that used the other name. Also drop the non-null assertions the compiler already smart-casts away in LimitsPolicy.capLimits and RelayProberFlowTest, and match the WebSocketListener parameter names in NegentropyStallRepro. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017X7C797zGYsiui5yj1JQcY |
||
|
Claude
|
3f087e5c60 |
Quartz: give SyncCoverage's persistence a typed band key
`export`/`restore` handed back `Map<String, Band>` where the string was the INTERNAL key — `"<relay-url> <filter-json>"`. That is fine for a file layer that writes the key back verbatim, and nothing else. A layer that wants its own layout — one object per relay, or per filter, or nested by both — had to split the key apart, and the separator was folklore it could only learn by reading this class. Two of them now do. So the key is a pair, with the joined form kept here as `encode`/`decode` for a file that does want one key per line. geode keeps its format byte-for-byte and stops pattern-matching on somebody else's string. It is also faster on the path that matters. `key()` built a new string per lookup, so a `legs()` over a fan-out COPIED the filter's json — tens of thousands of characters for an author-scoped filter — once per relay per cycle, then hashed all of it, since a freshly built string carries no cached hash. The pair hashes two halves it already holds: the url, and the fingerprint instance the cache above it already returns. No behaviour change: the same pairs key the same bands, a file written before this reads back through `decode`, and the format on disk is untouched. |
||
 Alex Gleason
|
d7226b7021 |
concord: implement CORD-02 §2 staff-held control_root write gate
Track the spec change in concord2 96f0647 (CORD-01 Write-Restricted Streams) and its review follow-up bbc67b6: the Control Plane's stream key splits, the signer keypair deriving from a new control_root held only by the owner and staff (concord/control-signer), while every member holds the delivered control_pk to subscribe and verify, reading under the community_root-derived read key the old concord/control derivation still yields. - ControlPlaneKeys models the three views of an epoch: staff (signer held), member (address held, read-only), legacy (pre-split, one key). ConcordStreamEnvelope gains write-restricted wrap/open forms; wrapping without the write key fails loudly instead of missigning. - Genesis mints the control_root beside the community_root; invites, the kind-13302 join material, and held roots carry control_pk (and, staff-side, control_root) since a split address is held, never derivable. A same-epoch list merge fills either side's missing key material, so a holder's own second device converges (CORD-02 §8); across epochs it is never inherited, being stale by construction. - Promotion delivers the secret inside the staff-making Grant itself: GrantEntity.control_wrap, a 40-byte epoch_be8‖control_root pairwise ciphertext (ControlRootWrap), adopted only when it derives to the held control_pk — fails closed. PIN_MESSAGES claims frozen bit 11 and the staff set is the six Control-writing bits, a normative list. - Refoundings roll the pair: base rekey blobs are now width-per-form (72 channel/legacy, 104 member +control_pk, 136 staff +control_root), a mismatched staff pair is refused, and a legacy 72-byte base blob is honored when reading old rotations, never minted anew — so a legacy community upgrades as a side effect of its next base rotation. - Sessions, the plane registry, the subscription planner, amy, and the app read the plane by held address per epoch; moderation verbs take ControlPlaneKeys, and both amy and the app refuse a Control write without the secret rather than throwing out of the envelope. Rank and possession diverge for as long as a promotee waits on delivery, so the app gates its mod affordances on the write key too. Stored control material only ever backstops its own epoch. The account drains staff-making Grants on the revision tick. Possession stays a spam gate, never authority: every edition is still judged by its sealed actor's rank in the owner-rooted Roster. |
||
|
Vitor PamplonaandGitHub
|
35f96a936d |
Merge pull request #3872 from vitorpamplona/perf/outbox-no-quadratic-publish
Stop the outbox getting slower with every publish |
||
|
Vitor PamplonaandClaude Opus 5
|
36a79d74de |
Stop the outbox getting slower with every publish
PoolEventOutbox kept its pending publishes in an immutable map and rebuilt
it on every send:
eventOutbox = eventOutbox + Pair(event.id, PoolEventOutboxState(...))
That copies every entry, per event, so publishing N events copies
1 + 2 + … + N. The relay-set bookkeeping alongside it was the same shape —
needsToUpdateRelays() and updateRelays() each walk every value, and both ran
on every send.
Measured on a bulk push against a relay with ~970k entries resident: 22.7ms
per event, of which ~20.5ms was the outbox. The store fetch feeding the same
loop cost 1.2ms and the configured pace 1ms, so the map was ~90% of the
budget — and the rate decayed as the backlog grew, 45.6 -> 44.6 -> 43.2 ev/s
across three windows.
The map is now LargeCache (ConcurrentHashMap on JVM/Android), so put/get/
remove are O(1) and the cross-thread visibility that @Volatile republishing
provided comes from the map itself.
The relay set is now maintained asymmetrically, because the two directions
are not equally expensive. Adding is exact and cheap: union the event's own
relays, touching the flow only when it actually changes. Deciding a relay may
LEAVE means asking whether any remaining entry still wants it, which is
inherently O(outbox) — so it is swept every SWEEP_EVERY removals, and always
when the outbox empties. Keeping a relay a little too long costs an idle
connection; scanning a million entries to retire it promptly costs the push.
The test asserts the SHAPE of the cost, not a wall-clock budget: equal
windows at the start and end of a 60k-publish run, where the late window
carries ~29x the backlog. Halves were not enough — over 20k publishes the
average backlog only grows 7k to 17k, a 2.4x expected ratio that hid inside
JIT noise, and the first version of this test passed against the very code it
was written to catch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Claude
|
8555309492 |
negentropy: audit fixes over the windowing change
A read-back over the two commits before this, rather than a failure — which is the only way these would have turned up, since every one of them lives on a path that runs when something has already gone wrong. **accept() is no longer single-threaded, and its comment said it was.** "Both phases run sequentially, so no concurrent access" was true right up until a paged window started running on a reconciler coroutine while the sync's own delivery consumer was still calling accept(). An unguarded HashSet between two coroutines can corrupt, and the delivered counter can lose updates. Now behind a Mutex — with onEvent kept INSIDE it, because callers are promised it never runs concurrently with itself and some of them keep unsynchronised state in that callback. pagedWindows becomes an AtomicInt for the same reason. **The kotlinx cap parse could take down the whole frame.** `.jsonPrimitive` throws on an object or array, so a relay putting something structured in the fourth element would have failed the NEG-ERR and lost the reason with it — where before that element existed, anything extra was simply ignored. `as?` restores that. Both mappers are now tested against a structured fourth element as well as a string one. **Int overflow in the split fan-out.** `mine + ceiling - 1` wraps when a window holds close to Int.MAX events, which is reachable on exactly the corpora this targets; done in Long now. **The count-driven split cuts N ways, not two.** The work queue is FIFO, so halving means every internal node's count() runs before the first NEG-OPEN goes out: on a corpus ~30,000 windows wide that is ~30,000 store counts of dead time with nothing downloading. Cutting into ceil(count/budget) pieces (capped at 32) reaches the same corpus in about three levels instead of fifteen, and pieces that guess wrong are re-split by the same rule. **The budget moves by CAS.** With reconcileConcurrency > 1 two reconcilers adjust it at once, and a lost SHRINK is the one that costs something real: the next window is then asked at a size the relay has already refused. |
||
|
Claude
|
18998c897c |
negentropy: size reconcile windows from the caller's own index
A NEG-OPEN is all-or-nothing at both ends of the wire and neither end can see the other's size. The relay half has been handled since windowing landed — refuse, halve, retry. The client half has not: localEntries has to hold every matching (created_at, id) pair before the first NEG-OPEN goes out, so peak memory is a property of the CORPUS, not of the window. On a multi-million-event filter that list is the sync's high-water mark, and it is built even when the sync then splits into windows that each touch a fraction of it. NegentropyLocalIndex is that half. A caller whose store answers by range passes an index instead of a list, and the engine reads a window's worth at a time. count() is what makes it work: a window is sized BEFORE the round trip, so entriesFor() is only ever asked for something bounded. Callers that pass a list are unchanged — internally the list becomes an index that sorts once and binary-searches per window, exactly what the engine did inline before. targetWindow (0 = off, the old behaviour) turns the two signals into one loop. Our count splits a window before asking; their refusal shrinks the target — straight to the relay's stated cap where there is one, halved where there isn't — and windows that reconcile in one piece grow it back toward, never past, the caller's number. Neither side knows anything about the other and the same work queue absorbs both, which is what makes it adapt rather than need tuning. peerCap carries the relay's number back out, so a caller can persist it and start the NEXT sync at a window that fits. The local pre-split deliberately does NOT count against MAX_WINDOWS: that backstop exists for an overflow loop that never converges, while this split is driven by a number that provably halves with the range. Also here, because it is the same loop: page the window that overflowed rather than the whole filter. A second dense enough to exceed the cap is reachable — created_at has second granularity and is author-controlled — and negentropySyncOrFetch used to answer it by re-paging everything, including every window that had already reconciled cleanly. reconcileWindows now takes onUnreconcilableWindow and hands that window over; the sweep carries on with the rest of the range, so a dense second costs that second. Raw negentropySync/negentropyReconcile callers that pass no hook still get the exception, unchanged. pagedFallback stays conservative and now means "any part of this range came over REQ rather than a reconcile", with pagedWindows saying how much — the distinction matters to anyone recording coverage, since a paged walk booked as a completed reconcile would claim a range nothing compared. The existing over-cap test is updated rather than deleted: its ten events share one created_at, so the whole filter IS the un-reconcilable window — same events, now via the window path instead of by abandoning the sync. Its sibling test, that raw negentropySync still throws, is untouched. |
||
|
Claude
|
d6b8a54d8a |
NEG-ERR: state the relay's max_sync_events on an overflow refusal
A client that is refused for matching too much has exactly one thing to decide — how much smaller to ask next time — and no way to find out. NIP-11 has no field for max_sync_events, so the only route to a window the relay will answer is to guess and halve, and every wrong guess costs the relay the snapshot scan that produces the refusal. strfry already states the number in its rejection text; this makes it a first-class part of the frame. ["NEG-ERR", <subId>, <reason>] unchanged, still what NIP-77 says ["NEG-ERR", <subId>, <reason>, <cap>] when the refusal is about size Both mappers write the fourth element only when there is one, so a refusal with nothing to state is byte-identical to before, and both tolerate a non-numeric fourth element from someone else's relay. NegErrMessage.statedCap reads either form — the wire field or strfry's "(2431002 > 1000000)" prose — but only for a refusal that is about SIZE. That gate is the point of the property: a rate limit or a quota can carry numbers too, and it does not shrink when the window shrinks, so a client that mistook one for a cap would shrink its windows forever against a relay that has no size limit at all. The relay side sends its own configured cap for the same reason it is cheap: it had to know the number to refuse. |
||
|
Vitor PamplonaandGitHub
|
a6f41072a9 |
Merge pull request #3863 from vitorpamplona/feat/suspend-subscription-onevent
Suspend the incoming-message chain down to SubscriptionListener.onEvent |
||
|
Vitor PamplonaandClaude Opus 5
|
bb95cad98b |
Audit fixes: one clock per record, no shared mutable list, pin the file format
Deep-audit pass over the branch. Nothing here changes what a band claims; these are the defects that pass tests and bite later. - record() read the clock twice PER KIND. A 40-kind map took 80 readings, and worse, a span's floor and ceiling were judged against two different instants — so a span could be accepted at one end and rejected at the other on a clock tick. One read, one instant, for the whole call. The aggregate path had the same double read and now shares it. - legs() handed the SAME MutableList instance to every Filter in a group, publishing its accumulator through a public return value. Filters are treated as immutable everywhere else; this keeps that true by construction rather than by nobody having tried yet. - The state file's round trip was asserted only for the fields, never for the behaviour. Three tests now pin it: per-kind spans survive a restart AND still narrow per kind afterwards; the ALL_KINDS sentinel survives its negative key through toString/toInt; and a pre-split file (min/max, no spans) loads as the claim it always was. Plus the rollback contract — `min`/`max` must remain the OUTER edges, since a binary from before per-kind spans reads those and would otherwise skip ground it has not covered. Checked and found sound, recorded so the next reader need not re-derive it: ConcurrentMap.snapshot() copies, so export() cannot be mutated under a writer; Band is immutable (widen() copies its map), so a shared Band across threads is safe; merge() keeps old.fullAt, preserving the re-walk clock across widening; and coveringWindow does NOT regress — a paged band gave >1 leg before this change too, and a reconciled band still collapses to one leg and narrows the shared snapshot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Vitor PamplonaandClaude Opus 5
|
a3fac4fc08 |
Suspend the incoming-message chain down to SubscriptionListener.onEvent
A consumer that cannot suspend has to block, and blocking here deadlocks
the whole client.
Measured on a mirror built against this library, twice, ~13 minutes after
each start: all 64 shared coroutine workers parked in `runBlocking` beneath
`trySendBlocking`, called from the websocket message callback. The consumer
draining that channel needed threads from the same pool to reach its store,
so it could never make room, so the producers never woke. Every stream, the
health reporter, all of it stopped, at 2% CPU with a healthy, idle backend.
A full queue was the symptom; producers eating the threads the drain needed
was the cause.
The coroutine context was already there — BasicOkHttpWebSocket has always
processed messages inside `scope.launch { for (message in incomingMessages) }`
— so the only thing forcing a blocking hand-off was that the hops in between
were declared non-suspend. Now they are not:
WebSocketListener.onMessage
RelayConnectionListener.onIncomingMessage
PoolRequests/PoolCounts/PoolEventOutbox.onIncomingMessage
SubscriptionListener.onEvent
fetchAllPages / negentropy accessories' onEvent parameter
A consumer that fills its buffer now suspends and releases its thread rather
than holding it, which is the same reasoning BasicOkHttpWebSocket already
documents for keeping its own channel UNLIMITED so a slow consumer cannot
block OkHttp reader threads. This extends it one layer down.
BLE is the one transport whose callback genuinely cannot suspend — the
platform hands notifications to a plain callback — so BleNostrClient gets
the same treatment the websocket transport already had: an UNLIMITED
hand-off channel so the BLE stack is never blocked, drained by ONE coroutine
so message order survives the boundary.
Tests that drove these entry points directly now do so from `runTest`, or
from `runBlocking` where the call sits inside a raw thread or Runnable that
models a platform callback.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Vitor PamplonaandClaude Opus 5
|
74145ee8f3 |
Code-review fixes: two ways per-kind spans could be recorded and not used
Both found in the review pass over the previous commit, both the same shape — a band written that no lookup can reach, or reaches wrongly. - Spans for kinds the filter never named were stored as given. Inert for legs(), which only looks up the filter's own kinds, but NOT for Band.minCreatedAt — and that is what SyncCoverageFile writes as its rollback-compat `min`/`max`. A relay answering with more than it was asked for (or a caller whose containment check runs against a different filter than the band is keyed by) would push that floor below anything the filter's kinds support, so a binary from before per-kind spans would read the file and over-claim. The fix, undone through the compatibility path it added. - observedByKind on a filter that names NO kinds was stored per kind, while legs() for such a filter reads only ALL_KINDS. The band was recorded, persisted, and never consulted: a resume that silently did not resume. Collapsed to the union, which is the only claim a kind-less filter can make. Why these were not in the initial diff: both live where the new per-kind path meets an OLD assumption — that record()'s input is already scoped to the filter, and that a band's keys are always the filter's kinds. Neither held once callers began supplying the map themselves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Vitor PamplonaandClaude
|
42a91ffb79 |
SyncCoverage: one band interval cannot speak for several kinds
A band held ONE created_at interval per (relay, filter). For a filter naming several kinds that is a claim no walk can support: ask for `kinds: [0, 30382]`, find profiles going back years and score cards only from last month, and the band records 2020..now for the pair. The next run then skips that whole interior for BOTH — so score cards written inside it are never asked for again, and nothing anywhere says so. A long-lived kind vouched for a short-lived one. Band.spans is now per kind. Each carries only the evidence actually collected for it, so the profile kind keeps its wide interval and the score kind keeps its narrow one, and legs() re-opens the interior for the second while still skipping it for the first. Three things keep the cost of that where it was: - legs() REGROUPS kinds by the windows they want. Identical coverage — the common case, and the only case until they diverge — collapses back into one ask, so a filter that produced two legs still produces two rather than two per kind. Only a kind whose evidence genuinely differs earns its own. - A finished reconcile needs no per-kind evidence and is given none: negentropy compares the filter's whole id set in one pass, so it covers every kind in the filter or none. Only the PAGED path changed. - Filters naming no kinds keep a single span under ALL_KINDS, which is the same claim as before, correctly scoped to the case where it is the only claim available. record() takes observedByKind, and SyncCoverage.observe() accumulates it as events arrive — replacing the pair of hand-rolled vars each caller kept, and moving the per-event isPlausible guard in with it. A paged walk over a MULTI-kind filter that supplies none earns no band at all, loudly, once: attributing one interval to every kind is exactly the over-claim this removes, and a band that over-claims skips events silently, which is worse than re-reading them. Single-kind filters are untouched — there the aggregate always was the per-kind answer. The state file gains a per-kind `spans` object and keeps `min`/`max` as the outer edges, so a rollback to a binary from before this reads the file and behaves as it always did. A file written BEFORE this loads its one interval under ALL_KINDS — the old, wider claim, kept rather than discarded because discarding it would re-download every upstream's corpus once on upgrade. The first per-kind walk replaces it. All 26 existing SyncCoverage tests pass unchanged, which is the evidence that single-kind behaviour did not move. The five new ones were checked against the pre-fix rule reinstated in place: the two behavioural ones fail there and pass here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Claude
|
5c56444054 |
fix: strip the whole punctuation tail from a detected url
Audit follow-up to the quote fix. The path, query and fragment readers only stop on a space, and readEnd dropped a single trailing delimiter, so a quoted link that closed a sentence kept its quote: He linked "https://example.com/some/path". -> https://example.com/some/path" (see "https://example.com/some/path") -> https://example.com/some/path" readEnd now strips the tail in a loop. The balance check runs on every round, so a url that legitimately ends in a matched closer still stops the strip: `[link](…/Bitcoin_(disambiguation)).` keeps `(disambiguation)` and drops the `).` that belongs to the sentence. Differential run over a 4000-string corpus against the previous commit: 8 rows change, every one of them the removal of extra trailing punctuation. No url is gained, lost or truncated mid-string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKCAegYMF9V9Nb8FHcMvT7 |
||
|
Claude
|
dc45477deb |
fix: don't glue quotes onto detected urls
A bare host wrapped in quotes ("relay.momostr.pink") was detected with the
opening quote attached, so the rendered link read `"relay.momostr.pink` and
pointed at a host that does not exist. The mirror case was also wrong: a
quoted url with a path/query/fragment kept the closing quote, because those
readers only stop on a space.
Quotes are not host characters, so they now end the current token exactly
like a space does in readDefault (covering the leading quote and a quote
glued to a previous word, e.g. `href="www.google.com"`), and they were added
to CANNOT_BEGIN_URLS_WITH / CANNOT_END_URLS_WITH so a trailing quote read as
part of a path, query or fragment is stripped on readEnd. The set covers the
ascii quotes plus the typographic family, including the guillemets below the
international-character threshold that the ascii boundary rule never cut.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GKCAegYMF9V9Nb8FHcMvT7
|
||
|
Claude
|
70d51cc98b |
fix(relay): parse the authority instead of substring-matching the url
Audit of the IPv6 work found a family of bugs in isLocalHost/isOnion, most
predating this branch, all with one root cause: the predicates ran `contains`
over the whole url rather than parsing the authority. These decide whether a
relay is exempt from Tor, and relay urls arrive from other people (NIP-65
lists, relay hints, r tags), so they are attacker-controlled input.
- A path could impersonate the host. `wss://evil.example.com/127.0.0.1`
answered isLocalHost() == true, so any relay list could hand the app a url
that silently dropped its own Tor routing. The IPv6 lookup added earlier on
this branch had the same flaw via `/[fd00::1]`, and IPv6 canonicalization
could rewrite a path outright, corrupting the url.
- `.onion:8080` never matched the `.onion/` test, so an onion relay on an
explicit port was not treated as onion at all: never forced onto Tor, and its
hostname went to the clearnet DNS resolver. The fully-qualified `.onion.`
spelling missed the same way.
- Host tests were case-sensitive, but fix() asks them before the RFC 3986 pass
folds case, so LOCALHOST:8080 and ABC.ONION:8080 were handed a wss:// scheme
neither host can serve.
- Private IPv4 was substring-matched, which missed 10.0.0.5, 172.16.3.4 and
127.1.2.3 — a LAN relay got wss:// and was dialed through Tor — while
matching 192.168.evil.com and 127.0.0.1.evil.com, registrable domains that
could therefore exempt themselves from Tor. Same for notlocalhost.example.com
against `contains("localhost")`.
- A `://` inside a path was read as a scheme separator, so
`relay.com/x://127.0.0.1` read its path as the authority.
Fixes: a shared hostStart/hostEnd/hostEndWithoutPort trio bounds every test to
the authority, strips :port and trailing dots and validates the scheme; private
ranges are parsed via a new Ipv4 util rather than substring-matched;
comparisons are case-insensitive per RFC 4343; NormalizedRelayUrl.isOnion()
delegates instead of keeping a second, weaker copy of the test.
No performance regression: the old form ran six full-string scans, the new one
bounds its work to the authority and rejects a DNS host from an IP parse on one
character. Ipv6.isLiteral gained a two-colon gate so the schemeless host:port
case answers without allocating the parser's buffer.
Ipv6 is now pinned by a differential test: 4000 random addresses round-trip
against java.net.InetAddress in both directions, and the canonical form is
asserted equal to OkHttp's host for the same address, so the relay identity the
app stores provably matches the host it dials.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQr8CDsznzCRUeB5tS8VYk
|
||
|
Claude
|
067d68b89c |
feat(relay): canonicalize IPv6 relay urls and support overlay meshes
Closes the four gaps the previous commit characterized for relays on an Yggdrasil overlay, where every relay is an IPv6 literal in 0200::/7 served over plain ws:// (no DNS, no CA-issuable certificate). New quartz/utils/Ipv6.kt: pure-Kotlin literal parsing, RFC 5952 canonical formatting and range classification. No java.net, so it works on every KMP target. - Canonicalize the bracketed host in RelayUrlNormalizer.norm(). RFC 4291 lets one address be spelled many ways and the RFC 3986 pass only folded hex case, so two spellings survived as two NormalizedRelayUrl values for one host — and that value keys the connection pool, the relay-list sets, the NIP-11 cache and the per-relay stats, so the app dialed one relay twice. The canonical form matches what OkHttp renders when it dials; the tests assert that agreement differentially. Relay lists rehydrate through normalizeOrNull, so stored entries fold on load and no migration is needed. - Add isOverlayNetwork() for 0200::/7 and default those relays to ws://: nothing can issue a certificate for the range, so wss:// could only fail its handshake, and the overlay already encrypts end to end. - Teach isLocalHost() the IPv6 twins of the literals it already knew — ::1, fc00::/7 and fe80::/10 — so a relay on one skips TLS and Tor and stays out of published relay lists, as its IPv4 equivalent already did. - Never route an overlay relay through Tor: the range is unroutable there, so proxying guaranteed failure rather than privacy. TorRelayEvaluation covers both the Android and desktop relay paths; RoleBasedHttpClientBuilder covers non-relay HTTP. - Bracket a bare IPv6 literal automatically (what yggdrasilctl getSelf prints), but only when the whole string parses as an address, so host:port and addressable pointers still fall through. RelayUrlEditField now shows an error instead of no-opping, fixing the silent Add button for all invalid input. Mesh relays are still published in NIP-65 and offered by the outbox model; the plan doc explains why that is left as a maintainer's call, and records that no live socket test was possible here (the container has no IPv6 stack). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQr8CDsznzCRUeB5tS8VYk |
||
|
Claude
|
129401bdaf |
test(relay): characterize Yggdrasil/IPv6 relay handling
Assesses how the app fares when relays live on an Yggdrasil overlay, where every relay is a bracketed IPv6 literal in 0200::/7 served over plain ws:// (no DNS, no CA-issuable certificate). The happy path works: a hand-typed ws://[...]:port normalizes, survives the RFC 3986 pass and is dialed by OkHttp; nothing in the stack is IPv4-only and cleartext is already permitted globally. Four gaps are pinned by the new characterization tests: 1. RelayUrlNormalizer folds hex case but not zero-compression, so two legal spellings of one address yield two NormalizedRelayUrl values while OkHttp collapses them to one host — duplicate sockets, REQs and stat entries. 2. isLocalHost() does not know 0200::/7, so a schemeless literal defaults to wss:// and can only fail its TLS handshake. 3. An unbracketed literal (what yggdrasilctl getSelf prints) is rejected, and RelayUrlEditField.submitRelay has no else branch — the Add button silently does nothing. 4. TorRelayEvaluation classifies mesh relays as "new", so with Tor on they are dialed through the SOCKS proxy, which cannot route 0200::/7. No behavior is changed. quartz/plans/2026-08-04-yggdrasil-ipv6-relays.md records the full assessment, the NIP-65/outbox propagation consequences of publishing a key-derived mesh address, and what could not be verified here (the analysis container has no IPv6 stack, so nothing below the socket was exercised). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQr8CDsznzCRUeB5tS8VYk |
||
|
Vitor PamplonaandGitHub
|
3a702add57 |
Merge pull request #3857 from vitorpamplona/claude/relay-url-normalizer-mggnkh
NIP-66 relay monitoring: streaming probes, read/write checks, URL fixes |
||
|
Claude
|
5ec35c7772 |
feat(quartz): default the read test to kind 0, limit 1
A kind-0, limit-1 REQ works everywhere: purpose relays (purplepag.es) reject kind-less filters outright, and practically every relay stores some profile. Verified against production — purplepag.es's read side now measures instead of going unobserved. Pass a different kinds list to probe a specific shelf, or null for a kind-less query on relays known to allow one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A9sSyh1QLJD3PZ18tVPPVK |
||
|
Claude
|
9a4f6c6cdd |
feat(quartz): optional kinds on the read-test filter (production finding)
Verified the new probe surface end-to-end against production relays
(probeFlow streaming, readWriteCheck, signed 30166 templates). One
compatibility finding: purpose relays like purplepag.es reject any REQ
that names no kind ('blocked: filters must specify at least one kind'),
leaving their read side unobserved. readTestFilter/readWriteCheck now
take an optional kinds list for those; the default stays kind-less
because naming kinds also narrows the query on every other relay.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9sSyh1QLJD3PZ18tVPPVK
|
||
|
Claude
|
c767298368 |
fix(quartz): audit fixes — foreign-OK confirmation bug, normalizer hot-path allocation
Audit findings across the branch, each verified with a failing test or
measurement before the fix:
- publishAndCollectResults counted an OK from a relay OUTSIDE relayList
(same event id — a probe-wave straggler, or any republish of the same
event to a different relay set) toward its confirmation window, ending
the wait loop early and misreporting still-pending listed relays as
NO_RESPONSE. The OK branch now carries the same relayList guard the
onCannotConnect/onDisconnected branches always had. Regression test
proves the failure without the guard. readWriteCheck additionally
varies the probe event content per wave so wave N's confirmation window
can never match wave N-1's event id at all.
- RelayUrlNormalizer.fix() called trimEnd('%','2','0') unconditionally,
allocating a full string copy for ANY url merely ending in '%', '2' or
'0' — which includes every relay port ending in zero (wss://host:3030).
Now gated on endsWith("%20"), keeping the hot path allocation-free;
semantics unchanged (test pins both the trim and the untouched-port
cases).
- amy relay probe --file: unreadable file is now a clean bad_args error
instead of a stack trace, and skipped onion urls are counted and
reported (file_onion_skipped) instead of vanishing from the tally.
- probeFlow KDoc now states that a slow collector eats into the current
wave's absolute deadline (answers are still recorded; silent relays get
less listening time), not just that it delays the next wave.
Verified non-issues: androidx.collection LruCache is internally locked
(safe for CachedNip11Fetcher/normalizer concurrency); probeWave's
per-terminal emission cannot lose or double-emit verdicts (remaining-set
guard, data maps read at emission time); existing publish callers all
benefit from the OK guard rather than depending on the old behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9sSyh1QLJD3PZ18tVPPVK
|
||
|
Vitor PamplonaandClaude Opus 5
|
0d0117061a |
fix(relay): compare every filter in FiltersChanged, not just the first
`needsToResendRequest(List, List)` used a non-local `return` inside `forEachIndexed`, so the loop always returned on iteration 0 and only `filters[0]` was ever compared. A subscription whose first filter happened to be unchanged reported "no resend needed" however much the rest had changed, leaving the relay serving a stale filter set and the app silently missing events. Only the size check offered any protection, so the bug was invisible whenever the filter count stayed constant. Replaces the loop with an indexed scan over all filters, which also drops the lambda allocation and matches the hot-path style in this package. Adds FiltersChangedTest. 3 of its 9 cases fail on the unfixed code — all of them changes beyond index 0 — while the other 6 pass both before and after, pinning the blast radius to exactly the buggy behaviour. Coverage includes the deliberate `since`-moves-forward exemption, which must not trigger a resend on any index. Note for reviewers: PoolRequests.kt:490 and :528 use this inverted as a "same as last" refusal check, so those become stricter — filter sets that differ only beyond index 0 were previously treated as identical and will now correctly be treated as changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |