mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 16:33:27 +00:00
2ecbb66cdbdd72ba74e5f9e96aaa24b7ea4c29b4
2975
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  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> |
||
|
Claude
|
fd5bd994a1 |
feat(quartz): read+write relay checks — observed facts only, no NIP-11 claims
RelayProber.readWriteCheck(relays, signer) is the deeper check pair for relays already proven live (warm sockets from a probe that just ran): - READ: a real limit-1 REQ the relay must query its store for, timed REQ→first answer (honest rtt-read on an open socket). - WRITE: one ephemeral RelayProbeWriteTest event signed by the monitor key, timed publish→OK (honest rtt-write). An OK false is a measured policy answer, kept with its NIP-01 machine-readable reason; only silence leaves the write side unobserved (writeAccepted = null). publishAndCollectResults now stamps each OK with its elapsedMs (a rejection is still a round trip; -1 when the relay never answered), so any caller gets write latency for free. toDiscoveryEventTemplate(readWrite = ...) folds the pair into the 30166 template: rtt-read/rtt-write when measured, R auth / R pow when the write was refused with auth-required:/pow:. NIP-11-derived tags (N supported NIPs, k kinds, T type) are deliberately NOT emitted — those are relay self-claims, and publishing them under a monitor signature without per-NIP compliance tests would launder claims into measurements. Per-NIP/per-kind compliance suites can come later as opt-in checks; open/read/write is the default surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A9sSyh1QLJD3PZ18tVPPVK |
||
|
Claude
|
a7eec1d605 |
feat(quartz): NIP-66 check options — read-test filters, write-test event, cached NIP-11 fetcher
RelayProber.probe()/probeFlow() take a filters option choosing the check: LIVENESS_FILTERS (default, impossible-id REQ — EOSE proves liveness with no payload) or readTestFilter(limit = 1) — a REQ the relay must actually work for, querying and streaming real events, making Verdict.rttEoseMs a genuine read test. RelayProbeWriteTest.build() creates the write-check event: ephemeral kind 20166 (never stored by compliant relays) carrying a NIP-40 expiration tag 60s out as belt-and-braces for relays that store unknown ephemeral kinds. Publish it under the monitor key, time the OK for rtt-write, map rejection prefixes to R requirement tags — an OK false still proves the write path. Nip11Fetcher is the missing fetch seam for relay information documents, mirroring Nip05Fetcher: the interface lives in commonMain, OkHttpNip11Fetcher (jvmAndroid) does the Accept: application/nostr+json GET, and CachedNip11Fetcher wraps any implementation with a TTL cache — successes trusted for a day, failures remembered for five minutes so a census doesn't hammer hosts that just refused. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A9sSyh1QLJD3PZ18tVPPVK |
||
|
Claude
|
d9a58950ec |
feat(quartz): stream NIP-66 probe verdicts and expose them as signable 30166 templates
RelayProber.probeFlow(urls) is a cold Flow that emits each relay's Verdict
the moment the relay resolves (EOSE, CLOSED or connect failure) instead of
at the end of the whole census; only silent relays wait for their wave's
deadline. probeWave now resolves verdicts per-terminal, so the batch
probe() shares the same path.
Verdict.toDiscoveryEventTemplate() renders a verdict as an UNSIGNED
kind:30166 template (d = normalized url, n network type, rtt-open when
reachable, R auth when the probe hit a NIP-42 auth-required CLOSED) so an
external consumer signs with its own monitor key:
prober.probeFlow(urls).map { it.toDiscoveryEventTemplate() }
.collect { publish(signer.sign(it)) }
rtt-eose is deliberately never published as rtt-read: it is measured from
the wave start (dial + TLS + queueing + read), and aggregators rank on
rtt values. The RelayObserver/RelayMonitor path supplies honest
rtt-read/rtt-write from real traffic.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9sSyh1QLJD3PZ18tVPPVK
|
||
|
Claude
|
156176f5fe |
fix(quartz): stop RelayUrlNormalizer from accepting urls that can never be relays
Validated against a 45k-entry corpus of relay-url hints exported from real events (317k tag occurrences). The normalizer was converting ~30k distinct https:// urls with paths (Mastodon/bridge actor urls from proxy tags, web pages, images) into wss:// addresses that can never answer, wasting connection attempts and relay-pool slots. - http(s) → ws(s) scheme swap now only applies to bare hosts (host[:port] plus optional trailing slash); an http url with a path, query or fragment is a web resource, not a mistyped relay. - Authority validation for all schemes: rejects empty hosts, userinfo (@), percent-encoding and commas in the host, and paths that start with // (the signature of a second pasted url, e.g. wss://https//host). - Interior whitespace and backslashes reject the whole string (multiple urls or prose in one field). - Zero-width characters (U+200B..D, U+2060, BOM) are stripped instead of corrupting the parse (wss://\u200Bnos.lol previously normalized to the scheme-less //nos.lol/). - Schemeless candidates must look like host[:port] (single colon, numeric port), rejecting addressable pointers (31990:pubkey:dtag) and bare scheme leftovers (wss:) before the expensive RFC 3986 parse. - Protocol-relative //host/ inputs normalize as wss:// instead of resolving to https://. - normalizeOrNull now double-checks the parser output still starts with ws(s):// and rejects otherwise. Corpus impact: 30,014 garbage urls (30,333 events) now rejected, 0 real relays lost (all 15,162 kept urls normalize byte-identically), 6 broken outputs fixed. fix() itself stays allocation-free on the happy path (~357ns vs ~318ns per call on the garbage-heavy corpus). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A9sSyh1QLJD3PZ18tVPPVK |
||
|
Claude
|
b48b87a60b |
Merge remote-tracking branch 'origin/main' into sync-accessories-from-vespa-relay
# Conflicts: # quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/server/backend/IngestQueue.kt # quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/IEventStore.kt # quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt |
||
|
Claude
|
cdbd550405 |
Fix audit findings across the sync accessories and their consumers
quartz: - SQLiteEventStore: classify per-row savepoint errors — policy refusals (blocked:/constraint/not allowed) stay Rejected, everything else is now Failed, so disk-full no longer masquerades as 2M duplicate rejections - IEventStore.batchInsert default: rethrow CancellationException and map unknown throws to Failed (re-offering a duplicate is idempotent; dropping a good event on a transient store error is not) - IngestQueue: rethrow CancellationException instead of stamping a cancelled batch Failed and continuing - HostStrikes: make the eviction verdict exactly-once under concurrency (deadHosts.add is the atomic gate) and re-check produced before publishing - SyncCoverage: bound the identity fingerprint cache (a caller minting fresh Filter instances per cycle could grow it forever); legs() gains a floor parameter so a complete band re-opens its older span when the caller's window deepens; coveringWindow no longer treats a fully covered relay as needing the whole filter - PagingWindowProgress: accept single-second windows (a band's re-read edge leg is exactly that shape) geode: - MirrorWorker: cap reconciledThrough at the leg's own ceiling — the older leg of a resumed catch-up no longer stamps the band complete through 'now' before the newer leg has run (silent event loss for up to fullResyncSeconds if that leg failed) - MirrorWorker: run negentropy and the paged fallback by hand instead of negentropySyncOrFetch: drops the O(delivered-ids) dedup set from the mirror path, and a fallback resets the observed span so a band never claims interior ranges only a half-finished reconcile scattered over - MirrorWorker: clamp a paged band's ceiling to the snapshot instant so one future-dated event cannot suppress the next boot's newer leg - MirrorWorker.close(): join the workers (bounded) so the final coverage flush carries the last records - Main: gate the coverage file on the store actually being persistent — database.file with in_memory=true (the default) persisted bands over a volatile store, and the next boot skipped the backfill over an empty database; honor --db overrides - SyncCoverageFile: request ATOMIC_MOVE explicitly; fix the restore/dirty comment - Import summary now prints the failed count; document mirror_sync_state_file in config.example.toml |
||
|
Claude
|
4081ef1681 |
nip01Core: one owner rule, one supersession rule, one tag-name rule
Three semantics rules each existed as multiple independent copies: - Event.owner() (gift-wrap recipient controls the wrap, else the author — NIP-09/62 authority) was derived inline in EventIndexesModule and again in EventStoreProjection.ownerOf. - The NIP-01 replaceable tiebreak (newest created_at, ties to the lexically smallest id) lived in EventStoreProjection.supersedes, in SQL, and downstream. - "Indexable tag name" was spelled `length == 1` in four places, which admits "5" and "#" — names the NIP-01 #x filter space (single a-zA-Z letters) cannot address, letting stores disagree about which tags filters reach. isIndexableTagName encodes the NIP-01 rule; converging FilterIndex and the SQLite IndexingStrategy on it deliberately tightens single-char non-letter tag names out of the index. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MfwV3xgMSmfxy16ujGPxGW |
||
|
Claude
|
804b850095 |
Audit fixes: empty exclusions, missed fallback, one vocabulary
An all-dash search token ("--") stripped to an empty exclusion that
toSearchString/stripExtensions round-tripped into a REQUIRED "-" term
reaching SQLite FTS; it is now dropped at parse. IngestQueue's
batchInsert fallback was the one site still hand-writing "insert
failed" (unprefixed) while every other path emits
RejectionReason.INSERT_FAILED. RejectionReason no longer duplicates
the NIP-01 prefixes MachineReadablePrefix already owns, and the
expiration trigger now rejects with the same words as the Kotlin
pre-check instead of its own spelling. Tests pin the "--" drop,
consecutive quoted spans, text after a closing quote, the extractor's
fallback tier, blank-content normalization, and the
unparseable-buzz-content hashtag seam; extractor KDoc now states
where the trimmed/non-empty and never-empty-Profile guarantees
actually live.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MfwV3xgMSmfxy16ujGPxGW
|
||
|
Claude
|
42fc73f6cb |
nip50Search: per-kind weighted search-field extraction
SearchableEvent.indexableContent() flattens a kind's searchable text into one blob, so a weighted full-text backend (title above summary above body) had to re-derive the decomposition itself — and drift every time a kind's parsing changed here. SearchFieldExtractor now lives beside the kinds it decomposes: title-like accessors primary, summary/description secondary, body tertiary, kind-0-shaped metadata in profile roles, with an indexableContent() fallback so every searchable kind, current or future, is covered. IndexableFields is a sealed shape — Profile or Tiered — so a kind cannot mix identity fields with content tiers, and each shape declares its own website role (a profile's homepage; a content kind's affiliation URLs). Multi-valued roles are carried UNJOINED, as lists: hashtag and location tags ride raw beside the tiers (filled by the one tiers() funnel every content branch uses, so no branch can forget them and profile shapes never see them), and separator or weighting choices — hashtags at summary weight, "\n" vs " ", arrays vs joined columns — belong to the backend, not the library. Empty extractions always normalize to None. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MfwV3xgMSmfxy16ujGPxGW |