diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt index 9de8c49293..1018dde865 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/LocalPreferences.kt @@ -112,6 +112,7 @@ private object PrefKeys { const val NIP46_SIGNER_ENABLED = "nip46SignerEnabled" const val NIP46_BUNKER_SECRET = "nip46BunkerSecret" const val NIP46_TRANSPORT_KEY = "nip46TransportKey" + const val NIP46_SEEN_IDS = "nip46SeenRequestIds" const val DEFAULT_HOME_FOLLOW_LIST = "defaultHomeFollowList" const val DEFAULT_STORIES_FOLLOW_LIST = "defaultStoriesFollowList" const val DEFAULT_NOTIFICATION_FOLLOW_LIST = "defaultNotificationFollowList" @@ -471,6 +472,7 @@ object LocalPreferences { putBoolean(PrefKeys.NIP46_SIGNER_ENABLED, settings.nip46SignerEnabled.value) putString(PrefKeys.NIP46_BUNKER_SECRET, settings.nip46BunkerSecret.value) putString(PrefKeys.NIP46_TRANSPORT_KEY, settings.nip46TransportKey.value) + putStringSet(PrefKeys.NIP46_SEEN_IDS, settings.nip46SeenRequestIds.value) putString(PrefKeys.DEFAULT_HOME_FOLLOW_LIST, JsonMapper.toJson(settings.defaultHomeFollowList.value)) putString(PrefKeys.DEFAULT_STORIES_FOLLOW_LIST, JsonMapper.toJson(settings.defaultStoriesFollowList.value)) @@ -684,6 +686,7 @@ object LocalPreferences { val nip46SignerEnabled = getBoolean(PrefKeys.NIP46_SIGNER_ENABLED, false) val nip46BunkerSecret = getString(PrefKeys.NIP46_BUNKER_SECRET, "") ?: "" val nip46TransportKey = getString(PrefKeys.NIP46_TRANSPORT_KEY, "") ?: "" + val nip46SeenRequestIds = getStringSet(PrefKeys.NIP46_SEEN_IDS, null) ?: setOf() val hideDeleteRequestDialog = getBoolean(PrefKeys.HIDE_DELETE_REQUEST_DIALOG, false) val hideBlockAlertDialog = getBoolean(PrefKeys.HIDE_BLOCK_ALERT_DIALOG, false) val hideNIP17WarningDialog = getBoolean(PrefKeys.HIDE_NIP_17_WARNING_DIALOG, false) @@ -866,6 +869,7 @@ object LocalPreferences { nip46SignerEnabled = MutableStateFlow(nip46SignerEnabled), nip46BunkerSecret = MutableStateFlow(nip46BunkerSecret), nip46TransportKey = MutableStateFlow(nip46TransportKey), + nip46SeenRequestIds = MutableStateFlow(nip46SeenRequestIds), defaultHomeFollowList = MutableStateFlow(followListPrefs.home), defaultStoriesFollowList = MutableStateFlow(followListPrefs.stories), defaultNotificationFollowList = MutableStateFlow(followListPrefs.notification), diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountSettings.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountSettings.kt index a8ea44e447..69fc7d61a5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountSettings.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountSettings.kt @@ -207,6 +207,13 @@ class AccountSettings( * `bunker://` address doesn't change. */ val nip46TransportKey: MutableStateFlow = MutableStateFlow(""), + /** + * The kind-24133 **event ids** this signer recently serviced. Persisted so that a relay replaying + * stored ephemeral requests across an app restart doesn't make it sign the same request twice — + * matched by exact event id, so it is immune to client clock skew (unlike a timestamp watermark, + * a global timestamp would wrongly drop a second app whose clock lags). Bounded to a recent window. + */ + val nip46SeenRequestIds: MutableStateFlow> = MutableStateFlow(emptySet()), /** * NIP-9B opt-in: when true, community feeds drop events whose latest cached * `kind:34551` rules document fails [com.vitorpamplona.quartz.nip72ModCommunities.rules.CommunityRulesValidator]. @@ -625,6 +632,14 @@ class AccountSettings( } } + /** Replaces the recent serviced-request id set (already bounded by the caller). */ + fun changeNip46SeenRequestIds(ids: Set) { + if (nip46SeenRequestIds.value != ids) { + nip46SeenRequestIds.tryEmit(ids) + saveAccountSettings() + } + } + fun changeLocalBlossomCacheProfilePicturesOnly(enabled: Boolean) { if (localBlossomCacheProfilePicturesOnly.value != enabled) { localBlossomCacheProfilePicturesOnly.tryEmit(enabled) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt index bc33625974..14eb8f3ebd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip46Signer/Nip46SignerState.kt @@ -56,6 +56,9 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +/** How many recently-serviced request ids to persist for cross-restart replay dedup. */ +private const val MAX_SEEN_IDS = 128 + /** * Runs Amethyst as a NIP-46 remote signer ("bunker") for the account, so other * apps can sign through it. While [AccountSettings.nip46SignerEnabled] is on, a @@ -91,6 +94,24 @@ class Nip46SignerState( /** Newest-first, in-memory feed of serviced requests, so the UI can show what apps are doing. */ val activityLog = Nip46ActivityLog() + /** + * A bounded, recently-serviced set of kind-24133 event ids, persisted so a relay replaying stored + * requests after an app restart is deduped by exact id (see [NostrConnectSignerService.initialSeen]). + * Touched only from the service's single consumer coroutine, so it needs no synchronization. + */ + private val recentHandledIds = LinkedHashSet(settings.nip46SeenRequestIds.value) + + private fun rememberHandledId(eventId: HexKey) { + if (!recentHandledIds.add(eventId)) return + while (recentHandledIds.size > MAX_SEEN_IDS) { + recentHandledIds.iterator().let { + it.next() + it.remove() + } + } + settings.changeNip46SeenRequestIds(recentHandledIds.toSet()) + } + /** * The dedicated per-account transport signer that wraps the kind-24133 envelope — a local key * unrelated to the account identity, so the bunker address/traffic doesn't reveal who it is for, @@ -184,6 +205,10 @@ class Nip46SignerState( ), ) }, + // Seed dedup with the ids we serviced last session so an app restart doesn't + // re-sign a relay's replay of the same stored requests (matched by exact id). + initialSeen = settings.nip46SeenRequestIds.value, + onHandledId = { id -> rememberHandledId(id) }, ) service.run() } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerService.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerService.kt index 247d9d85d3..ed7b05825b 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerService.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerService.kt @@ -97,6 +97,15 @@ class NostrConnectSignerService( * kept small so an app restart re-signs as little as possible (relays replay only this far back). */ val maxRequestAgeSeconds: Long = 30, + /** + * Event ids serviced in a previous run, used to seed the in-memory dedup set so a relay replaying + * stored requests across an app restart is caught by EXACT event id — immune to client clock skew, + * unlike a timestamp floor (which would wrongly drop a second app whose clock lags). The host + * persists these (bounded) and feeds them back on start; see [onHandledId]. + */ + val initialSeen: Set = emptySet(), + /** Invoked with each serviced request's kind-24133 event id so the host can persist it for [initialSeen]. */ + val onHandledId: (suspend (eventId: String) -> Unit)? = null, ) { /** * Fixed-window per-author rate limit. Touched only by the single consumer @@ -170,7 +179,9 @@ class NostrConnectSignerService( // Insertion-ordered dedup, confined to this one consumer coroutine (never the relay threads); // evicts the oldest id past the cap so a long-lived signer can't grow it without bound. - val seen = LinkedHashSet() + // Seed the dedup set with ids serviced in a prior run so a relay replaying stored requests after + // a restart is caught by exact id (see [initialSeen]). + val seen = LinkedHashSet(initialSeen) // Only ask relays for recent requests: kind-24133 is ephemeral, but relays that store it would // otherwise replay every old request each time we (re)subscribe. See [maxRequestAgeSeconds]. val filter = Filter(kinds = listOf(NostrConnectEvent.KIND), tags = mapOf("p" to listOf(self)), since = TimeUtils.now() - maxRequestAgeSeconds) @@ -185,11 +196,11 @@ class NostrConnectSignerService( it.remove() } } - // Drop stale requests a relay replayed from storage (the `since` filter covers compliant - // relays; this covers the rest). A live NIP-46 request is seconds old; a minutes-old one - // is a replay we may already have signed in a previous subscription. + // Drop stale requests a relay replayed from storage past the rolling age window (the + // `since` filter covers compliant relays; this covers the rest; exact-id replays within + // the window are already caught by [seen] above). A live NIP-46 request is seconds old. if (TimeUtils.now() - event.createdAt > maxRequestAgeSeconds) { - Log.w("NIP46Signer") { "ignoring stale request ${event.id.take(8)}… (${TimeUtils.now() - event.createdAt}s old)" } + Log.w("NIP46Signer") { "ignoring stale request ${event.id.take(8)}… (created ${event.createdAt})" } continue } // Rate-limit per author BEFORE decrypting — decryption can be an external-signer @@ -199,6 +210,8 @@ class NostrConnectSignerService( continue } handle(event) + // Remember this id (persisted by the host) so a later restart won't re-service the replay. + onHandledId?.invoke(event.id) } } finally { client.unsubscribe(subId) diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerServiceTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerServiceTest.kt index 6515e06621..98a7dcff66 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerServiceTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip46RemoteSigner/server/NostrConnectSignerServiceTest.kt @@ -252,6 +252,42 @@ class NostrConnectSignerServiceTest { assertEquals(0, client.published.size, "a minutes-old replayed request is dropped, not re-signed") } + @Test + fun aFreshRequestWhoseIdWasServicedLastSessionIsNotRepeated() = + runTest { + val client = LoopbackClient() + val signer = serverSigner() + val processor = BunkerRequestProcessor(signer, { setOf(relay) }, AllowAuthorizer()) + + // A still-fresh request whose id the previous run already serviced (seeded via initialSeen, + // as the host would restore from disk). It must be deduped by exact id — even though its + // created_at is within the window — so an app restart doesn't re-sign a relay's replay. + val template = EventTemplate(createdAt = 1L, kind = 1, tags = emptyArray(), content = "again") + val replayed = request(BunkerRequestSign(id = "req", event = template)) + val service = NostrConnectSignerService(client, signer, processor, setOf(relay), initialSeen = setOf(replayed.id)) + + backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { service.run() } + client.deliver(replayed) + + assertEquals(0, client.published.size, "an id serviced last session is not signed again after restart") + } + + @Test + fun aHandledIdIsReportedForPersistence() = + runTest { + val client = LoopbackClient() + val signer = serverSigner() + val processor = BunkerRequestProcessor(signer, { setOf(relay) }, AllowAuthorizer()) + val handled = mutableListOf() + val service = NostrConnectSignerService(client, signer, processor, setOf(relay), onHandledId = { handled.add(it) }) + + backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { service.run() } + val event = request(BunkerRequestSign(id = "req", event = EventTemplate(createdAt = 1L, kind = 1, tags = emptyArray(), content = "x"))) + client.deliver(event) + + assertEquals(listOf(event.id), handled, "the serviced event id is reported so the host can persist it") + } + @Test fun logoutRequestInvokesAuthorizerAndAcks() = runTest {