Skip to content

autoloop: support static loop-ins#1119

Open
starius wants to merge 10 commits into
lightninglabs:masterfrom
starius:static-autoloop
Open

autoloop: support static loop-ins#1119
starius wants to merge 10 commits into
lightninglabs:masterfrom
starius:static-autoloop

Conversation

@starius
Copy link
Copy Markdown
Collaborator

@starius starius commented Apr 14, 2026

This adds static-address-backed loop-ins to the current autoloop engine.

What changes

  • Liquidity params now include an explicit loop-in source, so operators can choose wallet-funded loop-ins or static-address-backed loop-ins.
  • The static-address subsystem now exposes an autoloop preparation helper that selects deposits, quotes the swap, and returns a ready-to-dispatch static loop-in request.
  • The liquidity planner can now produce static_loop_in suggestions, include them in forceautoloop, and enforce MaxAutoInFlight across all suggested swap types.
  • Persisted static loop-ins now count in budget accounting, in-flight accounting, and peer traffic backoff.
  • SuggestSwaps now exposes static loop-in suggestions through a dedicated RPC field and includes the static no-candidate disqualification reason.
  • The missing public mapping for AUTO_REASON_CUSTOM_CHANNEL_DATA is added, so custom asset channels are reported as disqualified instead of breaking the RPC response.
  • Static loop-in label validation now happens at the external RPC boundary, while internal autoloop dispatch can still use reserved autoloop labels.

Important design choices

  • This is intentionally a narrow first step: static autoloop uses full deposits only.
  • No partial static loop-ins are attempted, so no change output is created.
  • Static autoloop always uses fast=false.
  • There is no silent fallback from static-address loop-ins to wallet-funded loop-ins.
  • If a peer deficit can only be satisfied by a partial static loop-in, the planner returns no static candidate for that peer.
  • Static loop-ins are returned separately from legacy loop_in suggestions.
  • Failed static loop-ins are still accounted pessimistically because exact realized static swap cost is not persisted yet.
  • The main planner still excludes channels with custom channel data; this series just makes that exclusion visible and safe at the RPC layer.

Validation

  • Unit coverage was added for loop-in source params, static autoloop preparation, no-change deposit selection, static fee/weight helpers, accounting with persisted static loop-ins, RPC encoding, and planner behavior.
  • The feature was also validated end to end in regtest: a confirmed 1,000,000 sat static deposit was suggested through suggestswaps, dispatched through forceautoloop, completed successfully, moved the deposit to LOOPED_IN, increased outbound liquidity on the target BTC peer, and was not suggested again afterward.

Pull Request Checklist

  • Update release_notes.md if your PR contains major features, breaking changes or bugfixes

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces support for static-address-backed loop-ins within the existing autoloop engine. It enables operators to utilize static deposits for automated liquidity management, providing a more flexible alternative to traditional wallet-funded loop-ins. The changes include updates to the liquidity planner, budget accounting, and RPC interfaces to ensure static loop-ins are treated as first-class citizens in the autoloop lifecycle while maintaining safety and consistency.

Highlights

  • Static-Address Loop-In Support: Added support for static-address-backed loop-ins in the autoloop engine, allowing operators to choose between wallet-funded and static-address-backed sources.
  • Liquidity Planner Enhancements: The liquidity planner can now suggest static loop-ins, include them in autoloop dispatch, and enforce 'MaxAutoInFlight' limits across all swap types.
  • Budget and Traffic Accounting: Persisted static loop-ins are now correctly accounted for in budget limits, in-flight swap counts, and peer traffic backoff calculations.
  • RPC and API Updates: Exposed static loop-in suggestions via a new RPC field and added validation for static loop-in labels at the RPC boundary.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for static-address loop-in swaps within the autoloop planner. It adds a new configuration option to select the loop-in source, updates the liquidity manager to handle static loop-in snapshots for budget and traffic accounting, and includes necessary changes to the RPC interface and storage layer to support these swaps. I have no feedback to provide.

@starius starius force-pushed the static-autoloop branch 2 times, most recently from dcb8742 to 4f42af0 Compare April 14, 2026 09:05
@starius starius marked this pull request as ready for review April 15, 2026 05:23
@starius starius requested a review from hieblmi April 15, 2026 05:23
@hieblmi
Copy link
Copy Markdown
Collaborator

hieblmi commented May 6, 2026

@claude please review

@claude
Copy link
Copy Markdown

claude Bot commented May 6, 2026

Code Review: autoloop: support static loop-ins

This is a well-structured feature addition with clear separation of concerns, solid backward compatibility, and good test coverage. The design choices documented in the PR description are sensible — especially the intentional narrow scope (full deposits only, no change, no fallback to wallet loop-ins). Below are my findings organized by severity.


Medium: HtlcTxFeeRate=0 causes under-counted pending fee reservation

File: loopd/utils.go, liquidity/liquidity.go (checkExistingAutoLoopsWithStatic)

HtlcTxFeeRate on a StaticAddressLoopIn is only set once the server proposes the signed HTLC transactions — it's zero during the Initiating state. The pending-swap branch in checkExistingAutoLoopsWithStatic passes this zero rate directly to staticLoopInWorstCaseFees:

summary.pendingFees += staticLoopInWorstCaseFees(
    in.NumDeposits, in.HasChange, in.QuotedSwapFee,
    in.HtlcTxFeeRate,  // zero until HTLC is proposed
    defaultLoopInSweepFee,
)

With htlcFeeRate=0, staticLoopInOnchainFee returns only the sweep fee, so max(sweepFee, swapFee) reduces to just swapFee — the HTLC publishing fee is missed. During the short window a swap is in Initiating, the planner may not correctly reserve budget for the HTLC weight cost, potentially over-dispatching if budget is very tight.

Suggestion: Fall back to defaultLoopInSweepFee (or a similar safe constant) when HtlcTxFeeRate is zero, matching the dry-run estimation already used for new suggestions.


Medium: Branch-and-bound in selectNoChangeDeposits has O(2^N) worst case

File: staticaddr/loopin/autoloop.go

The recursive search function explores the full power set of deposits. The suffix-sum pruning eliminates many branches in practice, but in adversarial cases (e.g., many deposits of equal value, or maxAmount just above the total), the pruning degenerates and the full 2^N tree is explored.

With even 30 eligible deposits per peer tick, the worst case is prohibitive. I'd recommend adding a deposit count cap before the search (e.g., if len(deposits) > 20 { deposits = deposits[:20] }) with a comment explaining the trade-off, or documenting the expected deposit count bound that keeps this safe.


Low: Pessimistic cost for terminal Failed swaps inflates budget (tracked TODO)

File: liquidity/liquidity.go (checkExistingAutoLoopsWithStatic)

The TODO comment is honest about this, but it's worth highlighting the concrete impact: a failed swap whose timeout path was never actually confirmed on-chain will consume worst-case fee budget as if it had been. This will suppress future swaps more than necessary until the AutoloopBudgetLastRefresh window advances past it. Tracking a real loopdb.SwapCost equivalent for static loop-ins would fix this in a follow-up.


Low: Label validation ordering in StaticAddressLoopIn RPC

File: loopd/swapclient_server.go (~line 2127)

The labels.Validate(req.Label) check happens after req.LastHop = &lastHop. While not a bug (both operations are safe independently), the guard against reserved labels should logically come first — validate inputs, then derive derived fields. A reader scanning top-to-bottom will wonder why validation appears after mutation.


Low: Error message discards context when converting ErrNoAutoloopCandidate

File: loopd/utils.go

if errors.Is(err, loopin.ErrNoAutoloopCandidate) {
    return nil, liquidity.ErrNoStaticLoopInCandidate
}

The original error is replaced rather than wrapped, so any context added by staticaddr/loopin about why no candidate was found is lost. fmt.Errorf("%w: %w", liquidity.ErrNoStaticLoopInCandidate, err) (Go 1.20+) would preserve both sentinel identity and diagnostic context.


Low: --loopinsource error message doesn't echo the bad value

File: cmd/loop/liquidity.go

default:
    return fmt.Errorf("unknown loopinsource value")

Operators who mistype the flag will get no indication of what they typed. fmt.Errorf("unknown loopinsource value %q (use \"wallet\" or \"static-address\")", cmd.String("loopinsource")) is a small usability improvement.


Positive observations

  • Backward compatibility is well-handled. LoopInSource defaults to wallet-funded, existing code paths are unaffected, and StaticLoopIn in SuggestSwapsResponse is an additive field.
  • Test coverage is comprehensive. TestSelectNoChangeDeposits hits all documented tie-breakers. TestAutoLoopDispatchesStaticLoopIn, traffic accounting tests, and the planner-level tests with mocked callbacks cover the integration seams well.
  • checkExistingAutoLoopscheckExistingAutoLoopsWithStatic refactor is clean — removing the context.Context parameter that was only threaded through for the ListStaticLoopIn call simplifies callers.
  • The double-append pattern for nextExcluded (append(append([]string(nil), ...), ...)) correctly avoids aliasing; a make+two-appends version would be more idiomatic but this is fine.
  • RPC label boundary guard (labels.Validate in StaticAddressLoopIn) is a good defensive addition. Internal autoloop dispatch correctly bypasses this via the non-RPC path.
  • ReasonCustomChannelData fix is a good catch — the missing mapping was breaking SuggestSwaps responses silently.

Checklist reminder

The PR checklist notes release_notes.md is not yet updated. Given this adds a new --loopinsource CLI flag, a new LoopInSource proto enum, a new StaticLoopIn RPC field in SuggestSwapsResponse, and new AutoReason values, a release notes entry seems warranted.

@hieblmi
Copy link
Copy Markdown
Collaborator

hieblmi commented May 6, 2026

There is a finding regarding time consuming deposit selection:

[P1] staticaddr/loopin/autoloop.go:198 can do exponential subset search over deposits while building a static loop-in candidate. This is on the autoloop suggestion path, so a static address with many small deposits can make the liquidity manager stall when trying to decide that no candidate exists. The simple high-impact guard is to sum filtered swappable deposits and return ErrNoAutoloopCandidate if total < minAmount; I’d also bound the number of candidates or switch away from raw DFS for impossible [min,max] ranges.

I am currently looking to tAck the PR.

Copy link
Copy Markdown
Collaborator

@hieblmi hieblmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ tACK (pending the P1 item found in #1119 (comment))

I've tested the new mode with

loop --network=regtest setparams \
  --autoloop \
  --loopinsource=static-address

and then

loop --network=regtest setparams \
  --autoloop \
  --loopinsource=static-address \
  --maxswapfee=10 \
  --maxroutingfee=10 \
  --maxprepayfee=1 \
  --maxprepay=30000 \
  --maxminer=1500000 \
  --sweeplimit=3 \
  --autobudget=200000 \
  --autoinflight=2 \
  --minamt=250000 \
  --maxamt=5000000

while setting the ticker duration to 10s.

It only fired once the minimum amount of deposits was available. It then selected up to maxamt.

Very nice work!

@hieblmi hieblmi requested a review from sputn1ck May 6, 2026 12:32
@lightninglabs-deploy
Copy link
Copy Markdown

@sputn1ck: review reminder
@starius, remember to re-request review from reviewers when ready

@hieblmi hieblmi removed the request for review from sputn1ck May 13, 2026 14:33
starius added 10 commits May 14, 2026 03:50
Add a dedicated loop-in source enum to the liquidity parameters
rpc and wire it through the internal parameter model and CLI.

This keeps the source selection explicit before any static
autoloop planning lands, so operators can choose between the
legacy wallet-funded path and a future static-address-backed
path without relying on implicit fallback behavior.
Move static loop-in label validation to the rpc boundary and
remove the same check from the internal manager path.

This keeps external requests aligned with the existing swap rpc
surface while allowing internal autoloop callers to keep using
reserved labels for automated swaps. The tests cover both sides of
that contract: rpc requests still reject reserved labels, and the
manager path accepts them.
Test-only change. This is needed to reuse it in another test.
Add the static-address helper that prepares full-deposit autoloop loop-ins
without dispatching them. The helper selects no-change deposit sets, records
explicit outpoints, and quotes the exact selected amount before the planner
tries to dispatch anything.

The tests cover the full-deposit selector, the quoted request construction,
and excluded outpoint handling so later liquidity work can rely on a stable
preparation surface.
Teach the liquidity manager to include persisted static loop-ins
in budget accounting, in-flight limits, and peer traffic backoff.
This adds the static fee model used for conservative accounting
and passes storage errors through the relevant planner helpers.

The daemon wiring now exposes static loop-ins to liquidity so the
manager can see the same ongoing swaps that the static-address
subsystem persists, while easy autoloop keeps working with the new
fallible traffic lookup path.
Extend the public rpc surface for static autoloop integration
without turning the planner on yet. SuggestSwaps responses can
now carry static-address loop-in requests and the new planner
reason for missing static candidates is mapped over rpc.
Wire static-address-backed loop-ins into the existing autoloop
planner and dispatch path. Loop-in rules can now be converted into
static candidates, prepared after global sorting, filtered with
static fee limits, and dispatched through the static manager.

This also fixes MaxAutoInFlight enforcement across all suggested
swap types and adds planner tests for missing static candidates
and mixed in-flight filtering.
Static autoloop testing surfaced a SuggestSwaps failure when the
planner disqualified a custom asset channel.

Add the missing AutoReason enum value and handle ReasonCustomChannelData.
Replace the recursive full-deposit autoloop selector with a bounded-memory
DP implementation in staticaddr/loopin/autoloop_dp.go. The new selector
keeps the existing no-change semantics, first finds the best reachable
total, then applies the 25 percent band rule so earlier-expiring deposits
can win inside that near-optimal range.

The DP table is capped at 128 MiB and keeps exact satoshi sums alongside
compressed bucket weights, so planning stays memory-bounded without
allowing oversized candidates. The compressed weighting now rounds down
with a minimum of one bucket, which avoids rejecting valid sums after
multiple per-deposit rounding steps while leaving the exact-sum check
as the real safety boundary.
@starius starius force-pushed the static-autoloop branch from 4f42af0 to 6f82494 Compare May 14, 2026 08:50
@starius
Copy link
Copy Markdown
Collaborator Author

starius commented May 14, 2026

There is a finding regarding time consuming deposit selection:

[P1] staticaddr/loopin/autoloop.go:198 can do exponential subset search over deposits while building a static loop-in candidate. This is on the autoloop suggestion path, so a static address with many small deposits can make the liquidity manager stall when trying to decide that no candidate exists. The simple high-impact guard is to sum filtered swappable deposits and return ErrNoAutoloopCandidate if total < minAmount; I’d also bound the number of candidates or switch away from raw DFS for impossible [min,max] ranges.

I am currently looking to tAck the PR.

Great catch!

I replaced the exponential search with a DP over deposits. It uses up to 128 MiB of RAM for the candidate table. When the number of deposits is modest, it returns exact results, i.e. the same results the exponential search would have returned. With a very large number of deposits, it gradually reduces accuracy by increasing the bucket size from 1 satoshi to 2 satoshis, and so on.

I also changed the objective function. The selector first finds the maximum total within the [min, max] range, using fewer deposits as a tie-breaker. It then looks at the band [best - (best - min) / 4, best] and chooses the best subset by expiry within that band. The goal is to keep larger swaps preferred while still trying to consume older, earlier-expiring deposits when the amount trade-off is small.

I also applied these earlier Claude findings:

  • fix zero-HTLC-feerate fallback
  • improve the CLI error for an invalid --loopinsource value
  • reorder label validation

@starius starius requested a review from hieblmi May 14, 2026 09:00
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.

4 participants