Skip to content

feat(payments): NWC payment handler, e2e suite and docs, completing the rail - #119

Open
nisargpatel7042lva wants to merge 11 commits into
ContextVM:mainfrom
nisargpatel7042lva:feat/cep8-nwc-handler-b3
Open

feat(payments): NWC payment handler, e2e suite and docs, completing the rail#119
nisargpatel7042lva wants to merge 11 commits into
ContextVM:mainfrom
nisargpatel7042lva:feat/cep8-nwc-handler-b3

Conversation

@nisargpatel7042lva

@nisargpatel7042lva nisargpatel7042lva commented Oct 7, 2026

Copy link
Copy Markdown

part of #116

Third and last of the NWC sub-PRs (B3 of B1-B3). Stacked on #118, which is stacked on #117; diff base 4d7cc71 (the B2 commit). I'll rebase onto main once the parents merge.

Adds the client side and the end-to-end coverage, so the NWC rail is usable from both peers. With this, rail 1 of Phase B is complete; NIP-57 zaps (B4/B5) remain.

LnBolt11NwcPaymentHandler pays an offered BOLT11 invoice with pay_invoice. tests/payments_nwc_e2e.rs drives the whole thing over one MockRelayPool group, and tests/payments_nwc_real_wallet.rs is the #[ignore]d real-wallet pair.

The main thing to review: which lifecycle actually invokes the handler

I had assumed both did, and the e2e suite proved otherwise, so this is worth a second pair of eyes.

The handler is invoked automatically only in the transparent lifecycle, where the engine calls it on payment_required behind payment_policy. Explicit gating never touches it: the engine routes -32042 to the on_payment_required callback, and PaymentHandler is the transparent hook. A consumer wiring this rail for gating has to call the handler from inside that callback with the offered PaymentOption, then report paid: true so the engine retries.

That is not a change I made, it is how with_client_payments already works, but it is easy to get wrong and nothing said so. It is now documented in docs/payments.md under "Which lifecycle invokes the handler", and the gating e2e test shows the wiring in full rather than describing it.

Two smaller decisions in the handler, both deliberate:

  • can_handle stays at its default true. The spending decision belongs to payment_policy, which the engine consults first. A second gate here would split the budget across two places, and only the engine's gate can also synthesize a -32000 for the caller.
  • Its own response_timeout is load-bearing, not a tuning knob. The engine gives handle() neither a timeout nor a cancellation token, so it is the only thing between a silent wallet and a wedged payment task. the_handlers_own_timeout_bounds_a_silent_wallet pins it.

A wallet that answers with neither a result nor an error is treated as a failure. It has told us nothing, and reading that as paid would report success for a payment that may never have left.

The e2e suite does not stub the rail

Real client and server transports over one MockRelayPool group, registered through the production entry points, with a scripted NIP-47 wallet on each side. The server's processor really mints an invoice by publishing kind 23194 and reading kind 23195 back; the client's handler really pays by publishing to its own wallet.

Settlement is caused rather than scheduled: the issuing wallet answers lookup_invoice as pending until the paying wallet has actually been asked to pay, and only then flips to settled. A test that just slept and assumed would pass against a processor that never polled.

Five cases: the transparent lifecycle minting, paying and delivering the result at the client's own channel; explicit gating paying through the callback and the retry claiming the grant; a declining payment_policy that never reaches the wallet (asserted on the wallet's call count, not just the synthesized -32000); a pay_invoice error leaving the request pending with nothing forwarded; and a never-settling wallet ending at the TTL with no acceptance.

The gating test sleeps 1100 ms inside the callback before reporting paid. That is the same discipline payments_bilateral_e2e documents: a byte-identical retry inside the same wall-clock second mints the same Nostr event id, which the relay and the server's ingestion dedup both swallow. Without it the retry never arrives, which is exactly how I found it.

Builders on the option structs

Both NWC option structs now have new() and with_* builders. They are #[non_exhaustive] as #116 requires, and the integration tests are the first consumer outside this crate, which is where that bites: a downstream crate cannot name a #[non_exhaustive] struct in a struct expression, so Options { .. } and ..Default::default() are both rejected. Field-by-field mutation after default() works but is not what the rest of this crate does. This matches ClientPaymentsOptions and ServerPaymentsOptions.

This touches NwcClientOptions (from #117) and LnBolt11NwcPaymentProcessorOptions (from #118). Flagging it because it edits files the parent PRs introduce.

Real-wallet tests

#[ignore]d and environment-gated, needing two distinct wallets because one side issues and the other pays. Pointing both at one wallet makes it pay its own invoice, which most wallets refuse and which proves nothing about the rail. Each test skips with a printed reason when its variables are unset, so a bare --ignored run on a machine with no wallet is a no-op rather than a failure. Invoices are redacted to a 12-character prefix plus a length.

Not yet executed. I do not have two funded wallets to point at it, so the "real-wallet run executed and recorded in the PR" box on #116 is not ticked by this PR. Everything else in B1-B3 is. Happy to run it if someone can supply two throwaway connection strings, or to leave it for whoever merges.

Docs

docs/payments.md gains the rail: setup, both option tables, which lifecycle invokes the handler, how to test against a real wallet, and the operational notes #116 asked for.

  • A late settlement is money out with no execution: verify_payment stops at the middleware's TTL, but the invoice stays payable until its own expiry, so keep invoice_expiry at or below ttl and treat reconciliation as an operator concern.
  • The wallet is a trust boundary: the processor believes it about settlement, so a wallet reporting settled falsely gets a capability executed for free.
  • Rate limiting is still upstream's job, unchanged from implement CEP-8 Phase B Lightning rails (NWC and zaps) #116's closing note.

The intro and the old "Phase B: payment rails" section no longer describe Phase B as deferred, and LNbits now appears once, saying it was dropped. README updated likewise.

Verification

cargo fmt --check clean, cargo clippy --all-features --all-targets clean, 1121 tests pass under --all-features with 0 failures, --no-default-features and default builds green, MSRV 1.88 compiles, cargo doc builds.

Still open from #116

The mutation testing of the money paths that B2 asks for, and the real-wallet runs for both rails. Neither blocks this PR landing as the rail's code, but both are on the exit criteria.

@nisargpatel7042lva
feat(payments): NIP-47 client core and mock wallet for the NWC rail
408a37f 
First of the three NWC sub-PRs in ContextVM#116. Rail infrastructure only: no
processor, no handler, and no change to the Phase A traits, middlewares
or registration entry points.

Behind the off-by-default `nwc` feature, which only switches on
`nostr-sdk/nip47` and adds no new crate to the dependency tree.

- `parse_nwc_uri` reads a connection string in either shape wallets emit.
  `NostrWalletConnectURI::parse` takes the pubkey from the URI host only,
  so it rejects the `nostr+walletconnect:<pubkey>?...` pathname form the
  ts-sdk accepts; a connection string that works there must not fail
  here. A test pins the upstream rejection as a precondition.
- `NwcClient` speaks request/response and notifications over an injected
  `RelayPoolTrait`, so the same code path runs against a scripted mock
  wallet in CI and a real wallet in production. The `nwc` crate is not
  used: its relay pool is private, so nothing built on it is testable
  without a live wallet.
- One subscription and one reader task, correlating by request event id.
  `RelayPoolTrait` has no unsubscribe, so the ts-sdk
  subscription-per-request shape would leak one live REQ per payment.
- Wallet responses are parsed permissively, field for field with ts
  `nip47/types.ts`, rather than through the strict upstream response
  structs. On a money path a missing or unknown optional field must not
  turn a paid invoice into a failed verification.
- Notifications are decrypted by kind (23196 NIP-04, 23197 NIP-44). ts
  always decrypts as NIP-04 and so silently drops the NIP-44 form. Kind
  23197 has no named constant in `nostr` 0.44, hence `Kind::from`.
- `sats_to_msats` rejects a non-positive amount and uses `checked_mul`,
  both before any wallet call, where ts does a bare multiply.
- `MockWallet` behind `test-utils` runs the real wire protocol over a
  linked `MockRelayPool`, scripting answer, error, silence, delay and
  malformed replies, and can publish a `payment_received` notification.

A wallet-reported error is returned as `Ok(envelope)` with `error` set
rather than `Err`, matching ts: the processor in B2 has to treat
`NOT_FOUND` as pending but other codes as fatal, which needs the code at
the call site. `into_result()` collapses the envelope for callers that
do not classify, so a failed payment still cannot read as a settled one.

Verified: fmt and clippy clean, 1084 tests pass under `--all-features`,
`--no-default-features` and default builds green, MSRV 1.88 compiles.

Refs ContextVM#116
@nisargpatel7042lva nisargpatel7042lva mentioned this pull request Oct 7, 2026
Open
@nisargpatel7042lva
fix(payments): harden the NIP-47 client against wallet-facing failure
0ef9c44 
Addresses the review on ContextVM#117. The design held; what did not was
robustness against a wallet that misbehaves, and the money-path bounds.
A mock written alongside the client agreed with the client wherever the
client was wrong, so most of this was invisible to the old suite.

Critical

- One panic on the reader killed the rail for the process, silently,
  while requests kept publishing. Per-event handling now runs under
  catch_unwind, the reader marks the client closed and drains its
  waiters on exit, and `nostr` is floored at 0.44.7 (0.44.5 panics on a
  malformed NIP-04 payload, and "0.44" permitted it).
- One off-type optional field threw out the whole response.
  `#[serde(default)]` covers absent, not wrong-typed, so a string
  `settled_at` lost a result that said `"state":"settled"` and the payer
  would have paid for nothing. Every optional field now deserializes
  off-type values to `None`.

Major

- The response filter widens itself once, dropping `#p`, while nothing
  has ever correlated, so a wallet that omits the tag is not invisible.
- One deadline now covers start-up, publish and the wait, where publish,
  connect and subscribe used to sit outside it.
- A dropped request future no longer orphans its waiter: an RAII guard
  removes it. The processor's `select!` drops one per cancelled verify.
- `close()` is fail-fast. It used to leave `pay_invoice` publishing, and
  the wallet still paid.
- Errors are typed. `NwcError::Timeout` carries `published`, so a caller
  can tell "the wallet may have acted" from "it certainly did not",
  which is what a non-idempotent `pay_invoice` needs, and the trait
  boundary maps to Processor or Handler explicitly.
- Error text can no longer carry a BOLT11 invoice into operator logs:
  wallet free text is redacted and bounded, and a parse failure reports
  its category and position instead of the offending bytes.
- `MockWallet` can now break the client's assumptions: omit the `p` tag,
  answer NOT_IMPLEMENTED, mislabel `result_type`, drop the `e` tag,
  publish both notification kinds, and answer out of order. It serves
  each request on its own task, so a fast answer really can overtake a
  slow one.
- The public API is typed: `make_invoice` / `lookup_invoice` /
  `pay_invoice`, `request` crate-internal, wire types
  `#[non_exhaustive]`, and the options builder lands here.
- `NwcClient::connect` builds the client its own pool. `with_pool` stays
  for tests, documenting why sharing a transport's pool is a trap.

Minor

- Corrected the "adds no new crate" claim, which was wrong: the feature
  pulls in NIP-04 and so adds `aes`, `cbc` and `cipher`. Measured.
- Notifications decrypt by ciphertext shape, not event kind, so NIP-04
  content on kind 23197 (ts's own fixture) decodes.
- The subscription carries `since`, so a retaining relay does not replay
  this client's whole history into the sinks.
- The legacy `nostrwalletconnect://` scheme parses.
- A zero `response_timeout` is refused at construction.
- `pay_invoice` takes an optional NIP-40 expiration.
- Rustdoc corrected where it no longer matched behavior, including the
  claim about handing pieces to the upstream URI type, which it never did.

Non-blocking: a mismatched `result_type` is tolerated and warned about,
since correlation by event id already proves which request an answer
belongs to; duplicate `secret` is documented as last-wins; the `e` tag
parity note is recorded; the two quiet side effects of
`fetch_info_notification_types` are documented.

Tests go 38 to 61, and now bite: a five-mutant sweep over the author
check, the waiter guard, the closed flag, decrypt-by-shape and the
off-type leniency kills all five. The author check needed
`MockRelayPool::publish_event_unfiltered`, which models a relay that
ignores `authors`; without it the guard is unreachable, which is why
deleting it used to survive the suite.

Verified: fmt and clippy clean, 1104 tests pass under `--all-features`,
feature matrix green, MSRV 1.88 compiles.

Refs ContextVM#116
@nisargpatel7042lva
fix(payments): export MockWalletQuirks and correct the nip47 module doc
95dc0e9 
The quirks struct was reachable only by its full module path, so a
downstream test could not use it, and the module doc still described the
client as taking an injected pool after `connect` made it own one.
@nisargpatel7042lva
fix(tests): drop the deprecated fetch_update from the mock relay pool
d1860e0 
Rust 1.99 deprecates `Atomic::fetch_update` in favour of `try_update`,
and CI lints with `-D warnings`, so `cargo clippy --all-targets` fails
on main. `try_update` is only stable from 1.95, above the declared MSRV
of 1.88, so this spells the same decrement-while-non-zero out as a
compare-exchange loop, which compiles warning-free on both.
@nisargpatel7042lva
fix(payments): three-state delivery, redactor hardening and evidence-…
51160ab 
…led widening

Addresses the five blocking items from the second review of the NIP-47
client.

(8a) A deadline that fires inside the publish is not a non-delivery. The
pool writes the EVENT before it waits on the relay's OK, so the wallet
may have the request. `Delivery` replaces the `published` bool with
`NotSent`, `Unknown` and `Sent`, and `may_have_reached_wallet` treats
`Unknown` as "may have", which is the safe direction for `pay_invoice`.

(24) `Closed` carries the same `Delivery`, so a `close()` landing after
the request went out no longer reports a safe retry. `request` also
re-checks the closed flag after start-up: `close()` returns as soon as
it has set the flag, so one arriving during a slow connect used to be
followed by a publish.

(25) `sanitize_wallet_text` walks back to a character boundary before
truncating. `truncate` panics mid character, and the text is
wallet-controlled, so a long non-Latin message was a remote panic on the
pay path. `floor_char_boundary` is the obvious fix and is stable only
from 1.95, above the declared MSRV.

(26) Three remaining paths for wallet text to reach a log unredacted:
the error code was copied raw and unbounded, a mislabelled `result_type`
was logged raw, and the BOLT11 prefix test was `starts_with` on
whitespace-split words, which `lightning:lnbc...`, `(lnbc...)` and
quoted forms all passed while carrying the full payment hash. The prefix
is now matched anywhere in the token.

(3) Widening now requires evidence that the narrow filter has failed: a
request that reached a relay has waited out a full deadline, and nothing
has ever correlated. It is latched, since the pool has no unsubscribe
and a silent wallet would otherwise accumulate one subscription per
call; it widens the response kind only, leaving `#p` on both
notification kinds; and it logs at warn.

Tests: a nine-mutant sweep over these fixes, each killed. Two of them
survived the first pass because the delivery states were asserted in
isolation without anything proving `request` produces them, so both now
have tests that drive the real path. Also adds a response correlated to
a different request, which the existing no-`e`-tag test cannot see,
since that one is dropped before the correlation lookup is reached.
@nisargpatel7042lva
test(payments): make the reader, waiter and subscription guards bite
69212f4 
The review measured 17 of 35 mutants living through the suite and named
the three on money paths. This kills those and five more, so every guard
these PRs added now has a test that fails when the guard is removed.

The three named ones:

- the `Lagged` arm breaking the reader instead of continuing, which
  would close the client and fail every later request while requests
  already in flight could still have paid;
- registering the waiter after the publish rather than before, which
  loses an answer that arrives inside that window;
- bypassing the waiter guard, which leaks an `EventId` and a
  `oneshot::Sender` on every cancelled verification.

And five that were untested: `since` at subscribe, the NIP-40
expiration tag, the reader-exit drain, and the payload-free parse
error, whose existing test built its serde failure from a string with
no payload in it and so passed even when the whole serde message was
leaked.

Two needed infrastructure, so the test pool now owns the notification
stream. A test can overflow it synchronously, which on a current-thread
runtime guarantees the reader observes `Lagged` rather than racing for
it, and can close it to force a real reader exit. It can also park the
caller inside `publish_event` after the event is on the wire, which
makes the registration-order window deterministic.
@nisargpatel7042lva
docs(changelog): correct the NWC client entry to the shipped behaviour
0f3490e 
Three statements no longer matched the code, all of them on points the
review raised and I said were fixed.

The client no longer speaks "over an injected `RelayPoolTrait`": item
(12) made it build its own pool from the connection relays, with
injection kept for tests. The module doc was corrected at the time and
this was not.

"Decrypted by kind (23196 NIP-04, 23197 NIP-44)" describes exactly the
behaviour item (14) asked to change, and the review said so about this
line. It is decryption by ciphertext shape now.

"A missing or unknown optional field" understates item (2), which was
about a field present with the wrong JSON type, the case that lost a
settled response.

Also records the typed error and `Delivery`, which are public API a
caller needs in order to know whether retrying `pay_invoice` is safe.
@nisargpatel7042lva
feat(payments): NWC payment processor, the server side of the NWC rail
85cefa9 
Second of the three NWC sub-PRs in ContextVM#116. Adds
`LnBolt11NwcPaymentProcessor` for PMI `bitcoin-lightning-bolt11`, behind
the same off-by-default `nwc` feature. No trait, middleware or
registration entry point changes: it plugs into the `PaymentProcessor`
trait Phase A froze.

- `create_payment_required` mints a BOLT11 invoice with `make_invoice`
  in msats with an expiry, caches invoice to `payment_hash`, and
  advertises the configured TTL rather than the wallet's `expires_at`,
  which providers return in non-standard forms (ts parity). Amounts are
  validated before any wallet call.
- `verify_payment` polls `lookup_invoice` on the ts backoff schedule,
  floored at the configured poll interval and jittered, or waits on a
  `payment_received` notification when the wallet advertises one.
- In-flight dedup by `pay_req` over a shared future. `PaymentError` is
  not `Clone`, so the shared output is an `Arc<Result<..>>` and joiners
  re-wrap the error by message.
- `NOT_FOUND` is pending, every other wallet error is fatal, `expired`
  and `failed` are terminal, and settlement is either `state ==
  "settled"` or a positive `settled_at`, since wallets populate one or
  the other. The receipt prefers `payment_hash` and falls back to ts's
  `settled:<request_event_id>[:<settled_at>]`.
- Notification mode auto-detects once from the info event via a
  `OnceCell`, and the subscription installs exactly once: with no
  unsubscribe on `RelayPoolTrait`, a racing second subscribe would leak
  a sink for the process lifetime.
- Every wait selects on `params.cancel`. A cancelled verify returns
  `Err`, never an empty `Ok` the middleware would read as payment.

Two documented divergences from ts. Notification mode falls back to
polling when no `payment_hash` is cached, where ts throws and strands a
payable invoice. Jitter is derived from the clock rather than a RNG,
since this crate has no direct `rand` dependency and de-stampeding does
not need cryptographic randomness.

23 tests, including the ts unit tests ported, cancellation stopping the
poller, a hung wallet bounded by the response timeout, terminal states,
the fallback receipt, notification mode and its polling fallback,
amount rejection before any wallet call, and a check that no tracing
call binds the invoice as a field.

Verified: fmt and clippy clean, 1107 tests pass under `--all-features`,
feature matrix green, MSRV 1.88 compiles.

Refs ContextVM#116
@nisargpatel7042lva
docs(changelog): say what the processor does instead of naming a phase
a3237e5 
"the Phase B Lightning rail" is internal planning vocabulary that a
changelog reader cannot resolve, which the review noted about the
client entry. The same wording was in this one.
@nisargpatel7042lva
feat(payments): NWC payment handler, e2e suite and docs, completing t…
94a0d1d 
…he rail

Third and last of the NWC sub-PRs in ContextVM#116. Adds the client side and the
end-to-end coverage, so the rail is usable from both peers.

- `LnBolt11NwcPaymentHandler` pays an offered BOLT11 invoice with
  `pay_invoice`, bounded by its own `response_timeout`: the client engine
  gives `handle()` neither a timeout nor a cancellation token, so that
  bound is part of the rail's contract. The spending decision stays with
  `payment_policy`; adding a second gate in `can_handle` would split the
  budget across two places, and only the engine's gate can synthesize a
  `-32000` for the caller. A wallet answering with neither result nor
  error is a failure, not a silent success.
- `tests/payments_nwc_e2e.rs` runs real client and server transports over
  one `MockRelayPool` group with a scripted wallet on each side, through
  the production registration entry points. Settlement is causal rather
  than assumed: the issuing wallet answers `lookup_invoice` as pending
  until the paying wallet has actually been asked, then flips to settled.
- `tests/payments_nwc_real_wallet.rs` is `#[ignore]`d and environment
  gated, needing two distinct wallets since one issues and the other
  pays. Invoices are redacted in its output.

Both NWC option structs gain `new()` and `with_*` builders. They are
`#[non_exhaustive]` as ContextVM#116 requires, which makes a struct literal
unusable from a downstream crate, and the integration tests are the first
consumer outside this crate to hit that. Matches `ClientPaymentsOptions`
and `ServerPaymentsOptions`.

Worth knowing, and now documented: the handler is invoked automatically
only in the transparent lifecycle. Explicit gating routes `-32042` to the
`on_payment_required` callback, from which the handler can be driven by
hand; the e2e suite shows that wiring.

`docs/payments.md` documents the rail, its options, which lifecycle
invokes the handler, and its operational notes (a late settlement is
money out with no execution, the wallet is a trust boundary, rate
limiting is still upstream's job), and no longer calls Phase B deferred
or lists LNbits as planned. README and CHANGELOG updated.

Verified: fmt and clippy clean, 1121 tests pass under `--all-features`,
feature matrix green, MSRV 1.88 compiles, docs build.

Refs ContextVM#116
@nisargpatel7042lva
docs(payments): correct the feature's cost and document retry safety
efba75c 
Two fixes to `docs/payments.md`.

It still claimed the `nwc` feature "adds no new crate to the dependency
tree". That is the claim review item (13) corrected, and I said I had
fixed it everywhere; I had fixed `Cargo.toml`, the module doc and the
changelog, and missed this one. `cargo tree` measures `aes`, `cbc` and
`cipher`.

And nothing documented when a failed payment is safe to retry, which
is the whole point of the three-state `Delivery` the error now carries.
A caller reading this page to decide whether to re-send a `pay_invoice`
needs it, so there is now a table, including why an in-flight publish
is grouped with "sent" rather than "never sent".

The handler's changelog entry also drops the internal phase vocabulary,
notes the NIP-40 expiration, and no longer advertises removing a line
about LNbits that the page already handles in one sentence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@nisargpatel7042lva