From bbd43e34e98c93f31beca61eb7a501c9ece0aa9f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 25 May 2026 22:57:44 +0000 Subject: [PATCH] fix(cashu): close publish-bridge race + lock in exception types via tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the `var publishDelegate` set-after-construction pattern with an explicit `CashuWalletState.start(publish: suspend (Event) -> Unit)`. Account now calls `cashuWalletState.start { event -> sendLiterallyEverywhere(event) }` from its own init { } block, AFTER all field initializers complete. Why this matters: the previous code launched the backfill + cache-live collectors from inside the state's own init { } block. Those collectors could (and would, for returning users) fire an auto-redeem during Account's field-initializer phase — at which point `publishDelegate` was still the no-op default AND `followPlusAllMineWithIndex` (which sendLiterallyEverywhere depends on) wasn't initialized yet. The publish would silently swallow or NPE. Gating all of start()'s work behind a @Volatile started flag eliminates the window. Also: `MintExceptionTest` (+4 tests) pins down the runtime-exception contract of `MintHttpException` and the new `MintProtocolException` — the latter is what callers branch on when distinguishing "mint refused" from "HTTP failed". Kept simple so any future refactor that breaks the hierarchy fails loudly here instead of silently in describeMintError. 24/24 NIP-60 jvm tests passing. https://claude.ai/code/session_01MdWddiar819f8XYt5N8BjP --- .../vitorpamplona/amethyst/model/Account.kt | 11 ++-- .../model/nip60Cashu/CashuWalletState.kt | 28 ++++++-- .../nip60Cashu/mintApi/MintExceptionTest.kt | 66 +++++++++++++++++++ 3 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip60Cashu/mintApi/MintExceptionTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index 376eb247b8..10aec0aa2f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -3401,10 +3401,13 @@ class Account( init { Log.d("AccountRegisterObservers", "Init") - // Bridge CashuWalletOps's publish callback to our `sendLiterallyEverywhere` - // so the state object can push events to relays + cache without holding - // a direct reference back to Account. - cashuWalletState.publishDelegate = { event -> sendLiterallyEverywhere(event) } + // Start the Cashu wallet state observers AFTER all field initializers + // complete — auto-redeem can fire as soon as start() returns, and it + // calls back into sendLiterallyEverywhere which depends on + // followPlusAllMineWithIndex (initialized after cashuWalletState). + // Doing this in start() rather than in the state's own init { } closes + // the race where a publish would land on a half-built Account. + cashuWalletState.start { event -> sendLiterallyEverywhere(event) } // Restore Marmot MLS group state on startup if (marmotManager != null) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip60Cashu/CashuWalletState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip60Cashu/CashuWalletState.kt index c975cb54e2..3dae04c837 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip60Cashu/CashuWalletState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip60Cashu/CashuWalletState.kt @@ -152,7 +152,23 @@ class CashuWalletState( private val jobs = mutableListOf() private var currentSubscription: CashuWalletQueryState? = null - init { + @Volatile private var started = false + + /** + * Wire the publish bridge and begin observing the cache + relay flows. + * + * Account must call this from its own `init { }` block (after all field + * initializers complete) — that guarantees `sendLiterallyEverywhere` and + * its dependencies (`followPlusAllMineWithIndex`, etc.) are fully + * constructed before the first auto-redeem might fire. Calling start() + * inside the state's own `init { }` would race: the collectors could + * publish via a half-built Account. + */ + fun start(publish: suspend (Event) -> Unit) { + if (started) return + started = true + this.publish = publish + // Backfill from cache once. scope.launch(Dispatchers.Default) { val initial = scanCacheForOwnEvents() @@ -427,13 +443,13 @@ class CashuWalletState( // ============================================================ /** - * Bridge for [CashuWalletOps.publish]. Concrete `Account` plugs in its - * `sendLiterallyEverywhere` via the constructor-time wiring. We keep this - * delegate field separate to avoid an Account ↔ State direct dependency. + * Set exactly once in [start]; before that, every coroutine that could + * call [publishEvent] is gated behind `started` so the no-op default is + * never observed by produced events. */ - var publishDelegate: suspend (Event) -> Unit = { /* set by Account */ } + private var publish: suspend (Event) -> Unit = { error("CashuWalletState.start() not called") } private suspend fun publishEvent(event: Event) { - publishDelegate(event) + publish(event) } } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip60Cashu/mintApi/MintExceptionTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip60Cashu/mintApi/MintExceptionTest.kt new file mode 100644 index 0000000000..a2d2101588 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip60Cashu/mintApi/MintExceptionTest.kt @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip60Cashu.mintApi + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class MintExceptionTest { + @Test + fun httpExceptionPreservesDetail() { + val e = + MintHttpException( + httpStatus = 400, + detail = "amount too small", + code = 11000, + message = "amount too small", + ) + assertEquals(400, e.httpStatus) + assertEquals("amount too small", e.detail) + assertEquals(11000, e.code) + assertEquals("amount too small", e.message) + } + + @Test + fun httpExceptionAllowsNullDetail() { + val e = MintHttpException(httpStatus = 500, detail = null, code = null, message = "HTTP 500") + assertNull(e.detail) + assertNull(e.code) + assertEquals("HTTP 500", e.message) + } + + @Test + fun protocolExceptionCarriesMessage() { + val e = MintProtocolException("Melt not completed (state=UNPAID)") + assertEquals("Melt not completed (state=UNPAID)", e.message) + } + + @Test + fun bothAreRuntimeExceptions() { + // describeMintError lives in amethyst-layer, but at the quartz level we + // can at least confirm both exceptions are runtime — callers don't need + // to declare them. + assertTrue(MintHttpException(200, null, null, "m") is RuntimeException) + assertTrue(MintProtocolException("m") is RuntimeException) + } +}