Add NWC (Nostr Wallet Connect) as alternative Lightning backend

[themikemoniker]

Introduce a LightningBackend interface that both PhoenixService and a new NwcService implement. A --nwc-uri CLI arg determines which backend is active. Wallet API routes return the same JSON shapes regardless of backend, so the client works unchanged.

The NWC protocol layer (nwc/ package) handles NIP-47 request/response over Nostr relays using nostrino for secp256k1 crypto and Ktor WebSocket client for relay communication. Payment detection uses invoice polling since NWC has no webhook equivalent.

[themikemoniker]

bump up my post

[iorch]

Hey! 👋 This PR is ready for review. Here's a quick orientation:

What this adds

A NwcService that implements LightningBackend using Nostr Wallet Connect (NIP-47) instead of phoenixd. Any NWC-compatible wallet (Alby Hub, Mutiny, Zeus, etc.) can power the POS without running a self-hosted Lightning node. Activation via env var or the onboarding wizard:

NWC_URI="nostr+walletconnect://..." docker compose -f docker-compose.yml -f docker-compose.nwc.yml up -d

Where to start reviewing

• nwc/ — four new files (NwcClient, NostrRelay, Nip47Types, NwcConnectionInfo) — self-contained, easy to read in isolation
• services/NwcService.kt — the glue layer; key logic in pollPendingInvoices (settledAt detection → PaymentNotifier) and the Nip47Transaction extension functions at the bottom
• api/InitialSetup.kt + Onboarding/WalletBackendStep.jsx — new onboarding step lets users configure the NWC URI at setup time; URI is persisted to ambrosia.conf
• config/ConfigFile.kt — bugfix: split("=").last() was dropping everything before the last =, breaking any conf value with = in it (NWC URIs, future URLs)
• Api.kt — the only wiring change: --nwc-uri selects NwcService at startup

What's tested

• 24 unit tests (NwcUriParserTest, NwcServiceTest) covering URI parsing, msat conversion, polling TTL, pubkey fallback, and unsupported-operation throws
• End-to-end on regtest against Alby Hub: invoice creation, payment detection via polling, balance display, onboarding flow

Known limitations (intentional, documented in MERGE.md)

1. Fat LightningBackend interface — NwcService throws UnsupportedBackendOperationException for 7 methods (getSeed, createOffer, payOffer, payOnchain, bumpOnchainFees, csvExport, closeChannel). ISP violation is pre-existing, not introduced here.
2. Single relay — NwcConnectionInfo uses associate() on query params so only the last relay= value is used. Multi-relay fallback is a future improvement.
3. No TTL on pendingRequests — stranded CompletableDeferred entries accumulate if the relay drops mid-request. Low risk in practice given the 30s timeout.
4. No re-subscribe after reconnect — NostrRelay reconnects with exponential backoff but doesn't re-send the NIP-47 subscription filter. Responses may be missed after a relay drop.

Thanks for the review!

[chrisguida]

@iorch are you taking this PR over? guess i can test this with my new phoenixd + nwc...

[iorch]

@chrisguida yes, it was outdated, and had some hardcoded configuration.
Also, I added a selector in the ornboarding and some tests. Please test it and let me know if something fails.

[Sharmaz]

Frontend Reviewed, Requested Changes, fix the comments.

[chrisguida]

Tested this end-to-end against a phoenixd-NWC fork I'm running, plus full static review of the diff. Lots to like — clean separation of LightningBackend, the nwc/ package is self-contained and easy to read, and the unit tests cover the URI parser and the service-level conversions thoroughly. CI is green and ktlintCheck passes.

That said, this isn't ready to merge yet. Live testing surfaced a handful of blocking bugs and several UX issues that I think need to be addressed before this lands. Findings are grouped by severity below; references to file paths use the PR's nwc-integration branch.

Blocking

B1. Nip47Info deserialization is too strict and crashes get_info on real-wallet payloads

First call to /wallet/getinfo against my (admittedly non-spec) phoenixd-NWC fork produced:

Unhandled exception: Expected JsonPrimitive, but had JsonArray as the serialized body of string at element: $.block_hash
JSON input: []

Two things wrong here:

1. The block_hash field is never read by Ambrosia (grep blockHash in services/NwcService.kt returns nothing — only the Nip47Types.kt:68-69 declaration). Failing the entire get_info call for an unused field is gratuitous.
2. The FE renders this as the same "Lightning node is not available. Make sure phoenixd is running." error that phoenixd connection failures produce — actively misleading for an NWC user. Root cause is Wallet.kt routes returning a generic PhoenixServiceException mapping for any backend failure (Handler.kt:90-93).

Suggested fixes:

• Drop blockHash from Nip47Info entirely (preferred — it's unused).
• Add coerceInputValues = true to lenientJson in NwcClient.kt:28 for broader robustness against future spec drift.
• Add a JSON deserialization round-trip test using a captured wallet response (Alby Hub / Mutiny / Zeus). The current tests construct Nip47Info(...) in-memory and miss this entire failure mode.
• Also: when the backend is NwcService, the wallet error path shouldn't surface phoenixd-flavored copy. Either a new NwcConnectionException flow in Handler.kt or a generic backend-agnostic message.

B2. payInvoice reports wrong amount and empty payment hash on success

Paid a 1-sat fixed-amount invoice end-to-end. Payment went through (toast: "Payment sent successfully"), but the confirmation modal shows:

Amount Sent: 0 sats, Routing Fee: 0 sats, Payment Hash: (empty, with "Copy" button)

Bug in services/NwcService.kt:149-159:

recipientAmountSat = request.amountSat ?: 0,   // null when invoice carries amount → 0
paymentHash = "",                               // hardcoded empty

NIP-47 pay_invoice returns only preimage and feesPaid. The amount and hash both have to be derived from the bolt11 string (Ambrosia already has a Bolt11Decoder used by PhoenixService for extractDescription). NwcServiceTest.kt:130-140 only asserts routingFeeSat and paymentPreimage — neither of these fields is covered.

B3. NWC activation requires a manual server restart with no in-app trigger

After completing onboarding with NWC selected, the wallet page shows the same "Lightning node not available" error from B1 until the operator manually runs docker compose -f docker-compose.dev-nwc.yml restart ambrosia-dev (or equivalent for native installs). Root cause: configureWallet() in api/Wallet.kt:36-43 reads nwc-uri from environment.config exactly once at module init. The "NWC URI saved — restart required to activate" toast tells the user a restart is needed but provides no path to do it from the UI.

In a docker deployment (which this PR explicitly ships dev tooling for), there is no in-app way to recover. Suggestions in priority order:

1. Drop the "restart required" model — make configureWallet() re-resolve backend on each request, or on a config-change event.
2. Have the setup endpoint trigger a graceful self-restart after persisting NWC URI.
3. At minimum, route the wallet error through a different exception class so the UI message matches reality ("NWC not yet active — restart the server" instead of "phoenixd not running").

B4. "Close Channel" button is clickable but always 501s under NWC

getNodeInfo() in NwcService.kt:130-147 synthesizes a fake channel with state: "NORMAL" to keep the existing <ChannelCard> UI happy. That FE component then enables the "Close Channel" button whenever state === "NORMAL" (ChannelCard.jsx:91). Clicking it triggers closeChannel → UnsupportedBackendOperationException → 501 → unhandled error in the UI. Easy to miss in review because it requires clicking a button to discover the failure.

The whole "Lightning Channels" card is dead weight under NWC — see U2 below for the recommended fix.

B5. Base branch is development but PR is rebased onto main (per MERGE.md)

GitHub reports mergeable: CONFLICTING. MERGE.md explicitly states it's rebased onto origin/main (v0.6.0-beta). Whatever the intended target, this needs a clean rebase before merge.

Important (should fix before merge)

I1. feesPaid units mismatch in transaction listings

Nip47Transaction.feesPaid is millisats per spec. toIncomingPayment/toOutgoingPayment (NwcService.kt:290, 310) pass it straight to IncomingPayment.fees / OutgoingPayment.fees. Compare with payInvoice (line 154) which correctly divides by 1000. Listings will show fees 1000× too large.

I2. Resource leak — NwcService.create() never closes

NwcService.create() constructs an HttpClient(CIO) and CoroutineScope(SupervisorJob() + Dispatchers.IO) (NwcService.kt:243-249) but nothing closes them on server shutdown. Each configureWallet() call leaks both. Tolerable today because configureWallet() is called once at startup, but fragile.

I3. chain defaults silently to "mainnet" if wallet omits network

NwcService.kt:143 — chain = info.network ?: "mainnet". A wallet that doesn't return network (or returns an unrecognized value) would silently mislabel the chain on regtest/testnet. At minimum log a warning and surface "unknown" instead of "mainnet".

I4. Connect-and-poll race on first request

NwcService.create() returns synchronously while the relay connect happens in a fire-and-forget scope.launch{} (NwcService.kt:254-262). The wallet routes are immediately bound. A request that arrives before connect() completes will hit NostrRelay.send() which throws NwcConnectionException("Not connected to relay"). Hard to hit in practice (the relay handshake is fast) but worth either gating routes behind a ready flag or awaiting connect.

I5. No re-subscribe after relay reconnect

Acknowledged in MERGE.md but worth restating: NostrRelay reconnects with exponential backoff on disconnect (good), but NwcClient.connect() only subscribes once at startup. After a reconnect the relay no longer has the #p filter, so NIP-47 responses get lost. Manifests as the wallet UI silently going stale after a network blip.

I6. Polling makes one round-trip per pending invoice per cycle

pollPendingInvoices() (NwcService.kt:67-92) calls lookup_invoice for every entry every 3 seconds. With N concurrent unpaid invoices that's N×20 relay events per minute, all going through one WebSocket. A single list_transactions(unpaid=false, from=oldestPendingTime) would pull all settlements in one round-trip and let you reconcile against pendingInvoices. Important for any deployment with more than a handful of concurrent invoices.

UX

U1. NWC onboarding flow communicates the restart requirement, but only via a transient toast

The "NWC URI saved" toast disappears after a few seconds. After it's gone, the user has no indication that a restart is required and no way to recover. See B3 for full fix; at minimum surface a persistent banner on the post-onboarding landing page until the backend is actually active.

U2. "Lightning Channels" card under NWC is duplicative and confusing

The synthesized nwc-virtual channel has:

• channelId: nwc-virtual — meaningless to a non-node-runner.
• balanceSat == capacitySat == nodeBalance — duplicates the "Total Balance" card directly above it.
• inboundLiquiditySat: 0 — wrong; NWC has no way to query inbound, so claiming 0 is misleading.
• "Close Channel" button enabled (B4).

Clean fix: have NwcService.getNodeInfo() return channels: emptyList() and let the FE handle empty gracefully (it already does — the section just disappears). Honest, requires no FE change.

U3. docker-compose.dev-nwc.yml ports bound to 127.0.0.1 hang in browser

Compose binds 127.0.0.1:3001:3001. Next.js 16's allowedDevOrigins blocks 127.0.0.1 as cross-origin from localhost, killing dev-runtime hydration. First-time reviewers will see a blank "Loading…" page with no error. Fix: bind as localhost:3001:3001, or add allowedDevOrigins: ['127.0.0.1'] to client/next.config.js, or document in the compose file header.

U4. Compose file's documented NWC_URI=... shortcut doesn't actually work

The header comment in docker-compose.dev-nwc.yml says:

# Start with NWC already configured (skip onboarding):
#   NWC_URI="nostr+walletconnect://..." docker compose -f docker-compose.dev-nwc.yml up -d --build

But the ambrosia-dev service has no environment: block forwarding NWC_URI. The host env var won't reach the container. Add:

environment:
  NWC_URI: ${NWC_URI:-}

Nits

• N1: Drop MERGE.md from the repo. It belongs in the PR description (and is largely already duplicated there). Source-tree pollution of design doc + future-work notes.
• N2: Nip47Info.blockHash (B1 above) and the unused Nip47Transaction extension functions could be slimmed down — the descriptionHash, metadata, expiresAt-as-fallback fields aren't surfaced anywhere downstream.
• N3: getNodeInfo() makes two relay round-trips (getInfo + getBalance). Consider caching the balance for ~3 seconds since the same data feeds the Total Balance card and the synthesized channel.
• N4: NwcService.getOutgoingPaymentByHash uses lookupInvoice (which is for incoming) and converts the result to OutgoingPayment. NIP-47 has no equivalent of "outgoing lookup by hash" — this should probably also throw UnsupportedBackendOperationException for clarity, since the result for a real outgoing-only wallet is going to be wrong.
• N5: Existing review comments from @Sharmaz weren't addressed — generic abbreviated names in Onboarding.jsx:111, unnecessary comments in tests and Onboarding.jsx, restaurant-related changes in lib/modules.js.
• N6: Add a regtest path to the test instructions. Phoenixd has no regtest LSP so it can't be used for regtest testing; CLN + cln-nip47 plugin is the standard path. A docker-compose.regtest-nwc.yml overlay (bitcoind regtest → CLN → cln-nip47 → Ambrosia) would make this PR much easier to review and would unblock regtest CI integration tests.
• N7: pendingRequests in NwcClient doesn't TTL stranded CompletableDeferred entries on relay-mid-request drops — acknowledged in MERGE.md. Minor since the 30s withTimeout cleans up, but worth fixing alongside I6.

What I verified works

• All 24 server unit tests pass on Java 21 (NwcServiceTest, NwcUriParserTest).
• All 80 client unit tests pass.
• ktlintCheck is clean.
• NWC URI parsing across the cases the unit tests cover.
• After the deserialization fix on the wallet side and a server restart: relay connection, subscription, get_info → node info renders, make_invoice → invoice generated and tracked, payment polling detects settledAt within ~3s, PaymentNotifier broadcasts over /ws/payments, UI receives the event. Receive flow works end-to-end.

Test environment

• Local Ambrosia in docker-compose.dev-nwc.yml (Linux, Java 21, Node 20).
• A phoenixd-NWC fork as the wallet (chrisguida/phoenixd@electrum-nwc-bolt12), patched mid-test to drop a non-spec block_hash: [] from get_info. The wallet-side bug isn't relevant to this PR, but it surfaced the deserialization brittleness in B1.
• NWC URI over a public Nostr relay.

---

This is a substantial and useful contribution and most of these are mechanical to fix. I'd like to see B1–B5 resolved before merge, plus a triage pass on I1–I6. Happy to re-review once those are addressed.

[iorch]

Thanks for the thorough review — really useful findings. Here's what's been addressed:

Blocking

• B1 — Dropped the unused blockHash field from Nip47Info; ignoreUnknownKeys now handles any block_hash value a wallet sends. Added coerceInputValues = true to lenientJson for broader spec-drift resilience.
• B2 — payInvoice now decodes recipientAmountSat and paymentHash from the bolt11 string via Bolt11Decoder (NIP-47 pay_invoice only returns preimage).
• B3 — Dropped the restart requirement. Backend is held in an AtomicReference; InitialSetup calls reinitializeNwcBackend() immediately after writing the URI to ambrosia.conf — wallet is live on the next request.
• B4 — getNodeInfo() now returns channels = emptyList(). The synthesized channel was enabling the Close Channel button which always 501'd.
• B5 — Will rebase onto the target branch before merge.

Important

• I1 — Fixed: feesPaid in toIncomingPayment and toOutgoingPayment now divided by 1000.
• I3 — Fixed: null network logs a warning and returns "unknown" instead of silently claiming "mainnet".
• N4 — Fixed: getOutgoingPaymentByHash now throws UnsupportedBackendOperationException.
• I2, I4, I5, I6 — Acknowledged. I2 (resource leak) and I4 (connect race) are low-risk given single-init semantics; I5 (no re-subscribe on reconnect) and I6 (polling efficiency) are tracked for a follow-up. Happy to address them in this PR if you'd prefer.

UX / Docker

• U2 — Empty channel list (same fix as B4).
• U3 — Port binding changed to localhost in dev-nwc compose.
• U4 — Added NWC_URI env passthrough to the ambrosia-dev service.

Nits

• N1 — MERGE.md removed.
• N5 — Addressed Sharmaz's inline comments: renamed res → setupResponse, removed types: ["restaurant"] from orders/spaces routes (restaurant is disabled — those routes were effectively inaccessible), extracted navigateToStep helper in tests.

Regression tests added for B1 (Nip47DeserializationTest), B2 (Bolt11DecoderTest), and I1 (listing fee conversion in NwcServiceTest).

[chrisguida]

@iorch nice! will review again soon