From 573c5c2b0693397841594089d3223893a72d982b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 20:03:54 +0000 Subject: [PATCH] fix(marmot): trust MLS authentication, skip Nostr verify on inner events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit (6ad522b5) shielded the chatroom from the mistaken "duplicate" drop but was treating the symptom. The real cause of the "inner event already in cache" log — reported while Test 02's chat stayed completely empty — is that `cache.justConsume(innerEvent, null, wasVerified=false)` runs `event.verify()` on a wn-signed inner kind:9, that verify fails (likely JSON-canonicalization skew or a rumor-style empty signature), so `consumeRegularEvent` bails at line 633 before ever setting `note.event`. LocalCache then always had the empty Note registered by id but with no event content — subsequent arrivals re-hit line 615 (`note.event != null` is still false, not the "duplicate" path), retried verify, failed again, and the chatroom never got a populated message to render. The inner event has already been cryptographically authenticated by MLS — `MarmotInboundProcessor.processPrivateMessage` rejects any inner event whose `pubkey` doesn't match the MLS sender's credential identity (MIP-03, line 430). Running Nostr's `event.verify()` again is a redundant gate. Pass `wasVerified=true`: `consumeRegularEvent` skips `justVerify` and calls `note.loadEvent(...)` on first arrival, chatroom-add works immediately, and the test passes. Also tightens the "already in cache" comment to reflect what that branch actually means now (a legitimate hydrated-by-startup-restore duplicate, not a silent verify failure masquerading as one). --- .../loggedIn/DecryptAndIndexProcessor.kt | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt index 49b91b287..ef3894539 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt @@ -592,23 +592,28 @@ class GroupEventHandler( "GroupEventHandler.add: ApplicationMessage decrypted innerKind=${innerEvent.kind} " + "innerId=${innerEvent.id.take(8)}… author=${innerEvent.pubKey.take(8)}…" } - // `cache.justConsume` returns false if LocalCache already - // has this inner event id — typically because a previous - // session persisted the inner JSON and the startup - // restore loop in Account reloaded it into the cache - // before this kind:445 arrived. The chatroom may still - // not have it (e.g. the user's chatroom snapshot was - // dropped, or the restore-from-disk path raced ahead and - // the cache populated before the chatroom was hydrated). - // `addMessage` is itself idempotent (`addMessageSync` - // dedupes by Note identity), so always surface the note - // in the chatroom — otherwise the message is silently - // dropped and the operator sees only the "duplicate" log. - val isNew = cache.justConsume(innerEvent, null, false) + // Treat the inner event as pre-verified. MLS already + // authenticated the sender: MarmotInboundProcessor. + // processPrivateMessage rejects any inner event whose + // `pubkey` field doesn't match the MLS sender's + // credential identity. Running Nostr sig verify again + // here is a redundant gate — and one that silently + // dropped wn-signed inner events (wn's kind:9 rumors + // don't always round-trip Amethyst's `event.verify()`, + // e.g. because of JSON canonicalization differences). + // Without this, justConsume returned false, the chatroom + // never got the message, and the operator saw nothing + // but a misleading "inner event already in cache" log. + val isNew = cache.justConsume(innerEvent, null, true) val innerNote = cache.getOrCreateNote(innerEvent.id) - if (isNew) { - innerNote.event = innerEvent - } else { + if (!isNew) { + // Legit duplicate: a prior session persisted the + // inner JSON and the startup restore loop hydrated + // the cache before this kind:445 arrived. The + // chatroom may still not have it (restore order / + // chatroom snapshot drop), so `addMessage` runs + // unconditionally — `MarmotGroupChatroom.addMessageSync` + // dedupes by Note identity if it was already there. Log.d("MarmotDbg") { "GroupEventHandler.add: inner event already in cache — surfacing in chatroom anyway" }