fix(nwc): only claim a binding for a zap request the provider accepted

Kotlin review found the feature's own soundness property could be false on
the wire.

lnAddressInvoice drops the zap request for a provider that does not advertise
`allowsNostr` — `nostrRequest = if (allowsNostr) nostrRequest else null` — but
assembleInvoice set Payable.zapRequest unconditionally from the request it had
built. So paying a lightning address whose provider ignores `nostr=` still
attached metadata.nostr to the payment, for an invoice whose description_hash
commits to nothing about it. Every claim the feature makes — the KDoc, the
byte-identity test, the wallet-side binding check we asked BrollyZapper to
keep strict — rests on those bytes being what the callback hashed. Here they
were not.

A conformant wallet refuses such a row, so no false attribution was displayed;
what was wrong is that we asserted a binding we could not support, and spent
the 4096-char budget doing it. lnAddressInvoice now reports the request it
actually sent, and only that is carried forward.

The size estimate also counted raw string length for `comment`, which is free
text a user typed. JSON escaping expands it — a quote or backslash to two
characters, a control character to six — so an escaping-heavy comment could
breach the ceiling unnoticed, and NWC-06 makes the wallet drop the WHOLE
object then, taking recipient_data with it. escapedLength() counts what
actually reaches the wire; KEY_OVERHEAD drops to the fixed punctuation cost
now that escaping is no longer hiding inside it.

Both paths were untested and now have regression tests.

Not changed: dropMetadataIfUnsupported still mutates the caller's Request. The
review confirmed every current call site builds a fresh request inline, and
the contract is documented on both public send functions.

Verified: quartz + amethyst suites, commons/desktopApp/cli/geode compile,
spotless clean.

amethyst/src/main/java/com/vitorpamplona/amethyst/service/ZapPaymentHandler.kt

@@ -567,6 +567,10 @@ class ZapPaymentHandler(
     ): Payable {
         var progressThisPayment = 0.00f
 
+        // Only the request the provider actually accepted may be claimed as bound to
+        // this invoice; see lnAddressInvoice's onZapRequestSent.
+        var sentZapRequest: LnZapRequestEvent? = null
+
         val invoice =
             LightningAddressResolver().lnAddressInvoice(
                 lnAddress = lud16,
@@ -580,6 +584,7 @@ class ZapPaymentHandler(
                     onProgressStep(step)
                 },
                 context = context,
+                onZapRequestSent = { sentZapRequest = it },
             )
 
         onProgressStep(1 - progressThisPayment)
@@ -588,7 +593,7 @@ class ZapPaymentHandler(
             info = splitSetup,
             amountMilliSats = zapValue,
             invoice = invoice,
-            zapRequest = nostrZapRequest,
+            zapRequest = sentZapRequest,
             message = message,
         )
     }

amethyst/src/main/java/com/vitorpamplona/amethyst/service/lnurl/LightningAddressResolver.kt

@@ -201,6 +201,13 @@ class LightningAddressResolver {
             ?: response.code.toString()
     }
 
+    /**
+     * @param onZapRequestSent receives the zap request that was ACTUALLY sent to the
+     *   callback, or null when it was not. A provider that does not advertise
+     *   `allowsNostr` never sees [nostrRequest], and its invoice therefore commits to
+     *   nothing about it — so a caller must not go on to claim the two are bound. See
+     *   the drop below.
+     */
     suspend fun lnAddressInvoice(
         lnAddress: String,
         milliSats: Long,
@@ -209,6 +216,7 @@ class LightningAddressResolver {
         okHttpClient: (String) -> OkHttpClient,
         onProgress: (percent: Float) -> Unit,
         context: Context,
+        onZapRequestSent: (LnZapRequestEvent?) -> Unit = {},
     ): String {
         val mapper = jacksonObjectMapper()
 
@@ -264,12 +272,19 @@ class LightningAddressResolver {
             )
         }
 
+        // NIP-57 binds a zap request to its invoice through `description_hash`, and a
+        // provider that ignores `nostr=` mints an invoice that commits to nothing about
+        // it. Report what actually went, so a caller cannot attach the event to a
+        // payment it was never bound to.
+        val sentZapRequest = nostrRequest?.takeIf { allowsNostr }
+        onZapRequestSent(sentZapRequest)
+
         val invoice =
             fetchLightningInvoice(
                 lnCallback = callbackUrl,
                 milliSats = milliSats,
                 message = message,
-                nostrRequest = if (allowsNostr) nostrRequest else null,
+                nostrRequest = sentZapRequest,
                 okHttpClient = okHttpClient,
                 context = context,
             )

quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/rpc/NwcTransactionMetadata.kt

@@ -127,12 +127,32 @@ class NwcTransactionMetadata(
          */
         const val MAX_METADATA_CHARS = 4096
 
-        // 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
+        // The keys and punctuation around the values:
+        // `{"recipient_data":{"identifier":""},"comment":"","nostr":}` is 58 chars.
+        // Escaping is counted separately by [escapedLength], so this is a fixed cost.
+        private const val KEY_OVERHEAD = 64
+
+        /**
+         * The length a string occupies once JSON-escaped.
+         *
+         * `comment` is free text a user typed, so its raw length is not what reaches
+         * the wire: a quote or backslash becomes two characters and a control
+         * character becomes six. Counting the raw length instead would let an
+         * escaping-heavy comment breach [MAX_METADATA_CHARS] unnoticed — and NWC-06
+         * makes the wallet drop the WHOLE object then, losing `recipient_data` too,
+         * which is the degradation this budget exists to protect.
+         *
+         * Over-counts the short forms (`\n` is two characters, not six), which is
+         * the safe direction.
+         */
+        private fun escapedLength(value: String): Int =
+            value.sumOf { char ->
+                when {
+                    char == '"' || char == '\\' -> 2
+                    char < ' ' -> 6
+                    else -> 1
+                }
+            }
 
         /**
          * Assembles NWC-06 `metadata` for an outgoing payment, or null when there is
@@ -175,7 +195,7 @@ class NwcTransactionMetadata(
                 // string that gets embedded, not a reconstruction of it.
                 val raw = zapRequest.toJson()
                 val chars =
-                    recipientIdentifier.orEmpty().length + comment.orEmpty().length +
+                    escapedLength(recipientIdentifier.orEmpty()) + escapedLength(comment.orEmpty()) +
                         raw.length + KEY_OVERHEAD
                 if (chars <= MAX_METADATA_CHARS) {
                     lean["nostr"] = RawJson(raw)

quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/NwcOutgoingMetadataTest.kt

@@ -121,6 +121,40 @@ class NwcOutgoingMetadataTest {
         assertEquals("hi", meta["comment"])
     }
 
+    /**
+     * A provider that does not advertise `allowsNostr` never receives the zap request,
+     * so its invoice commits to nothing about it. Passing null here is how the caller
+     * says so, and the metadata must then make no claim it cannot support — while the
+     * payee's address still labels the row.
+     */
+    @Test
+    fun withoutAZapRequestTheRowIsStillNamedButClaimsNoBinding() {
+        val meta = assertNotNull(NwcTransactionMetadata.build(null, "user@domain.com", "great post"))
+
+        assertFalse(meta.containsKey("nostr"))
+        assertEquals(mapOf("identifier" to "user@domain.com"), meta["recipient_data"])
+        assertEquals("great post", meta["comment"])
+    }
+
+    /**
+     * `comment` is free text, and JSON escaping expands it on the way out. Counting the
+     * raw length would let an escaping-heavy comment breach the 4096 ceiling unnoticed
+     * — and NWC-06 makes the wallet drop the WHOLE object then, taking `recipient_data`
+     * with it.
+     */
+    @Test
+    fun theCeilingCountsEscapedLengthNotRawLength() {
+        // Every character escapes to six, so 900 raw chars occupy ~5400 on the wire.
+        val controlHeavy = "\u0001".repeat(900)
+        val meta = assertNotNull(NwcTransactionMetadata.build(zapRequest(), "user@domain.com", controlHeavy))
+
+        assertFalse(meta.containsKey("nostr"), "the escaped comment alone exceeds the ceiling")
+
+        // The same length in plain characters leaves room for the zap request.
+        val plain = "a".repeat(900)
+        assertTrue(assertNotNull(NwcTransactionMetadata.build(zapRequest(), "user@domain.com", plain)).containsKey("nostr"))
+    }
+
     @Test
     fun whatWeSendStaysUnderTheSpecCeiling() {
         val meta = NwcTransactionMetadata.build(zapRequest(), "user@domain.com", "great post")