diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index 35619d5e96..1163b08b42 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -267,25 +267,28 @@ import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.channels.awaitClose -import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asSharedFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.buffer import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch +import kotlinx.coroutines.supervisorScope import kotlinx.coroutines.sync.Semaphore import kotlinx.coroutines.sync.withPermit import java.io.File import java.io.FileOutputStream import java.io.IOException import java.util.SortedSet +import java.util.concurrent.ConcurrentHashMap interface ILocalCache { fun markAsSeen( @@ -1900,9 +1903,7 @@ object LocalCache : ILocalCache, ICacheProvider { // could otherwise put "-1" in the gallery as a negative-sats badge. val claimedSats = (event.claimedAmountInSats() ?: 0L).coerceAtLeast(0L) repliesTo.forEach { - if (!it.wasOnchainZapRejectedForSource(txid, event.pubKey)) { - it.addOnchainZap(note, txid, claimedSats, verifiedSats = 0L, OnchainZapStatus.UNVERIFIED) - } + it.addOnchainZap(note, txid, claimedSats, verifiedSats = 0L, OnchainZapStatus.UNVERIFIED) } } } @@ -1915,25 +1916,29 @@ object LocalCache : ILocalCache, ICacheProvider { // profile zap views see it, but it can't contribute to Note totals. val backend = onchainBackend ?: return !alreadyLoaded - // Re-arrival path: on a later relay echo of the same event, skip the verifier - // launch entirely if every target note already holds a CONFIRMED entry for this - // txid. Avoids the relay-echo fan-out where the same kind:8333 is delivered N - // times and spawns N redundant verifier coroutines (each potentially hitting - // Esplora for any uncached lookup). - val txid = event.txid() - if (alreadyLoaded && txid != null) { - val repliesTo = computeReplyTo(event) - val allConfirmed = - repliesTo.isNotEmpty() && - repliesTo.all { it.onchainZaps[txid]?.status == OnchainZapStatus.CONFIRMED } - if (allConfirmed) return false - } + // Skip the verifier launch entirely once the chain has spoken definitively + // (Confirmed, or hard-rejected with a non-transient reason). The resolved + // flag is per-source-event and travels with the Note, so it survives across + // relay echoes and even covers profile-only zaps (which would otherwise + // bypass any per-target CONFIRMED check because they have no replyTo notes). + if (note.onchainZapResolved) return !alreadyLoaded + + // De-duplicate concurrent launches. `verifyingEventIds.add` returns false if + // another coroutine has already started verifying this exact event id — + // covers (a) two relays delivering the same event simultaneously, + // (b) a relay echo arriving while the prior verifier is still in flight, + // (c) `reverifyOnchainZapsForNote` racing with `consume`. + if (!verifyingEventIds.add(event.id)) return !alreadyLoaded val verifier = OnchainZapVerifier(backend) val repliesTo = computeReplyTo(event) Amethyst.instance.applicationIOScope.launch { - verifyAndUpgradeOnchainZap(event, note, repliesTo, verifier) + try { + verifyAndUpgradeOnchainZap(event, note, repliesTo, verifier) + } finally { + verifyingEventIds.remove(event.id) + } } return !alreadyLoaded @@ -1945,6 +1950,9 @@ object LocalCache : ILocalCache, ICacheProvider { * monotonically (UNVERIFIED → PENDING → CONFIRMED) and won't move backwards, and * source-scoped removal prevents one event's rejection from erasing another sender's * legitimate entry that happens to share a txid. + * + * Callers must wrap the call site with a `verifyingEventIds`/`reverifyingNoteIds` + * gate; this function does not itself protect against duplicate concurrent runs. */ private suspend fun verifyAndUpgradeOnchainZap( event: OnchainZapEvent, @@ -1960,12 +1968,19 @@ object LocalCache : ILocalCache, ICacheProvider { repliesTo.forEach { it.addOnchainZap(source, result.txid, claimedSats, result.verifiedSats, OnchainZapStatus.CONFIRMED) } + // Terminal — the chain has confirmed. Future relay echoes of this + // event id skip the verifier entirely via the `onchainZapResolved` + // gate in `consume()`. + source.onchainZapResolved = true } is VerifiedOnchainZap.Pending -> { repliesTo.forEach { it.addOnchainZap(source, result.txid, claimedSats, result.verifiedSats, OnchainZapStatus.PENDING) } + // Not terminal — the tx is in the mempool. A future tip-poll or + // reverify call can upgrade it to CONFIRMED, so leave the + // resolved flag false. } is VerifiedOnchainZap.Rejected -> { @@ -1978,11 +1993,22 @@ object LocalCache : ILocalCache, ICacheProvider { } else if (result.txid.isNotEmpty()) { // Hard rejection — e.g. ZERO_VERIFIED_AMOUNT means this sender's // tx did not pay the recipient. Drop only entries whose source - // matches THIS event (Note.removeOnchainZapForSource enforces - // the match), so a spoofer can't erase a legitimate CONFIRMED - // entry that happens to share the same txid. + // matches THIS event AND that aren't CONFIRMED (the per-target + // CONFIRMED check inside `removeOnchainZapForSource` prevents a + // transient backend hiccup from wiping a previously-verified + // entry on a sibling target). Log.d("OnchainZap") { "rejected ${result.txid}: ${result.reason}" } repliesTo.forEach { it.removeOnchainZapForSource(result.txid, event.pubKey) } + // Terminal — don't re-verify on future relay echoes of this + // event id. (Different event ids with the same txid still go + // through their own verifier pass.) + source.onchainZapResolved = true + } else { + // MISSING_TXID etc. — log so we have observability when a + // malformed event reaches the backend. Mark terminal so we + // don't re-verify the same broken event on every echo. + Log.d("OnchainZap") { "rejected ${event.id}: ${result.reason} (no txid)" } + source.onchainZapResolved = true } } } @@ -1999,37 +2025,74 @@ object LocalCache : ILocalCache, ICacheProvider { * Safe to call from a screen-visibility hook or a chain-tip change observer; each * verifier call is bounded by the chain backend's cache TTLs. * - * Runs the per-entry verifier calls in parallel (capped by [reverifySemaphore]) - * so a thread root with many pending entries finishes within typical screen - * dwell time instead of N × RTT sequentially. + * - Per-note in-flight gate (`reverifyingNoteIds`) — if another caller is already + * reverifying this note (e.g. multiple visible galleries fired in the same tip + * tick) we skip the dup. + * - Per-event in-flight gate (`verifyingEventIds`) — coordinates with `consume()` + * so the same event isn't verified twice concurrently. + * - `supervisorScope` — a single verifier failure won't cancel sibling verifiers + * for other entries on the same note. + * - Bounded parallelism via [reverifySemaphore] so a thread with many pending + * entries doesn't blast Esplora. */ suspend fun reverifyOnchainZapsForNote(note: Note) { val backend = onchainBackend ?: return - val pendingEntries = - note.onchainZaps.values.filter { it.status != OnchainZapStatus.CONFIRMED } - if (pendingEntries.isEmpty()) return + if (!reverifyingNoteIds.add(note.idHex)) return + try { + val pendingEntries = + note.onchainZaps.values.filter { it.status != OnchainZapStatus.CONFIRMED } + if (pendingEntries.isEmpty()) return - val verifier = OnchainZapVerifier(backend) - coroutineScope { - pendingEntries - .mapNotNull { entry -> - val sourceEvent = entry.source.event as? OnchainZapEvent ?: return@mapNotNull null - async { - reverifySemaphore.withPermit { - val repliesTo = computeReplyTo(sourceEvent) - verifyAndUpgradeOnchainZap(sourceEvent, entry.source, repliesTo, verifier) + val verifier = OnchainZapVerifier(backend) + supervisorScope { + pendingEntries + .mapNotNull { entry -> + val sourceEvent = entry.source.event as? OnchainZapEvent ?: return@mapNotNull null + val source = entry.source + if (source.onchainZapResolved) return@mapNotNull null + if (!verifyingEventIds.add(sourceEvent.id)) return@mapNotNull null + async { + try { + reverifySemaphore.withPermit { + val repliesTo = computeReplyTo(sourceEvent) + verifyAndUpgradeOnchainZap(sourceEvent, source, repliesTo, verifier) + } + } finally { + verifyingEventIds.remove(sourceEvent.id) + } } - } - }.awaitAll() + }.awaitAll() + } + } finally { + reverifyingNoteIds.remove(note.idHex) } } + /** + * In-flight set of event ids currently being verified. Prevents two consume() + * calls (or a consume + a reverify) from issuing parallel Esplora fetches for + * the same event. `ConcurrentHashMap.newKeySet` gives lock-free atomic `add` + * returning `true` only for the inserting caller. + */ + private val verifyingEventIds: MutableSet = ConcurrentHashMap.newKeySet() + + /** + * In-flight set of note id strings currently being reverified. Lets the + * onchain-zap gallery driver be called from many visible composables without + * the same note's reverify pass running concurrently. The per-event gate is + * the backstop; this one short-circuits earlier and avoids creating async + * coroutines that would just no-op. + */ + private val reverifyingNoteIds: MutableSet = ConcurrentHashMap.newKeySet() + /** * Caps the parallelism of [reverifyOnchainZapsForNote] so a thread with many * pending entries doesn't blast public Esplora endpoints with a burst of - * simultaneous requests. + * simultaneous requests. Tuned a little higher than the previous 4 so that + * concurrent galleries each running a small reverify aren't fully serialized + * behind a single big one. */ - private val reverifySemaphore = Semaphore(permits = 4) + private val reverifySemaphore = Semaphore(permits = 8) /** * Shared poller for the current bitcoin chain tip height. Each gallery that @@ -2038,16 +2101,37 @@ object LocalCache : ILocalCache, ICacheProvider { * only fires when at least one UI surface needs it. * * Lazy initialization is required because [Amethyst.instance] may not exist - * when the `LocalCache` singleton is class-loaded. + * when the `LocalCache` singleton is class-loaded. Falls back to a constant + * null-emitting StateFlow if the application scope isn't available yet + * (e.g. unit tests, ContentProvider invocations), so the lazy field doesn't + * permanently fail with `UninitializedPropertyAccessException`. */ val onchainTipHeightFlow: StateFlow by lazy { + val scope = + runCatching { Amethyst.instance.applicationIOScope }.getOrNull() + ?: return@lazy MutableStateFlow(null).asStateFlow() + flow { while (true) { - emit(onchainBackend?.let { runCatching { it.tipHeight() }.getOrNull() }) + val tip = + onchainBackend?.let { backend -> + // Explicit try/catch instead of `runCatching` because the latter + // would swallow CancellationException too — when the upstream + // scope cancels, we must let it propagate so the flow tears down + // promptly instead of looping through one more delay(). + try { + backend.tipHeight() + } catch (e: CancellationException) { + throw e + } catch (t: Throwable) { + null + } + } + emit(tip) delay(ONCHAIN_TIP_POLL_INTERVAL_MS) } }.stateIn( - Amethyst.instance.applicationIOScope, + scope, SharingStarted.WhileSubscribed(stopTimeoutMillis = 5_000L), initialValue = null, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/OnchainZapGallery.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/OnchainZapGallery.kt index 8726ccba70..98b67d0a32 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/OnchainZapGallery.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/OnchainZapGallery.kt @@ -69,45 +69,62 @@ internal fun WatchOnchainZapsAndRenderGallery( ) { // Reuse the same flow the lightning gallery subscribes to. Note.addOnchainZap // invalidates flowSet.zaps, so this composable refreshes when on-chain zaps - // arrive or upgrade pending → confirmed. The flow also fires for lightning - // zap arrivals on the same note, so memoize the list snapshot. + // arrive or upgrade pending → confirmed. The flow ALSO fires for lightning + // zap arrivals on the same note — memoize on the onchainZaps map reference + // (a fresh immutable map per onchain mutation, stable across lightning-only + // updates) so a busy lightning thread doesn't churn this gallery's state. val zapsState by observeNoteZaps(baseNote, accountViewModel) + val onchainZapsMap = zapsState?.note?.onchainZaps val entries = - remember(zapsState) { - zapsState - ?.note - ?.onchainZaps - ?.values - ?.toImmutableList() ?: persistentListOf() + remember(onchainZapsMap) { + onchainZapsMap?.values?.toImmutableList() ?: persistentListOf() } if (entries.isNotEmpty()) { - // Drive periodic re-verification of any non-CONFIRMED entries while this - // gallery is on screen — covers home feed, profile, notifications, channels, - // single-note view, threads. Keyed on baseNote.idHex so the subscription is - // stable across recompositions and pauses cleanly when the gallery scrolls - // off-screen (WhileSubscribed on the shared tip flow). + // Drive re-verification of any non-CONFIRMED entries while this gallery + // is on screen — covers home feed, profile, notifications, channels, + // single-note view, threads. Per-note in-flight gating inside the cache + // dedupes the work when multiple gallery instances for the same note + // (lazy-list off/on screen flicker, split feed) all fire together. DriveOnchainZapReverification(baseNote, entries) RenderOnchainZapGallery(entries, nav, accountViewModel) } } /** - * Subscribes to the shared chain-tip flow and re-runs onchain-zap verification for - * [note] whenever the tip advances, while [entries] still contains non-CONFIRMED - * items. First-view kick happens via the StateFlow's initial null → first-tip - * transition. + * Drives on-chain zap re-verification for [note] while the gallery is composed. + * + * Three triggers fire reverify: + * 1. First view of this note (independent of tip availability — covers the cold- + * start case where the chain backend isn't wired yet, or `tipHeight()` is slow). + * 2. A new pending entry arrives (`entries.size` changes). + * 3. The chain tip advances (the shared StateFlow emits a new value). + * + * The cache's per-note + per-event gates dedupe concurrent calls; this composable + * doesn't need its own throttling. */ @Composable private fun DriveOnchainZapReverification( note: Note, entries: ImmutableList, ) { - val hasPending = remember(entries) { entries.any { it.status != OnchainZapStatus.CONFIRMED } } - if (!hasPending) return + val pendingCount = remember(entries) { entries.count { it.status != OnchainZapStatus.CONFIRMED } } + if (pendingCount == 0) return + // First-view kick — unconditional, doesn't wait for the tip flow. Keyed on + // (idHex, pendingCount) so a brand-new pending entry arriving while the + // gallery is still on screen also kicks an immediate reverify instead of + // waiting up to a full tip-poll interval. + LaunchedEffect(note.idHex, pendingCount) { + LocalCache.reverifyOnchainZapsForNote(note) + } + + // Tip-change kick — subscribes to the shared poller (lazy, WhileSubscribed + // so only one HTTP poller runs across the whole UI no matter how many + // galleries are visible). Skips the first emission (null) to avoid + // duplicating the first-view kick above. val tip by LocalCache.onchainTipHeightFlow.collectAsStateWithLifecycle() - LaunchedEffect(note.idHex, hasPending, tip) { + LaunchedEffect(note.idHex, tip) { if (tip != null) { LocalCache.reverifyOnchainZapsForNote(note) } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index 04d6316657..d250ae7d2a 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -173,15 +173,16 @@ open class Note( private set /** - * Anti-resurrection blocklist for hard-rejected NIP-BC onchain zaps. - * Key: txid. Value: set of source author pubkeys whose attempts for that txid have - * been verified-and-rejected on this note. Used by `LocalCache.consume(OnchainZapEvent)` - * to skip the optimistic UI attachment for known-bad (txid, sender) pairs that would - * otherwise re-flicker into the gallery on every fresh event id. + * True when the NIP-BC chain verifier has reached a terminal verdict for THIS + * note's OnchainZapEvent — i.e. on-chain Confirmed, or hard-rejected for a reason + * other than `TX_NOT_FOUND`. `LocalCache.consume()` uses this to skip re-launching + * the verifier on relay echoes once the chain has spoken definitively. + * + * Stays `false` for transient states (`UNVERIFIED`, `PENDING`, `TX_NOT_FOUND`) so + * the gallery's reverify driver can still upgrade them as the chain advances. */ @Volatile - var rejectedOnchainZapTxidsBySource = mapOf>() - private set + var onchainZapResolved: Boolean = false var zapPayments = mapOf() private set @@ -346,6 +347,7 @@ open class Note( reports = mapOf() zaps = mapOf() onchainZaps = mapOf() + onchainZapResolved = false zapPayments = mapOf() zapsAmount = BigDecimal.ZERO relays = listOf() @@ -452,13 +454,17 @@ open class Note( ): Boolean { val existing = onchainZaps[txid] if (existing != null) { + // Exact structural duplicate (same source Note + same fields) — typical + // relay echo of the same event. Skip the rewrite to avoid spurious + // flowSet invalidation. + if (entry == existing) return false // Reject downgrades using the explicit OnchainZapStatus.level (not ordinal) // so the upgrade contract survives future enum reordering or insertions. if (entry.status.level < existing.status.level) return false - // Same level: accept only when the on-chain verified amount has grown - // (e.g. the indexer revised its view of the tx's recipient outputs). - // Otherwise treat as a duplicate and keep the first source we got. - if (entry.status.level == existing.status.level && entry.verifiedSats <= existing.verifiedSats) return false + // Same level: accept only when verifiedSats grows OR the source differs + // (legitimate alternate signer republishing a split-zap receipt). A strictly + // smaller verifiedSats is a backend downgrade and we ignore it. + if (entry.status.level == existing.status.level && entry.verifiedSats < existing.verifiedSats) return false } onchainZaps = onchainZaps + Pair(txid, entry) return true @@ -467,27 +473,23 @@ open class Note( @Synchronized private fun innerRemoveOnchainZapForSource( txid: String, - sourceAuthorPubKey: HexKey?, + sourceAuthorPubKey: HexKey, ): Boolean { val existing = onchainZaps[txid] ?: return false // Anti-spoof: only remove the entry if its source matches the rejecting event. // Otherwise a malicious third party could erase a legitimate CONFIRMED entry // by publishing a spoofed kind:8333 with the same txid but a bystander recipient. + // Also refuse to remove a CONFIRMED entry — once chain-verified, only a fresh + // CONFIRMED replacement should change it; a transient backend hiccup must not + // wipe a previously-CONFIRMED entry just because some other target on the same + // event is still UNVERIFIED. + if (existing.status == OnchainZapStatus.CONFIRMED) return false + if (existing.source.author?.pubkeyHex == null) return false if (existing.source.author?.pubkeyHex != sourceAuthorPubKey) return false onchainZaps = onchainZaps - txid return true } - @Synchronized - private fun innerRecordOnchainZapRejection( - txid: String, - sourceAuthorPubKey: HexKey, - ) { - val current = rejectedOnchainZapTxidsBySource[txid].orEmpty() - if (sourceAuthorPubKey in current) return - rejectedOnchainZapTxidsBySource = rejectedOnchainZapTxidsBySource + Pair(txid, current + sourceAuthorPubKey) - } - /** * Register a NIP-BC onchain zap targeting this note. `source` is the OnchainZapEvent's own * note — `source.author` is the sender shown in the reactions gallery. @@ -519,33 +521,20 @@ open class Note( * Used when verification produced a hard rejection (e.g. the transaction paid zero to the * recipient — a spoof attempt). The source-scoped match prevents a malicious third party * from erasing a legitimate CONFIRMED entry by publishing a spoofed kind:8333 with the - * same txid but a different recipient pubkey. + * same txid but a different recipient pubkey. Per-target CONFIRMED check also prevents + * a transient backend reject from erasing an already-confirmed entry. */ fun removeOnchainZapForSource( txid: String, - sourceAuthorPubKey: HexKey?, + sourceAuthorPubKey: HexKey, ) { val removed = innerRemoveOnchainZapForSource(txid, sourceAuthorPubKey) - if (sourceAuthorPubKey != null) { - innerRecordOnchainZapRejection(txid, sourceAuthorPubKey) - } if (removed) { updateZapTotal() flowSet?.zaps?.invalidateData() } } - /** - * True if a previous verification rejected an onchain zap for this exact (txid, source) - * pair on this note. `LocalCache.consume(OnchainZapEvent)` uses this to skip the - * optimistic gallery attachment for known-bad senders, preventing attach-then-remove - * flicker when an attacker re-publishes the same spoof under a fresh event id. - */ - fun wasOnchainZapRejectedForSource( - txid: String, - sourceAuthorPubKey: HexKey, - ): Boolean = sourceAuthorPubKey in rejectedOnchainZapTxidsBySource[txid].orEmpty() - @Synchronized private fun innerAddZapPayment( zapPaymentRequest: Note, diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteOnchainZapTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteOnchainZapTest.kt index ca62a12f00..59064d5512 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteOnchainZapTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NoteOnchainZapTest.kt @@ -26,6 +26,8 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNotSame import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue @@ -35,8 +37,8 @@ class NoteOnchainZapTest { // Creates a sender-event Note whose `author.pubkeyHex` is set to [pubKey]. The Note's // own `idHex` is derived from the pubkey so it stays distinct from the target note. - // `wasOnchainZapRejectedForSource` and `removeOnchainZapForSource` both key off - // `source.author?.pubkeyHex`, so the author must be wired even in unit tests. + // `removeOnchainZapForSource` keys off `source.author?.pubkeyHex`, so the author + // must be wired even in unit tests. private fun sourceNote(pubKey: HexKey): Note { val src = Note(pubKey) src.author = User(pubKey, Note(pubKey + "n65"), Note(pubKey + "dm")) @@ -174,13 +176,13 @@ class NoteOnchainZapTest { } @Test - fun sameStatusLowerOrEqualVerifiedSatsIsIgnored() { + fun sameStatusLowerVerifiedSatsIsIgnored() { val target = freshNote() val firstSrc = sourceNote("a7".repeat(32)) val secondSrc = sourceNote("b8".repeat(32)) target.addOnchainZap(firstSrc, "tx1", claimedSats = 999L, verifiedSats = 999L, status = OnchainZapStatus.CONFIRMED) - // Lower verifiedSats: keep the original entry; don't downgrade. + // Strictly-lower verifiedSats: keep the original entry; don't downgrade. target.addOnchainZap(secondSrc, "tx1", claimedSats = 999L, verifiedSats = 500L, status = OnchainZapStatus.CONFIRMED) val entry = target.onchainZaps["tx1"] @@ -189,14 +191,55 @@ class NoteOnchainZapTest { assertEquals(BigDecimal.valueOf(999L), target.zapsAmount) } + @Test + fun sameStatusEqualVerifiedSatsDifferentSourceReplaces() { + // Multi-signer / split-zap-rebroadcast scenario: a second legitimate kind:8333 + // with the same txid and identical verifiedSats arrives from a different signer. + // The new entry should win so attribution isn't permanently locked to whichever + // relay delivered first. + val target = freshNote() + val firstSrc = sourceNote("c9".repeat(32)) + val secondSrc = sourceNote("d0".repeat(32)) + + target.addOnchainZap(firstSrc, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.CONFIRMED) + target.addOnchainZap(secondSrc, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.CONFIRMED) + + val entry = target.onchainZaps["tx1"] + assertSame(secondSrc, entry?.source) + assertEquals(BigDecimal.valueOf(1000L), target.zapsAmount) + } + + @Test + fun structuralDuplicateIsIgnored() { + // Exact structural duplicate: same source Note reference + same fields. This + // is the typical relay-echo case — N relays deliver the same event id, so + // `getOrCreateNote` returns the same Note instance for each. Skip the rewrite + // to avoid spurious flowSet invalidations. + val target = freshNote() + val src = sourceNote("e1".repeat(32)) + + target.addOnchainZap(src, "tx1", claimedSats = 100L, verifiedSats = 100L, status = OnchainZapStatus.CONFIRMED) + val firstEntry = target.onchainZaps["tx1"] + target.addOnchainZap(src, "tx1", claimedSats = 100L, verifiedSats = 100L, status = OnchainZapStatus.CONFIRMED) + val secondEntry = target.onchainZaps["tx1"] + + assertSame(firstEntry, secondEntry) + assertEquals(BigDecimal.valueOf(100L), target.zapsAmount) + } + @Test fun removeOnchainZapForMatchingSourceDropsEntryAndAdjustsTotal() { + // CONFIRMED entries are guarded against removal (see + // `confirmedEntryIsNotRemovableOnTransientReject`), so use a PENDING entry + // here to exercise the matching-source removal path. val target = freshNote() val srcKey = "c9".repeat(32) val src = sourceNote(srcKey) - target.addOnchainZap(src, "tx1", claimedSats = 4200L, verifiedSats = 4200L, status = OnchainZapStatus.CONFIRMED) - assertEquals(BigDecimal.valueOf(4200L), target.zapsAmount) + target.addOnchainZap(src, "tx1", claimedSats = 4200L, verifiedSats = 4200L, status = OnchainZapStatus.PENDING) + // PENDING entries don't contribute to total per spec. + assertEquals(BigDecimal.ZERO, target.zapsAmount) + assertNotNull(target.onchainZaps["tx1"]) target.removeOnchainZapForSource("tx1", srcKey) @@ -207,37 +250,66 @@ class NoteOnchainZapTest { @Test fun removeOnchainZapForMismatchedSourceKeepsLegitimateEntry() { // Regression test for the txid-collision attack: a spoofed kind:8333 with the - // same txid but a different sender pubkey must not erase a legitimate CONFIRMED - // entry. `removeOnchainZapForSource` requires the existing entry's source.author + // same txid but a different sender pubkey must not erase a legitimate entry. + // `removeOnchainZapForSource` requires the existing entry's source.author // to match the rejecting sender. val target = freshNote() val legitSrcKey = "1a".repeat(32) val attackerKey = "2b".repeat(32) val legitSrc = sourceNote(legitSrcKey) - target.addOnchainZap(legitSrc, "tx1", claimedSats = 4200L, verifiedSats = 4200L, status = OnchainZapStatus.CONFIRMED) + target.addOnchainZap(legitSrc, "tx1", claimedSats = 4200L, verifiedSats = 4200L, status = OnchainZapStatus.PENDING) - // Verifier rejects the attacker's spoof of "tx1" — must not touch Alice's entry. target.removeOnchainZapForSource("tx1", attackerKey) assertNotEquals(null, target.onchainZaps["tx1"]) - assertEquals(BigDecimal.valueOf(4200L), target.zapsAmount) - assertTrue(target.wasOnchainZapRejectedForSource("tx1", attackerKey)) - assertFalse(target.wasOnchainZapRejectedForSource("tx1", legitSrcKey)) } @Test - fun removeOnchainZapForUnknownTxidStillRecordsRejection() { - // Even when there's nothing to remove (the optimistic attach was skipped), - // the rejection blocklist must be updated so future fresh-event-id retries - // by the same attacker don't re-flicker into the gallery. + fun confirmedEntryIsNotRemovableOnTransientReject() { + // Once chain-verified, a CONFIRMED entry must not be erased by a later + // verifier reject (e.g. transient ZERO_VERIFIED_AMOUNT from a corrupted + // Esplora response, or a multi-target event where a sibling target hadn't + // confirmed yet). Only an explicit fresh CONFIRMED replacement should + // change a confirmed entry. + val target = freshNote() + val srcKey = "f1".repeat(32) + val src = sourceNote(srcKey) + + target.addOnchainZap(src, "tx1", claimedSats = 4200L, verifiedSats = 4200L, status = OnchainZapStatus.CONFIRMED) + assertEquals(BigDecimal.valueOf(4200L), target.zapsAmount) + + target.removeOnchainZapForSource("tx1", srcKey) + + assertNotNull(target.onchainZaps["tx1"]) + assertEquals(OnchainZapStatus.CONFIRMED, target.onchainZaps["tx1"]?.status) + assertEquals(BigDecimal.valueOf(4200L), target.zapsAmount) + } + + @Test + fun removeOnchainZapForUnknownTxidIsNoOp() { val target = freshNote() val srcKey = "3c".repeat(32) + val src = sourceNote(srcKey) + target.addOnchainZap(src, "tx1", claimedSats = 100L, verifiedSats = 100L, status = OnchainZapStatus.PENDING) target.removeOnchainZapForSource("tx-never-added", srcKey) - assertEquals(0, target.onchainZaps.size) - assertTrue(target.wasOnchainZapRejectedForSource("tx-never-added", srcKey)) + // The known entry survives; the unknown txid op is a no-op. + assertEquals(1, target.onchainZaps.size) + assertNotNull(target.onchainZaps["tx1"]) + } + + @Test + fun onchainZapResolvedFlagDefaultsFalseAndIsMutable() { + // The resolved flag is the dedup gate `LocalCache.consume()` uses to skip + // re-launching the verifier for terminally-resolved events. New notes start + // unresolved; the verifier flips the flag on Confirmed / hard-Rejected. + val src = sourceNote("9d".repeat(32)) + assertFalse(src.onchainZapResolved) + + src.onchainZapResolved = true + assertTrue(src.onchainZapResolved) } @Test @@ -275,4 +347,36 @@ class NoteOnchainZapTest { assertTrue(target.onchainZaps.containsKey("tx3")) assertTrue(target.onchainZaps.containsKey("tx4")) } + + @Test + fun removeAllChildNotesClearsOnchainZapResolvedFlag() { + // `removeAllChildNotes()` runs on delete-event handling and during cache + // pressure; the resolved flag must travel with the cleared state so a + // re-arrival of the same event gets re-verified instead of being silently + // skipped against stale state. + val src = sourceNote("ee".repeat(32)) + src.onchainZapResolved = true + + src.removeAllChildNotes() + + assertFalse(src.onchainZapResolved) + } + + @Test + fun upgradeProducesNewEntryInstance() { + // Sanity check: when an upgrade is accepted, the new immutable map entry is a + // fresh OnchainZapEntry instance (not a mutation of the existing one). Compose + // skipping/recomposition correctness depends on this. + val target = freshNote() + val firstSrc = sourceNote("11".repeat(32)) + val secondSrc = sourceNote("22".repeat(32)) + + target.addOnchainZap(firstSrc, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.PENDING) + val before = target.onchainZaps["tx1"]!! + target.addOnchainZap(secondSrc, "tx1", claimedSats = 1000L, verifiedSats = 1000L, status = OnchainZapStatus.CONFIRMED) + val after = target.onchainZaps["tx1"]!! + + assertNotSame(before, after) + assertEquals(OnchainZapStatus.CONFIRMED, after.status) + } }