Skip to content

swap: fund explicit Lightning routing budgets#1047

Open
bhandras wants to merge 9 commits into
mainfrom
agent/in-swap-routing-budget
Open

swap: fund explicit Lightning routing budgets#1047
bhandras wants to merge 9 commits into
mainfrom
agent/in-swap-routing-budget

Conversation

@bhandras

@bhandras bhandras commented Jul 23, 2026

Copy link
Copy Markdown
Member

What it does

Adds an explicit, client-funded Lightning routing allowance to Ark-to-Lightning swap quotes and creation.

The existing max_fee_sat remains the client's maximum accepted total fee. The new routing_fee_budget_sat is the amount the client actually authorizes the swap server to include in the vHTLC for Lightning routing. Keeping these values separate prevents a total acceptance cap from being mistaken for funded money.

Why

The swap server currently escrows only the point-in-time route estimate, while its payment path may permit a larger routing fee. A client may have declared a larger total fee cap, but that declaration does not fund the difference. Conversely, limiting payment to the point estimate can cause avoidable failures when the route changes after creation.

The intended server-side invariant is:

LND MaxFee = client-funded routing allowance + bounded server subsidy

This PR establishes the public client and wire contract needed to fund the first term explicitly.

Changes

  • Extends the swap-server RPC with:
    • an explicit routing budget on quote and create requests;
    • separate service-fee, route-estimate, and funded-budget response fields.
  • Adds InSwapOptions while preserving the existing SDK method signatures for compatibility.
  • Persists accepted service-fee and routing-budget terms with pay sessions so restart cannot infer different economics.
  • Exposes the same fee components through the daemon and prepared-send RPCs.
  • Preserves the reviewed routing allowance from PrepareSend through Send.
  • Adds wavecli send --routing-fee-budget for an explicit caller-selected allowance.
  • Persists and forwards the allowance through credit actors for mixed credit/Lightning pays.
  • Keeps old clients on the server's conservative point-estimate compatibility policy.

Commit structure

  1. Add the swap-server wire fields.
  2. Persist accepted SDK fee components.
  3. Add option-based SDK quote/create calls.
  4. Add wallet-facing RPC fields.
  5. Forward fields through the daemon swap service.
  6. Preserve the budget in prepared sends and expose the CLI flag.
  7. Persist the budget in durable credit operations.
  8. Forward it through credit-assisted pays.

Generated protobuf and sqlc changes are kept with their corresponding schema contracts. Each behavior change has focused tests.

Compatibility

All fields are additive. Existing method signatures remain available and send a zero routing budget, which asks the server to use its compatibility policy. A non-zero explicit budget fails clearly when used through a connection implementation that cannot carry it instead of silently dropping the caller's funded terms.

Verification

  • go test ./swaprpc/...
  • go test ./sdk/swaps/...
  • go test ./db/...
  • go test ./credit/...
  • go test -tags=swapruntime ./swapclientserver
  • go test -tags='wavewalletrpc swapruntime' ./swapwallet
  • go test ./cmd/wavecli/waveclicommands
  • make lint-changed-local
  • make tidy-module-check
  • make build
  • make commitmsg-lint range='origin/main..HEAD'

Follow-up

Server-side enforcement will consume these additive fields, persist a frozen payment limit, ensure operator caps apply only to server-funded subsidy, and reconcile routing-fee recovery against actual Lightning costs.

Review follow-up

  • Adds legacy-only connection tests at both SDK boundaries, locking in the guarantee that an explicit funded allowance is never silently dropped.
  • Reports only the routing allowance returned by the server in prepared-send previews; a zero or absent response is no longer replaced with the caller request.
  • Rejects persisted fee and routing-budget values that cannot fit signed BIGINT storage, while preserving the established MaxUint64 all-credit quote sentinel until it resolves to a concrete amount.
  • Documents zero as the compatibility-policy sentinel; an explicit zero allowance is not distinguishable in this additive proto3 contract.

The total fee cap expresses willingness; it does not fund LND routing.

Add explicit routing allowances and quote components.

This preserves the funding distinction across both sides.
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@bhandras
bhandras force-pushed the agent/in-swap-routing-budget branch 3 times, most recently from a601609 to af47a2c Compare July 23, 2026 18:03
@bhandras
bhandras marked this pull request as ready for review July 23, 2026 18:27
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@bhandras

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude finished @bhandras's task in 3m 12s —— View job


PR Review — swap: fund explicit Lightning routing budgets

I read the full diff against origin/main (SDK swap client/quote, swapclientserver, credit actor/registry, DB/sqlc/migrations, wavecli, and the three proto contracts). This is a clean, well-structured additive change. The WithOptions layering, the explicit "never silently drop a funded budget" fallthrough guards, and the persistence of accepted terms are all consistent and well-tested.

  • Gather context (diff, commits, key files)
  • Review SDK swap client & quote logic
  • Review swapclientserver service forwarding
  • Review credit actor/registry persistence
  • Review DB / sqlc / migrations
  • Review wavecli send flow
  • Compile findings & post review

What looks good

  • Compatibility fallthrough is airtight. In createSwap, quotePay, startPay, and both SDK quote/start methods, an explicit non-zero RoutingFeeBudgetSat on a connection/client that only implements the legacy WithCredits/QuoteInSwap interface returns a clear error rather than dropping the funded term. The legacy-only connection tests lock this in.
  • Proto field numbering is correctly additive across all three schemas (swap.proto 6/10-11, swap_client.proto 4/9-11/27-28, wallet.proto 7/16-18); nothing reuses a retired tag.
  • Preview reports the server's returned allowance, not the caller request (prepareInvoicePreviewFromQuote reads quote.GetServerFeeSat() / GetRoutingFeeBudgetSat()), matching the stated follow-up fix.
  • inSwapQuoteAccountKey nicely avoids the daemon IdentityPubKey dependency for pure-Lightning previews (maxCreditSat == 0), which is a real improvement over the old unconditional fetch.
  • Migrations (000016_credit_routing_budget, 000002_in_swap_routing_budget) are NOT NULL DEFAULT 0 and sequenced correctly, and the MaxCreditSat == ^uint64(0) sentinel is preserved in validateInSwapOptions.

Findings

1. (medium) Server-returned ServerFeeSat / RoutingFeeBudgetSat are not range-checked before int64 persistence. sdk/swaps/grpc_conn.go

In inSwapConfigFromProto, the create response's FeeSat is validated against maxInt64Uint (via validateInSwapQuote at in_swap.go:620in_swap_quote.go:306), and AmountSat is checked at grpc_conn.go:875. But resp.GetServerFeeSat() and resp.GetRoutingFeeBudgetSat() (lines 862-863 and 897-898) are copied straight into InSwapConfig with no overflow guard, then persisted as int64(s.cfg.ServerFeeSat) / int64(s.cfg.RoutingFeeBudgetSat) in store_sessions.go persist(). A server returning a component > math.MaxInt64 wraps to a negative value in durable BIGINT storage.

This is exactly the defense the "Review follow-up" bullet claims to add for persisted fee/routing-budget values — the guard just doesn't cover the create-response components. Since these are individually persisted (not derived from the already-validated FeeSat), they should get the same > maxInt64Uint rejection, e.g. inside validateInSwapQuote alongside the existing cfg.FeeSat check. Fix this →

2. (minor) paySession.routingFeeBudgetSat is not restored in paySessionFromRow. sdk/swaps/store_sessions.go:924

The restored session sets maxFeeSat: uint64(row.MaxFeeSat) but not routingFeeBudgetSat (the value only lands on cfg.RoutingFeeBudgetSat). This is harmless today because createSwap — the only reader of s.routingFeeBudgetSat — runs before the first persist(), so a restored row is always past swap creation. But it's an asymmetry with maxFeeSat; setting routingFeeBudgetSat: uint64(row.RoutingFeeBudgetSat) for consistency would prevent a latent bug if a future change ever re-drives creation from a restored session.

Non-issues I checked

  • The ^uint64(0) credit sentinel from router.prepareInvoice never reaches validateStartCreditPayRequest (which has no sentinel exemption) — the intent stores the concrete resolved saturatingAddSat(...) credit amount, not the sentinel, so the stricter credit-registry validation is fine.
  • Quote-path ServerFeeSat/EstimatedRoutingFeeSat/RoutingFeeBudgetSat are not persisted (preview only), so their lack of overflow guard is not a durability concern.
    · branch agent/in-swap-routing-budget

bhandras added 7 commits July 23, 2026 20:45
Accepted in-swap terms must survive daemon restarts.

Store the service fee and client-funded allowance with each pay session.

Do not infer accepted terms from current policy.
Old callers retain the point-estimate compatibility path.

New option calls carry a funded allowance through quote and create.

The established interfaces remain source compatible.
A total fee cap does not describe the routing allowance a wallet funds.

Carry both values through quote, start, and prepared-send RPCs.
One daemon fee cap blurred willingness and funding.

Forward explicit allowances through quote and start.

Expose accepted fee components in durable summaries.
A prepared send must use the routing allowance the user reviewed.

Keep that value in the intent and expose an explicit CLI flag.
Mixed credit pays may resume after the prepared send has disappeared.

Store their funded routing allowance with the durable credit operation.
Mixed pays cross a durable credit actor before swap creation.

Carry the allowance through it so restarts cannot drop accepted terms.
@bhandras
bhandras force-pushed the agent/in-swap-routing-budget branch from af47a2c to 63d3dc2 Compare July 23, 2026 18:48
@bhandras

Copy link
Copy Markdown
Member Author

Addressed both findings and folded the fixes into the original commits:

  • validateInSwapQuote now rejects server-returned server_fee_sat and routing_fee_budget_sat values above math.MaxInt64 before either component can reach durable BIGINT storage. The existing mismatch table covers both boundaries.
  • paySessionFromRow now restores routingFeeBudgetSat on the live session as well as cfg, and the persistence/resume test asserts the restored value.

Validated with the full sdk/swaps test package, changed-file lint, formatting, sqlc regeneration, module tidy, schema consistency, and commit-message lint. All rewritten commits are signed.

@claude review

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude finished @bhandras's task in 1m 14s —— View job


PR Review — follow-up fixes verified ✅

Both findings from the prior review are correctly resolved and folded into their original commits.

  • Confirm validateInSwapQuote overflow guard now covers ServerFeeSat / RoutingFeeBudgetSat
  • Confirm paySessionFromRow restores routingFeeBudgetSat + test asserts it
  • Scan the rest of the changed code for anything new

Finding 1 (medium) — server-returned components range-checked before int64 persistence — resolved

sdk/swaps/in_swap_quote.go:306-314 now rejects both components alongside the existing FeeSat check:

if cfg.ServerFeeSat > maxInt64Uint {
        return fmt.Errorf("in-swap server fee overflows int64 range")
}
if cfg.RoutingFeeBudgetSat > maxInt64Uint {
        return fmt.Errorf("in-swap routing fee budget overflows " +
                "int64 range")
}

This runs on the create-response path — validateInSwapQuote is called at sdk/swaps/in_swap.go:620, after both CreateInSwapWithCredits and CreateInSwap, and before persist()'s int64(...) conversions at store_sessions.go:658-661. So a server returning a component > math.MaxInt64 is now rejected before it can wrap negative in BIGINT storage. The mismatch table in in_swap_test.go covers both new boundaries (server fee overflows int64 at :587, routing fee budget overflows int64 at :596).

Finding 2 (minor) — paySessionFromRow restores routingFeeBudgetSatresolved

sdk/swaps/store_sessions.go:928 now sets it on the live session, symmetric with maxFeeSat:

maxFeeSat:           uint64(row.MaxFeeSat),
routingFeeBudgetSat: uint64(row.RoutingFeeBudgetSat),

The persistence/resume test asserts the restored value end-to-end (store_test.go:172: require.EqualValues(t, 7, resumed.routingFeeBudgetSat)), in addition to the cfg.RoutingFeeBudgetSat round-trip that was already covered.


No new issues surfaced in the re-scan. The change is consistent with the stated "never silently drop a funded budget" contract and both durability defenses now hold. LGTM.
· branch agent/in-swap-routing-budget

Drive the resumed funding reconciliation synchronously.

Avoid a five millisecond SQLite timing assumption under race tests.
@bhandras

Copy link
Copy Markdown
Member Author

The remaining unit-race failure was a pre-existing timing assumption in TestPaySessionResumeFundingGraceEventuallyRetries: the test canceled reconciliation after 5 ms and then expected SQLite persistence to have completed. Under -race, cancellation could land after the mocked send but before the store write. The new standalone commit drives exactly one resumed funding reconciliation synchronously, preserving the same grace-period and durable-session assertions without changing production behavior. It passed 50 consecutive focused race runs, the full sdk/swaps race suite, the normal suite, and commit-message lint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant