-
Notifications
You must be signed in to change notification settings - Fork 123
liquidity: add swap type and loop in fee logic #433
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
Conversation
|
Split up #419 into 2 parts for easier review. Follow up PR is up (just one unit test needs fixing) for reference of how this will be used. I intend to merge them at the same time, since this doesn't really add much functionality, just easier to split early IMO. |
arshbot
left a comment
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.
A necessary PR, love the inclusion of loop in's to the accounting stuff, that'll be useful to a lot of people. Left some minor nits and context qs
f3b6825 to
d147ac6
Compare
|
@bhandras PTAL! |
bhandras
left a comment
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.
LGTM 💯
| switch rule.Type { | ||
| case swap.TypeOut: | ||
| builder = newLoopOutBuilder(m.cfg) | ||
| restrictions = outRestrictions |
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.
nit: I'd keep the incoming outRestrictions arg with it's old name (restrictions) instead.
9925339 to
37ce888
Compare
The current wording in this function is very specific to loop out. In this commit, we refactor to use `target` and `reserve` rather than inbound and outbound so that it can be used more generically.
In preparation for adding loop in swaps, we relate liquidity rules to a specific type of swap that we want to dispatch. This allows us to use a single rule format for multiple swap types.
In preparation of supporting multiple swap types, we move our swap builder into a single place, so that we can check our `maySwap` restriction per-swap (since we'll now have different checks for different swap types. To save ourselves from making multiple calls to the loop server for the restrictions placed on each swap type, we still pass a single set of restrictions in.
Add an implementation of our swap interface which can be used for loop in, and fee estimation. For fee estimation, we always want to calculate worst case loop in fees, so that autoloop never goes over its budget. However, for loop in we can't estimate how much a timeout would cost, because we must sweep the output (can't set a limit like loop out), and fee estimation in a few hundred blocks (when we'd sweep the timeout) is totally unreliable. Instead, we use a high fee rate as our best-effort fee rate for the future. We can also be confident that that loop in swaps will succeed, since once the htlc is locked in, all that is required is for the server to sweep.
37ce888 to
8113c34
Compare
arshbot
left a comment
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.
LGTM!!
This PR does some preliminary work for adding autoloopin support. Primarily adding a swap type to our liquidity rules, and implementing some of the fees + budgeting plumbing we need.
Pull Request Checklist
release_notes.mdif your PR contains major features, breaking changes or bugfixes