fix(clink): spec-conformance hardening (k1 length, frequency, description, GFY detail)

Follow-ups from the line-by-line spec audit, scoped to the consume-only client:

- NDebit.parse rejects a TLV-3 session id that isn't exactly 32 bytes (64 hex),
  per clink-debits: a wrong-length k1 is a malformed session pointer.
- DebitClient.requestBudget validates frequency.unit is one of day/week/month
  (DebitFrequency.VALID_UNITS) instead of sending a unit a node service will GFY.
- OfferClient caps the invoice description at 100 chars per clink-offers.
- DebitResponse.failureDetail() composes the GFY error with its actionable extra
  (allowed range for code 5, retry_after for code 4); the debit zap path now
  surfaces that instead of the bare error string.

Adds regression tests for each (malformed-k1 rejection, invalid-unit throw,
description truncation, failureDetail range/retry_after).

https://claude.ai/code/session_01NM2TyJtosLdY5ycjyabSRS
This commit is contained in:
Claude
2026-06-10 16:44:00 +00:00
parent c78ba27c86
commit f4e0bcf73d
8 changed files with 81 additions and 2 deletions
@@ -416,7 +416,7 @@ class ZapPaymentHandler(
if (!paid) {
onError(
stringRes(context, R.string.error_dialog_pay_invoice_error),
response?.error?.takeIf { it.isNotBlank() }
response?.failureDetail()
?: stringRes(context, R.string.clink_debit_no_response),
payable.info.user,
)
@@ -69,6 +69,11 @@ class DebitClient(
description: String? = null,
createdAt: Long = TimeUtils.now(),
): DebitEvent {
// Per spec the recurring cadence unit is one of day/week/month; reject anything else
// rather than sending a request a node service will GFY.
require(frequency == null || frequency.unit in DebitFrequency.VALID_UNITS) {
"Invalid budget frequency unit: ${frequency?.unit}"
}
val request =
DebitRequest(
pointer = pointer.pointer,
@@ -71,7 +71,9 @@ class OfferClient(
payer_data = payerData,
zap = zap,
expires_in_seconds = expiresInSeconds,
description = description,
// The spec caps the invoice description at 100 chars; trim so an over-long
// value doesn't get the whole request rejected by the service.
description = description?.take(100),
)
return OfferEvent.createRequest(request, servicePubKey, signer, createdAt)
}
@@ -47,6 +47,9 @@ class DebitFrequency(
const val UNIT_DAY = "day"
const val UNIT_WEEK = "week"
const val UNIT_MONTH = "month"
/** The only cadence units the spec defines. */
val VALID_UNITS = setOf(UNIT_DAY, UNIT_WEEK, UNIT_MONTH)
}
}
@@ -67,6 +70,25 @@ class DebitResponse(
) : OptimizedSerializable {
fun isOk(): Boolean = res == OK
/**
* A failure detail string for a GFY response: the service's [error] plus the actionable
* structured extra the spec attaches per code — the allowed [range] (code 5) or
* [retry_after] timestamp (code 4). Returns null for a success ([isOk]) response. The
* text is protocol-neutral (English, like the service's own `error`), meant to be shown
* as the detail line under a localized "payment failed" title.
*/
fun failureDetail(): String? {
if (isOk()) return null
val base = error?.takeIf { it.isNotBlank() }
val extra =
when {
range != null -> "allowed ${range?.min ?: "?"} to ${range?.max ?: "?"} sats"
retry_after != null -> "retry after $retry_after"
else -> null
}
return listOfNotNull(base, extra?.let { "($it)" }).joinToString(" ").ifBlank { null }
}
companion object {
const val OK = "ok"
const val GFY = "GFY"
@@ -71,6 +71,10 @@ data class NDebit(
val relays = tlv.asString(ClinkTlv.RELAY)?.mapNotNull { RelayUrlNormalizer.normalizeOrNull(it) } ?: emptyList()
val pointer = tlv.firstAsString(ClinkTlv.POINTER)
val k1 = tlv.firstAsHex(ClinkTlv.K1)
// A session id (TLV 3) MUST be exactly 32 bytes (64 hex chars) per the spec; a
// wrong-length value means a malformed session pointer, so reject the whole thing
// rather than silently treating it as static or sending a bad k1.
if (k1 != null && k1.length != 64) return null
return NDebit(pubKey, relays, pointer, k1)
}
@@ -20,10 +20,13 @@
*/
package com.vitorpamplona.quartz.experimental.clink
import com.vitorpamplona.quartz.experimental.clink.client.DebitClient
import com.vitorpamplona.quartz.experimental.clink.client.OfferClient
import com.vitorpamplona.quartz.experimental.clink.debits.DebitEvent
import com.vitorpamplona.quartz.experimental.clink.debits.DebitFrequency
import com.vitorpamplona.quartz.experimental.clink.offers.OfferEvent
import com.vitorpamplona.quartz.experimental.clink.offers.OfferReceipt
import com.vitorpamplona.quartz.experimental.clink.pointers.NDebit
import com.vitorpamplona.quartz.experimental.clink.pointers.NOffer
import com.vitorpamplona.quartz.experimental.clink.server.ClinkServer
import com.vitorpamplona.quartz.experimental.clink.server.K1Tracker
@@ -71,6 +74,27 @@ class ClinkClientServerTest {
assertEquals("ab".repeat(32), receipt.preimage)
}
@Test
fun requestBudgetRejectsInvalidFrequencyUnit() =
kotlinx.coroutines.test.runTest {
val client = DebitClient(NDebit(servicePubKey, listOf(relay), null, null), signer)
kotlin.test.assertFailsWith<IllegalArgumentException> {
client.requestBudget(1000, DebitFrequency(1, "fortnight"))
}
// valid units do not throw
client.requestBudget(1000, DebitFrequency(1, DebitFrequency.UNIT_MONTH))
}
@Test
fun offerRequestTruncatesDescriptionTo100Chars() =
kotlinx.coroutines.test.runTest {
val service = NostrSignerInternal(KeyPair())
val client = OfferClient(NOffer(service.pubKey, listOf(relay), "o", null, null), signer)
val event = client.requestInvoice(amountSats = 100, description = "x".repeat(150))
val decrypted = event.decryptRequest(service)
assertEquals(100, decrypted.description?.length)
}
@Test
fun serverRequestFilterTargetsRecipientByPTag() {
val filter = ClinkServer.debitRequestFilter(servicePubKey, since = 100L)
@@ -197,6 +197,19 @@ class ClinkWireShapeTest {
assertEquals(500000L, res.range?.max)
}
@Test
fun debitFailureDetailSurfacesRangeAndRetryAfter() {
val range = parse<DebitResponse>("""{"res":"GFY","code":5,"error":"Invalid Amount","range":{"min":1000,"max":500000}}""").failureDetail()!!
assertTrue(range.contains("Invalid Amount"))
assertTrue(range.contains("1000") && range.contains("500000"))
val rateLimited = parse<DebitResponse>("""{"res":"GFY","code":4,"error":"Rate Limited","retry_after":1750000000}""").failureDetail()!!
assertTrue(rateLimited.contains("Rate Limited"))
assertTrue(rateLimited.contains("1750000000"))
assertNull(parse<DebitResponse>("""{"res":"ok"}""").failureDetail())
}
@Test
fun debitGfyInvalidRequest() {
val res = parse<DebitResponse>("""{"res":"GFY","code":6,"error":"Invalid Request: K1 already processed"}""")
@@ -95,6 +95,15 @@ class ClinkPointerTest {
assertEquals(debit, parsed)
}
@Test
fun debitRejectsMalformedK1Length() {
// A session id (TLV 3) must be exactly 32 bytes; a wrong-length k1 is a malformed
// session pointer and the parser must reject it rather than accept a bad session.
val shortK1 = "abcd"
val encoded = NDebit(pubKey, listOf(relay), null, shortK1).encode()
assertNull(ClinkPointerParser.parse(encoded))
}
@Test
fun manageRoundTrip() {
val manage = NManage(pubKey, listOf(relay), null)