Re-reviewed every finding against the shipping app rather than against the
protocol, and split the list in two: what a banned user can do with stock
Amethyst (our bugs) versus what needs a hand-written client (fix in the fold, or
defend against). Several items moved, and the review turned up a new one that
belongs at the top.
A1 is new and is the realistic attack. mintConcordInvite checks only that the
account is writeable and that we hold the community — no CREATE_INVITE, no
banlist — and unlike the Edit and channel buttons next to it, the invite
IconButton carries no guard at all. A banned user stays in the app, taps
person-add, and shares a working link to the community. The mint publishes a
fresh link signer, so revoking the links they were given does not touch the ones
they make; and because the bundle is a standalone kind-33301 outside the Control
Plane, the CREATE_INVITE bit the fold enforces on INVITE_* entities never
applies to the actual invite mechanism.
A3 is the general form: every moderation verb checks isWriteable() and the
Control write key and nothing else, so authority lives in the composable that
draws the button — and those gates use effectivePermissions, which is ban-blind.
Ban and Remove survive only because a second, unrelated condition routes through
the ban-aware canActOn. refoundConcordCommunity guards itself with
effectivePermissions outright, so a banned BAN-holder can launch a Refounding
from the shipping app; honest receivers refuse it, but that is a race against
banlist propagation, not a check.
A2 moves to Part A because our own client is what performs it: the recovery
sweep runs every 15 minutes with no banlist check.
C2 (voice) is downgraded from High — ConcordBrokerToken and VoicePresence are
referenced nowhere outside quartz, so there is no shipping path to attack. It is
a note for whoever wires one up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj
The first pass was bounded by the Control Plane, the fold and the relay. Three
more findings from the surfaces it skipped, plus an explicit list of what is
still unexamined so the next reader knows where the edges are.
V10 is the serious one, and it forks. ConcordStrandedRecovery.isStranded takes
only (entry, bundle): no banlist check, no check that we were legitimately
re-keyed. The whole test is "the bundle at my stored invite_ref sits at a higher
epoch than I do", and the unlock token lives in the link fragment an ex-member
keeps forever. So whether a removed member walks back in depends only on whether
anything re-mints at that coordinate. Amethyst mints a fresh link signer per
invite and the Refounding neither re-mints nor revokes, so today nothing does —
which means stranded recovery never fires for anyone, and the cure that
drainConcordRekeys' KDoc points to for "a BAN-holder can evict anyone, the owner
included, by omission" does not actually exist. If any client does re-mint at a
stable coordinate, as CORD-05's design describes, then every removed member
auto-recovers the new root on the 15-minute sweep and re-announces a Guestbook
join. Either the safety net is missing or the only hard removal is undone;
which one it is needs a spec answer, not a patch.
V11: voice rooms authenticate with the channel's derived voice signer key
against a stateless SFU that holds no community secret and cannot know a banlist
exists, so a banned member keeps talking until a Refounding. V12: ingestTyping
filters on binding and self only, so they keep showing as "typing".
Checked and sound, recorded so they are not re-audited: the envelope pins
rumor.pubKey == seal.pubKey (no author impersonation), and Note.latestConcordEdit
is author-gated, so a member cannot rewrite someone else's message.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj
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
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
A soft ban leaves the community_root in the ex-member's hands, so they keep
deriving the channel's stream key. CORD-01 signs every wrap with that shared
key rather than with the author, so on the wire a Concord channel looks like a
single author publishing everything — and NIP-09/NIP-62 authorize on the outer
pubkey. Read naively that hands any ex-member a one-event wipe of the whole
community's history, and geode's own Nip09DeletionTest guarantee ("a kind-5
from pubkey X cannot delete pubkey Y's events") would be vacuous inside a plane.
It is refused, but only because of a rule written for something else:
Event.owner() gives a kind-1059 to its p-tag RECIPIENT rather than its signer,
and ConcordStreamEnvelope stamps a freshly random p-tag on every wrap. Each
wrap is therefore owned by a one-time key nobody holds, attacker included.
Neither half was written with this attack in mind and either one silently
re-opens it, so both are pinned: two tests fail if ownership ever moves back to
the signer, and a counterfactual (a wrap addressed to a real key IS deletable
by its holder) fails the moment that p-tag becomes anything a member holds.
Scope: this is our relay's rule, not the protocol's. A third-party relay that
authorizes deletion by matching pubkey still hands every ex-member a wipe
button, and a Refounding only protects the future.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DrJhpFhhLjuDJQNkGvYMGj
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
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.
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.
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`.
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.
A staff-making Grant delivers the `control_root` inside the fold itself
(CORD-04 §3), so it lands on an entry whose session was built as a
read-only member long before. `ConcordSessionRegistry.sync` only rebuilt a
session when `root`/`rootEpoch` changed, and `ConcordCommunitySession`
derived `controlKeys` once at construction — adoption changes neither, so
the live session kept `signer = null` and `canWrite == false` for the rest
of the process.
The promoted staffer saw their new role badge appear (that half reads the
folded `state` flow) while every write affordance stayed hidden and
`controlKeysForWrite` refused, until the app was restarted.
Rebuilding the session on the change is not the fix: the new session starts
with no buffered Control Plane wraps, so the community folds to "No channels
yet" until every wrap happens to be re-delivered. Instead refresh the key
material in place. Nothing about the plane moves — adoption is gated on the
secret deriving to exactly the `control_pk` already held (CORD-02 §5) — so
the address, read key, buffered wraps and subscription set are all
invariant, and only the signer appears. `adoptControlMaterial` fails closed
on a different community/root/epoch or an address change, leaving those to a
rebuild.
Verified on device (SM-T220, Android 14) against a loopback geode relay: an
account promoted to staff while sitting on the community screen gains the
edit/create affordances with no restart, keeps its folded channel list, and
its next Control edition lands on the wire signed by `control_pk`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ResourceUsageStore.persist() rewrites the whole file on every merge, and
merges fire on the accountant's 30s flush debounce while traffic flows.
Only today's bucket ever changes, so retention is a write-amplification
setting as much as a history setting. Nothing reads past 7 days.
- fix(resourceusage): atomic subscription map, and build the report off Main
- fix(resourceusage): bound the technical dump so the report stays sendable
- an orphan KDoc in ResourceUsageReportAssembler with no declaration
under it, which Kotlin silently bound to formatBytes
- paragraphs in ResourceUsageAccountant and ResourceUsageStore claiming
the key space has no fixed upper bound; every remaining counter is
compile-time bounded
- the user-facing privacy string, which claimed the report contains the
host names of the relays that reconnected most. It does not, and that
file is Crowdin-bound, so the false claim would have reached
translators.
The only way out of the Resource Usage screen was "Send report via DM",
which builds the text into a draft message to a fixed pubkey. Reading
your own report meant opening a composer and copying out of it.
Adds Copy and Share beside it, handing over the same string for a bug
report or a file. Reuses Clipboard.setText from ClipboardExt and the
ACTION_SEND chooser pattern from ShareActions.
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
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
`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.
- fix(media): stop ogg bypassing the ambiguity guard it is listed in
- fix(media): don't guess a family for an ambiguous bare subtype
- refactor(media): normalize the mime at the chokepoints, not one call site
A NIP-92 `imeta` is meant to carry a full `type/subtype`, but some clients
emit only the subtype — Primal iOS writes `m jpeg` instead of `m image/jpeg`.
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.
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>
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.
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.
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.
Its job was to stop a composer's pop from racing the IME close animation,
and that pop is `nav.popBack()` in all 11 call sites — exactly what the
ImeSettler on Nav now serializes. So it no longer carries the fix; a plain
BackHandler reaches the same place safely.
What it did still provide was the two-back convention: first back dismisses
the keyboard (via the system's own animation, which on recent Android follows
the gesture), second back leaves. That came at a price it did not used to
have. The mechanism is to NOT consume back while the keyboard is up and let
the IME consume it instead — so on any device or API level where the IME does
not, back reaches the NavController, which pops without ever running onBack
and silently drops the draft, since nothing else saves one. Now that Nav
settles, that failure would also be invisible: no stranded padding to hint at
it, just a missing draft.
A plain BackHandler has no such failure mode. It always consumes, so the
draft is always flushed, on the first back rather than the second.
KeyboardState.kt keeps keyboardAsState(), which is a separate concern —
AppBottomBar uses it to hide the bottom bar while typing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LfUMGWYu2uTSyh17JJonfN
Leaving a screen while the soft keyboard is still animating strands
`imePadding()` at keyboard height for the whole app — `WindowInsets.ime` is a
single shared holder, so the padding survives leaving the screen that caused
it. PR #3864 fixed this for the post composers, at their call sites. That was
the wrong altitude: Search strands it too, and Search has no BackHandler and
no top bar of ours.
Search is the clearest case: it focuses its field on arrival, so the keyboard
is up before the user has done anything, and every way out is a navigation — a
bottom-nav tab, a tapped result, back. Any destination that can focus a text
field can strand the padding on the way out. There are 174 files with text
input in this module; enumerating the screens was never going to converge.
Two facts make a central fix possible: every in-app navigation goes through
INav (there is not one `controller.navigate` outside navigation/navs/, and
nothing touches OnBackPressedDispatcher, navigateUp or popBackStack directly),
and every Nav method already runs inside `navigationScope.launch`. So Nav
awaits an ImeSettler before each transition: keyboard down, it returns
immediately and nothing changes; keyboard up, it clears focus, hides the IME
and waits for the inset to actually reach zero, bounded, so the two animations
never overlap. ObservableNav delegates to Nav and inherits it.
That subsumes #3864's call-site patches, so they are removed rather than left
as a second mechanism: ActionTopBar goes back to plain callbacks (which also
drops the composition-scoped deferral of onPost, so posting no longer depends
on the top bar staying composed), and KeyboardAwareBackHandler keeps only its
imeAnimationTarget gate — the part that stops back falling through and
silently dropping a draft. It is now a UX preference (let the system animate
the dismissal) rather than the safety mechanism.
NavImeSettleTest pins the ordering: each transition must settle before it
navigates, and a settler that suspends must hold the navigation back rather
than run alongside it. All four fail with the settle calls removed.
Known gap: on a screen with no BackHandler the system's back pops through the
NavController directly, not Nav.popBack(), so a second back landing inside the
~250ms retraction can still race. Closing it needs a shell-level handler
registered after the NavHost to outrank its back callback, which is a
composition-order dependency subtle enough to break silently — worth a
deliberate decision rather than smuggling in here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LfUMGWYu2uTSyh17JJonfN
Merges nostr proposal 9142a140 (v2) into main:
- feat(gitRepositories): add ngit-specific search on the Git Repositories screen
- refactor(commons): move GitRepositorySearchMatcher to commons/search
Adds a client-side filter to the Git Repositories screen that searches the
fields NIP-34 kind:30617 announcements actually carry — name, `d` identifier,
description, hashtags, clone/web/relay URLs, maintainer and author pubkeys
(hex or npub), and the earliest-unique-commit hash. Before this, the only
search affordance on the screen navigated away to the generic Route.Search,
which matches people/notes/hashtags/channels but never repositories. A filter
icon toggles an inline text field over the feed; the generic search icon stays
beside it. Whitespace-separated terms are ANDed, matching is case-insensitive
substring, and `npub1…` queries are decoded to hex before matching.
The matcher itself lives in commons/search (per commons/ARCHITECTURE.md:
"event search filtering/ranking", non-UI and CLI-safe) rather than in
amethyst, since it is pure platform-agnostic Kotlin over a Quartz event type.
Desktop can reuse it when that screen grows the same affordance. Its test sits
in commons/commonTest on kotlin.test, so it also covers the iOS targets that
source set compiles for.
Verified before merge: :commons:jvmTest 15/15, :commons:verifyKmpPurity,
:commons:compileTestKotlinIosSimulatorArm64, :amethyst:compileFdroidDebugKotlin.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The matcher is pure platform-agnostic Kotlin over a Quartz event type —
no Compose, no Android, no platform APIs — so per the sharing philosophy
it belongs in commons rather than amethyst. commons/ARCHITECTURE.md
assigns "event search filtering/ranking" to the `search` package
(non-UI, CLI-safe), which is where it lands. The desktop Git
Repositories screen can now use the same matcher instead of growing its
own copy.
The test moves to commons/src/commonTest and swaps org.junit for
kotlin.test, matching every other test in that source set. That gains
iOS coverage for free, and keeps the source set compiling for the
native targets — commonTest is built for iosArm64/iosSimulatorArm64 too,
so a JUnit import there is a build break, not a style nit. The test
builds GitRepositoryEvent from raw tags and never signs, so it needs no
secp256k1 binding.
Also drops `filter()`. It had no caller — the screen filters the loaded
feed itself with `matches` — and its KDoc promised behaviour it never
implemented ("duplicate `d` tags collapse to the newest event"; it did
no deduplication at all). Better to delete the unused API than to ship
a dedup nobody asked for or a doc comment that lies. Its two tests go
with it; the empty-query contract the screen does rely on stays
covered.
Verified: :commons:jvmTest (15/15 in the new location),
:commons:compileTestKotlinIosSimulatorArm64, :commons:verifyKmpPurity,
:amethyst:compileFdroidDebugKotlin, spotlessApply — all green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a client-side filter for the Git Repositories page that only searches
fields relevant to ngit repository announcements (kind:30617 NIP-34):
repo name, `d` identifier, description, hashtags/topics, clone URLs, web
URLs, maintainer relays, maintainer/author pubkeys (accepting both hex and
`npub…` bech32 in the query), and the earliest-unique-commit hash.
Before this change, the only search affordance on the screen was the
generic Nostr search icon that navigated away to `Route.Search`, which
matches people, notes, hashtags, and channels — none of which are ngit
repositories. Users who wanted to find a repo they'd already discovered
had to scroll through the full follow-list-scoped feed.
UX:
- A filter icon in the top bar toggles an inline `OutlinedTextField`
directly above the feed. First-appearance focus opens the keyboard
without a second tap.
- The general search icon is preserved beside the filter icon so
outbound searches still work.
- While filtering, results render with the same `NoteCompose` cells the
feed uses so every affordance (bookmark, open, share) still works.
- Filtered rendering uses a scoped `LazyListState` because the item-key
set of the filtered list is not stable against the feed's cached
scroll offset; sharing them would jump the user to an unrelated repo.
- Closing the filter icon clears the query, restoring the full feed
in one tap.
Filter semantics:
- Whitespace-separated terms are ANDed against each repo (`amethyst
nostr` matches only repos that carry both terms in some indexed field).
- Case-insensitive substring match on each indexed field.
- `npub1…` queries are decoded to hex before matching, so a maintainer
can be found by either encoding.
Tests: `GitRepositorySearchMatcherTest` (17 hermetic cases) pins every
indexed field, plus the "empty query returns nothing / filter blank
returns everything" contract that the caller relies on to skip the
filter path.
Build check: `./gradlew :amethyst:compileFdroidDebugKotlin
:amethyst:testFdroidDebugUnitTest --tests
'com.vitorpamplona.amethyst.ui.screen.loggedIn.gitRepositories.GitRepositorySearchMatcherTest'
:amethyst:spotlessCheck` all green.
Merges nostr proposal 3ac62492 (v2) into main:
- ci: publish windows-arm64 desktop + windows amy/geode release assets
- ci(release): build windows-arm64 desktop as a portable zip only
Extends the release matrix to Windows on Arm using the free public-repo
windows-11-arm runner, and adds Windows legs (x64 + arm64) to build-cli and
build-geode. amyImage / geodeImage now emit both a POSIX launcher and a
.bat launcher so the flat image layout is uniform regardless of build host;
collect_cli_assets / collect_geode_assets package it as .zip on Windows and
tar.gz everywhere else.
The arm64 desktop leg ships the portable .zip only — no MSI. jpackage
--type msi shells out to WiX 3's heat/candle/light and the Windows 11 Arm64
runner image has no WiX (windows-latest has WiX 3.14 preinstalled, which is
why the x64 leg still packages an MSI). Installing it would mean pulling an
archived, x86-only toolchain (wixtoolset/wix3 archived Feb 2025; WiX 4+
dropped the candle/light CLI jpackage drives) into the job that publishes
signed release assets.
BUILDING.md now enumerates the expected release assets per matrix leg
instead of carrying a stale total: 14 desktop + 13 Android + 10 amy +
10 geode = 47.
The arm64 runner legs themselves are first exercised by the next real
release run; nothing about them can be verified harder locally, since
jlink/jpackage cannot cross-compile.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude
Vitor PamplonaandGitHub
Vitor PamplonaandClaude Opus 5
vitorpamplonaandgithub-actions[bot]
davotoula
davotoulaandgithub-actions[bot]
Alex Gleason
mstrofnoneandVitor Pamplona