mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix(nwc): omit absent request params instead of sending them as null
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.
This commit is contained in:
davotoula
+28
@@ -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())
|
||||
|
||||
+45
@@ -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
|
||||
+115
@@ -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<Pair<String, Request>> =
|
||||
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<Request>(json)
|
||||
|
||||
assertIs<ListTransactionsMethod>(back)
|
||||
assertEquals(20, back.params?.limit)
|
||||
assertEquals(0, back.params?.offset)
|
||||
assertEquals(false, back.params?.unpaid)
|
||||
assertEquals(null, back.params?.from)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user