From 0ec1544d66244976fc8b075db998befe49fe48bf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Apr 2026 15:39:56 +0000 Subject: [PATCH] fix: keep undecryptable Marmot kind:445 events retryable after epoch advance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MarmotInboundProcessor.processGroupEvent` added every event id to its dedup set regardless of result, including `UndecryptableOuterLayer`. The handler buffers those events for a post-commit retry, but the replay hit the Duplicate early-return and skipped MLS decryption — so future-epoch messages and group-metadata commits that arrived before their epoch's commit were silently dropped until an app restart re-fetched them. This is interop test 12 (offline catch-up): messages 6-8 and the rename commit only appeared after restarting Amethyst. Skip the dedup write on `UndecryptableOuterLayer` so retries actually re-attempt decrypt once the epoch advances. Memory is still bounded by the handler's per-group pending buffer (64 events, FIFO-evicted). --- .../quartz/marmot/MarmotInboundProcessor.kt | 28 ++++++---- .../quartz/marmot/MarmotPipelineTest.kt | 55 +++++++++++++++++++ 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotInboundProcessor.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotInboundProcessor.kt index 412133729..77b745796 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotInboundProcessor.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotInboundProcessor.kt @@ -229,16 +229,24 @@ class MarmotInboundProcessor( GroupEventResult.Error(groupId, "Failed to process GroupEvent: ${e.message}", e) } - // Track ALL processed events for deduplication (including errors to prevent replay DoS) - processedIdsMutex.withLock { - processedEventIds.add(eventId) - // Trim the set if it exceeds the max size - if (processedEventIds.size > MAX_PROCESSED_IDS) { - val iterator = processedEventIds.iterator() - val toRemove = processedEventIds.size - MAX_PROCESSED_IDS - repeat(toRemove) { - iterator.next() - iterator.remove() + // Track processed events for dedup — except UndecryptableOuterLayer, + // which must stay retryable. These events are typically future-epoch + // arrivals buffered by the handler and replayed after a + // CommitProcessed advances our epoch; marking them processed here + // would cause the retry to hit the Duplicate early-return above and + // skip MLS decryption entirely. DoS is already bounded by the + // handler's per-group pending buffer. + if (result !is GroupEventResult.UndecryptableOuterLayer) { + processedIdsMutex.withLock { + processedEventIds.add(eventId) + // Trim the set if it exceeds the max size + if (processedEventIds.size > MAX_PROCESSED_IDS) { + val iterator = processedEventIds.iterator() + val toRemove = processedEventIds.size - MAX_PROCESSED_IDS + repeat(toRemove) { + iterator.next() + iterator.remove() + } } } } diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt index 9727705f2..106a33f98 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt @@ -236,6 +236,61 @@ class MarmotPipelineTest { } } + /** + * Regression: the interop harness's Test 12 drove future-epoch kind:445 + * events into Amethyst before the epoch-advancing commit arrived — those + * failed with [GroupEventResult.UndecryptableOuterLayer] and were buffered + * by the handler for a post-commit retry. The retry fired after the + * commit but then came back as [GroupEventResult.Duplicate] instead of + * re-attempting decrypt, so the messages were silently dropped until app + * restart re-fetched them from relays. + * + * The cause was [MarmotInboundProcessor.processGroupEvent] adding the + * outer event id to `processedEventIds` after *every* call, including + * UndecryptableOuterLayer. The retry replayed the same event, hit the + * dedup early-return, and skipped MLS processing entirely. An undecryptable + * outer layer must stay retryable — the handler's pending buffer already + * bounds memory per-group. + */ + @Test + fun testInboundUndecryptableOuterLayerStaysRetryable() { + runBlocking { + val manager = createGroupManager() + manager.createGroup(groupId, "alice".encodeToByteArray()) + + val keyPackageRotationManager = KeyPackageRotationManager() + val inbound = MarmotInboundProcessor(manager, keyPackageRotationManager) + + // A kind:445 for our group whose ciphertext won't decrypt under + // our current exporter key — the shape a future-epoch event has + // before its commit arrives. 64 bytes is well past MIN_CONTENT_LENGTH + // so we exercise the AEAD failure path, not a short-input reject. + @OptIn(kotlin.io.encoding.ExperimentalEncodingApi::class) + val garbageCiphertext = + kotlin.io.encoding.Base64 + .encode(ByteArray(64) { 0x42.toByte() }) + val template = + GroupEvent.build( + encryptedContentBase64 = garbageCiphertext, + nostrGroupId = groupId, + ) + val signer = NostrSignerInternal(KeyPair()) + val event: GroupEvent = signer.sign(template) + + val first = inbound.processGroupEvent(event) + assertIs( + first, + "first pass must surface UndecryptableOuterLayer so the handler can buffer for retry", + ) + + val second = inbound.processGroupEvent(event) + assertIs( + second, + "replaying the same undecryptable event (what retryPendingFor does after a CommitProcessed) must re-attempt decrypt, not short-circuit as Duplicate", + ) + } + } + @Test fun testInboundRejectsNonMemberGroup() { runBlocking {