-
Notifications
You must be signed in to change notification settings - Fork 47
feature: add support for unannounced and zero conf channels #385
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
2251893 to
8630920
Compare
guggero
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.
Code looks pretty good, though I was unable to test with current lnd master (0.15.0-beta commit=v0.15.0-beta-226-gd0d4782c3).
92fac40 to
87ef82e
Compare
3c4bcd3 to
1e607cb
Compare
guggero
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.
Almost there, just two more small requests. Was able to test end to end this time around, nice work 🎉
fa1118e to
9a97736
Compare
guggero
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 🎉
9ed7956 to
53eafa5
Compare
53eafa5 to
403cab8
Compare
|
Need ZC to run in subasta |
|
@Roasbeef: review reminder |
403cab8 to
e519dd9
Compare
guggero
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.
Nice work, LGTM 🎉
Just a few typos and style nits, nothing major. Going to take a look at the server PRs now, might need to circle back if there's something that needs to be adjusted in the client PR.
|
Oh, I think we need to consider which of the two fields (on the bid side, probably both) we need to add to the sidecar ticket? Assuming this might be something that could be offered through the sidecar flow. |
e519dd9 to
38cfec2
Compare
38cfec2 to
7d97d79
Compare
|
There are 3 new commits Action Items: |
guggero
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.
Sidecar stuff looks good, just two small comments.
Was able to test sidecars with both an old Pool client (and none of the two flags set) and a private channel through a ticket with a new Pool client 🎉
a59070e to
7b797c3
Compare
Roasbeef
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 🎊
I really dig the sidecar additions as well! I wager unannounced sidecar channels will soon become the new default/norm.
For unannounced channels: Bids can decide if they are interested in announced or unannounced channels (exclusive or). Asks can decide in what market they want to provide liquidity: announced channels, unannounced channels or both. For zero conf channels: New constraints based on how many blocks are needed before considering a channel "confirmed".
Includes the definition of matchable orders based on the new constraint fields.
bonus: fix style for some +80 column lines
Check that we cannot submit inconsistent orders: - Orders that may result in zero conf channels without supporting them.
A sidecar ticket will always use the minimum version needed to encode all the relavant information for that offer. In that way the tickets from a provider with a newer version can be redeemed by recipeients using older versions as long as the ticket does not encode new functionalities.
8cf19cb to
9ce8d4b
Compare
For unannounced channels:
Old discussion bout unannounced channels
Askers are able to decide (and price properly) in which market they want to provide liquidity:
Bidders can define what kind of channels they are interested in: announced or unannounced (exclusive or)
For zero conf:
Askers are able to decide (and price properly) in which market they want to provide liquidity:
Bidders can define what kind of channels they are interested in: confirmed/zero conf (exclusive or)
In order to match zero conf orders the client needs to be running in a batch version that support this feature at runtime (during the match).
The channel acceptor works a bit different than the one for unannounced channels. More in LND zero conf channels docs
Pull Request Checklist
used.