A production ANR on a Pixel 8 (Amethyst 1.13.1, anr_2026-08-03-12-55-26-256)
showed the app burning 596% CPU — 6 of 9 cores — with the main thread stuck in
WaitingForGcToComplete. 37 of 52 runnable DefaultDispatcher workers sat at ONE
program point inside PoolRequests.onIncomingMessage and 12 more at one point in
syncState, all state=R, while the single thread actually holding the lock was
itself parked in GC.
Root cause: RequestSubscriptionState.withLock was a raw busy-wait
(`while (lock.exchange(true)) { while (lock.load()) {} }`) with no yield or
backoff, and being `inline` it disappeared into its callers' frames. The lock is
per subId, but one subId spans every relay it runs on — 191 live sockets on that
device — so dozens of relay-dispatch threads piled onto a single AtomicBoolean.
Spinning is only correct when the holder cannot be descheduled; on Android it
always can.
The fix stripes the lock per (subId, relay) rather than making waiting cheaper.
All 11 withLock bodies in PoolRequests are already scoped to exactly one relay,
and every field of RequestSubscriptionState is keyed by relay, so the sharing was
purely an artifact of mutableMapOf not being thread-safe. State moves into a
ConcurrentMap<T, RelayState>; locks live in a fixed 32-entry stripe array that is
never mutated, so lock identity stays stable — if locks lived inside the map
values, a thread holding one while another dropped and re-created that entry
would leave both inside the critical section excluding nothing.
A suspending Mutex was measured and rejected: it needs 262 method overrides and
110 call sites to become suspend, and ran at 0.35-0.63x the current throughput.
Measured (LockDesignComparisonBenchmark, 191 relays / 64 dispatcher threads):
striped vs per-sub lock 1.5-2.8x throughput, bystander p50 halved
On device (SM-T220, playBenchmark, same account, n=3 per design):
DefaultDispatcher CPU -35% mean / -30% median vs the spin lock,
with non-overlapping ranges; GC -18%
Plus 10 min of driven UI stress (feed, profiles, chat, notifications,
communities): no ANRs, no crashes, thread pools stable.
Also here:
- PlatformLock: new expect/actual parking lock (ReentrantLock on jvmAndroid,
NSRecursiveLock on Apple, spin only on linuxX64 which is a CI target). quartz
cannot use commons' equivalent KmpLock because commons depends on quartz.
- LiveNegentropyIndex had the identical busy-wait with a full list SORT inside
the critical section; switched to PlatformLock.
- ConcurrentMap.remove (+ tests), with a caution that a removable value must not
own a lock callers acquire.
- SpinLockConvoyBenchmark: regression guard asserting contended waiters PARK
rather than spin (fails-before / passes-after). Pure benchmarks are gated
behind -PprodRelayBench=1, so CI cost is 0.3s rather than 51.5s.
Analysis and measurements: quartz/plans/2026-08-03-poolrequests-lock-contention.md
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Audit follow-up on the timeout work.
- fetchAllPages matched each event with `for ((i, f) in activeFilters)`.
That destructuring form allocates an Iterator on every event, on the
relay's reader thread, for the whole download -- millions of short-lived
objects in a bulk walk. Switched to an indexed loop, which is why quartz
uses the fast* operators elsewhere in hot event paths (those only cover
Array, so a List needs the index form).
- count() and fetchFirst() drain their channels in an inner loop that only
suspends when the channel is empty, so a backlog was consumed with no
cancellation check: neither the idle window expiring nor the caller
giving up could interrupt it mid-drain. Added an explicit ensureActive(),
matching the check fetchAllPages already does per page.
- count() could also lose a result that arrived after the last window
closed but before unsubscribe, understating the returned map. Added the
post-loop tryReceive drain that fetchAllWithHooks and fetchFirst have.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZe1Pq2ejoHehdufhPDRgb
Every wait in the accessories package is an idle window measured from the
relay's most recent progress, so the parameter now says so. The name is
the contract: a caller reading timeoutMs reasonably expects a deadline,
which is exactly the misreading that made fetchAllPages' hard per-page
cap look correct for so long.
Renamed across the public surface -- fetchAll (7 overloads),
fetchAllWithHooks, fetchAllPages (2), fetchAllPagesFromPool,
fetchAllPagesFromPoolWithHooks, fetchFirst, count (2), countMerged -- plus
the quartz wrappers whose own parameter is a pure pass-through of that
window (KeyPackageFetcher, RecipientRelayFetcher, FollowerCrawler.Config)
and every call site across commons, cli, desktopApp and amethyst.
Deliberately NOT renamed, because these are genuine wall-clock bounds and
the differing name is the tell:
- publishAndConfirm's timeoutInSeconds -- one fixed window to collect
the OKs, a bounded confirmation round-trip rather than a stream.
- GrapeRankCrawler.Config.timeoutMs -- a hard per-drain gate
(withTimeoutOrNull(config.timeoutMs)) that also drives parking.
- Context.awaitReply's timeoutMs, Context.syncIncoming, and the
non-accessory app-layer helpers (RelayProber, FeedMetadataCoordinator,
RelayAuthPromptBus).
This is a source-breaking change for named-argument callers of quartz.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZe1Pq2ejoHehdufhPDRgb
Follow-up audit on the timeout normalization. Three findings.
1. The maxTotalMs I added to fetchFirst and multi-relay count was the
wrong shape twice over. A hard wall-clock bound already composes at the
call site -- withTimeoutOrNull(ms) { fetchFirst(...) } -- so putting it
in the signature duplicates what the caller has for free. And it was
papering over the real defect: repeat chatter from a relay already
accounted for (a CLOSED/reconnect loop, a duplicate COUNT) was treated
as activity and restarted the idle window, so a flapping relay could
hold the call open indefinitely. Both now reset the window only on
genuine progress -- an event, or the first terminal signal from a relay
still being waited on -- which is the rule the negentropy watchdog
already applies to NOTICE/CLOSED chatter, and which makes both calls
self-bounding at one window per relay. Ceiling params removed; the
overflow guard they needed goes with them.
2. fetchFirst could drop a match: an event landing after the last terminal
signal but before unsubscribe was left unread in the channel and the
fetch reported nothing found. Added the post-loop drain that
fetchAllWithHooks already does. Covered by a test.
3. fetchAllPages published its per-page counters across threads without a
barrier on the idle path. received/delivered/pageMinTs/idsAtPageMin are
written on the relay reader thread and read by the driver once the wait
ends; the EOSE path gets happens-before from the channel, the idle path
had none, so the driver could read a stale pageMinTs (ending the walk
early) or an unsafely published idsAtPageMin. The volatile IdleClock
bump now runs in a finally, so it covers every event including the
early-returning duplicate and orders after the counters.
Also folded the single-relay count channel close into its finally, so a
throw mid-wait cleans up like every sibling accessory.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZe1Pq2ejoHehdufhPDRgb
The per-page wall-clock ceiling added alongside the idle-window change did
not do what it claimed. fetchAllPages waits inside a while(true): when a
page's wait ends, the loop advances the until cursor and issues the next
REQ rather than returning, so a ceiling bounds one page, never the call.
Measured against a relay trickling events forever with an unbounded
filter, maxPageMs=400 produced 8 REQs and no return -- the walk ran until
the caller cancelled, exactly as it would with no ceiling at all.
It also made truncation unsafe. Cutting a page mid-stream advances until
to the oldest event received so far, which only preserves the set if the
relay streams strictly newest-first -- NIP-01 recommends that but does not
require it -- so a ceiling firing on an out-of-order relay can skip the
not-yet-sent events above that cursor. That is the same class of silent
gap the idle window was introduced to close.
What actually bounds a paged download is the filter's limit (already the
documented way) or cancelling the caller, which the ensureActive() at the
top of each page honors. Removed from fetchAllPages, fetchAllPagesFromPool
and fetchAllPagesFromPoolWithHooks; a regression test pins the real
contract so nobody re-adds a ceiling believing it caps the walk.
maxTotalMs stays on fetchAll/fetchAllWithHooks, fetchFirst and multi-relay
count: those wait in a single loop and return, so there the ceiling
genuinely ends the call (each is covered by a test asserting it does).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZe1Pq2ejoHehdufhPDRgb
Audit follow-ups on the idle-window normalization:
- maxTotalMs/maxPageMs defaults are timeoutMs * 10, which silently
overflows to a negative Long for an effectively-infinite idle window
(Long.MAX_VALUE * 10 wraps to -10). withTimeoutOrNull then expired
immediately, inverting 'wait forever' into 'never wait'. All three
ceilings (fetchAllPages, fetchFirst, fetchAllWithHooks) now treat a
non-positive ceiling as uncapped, matching the idle window's <= 0
convention. Regression-tested via fetchFirst.
- count(filters) leaked its RelayConnectionListener and left COUNT subs
open if the caller cancelled mid-wait or a listener threw: the cleanup
ran as straight-line code with no try/finally, unlike every sibling
accessory. Wrapped so unsubscribe + removeConnectionListener + channel
close always run.
- New FetchFirstIdleTimeoutTest pins fetchFirst's idle-window semantics
(signals restart the window, silence costs exactly one window, the
ceiling stops endless terminal chatter, overflow means uncapped).
- README: note publishAndConfirm's fixed window as the deliberate
exception, and correct the fetchAllPagesFromPool row, which claimed
cross-relay dedup the function explicitly does not do.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZe1Pq2ejoHehdufhPDRgb
fetchAllPages waited for each page's EOSE under a hard wall-clock
withTimeoutOrNull, unlike fetchAll / fetchAllWithHooks / the negentropy
sync, whose timeouts are idle windows reset by every arriving message.
A slow-but-streaming page could be silently truncated, and a relay
slower than timeoutMs to first byte was mistaken for a drained set.
- Extract IdleClock + receiveWithinIdle out of NostrClientNegentropySyncExt
into a shared internal IdleWatchdog.kt and document the package-wide
convention in the accessories README.
- fetchAllPages (+ pool/hooks variants): the per-page timeout is now an
idle window bumped by every event; a maxPageMs ceiling (10x, matching
fetchAllWithHooks' maxTotalMs) backstops a never-EOSE trickle. A
non-positive ceiling means uncapped, mirroring the idle <= 0 convention.
- fetchFirst: the wait restarts on every arriving signal instead of one
hard deadline across all relays; maxTotalMs ceiling added.
- count (multi-relay): each arriving COUNT result restarts the window;
maxTotalMs ceiling added. Single-relay count is trivially idle already.
- NegentropyStoreSync page fallback clamps a disabled watchdog (0) to
DEFAULT_DOWNLOAD_IDLE_MS like fetchByIds, instead of paging unbounded.
- New NostrClientFetchAllPagesIdleTimeoutTest pins the semantics
(verified to fail against the old hard-deadline wait).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KZe1Pq2ejoHehdufhPDRgb
A relay holding 12,289,614 profiles answers
["COUNT","c1",{"kinds":[0]}] -> {"count":500}
LimitsPolicy ran the same clampLimits over CountCmd as over ReqCmd, so
an unbounded COUNT was given RelayLimits.defaultLimit and the store then
counted at most that many.
defaultLimit answers "how many events should a REQ return when the client
names none". A COUNT returns no events, so the question has no meaning
for it, and applying the answer anyway turns every unbounded COUNT into
min(matches, defaultLimit).
The failure is quiet, which is what let it survive: 500 is a plausible
number, and the kinds under the default answered correctly. On the relay
that surfaced it, kinds 1 and 10040 were right while 0, 10002 and 30382
were all exactly 500.
maxLimit still applies — a client asking to count at most N is asking
something a relay may bound. Only the invented default is dropped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It was measured from onConnecting to onConnected, which includes the time
the call sat in the client's dispatcher queue. Under a 16,507-relay
fan-out that queue dominates everything else: published records showed a
median rtt-open of 33.5 SECONDS and a max of 90, against a true minimum
of 140ms.
That is the field aggregators rank relays by, so it was worse than
publishing nothing — a signed claim that healthy relays are slow, when
the slowness was ours.
pingMillis already carried the right number and was being handed to the
listener unused: BasicOkHttpWebSocket computes it as
receivedResponseAtMillis - sentRequestAtMillis, so it starts when the
upgrade request actually goes out and excludes everything before it.
Zero or negative means the transport could not time the handshake, and
then no timing is published rather than a fabricated one — the same rule
the rest of this class already followed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Audit finding on the StoreQueryContext seam: RelaySession kept
authenticatedUsers as a plain mutable LinkedHashSet and handed out a
live view through RequestContext. A store (or EventSource) reading the
set during a long REQ replay — exactly what StoreQueryContext invites —
could race a concurrent NIP-42 AUTH commit on another coroutine:
iteration vs. add on an unsynchronized set is a
ConcurrentModificationException or a torn read.
The engine now swaps an immutable Set behind a @Volatile field on each
AUTH (the only writer), so every read is a consistent snapshot and
holding one across a replay is safe. AUTH is rare; the copy is off the
hot path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hH4RY2AUwfMZ54RkMiT45
The IEventStore contract now defines the seams a store implementation
needs instead of having the relay layer decide for it:
- NIP-50 search extensions: LiveEventStore no longer strips key:value
tokens before the store sees them. Filter.search reaches every store
verbatim, and each implementation decides which extensions it
supports — the store, not the middleware, is the only component that
knows whether sort:rank is a directive or noise. The built-in SQLite
and filesystem stores strip at their own boundary (their FTS engines
would otherwise error on / literally match the tokens), preserving
the NIP-50 'ignore unsupported extensions' behavior end to end.
Extension-aware stores need no side channel to recover the raw
string anymore. Fs delete now checks emptiness on the stripped
filter so an extensions-only search cannot wipe the store.
- Caller identity: new StoreQueryContext coroutine-context element,
installed by LiveEventStore around every REQ/COUNT store call when
the connection has NIP-42-authenticated pubkeys. Observer-relative
stores (web-of-trust ranking, for-you relevance) read it off the
coroutine context; ranking context only, never match-set changes.
- Negentropy liveness: snapshotIdsForNegentropy gains an optional
onProgress hook so mirrors syncing huge corpora get a running count
through the interface type instead of a concrete-class overload.
SQLite reports from its row loop; the interface default streams and
reports too.
- FtsReindexProgress.cursor documented as opaque and store-defined so
resumable-reindex callers never assume id semantics.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hH4RY2AUwfMZ54RkMiT45
RelayObserver is a RelayConnectionListener, so on its own it can only
report on relays something opened a websocket to. On a large fan-out that
is a small minority, and it is the wrong minority: the cheap checks that
decide NOT to dial — a TCP probe, a DNS failure, a host struck out after
repeated silence — are precisely the ones that learn a relay is gone, and
their findings had nowhere to go.
Measured on a 16,507-relay list: 104 records published. Everything else
was ruled out before the client ever saw it, so the monitor had nothing
to say about 99% of the relays it had just formed an opinion on.
record() takes those findings. Same rules as the connection path — a
relay that answered is not demoted by one failed probe, and a reachable
relay with no measured time is published with no time rather than a
fabricated zero.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nip42AuthDmDeliveryTest stalled for its full 10s timeout on CI while
passing locally. The stall is a race in InProcessWebSocket.connect():
server.connect() runs the session's connect-time policies synchronously,
so FullAuthPolicy's AUTH challenge reached the client's listener before
the socket assigned its `incoming` channel and before onOpen fired —
breaking the WebSocketListener contract (no onMessage before onOpen).
RelayAuthenticator answers that challenge on its own coroutine. When the
signed AUTH reply hit send() before the connect thread reached the
`incoming` assignment, send() returned false and the reply was silently
dropped. Nothing recovers from that: the challenge is already dedup'd as
answered, and an EVENT rejected with OK-false `auth-required:` never
re-triggers auth (only a CLOSED does), so the pending gift wrap was
never resent — exactly the CI signature (10.011s, no auth activity
between the authenticator's Init and Destroy logs).
Server->client frames now go through an outbound channel drained by a
coroutine started only after onOpen, so every connect-time frame reaches
the listener with the socket fully wired. Order is preserved by the
single drainer, same as the existing inbound path.
Both new InProcessWebSocketTest cases fail deterministically without the
reorder (the challenge always outran onOpen; a reply sent from the first
onMessage was always rejected) and pass with it, on top of the full
:geode:test and :quartz:jvmTest suites.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bf1Y91sfjxwi2ig4ymGTA9
Native targets reject a comma inside a backticked name, so five tests
that read fine on JVM broke every Kotlin/Native build. Renamed without
them.
The override parameters now match RelayConnectionListener — pingMillis,
compressed, cmdStr, cmd, msg, errorMessage — which silences six warnings
and, more to the point, fixes a misreading: onConnected's second and
third parameters are the connection's ping and whether it is compressed,
and I had them named attempt and success.
Both were missed the same way: jvmTest passes without ever compiling the
native TEST sources. All five targets now compile, main and test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
commonMain, so it compiled on JVM and broke every native target. Sorted
into a LinkedHashMap instead, which is the same output everywhere.
Found by CI on iosSimulatorArm64 because I had only compiled the JVM
target locally; all five now build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A monitor normally probes — opens connections purely to measure, then
throws them away. A client that is already subscribing, fetching and
publishing has better data for free: measured under real load, against
the relays it actually uses, at the concurrency it actually runs.
RelayObserver is a RelayConnectionListener, so it sees every connection
whichever code path opened it and none of them has to report anything:
rtt-open onConnecting to onConnected
rtt-read first REQ to its EOSE
rtt-write first EVENT to its OK
reachable it opened, or served something
auth-required it sent AUTH, or CLOSED saying so
the error, verbatim, when it never opened
Everything is OBSERVED. Nothing is copied from a relay's NIP-11: that is
the relay's own claim, available to anyone who asks, and republishing it
under a monitor's signature adds nothing but a chance to go stale. Where
the two disagree — a relay advertising open reads that then challenges
us — the observation is the half worth having, and copying the claim
would erase it. It also keeps quartz free of an HTTP dependency.
RelayMonitor is the whole wiring: construct one and connections are
measured, signed as 30166s on an interval, and folded into a cheap
in-memory isKnownDead for picking relays. That read has to be cheap — an
outbox picker runs per event — so it answers from a snapshot refreshed on
an interval, never a store query.
The signer is required. Measuring relay quality and letting others check
it IS NIP-66, and an optional signer would just add the failure mode this
library keeps designing out: configured, silent, doing nothing. A client
that should not publish does not construct one.
RelayObserver also replaces the CLI's RelayDiagnostics, which was the
same listener minus the timings. Porting it surfaced a bug both shared:
substringBefore(':') returns the WHOLE string when there is no colon, so
a relay's free-form CLOSED prose became its own tally key and the map
grew with the number of distinct sentences relays wrote. The colon is
now required.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
javap showed the hexToByte/byteToHex field re-loaded on every use inside
these methods (16 times per readLong call) — the JVM/ART doesn't reliably
prove the load loop-invariant. Hoisting it into a local measured ~25%
faster for decode and ~10% for isEqual and readLong on the JVM
(4096 random 32-byte ids, best-of-150 rounds, 3 repeats); encode was
neutral on HotSpot but is hoisted too since ART is historically worse
at this (see the internalIsHex comment).
Branchless variants of isHex/isHex64 were also measured and were a
wash-to-slightly-worse than the branchy early-exit versions on valid
input, so those keep their current implementations.
Also adds the new exact-size codecs to the on-device HexBenchmark
(decode64, decode64OrNull, encode64, decode128, encode128, toLong256,
and the old isHex64+decode two-pass for comparison) so ART numbers can
be collected with the existing benchmark harness.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hwv6XwT9mwGUQc57zH4ky4
javap on the previous version showed the hexToByte field re-loaded
twice per iteration, the trip count as a runtime parameter, and the
whole loop wrapped in an exception table (only needed because chars
above 0xFF overflow the 256-entry lookup table).
Now the table is hoisted into a local, the function is inline so the
32/64-byte length becomes a compile-time constant at each call site,
and out-of-range chars are rejected branchlessly: the index is masked
with 'and 0xFF' so it cannot overflow, while '255 - code' goes negative
for any char above 0xFF and is folded into the same sign-bit
accumulator that already catches invalid hex digits. No try/catch, no
exception table, no branches in the loop.
Measured on the JVM (4096 random ids, best-of-200 rounds, two runs
with variant order reversed to rule out JIT profile artifacts):
~25% faster than the previous version and ~2x faster than the
isHex64 + decode two-pass combination.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hwv6XwT9mwGUQc57zH4ky4
Adds exact-size codec entry points to the Hex utility so 32-byte
pubkeys/event ids (64 chars) and 64-byte signatures (128 chars) with the
wrong size or invalid characters are rejected instead of silently
decoded:
- decode64 / decode128 throw IllegalArgumentException; the OrNull
variants return null for untrusted input.
- encode64 / encode128 require exactly 32 / 64 input bytes.
The decode is single-pass: character validation is folded into the
decode loop via a sign-bit OR-accumulator (the lookup table yields -1
for invalid chars), so it is faster than the isHex64 + decode
two-pass combination.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hwv6XwT9mwGUQc57zH4ky4
Replaces the hex-string cache key with a small CacheKey holding the raw
(privateKey, pubKey) references — no byte copy and no ~400-byte hex String per
lookup. The precomputed Int hash is only a bucket selector; equals() does the
authoritative full-content comparison, so collisions share a bucket and are
disambiguated rather than returning the wrong peer's secret. Same value-type
immutability contract already relied on by X25519KeyPair as a map key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PY7kpiTcFiDshYqBsQ5KVC
Two hardening fixes in the NIP-44/NIP-04 encryption primitives:
1. SharedKeyCache keyed conversation/shared secrets by a 32-bit polynomial
hashCode of (privateKey || pubKey). Distinct peers whose bytes hash to the
same Int collided on one LRU slot, so get() could return one peer's key for
a message meant for another — a silent wrong-key encrypt/decrypt. The hash
collides independently of the private key, so a pubkey aliasing a victim's
contact is grindable. Now keyed on the full (priv || pub) byte content.
Added SharedKeyCacheTest, which fails on the old hashCode key and passes now.
2. NIP-44 MAC verification (Hkdf.fastExpand and Nip44v2.checkHMacAad) used
ByteArray.contentEquals, which short-circuits on the first mismatching byte
and leaks a timing side channel on the HMAC tag. Switched to a shared
equalsConstantTime helper (utils/ConstantTime.kt), matching the constant-time
tag checks already used in ChaCha20Poly1305/XChaCha20Poly1305/MLS.
Also corrects the X25519 KDoc, which claimed the JVM/Android actual delegates to
java.security XDH; it is in fact the same pure-Kotlin Montgomery ladder as the
other targets.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PY7kpiTcFiDshYqBsQ5KVC
ServiceType.parse destructured the result of split(":", limit = 2) into
two components, so any value without a colon (e.g. the "client" of a
["client", "nostria"] tag) threw IndexOutOfBoundsException instead of
returning null. It also accepted "30382:" as an empty service type.
ServiceType.isOfKind had the same class of bug: it read
serviceType[kind.length] after startsWith, which is out of bounds when
the value equals the kind exactly ("30382" vs "30382").
Parse the separator by index and check the length before indexing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112ysMEzSaezH9mXFteNt13
contactMetaData() decodes kind-0 content with the lenient JsonMapper, but
contactMetadataJson() ran the strict default parser. Profiles that sit in
that gap (bare keys, unquoted values) rendered fine yet read back as null,
so updateFromPast() started from an empty map and silently dropped every
field Amethyst does not edit itself the next time the owner touched their
profile — the exact thing its "tries to not delete any existing attribute
that we do not work with" contract promises not to do.
Both accessors now use JsonMapper.jsonInstance. The lenient reader tags
unquoted tokens as strings, so re-encoding still emits valid JSON.
Also pins the behaviour of two malformed kind-0s seen in the wild — a
JavaScript object literal and a value truncated by stray quotes. Neither
is recoverable by any parser; the test records that they are dropped with
a warning and never throw.
The always-on notification says "connected to N relays" and nothing more. That
count is emergent, not curated: NotificationRelayService owns no subscriptions,
and nothing closes a socket when it stops being useful, so a relay stays
connected exactly as long as some filter still references it. Neither a user nor
a developer can tell whether those N relays are carrying DMs or re-dialling a
stale outbox hint.
The client already tracks every in-flight REQ per relay with its filters
(INostrClient.activeRequests, which the Connected Relays screen reads). A filter
says *what* is matched — kinds, authors, tags — but not *who asked*, and subIds
are random. This adds the missing half.
`Filter` becomes `open`, with `copy()` open too, and commons gets
`ExplainedFilter` carrying a `SubPurpose`. The purpose rides on the filter, so it
arrives with the data the relay screens already read — no parallel registry to
keep in sync or leak on teardown.
**It never reaches a relay.** FilterSerializer is registered against Filter and
writes an explicit protocol field list, and Jackson applies a serializer
registered for a class to its subclasses — so an ExplainedFilter serializes to
byte-identical JSON. That is the point: telling relays what each REQ is *for*
would hand them a ready-made fingerprint of client intent and correlate
subscriptions that are deliberately kept apart.
`copy()` is overridden because filters are copied on the live path — assemblers
call copy(since = …) after every EOSE. Inheriting the base implementation would
downgrade to a plain Filter on the first window refresh, so the purpose would
survive the opening REQ and vanish seconds later.
Both invariants are pinned by tests, and both were mutation-checked rather than
assumed:
- removing the copy() override -> `copy preserves the purpose` fails
- unregistering FilterSerializer -> 3 tests fail, one reporting the literal
leak: `purpose leaked to the wire: {…}`
A test also asserts FiltersChanged does not see the new field, so tagging a
filter cannot trigger a re-REQ storm across ~400 relays.
Wired three assemblers as proof (metadata, reactions, outbox finder) and surfaced
the derived set on BasicRelaySetupInfo.purposes for the Connected Relays screen.
Verified on device: 8 relays attributed RELAY_LISTS through
client.activeRequests() — purpleplag.es, user.kindpag.es, indexer.coracle.social,
directory.yabu.me and friends, which is semantically right — across 2,709 sent
REQs containing zero occurrences of "purpose" or any SubPurpose name.
The remaining assemblers are untagged, which is why `purposes` is documented as
"not yet attributed" rather than "idle". Notification-popup copy is deliberately
not built yet: it should describe the measured background grouping, not the
intended one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four kind-0 events collected from relays exposed two content-parsing
problems. Both `MetadataEvent.contactMetaData()` and `contactMetadataJson()`
are shared code, so Jackson (JVM/Android) and kotlinx (native) were equally
affected — verified against both mappers.
Empty content is a valid, empty profile — someone wiping their metadata —
not a parse failure. It logged "Content Parse Error" and returned null,
which made `LocalCache.consume` drop the event, so the stale profile stayed
in place forever even though a newer replaceable event had arrived. Blank
content now decodes to a blank `UserMetadata` (and an empty `JsonObject`).
A string field wrapped in a one-element array (`"nip05":["a@b.com"]`) has
only one possible reading, so `TolerantStringSerializer` now unwraps it
rather than silently dropping the user's NIP-05 verification. Empty,
multi-element, and non-primitive arrays stay ignored.
The other two events already behaved correctly and are pinned as
regressions: `"nip05":{}` with foreign client keys, and Ditto's ambiguous
`"birthday":"10-24"` — both drop just the offending field.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XGYcqfBKsP9SCvRdPzdkSx
An entity edited after a CORD-06 Refounding was frozen at its pre-Refounding
version and its newer state silently discarded on every refold. Observed on
device: entity e83ee182 of a real community held at v1 across 21 consecutive
refolds while the current epoch offered v2 citing v1's own hash.
`EditionFold.foldEntity` anchored the walk by requiring the offered set to
*contain* the floor edition. But a Refounding re-wraps ONE edition per entity
(CORD-06 §3, "the last Control Plane state is simply rewrapped"), so an entity
edited afterwards has only its successor on the new epoch while the floor
edition stays behind on the old one. `prev == floor.hash` is a stronger proof
the chain connects than mere presence, and it was being ignored. Worse, the
gap made `admissible` — the pre-filter every derived fold shares — keep only
`version < floor.version`, so the newer edition was dropped every refold: a
role edit, channel rename or banlist entry reverting itself for that user.
Checked against Armada (semantics only, AGPLv3) and the canonical spec before
changing consensus behaviour. Armada selects an arm per entity, and Amethyst
was missing both halves:
1. Its chain-walk arm has three anchor branches to our one; the missing
`versions[0] === floor + 1n -> bytesEq(lo.prevHash, floorHash)` is now
implemented.
2. Once an entity has been re-wrapped into the epoch being folded it does not
chain-walk at all — it anchors on version alone (`bootstrapHead`), because
behind a compaction dangling `prev`s are normal and, since seal signatures
survive re-wrap, any group-key holder can re-serve a genuine old edition
under the current group. A re-wrap cannot raise the version inside the
signed seal, so version is what bounds that. This is the half that fixes
the observed pin; branch 1 alone would still refuse a floor-v1 entity whose
only served edition is v3.
Implemented as an optional `snapshot` (the rumor ids of the epoch being
folded), defaulting to null = pure chain walk for every other caller.
`ConcordCommunityState.fold`/`authorizedHeads` capture it from their own
editions argument before `admissible` re-seats older-epoch heads — Amethyst
folds exactly one epoch per call, so the argument is the snapshot. `admissible`
and `candidates` now ask `foldEntity` whether it gapped rather than
re-deriving the test, so the three sites cannot drift apart again.
`LOG_GAP` also dedups on (entity, floor, offered): the same refusal was
re-reported on every refold, 22 byte-identical warnings a boot, which read as
22 attacks rather than one unchanged state. Deduping is what made the two
genuinely distinct refusals visible and led here. Fails open above 4096
distinct refusals — a flood is when the warnings matter most.
Anti-rollback is unchanged: all 12 pre-existing floor tests still pass, and
three new negative tests cover the fork, the below-floor re-serve, and an
entity absent from the snapshot. Device gaps for the pinned entity: 21 -> 0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A cold start emitted 6,198 app lines in 80s (~77/sec) on a real device.
Six tags produced 83% of it, and the lines that are actually actionable —
relay protocol refusals like `auth-required`, `rate-limited`,
`unsupported: too many filters` — were buried among them.
`Log.minLevel` was `DEBUG` in debug and `ERROR` in release: two settings,
both wrong. Release dropped all 460 `Log.w` call sites, so the field never
saw a single relay refusal. Debug kept everything.
Now debug defaults to INFO and release to WARN, with `Amethyst.VERBOSE_LOGS`
to restore the firehose. 333 files already route through `quartz.utils.Log`
(only 13 use raw `android.util.Log`), so `minLevel` is a real global gate —
which is why three of the four loudest buckets needed no code change at all:
relay socket lifecycle (1,659 lines), `RelaySpeedLogger` (1,308) and Arti's
per-stream SOCKS errors (753) were already `Log.d`.
The rest:
- `RelayLogger.onCannotConnect` E -> D. Under the outbox model a boot dials
~400 relays and most fail identically; one line per dead socket is not
actionable, and its multi-line TLS certificate dumps were 3-4 logcat lines
each. This one mattered most: at `Log.e` it survived every level.
- `BootRelayDiagnostics` rollup -> INFO, per-relay WASTE/SERVE tables -> DEBUG.
The census line carries the aggregate the 655 per-socket errors spelled out.
- `Duplicated/SPAM` -> DEBUG: a detection is the filter working, and it is
already reported via `relayStats.newSpam()` and `flowSpam` to the UI.
- `MarmotDbg` "not a member of group" -> DEBUG: relays serve kind:445 for
every group they carry, so this is the steady state. It was burying the
real warning next to it ("Generation N already consumed").
Measured on emulator (4 accounts, Tor on): 6,198 -> 433 lines. Flipping
VERBOSE_LOGS gives 5,574 back, so nothing was deleted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two channel-management gaps vs the Buzz interface (both ride kind-9002 tags; no
new protocol), verified against block/buzz:
Archive/Unarchive — the reversible hide-from-the-sidebar the Buzz client has,
distinct from delete. EditMetadataEvent gains an `archived` tag; Account/
AccountViewModel expose archiveRelayGroup; the channel and forum top bars offer
Archive/Unarchive to admins (no confirm — it's reversible). The relay stamps
`["archived","true"]` on the 39000, so GroupMetadataEvent.isArchived() /
RelayGroupChannel.isArchived() read it directly; the community list drops archived
channels out of Channels/Forums into a collapsed "Archived" tail from which they
can be reopened and unarchived.
Visibility-on-edit — a Buzz relay reads its own `visibility` (open/private) tag,
not the NIP-29 `private` status flag, so the edit screen's private toggle was a
silent no-op on Buzz. editRelayGroupMetadata now sends the `visibility` tag on
Buzz relays (status flag still sent for vanilla NIP-29).
Not gaps (checked): topic/purpose/TTL are in the relay's system-message
vocabulary but not extracted on 9002/9007, so there's nothing to mirror.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016MNVEKhaAu4vQRZnXv3rfG
Address composer feedback:
- The annotation field is now the short-note composer's rich MessageField
(@-mention + custom-emoji autocomplete, inline previews). On publish it becomes
the highlight's `comment` tag, with the mentions (`p`), emoji, URLs (`r`),
hashtags and quotes it references emitted as their own tags — via
NewMessageTagger + a new tag-builder initializer on HighlightEvent.build().
- A nostr source now renders as a reply-style preview card (NoteCompose, quoted
style) instead of showing a URL field; the URL field appears only for web
sources. The source note is resolved from the a/e coordinate carried in the
route.
Verified with :quartz:jvmTest and :amethyst:compileFdroidDebugKotlin.
- Add a "Highlight" action to the note action menu (3-dot menu + chat long-press
sheet). It opens the NIP-84 composer pre-tagged with the note as source — an
`a` reference for an addressable article, else an `e` reference, plus the
author `p` — with the passage left for the user to type/paste. Prose kinds
only (text notes, long-form), and never a private rumor (a public highlight
would leak an e-tag of the unsigned rumor). This is the in-app entry point for
"post with the source as an event".
- Remove the live preview from the New Highlight screen per review.
- Add two real-world regression tests for the shared-highlight parser: Chrome
"copy link to highlight" with a prefix/suffix fragment (incl. an encoded
hyphen inside the prefix) and a long start-only fragment with encoded commas.
Verified with :quartz:jvmTest and :amethyst:compileFdroidDebugKotlin.
Redesign the New Highlight composer as a pull-quote you craft: a rounded tonal
hero card with a quotation-mark watermark, a highlighter-amber accent bar, and
the passage in large type, plus a live highlighter-pen preview (reusing the feed's
HighlightedQuote) and icon-led source/note fields.
Also extend HighlightEvent.create()/build() to emit a/e/p tags, so the builder
now covers every NIP-84 source — a web page (r), a nostr article (a), a nostr
note (e), each with optional author attribution (p) and context — not just web
highlights. Route.NewHighlight and the composer ViewModel carry the nostr source
(address/event/author) and context through so a future in-app "highlight this
passage" action can open the composer for a nostr article/note.
Covered by a new builder test for the a/e/p tags; verified with :quartz:jvmTest
and :amethyst:compileFdroidDebugKotlin.
`trimUrlEnd` strips a wrapping `)` off the URL token, but the `(` it opened
stayed at the tail of the passage, so sharing
`See this quote (https://example.com/article)` published a highlight whose
content read `See this quote (`.
Trim an unbalanced bracket left at the end of the passage after the URL token
is removed. Only an unbalanced one at the very end goes, so `He said (see
below) https://…` keeps its matched pair, and `.../Mercury_(planet)` — where
nothing was trimmed off the URL — is untouched on both sides.
The existing paren tests only asserted `result.url`, which is why this slipped
through; the new cases assert `result.quote` too, including a regression guard
for the slug-paren direction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The URL trailing-punctuation trim now leaves a closing bracket in place when
it is balanced within the token, so a shared Wikipedia link like
`.../wiki/Mercury_(planet)` keeps its `)` while a wrapping `(https://…)` still
loses the paren the surrounding prose added.
Adds a "New Highlight" share target and composer so a user can select text in
a browser, hit Share → Amethyst, and publish it as a kind:9802 highlight.
- Manifest: a text/plain-only ShareAsHighlightAlias activity-alias, matched at
runtime by ShareIntentRouting.isShareAsHighlight (mirrors the Send-as-DM
alias pattern).
- AppNavigation: both the launch-intent and warm onNewIntent parse blocks now
branch on the highlight alias, run the shared text through
SharedHighlightParser, and open Route.NewHighlight pre-split into
quote/url/prefix/suffix.
- NewHighlightScreen + NewHighlightPostViewModel: a trimmed composer (passage,
source URL, optional note — no polls/zaps/media/scheduling) that publishes via
account.signAndComputeBroadcast(HighlightEvent.build(...)).
Also adds HighlightEvent.build(), the unsigned EventTemplate counterpart of
create(), for the sign-and-broadcast pipeline. Verified with :quartz:jvmTest
and :amethyst:compileFdroidDebugKotlin.
Adds a shared-highlight parsing layer under nip84Highlights/parse that turns
the free-form text a browser hands Amethyst on "Share selection" into the
pieces of a kind:9802 highlight:
- SharedHighlightParser normalises selection-only, selection+URL, URL-only and
"link to highlight" (#:~:text= fragment) shares into a SharedHighlight.
- TextFragmentParser decodes/strips WICG text-fragment directives (prefix,
start, end, suffix), leaving literal '+' verbatim.
- UrlTrackerCleaner strips utm_*/fbclid/etc. from the source URL per NIP-84's
"clean the URL from trackers" guidance, preserving path and fragment.
Also adds a HighlightEvent.create() builder overload that assembles the r,
textquoteselector, context and comment tags from parsed data, centralising
what the desktop publish action does by hand.
Covered by commonTest suites for each piece plus a builder round-trip.
A kind:9802 highlight that mentions users before its author rendered the
wrong name and a stray "published by …" caption.
- HighlightEvent.author() took the first p tag regardless of role, so a
highlight with leading "mention" p tags was attributed to a mention
instead of the "author"-marked one. Prefer the NIP-84 author marker,
falling back to the first p tag for highlights (including Amethyst's own)
that omit the marker.
- DisplayEntryForNote used the source note's title/subject/alt as a caption.
For a kind-1 note there is no title/subject, so it fell to the NIP-31 alt
tag — which clients like Jumble fill with a generic "This event was
published by https://jumble.imwald.eu." line. Drop alt from the lookup and
name the source by its event kind instead, kept clickable to the note.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LJh3QYonEa9hw6mJcnrmve
Add a CONTINENT(1) level to GeohashChannelLevel so the coarsest selectable
geohash channel is a single character (~5000 km, one of 32 cells for the
globe), previously floored at REGION(2). Every precision picker iterates
GeohashChannelLevel.ordered, so the map picker, Teleport, "Near me" list and
New-geohash-chat all pick it up automatically; GeohashChatsScreen now labels a
1-char cell "Continent" instead of showing no level.
Adds the "Continent" label and "~5000 km" chip subtitle, and extends the
GeohashChannelLevel test coverage. REGION..BUILDING (2-8 chars) still mirror
the Bitchat channel levels; CONTINENT is an Amethyst extension beyond them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vtvo2iNSnG3M21QwbFPznU
Both nostr proposals merged just now (82e72369, bdb4b03e) referenced types
by fully-qualified name inside function bodies, which CLAUDE.md's Kotlin
style rule forbids. Merged them as authored rather than rewriting someone
else's patch mid-merge; this is the follow-up.
- NamecoinNameResolver: import kotlinx.coroutines.CancellationException.
Both catch sites were inline-qualified (one added by the proposal, one
already there), and the sibling resolvers in this same package
(Nip05Client, UserHexResolver) already import it.
- desktop Main.kt: import the five notification symbols in the block the
proposal touched — the two Preferences* factories and the three
Local*Notification* CompositionLocals. Each occurs exactly once, so no
fully-qualified stragglers are left behind for those names.
Deliberately scoped to that block: Main.kt has ~130 other inline
fully-qualified names, and sweeping them belongs in its own change, not
tacked onto a style follow-up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merges nostr proposal bdb4b03e into main:
- Catch NamecoinLookupException in performLookup and return null, restoring
resolve()'s documented "null on any failure" contract. nameShowWithFallback
throws NameNotFound / NameExpired / ServersUnreachable, which escaped
resolve() and reached Nip05State.checkAndUpdate as a hard error.
- CancellationException is rethrown first so coroutine cancellation is not
swallowed.
- resolveDetailed() is unaffected: it goes through performLookupDetailed(),
which still distinguishes each failure reason.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Posts were publishing a flood of bogus `r` tags — the v1.13.0 release note
shipped 11 junk references such as `https://.deb/`, `https://window.nostr/`,
`https://kind:30166/` and `https://crowdin.pretended462/`.
The reference extractor `findURLs` fed the raw, scheme-less UrlDetector output
straight into `r` tags. That detector is deliberately eager: to let the
rich-text renderer linkify a bare `example.com`, it also reports every
`word.word`, `word/word` or `word:port` token with no real-TLD whitelist.
Prose is full of those (`.deb`, `.rpm`, `[database].backend`,
`nostr-wallet-connect/nwc`, `~2.5x`, `@mentions`), so each one became a
reference on the published note. The rich-text side already guards against
this via UrlParser (TLD validation + scheme separation); the tag path never
got the same guard.
Require an explicit http/https scheme and a valid TLD before a detected URL
becomes a reference. Rendering is untouched (separate parser), so bare domains
still show as links — they just no longer pollute the tags.
Adds regression coverage over the exact fragments from the v1.13.0 note plus
checks that real, explicitly-schemed links are still extracted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L8KXx8UQ6mgyxjBSWW6sQV
"Remove from Messages" was hardcoded on every surface, so it never showed
an "Add to Messages" counterpart for a channel that was already off the
list, and the Buzz workspace rows read a session-local snapshot of the
kind-10009 list that was seeded once and only ever grew — a channel taken
off Messages still rendered as a disabled "Added", leaving no way back.
Removal itself always worked (verified on-device: the kind-10009
republished and the row left Messages); what was missing was any read of
that list on the way back.
- RelayGroupListState: expose liveRelayGroupIds, the joined groups as
normalized GroupIds, so the UI can ask whether a channel is on Messages
without string-matching a raw relay url another client may not have
normalized the way we do.
- RelayGroupTopBar / BuzzImportRow: one Add/Remove toggle driven by that
flow. Remove no longer pops back — you stay a member reading the
channel, and staying is what makes the entry flip so the action is
visibly undoable. Leave still pops.
- BuzzRelayImportViewModel: track "added" against the live list instead of
a one-shot seed, and add remove(); add() now also clears the dismissal
so a relay's kind-44100 re-announcement isn't filtered back out.
- AccountViewModel: addRelayGroupToMessages() as the counterpart to
removeRelayGroupFromMessages(); acceptChannelInvite() delegates to it.
Buzz DMs had the same one-way shape for a different reason: hiding is a
relay-side per-viewer flag (kind-41012 -> the kind-30622 snapshot), and
rebuildRows dropped hidden DMs on the floor, so a hidden conversation was
gone for good. There is no unhide command — re-opening is the unhide, a
kind-41010 with the same participants resolving to the same canonical
channel. Hidden DMs are now projected into their own list behind a
collapsible "Hidden (N)" header, faded but still openable, each offering
"Add to Messages". Also added to the community view's inline DM rows,
which had no menu at all and are where DMs actually live — the full inbox
sits behind a "see all" row that only appears above six DMs, so in a small
workspace the hidden section would have been unreachable.
Both list screens now leave bottom room for the FAB, which the Scaffold's
padding deliberately doesn't account for; the last row's overflow menu was
sitting underneath it.
Adds SimpleGroupListEventTest covering the removal path, including that a
renamed channel still matches (removal keys on group id + relay only).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A kind:9802 highlight with no `context` tag falls back to reconstructing
the surrounding passage from the W3C `textquoteselector` prefix/suffix.
Those fragments are scraped from the source web page, so they carry the
page's block-boundary whitespace (runs of newlines/spaces between DOM
nodes). Glued in verbatim as `prefix + content + suffix`, they render as a
stack of blank lines above the marked quote.
Collapse each whitespace run in the prefix/suffix to a single space (and
trim the outer edges) in `HighlightEvent.contextOrReconstructed()`. The
highlight's own `content` is left verbatim so its offsets inside the
reconstructed context stay exact for the in-context marker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ApuEseGcFjUFqYoLhCuR91
Vitor PamplonaandClaude Opus 5
Vitor PamplonaandGitHub