From ac47af9101d25d017de37bda92683754da7e6d55 Mon Sep 17 00:00:00 2001
From: Taksh <takshkothari09@gmail.com>
Date: Mon, 10 Aug 2026 20:09:00 +0530
Subject: [PATCH 1/2] fix: synchronize totalChecks increment in
 NostrEventDeduplicator

isDuplicate() incremented totalChecks outside the lruLock that guards
every other mutation on this class, so concurrent callers raced on the
read-modify-write and lost increments. duplicateCount and evictionCount
were already incremented under the lock; totalChecks was the one
counter left outside it.

getStats().totalChecks feeds hitRate and is the only denominator for
duplicateCount, so a systematic undercount skews both. Moved the
increment inside the existing synchronized block.

Added a concurrency test that reproduces the race: run against the
prior code it failed reliably (3/3 runs); against the fix it passes.
---
 .../android/nostr/NostrEventDeduplicator.kt   |  4 +-
 .../nostr/NostrEventDeduplicatorTest.kt       | 66 +++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 app/src/test/kotlin/com/bitchat/android/nostr/NostrEventDeduplicatorTest.kt

diff --git a/app/src/main/java/com/bitchat/android/nostr/NostrEventDeduplicator.kt b/app/src/main/java/com/bitchat/android/nostr/NostrEventDeduplicator.kt
index 0638eafd..138ffb0a 100644
--- a/app/src/main/java/com/bitchat/android/nostr/NostrEventDeduplicator.kt
+++ b/app/src/main/java/com/bitchat/android/nostr/NostrEventDeduplicator.kt
@@ -79,9 +79,9 @@ class NostrEventDeduplicator(
      * @return true if the event is a duplicate (already seen), false if it's new
      */
     fun isDuplicate(eventId: String): Boolean {
-        totalChecks++
-        
         synchronized(lruLock) {
+            totalChecks++
+
             val existingNode = nodeMap[eventId]
             
             if (existingNode != null) {
diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/NostrEventDeduplicatorTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/NostrEventDeduplicatorTest.kt
new file mode 100644
index 00000000..557ead2e
--- /dev/null
+++ b/app/src/test/kotlin/com/bitchat/android/nostr/NostrEventDeduplicatorTest.kt
@@ -0,0 +1,66 @@
+package com.bitchat.android.nostr
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.Executors
+import java.util.concurrent.TimeUnit
+
+class NostrEventDeduplicatorTest {
+    @Test
+    fun `isDuplicate flags a repeated event id`() {
+        val deduplicator = NostrEventDeduplicator(maxCapacity = 10)
+
+        assertEquals(false, deduplicator.isDuplicate("event-1"))
+        assertEquals(true, deduplicator.isDuplicate("event-1"))
+    }
+
+    @Test
+    fun `capacity evicts the least recently used event id`() {
+        val deduplicator = NostrEventDeduplicator(maxCapacity = 2)
+
+        deduplicator.isDuplicate("a")
+        deduplicator.isDuplicate("b")
+        deduplicator.isDuplicate("c") // evicts "a"
+
+        assertTrue(deduplicator.contains("b"))
+        assertTrue(deduplicator.contains("c"))
+        assertEquals(false, deduplicator.contains("a"))
+    }
+
+    /**
+     * totalChecks used to be incremented outside the lruLock that guards every other
+     * mutation in this class, so concurrent callers could race on the read-modify-write
+     * and lose increments. Every other counter (duplicateCount, evictionCount) was
+     * already incremented under the lock, which is why only this one drifted.
+     */
+    @Test
+    fun `totalChecks counts every call exactly once under concurrent access`() {
+        val deduplicator = NostrEventDeduplicator(maxCapacity = 10_000)
+        val threadCount = 8
+        val checksPerThread = 2_000
+        val executor = Executors.newFixedThreadPool(threadCount)
+        val start = CountDownLatch(1)
+        val done = CountDownLatch(threadCount)
+
+        repeat(threadCount) { threadIndex ->
+            executor.submit {
+                start.await()
+                repeat(checksPerThread) { callIndex ->
+                    deduplicator.isDuplicate("thread-$threadIndex-event-$callIndex")
+                }
+                done.countDown()
+            }
+        }
+
+        start.countDown()
+        assertTrue(done.await(10, TimeUnit.SECONDS))
+        executor.shutdown()
+
+        assertEquals(
+            (threadCount * checksPerThread).toLong(),
+            deduplicator.getStats().totalChecks
+        )
+    }
+}