refactor(nwc): share anyToJsonElement, and drop a refresh that never refreshed

Cleanup pass over the squashed branch. Net -109 lines. anyToJsonElement was a second copy of a private helper that already existed in ClinkKSerializers, serializing the same Map<String, Any?> shape. Worse than tidiness: RawJson is declared in nip01Core and registered globally for Jackson, but the kotlinx half lived inside one NIP's package, so a RawJson routed through Clink's copy would have been emitted as a quoted, escaped JSON string — exactly the corruption RawJson exists to prevent. One declaration now, beside the other kotlinx serializers at nip01Core level, and Clink picks up the RawJson and Array branches its copy lacked. The getFresh call in fetchTransactions is deleted. Its own KDoc claimed it re-read capabilities "bypassing the info cache's TTL", and getFresh does no such thing: it returns a fresh entry as-is, so the case it was written for — a wallet that added `06` twenty minutes ago — was the one case it could not cover. It also refreshed the SELECTED wallet while zaps read the DEFAULT one. The send path's currentOrFetch already fetches on cold and background-refreshes on stale, so nothing is lost. A real force-refresh would mean a relay request per refresh press, which is a policy decision rather than a cleanup. Three KDoc blocks documented behaviour their function no longer had after the walletInfo refactor: prefersNip44 kept four paragraphs about waiting, and supportsMetadata opened "WAITS ON A COLD CACHE" while doing neither. The rationale now lives once, on the one function that waits, and supportsMetadata is inlined into its only caller. Also deleted a comment claiming a metadata-free method "returns before the info cache is consulted" — both call sites fetch first, so it never did. Smaller: RawJson becomes a data class; the unused metadata parameter comes off PayInvoiceMethod.create(bolt11, amount); TransactionRowLabels drops a derivable flag and a twice-computed fallback; KEY_OVERHEAD's comment now says what its slack is for; the three blank-description tests become one loop; a test that asserted the Kotlin stdlib now calls displayDescription(); and two test comments had lost their backticked literal to a heredoc. Verified: quartz + amethyst suites, commons/desktopApp/cli/geode compile, spotless clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019upqJTtAMNNfxKCDV1xDn3
2026-10-06 11:48:24 +00:00 · 2026-08-30 22:31:29 +02:00
parent 790458f9a0
commit c25517c2d6
13 changed files with 122 additions and 173 deletions
@@ -143,87 +143,54 @@ class NwcSignerState(
     * The negotiated encryption preference for a wallet. NIP-47 says a client
     * "should always prefer nip44 if supported by the wallet service", so a false
     * here has to mean "the wallet does not offer NIP-44" — not "we have not asked
-     * yet".
-     *
-     * That distinction is why this waits. The info cache is per-account and held in
-     * memory only, so it starts empty on every app launch, and reading it without
-     * waiting made the first transaction to each wallet after every launch fall
-     * back to NIP-04 even against a wallet advertising `nip44_v2`. Only a cold
-     * cache waits: a stale entry still says what the wallet advertises and is used
-     * as-is while it refreshes in the background.
-     *
-     * The wait is capped, because this sits in front of a payment the user has
-     * already tapped. Its own fetch is bounded only by the relay accessory's 30s
-     * idle window, and the no-response timer below does not start until this
-     * returns. On expiry we send NIP-04 for this one request rather than hold the
-     * tap; the fetch keeps running in the cache's scope, so the next request gets
-     * the negotiated scheme. Never bound the fetch itself instead — a null from it
-     * is cached as a definitive "no info event" for the whole TTL, which would pin
-     * the wallet to NIP-04 for days.
+     * yet". [walletInfo] is what makes that distinction true.
     */
    private fun prefersNip44(info: NwcInfoEvent?): Boolean = info?.encryptionSchemes()?.any { it.equals("nip44_v2", ignoreCase = true) } == true

    /**
-     * The wallet's advertised capabilities, fetched AT MOST ONCE per send.
+     * The wallet's advertised capabilities: the one place a send waits on them, and
+     * it waits AT MOST ONCE.
     *
-     * Every send asks this event two questions — which encryption to use, and
-     * whether NWC-06 metadata may travel — and each used to fetch it for itself.
-     * That is free on a warm cache and doubles the stall on a cold one, because
-     * [NwcInfoCache] deliberately does not cache a FAILED fetch: when the wallet's
-     * relay is down, both waits run in full. A relay dropping hourly turned a 3s
-     * worst case into 6s, which is what the wallet operator saw as a stall.
+     * WAITING IS THE POINT. The info cache is per-account and in memory only, so it
+     * starts empty on every app launch, and reading it without waiting makes "not
+     * fetched yet" indistinguishable from "not supported". That shipped twice: the
+     * first transaction to each wallet after a launch fell back to NIP-04 against a
+     * wallet advertising `nip44_v2`, and a payment to a wallet that had been
+     * advertising NWC-06 for twenty minutes still went out bare — with nothing, on
+     * either side, reporting an error.
     *
-     * Bounded, and a timeout answers null — the callers treat "don't know" as
-     * NIP-04 and as no-metadata respectively, both of which are safe.
+     * ONCE, because both questions read the same event. Each used to fetch for
+     * itself, which is free on a warm cache and doubles the stall on a cold one:
+     * [NwcInfoCache] deliberately does not cache a FAILED fetch, so with the relay
+     * down both waits ran in full and a 3s worst case became 6s.
+     *
+     * BOUNDED, because this sits in front of a payment the user has already tapped
+     * and the no-response timer does not start until it returns. On expiry the
+     * answer is null — read as NIP-04 and as no-metadata, both of them the safe
+     * direction — while the fetch keeps running in the cache's own scope so the next
+     * request gets the negotiated scheme. Never bound the fetch itself instead: a
+     * null from it is cached as a definitive "no info event" for the whole TTL,
+     * which would pin the wallet to NIP-04 for days.
     */
    private suspend fun walletInfo(uri: Nip47WalletConnect.Nip47URINorm?): NwcInfoEvent? {
        uri ?: return null
        return withTimeoutOrNull(NIP44_NEGOTIATION_WAIT_MS) { infoCache?.currentOrFetch(uri) }
    }

-    /**
-     * Whether this wallet advertises NWC-06 (metadata conventions) and may therefore
-     * be sent a populated `metadata`.
-     *
-     * WAITS ON A COLD CACHE, and that is the whole point. The first version paired
-     * [NwcInfoCache.refreshIfStale] — which only *starts* a fetch — with [current],
-     * read immediately after, so a wallet whose info event had not been fetched yet
-     * answered "no" and the payment went out bare. Interop testing against a live
-     * wallet caught it: a pairing whose wallet had been advertising `06` for twenty
-     * minutes still sent no metadata, and nothing anywhere reported an error.
-     *
-     * This is the same trap [currentOrFetch] was written for on the NIP-44 path —
-     * "not fetched yet" is indistinguishable from "not supported" — so it takes the
-     * same remedy, including the bounded wait so a slow relay cannot stall a
-     * payment. A timeout still reads as NO: the cost of that is a history row
-     * without a recipient, whereas a wrong YES to a wallet that types `metadata`
-     * narrowly costs the user a refused payment.
-     *
-     * [currentOrFetch] also kicks a background refresh for an expired entry, so a
-     * wallet that adds the tag later is picked up without any user action.
-     */
-    private fun supportsMetadata(info: NwcInfoEvent?): Boolean = info?.supportsExtension(ExtensionsTag.METADATA_CONVENTIONS) == true
-
    /**
     * Strips NWC-06 `metadata` from a request bound for a wallet that never said it
-     * understands the field.
+     * understands the field — [MetadataCarrying] has the reason that matters.
     *
-     * APPLIED WHERE THE REQUEST IS BUILT rather than at each call site, because the
-     * cost of forgetting is a refused payment: a wallet that types `metadata`
-     * narrowly accepts a null but cannot decode an object, and answers a params
-     * error instead of paying. Every send path funnels through here, so populating
+     * APPLIED WHERE THE REQUEST IS BUILT rather than at each call site, so populating
     * `metadata` anywhere upstream is safe by construction.
     *
     * MUTATES the request in place — see the callers' KDoc. Requests are built per
-     * send and not reused, and stripping a copy would mean rebuilding a params
-     * object whose field list would then drift from the original.
-     *
-     * A method with no metadata returns before the info cache is consulted, so this
-     * costs nothing on the RPCs that can never carry any.
+     * send and not reused, and stripping a copy would mean rebuilding a params object
+     * whose field list would then drift from the original.
     */
    private fun Request.dropMetadataIfUnsupported(info: NwcInfoEvent?) {
        val carrier = metadataCarrier ?: return
-        if (carrier.metadata == null || supportsMetadata(info)) return
+        if (carrier.metadata == null || info?.supportsExtension(ExtensionsTag.METADATA_CONVENTIONS) == true) return
        carrier.metadata = null
    }

@@ -75,18 +75,14 @@ data class TransactionRowLabels(
                    description == null || !comment.equals(description, ignoreCase = true)
                }

-            // Whether the title names a counterparty rather than describing the payment.
-            // A named row wants a second line saying what the payment was; a row whose
-            // title IS the description must not repeat it underneath.
-            val isNamed = pubkeyHex != null || displayName != null
-
-            val title =
-                pubkeyHex?.let { Title.User(it, displayName) }
-                    ?: Title.Literal(displayName ?: description ?: directionLabel)
+            // A title that NAMES a counterparty wants a second line saying what the
+            // payment was; a title that IS the description must not repeat it below.
+            val named = pubkeyHex?.let { Title.User(it, displayName) } ?: displayName?.let { Title.Literal(it) }
+            val fallback = description ?: directionLabel

            return TransactionRowLabels(
-                title = title,
-                subtitle = comment ?: if (isNamed) description ?: directionLabel else null,
+                title = named ?: Title.Literal(fallback),
+                subtitle = comment ?: fallback.takeIf { named != null },
            )
        }
    }
@@ -550,19 +550,6 @@ class WalletViewModel : ViewModel() {
        val acc = account ?: return
        val walletUri = getWalletUri(walletId) ?: return

-        // Re-read the wallet's advertised capabilities whenever the user opens or
-        // refreshes this screen, bypassing the info cache's TTL.
-        //
-        // A wallet that ADDS an extension is otherwise invisible for the life of the
-        // cached entry: nothing errors, the feature simply does not appear, and the
-        // user has no way to learn that. Interop testing found a pairing sending no
-        // NWC-06 metadata to a wallet that had been advertising `06` for twenty
-        // minutes. This is the deliberate user-visible remedy — its own coroutine, so
-        // a slow info fetch never delays the transaction list.
-        viewModelScope.launch(Dispatchers.IO) {
-            acc.nip47SignerState.infoCache?.getFresh(walletUri)
-        }
-
        viewModelScope.launch(Dispatchers.IO) {
            _isLoading.value = true
            _error.value = null
@@ -37,23 +37,15 @@ class TransactionRowLabelsTest {
     * nothing in it. Outgoing rows looked like a bare arrow and a date.
     */
    @Test
-    fun anEmptyDescriptionFallsBackToTheDirection() {
-        val labels = TransactionRowLabels.resolve(NwcTransaction(type = "outgoing", description = ""), "Sent")
+    fun aDescriptionWithNothingInItFallsBackToTheDirection() {
+        // Empty and whitespace are what wallets actually send for a payment with no
+        // memo; absent is the spec-clean form. All three must reach the fallback.
+        listOf("", "   ", null).forEach { description ->
+            val labels = TransactionRowLabels.resolve(NwcTransaction(type = "outgoing", description = description), "Sent")

-        assertEquals(TransactionRowLabels.Title.Literal("Sent"), labels.title)
-        assertNull(labels.subtitle)
-    }
-
-    @Test
-    fun aWhitespaceDescriptionIsTreatedTheSameWay() {
-        val labels = TransactionRowLabels.resolve(NwcTransaction(type = "outgoing", description = "   "), "Sent")
-        assertEquals(TransactionRowLabels.Title.Literal("Sent"), labels.title)
-    }
-
-    @Test
-    fun anAbsentDescriptionStillFallsBack() {
-        val labels = TransactionRowLabels.resolve(NwcTransaction(type = "outgoing", description = null), "Sent")
-        assertEquals(TransactionRowLabels.Title.Literal("Sent"), labels.title)
+            assertEquals("description=<$description>", TransactionRowLabels.Title.Literal("Sent"), labels.title)
+            assertNull("description=<$description>", labels.subtitle)
+        }
    }

    @Test
@@ -33,6 +33,7 @@ import com.vitorpamplona.quartz.experimental.clink.manage.OfferFields
 import com.vitorpamplona.quartz.experimental.clink.offers.OfferReceipt
 import com.vitorpamplona.quartz.experimental.clink.offers.OfferRequest
 import com.vitorpamplona.quartz.experimental.clink.offers.OfferResponse
+import com.vitorpamplona.quartz.nip01Core.kotlinSerialization.anyToJsonElement
 import com.vitorpamplona.quartz.nip47WalletConnect.kotlinSerialization.toAnyMap
 import kotlinx.serialization.KSerializer
 import kotlinx.serialization.descriptors.SerialDescriptor
@@ -45,7 +46,6 @@ import kotlinx.serialization.json.JsonElement
 import kotlinx.serialization.json.JsonEncoder
 import kotlinx.serialization.json.JsonNull
 import kotlinx.serialization.json.JsonObject
-import kotlinx.serialization.json.JsonPrimitive
 import kotlinx.serialization.json.add
 import kotlinx.serialization.json.buildJsonArray
 import kotlinx.serialization.json.buildJsonObject
@@ -63,18 +63,6 @@ import kotlinx.serialization.json.put
 * (Jackson's `ACCEPT_SINGLE_VALUE_AS_ARRAY`) — so native targets parse the same wire shapes.
 */

-private fun anyToJsonElement(value: Any?): JsonElement =
-    when (value) {
-        null -> JsonNull
-        is JsonElement -> value
-        is String -> JsonPrimitive(value)
-        is Boolean -> JsonPrimitive(value)
-        is Number -> JsonPrimitive(value)
-        is Map<*, *> -> buildJsonObject { value.forEach { (k, v) -> put(k.toString(), anyToJsonElement(v)) } }
-        is Iterable<*> -> buildJsonArray { value.forEach { add(anyToJsonElement(it)) } }
-        else -> JsonPrimitive(value.toString())
-    }
-
 private fun JsonObject.stringOrNull(key: String): String? = get(key)?.let { if (it is JsonNull) null else it.jsonPrimitive.content }

 private fun JsonObject.longOrNull(key: String): Long? = get(key)?.let { if (it is JsonNull) null else it.jsonPrimitive.longOrNull }
@@ -35,12 +35,8 @@ package com.vitorpamplona.quartz.nip01Core.core
 * `JsonUnquotedLiteral`. [json] MUST already be well-formed JSON; nothing
 * validates it, and an invalid value corrupts the whole document.
 */
-class RawJson(
+data class RawJson(
    val json: String,
 ) {
    override fun toString() = json
-
-    override fun equals(other: Any?) = other is RawJson && other.json == json
-
-    override fun hashCode() = json.hashCode()
 }
@@ -0,0 +1,58 @@
+/*
+ * 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.nip01Core.kotlinSerialization
+
+import com.vitorpamplona.quartz.nip01Core.core.RawJson
+import kotlinx.serialization.json.JsonElement
+import kotlinx.serialization.json.JsonNull
+import kotlinx.serialization.json.JsonPrimitive
+import kotlinx.serialization.json.JsonUnquotedLiteral
+import kotlinx.serialization.json.add
+import kotlinx.serialization.json.buildJsonArray
+import kotlinx.serialization.json.buildJsonObject
+import kotlinx.serialization.json.put
+
+/**
+ * Encodes an untyped `Any?` tree — the shape several Nostr RPCs carry as a free-form
+ * `Map<String, Any?>` — into a [JsonElement].
+ *
+ * `Json.encodeToJsonElement` CANNOT do this: it resolves a serializer from the STATIC
+ * type, and `Any` has none, so it throws `SerializationException: Serializer for class
+ * 'Any' is not found` at runtime for every populated map. This walks the value instead.
+ *
+ * DECLARED AT nip01Core LEVEL, beside the other kotlinx serializers, because [RawJson]
+ * is: it is registered globally on the Jackson side, so a per-NIP copy of this function
+ * would leave the kotlinx backend supporting raw JSON in some packages and silently
+ * quoting it into a string in others — the exact corruption [RawJson] exists to prevent.
+ */
+fun anyToJsonElement(value: Any?): JsonElement =
+    when (value) {
+        null -> JsonNull
+        is RawJson -> JsonUnquotedLiteral(value.json)
+        is JsonElement -> value
+        is String -> JsonPrimitive(value)
+        is Boolean -> JsonPrimitive(value)
+        is Number -> JsonPrimitive(value)
+        is Map<*, *> -> buildJsonObject { value.forEach { (k, v) -> put(k.toString(), anyToJsonElement(v)) } }
+        is Iterable<*> -> buildJsonArray { value.forEach { add(anyToJsonElement(it)) } }
+        is Array<*> -> buildJsonArray { value.forEach { add(anyToJsonElement(it)) } }
+        else -> JsonPrimitive(value.toString())
+    }
@@ -20,17 +20,10 @@
 */
 package com.vitorpamplona.quartz.nip47WalletConnect.kotlinSerialization

-import com.vitorpamplona.quartz.nip01Core.core.RawJson
 import kotlinx.serialization.json.JsonArray
 import kotlinx.serialization.json.JsonElement
-import kotlinx.serialization.json.JsonNull
 import kotlinx.serialization.json.JsonObject
 import kotlinx.serialization.json.JsonPrimitive
-import kotlinx.serialization.json.JsonUnquotedLiteral
-import kotlinx.serialization.json.add
-import kotlinx.serialization.json.buildJsonArray
-import kotlinx.serialization.json.buildJsonObject
-import kotlinx.serialization.json.put

 // Helper function to convert JsonElement to standard Kotlin types recursively
 fun JsonElement.toAnyValue(): Any =
@@ -53,30 +46,3 @@ fun JsonElement.toAnyValue(): Any =
    }

 fun JsonObject.toAnyMap(): Map<String, Any?> = entries.associate { it.key to it.value.toAnyValue() }
-
-/**
- * The inverse of [toAnyValue], for the `Map<String, Any?>` blobs NIP-47 carries as
- * `metadata`.
- *
- * `Json.encodeToJsonElement` CANNOT do this — it needs a serializer for the static
- * type, and `Any` has none, so it throws `SerializerException: Serializer for class
- * 'Any' is not found` at runtime for every populated metadata object. This walks
- * the value instead.
- *
- * [RawJson] becomes a `JsonUnquotedLiteral` so pre-serialized JSON reaches the wire
- * byte-for-byte; that is what lets a zap request still hash to the invoice's
- * `description_hash` after a round trip through this map.
- */
-fun anyToJsonElement(value: Any?): JsonElement =
-    when (value) {
-        null -> JsonNull
-        is RawJson -> JsonUnquotedLiteral(value.json)
-        is JsonElement -> value
-        is String -> JsonPrimitive(value)
-        is Boolean -> JsonPrimitive(value)
-        is Number -> JsonPrimitive(value)
-        is Map<*, *> -> buildJsonObject { value.forEach { (k, v) -> put(k.toString(), anyToJsonElement(v)) } }
-        is Iterable<*> -> buildJsonArray { value.forEach { add(anyToJsonElement(it)) } }
-        is Array<*> -> buildJsonArray { value.forEach { add(anyToJsonElement(it)) } }
-        else -> JsonPrimitive(value.toString())
-    }
@@ -20,6 +20,7 @@
 */
 package com.vitorpamplona.quartz.nip47WalletConnect.kotlinSerialization

+import com.vitorpamplona.quartz.nip01Core.kotlinSerialization.anyToJsonElement
 import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CancelHoldInvoiceMethod
 import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CancelHoldInvoiceParams
 import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CreateConnectionMethod
@@ -20,6 +20,7 @@
 */
 package com.vitorpamplona.quartz.nip47WalletConnect.kotlinSerialization

+import com.vitorpamplona.quartz.nip01Core.kotlinSerialization.anyToJsonElement
 import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CancelHoldInvoiceSuccessResponse
 import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CreateConnectionSuccessResponse
 import com.vitorpamplona.quartz.nip47WalletConnect.rpc.GetBalanceSuccessResponse
@@ -127,9 +127,11 @@ class NwcTransactionMetadata(
         */
        const val MAX_METADATA_CHARS = 4096

-        // The keys and punctuation around the values once serialized —
-        // `{"recipient_data":{"identifier":""},"comment":"","nostr":}` is ~56 chars.
-        // Only this wrapper is estimated now; every value's length is exact.
+        // The keys and punctuation around the values —
+        // `{"recipient_data":{"identifier":""},"comment":"","nostr":}` is 58 chars —
+        // plus room for JSON escaping to expand `identifier` and `comment` on the way
+        // out, which is the only thing that can make this estimate low. `nostr` needs
+        // no such allowance: the length added below is the serialized string itself.
        private const val KEY_OVERHEAD = 96

        /**
@@ -40,16 +40,15 @@ sealed class Request(
    var method: String? = null,
 ) : OptimizedSerializable {
    /**
-     * This request's NWC-06 metadata, or null for a method that has none.
+     * This request's NWC-06 metadata carrier, or null for a method that has none.
     *
-     * Declared HERE, and overridden beside each method that carries one, so that
-     * adding a metadata-bearing method is a decision made where the method is
-     * written. The alternative — a `when` over request types in the client — needs
-     * an `else`, and an `else` silently leaks the field to a wallet that never
-     * opted in.
+     * Declared HERE, and overridden beside each method that carries one, so adding a
+     * metadata-bearing method is a decision made where the method is written. The
+     * alternative — a `when` over request types in the client — needs an `else`, and
+     * an `else` silently leaks the field to a wallet that never opted in.
     *
-     * create_connection is deliberately absent: its `metadata` names the
-     * connection and is not NWC-06's per-payment blob.
+     * create_connection is deliberately absent: its `metadata` names the connection
+     * and is not NWC-06's per-payment blob.
     */
    open val metadataCarrier: MetadataCarrying? get() = null
 }
@@ -67,10 +66,7 @@ class PayInvoiceMethod(
    override val metadataCarrier get() = params

    companion object {
-        // `metadata` is NIP-47's optional per-payment blob, whose keys NWC-06 defines.
-        // Only ever populate it for a wallet that advertises NWC-06 (`06` in the info
-        // event's `extensions` tag): a wallet is free to type the field narrowly, and
-        // one that cannot decode an object it never asked for refuses the payment.
+        // `metadata` may only travel to a wallet that advertised it — see [MetadataCarrying].
        fun create(
            bolt11: String,
            metadata: Map<String, Any?>? = null,
@@ -79,8 +75,7 @@ class PayInvoiceMethod(
        fun create(
            bolt11: String,
            amount: Long,
-            metadata: Map<String, Any?>? = null,
-        ): PayInvoiceMethod = PayInvoiceMethod(PayInvoiceParams(bolt11, amount, metadata))
+        ): PayInvoiceMethod = PayInvoiceMethod(PayInvoiceParams(bolt11, amount))
    }
 }

@@ -195,7 +195,7 @@ class NwcOutgoingMetadataTest {
        // The p tag, not the pubkey: on an outgoing zap the pubkey is US.
        assertEquals(recipientHex, parsed.recipientPubkeyHex())
        assertEquals("user@domain.com", parsed.recipientIdentifier())
-        // A wallet storing only  still yields the message.
+        // A wallet storing only `nostr` still yields the message.
        assertEquals("for the article", parsed.displayComment())
    }

@@ -217,10 +217,10 @@ class NwcOutgoingMetadataTest {

    @Test
    fun emptyDescriptionIsNotAName() {
-        // The wallet-side habit this exists for:  rather than an
-        // omitted field. The row must fall back, not render an empty line.
+        // The wallet-side habit this exists for: an empty `description` string
+        // rather than an omitted field. The row must fall back, not render an empty line.
        val tx = NwcTransaction(type = "outgoing", description = "", amount = 21000L)
-        assertEquals("", tx.description)
-        assertNull(tx.description?.ifBlank { null })
+        assertEquals("", tx.description, "the raw field keeps what the wallet sent")
+        assertNull(tx.displayDescription(), "but nothing downstream sees an empty name")
    }
 }