multi: add a REST transport option for the lnd wallet backend - #965
multi: add a REST transport option for the lnd wallet backend#965ellemouton wants to merge 3 commits into
Conversation
waved's LND wallet backend connected to lnd over gRPC only (via lndclient, which has no REST mode). LND exposes every subserver method the backend uses over its grpc-gateway REST interface -- signer (including MuSig2), walletkit, chainkit, and the streaming chain-notifier -- so add lnd.transport=rest to talk to lnd over REST instead of gRPC. - lndrest: REST implementations of the lndclient SignerClient, WalletKitClient, ChainKitClient, ChainNotifierClient (streaming), and the LightningClient slice (GetInfo/WalletBalance) the daemon uses, built on the existing rpc/restclient HTTP + server-stream plumbing. Methods the daemon never calls in lnd mode return errUnsupportedOverREST. - waved: abstract s.lnd behind an lndServices interface so either a gRPC (*lndclient.GrpcLndServices) or REST (*lndrest.Backend) backend drops in; connectLnd branches on lnd.transport. New LndConfig.Transport (default grpc) + lnd.transport flag + sample-waved.conf; Host is reinterpreted per transport (host:port for gRPC, HTTPS base URL for REST). transport=rest requires lnd.macaroonpath, since the per-network default cannot be resolved over REST. - lndbackend: route the locator-aware SignOutputRaw through a narrow Signer interface so the gRPC path keeps the raw-client behavior and the REST signer sets the key locator directly (no raw gRPC client). Verified with unit tests for the config and the REST adapters (unary + streaming) and a systest (TestLNDRESTBackendListVTXO) that runs a REST-backed daemon against the harness lnd REST port end to end.
The lnd REST wallet backend added in the previous commit was only covered by unit tests plus one systest that seeds and lists a VTXO (connect + chain backend, no signing). Add a CI configuration that reruns the signing-capable directed-send flows against the REST backend so the signer, walletkit, chainkit, and streaming chain-notifier are all exercised end to end over REST. - systest: newDirectedSendFixture reads ARK_SYSTEST_LND_TRANSPORT and, when it is "rest", defaults every daemon it builds onto the lnd REST transport (a per-test config mutator still wins). Unset (or "grpc") keeps the historical gRPC path, so local `make systest` is unchanged. - Makefile: the systest / systest-verbose targets gain a case=<regexp> filter (mirroring the unit target) so a run can be scoped to a subset of tests. - ci: a new systest-lnd-rest job sets ARK_SYSTEST_LND_TRANSPORT=rest and runs the fixture-driven send / OOR / on-chain / refresh / leave / vHTLC-recovery flows (case='Send|LNDREST|Stranded|VHTLCRecovery'). A single db backend (sqlite) is used since the wallet transport is orthogonal to the DB; the remaining systests do not use the lnd backend, so rerunning them over REST would add nothing. Verified locally: all eight fixture-driven flows pass over REST, and the daemon log confirms it connects to lnd's REST gateway port rather than the gRPC port.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a REST-based transport option for connecting to lnd, allowing the daemon to communicate via lnd's grpc-gateway instead of native gRPC. It includes a new lndrest package that implements the necessary service interfaces, updates the waved server to support this transport via a new lnd.transport configuration, and adds comprehensive system tests to verify the REST backend functionality. My feedback focuses on the use of context.TODO() in SignOutputRaw, suggesting that the input.Signer interface be updated to accept a context for better traceability and cancellation propagation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| c.logger(context.TODO()).DebugS( | ||
| context.TODO(), | ||
| "Signing output raw via LND remote signer", | ||
| slog.Int("input_index", signDesc.InputIndex), | ||
| slog.String("sign_method", signDesc.SignMethod.String()), | ||
| ) | ||
|
|
||
| lndDesc := inputDescToLndclient(signDesc) | ||
|
|
||
| // Collect prevouts for taproot sighash computation. When the | ||
| // caller provides a PrevOutputFetcher we extract every input's | ||
| // previous output; otherwise we fall back to the single output | ||
| // in the sign descriptor. | ||
| prevOuts := prevOutputsFromDesc(tx, signDesc) | ||
|
|
||
| sigs, err := c.signer.SignOutputRawWithLocator( | ||
| context.Background(), tx, []*lndclient.SignDescriptor{ |
There was a problem hiding this comment.
The SignOutputRaw method currently uses context.TODO() for logging and context.Background() for the signing call. This is because the input.Signer interface it implements doesn't accept a context, which hinders traceability and proper cancellation propagation.
While changing the upstream input.Signer interface might be a larger effort, it would be the ideal solution. A possible change would be:
// In input.Signer
SignOutputRaw(ctx context.Context, tx *wire.MsgTx, signDesc *input.SignDescriptor) (input.Signature, error)This would allow you to pass a proper context through the call chain, improving observability and ensuring that operations can be cancelled correctly. If changing the interface is out of scope, consider adding a comment explaining why context.TODO() is used here.
|
@claude review this |
|
Claude finished @ellemouton's task in 4m 13s —— View job Review: REST transport for the lnd wallet backendRead the full diff ( A few things worth addressing before merge. 1. Network-name check will reject
|
Three fixes from the PR review: - lndrest: compare lnd's reported network against the configured network in lnd's naming convention instead of chaincfg.Params.Name. lnd reports the testnet3 network as "testnet" while chaincfg names it "testnet3", so the old check rejected a correctly-configured testnet node at connect time (regtest/mainnet/signet happen to match, which is why the systests never caught it). The comparison is now a small networkMismatch helper with a unit test covering the testnet3 case. - waved: require lnd.macaroonpath in Config.Validate() when lnd.transport is rest, so the missing-macaroon failure surfaces at config-validation time rather than later at connect time. - lndrest: decode the NextAddr response against the configured chain params rather than nil, matching ImportTaprootScript and rejecting a network-mismatched address.
|
@ellemouton, remember to re-request review from reviewers when ready |
1 similar comment
|
@ellemouton, remember to re-request review from reviewers when ready |
3 participants
What
Add a REST transport option for the lnd wallet backend, plus a CI
configuration that exercises it. Today
waved's lnd backend connects overgRPC only (via
lndclient, which has no REST mode). Setlnd.transport=restto talk to lnd over its grpc-gateway REST interface instead.
Why
The lnd backend was gRPC-only, and a reviewer asked whether it could also run
over REST. lnd already exposes every subserver method the backend uses over
its grpc-gateway REST interface — signer (including MuSig2), walletkit,
chainkit, and the streaming chain-notifier — so a REST backend is a matter of
adapting the existing
rpc/restclientplumbing to thelndclientinterfacesrather than a fork away from it.
What this PR does
lndrest(new package): REST implementations of thelndclientSignerClient,WalletKitClient,ChainKitClient,ChainNotifierClient(server-streaming), and the slice of
LightningClientthe daemon actuallyuses (
GetInfo/WalletBalance), built on the existingrpc/restclientHTTP + server-stream helpers. Methods the daemon never calls in lnd mode
return
errUnsupportedOverREST.waved:s.lndis abstracted behind anlndServicesseam so either agRPC (
*lndclient.GrpcLndServices) or REST (*lndrest.Backend) backend dropsin;
connectLndbranches onlnd.transport. NewLndConfig.Transport(default
grpc) +lnd.transportflag +sample-waved.confentry.Hostisreinterpreted per transport (
host:portfor gRPC, an HTTPS base URL forREST).
transport=restrequireslnd.macaroonpath, since the per-networkdefault macaroon path cannot be resolved over REST.
lndbackend: the locator-awareSignOutputRawis routed through a narrowSignerinterface so the gRPC path keeps its raw-client behavior and the RESTsigner sets the key locator directly (no raw gRPC client).
Testing / CI
lndrest.TestLNDRESTBackendListVTXOstands up a REST-backed daemonagainst the harness lnd REST gateway and lists a seeded VTXO (connect +
chain backend, end to end).
systest-lnd-rest: reruns the signing-capable directed-sendflows over REST via
ARK_SYSTEST_LND_TRANSPORT=rest(
case='Send|LNDREST|Stranded|VHTLCRecovery') — send / OOR / on-chain /refresh / leave / vHTLC-recovery — so the signer, walletkit, chainkit, and
streaming chain-notifier are exercised over REST rather than just gRPC. All
eight fixture-driven flows pass over REST locally, and the daemon log
confirms it connects to lnd's REST gateway port rather than gRPC.
Notes / follow-ups
connectLndRESTdoes not block on chain-sync /wallet-unlock (
connectLndGRPCsetsBlockUntilChainSynced/BlockUntilUnlocked). It proved reliable across the REST systests, butmatching the gRPC sync gate (poll
GetInfountilsynced_to_chain, boundedby a configurable timeout) would be the safer parity and is a natural
follow-up.
lndrest'sLightningClientembeds a nillndclient.LightningClient, so anunexpected
LightningClientcall would nil-panic; those methods areunreachable in lnd mode today.
REST-backed client would extend signing coverage against a real operator,
once this lands and the submodule is bumped.
🤖 Generated with Claude Code