mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-10 00:16:59 +00:00
Merge branch 'main' into claude/poll-text-persistence-hFoho
This commit is contained in:
Vitor Pamplona
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
is HighlightEvent -> {
|
||||
RenderHighlight(
|
||||
baseNote,
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
/*
|
||||
* 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.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, so it takes no makeItShort flag.
|
||||
*/
|
||||
@Composable
|
||||
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(
|
||||
text = pluralStringResource(R.plurals.birdex_species_count, names.size, names.size),
|
||||
style = MaterialTheme.typography.titleMedium,
|
||||
)
|
||||
|
||||
if (preview.isNotEmpty()) {
|
||||
Spacer(Modifier.height(6.dp))
|
||||
Text(
|
||||
text =
|
||||
if (remaining > 0) {
|
||||
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,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
+3
@@ -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()) ||
|
||||
|
||||
+3
@@ -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()) ||
|
||||
|
||||
+2
@@ -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 ||
|
||||
|
||||
+2
@@ -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 ||
|
||||
|
||||
+4
@@ -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)
|
||||
} else if (noteEvent is RepostEvent || noteEvent is GenericRepostEvent) {
|
||||
RenderRepost(baseNote, quotesLeft = 3, backgroundColor, accountViewModel, nav)
|
||||
} else if (noteEvent is RelayDiscoveryEvent) {
|
||||
|
||||
@@ -2815,6 +2815,8 @@
|
||||
<!-- NIP-75 Zap Goals -->
|
||||
<string name="goal_closed">यह उद्देश्य समाप्त हो चुका है</string>
|
||||
<string name="goal_progress">%1$s वित्तपोषित %2$s साट्स उद्देश्य में से</string>
|
||||
<string name="fundraiser_ends">समाप्ति %1$s</string>
|
||||
<string name="fundraiser_onchain_donation">खण्डश्रृंखलाबद्ध दान</string>
|
||||
<string name="goal_amount_label">उद्देश्य संख्या (साट्स)</string>
|
||||
<string name="goal_amount_placeholder">१०००००</string>
|
||||
<string name="goal_description_label">आपके उद्देश्य का विवरण करें</string>
|
||||
|
||||
@@ -2853,6 +2853,8 @@ Zaplanowane posty z innych kont nie zostaną opublikowane, dopóki to konto jest
|
||||
<!-- NIP-75 Zap Goals -->
|
||||
<string name="goal_closed">Ta zbiórka została zamknięta</string>
|
||||
<string name="goal_progress">Sfinansowano %1$s z %2$s satoszów</string>
|
||||
<string name="fundraiser_ends">Kończy się %1$s</string>
|
||||
<string name="fundraiser_onchain_donation">Darowizna on-chain</string>
|
||||
<string name="goal_amount_label">Kwota zbiorki (w satoszach)</string>
|
||||
<string name="goal_amount_placeholder">100000</string>
|
||||
<string name="goal_description_label">Opisz cel zbiórki</string>
|
||||
|
||||
@@ -2789,6 +2789,8 @@
|
||||
<!-- NIP-75 Zap Goals -->
|
||||
<string name="goal_closed">此目标已关闭</string>
|
||||
<string name="goal_progress">设定目标为 %2$s sats，筹集到 %1$s</string>
|
||||
<string name="fundraiser_ends">结束 %1$s</string>
|
||||
<string name="fundraiser_onchain_donation">链上捐助</string>
|
||||
<string name="goal_amount_label">目标金额 (sats)</string>
|
||||
<string name="goal_amount_placeholder">100000</string>
|
||||
<string name="goal_description_label">描述您的目标</string>
|
||||
|
||||
@@ -3138,6 +3138,14 @@
|
||||
<string name="goal_progress">%1$s funded of %2$s sats goal</string>
|
||||
<string name="fundraiser_ends">Ends %1$s</string>
|
||||
<string name="fundraiser_onchain_donation">On-chain donation</string>
|
||||
<plurals name="birdex_species_count">
|
||||
<item quantity="one">Birdex · %1$d species</item>
|
||||
<item quantity="other">Birdex · %1$d species</item>
|
||||
</plurals>
|
||||
<plurals name="birdex_species_preview_more">
|
||||
<item quantity="one">%1$s +%2$d more</item>
|
||||
<item quantity="other">%1$s +%2$d more</item>
|
||||
</plurals>
|
||||
<string name="goal_amount_label">Goal amount (sats)</string>
|
||||
<string name="goal_amount_placeholder">100000</string>
|
||||
<string name="goal_description_label">Describe your goal</string>
|
||||
|
||||
@@ -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<T>` 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<BunkerResponse>(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<String, Channel<BunkerResponse>>()
|
||||
|
||||
suspend fun newResponse(responseEvent: NostrConnectEvent) {
|
||||
val decryptedJson = signer.decrypt(responseEvent.content, remoteKey)
|
||||
val bunkerResponse = OptimizedJsonMapper.fromJsonTo<BunkerResponse>(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 <T : SignerResult.RequestAddressed> 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<BunkerResponse>(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<String, Channel<BunkerResponse>>`
|
||||
- `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<BunkerResponse>(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<IntentResult>(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<SignerResult.RequestAddressed.Result<*>>(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<SignerResult.RequestAddressed.TimedOut>(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<SignerResult.RequestAddressed.Result<*>>(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<T>` 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
|
||||
+52
-66
@@ -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<String, Continuation<IntentResult>>(2000)
|
||||
private val pending = LargeCache<String, Channel<IntentResult>>()
|
||||
|
||||
// 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<T>].
|
||||
* @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 <T : IResult> launchWaitAndParse(
|
||||
requestIntentBuilder: () -> Intent,
|
||||
parser: (intent: IntentResult) -> SignerResult.RequestAddressed<T>,
|
||||
): SignerResult.RequestAddressed<T> =
|
||||
appLauncher?.let { launcher ->
|
||||
val requestIntent = requestIntentBuilder()
|
||||
val callId = RandomInstance.randomChars(32)
|
||||
): SignerResult.RequestAddressed<T> {
|
||||
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<IntentResult>(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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+70
@@ -0,0 +1,70 @@
|
||||
/*
|
||||
* 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<Array<String>>,
|
||||
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() = 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
|
||||
}
|
||||
}
|
||||
+86
@@ -0,0 +1,86 @@
|
||||
/*
|
||||
* 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.descriptors.nullable
|
||||
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<Birthday?> {
|
||||
private val delegate = Birthday.serializer()
|
||||
|
||||
// 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" }
|
||||
|
||||
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 the JSON kind only, not the raw
|
||||
// (untrusted, network-sourced) value.
|
||||
Log.w("BirthdayTolerantSerializer") { "Ignoring non-object birthday (${element::class.simpleName})" }
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
+2
@@ -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
|
||||
|
||||
+39
-28
@@ -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<NormalizedRelayUrl>,
|
||||
val maxRetries: Int = 1,
|
||||
) {
|
||||
private val awaitingRequests = LargeCache<String, Continuation<BunkerResponse>>()
|
||||
private val pending = LargeCache<String, Channel<BunkerResponse>>()
|
||||
|
||||
suspend fun newResponse(responseEvent: NostrConnectEvent) {
|
||||
val decryptedJson = signer.decrypt(responseEvent.content, remoteKey)
|
||||
val bunkerResponse = OptimizedJsonMapper.fromJsonTo<BunkerResponse>(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<T>].
|
||||
@@ -63,36 +72,38 @@ class RemoteSignerManager(
|
||||
bunkerRequestBuilder: () -> BunkerRequest,
|
||||
parser: (response: BunkerResponse) -> SignerResult.RequestAddressed<T>,
|
||||
): SignerResult.RequestAddressed<T> {
|
||||
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<BunkerResponse>(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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
+78
@@ -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<BirdexEvent>(event)
|
||||
|
||||
assertEquals(3, event.speciesCount())
|
||||
assertEquals(
|
||||
listOf("Icterus galbula", "Baeolophus bicolor", "Mimus polyglottos"),
|
||||
event.speciesNames(),
|
||||
)
|
||||
assertEquals("Birdex: 3 species", event.summary())
|
||||
}
|
||||
}
|
||||
+91
@@ -0,0 +1,91 @@
|
||||
/*
|
||||
* 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<UserMetadata>(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<UserMetadata>(meta, "profile must survive birthday=$json")
|
||||
assertEquals("A", meta.name)
|
||||
assertNull(meta.birthday)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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()
|
||||
assertIs<UserMetadata>(meta)
|
||||
val serialized = JsonMapper.toJson(meta)
|
||||
assertTrue("birthday" !in serialized, "a null birthday should not be serialized back out: $serialized")
|
||||
}
|
||||
}
|
||||
+210
@@ -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<BunkerRequest>(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<SignerResult.RequestAddressed.Successful<PingResult>>(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<SignerResult.RequestAddressed.TimedOut<PingResult>>(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<SignerResult.RequestAddressed.Successful<PingResult>>(result)
|
||||
assertEquals(attempt2Id, success.result.pong)
|
||||
}
|
||||
}
|
||||
|
||||
private class CapturingNostrClient : INostrClient {
|
||||
val publishedEvents = mutableListOf<Event>()
|
||||
|
||||
override fun connectedRelaysFlow(): StateFlow<Set<NormalizedRelayUrl>> = MutableStateFlow(emptySet())
|
||||
|
||||
override fun availableRelaysFlow(): StateFlow<Set<NormalizedRelayUrl>> = 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<NormalizedRelayUrl, List<Filter>>,
|
||||
listener: SubscriptionListener?,
|
||||
) {}
|
||||
|
||||
override fun count(
|
||||
subId: String,
|
||||
filters: Map<NormalizedRelayUrl, List<Filter>>,
|
||||
) {}
|
||||
|
||||
override fun unsubscribe(subId: String) {}
|
||||
|
||||
override fun publish(
|
||||
event: Event,
|
||||
relayList: Set<NormalizedRelayUrl>,
|
||||
) {
|
||||
publishedEvents.add(event)
|
||||
}
|
||||
|
||||
override fun pendingPublishRelaysFor(eventId: String): Set<NormalizedRelayUrl>? = null
|
||||
|
||||
override fun addConnectionListener(listener: RelayConnectionListener) {}
|
||||
|
||||
override fun removeConnectionListener(listener: RelayConnectionListener) {}
|
||||
|
||||
override fun getReqFiltersOrNull(subId: String): Map<NormalizedRelayUrl, List<Filter>>? = null
|
||||
|
||||
override fun getCountFiltersOrNull(subId: String): Map<NormalizedRelayUrl, List<Filter>>? = null
|
||||
|
||||
override fun activeRequests(url: NormalizedRelayUrl): Map<String, List<Filter>> = emptyMap()
|
||||
|
||||
override fun activeCounts(url: NormalizedRelayUrl): Map<String, List<Filter>> = emptyMap()
|
||||
|
||||
override fun activeOutboxCache(url: NormalizedRelayUrl): Set<HexKey> = emptySet()
|
||||
|
||||
override fun close() {}
|
||||
}
|
||||
|
||||
private class CountingNostrClient(
|
||||
|
||||
Reference in New Issue
Block a user