From 8b9875d9cc70ec51109e357c9637530f5a50aadf Mon Sep 17 00:00:00 2001 From: nrobi144 Date: Sun, 7 Jun 2026 14:36:29 +0300 Subject: [PATCH 1/6] fix(quartz): NIP-46 bunker double-resume + retry id-reuse races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two correctness bugs in `RemoteSignerManager` (NIP-46) and its NIP-55 sibling `IntentRequestManager`: 1. **Double-resume crash** — `awaitingRequests.get(id)?.resume(value)` was non-atomic. Multi-relay delivery, bunker echo/retry, and late-after-timeout responses could call `resume` twice for the same continuation, throwing `IllegalStateException: Already resumed` on a `Dispatchers.Default` worker. 2. **Retry id-reuse → wrong data** (NIP-46 only) — `launchWaitAndParse` built the request and event once, then re-used the same `request.id` across retry attempts. A late response from attempt N could resume attempt N+1's continuation with stale data. Replace the cached-`Continuation` map with the in-house Channel-per-request correlation pattern already used in `quartz/.../accessories/NostrClientPublishExt.kt` (`LargeCache(capacity=1)>` + atomic `remove` + `trySend` + `withTimeoutOrNull { receive() }`). Each retry attempt now builds a fresh request with a new id; the builder is still called only once. `finally`-block cleanup removes the cache entry on every path, incidentally fixing a slow leak on the success path. Adds three regression tests: - duplicate responses → no crash + single resume (fails on \`main\` with \`IllegalStateException\`) - late response after timeout → silently discarded - late attempt-1 response does not corrupt attempt-2 result (fails on \`main\`: the two attempts share an id) Design + review notes: \`quartz/plans/2026-06-03-fix-nip46-bunker-double-resume-plan.md\` --- ...-03-fix-nip46-bunker-double-resume-plan.md | 523 ++++++++++++++++++ .../api/foreground/IntentRequestManager.kt | 118 ++-- .../signer/RemoteSignerManager.kt | 67 ++- .../signer/RemoteSignerManagerRetryTest.kt | 210 +++++++ 4 files changed, 824 insertions(+), 94 deletions(-) create mode 100644 quartz/plans/2026-06-03-fix-nip46-bunker-double-resume-plan.md diff --git a/quartz/plans/2026-06-03-fix-nip46-bunker-double-resume-plan.md b/quartz/plans/2026-06-03-fix-nip46-bunker-double-resume-plan.md new file mode 100644 index 0000000000..03351fd0a4 --- /dev/null +++ b/quartz/plans/2026-06-03-fix-nip46-bunker-double-resume-plan.md @@ -0,0 +1,523 @@ +--- +title: "fix(quartz): NIP-46 bunker double-resume + retry id-reuse races" +type: fix +status: completed +date: 2026-06-03 +origin: docs/brainstorms/2026-06-03-nip46-bunker-double-resume-brainstorm.md +--- + +# fix(quartz): NIP-46 bunker double-resume + retry id-reuse races + +## Revision Note (2026-06-03, post-review) + +Three reviews (simplicity / architecture / pattern-recognition) converged on +pivoting the approach. Original plan used atomic-remove + `tryResume` on a +cached `Continuation` map. **Revised approach: Channel-per-request (per +retry attempt) + fresh `request.id` per attempt**, matching the de facto +Quartz convention used in `NostrClientPublishExt.kt` and 4 sibling files +under `quartz/.../accessories/`. + +### Why the pivot + +| Driver | Detail | +|---|---| +| **Architecture review finding** | Quartz already uses Channel-per-request as house style (5+ files). The cached-`Continuation` pattern in `RemoteSignerManager` / `IntentRequestManager` is the outlier — only 2 files. | +| **4th failure mode discovered** | `RemoteSignerManager.kt:74-101` retry loop reuses the same `request.id` across attempts. Late response from attempt 1 can resume attempt 2's continuation with **stale data** (correctness bug, not just crash). Investigation verdict: HIGH probability on flaky relays. Fresh-id-per-retry naturally fixes this. | +| **Simplicity review** | Pivoting eliminates the `@InternalCoroutinesApi` opt-in surface entirely, kills the Plan C / SingleShotContinuation alternative discussions, removes the `TestLogCapture` test-infra invention, and removes the Strategy B stress-loop test. | +| **Pattern review** | "Aligns the existing outlier with the convention" — fewer correlation styles in the codebase, not more. | + +### Scope unchanged from original plan + +- Both managers fixed in the same PR (NIP-46 + NIP-55 sibling). +- Function-level concurrency primitives — no public API change to + `launchWaitAndParse`. 17 call sites of `launchWaitAndParse` across + `NostrSignerRemote.kt`, `ForegroundRequestHandler.kt`, and the retry test + are untouched. +- `tryAndWait` (`ParallelUtils.kt:71-79`) is **retained** — still used by + `collectSuccessfulOperationsReturning` (`ParallelUtils.kt:98`). Only its + use inside the two managers is replaced. + +--- + +## Overview + +Fix two related correctness bugs in the NIP-46 bunker signer +(`RemoteSignerManager`) and its NIP-55 Android sibling +(`IntentRequestManager`): + +1. **Double-resume crash** — `Continuation.resume(...)` called twice for + the same id, throwing `IllegalStateException: Already resumed`. + Triggered by (a) multi-relay delivery, (b) bunker echo/retry, + (c) late response after `tryAndWait` timeout fires. +2. **Retry id-reuse → wrong-data bug** (NIP-46 only) — retry attempts + reuse the same `request.id`, so a late response from attempt N can + resume attempt N+1's continuation with attempt N's data. + +The fix replaces the cached `Continuation` map with a +**Channel-per-request** correlation pattern (mirroring the convention +in `quartz/.../accessories/NostrClientPublishExt.kt`), and regenerates +`request.id` per retry attempt. + +## Problem Statement + +### Bug 1 — Double-resume crash + +```kotlin +// RemoteSignerManager.kt:46-50 +suspend fun newResponse(responseEvent: NostrConnectEvent) { + val decryptedJson = signer.decrypt(responseEvent.content, remoteKey) + val bunkerResponse = OptimizedJsonMapper.fromJsonTo(decryptedJson) + awaitingRequests.get(bunkerResponse.id)?.resume(bunkerResponse) // ← unsafe +} +``` + +Non-atomic `get(id)?.resume(value)` — three races trigger double-resume: + +| Race | Sequence | Result | +|---|---|---| +| **Late response after timeout** | `tryAndWait`'s `withTimeoutOrNull` completes continuation with `null`; bunker's actual response arrives ms later; `newResponse` calls `resume` on already-completed continuation. | `IllegalStateException` at `RemoteSignerManager.kt:49` | +| **Multi-relay delivery (NIP-46 only)** | `NostrSignerRemote.kt:82` fires `scope.launch { manager.newResponse(event) }` per delivered event, no dedupe. Two relays delivering same response → two concurrent `newResponse` calls → both `get` same continuation → both `resume`. | First wins, second throws. | +| **Bunker echo / retry (NIP-46 only)** | Some bunker servers re-publish on relay reconnect. Same as multi-relay but with longer time gap. | Second resume throws. | + +Stack trace: + +``` +Exception in thread "DefaultDispatcher-worker-42" java.lang.IllegalStateException: + Already resumed, but proposed with update BunkerResponse@… + at kotlinx.coroutines.CancellableContinuationImpl.alreadyResumedError(CancellableContinuationImpl.kt:556) + at com.vitorpamplona.quartz.nip46RemoteSigner.signer.RemoteSignerManager.newResponse(RemoteSignerManager.kt:49) + at com.vitorpamplona.quartz.nip46RemoteSigner.signer.NostrSignerRemote$subscription$2$1.invokeSuspend(NostrSignerRemote.kt:83) +``` + +### Bug 2 — Retry id-reuse → wrong data (NIP-46 only) + +```kotlin +// RemoteSignerManager.kt:66-101 (paraphrased) +val request = buildRequest(...) // ← request.id assigned ONCE +val event = signer.encrypt(request, ...) // event id derived once +var attempt = 0 +while (true) { + val result = tryAndWait(timeout) { continuation -> + continuation.invokeOnCancellation { awaitingRequests.remove(request.id) } + awaitingRequests.put(request.id, continuation) // ← SAME id every attempt + client.publish(event, relayList = relayList) + } + when { + result != null -> return parser(result) + attempt >= maxRetries -> return SignerResult.RequestAddressed.TimedOut() + else -> { attempt++; delay(2_000L) } + } +} +``` + +Race sequence (default `timeout = 65_000L`): + +``` +T+0: Attempt 1: put(continuation_1, "req123") +T+65s: Attempt 1: timeout → invokeOnCancellation removes "req123" +T+67s: Attempt 2: put(continuation_2, "req123") ← same id +T+70s: Bunker's late response for ATTEMPT 1 arrives + newResponse() resumes continuation_2 with attempt_1's payload + ⚠ wrong data delivered to caller +``` + +Likelihood: HIGH when relay RTT approaches timeout. Atomic-remove + `tryResume` +**does not fix this** — continuation_2 is genuinely live; `tryResume` +succeeds with stale data. Only fresh-id-per-attempt closes this race. + +`IntentRequestManager.kt:119` already uses a fresh `RandomInstance.randomChars(32)` per +call and has no retry loop — Bug 2 does not apply there. + +## Proposed Solution + +### Mechanism: Channel-per-request + +```kotlin +// after — RemoteSignerManager (paraphrased shape) + +private val pending = ConcurrentHashMap>() + +suspend fun newResponse(responseEvent: NostrConnectEvent) { + val decryptedJson = signer.decrypt(responseEvent.content, remoteKey) + val bunkerResponse = OptimizedJsonMapper.fromJsonTo(decryptedJson) + + // Atomic remove. Multi-relay / bunker-echo duplicates: losers see null. + val channel = pending.remove(bunkerResponse.id) + if (channel == null) { + Log.d("NIP46") { "no channel for bunker response id=${bunkerResponse.id} (duplicate or unknown)" } + return + } + // capacity = 1: first delivery wins. trySend on a closed/full channel + // is a no-op — late response after timeout cannot crash. + channel.trySend(bunkerResponse) +} + +private suspend fun launchWaitAndParse( + request: BunkerRequest, + parser: (BunkerResponse) -> T, +): T { + var attempt = 0 + while (true) { + // Fresh id per attempt: each attempt is a brand-new request to the bunker. + val attemptRequest = request.copy(id = RandomInstance.randomChars(32)) + val event = signer.encrypt(attemptRequest, ...) + val channel = Channel(capacity = 1) + pending[attemptRequest.id] = channel + try { + client.publish(event, relayList = relayList) + val response = withTimeoutOrNull(timeout) { channel.receive() } + when { + response != null -> return parser(response) + attempt >= maxRetries -> return SignerResult.RequestAddressed.TimedOut() + else -> { attempt++; delay(2_000L) } + } + } finally { + pending.remove(attemptRequest.id) // cleanup on both happy + timeout paths + channel.close() + } + } +} +``` + +### Mechanism applied to `IntentRequestManager` + +Same shape, no retry loop, single attempt — `IntentResult` instead of +`BunkerResponse`, `LruCache` becomes `ConcurrentHashMap` (which is also +the structure used by the existing Channel-per-request files), and the +log tag becomes `"NIP55"`. + +### Why this fix + +| Property | Cached `Continuation` (today) | Atomic `remove` + `tryResume` (original plan) | Channel-per-request (this plan) | +|---|---|---|---| +| Double-resume crash | ❌ | ✅ | ✅ | +| Late response after timeout | ❌ | ✅ (`tryResume` returns null) | ✅ (`trySend` on closed channel is a no-op) | +| Multi-relay delivery | ❌ | ✅ (atomic remove) | ✅ (atomic remove) | +| Bunker echo / retry | ❌ | ✅ | ✅ | +| **Retry id-reuse → wrong data** | ❌ | ❌ | ✅ (fresh id per attempt) | +| `@InternalCoroutinesApi` | n/a | required | not required | +| House style match | outlier | outlier (still cached map) | ✅ matches `accessories/` convention | +| Memory leak on success path | leaks | incidentally fixed | fixed (finally block) | + +## Technical Considerations + +### Channel capacity = 1 + +Each request has at most one valid response (NIP-46 `auth_url` flow not +implemented in Amethyst — see "NIP-46 spec" below). `Channel(capacity = 1)`: + +- First `trySend` succeeds → `receive` resumes with the value. +- Concurrent / late `trySend` after the channel has been drained returns + a `ChannelResult.Closed` once the `finally` block calls `close()`. No-op, + no throw. +- `withTimeoutOrNull(timeout) { channel.receive() }` returns `null` on + timeout; the `finally` block cleans up the map entry and closes the + channel. + +### Fresh `request.id` per retry attempt + +NIP-46 has no idempotency contract — each retry is a new request from the +bunker's perspective. Generating a fresh id per attempt is consistent +with the spec and is also what `IntentRequestManager` already does for +single attempts. + +The user-visible cost: a slow bunker that finishes processing attempt 1 +mid-way through attempt 2 will not have its attempt-1 work "rescued" — +the response is discarded and attempt 2's response is the one we return. +This is the **correct** behaviour; the alternative (rescuing attempt 1 +into attempt 2's slot) is the very bug we're fixing. + +### `IntentRequestManager.LruCache` → `ConcurrentHashMap` + +`androidx.collection.LruCache` was sized at 2000 for bounded growth. +With the `finally`-block cleanup the map shrinks on every completed call, +so an unbounded `ConcurrentHashMap` matches the Quartz `accessories/` +convention without leak risk. (If we ever want a safety cap, `Caffeine` +is in the dependency graph already, but YAGNI.) + +### NIP-46 spec: `auth_url` (multi-response per id) + +Spec allows a second response per id when the bunker emits an `auth_url` +challenge first. **Amethyst has zero `auth_url` handling code today**; +every parser maps any non-null `error` to `Rejected`. Channel(capacity=1) +mirrors that current contract: first response is terminal. If `auth_url` +support is added later, the right fix is at the parser / +`launchWaitAndParse` layer (e.g., keep the channel open until the parser +returns `SignerResult.AwaitingAuth`, then `receive` again). Not in scope +here. + +### `tryAndWait` retained for `collectSuccessfulOperationsReturning` + +`tryAndWait` is still used by `ParallelUtils.kt:98` +(`collectSuccessfulOperationsReturning`). We are not deleting it — +only its uses inside the two managers go away. + +### Performance implications + +None measurable. One `Channel(1)` allocation + one entry in +`ConcurrentHashMap` per request, both freed in the `finally` block. +NIP-46 throughput is < 10 req/s in practice; this is noise. + +### Security considerations + +None. Thread-safety + correctness only; no protocol surface change, +no new trust assumptions, no new data exposed. + +## System-Wide Impact + +- **Interaction graph:** Relay → `INostrClient.subscribe` → + `NostrSignerRemote` callback → `scope.launch` → `manager.newResponse` → + channel `trySend` → `launchWaitAndParse`'s `receive` unblocks → + parser returns to Amethyst UI. The fix sits at the correlation layer; + everything downstream is unchanged. Upstream (`withTimeoutOrNull`, the + subscription callback) is unchanged. +- **`launchWaitAndParse` public signature unchanged.** All 17 call sites + (8 in `NostrSignerRemote.kt`, 9 in `ForegroundRequestHandler.kt`, + 4 in tests) are untouched. +- **State lifecycle:** `pending` is the only mutable state. Atomic + `ConcurrentHashMap.put / remove` semantics + `finally`-block cleanup + guarantee no entries leak. +- **Error propagation:** Currently the `IllegalStateException` is thrown + on a `Dispatchers.Default` worker inside `scope.launch { ... }`. After + fix, no exception path remains — duplicates trigger a debug log line. +- **Integration test scenarios** (manual / amy): + 1. **NIP-46 cold-start with multi-relay delivery:** subscribe on N=5 relays → send request → all 5 deliver same response → only one resume, no crash. + 2. **NIP-46 late-response after timeout:** request with `timeout=100ms` against a bunker that responds at 200ms → returns `TimedOut`, debug log fires. + 3. **NIP-46 retry loop with mid-flight stale response:** force `timeout < network RTT` (`timeout=100ms`, RTT=300ms) → attempt 1 times out, attempt 2 in flight, attempt 1's actual response arrives → silently discarded; attempt 2's eventual response is what we return. + 4. **NIP-55 multi-result Intent:** Android signer returns Intent with multiple `results` entries that defensively repeat the same id → no crash. + +## Acceptance Criteria + +### Functional + +- [x] `RemoteSignerManager.newResponse` never throws — late responses, duplicates from multiple relays, and bunker echoes are all silently dropped after a debug log. +- [x] `IntentRequestManager.newResponse` has the equivalent guarantee. +- [x] Each retry attempt in `RemoteSignerManager.launchWaitAndParse` uses a fresh `request.id`. A late response from attempt N cannot resume attempt N+1. +- [x] `pending` map entries are always removed in a `finally` block — no leaks on success, timeout, or thrown exception. +- [x] Caller-visible behaviour of `launchWaitAndParse` is unchanged in the happy path: same return value, same retry semantics, same `SignerResult.RequestAddressed` shapes. + +### Non-functional + +- [x] No `@OptIn(InternalCoroutinesApi::class)` introduced. +- [x] Channel capacity = 1; channel scoped to a single retry attempt (created inside loop, closed in `finally`). + +### Quality gates + +- [x] `./gradlew :quartz:compileKotlinJvm :quartz:compileKotlinAndroid` passes. +- [x] `./gradlew :quartz:jvmTest --tests "*RemoteSignerManager*"` passes, including new race-condition tests. +- [x] `./gradlew spotlessApply` clean before commit. +- [x] Existing `RemoteSignerManagerRetryTest` (5 tests) still passes. +- [x] Three new tests added covering: (a) duplicate response → no crash + one successful resume; (b) late response after timeout → no crash + caller sees `TimedOut`; (c) cross-attempt stale-response → attempt 1's late response does NOT corrupt attempt 2's result. + +## Implementation Plan + +### File-level changes + +``` +quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/ +└── RemoteSignerManager.kt # MODIFY + +quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/nip55AndroidSigner/api/foreground/ +└── IntentRequestManager.kt # MODIFY + +quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/ +└── RemoteSignerManagerRetryTest.kt # MODIFY (add three tests) +``` + +### Step 1 — `RemoteSignerManager.kt` + +Replace `awaitingRequests` cache + `tryAndWait`-based loop with +Channel-per-attempt + fresh id: + +- Cache type: `ConcurrentHashMap>` +- `newResponse`: atomic `remove` + `trySend` on `Channel(1)` (no-op on closed/full). +- `launchWaitAndParse`: inside the retry loop, copy request with fresh + `request.id` via `RandomInstance.randomChars(32)`, create + `Channel(capacity = 1)`, register under the new id, + `client.publish`, then + `withTimeoutOrNull(timeout) { channel.receive() }`. Cleanup in `finally`. + +Imports added: +- `kotlinx.coroutines.channels.Channel` +- `kotlinx.coroutines.withTimeoutOrNull` +- `com.vitorpamplona.quartz.utils.RandomInstance` +- `java.util.concurrent.ConcurrentHashMap` +- `com.vitorpamplona.quartz.utils.Log` + +Imports removed: +- `kotlin.coroutines.Continuation` +- `kotlin.coroutines.resume` +- `com.vitorpamplona.quartz.utils.cache.LargeCache` +- `com.vitorpamplona.quartz.utils.tryAndWait` (no longer used here; keep in `ParallelUtils.kt`) + +### Step 2 — `IntentRequestManager.kt` + +Same shape, no retry loop, single attempt. `LruCache` → `ConcurrentHashMap`, +`Continuation` → `Channel(capacity = 1)`, atomic `remove` + +`trySend`. Log tag `"NIP55"`. The `forEach` over multi-result Intents +now does `pending.remove(id)?.trySend(result)` per entry. + +### Step 3 — Tests (`RemoteSignerManagerRetryTest.kt`) + +Three new tests. All use the existing `runTest`-based scaffolding — +no new test infra (no `TestLogCapture`, no `Dispatchers.Default` stress +loops). Each test must be observable via the public `launchWaitAndParse` +return value, not internal log state. + +```kotlin +@Test +fun `duplicate response events do not crash and resume once`() = runTest { + val client = TestClient() + val manager = RemoteSignerManager(timeout = 5000L, client = client, ...) + val resultDeferred = async { manager.launchWaitAndParse(...) } + runCurrent() + val response = client.captureRequestEvent().toResponse() + // Three deliveries of the same response — only the first should reach the caller. + launch { manager.newResponse(response) } + launch { manager.newResponse(response) } + launch { manager.newResponse(response) } + advanceUntilIdle() + val result = resultDeferred.await() + assertIs>(result) + // Bug exists on main: second/third launch throws IllegalStateException → fails test. +} + +@Test +fun `late response after timeout is silently discarded`() = runTest { + val client = TestClient(neverResponds = true) + val manager = RemoteSignerManager(timeout = 100L, maxRetries = 0, client = client, ...) + val resultDeferred = async { manager.launchWaitAndParse(...) } + val response = client.captureRequestEvent().toResponse() + advanceTimeBy(200L) // timeout fires + val result = resultDeferred.await() + assertIs(result) + // Now the late response arrives — must not crash, must not affect caller. + manager.newResponse(response) + advanceUntilIdle() + // No assertion needed: lack of crash + caller already got TimedOut is the success criterion. +} + +@Test +fun `late response from attempt 1 does not corrupt attempt 2 result`() = runTest { + val client = TestClient() + val manager = RemoteSignerManager(timeout = 100L, maxRetries = 1, client = client, ...) + val resultDeferred = async { manager.launchWaitAndParse(buildRequest("PAYLOAD_A")) } + runCurrent() + val attempt1Event = client.captureRequestEvent() // captures id_1 + advanceTimeBy(150L) // attempt 1 times out + delay(2000) begins + advanceTimeBy(2000L) // retry kicks in + val attempt2Event = client.captureRequestEvent() // captures id_2 — must be different from id_1 + assertNotEquals(attempt1Event.requestId, attempt2Event.requestId) + // Late response for attempt 1 arrives while attempt 2 is in flight + manager.newResponse(attempt1Event.toResponse(payload = "STALE_A")) + // Real response for attempt 2 arrives + manager.newResponse(attempt2Event.toResponse(payload = "FRESH_B")) + advanceUntilIdle() + val result = resultDeferred.await() + assertIs>(result) + assertEquals("FRESH_B", (result as SignerResult.RequestAddressed.Result<*>).value) + // On main: result would be "STALE_A" (Bug 2) — test fails. Also Bug 1 would crash. +} +``` + +### Step 4 — Verify on `main` first + +Before applying Step 1, run the three new tests against `main`. Expected +failures: + +- Test 1: `IllegalStateException: Already resumed` from second/third + `launch { newResponse }`. +- Test 2: `IllegalStateException` from the late `newResponse` call. +- Test 3: either `IllegalStateException` (Bug 1) or `assertEquals` failure + with `actual = "STALE_A"` (Bug 2). Both branches prove the test exercises + the race. + +Apply Step 1, re-run, all three pass. + +### Step 5 — Format + final build + +```bash +./gradlew spotlessApply +./gradlew :quartz:build +``` + +## Success Metrics + +- Zero `IllegalStateException: Already resumed` log entries from + `RemoteSignerManager.newResponse` or `IntentRequestManager.newResponse` + after the fix lands. +- Zero stale-data correctness reports tied to retry id reuse. +- Three new tests in `RemoteSignerManagerRetryTest` pass; same tests fail + on `main`. +- No regressions in the existing 5 retry tests. + +## Dependencies & Risks + +| Item | Risk | Mitigation | +|---|---|---| +| Channel migration changes the correlation primitive | If a future feature needs multi-response per id (e.g., NIP-46 `auth_url`), the channel must be re-`receive`'d after the parser yields `AwaitingAuth` | Not blocking today (no `auth_url` code). Documented as the right architectural seam. | +| Fresh id per retry attempt changes wire behaviour | Bunkers that cache responses by id will not see the second request as a duplicate | Acceptable — NIP-46 has no idempotency contract; this matches `IntentRequestManager`'s existing behaviour. | +| `LruCache` (Android) → `ConcurrentHashMap` change | `LruCache` had a 2000-entry cap; `ConcurrentHashMap` is unbounded | `finally`-block cleanup guarantees entries shrink on every completed call. Mirrors the unbounded `ConcurrentHashMap` already used in `quartz/.../accessories/`. | +| Existing 5 retry tests rely on the old continuation-cache API | Tests may not compile against the new `pending` map | Tests should only interact through the public `launchWaitAndParse` + `newResponse` surface (verified in deepen-plan research). If any directly inspect `awaitingRequests`, update to inspect `pending` or pivot to result-based assertion. | + +## Alternative Approaches Considered + +1. **Atomic `remove` + `tryResume` on cached `CancellableContinuation`** (original plan) + Fixes Bug 1 cleanly but leaves Bug 2 (retry id-reuse) intact. Pulls in + `@InternalCoroutinesApi`. Stays with the outlier pattern. Rejected + after architecture + simplicity reviews. + +2. **try/catch `IllegalStateException` around `resume`** + Anti-pattern; doesn't fix Bug 2. Rejected. + +3. **`SingleShotContinuation` wrapper helper** + Reusable abstraction for a problem already solved by Channel. Doesn't + fix Bug 2. YAGNI; rejected. + +4. **Subscription-level dedupe in `NostrSignerRemote.kt:82`** + Bounded LRU of seen event ids. Suppresses duplicate work upstream. Not + needed because Channel `trySend` on capacity-1 is already O(1) and + guard at the receive side is atomic. Deferred unless field telemetry + shows high duplicate volume. + +5. **Fix only `RemoteSignerManager`, leave `IntentRequestManager`** + Sibling bug is identical; bundling is cheaper than a follow-up. + +6. **Keep `tryAndWait` for the managers** + Rejected because `tryAndWait`'s `CancellableContinuation`-cache + pattern is the source of both bugs. Keeping `tryAndWait` for + `collectSuccessfulOperationsReturning` is fine — different use case + (no shared id, no concurrent multi-source delivery). + +## Sources & References + +### Origin + +- **Brainstorm:** [`docs/brainstorms/2026-06-03-nip46-bunker-double-resume-brainstorm.md`](../../docs/brainstorms/2026-06-03-nip46-bunker-double-resume-brainstorm.md) +- Decisions changed during deepen-plan + review pass: + - Fix mechanism: atomic remove + `tryResume` → Channel-per-request + - Scope: added Bug 2 (retry id-reuse) as in-scope after investigation + showed HIGH probability of reaching it on flaky relays + +### Internal references + +- `quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManager.kt:44,46-50,66-101` — the two bug sites +- `quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/NostrSignerRemote.kt:69-86` — subscription handler (line 82 = source of multi-relay concurrent calls) +- `quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/ParallelUtils.kt:71-79,98` — `tryAndWait` (retained for `collectSuccessfulOperationsReturning`) +- `quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/NostrClientPublishExt.kt` — house-style reference for Channel-per-request +- `quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/nip55AndroidSigner/api/foreground/IntentRequestManager.kt:63,80-97,119,126` — sibling bug + existing fresh-id usage at line 119 +- `quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManagerRetryTest.kt` — existing test scaffolding to extend + +### External references + +- [NIP-46 spec](https://github.com/nostr-protocol/nips/blob/master/46.md) — request/response shapes + the `auth_url` challenge flow +- [kotlinx.coroutines `Channel`](https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.channels/-channel/) — capacity, `trySend`, `receive`, `close` semantics +- [`runTest` docs](https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-test/kotlinx.coroutines.test/run-test.html) — virtual time + single-threaded dispatcher + +--- + +## Unanswered Questions + +- `pending` final naming — `pending` vs `awaitingRequests` (retain old name for grep continuity)? — minor; leaning `pending` (matches `accessories/` convention) +- Whether the NIP-55 Intent multi-result branch is ever actually hit in practice — defensive fix either way; if telemetry confirms it's dead code we could collapse to single-result path in follow-up +- Whether to grep existing tests for direct `awaitingRequests` access before Step 1 — yes, do it at the start of /ce:work to flag breakage early diff --git a/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/nip55AndroidSigner/api/foreground/IntentRequestManager.kt b/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/nip55AndroidSigner/api/foreground/IntentRequestManager.kt index 8bab2bffaa..8577ba74f5 100644 --- a/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/nip55AndroidSigner/api/foreground/IntentRequestManager.kt +++ b/quartz/src/androidMain/kotlin/com/vitorpamplona/quartz/nip55AndroidSigner/api/foreground/IntentRequestManager.kt @@ -22,47 +22,37 @@ package com.vitorpamplona.quartz.nip55AndroidSigner.api.foreground import android.content.ActivityNotFoundException import android.content.Intent -import androidx.collection.LruCache import com.vitorpamplona.quartz.nip55AndroidSigner.api.IResult import com.vitorpamplona.quartz.nip55AndroidSigner.api.SignerResult import com.vitorpamplona.quartz.nip55AndroidSigner.api.foreground.intents.results.IntentResult import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.RandomInstance -import com.vitorpamplona.quartz.utils.tryAndWait -import kotlin.coroutines.Continuation -import kotlin.coroutines.resume +import com.vitorpamplona.quartz.utils.cache.LargeCache +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.withTimeoutOrNull /** - * This class manages the lifecycle of foreground signing requests in a NIP-55 compliant Android signer flow. + * Manages the lifecycle of foreground signing requests in a NIP-55 compliant Android signer flow. * - * - It tracks pending signing requests using a unique ID and allows for awaiting their results via coroutines. + * - Tracks pending signing requests by a unique call id via a per-request [Channel]. * - Provides a way to launch foreground Intents (to request user approval) and wait for the response. - * - Handles timeouts on user approval using [tryAndWait]. + * - Handles approval timeouts via [withTimeoutOrNull]. * - Collects results via [newResponse] when the user responds to the foreground request. * - * Main components: + * Key usage flow: request initiated -> store channel by id -> launch intent -> withTimeoutOrNull + * receive -> finally cleanup. User responds -> atomic remove + trySend on the channel. * - * - `awaitingRequests`: LRU cache mapping request IDs to continuations for async response handling. - * - `appLauncher`: Function reference to launch foreground Intents (typically provided by an Activity). - * - `launchAndWait`: Suspend function to send an Intent, wait for an answer, and parse the result. - * - `newResponse`: Handles incoming results from the foreground activity using a unique ID. - * - * Key usage flows: - * - * - Request initiated -> store continuation by ID -> launch intent -> wait - * - User responds -> resume continuation -> remove ID -> return parsed result - * - * The class also cleans up pending requests on timeout or cancellation. + * Duplicate, unknown, or late deliveries (e.g. if the activity surfaces multiple results for the + * same id) atomically read out as null and are dropped after a debug log line — no continuation + * is ever resumed twice. */ class IntentRequestManager( val foregroundApprovalTimeout: Long = 30000, ) { val activityNotFoundIntent = Intent() - // LRU cache to store pending requests and their continuations. - private val awaitingRequests = LruCache>(2000) + private val pending = LargeCache>() - // Function to launch an Intent in the foreground. private var appLauncher: ((Intent) -> Unit)? = null /** Call this function when the launcher becomes available on activity, fragment or compose */ @@ -82,70 +72,66 @@ class IntentRequestManager( if (results != null) { // This happens when the intent responds to many requests at the same time. IntentResult.fromJsonArray(results).forEach { result -> - if (result.id != null) { - awaitingRequests[result.id]?.resume(result) - awaitingRequests.remove(result.id) - } + if (result.id != null) dispatch(result.id, result) } } else { val result = IntentResult.fromIntent(data) - if (result.id != null) { - awaitingRequests[result.id]?.resume(result) - awaitingRequests.remove(result.id) - } + if (result.id != null) dispatch(result.id, result) } } + private fun dispatch( + id: String, + result: IntentResult, + ) { + val channel = pending.remove(id) + if (channel == null) { + Log.d("NIP55") { "no channel for intent result id=$id (duplicate, unknown, or late)" } + return + } + channel.trySend(result) + } + fun hasForegroundActivity() = appLauncher != null /** - * Launches the signer, waits and parses the result + * Launches the signer, waits and parses the result. * - * @param requestIntent The Intent to be launched. + * @param requestIntentBuilder Builder for the Intent to be launched. * @param parser A function that parses the response Intent into a [SignerResult.RequestAddressed]. * @return The result after parsing the Intent using the provided parser. - * - * This function uses the [tryAndWait] utility to implement a timeout on the foreground approval. - * It assigns a unique ID to the request and keeps a continuation to resume once the result is received. - * If the timeout occurs or the continuation is cancelled, the request ID is cleaned up from [awaitingRequests]. - * Flags are added to the Intent to ensure it is brought to the front if already running. */ suspend fun launchWaitAndParse( requestIntentBuilder: () -> Intent, parser: (intent: IntentResult) -> SignerResult.RequestAddressed, - ): SignerResult.RequestAddressed = - appLauncher?.let { launcher -> - val requestIntent = requestIntentBuilder() - val callId = RandomInstance.randomChars(32) + ): SignerResult.RequestAddressed { + val launcher = appLauncher ?: return SignerResult.RequestAddressed.NoActivityToLaunchFrom() - requestIntent.putExtra("id", callId) - requestIntent.addFlags(Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP) + val requestIntent = requestIntentBuilder() + val callId = RandomInstance.randomChars(32) + requestIntent.putExtra("id", callId) + requestIntent.addFlags(Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP) + + val channel = Channel(capacity = 1) + pending.put(callId, channel) + + return try { try { - val resultIntent = - tryAndWait(foregroundApprovalTimeout) { continuation -> - continuation.invokeOnCancellation { - awaitingRequests.remove(callId) - } - - awaitingRequests.put(callId, continuation) - - try { - launcher.invoke(requestIntent) - } catch (e: Exception) { - Log.e("ExternalSigner", "Error launching intent", e) - awaitingRequests.remove(callId) - throw e - } - } - - when (resultIntent) { - null -> SignerResult.RequestAddressed.TimedOut() - else -> parser(resultIntent) - } + launcher.invoke(requestIntent) } catch (e: ActivityNotFoundException) { Log.e("ExternalSigner", "Error launching intent: Signer not found", e) - SignerResult.RequestAddressed.SignerNotFound() + return SignerResult.RequestAddressed.SignerNotFound() } - } ?: SignerResult.RequestAddressed.NoActivityToLaunchFrom() + + val resultIntent = withTimeoutOrNull(foregroundApprovalTimeout) { channel.receive() } + when (resultIntent) { + null -> SignerResult.RequestAddressed.TimedOut() + else -> parser(resultIntent) + } + } finally { + pending.remove(callId) + channel.close() + } + } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManager.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManager.kt index 43c8c0bd5b..432c715e41 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManager.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManager.kt @@ -27,11 +27,12 @@ import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponse import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent +import com.vitorpamplona.quartz.utils.Log +import com.vitorpamplona.quartz.utils.RandomInstance import com.vitorpamplona.quartz.utils.cache.LargeCache -import com.vitorpamplona.quartz.utils.tryAndWait +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay -import kotlin.coroutines.Continuation -import kotlin.coroutines.resume +import kotlinx.coroutines.withTimeoutOrNull class RemoteSignerManager( val timeout: Long = 65_000, @@ -41,19 +42,27 @@ class RemoteSignerManager( val relayList: Set, val maxRetries: Int = 1, ) { - private val awaitingRequests = LargeCache>() + private val pending = LargeCache>() suspend fun newResponse(responseEvent: NostrConnectEvent) { val decryptedJson = signer.decrypt(responseEvent.content, remoteKey) val bunkerResponse = OptimizedJsonMapper.fromJsonTo(decryptedJson) - awaitingRequests.get(bunkerResponse.id)?.resume(bunkerResponse) + + val channel = pending.remove(bunkerResponse.id) + if (channel == null) { + Log.d("NIP46") { "no channel for bunker response id=${bunkerResponse.id} (duplicate, unknown, or late)" } + return + } + channel.trySend(bunkerResponse) } /** * Launches the signer, waits and parses the result. * - * Builds the request once and republishes the same event on retry to ensure - * the bunker's response (keyed by request ID) can always be matched. + * Each retry attempt uses a fresh request id so a late response from a previous + * attempt cannot resume the current attempt with stale data. The bunker request + * builder is still called only once per call; the manager rewrites the id per + * attempt internally. * * @param bunkerRequestBuilder The BunkerRequest to be sent. * @param parser A function that parses the BunkerResponse into a [SignerResult.RequestAddressed]. @@ -63,36 +72,38 @@ class RemoteSignerManager( bunkerRequestBuilder: () -> BunkerRequest, parser: (response: BunkerResponse) -> SignerResult.RequestAddressed, ): SignerResult.RequestAddressed { - val request = bunkerRequestBuilder() - val event = - NostrConnectEvent.create( - message = request, - remoteKey = remoteKey, - signer = signer, - ) + val template = bunkerRequestBuilder() var attempt = 0 while (true) { - val result = - tryAndWait(timeout) { continuation -> - continuation.invokeOnCancellation { - awaitingRequests.remove(request.id) - } + val attemptRequest = + BunkerRequest( + id = RandomInstance.randomChars(32), + method = template.method, + params = template.params, + ) + val event = + NostrConnectEvent.create( + message = attemptRequest, + remoteKey = remoteKey, + signer = signer, + ) - awaitingRequests.put(request.id, continuation) + val channel = Channel(capacity = 1) + pending.put(attemptRequest.id, channel) + val response = + try { client.publish(event, relayList = relayList) + withTimeoutOrNull(timeout) { channel.receive() } + } finally { + pending.remove(attemptRequest.id) + channel.close() } when { - result != null -> { - return parser(result) - } - - attempt >= maxRetries -> { - return SignerResult.RequestAddressed.TimedOut() - } - + response != null -> return parser(response) + attempt >= maxRetries -> return SignerResult.RequestAddressed.TimedOut() else -> { attempt++ delay(2_000L) diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManagerRetryTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManagerRetryTest.kt index 85a652b874..99d18cb353 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManagerRetryTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/signer/RemoteSignerManagerRetryTest.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.quartz.nip46RemoteSigner.signer import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.core.OptimizedJsonMapper import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.relay.client.EmptyNostrClient import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient @@ -31,21 +32,46 @@ import com.vitorpamplona.quartz.nip01Core.relay.client.single.IRelayClient import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequest import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerRequestPing +import com.vitorpamplona.quartz.nip46RemoteSigner.BunkerResponsePong +import com.vitorpamplona.quartz.nip46RemoteSigner.NostrConnectEvent import com.vitorpamplona.quartz.utils.Hex +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertIs +import kotlin.test.assertNotEquals +@OptIn(ExperimentalCoroutinesApi::class) class RemoteSignerManagerRetryTest { private val signer = NostrSignerInternal(KeyPair()) private val remoteKeyPair = KeyPair() private val remoteKey = Hex.encode(remoteKeyPair.pubKey) + private val bunkerSigner = NostrSignerInternal(remoteKeyPair) private val relay = NormalizedRelayUrl("wss://relay.test") + private suspend fun decodeRequestId(event: Event): String { + val plaintext = bunkerSigner.decrypt(event.content, event.pubKey) + val request = OptimizedJsonMapper.fromJsonTo(plaintext) + return request.id + } + + private suspend fun bunkerPongFor(requestId: String): NostrConnectEvent = + NostrConnectEvent.create( + message = BunkerResponsePong(requestId), + remoteKey = signer.pubKey, + signer = bunkerSigner, + ) + @Test fun timeoutReturnsTimedOutAfterMaxRetries() = runTest { @@ -165,6 +191,190 @@ class RemoteSignerManagerRetryTest { assertEquals(1, manager.maxRetries) } + + @Test + fun duplicateResponsesAreSafeAndResumeOnce() = + runTest { + val capturing = CapturingNostrClient() + val manager = + RemoteSignerManager( + timeout = 5_000, + client = capturing, + signer = signer, + remoteKey = remoteKey, + relayList = setOf(relay), + maxRetries = 0, + ) + + val deferred = + async { + manager.launchWaitAndParse( + bunkerRequestBuilder = { BunkerRequestPing() }, + parser = PingResponse::parse, + ) + } + runCurrent() + + val publishedRequestId = decodeRequestId(capturing.publishedEvents.single()) + val response = bunkerPongFor(publishedRequestId) + + // Three deliveries of the same response — only one continuation exists, + // so the second and third would have crashed on the old `get(id)?.resume(...)` + // path. With atomic remove + Channel(1) trySend they are safe no-ops. + launch { manager.newResponse(response) } + launch { manager.newResponse(response) } + launch { manager.newResponse(response) } + advanceUntilIdle() + + val result = deferred.await() + assertIs>(result) + } + + @Test + fun lateResponseAfterTimeoutIsSilentlyDiscarded() = + runTest { + val capturing = CapturingNostrClient() + val manager = + RemoteSignerManager( + timeout = 100, + client = capturing, + signer = signer, + remoteKey = remoteKey, + relayList = setOf(relay), + maxRetries = 0, + ) + + val deferred = + async { + manager.launchWaitAndParse( + bunkerRequestBuilder = { BunkerRequestPing() }, + parser = PingResponse::parse, + ) + } + runCurrent() + + val publishedRequestId = decodeRequestId(capturing.publishedEvents.single()) + + // Let the timeout fire. Caller resolves to TimedOut and the channel is closed. + val result = deferred.await() + assertIs>(result) + + // Now the late response arrives. On the old `get(id)?.resume(...)` path the + // continuation was already completed by the timeout, so this would throw + // IllegalStateException("Already resumed"). With the fix the entry is gone + // from `pending` and trySend on the closed channel is a no-op. + val response = bunkerPongFor(publishedRequestId) + manager.newResponse(response) + advanceUntilIdle() + } + + @Test + fun lateResponseFromAttempt1DoesNotCorruptAttempt2() = + runTest { + val capturing = CapturingNostrClient() + val manager = + RemoteSignerManager( + timeout = 100, + client = capturing, + signer = signer, + remoteKey = remoteKey, + relayList = setOf(relay), + maxRetries = 1, + ) + + val deferred = + async { + manager.launchWaitAndParse( + bunkerRequestBuilder = { BunkerRequestPing() }, + parser = PingResponse::parse, + ) + } + runCurrent() + + // Attempt 1 publishes, then times out at T=100. + val attempt1Id = decodeRequestId(capturing.publishedEvents[0]) + advanceTimeBy(150) + // delay(2_000) between attempts: attempt 2 starts at T=2_100. + // Land mid-window so attempt 2's own 100 ms timeout (T=2_200) hasn't fired yet. + advanceTimeBy(2_000) + runCurrent() + + // Attempt 2 must use a different id — otherwise a late attempt-1 response + // could resume the attempt-2 channel with stale data. + assertEquals(2, capturing.publishedEvents.size) + val attempt2Id = decodeRequestId(capturing.publishedEvents[1]) + assertNotEquals(attempt1Id, attempt2Id) + + // Late delivery of attempt 1's response. With the fix it has no entry in + // `pending` and is silently discarded. + manager.newResponse(bunkerPongFor(attempt1Id)) + // Attempt 2's real response. + manager.newResponse(bunkerPongFor(attempt2Id)) + advanceUntilIdle() + + val result = deferred.await() + val success = assertIs>(result) + assertEquals(attempt2Id, success.result.pong) + } +} + +private class CapturingNostrClient : INostrClient { + val publishedEvents = mutableListOf() + + override fun connectedRelaysFlow(): StateFlow> = MutableStateFlow(emptySet()) + + override fun availableRelaysFlow(): StateFlow> = MutableStateFlow(emptySet()) + + override fun connect() {} + + override fun disconnect() {} + + override fun reconnect( + onlyIfChanged: Boolean, + ignoreRetryDelays: Boolean, + ) {} + + override fun isActive(): Boolean = false + + override fun syncFilters(relay: IRelayClient) {} + + override fun subscribe( + subId: String, + filters: Map>, + listener: SubscriptionListener?, + ) {} + + override fun count( + subId: String, + filters: Map>, + ) {} + + override fun unsubscribe(subId: String) {} + + override fun publish( + event: Event, + relayList: Set, + ) { + publishedEvents.add(event) + } + + override fun pendingPublishRelaysFor(eventId: String): Set? = null + + override fun addConnectionListener(listener: RelayConnectionListener) {} + + override fun removeConnectionListener(listener: RelayConnectionListener) {} + + override fun getReqFiltersOrNull(subId: String): Map>? = null + + override fun getCountFiltersOrNull(subId: String): Map>? = null + + override fun activeRequests(url: NormalizedRelayUrl): Map> = emptyMap() + + override fun activeCounts(url: NormalizedRelayUrl): Map> = emptyMap() + + override fun activeOutboxCache(url: NormalizedRelayUrl): Set = emptySet() + + override fun close() {} } private class CountingNostrClient( From cdb76e448c887d3ab7d529899737aaa33eb306d1 Mon Sep 17 00:00:00 2001 From: Crowdin Bot Date: Sun, 7 Jun 2026 12:21:45 +0000 Subject: [PATCH 2/6] New Crowdin translations by GitHub Action --- amethyst/src/main/res/values-hi-rIN/strings.xml | 2 ++ amethyst/src/main/res/values-pl-rPL/strings.xml | 2 ++ amethyst/src/main/res/values-zh-rCN/strings.xml | 2 ++ 3 files changed, 6 insertions(+) diff --git a/amethyst/src/main/res/values-hi-rIN/strings.xml b/amethyst/src/main/res/values-hi-rIN/strings.xml index 32378e17ca..97e561d54b 100644 --- a/amethyst/src/main/res/values-hi-rIN/strings.xml +++ b/amethyst/src/main/res/values-hi-rIN/strings.xml @@ -2815,6 +2815,8 @@ यह उद्देश्य समाप्त हो चुका है %1$s वित्तपोषित %2$s साट्स उद्देश्य में से + समाप्ति %1$s + खण्डश्रृंखलाबद्ध दान उद्देश्य संख्या (साट्स) १००००० आपके उद्देश्य का विवरण करें diff --git a/amethyst/src/main/res/values-pl-rPL/strings.xml b/amethyst/src/main/res/values-pl-rPL/strings.xml index 0a599454e2..c7b100a9f1 100644 --- a/amethyst/src/main/res/values-pl-rPL/strings.xml +++ b/amethyst/src/main/res/values-pl-rPL/strings.xml @@ -2853,6 +2853,8 @@ Zaplanowane posty z innych kont nie zostaną opublikowane, dopóki to konto jest Ta zbiórka została zamknięta Sfinansowano %1$s z %2$s satoszów + Kończy się %1$s + Darowizna on-chain Kwota zbiorki (w satoszach) 100000 Opisz cel zbiórki diff --git a/amethyst/src/main/res/values-zh-rCN/strings.xml b/amethyst/src/main/res/values-zh-rCN/strings.xml index 7416df43c6..8dca9af449 100644 --- a/amethyst/src/main/res/values-zh-rCN/strings.xml +++ b/amethyst/src/main/res/values-zh-rCN/strings.xml @@ -2789,6 +2789,8 @@ 此目标已关闭 设定目标为 %2$s sats，筹集到 %1$s + 结束 %1$s + 链上捐助 目标金额 (sats) 100000 描述您的目标 From 98ff13b83ff4963c3f1dcc318d1d24e08d24eea8 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 7 Jun 2026 21:27:08 +0200 Subject: [PATCH 3/6] feat(birdstar): render Birdex species collections (kind 12473) --- .../amethyst/model/LocalCache.kt | 5 ++ .../amethyst/ui/note/NoteCompose.kt | 6 ++ .../amethyst/ui/note/types/Birdex.kt | 89 +++++++++++++++++++ .../dal/FollowPackFeedNewThreadFeedFilter.kt | 3 + .../home/dal/HomeNewThreadFeedFilter.kt | 3 + .../mutual/dal/UserProfileMutualFeedFilter.kt | 2 + .../dal/UserProfileNewThreadFeedFilter.kt | 2 + .../loggedIn/threadview/ThreadFeedView.kt | 4 + amethyst/src/main/res/values/strings.xml | 5 ++ .../experimental/birdstar/BirdexEvent.kt | 71 +++++++++++++++ .../quartz/utils/EventFactory.kt | 2 + .../experimental/birdstar/BirdexEventTest.kt | 78 ++++++++++++++++ 12 files changed, 270 insertions(+) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt create mode 100644 quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEventTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index 6a0a31f9e6..b316174452 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -54,6 +54,7 @@ import com.vitorpamplona.quartz.experimental.attestations.recommendation.Attesto import com.vitorpamplona.quartz.experimental.attestations.request.AttestationRequestEvent import com.vitorpamplona.quartz.experimental.audio.header.AudioHeaderEvent import com.vitorpamplona.quartz.experimental.audio.track.AudioTrackEvent +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent import com.vitorpamplona.quartz.experimental.edits.TextNoteModificationEvent import com.vitorpamplona.quartz.experimental.ephemChat.chat.EphemeralChatEvent import com.vitorpamplona.quartz.experimental.ephemChat.chat.RoomId @@ -3371,6 +3372,10 @@ object LocalCache : ILocalCache, ICacheProvider { consumeBaseReplaceable(event, relay, wasVerified) } + is BirdexEvent -> { + consumeBaseReplaceable(event, relay, wasVerified) + } + is CommentEvent -> { consumeRegularEvent(event, relay, wasVerified) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteCompose.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteCompose.kt index ffbc121bcd..dbb4a20e1d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteCompose.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteCompose.kt @@ -117,6 +117,7 @@ import com.vitorpamplona.amethyst.ui.note.types.RenderAttestorRecommendation import com.vitorpamplona.amethyst.ui.note.types.RenderAudioHeader import com.vitorpamplona.amethyst.ui.note.types.RenderAudioTrack import com.vitorpamplona.amethyst.ui.note.types.RenderBadgeAward +import com.vitorpamplona.amethyst.ui.note.types.RenderBirdex import com.vitorpamplona.amethyst.ui.note.types.RenderCalendarCollectionEvent import com.vitorpamplona.amethyst.ui.note.types.RenderCalendarDateSlotEvent import com.vitorpamplona.amethyst.ui.note.types.RenderCalendarRSVPEvent @@ -216,6 +217,7 @@ import com.vitorpamplona.quartz.experimental.attestations.recommendation.Attesto import com.vitorpamplona.quartz.experimental.attestations.request.AttestationRequestEvent import com.vitorpamplona.quartz.experimental.audio.header.AudioHeaderEvent import com.vitorpamplona.quartz.experimental.audio.track.AudioTrackEvent +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent import com.vitorpamplona.quartz.experimental.bounties.bountyBaseReward import com.vitorpamplona.quartz.experimental.edits.TextNoteModificationEvent import com.vitorpamplona.quartz.experimental.forks.IForkableEvent @@ -1264,6 +1266,10 @@ private fun RenderNoteRow( RenderFundraiser(baseNote, makeItShort, accountViewModel, nav) } + is BirdexEvent -> { + RenderBirdex(baseNote, makeItShort, accountViewModel) + } + is HighlightEvent -> { RenderHighlight( baseNote, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt new file mode 100644 index 0000000000..8346a62aed --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.note.types + +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.pluralStringResource +import androidx.compose.ui.text.style.TextOverflow +import androidx.compose.ui.unit.dp +import com.vitorpamplona.amethyst.R +import com.vitorpamplona.amethyst.model.Note +import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel +import com.vitorpamplona.amethyst.ui.stringRes +import com.vitorpamplona.amethyst.ui.theme.placeholderText +import com.vitorpamplona.amethyst.ui.theme.replyModifier +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent + +/** How many species names to list before collapsing into a "+N more" suffix. */ +private const val SPECIES_PREVIEW_LIMIT = 6 + +/** + * Minimal, fixed-size summary card for a Birdstar "Birdex" (kind 12473). + * + * The event has no body and no images, only a species list. To keep the card + * bounded regardless of how many species a Birdex holds, we show the count and a + * short preview of scientific names with a "+N more" suffix — no images, no + * expansion, no network calls. The card is identical in the feed and the opened + * view (it ignores [makeItShort]). + */ +@Composable +fun RenderBirdex( + baseNote: Note, + makeItShort: Boolean, + accountViewModel: AccountViewModel, +) { + val noteEvent = baseNote.event as? BirdexEvent ?: return + + val names = remember(noteEvent) { noteEvent.speciesNames() } + val preview = remember(names) { names.take(SPECIES_PREVIEW_LIMIT) } + val remaining = names.size - preview.size + + Column(MaterialTheme.colorScheme.replyModifier.padding(10.dp)) { + Text( + text = pluralStringResource(R.plurals.birdex_species_count, names.size, names.size), + style = MaterialTheme.typography.titleMedium, + ) + + if (preview.isNotEmpty()) { + Spacer(Modifier.height(6.dp)) + val joined = preview.joinToString(", ") + Text( + text = + if (remaining > 0) { + stringRes(R.string.birdex_species_preview_more, joined, remaining.toString()) + } else { + joined + }, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.placeholderText, + overflow = TextOverflow.Ellipsis, + ) + } + } +} diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/followPacks/feed/dal/FollowPackFeedNewThreadFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/followPacks/feed/dal/FollowPackFeedNewThreadFeedFilter.kt index e3b823bf55..02146a7145 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/followPacks/feed/dal/FollowPackFeedNewThreadFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/followPacks/feed/dal/FollowPackFeedNewThreadFeedFilter.kt @@ -34,6 +34,7 @@ import com.vitorpamplona.amethyst.ui.dal.FilterByListParams import com.vitorpamplona.quartz.experimental.agora.FundraiserEvent import com.vitorpamplona.quartz.experimental.audio.header.AudioHeaderEvent import com.vitorpamplona.quartz.experimental.audio.track.AudioTrackEvent +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent import com.vitorpamplona.quartz.experimental.interactiveStories.InteractiveStoryPrologueEvent import com.vitorpamplona.quartz.experimental.music.playlist.MusicPlaylistEvent import com.vitorpamplona.quartz.experimental.music.track.MusicTrackEvent @@ -68,6 +69,7 @@ class FollowPackFeedNewThreadFeedFilter( NipTextEvent.KIND, ClassifiedsEvent.KIND, FundraiserEvent.KIND, + BirdexEvent.KIND, LongTextNoteEvent.KIND, ) } @@ -137,6 +139,7 @@ class FollowPackFeedNewThreadFeedFilter( noteEvent is TextNoteEvent || noteEvent is ClassifiedsEvent || noteEvent is FundraiserEvent || + noteEvent is BirdexEvent || noteEvent.isRenderableRepost() || (noteEvent is LongTextNoteEvent && noteEvent.content.isNotEmpty()) || (noteEvent is WikiNoteEvent && noteEvent.content.isNotEmpty()) || diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/dal/HomeNewThreadFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/dal/HomeNewThreadFeedFilter.kt index e1d086328b..b63c4117da 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/dal/HomeNewThreadFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/dal/HomeNewThreadFeedFilter.kt @@ -37,6 +37,7 @@ import com.vitorpamplona.quartz.experimental.attestations.recommendation.Attesto import com.vitorpamplona.quartz.experimental.attestations.request.AttestationRequestEvent import com.vitorpamplona.quartz.experimental.audio.header.AudioHeaderEvent import com.vitorpamplona.quartz.experimental.audio.track.AudioTrackEvent +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent import com.vitorpamplona.quartz.experimental.interactiveStories.InteractiveStoryPrologueEvent import com.vitorpamplona.quartz.experimental.music.playlist.MusicPlaylistEvent import com.vitorpamplona.quartz.experimental.music.track.MusicTrackEvent @@ -70,6 +71,7 @@ class HomeNewThreadFeedFilter( WikiNoteEvent.KIND, ClassifiedsEvent.KIND, FundraiserEvent.KIND, + BirdexEvent.KIND, LongTextNoteEvent.KIND, LiveChessGameEndEvent.KIND, AttestationEvent.KIND, @@ -126,6 +128,7 @@ class HomeNewThreadFeedFilter( noteEvent is TextNoteEvent || noteEvent is ClassifiedsEvent || noteEvent is FundraiserEvent || + noteEvent is BirdexEvent || noteEvent.isRenderableRepost() || (noteEvent is LongTextNoteEvent && noteEvent.content.isNotEmpty()) || (noteEvent is WikiNoteEvent && noteEvent.content.isNotEmpty()) || diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/mutual/dal/UserProfileMutualFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/mutual/dal/UserProfileMutualFeedFilter.kt index d451ca0325..e3fa8265e0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/mutual/dal/UserProfileMutualFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/mutual/dal/UserProfileMutualFeedFilter.kt @@ -31,6 +31,7 @@ import com.vitorpamplona.amethyst.ui.dal.DefaultFeedOrder import com.vitorpamplona.quartz.experimental.agora.FundraiserEvent import com.vitorpamplona.quartz.experimental.audio.header.AudioHeaderEvent import com.vitorpamplona.quartz.experimental.audio.track.AudioTrackEvent +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent import com.vitorpamplona.quartz.experimental.interactiveStories.InteractiveStoryPrologueEvent import com.vitorpamplona.quartz.experimental.music.playlist.MusicPlaylistEvent import com.vitorpamplona.quartz.experimental.music.track.MusicTrackEvent @@ -78,6 +79,7 @@ class UserProfileMutualFeedFilter( it.event is TextNoteEvent || it.event is ClassifiedsEvent || it.event is FundraiserEvent || + it.event is BirdexEvent || it.event.isRenderableRepost() || it.event is LongTextNoteEvent || it.event is WikiNoteEvent || diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/newthreads/dal/UserProfileNewThreadFeedFilter.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/newthreads/dal/UserProfileNewThreadFeedFilter.kt index cfcd046558..fff7c182d2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/newthreads/dal/UserProfileNewThreadFeedFilter.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/newthreads/dal/UserProfileNewThreadFeedFilter.kt @@ -35,6 +35,7 @@ import com.vitorpamplona.quartz.experimental.attestations.recommendation.Attesto import com.vitorpamplona.quartz.experimental.attestations.request.AttestationRequestEvent import com.vitorpamplona.quartz.experimental.audio.header.AudioHeaderEvent import com.vitorpamplona.quartz.experimental.audio.track.AudioTrackEvent +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent import com.vitorpamplona.quartz.experimental.interactiveStories.InteractiveStoryPrologueEvent import com.vitorpamplona.quartz.experimental.music.playlist.MusicPlaylistEvent import com.vitorpamplona.quartz.experimental.music.track.MusicTrackEvent @@ -84,6 +85,7 @@ class UserProfileNewThreadFeedFilter( it.event is CommentEvent || it.event is ClassifiedsEvent || it.event is FundraiserEvent || + it.event is BirdexEvent || it.event.isRenderableRepost() || it.event is LongTextNoteEvent || it.event is WikiNoteEvent || diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/threadview/ThreadFeedView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/threadview/ThreadFeedView.kt index 9bde3c4686..689c8391a9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/threadview/ThreadFeedView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/threadview/ThreadFeedView.kt @@ -147,6 +147,7 @@ import com.vitorpamplona.amethyst.ui.note.types.RenderAttestation import com.vitorpamplona.amethyst.ui.note.types.RenderAttestationRequest import com.vitorpamplona.amethyst.ui.note.types.RenderAttestorProficiency import com.vitorpamplona.amethyst.ui.note.types.RenderAttestorRecommendation +import com.vitorpamplona.amethyst.ui.note.types.RenderBirdex import com.vitorpamplona.amethyst.ui.note.types.RenderCalendarDateSlotEvent import com.vitorpamplona.amethyst.ui.note.types.RenderCalendarTimeSlotEvent import com.vitorpamplona.amethyst.ui.note.types.RenderCashuMint @@ -229,6 +230,7 @@ import com.vitorpamplona.quartz.experimental.attestations.recommendation.Attesto import com.vitorpamplona.quartz.experimental.attestations.request.AttestationRequestEvent import com.vitorpamplona.quartz.experimental.audio.header.AudioHeaderEvent import com.vitorpamplona.quartz.experimental.audio.track.AudioTrackEvent +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent import com.vitorpamplona.quartz.experimental.bounties.bountyBaseReward import com.vitorpamplona.quartz.experimental.edits.TextNoteModificationEvent import com.vitorpamplona.quartz.experimental.forks.IForkableEvent @@ -769,6 +771,8 @@ private fun FullBleedNoteCompose( RenderGoal(baseNote, accountViewModel, nav) } else if (noteEvent is FundraiserEvent) { RenderFundraiser(baseNote, makeItShort = false, accountViewModel, nav) + } else if (noteEvent is BirdexEvent) { + RenderBirdex(baseNote, makeItShort = false, accountViewModel) } else if (noteEvent is RepostEvent || noteEvent is GenericRepostEvent) { RenderRepost(baseNote, quotesLeft = 3, backgroundColor, accountViewModel, nav) } else if (noteEvent is RelayDiscoveryEvent) { diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 40bed9f73f..400852fb5f 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -3138,6 +3138,11 @@ %1$s funded of %2$s sats goal Ends %1$s On-chain donation + + Birdex · %1$d species + Birdex · %1$d species + + %1$s +%2$s more Goal amount (sats) 100000 Describe your goal diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt new file mode 100644 index 0000000000..4b170db4b8 --- /dev/null +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt @@ -0,0 +1,71 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.experimental.birdstar + +import androidx.compose.runtime.Immutable +import com.vitorpamplona.quartz.nip01Core.core.BaseReplaceableEvent +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.core.firstTagValue +import com.vitorpamplona.quartz.nip01Core.core.mapValueTagged + +/** + * Birdstar "Birdex" species collection (kind 12473). + * + * An app-specific **replaceable** kind published by the Birdstar app + * (`birdstar.app`) — a birdwatching life-list. It is **not** defined by any NIP; + * the schema below is derived from events seen in the wild. Being replaceable, + * each author keeps a single, latest Birdex. + * + * The event carries no body and no images — its payload is the species list, + * one entry per observed species, as alternating tags: + * + * - `n` — the species' scientific name (e.g. `Icterus galbula`). + * - `i` — an external identity reference for the species, a Wikidata entity URL + * (NIP-73 style, e.g. `https://www.wikidata.org/entity/Q805774`). + * - `alt` — a human-readable summary written by the publisher + * (e.g. `Birdex: 24 species`). + * + * Amethyst renders a minimal, fixed-size summary card from [speciesNames] and + * [speciesCount]; it does not resolve the Wikidata references to images. + */ +@Immutable +class BirdexEvent( + id: HexKey, + pubKey: HexKey, + createdAt: Long, + tags: Array>, + content: String, + sig: HexKey, +) : BaseReplaceableEvent(id, pubKey, createdAt, KIND, tags, content, sig) { + /** Scientific names of the collected species, in event order, from the `n` tags. */ + fun speciesNames() = tags.mapValueTagged("n") { it } + + /** Number of collected species (one `n` tag per species). */ + fun speciesCount() = tags.count { it.size > 1 && it[0] == "n" } + + /** Publisher-provided human-readable summary, from the `alt` tag (may be null). */ + fun summary() = tags.firstTagValue("alt") + + companion object { + const val KIND = 12473 + const val ALT_DESCRIPTION = "A Birdex species collection" + } +} diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/EventFactory.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/EventFactory.kt index f0b180b0c3..6356a1667a 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/EventFactory.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/EventFactory.kt @@ -29,6 +29,7 @@ import com.vitorpamplona.quartz.experimental.attestations.recommendation.Attesto import com.vitorpamplona.quartz.experimental.attestations.request.AttestationRequestEvent import com.vitorpamplona.quartz.experimental.audio.header.AudioHeaderEvent import com.vitorpamplona.quartz.experimental.audio.track.AudioTrackEvent +import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent import com.vitorpamplona.quartz.experimental.edits.TextNoteModificationEvent import com.vitorpamplona.quartz.experimental.ephemChat.chat.EphemeralChatEvent import com.vitorpamplona.quartz.experimental.ephemChat.list.EphemeralChatListEvent @@ -341,6 +342,7 @@ class EventFactory { AudioTrackEvent.KIND -> AudioTrackEvent(id, pubKey, createdAt, tags, content, sig) BadgeAwardEvent.KIND -> BadgeAwardEvent(id, pubKey, createdAt, tags, content, sig) BadgeDefinitionEvent.KIND -> BadgeDefinitionEvent(id, pubKey, createdAt, tags, content, sig) + BirdexEvent.KIND -> BirdexEvent(id, pubKey, createdAt, tags, content, sig) BidEvent.KIND -> BidEvent(id, pubKey, createdAt, tags, content, sig) BidConfirmationEvent.KIND -> BidConfirmationEvent(id, pubKey, createdAt, tags, content, sig) BlockedRelayListEvent.KIND -> BlockedRelayListEvent(id, pubKey, createdAt, tags, content, sig) diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEventTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEventTest.kt new file mode 100644 index 0000000000..c0ab794c9f --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEventTest.kt @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.experimental.birdstar + +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.utils.EventFactory +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.test.assertTrue + +class BirdexEventTest { + private fun sampleEvent(): Event = + EventFactory.create( + id = "a099d4db563041bb289d3704f983fc148fc805860303a4f479a8264dc6a2d7cc", + pubKey = "932614571afcbad4d17a191ee281e39eebbb41b93fac8fd87829622aeb112f4d", + createdAt = 1_780_836_939L, + kind = BirdexEvent.KIND, + tags = + arrayOf( + arrayOf("alt", "Birdex: 3 species"), + arrayOf("i", "https://www.wikidata.org/entity/Q805774"), + arrayOf("n", "Icterus galbula"), + arrayOf("i", "https://www.wikidata.org/entity/Q738534"), + arrayOf("n", "Baeolophus bicolor"), + arrayOf("i", "https://www.wikidata.org/entity/Q829683"), + arrayOf("n", "Mimus polyglottos"), + arrayOf("client", "birdstar.app"), + ), + content = "", + sig = "00".repeat(64), + ) + + @Test + fun factoryBuildsBirdexForKind12473() { + val event = sampleEvent() + assertTrue( + event is BirdexEvent, + "Expected a BirdexEvent but got ${event::class.simpleName}", + ) + } + + @Test + fun kind12473IsNowKnown() { + assertTrue(EventFactory.isKnownKind(BirdexEvent.KIND), "kind 12473 should be a known kind") + } + + @Test + fun parsesBirdexFields() { + val event = sampleEvent() + assertIs(event) + + assertEquals(3, event.speciesCount()) + assertEquals( + listOf("Icterus galbula", "Baeolophus bicolor", "Mimus polyglottos"), + event.speciesNames(), + ) + assertEquals("Birdex: 3 species", event.summary()) + } +} From fba4b933b0e3f6ff4ebf9ffdcb8d2ec6e4d9e260 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 7 Jun 2026 22:16:29 +0200 Subject: [PATCH 4/6] Code review: - Make birdex_species_preview_more a keyed on the remaining count - Bound the species preview with maxLines=2 - Hoist the joined-names remember out of the conditional (stable slot). - Drop the unused accountViewModel parameter - BirdexEvent.speciesCount() derives from speciesNames().size instead of re-scanning tags - remember() the joined species-name string so it is not rebuilt on every recomposition. --- .../vitorpamplona/amethyst/ui/note/NoteCompose.kt | 2 +- .../amethyst/ui/note/types/Birdex.kt | 15 +++++---------- .../screen/loggedIn/threadview/ThreadFeedView.kt | 2 +- amethyst/src/main/res/values/strings.xml | 5 ++++- .../quartz/experimental/birdstar/BirdexEvent.kt | 3 +-- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteCompose.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteCompose.kt index dbb4a20e1d..b33649a6a0 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteCompose.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/NoteCompose.kt @@ -1267,7 +1267,7 @@ private fun RenderNoteRow( } is BirdexEvent -> { - RenderBirdex(baseNote, makeItShort, accountViewModel) + RenderBirdex(baseNote) } is HighlightEvent -> { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt index 8346a62aed..5237134276 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Birdex.kt @@ -34,8 +34,6 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.model.Note -import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel -import com.vitorpamplona.amethyst.ui.stringRes import com.vitorpamplona.amethyst.ui.theme.placeholderText import com.vitorpamplona.amethyst.ui.theme.replyModifier import com.vitorpamplona.quartz.experimental.birdstar.BirdexEvent @@ -50,19 +48,16 @@ private const val SPECIES_PREVIEW_LIMIT = 6 * bounded regardless of how many species a Birdex holds, we show the count and a * short preview of scientific names with a "+N more" suffix — no images, no * expansion, no network calls. The card is identical in the feed and the opened - * view (it ignores [makeItShort]). + * view, so it takes no makeItShort flag. */ @Composable -fun RenderBirdex( - baseNote: Note, - makeItShort: Boolean, - accountViewModel: AccountViewModel, -) { +fun RenderBirdex(baseNote: Note) { val noteEvent = baseNote.event as? BirdexEvent ?: return val names = remember(noteEvent) { noteEvent.speciesNames() } val preview = remember(names) { names.take(SPECIES_PREVIEW_LIMIT) } val remaining = names.size - preview.size + val joined = remember(preview) { preview.joinToString(", ") } Column(MaterialTheme.colorScheme.replyModifier.padding(10.dp)) { Text( @@ -72,16 +67,16 @@ fun RenderBirdex( if (preview.isNotEmpty()) { Spacer(Modifier.height(6.dp)) - val joined = preview.joinToString(", ") Text( text = if (remaining > 0) { - stringRes(R.string.birdex_species_preview_more, joined, remaining.toString()) + pluralStringResource(R.plurals.birdex_species_preview_more, remaining, joined, remaining) } else { joined }, style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.placeholderText, + maxLines = 2, overflow = TextOverflow.Ellipsis, ) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/threadview/ThreadFeedView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/threadview/ThreadFeedView.kt index 689c8391a9..2de99d5f54 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/threadview/ThreadFeedView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/threadview/ThreadFeedView.kt @@ -772,7 +772,7 @@ private fun FullBleedNoteCompose( } else if (noteEvent is FundraiserEvent) { RenderFundraiser(baseNote, makeItShort = false, accountViewModel, nav) } else if (noteEvent is BirdexEvent) { - RenderBirdex(baseNote, makeItShort = false, accountViewModel) + RenderBirdex(baseNote) } else if (noteEvent is RepostEvent || noteEvent is GenericRepostEvent) { RenderRepost(baseNote, quotesLeft = 3, backgroundColor, accountViewModel, nav) } else if (noteEvent is RelayDiscoveryEvent) { diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index 400852fb5f..debeb95ff7 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -3142,7 +3142,10 @@ Birdex · %1$d species Birdex · %1$d species - %1$s +%2$s more + + %1$s +%2$d more + %1$s +%2$d more + Goal amount (sats) 100000 Describe your goal diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt index 4b170db4b8..e147d4c556 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/experimental/birdstar/BirdexEvent.kt @@ -59,13 +59,12 @@ class BirdexEvent( fun speciesNames() = tags.mapValueTagged("n") { it } /** Number of collected species (one `n` tag per species). */ - fun speciesCount() = tags.count { it.size > 1 && it[0] == "n" } + fun speciesCount() = speciesNames().size /** Publisher-provided human-readable summary, from the `alt` tag (may be null). */ fun summary() = tags.firstTagValue("alt") companion object { const val KIND = 12473 - const val ALT_DESCRIPTION = "A Birdex species collection" } } From 39531b85fba4ff9530d99e1f744dc7493d1f22d3 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 7 Jun 2026 23:04:13 +0200 Subject: [PATCH 5/6] fix(metadata): tolerate non-spec birthday so it can't drop the profile --- .../metadata/BirthdayTolerantSerializer.kt | 82 ++++++++++++ .../quartz/nip01Core/metadata/UserMetadata.kt | 2 + .../BirthdayTolerantSerializerTest.kt | 120 ++++++++++++++++++ 3 files changed, 204 insertions(+) create mode 100644 quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializer.kt create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializerTest.kt diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializer.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializer.kt new file mode 100644 index 0000000000..df549128e4 --- /dev/null +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializer.kt @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.metadata + +import com.vitorpamplona.quartz.utils.Log +import kotlinx.serialization.KSerializer +import kotlinx.serialization.descriptors.SerialDescriptor +import kotlinx.serialization.encoding.Decoder +import kotlinx.serialization.encoding.Encoder +import kotlinx.serialization.json.JsonDecoder +import kotlinx.serialization.json.JsonObject + +/** + * Tolerant serializer for the kind-0 `birthday` field. + * + * NIP-24 defines `birthday` as an object `{ "year", "month", "day" }` (each field + * optional). Some clients (e.g. Ditto / divine.video) instead write a string such + * as `"10-24"`, which is not spec-compliant. With the default serializer that type + * mismatch throws, and because [MetadataEvent.contactMetaData] turns any parse + * exception into `null`, a single malformed `birthday` would discard the **entire** + * profile (name, picture, about…). + * + * This serializer parses the spec object form and treats anything else as absent + * (`null`) rather than failing, so one non-conformant field can no longer break + * profile rendering. The string form is intentionally not "recovered": the spec + * has no string format, and a bare `"10-24"` is ambiguous (MM-DD vs DD-MM). + * + * Modelled on [com.vitorpamplona.quartz.nip11RelayInfo.FlexibleIntListSerializer]. + */ +object BirthdayTolerantSerializer : KSerializer { + private val delegate = Birthday.serializer() + + override val descriptor: SerialDescriptor = delegate.descriptor + + override fun deserialize(decoder: Decoder): Birthday? { + require(decoder is JsonDecoder) { "This serializer can only be used with Json format" } + + val element = decoder.decodeJsonElement() + if (element !is JsonObject) { + // Non-spec birthday (e.g. Ditto's "10-24" string). Ignore it rather than + // failing the whole profile parse. + Log.w("BirthdayTolerantSerializer") { "Ignoring non-object birthday: $element" } + return null + } + + return try { + decoder.json.decodeFromJsonElement(delegate, element) + } catch (e: Exception) { + Log.w("BirthdayTolerantSerializer") { "Ignoring malformed birthday object: ${e.message}" } + null + } + } + + override fun serialize( + encoder: Encoder, + value: Birthday?, + ) { + if (value == null) { + encoder.encodeNull() + } else { + delegate.serialize(encoder, value) + } + } +} diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/UserMetadata.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/UserMetadata.kt index 71362b9d79..6ebd2ad25c 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/UserMetadata.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/UserMetadata.kt @@ -48,6 +48,8 @@ class UserMetadata { var about: String? = null var bot: Boolean? = null var pronouns: String? = null + + @Serializable(with = BirthdayTolerantSerializer::class) var birthday: Birthday? = null var nip05: String? = null var domain: String? = null diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializerTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializerTest.kt new file mode 100644 index 0000000000..1495b3262f --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializerTest.kt @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.metadata + +import com.vitorpamplona.quartz.nip01Core.core.JsonMapper +import com.vitorpamplona.quartz.utils.EventFactory +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class BirthdayTolerantSerializerTest { + private fun metaWith(content: String): MetadataEvent = + EventFactory.create( + id = "ed269c23907649461da4b0fe109eed689ed1a562d33873b97ed01496dd02b87c", + pubKey = "932614571afcbad4d17a191ee281e39eebbb41b93fac8fd87829622aeb112f4d", + createdAt = 1L, + kind = MetadataEvent.KIND, + tags = emptyArray(), + content = content, + sig = "00".repeat(64), + ) as MetadataEvent + + /** + * Regression for the Ditto / divine.video profile (npub1jvnpg4c…, "MK Fain") + * whose `birthday` is the non-spec string "10-24". Before the tolerant + * serializer this threw and [MetadataEvent.contactMetaData] returned null, + * dropping the whole profile. + */ + @Test + fun stringBirthdayDoesNotDropTheProfile() { + val meta = + metaWith( + """{"name":"MK Fain","about":"Team Soapbox","picture":"https://blossom.ditto.pub/x.jpg","nip05":"mk@ditto.pub","birthday":"10-24"}""", + ).contactMetaData() + + assertIs(meta, "profile must still parse despite the malformed birthday") + assertEquals("MK Fain", meta.name) + assertEquals("https://blossom.ditto.pub/x.jpg", meta.picture) + assertEquals("mk@ditto.pub", meta.nip05) + assertNull(meta.birthday, "non-object birthday must be ignored, not fatal") + } + + @Test + fun otherNonObjectBirthdaysAreIgnored() { + // number, array, and JSON null are all non-spec for `birthday`. + listOf( + """{"name":"A","birthday":1024}""", + """{"name":"A","birthday":[10,24]}""", + """{"name":"A","birthday":null}""", + ).forEach { json -> + val meta = metaWith(json).contactMetaData() + assertIs(meta, "profile must survive birthday=$json") + assertEquals("A", meta.name) + assertNull(meta.birthday) + } + } + + @Test + fun specObjectBirthdayStillParses() { + val meta = metaWith("""{"name":"A","birthday":{"year":1990,"month":6,"day":15}}""").contactMetaData() + assertIs(meta) + val birthday = meta.birthday + assertIs(birthday) + assertEquals(1990, birthday.year) + assertEquals(6, birthday.month) + assertEquals(15, birthday.day) + } + + @Test + fun partialObjectBirthdayStillParses() { + val meta = metaWith("""{"name":"A","birthday":{"month":6,"day":15}}""").contactMetaData() + assertIs(meta) + val birthday = meta.birthday + assertIs(birthday) + assertNull(birthday.year) + assertEquals(6, birthday.month) + assertEquals(15, birthday.day) + } + + @Test + fun objectBirthdayRoundTrips() { + val meta = metaWith("""{"name":"A","birthday":{"year":1990,"month":6,"day":15}}""").contactMetaData() + assertIs(meta) + val serialized = JsonMapper.toJson(meta) + val reparsed = JsonMapper.fromJson(serialized) + val birthday = reparsed.birthday + assertIs(birthday) + assertEquals(1990, birthday.year) + assertEquals(6, birthday.month) + assertEquals(15, birthday.day) + } + + @Test + fun nullBirthdayIsOmittedOnSerialization() { + val meta = metaWith("""{"name":"A","birthday":"10-24"}""").contactMetaData() + assertIs(meta) + val serialized = JsonMapper.toJson(meta) + assertTrue("birthday" !in serialized, "a null birthday should not be serialized back out: $serialized") + } +} From 0107808ef6b312aef9de99d22ac05bfec0e2dcba Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 7 Jun 2026 23:10:16 +0200 Subject: [PATCH 6/6] Code review: - Expose a nullable descriptor - Log the JSON element kind instead of the raw, network-sourced value. - drop birthday happy-path tests duplicated by UpdateMetadataTest --- .../metadata/BirthdayTolerantSerializer.kt | 10 +++-- .../BirthdayTolerantSerializerTest.kt | 41 +++---------------- 2 files changed, 13 insertions(+), 38 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializer.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializer.kt index df549128e4..5e04867275 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializer.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializer.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.quartz.nip01Core.metadata import com.vitorpamplona.quartz.utils.Log import kotlinx.serialization.KSerializer import kotlinx.serialization.descriptors.SerialDescriptor +import kotlinx.serialization.descriptors.nullable import kotlinx.serialization.encoding.Decoder import kotlinx.serialization.encoding.Encoder import kotlinx.serialization.json.JsonDecoder @@ -48,7 +49,9 @@ import kotlinx.serialization.json.JsonObject object BirthdayTolerantSerializer : KSerializer { private val delegate = Birthday.serializer() - override val descriptor: SerialDescriptor = delegate.descriptor + // Nullable serializer ⇒ nullable descriptor, so the framework's metadata stays + // honest even on code paths that consult it (e.g. coerceInputValues). + override val descriptor: SerialDescriptor = delegate.descriptor.nullable override fun deserialize(decoder: Decoder): Birthday? { require(decoder is JsonDecoder) { "This serializer can only be used with Json format" } @@ -56,8 +59,9 @@ object BirthdayTolerantSerializer : KSerializer { val element = decoder.decodeJsonElement() if (element !is JsonObject) { // Non-spec birthday (e.g. Ditto's "10-24" string). Ignore it rather than - // failing the whole profile parse. - Log.w("BirthdayTolerantSerializer") { "Ignoring non-object birthday: $element" } + // failing the whole profile parse. Log the JSON kind only, not the raw + // (untrusted, network-sourced) value. + Log.w("BirthdayTolerantSerializer") { "Ignoring non-object birthday (${element::class.simpleName})" } return null } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializerTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializerTest.kt index 1495b3262f..839545b344 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializerTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/metadata/BirthdayTolerantSerializerTest.kt @@ -75,41 +75,12 @@ class BirthdayTolerantSerializerTest { } } - @Test - fun specObjectBirthdayStillParses() { - val meta = metaWith("""{"name":"A","birthday":{"year":1990,"month":6,"day":15}}""").contactMetaData() - assertIs(meta) - val birthday = meta.birthday - assertIs(birthday) - assertEquals(1990, birthday.year) - assertEquals(6, birthday.month) - assertEquals(15, birthday.day) - } - - @Test - fun partialObjectBirthdayStillParses() { - val meta = metaWith("""{"name":"A","birthday":{"month":6,"day":15}}""").contactMetaData() - assertIs(meta) - val birthday = meta.birthday - assertIs(birthday) - assertNull(birthday.year) - assertEquals(6, birthday.month) - assertEquals(15, birthday.day) - } - - @Test - fun objectBirthdayRoundTrips() { - val meta = metaWith("""{"name":"A","birthday":{"year":1990,"month":6,"day":15}}""").contactMetaData() - assertIs(meta) - val serialized = JsonMapper.toJson(meta) - val reparsed = JsonMapper.fromJson(serialized) - val birthday = reparsed.birthday - assertIs(birthday) - assertEquals(1990, birthday.year) - assertEquals(6, birthday.month) - assertEquals(15, birthday.day) - } - + /** + * End-to-end check that a dropped birthday does not leak back into the + * serialized profile. The omission itself is the decoder's default-null + * suppression (encodeDefaults stays false on JsonMapper), not the serializer — + * this just pins the real-world JsonMapper output. + */ @Test fun nullBirthdayIsOmittedOnSerialization() { val meta = metaWith("""{"name":"A","birthday":"10-24"}""").contactMetaData()