Skip to content

rpcserver: use an independent bounded context for post-funding lease cleanup in CommitVirtualPsbts #2206

Description

@darioAnongba

Problem

CommitVirtualPsbts asks lnd to fund the anchor transaction, which leases (locks) the selected on-chain UTXOs. If anything after funding fails, a deferred cleanup is supposed to release those leases so the coins become available again. That cleanup, however, runs on the same context.Context that the gRPC request carries. When the request context is canceled — client-side deadline exceeded, client disconnect, or an upstream cancellation — the cleanup fires but does nothing, because every ReleaseOutput call it makes is issued on the already-canceled context and returns immediately with context canceled. The net result is leaked/locked UTXOs that stay unspendable until the lock expiration elapses (LockExpirationSeconds, up to lnd's max of 10 minutes by default).

This matters most for lightweight/custom-anchor integrations (the tap-sdk advanced builder used by SwapDK), where the caller owns the BTC anchor transaction and drives CommitVirtualPsbts remotely. Remote callers are exactly the ones that hit request-context cancellation (network blips, timeouts), and they cannot see or clean up the lnd-side leases themselves — the whole point of the deferred cleanup is to compensate for a request that is being torn down, so it must not itself depend on that request's lifecycle.

Current behaviour

The handler receives the request context and funds via lnd, capturing the leased UTXOs:

  • rpcserver/rpcserver.go:3011func (r *RPCServer) CommitVirtualPsbts(ctx context.Context, ...); ctx is the gRPC request context.
  • rpcserver/rpcserver.go:3111lndWallet.FundPsbt(ctx, fundRequest) returns lockedUTXO, the coins lnd just locked for us.

The cleanup is a defer guarded by a success flag, and it releases each lease using the request ctx:

  • rpcserver/rpcserver.go:3131-3147 — the deferred cleanup loop.
  • rpcserver/rpcserver.go:3141err := lndWallet.ReleaseOutput(ctx, lockID, op) — the release is issued on the request context.
  • rpcserver/rpcserver.go:3223success = true is only set on the fully-successful path, so any early error return between funding and here invokes the cleanup.

Why the cleanup is a no-op once ctx is canceled: ReleaseOutput derives its RPC context from the passed-in context, so a dead parent context yields a dead child:

  • lndclient@v0.21.0-1 walletkit_client.go:168-183func (m *walletKitClient) ReleaseOutput(ctx, ...) does rpcCtx, cancel := context.WithTimeout(ctx, rpcTimeout) and then calls m.client.ReleaseOutput(rpcCtx, ...). If ctx is already canceled, rpcCtx is born canceled and the gRPC call fails immediately; the lease is never released.

So the leak occurs whenever the handler returns before line 3223 with success == false while ctx is canceled — e.g. the request deadline expires (or the client disconnects) after FundPsbt locked the UTXOs, and any subsequent step (commitment/proof-suffix construction or serialization at rpcserver/rpcserver.go:3152-3208) then returns an error, or the cancellation itself surfaces as the error.

The same request-context-scoped release anti-pattern exists at these related sites and is worth fixing in the same pass for consistency, though CommitVirtualPsbts is the one that leaks under remote/custom-anchor use:

  • tapfreighter/wallet.go:675-700 (FundPacket deferred ReleaseCoins(ctx, ...)) and the analogous defer in FundBurn around tapfreighter/wallet.go:753-780.
  • lndservices/wallet_anchor.go:205-226 (UnlockInput), which also releases via the caller's ctx.

Proposed change

Make the deferred cleanup in CommitVirtualPsbts use an independent, bounded context that is not derived from the request context, so lease release still runs when the request is being canceled:

  • In the defer at rpcserver/rpcserver.go:3131, derive a fresh context for the release loop, e.g. releaseCtx, releaseCancel := context.WithTimeout(context.Background(), fetchTimeout); defer releaseCancel(), and call lndWallet.ReleaseOutput(releaseCtx, lockID, op).
  • fetchTimeout (rpcserver/rpcserver.go:130, currently 30s) is already documented as "a generic timeout to use when fetching data ... during any RPC calls that don't have a parent context," which makes it the natural bound here; a small dedicated constant (e.g. leaseReleaseTimeout) would also be fine. The context must be bounded so a stuck lnd can't hang the goroutine indefinitely.

This keeps the existing success-flag semantics; only the context used for the compensating release changes. Optionally apply the same treatment to the related tapfreighter/lndservices sites listed above so abandoned batches/sends also release leases when their caller context is gone.

Context

Surfaced while building the tap-sdk advanced custom-anchor transaction builder (lightninglabs/tap-sdk#158), where the SDK owns the BTC anchor, calls CommitVirtualPsbts to commit assets, signs externally, then publishes. Because the SDK cannot release lnd-side leases itself and cannot rely on tapd's cleanup running under a canceled request context, it currently fails closed: it treats leased UTXOs as potentially leaked on any CommitVirtualPsbts timeout/cancellation and relies on the lock-expiration backstop rather than prompt release. An independent bounded cleanup context in tapd would let the SDK depend on prompt lease release instead of the expiration timeout.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    Status
    🆕 New

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions