refactor(clink): audit follow-ups — consistent error detail, non-null priceType, budget guard

From the audit of this session's changes:

- Error surfacing: the budget (WalletScreen) and offer/invoice card
  (InvoicePaymentDispatcher) paths now use DebitResponse.failureDetail() like the
  zap path, so a GFY code-5/code-4 surfaces its range/retry_after instead of just
  the bare error string.
- NOffer.priceType is now non-null: decode already defaults an absent TLV 3 to
  SPONTANEOUS, so the nullable type was misleading and the '?: SPONTANEOUS'
  fallbacks in ClinkOfferPreview were dead. Drops them and the now-redundant
  always-emit-TLV3 test (covered by the spontaneous round-trip).
- WalletViewModel.requestDebitBudget catches the budget-validation
  IllegalArgumentException so a malformed frequency dismisses the dialog instead
  of hanging the spinner.
- Document why ClinkDebitPayer signs with the persistent account key (stable
  identity for budgets) while ClinkOfferPayer uses an ephemeral key.

https://claude.ai/code/session_01NM2TyJtosLdY5ycjyabSRS
This commit is contained in:
Claude
2026-06-10 18:55:57 +00:00
parent 33f1d20a21
commit 7e7898bf77
8 changed files with 24 additions and 22 deletions
@@ -80,6 +80,9 @@ object ClinkDebitPayer {
return sendAndAwait(account, client, client.requestBudget(amountSats, frequency), timeoutMs)
}
// Debits sign with the persistent account identity (unlike offer requests, which use a
// throwaway key — see ClinkOfferPayer): the service must see one stable app identity so a
// budget authorization can cover repeat debits instead of prompting on every payment.
private fun clientFor(
pointer: NDebit,
account: Account,
@@ -84,7 +84,7 @@ fun ClinkOfferPreview(
var errorMessage by remember { mutableStateOf<String?>(null) }
var payingInvoice by remember { mutableStateOf<String?>(null) }
var amountInput by remember { mutableStateOf("") }
var needsAmount by remember { mutableStateOf((offer.priceType ?: OfferPriceType.SPONTANEOUS) == OfferPriceType.SPONTANEOUS) }
var needsAmount by remember { mutableStateOf(offer.priceType == OfferPriceType.SPONTANEOUS) }
var amountRange by remember { mutableStateOf<SatRange?>(null) }
// The pointer actually paid: starts as the rendered offer, swapped if the service
// replies "Expired or Moved" (code 3) with a replacement noffer.
@@ -147,7 +147,7 @@ fun ClinkOfferPreview(
// when the pointer omits a price type) require the payer to enter an amount.
// Reflect the pointer actually being charged (which may have changed if the
// service redirected us to a replacement noffer via "Expired or Moved").
val effectiveType = activeOffer.priceType ?: OfferPriceType.SPONTANEOUS
val effectiveType = activeOffer.priceType
if (effectiveType == OfferPriceType.FIXED) {
activeOffer.price?.let {
@@ -104,7 +104,7 @@ fun InvoicePaymentDispatcher(
onSuccess()
} else {
onError(
response?.error?.takeIf { it.isNotBlank() }
response?.failureDetail()
?: stringRes(context, R.string.clink_debit_no_response),
)
}
@@ -258,7 +258,7 @@ private fun MultiWalletHomeContent(
if (!walletInfo.canShowBalance) {
{ amount, frequency ->
walletViewModel.requestDebitBudget(walletInfo.walletId, amount, frequency) { response ->
val error = response?.error
val error = response?.failureDetail()
val msg =
when {
response?.isOk() == true -> context.getString(R.string.clink_budget_approved)
@@ -339,7 +339,15 @@ class WalletViewModel : ViewModel() {
val acc = account ?: return
val pointer = _debitWallets.value.firstOrNull { it.id == walletId }?.pointer ?: return
viewModelScope.launch {
onResult(ClinkDebitPayer.requestBudget(acc, pointer, amountSats, frequency))
// A malformed budget (e.g. an out-of-spec frequency unit) makes requestBudget throw;
// treat it as "no response" so the dialog dismisses instead of hanging on a spinner.
val response =
try {
ClinkDebitPayer.requestBudget(acc, pointer, amountSats, frequency)
} catch (_: IllegalArgumentException) {
null
}
onResult(response)
}
}
@@ -39,10 +39,10 @@ data class NOffer(
override val relays: List<NormalizedRelayUrl>,
override val pointer: String?,
/**
* TLV 3 — how the offer is priced. A decoded pointer always reports a concrete type
* ([OfferPriceType.SPONTANEOUS] when the wire field was absent, per the CLINK spec).
* TLV 3 — how the offer is priced. Always a concrete type: when the wire field is
* absent it is [OfferPriceType.SPONTANEOUS], per the CLINK spec.
*/
val priceType: OfferPriceType?,
val priceType: OfferPriceType,
/** TLV 4 — price in sats (display/fixed offers), 4-byte big-endian *unsigned* per the SDK. */
val price: Long?,
) : ClinkPointer {
@@ -55,7 +55,7 @@ data class NOffer(
// Always emit TLV 3, even for spontaneous offers: the reference SDK and
// bridgelet decoders throw on a missing price-type field, so an absent TLV 3
// would make our pointers undecodable by every JS consumer.
addHex(ClinkTlv.PRICE_TYPE, (priceType ?: OfferPriceType.SPONTANEOUS).code.toSingleByteHex())
addHex(ClinkTlv.PRICE_TYPE, priceType.code.toSingleByteHex())
// addInt writes the low 32 bits big-endian; for an unsigned price up to
// 2^32-1 that is the correct 4-byte field even when it overflows a signed Int.
price?.let { addInt(ClinkTlv.PRICE, it.toInt()) }
@@ -28,6 +28,7 @@ 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.pointers.OfferPriceType
import com.vitorpamplona.quartz.experimental.clink.server.ClinkServer
import com.vitorpamplona.quartz.experimental.clink.server.K1Tracker
import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair
@@ -45,7 +46,7 @@ class ClinkClientServerTest {
@Test
fun offerClientExposesPointerRoutingAndResponseFilter() {
val client = OfferClient(NOffer(servicePubKey, listOf(relay), "offer-id", null, null), signer)
val client = OfferClient(NOffer(servicePubKey, listOf(relay), "offer-id", OfferPriceType.SPONTANEOUS, null), signer)
assertEquals(servicePubKey, client.servicePubKey)
assertEquals(listOf(relay), client.relays)
@@ -61,7 +62,7 @@ class ClinkClientServerTest {
kotlinx.coroutines.test.runTest {
val payer = NostrSignerInternal(KeyPair())
val service = NostrSignerInternal(KeyPair())
val offer = NOffer(service.pubKey, listOf(relay), "offer-id", null, null)
val offer = NOffer(service.pubKey, listOf(relay), "offer-id", OfferPriceType.SPONTANEOUS, null)
val client = OfferClient(offer, payer)
// payer asks, service settles and sends a receipt referencing the request
@@ -89,7 +90,7 @@ class ClinkClientServerTest {
fun offerRequestTruncatesDescriptionTo100Chars() =
kotlinx.coroutines.test.runTest {
val service = NostrSignerInternal(KeyPair())
val client = OfferClient(NOffer(service.pubKey, listOf(relay), "o", null, null), signer)
val client = OfferClient(NOffer(service.pubKey, listOf(relay), "o", OfferPriceType.SPONTANEOUS, null), signer)
val event = client.requestInvoice(amountSats = 100, description = "x".repeat(150))
val decrypted = event.decryptRequest(service)
assertEquals(100, decrypted.description?.length)
@@ -43,16 +43,6 @@ class ClinkPointerTest {
assertEquals(offer, ClinkPointerParser.parse(encoded))
}
@Test
fun offerAlwaysEncodesPriceTypeTlv() {
// Even when the model leaves priceType null, encode must emit TLV 3 (the SDK and
// bridgelet decoders reject a noffer without it); it decodes back as SPONTANEOUS.
val offer = NOffer(pubKey, listOf(relay), null, null, null)
val parsed = ClinkPointerParser.parse(offer.encode()) as NOffer
assertEquals(OfferPriceType.SPONTANEOUS, parsed.priceType)
}
@Test
fun offerFixedPriceRoundTrip() {
val offer = NOffer(pubKey, listOf(relay), null, OfferPriceType.FIXED, 21_000)