-
Notifications
You must be signed in to change notification settings - Fork 124
liquidity: scriptable autoloop #1069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Add go.starlark.net for Starlark scripting support. Starlark is a Python-like language designed for configuration and scripting, originally developed for Bazel. This will be used for scriptable autoloop to allow custom swap decision logic.
Add a new package for evaluating Starlark scripts in the autoloop context. This includes: - context.go: Types exposed to scripts (ChannelInfo, PeerInfo, SwapRestrictions, BudgetInfo, InFlightInfo, AutoloopContext, SwapDecision) with Starlark Value interface implementations - builtins.go: Helper functions loop_out() and loop_in() for creating swap decisions in scripts - starlark.go: Evaluator for executing scripts with context data - starlark_test.go: Unit tests for script evaluation Starlark provides Python-like syntax with variables, functions, list comprehensions, and sorting - making complex swap logic readable and maintainable compared to expression languages.
Add scriptable autoloop as a new mode alongside easy autoloop and
threshold rules. This allows users to provide custom Starlark scripts
that have full control over swap decisions.
Key changes:
- scriptable.go: ScriptableManager that evaluates user scripts on
each tick, builds context from current channel/peer state, and
converts script decisions to swap suggestions
- parameters.go: Add ScriptableAutoloop, ScriptableScript, and
ScriptableTickInterval parameters with validation to ensure
scriptable mode is mutually exclusive with easy autoloop and rules
- liquidity.go: Wire up scriptable manager to the autoloop system
- script_equivalence_test.go: Tests verifying Starlark scripts can
replicate easy-autoloop behavior plus advanced scenarios
The scriptable mode gives operators fine-grained control with
readable Python-like syntax:
def autoloop():
eligible = [c for c in channels if c.active]
eligible = sorted(eligible, key=lambda c: -c.local_balance)
if eligible and total_local > 100000:
return [loop_out(50000, [eligible[0].channel_id])]
return []
decisions = autoloop()
Add new fields to LiquidityParameters for scriptable autoloop: - scriptable_autoloop (bool): Enable scriptable autoloop mode - scriptable_script (string): The Starlark script content - scriptable_tick_interval_sec (uint64): Custom tick interval These fields allow clients to configure scriptable autoloop via the SetLiquidityParams RPC. The script is validated on the server side before being accepted.
Summary of ChangesHello @sputn1ck, 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 significantly enhances the liquidity management capabilities by introducing a highly flexible 'scriptable autoloop' mode. This new mode empowers operators to define sophisticated, custom swap strategies using Starlark, a Python-like scripting language. By providing a rich set of contextual data and built-in functions to these scripts, users can implement complex decision-making processes that go beyond the limitations of previous rule-based systems, leading to more tailored and efficient liquidity rebalancing. Highlights
🧠 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 AssistThe 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
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 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
727d7e6 to
c4d1f3e
Compare
There was a problem hiding this 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 a powerful new feature: scriptable autoloop using Starlark. This allows for much more flexible and complex autoloop logic than the existing modes. The implementation is comprehensive, covering CLI, RPC, and the core liquidity management logic. I've identified a few areas for improvement, primarily concerning performance and thread safety, which are critical for a long-running daemon process. Overall, this is a great addition.
| var scriptEvaluator *script.Evaluator | ||
|
|
||
| // getScriptEvaluator returns a cached Starlark evaluator, creating one if | ||
| // needed. | ||
| func getScriptEvaluator() (*script.Evaluator, error) { | ||
| if scriptEvaluator != nil { | ||
| return scriptEvaluator, nil | ||
| } | ||
| eval, err := script.NewEvaluator() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| scriptEvaluator = eval | ||
| return scriptEvaluator, nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getScriptEvaluator function is not safe for concurrent use. If two goroutines call it simultaneously while scriptEvaluator is nil, both could enter the if block, leading to a race condition where two evaluators are created and one overwrites the other. This should be protected using sync.Once to ensure the evaluator is initialized only once. You'll also need to import the sync package.
var (
scriptEvaluator *script.Evaluator
evaluatorOnce sync.Once
evaluatorErr error
)
// getScriptEvaluator returns a cached Starlark evaluator, creating one if
// needed.
func getScriptEvaluator() (*script.Evaluator, error) {
evaluatorOnce.Do(func() {
scriptEvaluator, evaluatorErr = script.NewEvaluator()
})
return scriptEvaluator, evaluatorErr
}| // Get the first channel to determine the peer. | ||
| chanID := d.ChannelIDs[0] | ||
| var peerPubkey route.Vertex | ||
| for _, ch := range scriptCtx.Channels { | ||
| if ch.ChannelID == chanID { | ||
| pubkeyBytes, err := hex.DecodeString(ch.PeerPubkey) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid peer pubkey: %w", err) | ||
| } | ||
| copy(peerPubkey[:], pubkeyBytes) | ||
| break | ||
| } | ||
| } | ||
|
|
||
| // Build outgoing channel set. | ||
| outgoing := make([]lnwire.ShortChannelID, len(d.ChannelIDs)) | ||
| for i, id := range d.ChannelIDs { | ||
| outgoing[i] = lnwire.NewShortChanIDFromInt(id) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function dispatchScriptableLoopOut determines the peer for a loop out swap based only on the first channel ID in the d.ChannelIDs list. If the script provides a list of channels belonging to different peers in a single loop_out call, this will lead to an incorrect peer being associated with the swap, likely causing it to fail. A validation should be added to ensure all channel IDs in a single loop_out decision belong to the same peer before dispatching the swap.
// Get the peer for the first channel and validate that all other
// channels in the decision belong to the same peer.
chanMap := make(map[uint64]string)
for _, ch := range scriptCtx.Channels {
chanMap[ch.ChannelID] = ch.PeerPubkey
}
firstPeerPubkey, ok := chanMap[d.ChannelIDs[0]]
if !ok {
return fmt.Errorf("channel %d not found", d.ChannelIDs[0])
}
pubkeyBytes, err := hex.DecodeString(firstPeerPubkey)
if err != nil {
return fmt.Errorf("invalid peer pubkey: %w", err)
}
var peerPubkey route.Vertex
copy(peerPubkey[:], pubkeyBytes)
outgoing := make([]lnwire.ShortChannelID, len(d.ChannelIDs))
for i, chanID := range d.ChannelIDs {
peer, ok := chanMap[chanID]
if !ok {
return fmt.Errorf("channel %d not found", chanID)
}
if peer != firstPeerPubkey {
return fmt.Errorf("all channels in a loop out decision "+
"must belong to the same peer",
)
}
outgoing[i] = lnwire.NewShortChanIDFromInt(chanID)
}| func (e *Evaluator) Evaluate(script string, | ||
| ctx *AutoloopContext) ([]SwapDecision, error) { | ||
|
|
||
| // Build the predeclared globals with context data and builtins. | ||
| globals := e.buildGlobals(ctx) | ||
|
|
||
| // Create a new thread for execution. | ||
| thread := &starlark.Thread{Name: "autoloop"} | ||
|
|
||
| // Execute the script using the new API with FileOptions. | ||
| result, err := starlark.ExecFileOptions( | ||
| &syntax.FileOptions{}, | ||
| thread, "autoloop.star", script, globals, | ||
| ) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("Starlark execution error: %w", err) | ||
| } | ||
|
|
||
| // Extract decisions from the result. | ||
| return extractDecisions(result) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Evaluate function re-parses and re-compiles the Starlark script on every call, which is inefficient for a function called on a ticker. The Evaluator struct includes a cache that should be used to store the compiled *starlark.Program.
You can implement caching by:
- Generating a hash of the script content to use as a cache key.
- On
Evaluate, check if a compiled program for that key exists in the cache. - If not, parse and compile the script using
syntax.Parseandstarlark.Compiler{File: f}.Compile(), then store the resulting*starlark.Programin the cache. - Execute the cached (or newly compiled) program using
prog.Init(thread, globals).
This will avoid the overhead of compilation on every tick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds scriptable autoloop functionality that allows operators to write custom swap decision logic using Starlark scripts (Python-like syntax) instead of CEL expressions. The feature provides a more powerful alternative to easy autoloop and threshold rules.
Changes:
- Added Starlark-based scripting engine with context types (channels, peers, restrictions, budget, in-flight info) and helper functions (loop_out, loop_in)
- Integrated scriptable autoloop mode into the liquidity manager with parameter validation ensuring mutual exclusivity with easy autoloop and threshold rules
- Extended RPC/CLI interfaces with scriptable configuration fields including script content, tick interval, and enable flag
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| looprpc/client.proto | Added protobuf fields for scriptable autoloop configuration (enabled flag, script content, tick interval) |
| looprpc/client.pb.go | Generated Go code from protobuf definitions with getter methods for new scriptable fields |
| looprpc/client.swagger.json | Updated Swagger/OpenAPI documentation with scriptable autoloop field descriptions |
| liquidity/scriptable.go | Core implementation: script evaluation, context building, swap dispatching for scriptable mode |
| liquidity/script/context.go | Starlark context types implementing starlark.Value interface (ChannelInfo, PeerInfo, etc.) |
| liquidity/script/starlark.go | Starlark evaluator with script execution, decision extraction, and validation logic |
| liquidity/script/builtins.go | Helper functions loop_out() and loop_in() for creating swap decisions in scripts |
| liquidity/script_equivalence_test.go | Tests verifying Starlark scripts can replicate easy-autoloop behavior |
| liquidity/script/starlark_test.go | Unit tests for Starlark evaluation, filtering, sorting, and validation |
| liquidity/parameters.go | Parameter validation ensuring scriptable mode exclusivity and script requirement |
| liquidity/liquidity.go | Integration of scriptable autoloop into main event loop with priority over other modes |
| cmd/loop/liquidity.go | CLI flags for enabling scriptable mode and providing script via file or inline |
| go.mod | Added go.starlark.net dependency |
| go.sum | Checksums for go.starlark.net and updated grpc/genproto dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Name: "scriptautoloop", | ||
| Usage: "set to true to enable scriptable autoloop " + | ||
| "using Starlark scripts. This allows custom " + | ||
| "swap logic with variables, functions, loops, " + | ||
| "and sorting. Mutually exclusive with " + | ||
| "easyautoloop and threshold rules.", |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flag name 'scriptautoloop' is inconsistent with the naming pattern used elsewhere in the codebase and the PR. Throughout the protobuf definitions, Go structs, and documentation, the feature is called 'scriptable_autoloop' (with 'able'). The CLI flag should be '--scriptableautoloop' to match this convention and make the feature name consistent across the entire codebase.
| // ScriptableAutoloop enables Starlark-based scriptable autoloop mode. | ||
| // This mode is mutually exclusive with EasyAutoloop and threshold rules. |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on line 132 incorrectly states this enables "CEL-based scriptable autoloop" when the feature actually uses Starlark, not CEL. The PR description explicitly explains why Starlark was chosen over CEL. This comment should say "Starlark-based scriptable autoloop" to be consistent with the actual implementation and the rest of the codebase.
| // scriptEvaluator is a cached Starlark evaluator for scriptable autoloop. | ||
| var scriptEvaluator *script.Evaluator |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global variable 'scriptEvaluator' creates a shared mutable state that could lead to race conditions if multiple Manager instances exist or if the manager is accessed concurrently. Consider either: (1) making this a field on the Manager struct instead of a package-level global, or (2) adding proper synchronization (mutex) around access to this cached evaluator. The current implementation could result in data races if getScriptEvaluator is called concurrently.
| func getScriptEvaluator() (*script.Evaluator, error) { | ||
| if scriptEvaluator != nil { | ||
| return scriptEvaluator, nil | ||
| } | ||
| eval, err := script.NewEvaluator() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| scriptEvaluator = eval | ||
| return scriptEvaluator, nil | ||
| } |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getScriptEvaluator function has a race condition. If called concurrently by multiple goroutines, both could see scriptEvaluator as nil and create separate evaluator instances. While not necessarily harmful for this use case, it defeats the purpose of caching and could lead to unexpected behavior. Add proper synchronization using sync.Once or a mutex to ensure thread-safe lazy initialization.
| scriptCtx.InFlight.TotalCount++ | ||
|
|
||
| case script.SwapTypeLoopIn: | ||
| err := m.dispatchScriptableLoopIn(ctx, d) | ||
| if err != nil { | ||
| log.Errorf("scriptable autoloop: loop in "+ | ||
| "dispatch failed: %v", err) | ||
| continue | ||
| } | ||
| scriptCtx.InFlight.TotalCount++ |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The InFlight.TotalCount is being incremented directly in the scriptCtx after dispatching swaps, but this mutates the context that was built earlier. If a dispatch fails partway through, the TotalCount will be inconsistent with the actual number of successful dispatches. Consider either: (1) tracking successful dispatches in a separate variable, or (2) rebuilding the context after each successful dispatch if accurate in-flight counts are critical for subsequent decisions.
| // Get the first channel to determine the peer. | ||
| chanID := d.ChannelIDs[0] | ||
| var peerPubkey route.Vertex | ||
| for _, ch := range scriptCtx.Channels { | ||
| if ch.ChannelID == chanID { | ||
| pubkeyBytes, err := hex.DecodeString(ch.PeerPubkey) | ||
| if err != nil { | ||
| return fmt.Errorf("invalid peer pubkey: %w", err) | ||
| } | ||
| copy(peerPubkey[:], pubkeyBytes) | ||
| break | ||
| } | ||
| } |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After determining the peerPubkey from the first channel, there's no validation that the peerPubkey was actually found. If none of the channels in scriptCtx.Channels match the requested chanID, peerPubkey will remain zero-valued but the code continues to use it. This could lead to dispatching a swap with an invalid peer. Add a check after the loop to return an error if the channel was not found in the context.
|
|
||
| // ScriptableTickInterval overrides the default tick interval for | ||
| // scriptable mode. Zero means use DefaultAutoloopTicker. | ||
| ScriptableTickInterval time.Duration |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ScriptableTickInterval parameter is defined and passed through RPC but is never actually used in the implementation. The scriptableAutoLoop function doesn't use a custom ticker based on this interval - it just uses the same ticker as other autoloop modes. Either implement the custom tick interval functionality by creating a separate ticker when ScriptableTickInterval is set, or remove this unused parameter from the API to avoid misleading users.
| // ScriptableTickInterval overrides the default tick interval for | |
| // scriptable mode. Zero means use DefaultAutoloopTicker. | |
| ScriptableTickInterval time.Duration |
| type Evaluator struct { | ||
| // Cache compiled programs by script hash. | ||
| cacheMu sync.RWMutex | ||
| cache map[string]*starlark.Program |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Evaluator has a cache field (cacheMu and cache) that is never actually used in the Evaluate method. The cache is defined and can be cleared via ClearCache(), but the Evaluate method always executes scripts without checking or populating the cache. Either implement the caching logic to improve performance for repeated script evaluations, or remove the unused cache fields and ClearCache method to avoid confusion.
| // ScriptableScript is the Starlark script to evaluate on each tick. | ||
| // Required when ScriptableAutoloop is true. |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on line 136 incorrectly refers to "CEL expression" when the field actually contains a Starlark script. This should be updated to "Starlark script" to match the actual implementation and be consistent with the field name "ScriptableScript" and all other documentation in the PR.
| // ScriptableScript is the Starlark script to evaluate on each tick. | |
| // Required when ScriptableAutoloop is true. | |
| // ScriptableScript is the Starlark script source to evaluate on each tick, | |
| // and is required when ScriptableAutoloop is true. |
da10903 to
ee08aea
Compare
Add new flags to 'loop setparams' for configuring scriptable autoloop:
- --scriptautoloop: Enable/disable scriptable autoloop mode
- --scriptfile: Path to a Starlark script file (recommended for
production - enables version control, syntax highlighting, and
proper editing workflows)
- --script: Inline Starlark script for simple cases
- --scripttickinterval: Custom tick interval in seconds
Example usage:
# Enable with script file (recommended)
loop setparams --scriptautoloop --scriptfile /path/to/autoloop.star
# Enable with inline script
loop setparams --scriptautoloop --script 'decisions = []'
# Disable
loop setparams --scriptautoloop=false
The --scriptfile and --script flags are mutually exclusive. Scriptable
autoloop cannot be used together with easy autoloop.
ee08aea to
b62e81b
Compare
Summary
This PR adds scriptable autoloop - a new autoloop mode that allows operators to write custom swap decision logic using Starlark scripts (Python-like syntax).
Example Script
CLI Usage
Changes
go.starlark.netdependencyTest plan
liquidity/script/)