Add BIP-321 payment methods - #2
Conversation
| "payment": "bitcoin:?lno=lno1...", // BIP-321 URI, required | ||
| "amount_msat": 123000, // required if the selected instruction has no amount | ||
| "payer_note": "string", // payer-provided message, optional | ||
| "timeout_seconds": 60, // payment timeout, optional |
There was a problem hiding this comment.
is it necessary to specify a timeout? and why only here and not in any of the other commands? EDIT: but it is kind of nice that you can get "pending" back with it.
|
|
||
| ## BIP-321 processing | ||
|
|
||
| The wallet service MUST parse the URI according to BIP-321. This requirement includes case-insensitive keys, required parameters, duplicate parameters, and percent-encoding. |
There was a problem hiding this comment.
| The wallet service MUST parse the URI according to BIP-321. This requirement includes case-insensitive keys, required parameters, duplicate parameters, and percent-encoding. | |
| The wallet service MUST parse the URI according to BIP-321. |
(I guess people/agents should read BIP-321 fully, not just things mentioned here)
|
|
||
| The wallet service MUST make sure that the Bitcoin network matches before payment. It MUST reject a payment instruction for a different network. | ||
|
|
||
| A URI can contain `lightning`, `lno`, or both instruction types. If the URI contains multiple supported instructions, the wallet service MUST select and pay only one instruction. |
There was a problem hiding this comment.
| A URI can contain `lightning`, `lno`, or both instruction types. If the URI contains multiple supported instructions, the wallet service MUST select and pay only one instruction. | |
| A URI can contain `lightning`, `lno`, or both instruction types. |
I think it should be clear that not multiple instructions should be paid
There was a problem hiding this comment.
I'd rather keep this, some people could get confused thinking they are supposed to pay multiple, or people might misuse and try to do something like pay_multi_invoice through this
| "payment_id": "string", // wallet-scoped payment identifier | ||
| "state": "settled", // "pending", "settled", "failed", "expired", or "canceled" | ||
| "instruction_type": "bolt12", // "bolt11" or "bolt12" | ||
| "amount_msat": 123000, // paid amount |
There was a problem hiding this comment.
Up until now NWC everywhere uses msats, do we need to add this prefix and is it ok that we only add it here? (wondering about on-chain payments)
There was a problem hiding this comment.
matched the rest of the docs
| "method": "receive", | ||
| "params": { | ||
| "amount_msat": 123000, // optional; omit or use null for a variable amount | ||
| "description": "string" // optional |
There was a problem hiding this comment.
can we also add an optional metadata field here
| "result_type": "pay", | ||
| "result": { | ||
| "payment_id": "string", // wallet-scoped payment identifier | ||
| "state": "settled", // "pending", "settled", "failed", "expired", or "canceled" |
There was a problem hiding this comment.
Should "expired" and "cancelled" be a type of "failed" (maybe as an error message or type?)
There was a problem hiding this comment.
yeah just made it to failed but added a failure_reason
| "result_type": "receive", | ||
| "result": { | ||
| "bip321": "bitcoin:?lightning=lnbc...&lno=lno1...", // BIP-321 URI | ||
| "receive_id": "string", // wallet-scoped receive identifier, optional |
There was a problem hiding this comment.
wondering if instead of send_id and receive_id we should call it the same for send and receive? (thinking about list_transactions for example)
|
Do we need changes to any other methods with this? (for example |
I don't think so? They all have ids I believe so they should be matchable to these |
|
Is any NWC service ready for this that I can test with? |
…wc#2) Adds the generalized `pay` (BIP321 payment instruction, incl. BOLT12 `lno=`) and `receive` NIP-47 methods, plus the UNSUPPORTED_PAYMENT_INSTRUCTION / UNSUPPORTED_NETWORK error codes. The `pay` result carries `payer_proof` (lnp1…) — the proof a kind:9736 BOLT12 zap needs. Wired through both serialization backends (kotlinx + Jackson) with round-trip tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpgpWLKzgD7vS9Fs4CXTR3
|
My clanker has some questions for your clanker. Does payer_note map to the BOLT12 invreq_payer_note, and can payer_proof be relied on?
The spec says only: "If payer_note is not empty, the selected instruction MUST support payer-provided messages." For a BOLT12 offer, is the wallet required to place payer_note into the invoice request's invreq_payer_note (TLV type 89)? This matters because NIP-2421 binds a zap to its signed intent through exactly that field — the returned proof must carry invreq_payer_note == nostr:nipXX:. If a wallet routes payer_note anywhere else (or drops it from the request that gets signed into the proof), the payer_proof can't be turned into a valid zap receipt. Could the spec state explicitly that, for BOLT12, payer_note is carried in invreq_payer_note?
It's currently documented as // BOLT12 payer proof, optional if unavailable, with no wallet-support mandate. For a caller whose whole reason to use pay is to obtain the proof, "best-effort" is hard to build on. Is there any way for a client to know up front whether a given wallet will return payer_proof — e.g. a capability advertised in the info event / get_info? Otherwise a payment succeeds but the caller silently can't produce the artifact it needed. |
Yeah that is the intention but it can be re-used for adding a memo or whatever
should be returned for successful bolt12 payments, wouldn't be filled in for other payment types |
https://github.com/benthecarman/nostr-wallet-connect-lnd & https://github.com/benthecarman/nostr-wallet-connect-ldk-server |
|
I couldn't test end-to-end at all, but added: vitorpamplona/amethyst#3685 |
|
|
||
| ## Methods | ||
|
|
||
| ### `pay` |
There was a problem hiding this comment.
Should this be called pay_bip321?
I think of "pay" as the highest-level, most general "here's a payable thing, pay it" API possible.
Here are all the things that the pay endpoint supports in our SDK, for example:
- BIP 321 URI:
bitcoin:bc1... - Lightning URI:
lightning:ln... - BOLT 11 invoice:
lnbc1... - BOLT 12 offer:
lno1... - Onchain bitcoin address:
bc1... - Human Bitcoin Address:
₿username@lexe.app - Lightning Address:
username@lexe.app - LNURL:
lnurl1...orlnurlp://domain.com/path
There was a problem hiding this comment.
Originally was but everyone thought it'd be better to do as just pay as we hope to move this to be the standard function you call
There was a problem hiding this comment.
I guess this shifts the work of payment uri parsing and resolution from the NWC client to the server, which makes more sense and is more secure anyway - better to have clients make the resolution requests rather than have the server (which is holding the keys and the Lightning funds) open direct TCP connections to potentially-malicious webservers. And BIP 321 is flexible enough to support this - everything I listed above can be first resolved into a more direct form (HBA -> offer for example) and then passed into pay as a BIP 321 URI.
Tightens our implementation of the BIP-321-based NWC methods proposed in nostr-wallet-connect/nwc#2 — the successor to pay_invoice/make_invoice that clients are asked to transition to for bolt12 zaps in nostr-protocol/nips#2421: - reject cross-network payment instructions before paying (UNSUPPORTED_NETWORK) instead of failing after the debit - reject unknown req- BIP-321 params, including req-pop since we can't open proof-of-payment callbacks - reject a request amount that conflicts with the instruction amount - payer_note is delivered or rejected, never dropped: bolt11+note is rejected, internal settlements hand the note over as the payment memo - receive mints a per-request bolt12 offer carrying the requested description/amount instead of reusing the standing amountless offer - get_info reports the node's actual network instead of "mainnet" Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Just deployed support in coinos, I still need to test it myself a bit more thoroughly |
|
Minor inconsistency:
Both values are BIP-321 URIs. I would vote them both to be named the same. I started WIP PRs to add support to Alby Hub and Alby JS SDK |
Added as 321.md so we can hopefully move to the primary path in the future without disturbing the ordering.