From 0030432e20e2ecd1a532651397e150fb756b2c7f Mon Sep 17 00:00:00 2001 From: davotoula Date: Mon, 31 Aug 2026 07:35:44 +0200 Subject: [PATCH] fix(nwc): omit absent request params instead of sending them as null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Viewing transactions on one NWC wallet failed with Invalid list_transactions params: from must be an integer because Amethyst sent every optional parameter explicitly: {"method":"list_transactions","params":{"from":null,"until":null,"limit":20, "offset":0,"unpaid":false,"unpaid_outgoing":null,"unpaid_incoming":null,"type":null}} NIP-47 marks those optional, and a wallet is free to type `from` as an integer and refuse a null. Nothing in the request was wrong except the nulls. The two serialization backends had disagreed since they were written. Nip47RequestKSerializer builds every params object with `params.x?.let { put("x", it) }`, so kotlinx has always omitted nulls; Jackson serializes the params classes reflectively and wrote them. The same request was two different documents depending on the platform, and only JVM/Android was broken — which is why it survived: the tests that cover this shape run against the backend that was already correct. A Jackson mixin now applies NON_NULL to all twelve NIP-47 params classes. A mixin rather than an annotation because the classes live in commonMain and Jackson annotations are JVM-only. The regression test asserts the property rather than the symptom: no request type may emit a null param, and both backends must produce the same document. The second is the one that would have caught this. Not new to any recent change — the reflective serialization predates it. What changed is that 24a8540ad9 surfaces a NIP-47 refusal instead of rendering it as an empty list, so users now see the error rather than an empty transaction screen. Older builds sent the same request and were refused just as silently. --- .../quartz/nip01Core/jackson/JacksonMapper.kt | 28 +++++ .../jackson/OmitNullParamsMixin.kt | 45 +++++++ .../Nip47NullParamOmissionTest.kt | 115 ++++++++++++++++++ 3 files changed, 188 insertions(+) create mode 100644 quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/jackson/OmitNullParamsMixin.kt create mode 100644 quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/Nip47NullParamOmissionTest.kt diff --git a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt index d926ad77a4..37daf0aa6d 100644 --- a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt +++ b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt @@ -55,13 +55,26 @@ import com.vitorpamplona.quartz.nip46RemoteSigner.jackson.BunkerResponseDeserial import com.vitorpamplona.quartz.nip46RemoteSigner.jackson.BunkerResponseSerializer import com.vitorpamplona.quartz.nip47WalletConnect.jackson.NotificationDeserializer import com.vitorpamplona.quartz.nip47WalletConnect.jackson.NotificationSerializer +import com.vitorpamplona.quartz.nip47WalletConnect.jackson.OmitNullParamsMixin import com.vitorpamplona.quartz.nip47WalletConnect.jackson.RequestDeserializer import com.vitorpamplona.quartz.nip47WalletConnect.jackson.RequestSerializer import com.vitorpamplona.quartz.nip47WalletConnect.jackson.ResponseDeserializer import com.vitorpamplona.quartz.nip47WalletConnect.jackson.ResponseSerializer +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CancelHoldInvoiceParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CreateConnectionParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.ListTransactionsParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.LookupInvoiceParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.MakeHoldInvoiceParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.MakeInvoiceParams import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Notification +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayKeysendParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.ReceiveParams import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Request import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Response +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.SettleHoldInvoiceParams +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.SignMessageParams import com.vitorpamplona.quartz.nip59Giftwrap.rumors.Rumor import com.vitorpamplona.quartz.nip59Giftwrap.rumors.jackson.RumorDeserializer import com.vitorpamplona.quartz.nip59Giftwrap.rumors.jackson.RumorSerializer @@ -108,6 +121,21 @@ class JacksonMapper { .addDeserializer(Request::class.java, RequestDeserializer()) .addSerializer(Notification::class.java, NotificationSerializer()) .addDeserializer(Notification::class.java, NotificationDeserializer()) + // NIP-47's optional params are OMITTED when null, never written as + // `null` — see OmitNullParamsMixin. Matches what the kotlinx backend + // has always done, and what a strictly-typed wallet accepts. + .setMixInAnnotation(PayInvoiceParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(PayParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(ReceiveParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(PayKeysendParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(MakeInvoiceParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(LookupInvoiceParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(ListTransactionsParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(MakeHoldInvoiceParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(CancelHoldInvoiceParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(SettleHoldInvoiceParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(SignMessageParams::class.java, OmitNullParamsMixin::class.java) + .setMixInAnnotation(CreateConnectionParams::class.java, OmitNullParamsMixin::class.java) // nip 46 .addDeserializer(BunkerMessage::class.java, BunkerMessageDeserializer()) .addSerializer(BunkerRequest::class.java, BunkerRequestSerializer()) diff --git a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/jackson/OmitNullParamsMixin.kt b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/jackson/OmitNullParamsMixin.kt new file mode 100644 index 0000000000..786835d3c5 --- /dev/null +++ b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/jackson/OmitNullParamsMixin.kt @@ -0,0 +1,45 @@ +/* + * 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.nip47WalletConnect.jackson + +import com.fasterxml.jackson.annotation.JsonInclude + +/** + * Applied to every NIP-47 request `params` class so Jackson OMITS a null field + * instead of writing it. + * + * NIP-47 marks these parameters optional, and a wallet is free to type one + * strictly: sending `"from": null` for an absent `from` earns + * `Invalid list_transactions params: from must be an integer` from a wallet that + * expects an integer or nothing, and the request fails. + * + * THE TWO BACKENDS DISAGREED, which is the reason this is a mixin rather than a + * fix at one call site. [com.vitorpamplona.quartz.nip47WalletConnect.kotlinSerialization.Nip47RequestKSerializer] + * builds every params object with `params.x?.let { put("x", it) }`, so the + * kotlinx (native) backend has always omitted nulls; Jackson serializes the + * params classes reflectively and wrote them. The same request was two different + * documents depending on the platform. + * + * A MIXIN rather than an annotation on the class, because the params classes live + * in `commonMain` and Jackson annotations are JVM-only. + */ +@JsonInclude(JsonInclude.Include.NON_NULL) +abstract class OmitNullParamsMixin diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/Nip47NullParamOmissionTest.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/Nip47NullParamOmissionTest.kt new file mode 100644 index 0000000000..70e6c273ae --- /dev/null +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip47WalletConnect/Nip47NullParamOmissionTest.kt @@ -0,0 +1,115 @@ +/* + * 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.nip47WalletConnect + +import com.vitorpamplona.quartz.nip01Core.core.OptimizedJsonMapper +import com.vitorpamplona.quartz.nip47WalletConnect.kotlinSerialization.Nip47RequestKSerializer +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.CreateConnectionMethod +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.GetBalanceMethod +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.ListTransactionsMethod +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.LookupInvoiceMethod +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.MakeInvoiceMethod +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayInvoiceMethod +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.PayKeysendMethod +import com.vitorpamplona.quartz.nip47WalletConnect.rpc.Request +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonNull +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.jsonObject +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.test.assertTrue + +/** + * NIP-47 marks request parameters optional, and a wallet may type one strictly. + * Sending an absent parameter as an explicit `null` earned + * `Invalid list_transactions params: from must be an integer` from a real wallet + * and failed the request outright. + * + * The two serialization backends had disagreed since they were written: kotlinx + * omits nulls (`params.x?.let { put(...) }`), Jackson wrote them reflectively. The + * same request was two different documents depending on the platform, and only the + * JVM/Android one was broken. + */ +class Nip47NullParamOmissionTest { + private val requests: List> = + listOf( + // The reported failure: every field but three is absent. + "list_transactions" to ListTransactionsMethod.create(limit = 20, offset = 0, unpaid = false), + "pay_invoice" to PayInvoiceMethod.create("lnbc50n1abc"), + "make_invoice" to MakeInvoiceMethod.create(amount = 21000L), + "lookup_invoice" to LookupInvoiceMethod.createByHash("31afdf1"), + "pay_keysend" to PayKeysendMethod.create(amount = 21000L, pubkey = "0266e4"), + "create_connection" to CreateConnectionMethod.create(pubkey = "abc123", name = "app"), + "get_balance" to GetBalanceMethod.create(), + ) + + @Test + fun noRequestEverSendsANullParam() { + requests.forEach { (name, request) -> + val json = OptimizedJsonMapper.toJson(request) + val params = Json.parseToJsonElement(json).jsonObject["params"] as? JsonObject + + params?.forEach { (key, value) -> + assertTrue(value !is JsonNull, "$name sent \"$key\": null - omit it instead. Full: $json") + } + } + } + + /** The exact document the failing wallet rejected, now minimal. */ + @Test + fun listTransactionsCarriesOnlyWhatWasAsked() { + val json = OptimizedJsonMapper.toJson(ListTransactionsMethod.create(limit = 20, offset = 0, unpaid = false)) + + assertEquals( + """{"method":"list_transactions","params":{"limit":20,"offset":0,"unpaid":false}}""", + json, + ) + } + + /** + * The invariant the bug broke: both backends must produce the same document. + * Compared as parsed objects, since key ORDER is each backend's own business. + */ + @Test + fun bothBackendsProduceTheSameDocument() { + requests.forEach { (name, request) -> + val viaJackson = Json.parseToJsonElement(OptimizedJsonMapper.toJson(request)).jsonObject + val viaKotlinx = Json.parseToJsonElement(Json.encodeToString(Nip47RequestKSerializer, request)).jsonObject + + assertEquals(viaKotlinx, viaJackson, "$name differs between backends") + } + } + + /** Omitting a field must not change how the request reads back. */ + @Test + fun anOmittedParamStillParsesBack() { + val json = OptimizedJsonMapper.toJson(ListTransactionsMethod.create(limit = 20, offset = 0, unpaid = false)) + val back = OptimizedJsonMapper.fromJsonTo(json) + + assertIs(back) + assertEquals(20, back.params?.limit) + assertEquals(0, back.params?.offset) + assertEquals(false, back.params?.unpaid) + assertEquals(null, back.params?.from) + } +}