Keep both halves of a NIP-65 relay list, and publish only to one #12

Open
laoc wants to merge 2 commits from feat/nip65-outbox into fix/bunker-relogin
Owner

Issue 0ac127e0. Stacks on #9 (fix/bunker-relogin), because the code it changes is the fetchRelayList / seedRelaysFromNip65 path that #7 rewrote.

Item 1 of the issue's suggested order ships here. Items 2 and 4 do not, and the reason is a number, not effort — see below.

The bug

fetchRelayList filtered relay tags with

it.size >= 2 && it[0] == "r" && (it.size < 3 || it[2] != "read")

on the grounds that "a read-only relay is not somewhere to publish". True, and the wrong half to discard: in NIP-65 a user's read relays are their inbox. Dropping them meant Stash could neither read from them nor show them in settings — a user whose relay list marks a relay read watched it vanish from "Your relay list".

What changed

Nip65.kt, pure. Marker rules out of the SDK call and into something testable. The decisions that are ours rather than the spec's are written down where they are made: an unrecognised marker falls back to the spec default (both halves) rather than dropping a relay the user did publish; markers are trimmed and case-folded; a relay named once read and once write merges to one entry doing both, deduped on the same key the rest of the app uses so wss://nos.lol and the SDK-normalised wss://nos.lol/ are one relay.

Publishing is aimed, not broadcast. Keeping the inbox half creates the risk it was wrongly protecting against — the working set is also the publish target, so un-dropping those relays would start putting bookmarks on relays the user marked inbox-only. publishUrls on the repository, prefs.publishRelays = app relays + write-marked. Null means "everywhere", i.e. exactly the old behaviour for anyone whose relay list has never been read. Passed at construction as well as at login, because a cold start of an already-signed-in account publishes before re-seeding runs. Cleared on logout with the rest of the account state.

Gossip: measured, works, deliberately off

ClientBuilder.gossip(NostrGossip.inMemory()) is the switch. ClientOptions.gossip(GossipOptions) alone is not — it reads identically to the negative control. Both established with controls on either side (a client told only about relay A returning an event that exists only on relay B, and returning nothing when built plain).

What stops it is the cost. Fresh relays, harness guard first, hit and miss authors measured in the same process:

client author first touch second
plain has a kind:10002 12 ms 35 ms
plain has nothing 40 ms 43 ms
gossip has a kind:10002 10057 ms 32 ms
gossip has nothing 10044 ms 39 ms

A flat ~10s on the first resolution of any author, hit or miss — I expected the miss to be the expensive case and it is not. Cached afterwards, but per author, not per client: three different unknown keys on one client cost 10055 / 10046 / 10045 ms. So the discover feed would pay it once per stranger and a cold start before the user's own bookmarks paint — a worse stall than the 12s one #1 removed.

Nothing exposed tunes it: GossipOptions carries only the five per-user relay counts, and NostrGossip has no methods at all, so the table cannot be pre-warmed. Enabling it turned the device suite from 84/84 into started=50 inside a ten-minute budget.

Issue item 3 (in-memory vs SQLite) is not a decision: inMemory() is the only backing in these bindings and NostrGossipInterface is an empty marker, so we cannot supply our own.

Verification

  • JVM 402/402, 0 failures — counted from the XML, not a tailed summary line.
  • Device 84/84, started == numtests, 152 s.
  • Mutation, Nip65.kt: 15/15 caught. One survivor on the first pass — deleting .trim() from the marker read changed nothing, because no test used a padded marker. Untrimmed, " read " falls through to the unrecognised-marker branch and widens an inbox-only relay to both halves, which is the exact bug this file exists to prevent. Test added, row re-run, now caught.
  • Red-first, marker fix: grafted onto the unfixed base in a throwaway worktree. Control passed (the relay list was readable), real assertion failed with the inbox relay is missing from the relay list: [ws://10.0.2.2:10547].
  • Red-first, publish routing: cannot be grafted (the mechanism does not exist on base), so proved by mutation instead — dropping the narrowing from sendToPublishSet fails exactly one test, bookmarks_are_not_published_to_an_inbox_only_relay, with the bookmark was published to a relay the user marked read-only.

OutboxE2ETest runs against two real relays because one cannot express "where we read" versus "where we publish"; e2e-direct.sh gains an ordinary relay on :10551. The publish test carries a positive control that the bookmark reached the write relay, so "landed nowhere" cannot pass as "correctly withheld". Its third test pins per-author routing as deliberately absent and names the 10s in its own failure message, so switching gossip on turns it red rather than leaving the decision to be rediscovered in three months.

Not verified: any of this on a real phone. Same standing caveat as the rest of the 0.12 stack.

Issue `0ac127e0`. Stacks on #9 (`fix/bunker-relogin`), because the code it changes is the `fetchRelayList` / `seedRelaysFromNip65` path that #7 rewrote. **Item 1 of the issue's suggested order ships here. Items 2 and 4 do not, and the reason is a number, not effort — see below.** ## The bug `fetchRelayList` filtered relay tags with ```kotlin it.size >= 2 && it[0] == "r" && (it.size < 3 || it[2] != "read") ``` on the grounds that "a read-only relay is not somewhere to publish". True, and the wrong half to discard: in NIP-65 a user's **read** relays are their **inbox**. Dropping them meant Stash could neither read from them nor show them in settings — a user whose relay list marks a relay `read` watched it vanish from "Your relay list". ## What changed **`Nip65.kt`, pure.** Marker rules out of the SDK call and into something testable. The decisions that are ours rather than the spec's are written down where they are made: an unrecognised marker falls back to the spec default (both halves) rather than dropping a relay the user did publish; markers are trimmed and case-folded; a relay named once `read` and once `write` merges to one entry doing both, deduped on the same key the rest of the app uses so `wss://nos.lol` and the SDK-normalised `wss://nos.lol/` are one relay. **Publishing is aimed, not broadcast.** Keeping the inbox half creates the risk it was wrongly protecting against — the working set is also the publish target, so un-dropping those relays would start putting bookmarks on relays the user marked inbox-only. `publishUrls` on the repository, `prefs.publishRelays` = app relays + write-marked. Null means "everywhere", i.e. exactly the old behaviour for anyone whose relay list has never been read. Passed at construction as well as at login, because a cold start of an already-signed-in account publishes before re-seeding runs. Cleared on logout with the rest of the account state. ## Gossip: measured, works, deliberately off `ClientBuilder.gossip(NostrGossip.inMemory())` is the switch. `ClientOptions.gossip(GossipOptions)` alone is **not** — it reads identically to the negative control. Both established with controls on either side (a client told only about relay A returning an event that exists only on relay B, and returning nothing when built plain). What stops it is the cost. Fresh relays, harness guard first, hit and miss authors measured in the same process: | client | author | first touch | second | |---|---|---|---| | plain | has a `kind:10002` | 12 ms | 35 ms | | plain | has nothing | 40 ms | 43 ms | | gossip | has a `kind:10002` | **10057 ms** | 32 ms | | gossip | has nothing | **10044 ms** | 39 ms | A flat ~10s on the first resolution of **any** author, hit or miss — I expected the miss to be the expensive case and it is not. Cached afterwards, but per **author**, not per client: three different unknown keys on one client cost 10055 / 10046 / 10045 ms. So the discover feed would pay it once per stranger and a cold start before the user's own bookmarks paint — a worse stall than the 12s one #1 removed. Nothing exposed tunes it: `GossipOptions` carries only the five per-user relay counts, and `NostrGossip` has no methods at all, so the table cannot be pre-warmed. Enabling it turned the device suite from 84/84 into `started=50` inside a ten-minute budget. Issue item 3 (in-memory vs SQLite) is not a decision: `inMemory()` is the only backing in these bindings and `NostrGossipInterface` is an empty marker, so we cannot supply our own. ## Verification - JVM **402/402**, 0 failures — counted from the XML, not a tailed summary line. - Device **84/84**, `started == numtests`, 152 s. - **Mutation, `Nip65.kt`: 15/15 caught.** One survivor on the first pass — deleting `.trim()` from the marker read changed nothing, because no test used a padded marker. Untrimmed, `" read "` falls through to the unrecognised-marker branch and widens an inbox-only relay to both halves, which is the exact bug this file exists to prevent. Test added, row re-run, now caught. - **Red-first, marker fix:** grafted onto the unfixed base in a throwaway worktree. Control passed (the relay list was readable), real assertion failed with `the inbox relay is missing from the relay list: [ws://10.0.2.2:10547]`. - **Red-first, publish routing:** cannot be grafted (the mechanism does not exist on base), so proved by mutation instead — dropping the narrowing from `sendToPublishSet` fails exactly one test, `bookmarks_are_not_published_to_an_inbox_only_relay`, with `the bookmark was published to a relay the user marked read-only`. `OutboxE2ETest` runs against two real relays because one cannot express "where we read" versus "where we publish"; `e2e-direct.sh` gains an ordinary relay on :10551. The publish test carries a positive control that the bookmark reached the write relay, so "landed nowhere" cannot pass as "correctly withheld". Its third test pins per-author routing as deliberately absent and names the 10s in its own failure message, so switching gossip on turns it red rather than leaving the decision to be rediscovered in three months. Not verified: any of this on a real phone. Same standing caveat as the rest of the 0.12 stack.
`fetchRelayList` filtered relay tags with

    it.size >= 2 && it[0] == "r" && (it.size < 3 || it[2] != "read")

on the grounds that "a read-only relay is not somewhere to publish". True,
and the wrong half to discard. In NIP-65 a user's **read** relays are their
**inbox** — where other people send them things. Dropping them meant Stash
could neither read from them nor show them in settings, so a user whose
relay list said `read` on a relay saw it vanish from "Your relay list"
entirely.

The marker rules now live in a pure `Nip65.parse`, away from the SDK call,
because they are rules and rules deserve tests. The decisions that are ours
rather than the spec's are written down where they are made: an
unrecognised marker falls back to the spec default (both halves) rather
than dropping a relay the user did publish; markers are trimmed and
case-folded; a relay named once `read` and once `write` merges into one
entry doing both, deduped on the same key the rest of the app uses so
`wss://nos.lol` and the SDK-normalised `wss://nos.lol/` are one relay.

Keeping the inbox half creates the risk it was wrongly protecting against.
The working set is also what we publish to, so un-dropping those relays
would start putting the user's bookmarks on relays they marked as inbox
only. So publishing is now aimed rather than broadcast: `publishUrls` on
the repository, `prefs.publishRelays` = app relays + write-marked. Null
means "everywhere", which is exactly the old behaviour for anyone whose
relay list has never been read. It is passed at construction as well as set
at login, because a cold start of an already-signed-in account publishes
before re-seeding runs.

Gossip is deliberately NOT enabled, and `client()` says why at length.
`ClientBuilder.gossip(NostrGossip.inMemory())` is the switch — measured,
with both controls, and `ClientOptions.gossip(GossipOptions)` alone is not.
What stops it is the cost: first resolution of ANY author is a flat ~10s in
the 0.44.6 bindings — 10057ms for an author whose kind:10002 is on the
connected relay, 10044ms for one with no relay list at all, against 12-43ms
for the same fetch on a plain client. Cached afterwards (~35ms) but per
AUTHOR, not per client, so the discover feed would pay it once per stranger
and a cold start before the user's own bookmarks paint. Nothing exposed
tunes it: `GossipOptions` carries only per-user relay counts and
`NostrGossip` has no methods to pre-warm. Enabling it turned the device
suite from 84/84 into `started=50` inside a ten-minute budget.

`OutboxE2ETest` runs against two real relays because one cannot express
"where we read" versus "where we publish" at all; `e2e-direct.sh` gains an
ordinary relay on :10551 for it. The publish test carries a positive
control that the bookmark reached the write relay, so "landed nowhere"
cannot pass as "correctly withheld". Its third test pins per-author routing
as deliberately absent and names the 10s in its own failure message, so
switching gossip on turns it red rather than leaving the decision to be
rediscovered.

Measured at this commit: JVM 402/402 (0 failures, read from the XML),
device 84/84 with started == numtests, 152s.

Refs: issue 0ac127e0

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deleting `.trim()` from the marker read changed nothing: no test used a
padded marker. Untrimmed, " read " falls through to the unrecognised-marker
branch and widens an inbox-only relay to both halves — the exact bug this
file exists to prevent, reachable from any hand-written relay list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/nip65-outbox:feat/nip65-outbox
git switch feat/nip65-outbox

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch fix/bunker-relogin
git merge --no-ff feat/nip65-outbox
git switch feat/nip65-outbox
git rebase fix/bunker-relogin
git switch fix/bunker-relogin
git merge --ff-only feat/nip65-outbox
git switch feat/nip65-outbox
git rebase fix/bunker-relogin
git switch fix/bunker-relogin
git merge --no-ff feat/nip65-outbox
git switch fix/bunker-relogin
git merge --squash feat/nip65-outbox
git switch fix/bunker-relogin
git merge --ff-only feat/nip65-outbox
git switch fix/bunker-relogin
git merge feat/nip65-outbox
git push origin fix/bunker-relogin
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
laoc/stash-bookmark!12
No description provided.