Skip to content

Conversation

@positiveblue
Copy link
Contributor

@positiveblue positiveblue commented Aug 11, 2022

For unannounced channels:

Old discussion bout unannounced channels

Askers are able to decide (and price properly) in which market they want to provide liquidity:

No preference (default)
Only announced channels
Only unannounced channels

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:

  • No preference (default)
  • Only confirmed channels
  • Only zero conf channels

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

  • LndServices minimum version has been updated if new lnd apis/fields are
    used.

Copy link
Contributor

@guggero guggero left a 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).

@positiveblue positiveblue force-pushed the zero_conf branch 2 times, most recently from 92fac40 to 87ef82e Compare August 18, 2022 06:48
@positiveblue positiveblue marked this pull request as ready for review August 18, 2022 14:57
@positiveblue positiveblue changed the title [WIP] feature: add support for zero conf channels feature: add support for zero conf channels Aug 18, 2022
@positiveblue positiveblue force-pushed the zero_conf branch 2 times, most recently from 3c4bcd3 to 1e607cb Compare August 20, 2022 16:19
Copy link
Contributor

@guggero guggero left a 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 🎉

@positiveblue positiveblue force-pushed the zero_conf branch 5 times, most recently from fa1118e to 9a97736 Compare August 22, 2022 21:30
@positiveblue positiveblue requested a review from guggero August 23, 2022 02:28
Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@dstadulis
Copy link
Contributor

Need ZC to run in subasta
Need tests to pass

@lightninglabs-deploy
Copy link
Collaborator

@Roasbeef: review reminder
@positiveblue, remember to re-request review from reviewers when ready

Copy link
Contributor

@guggero guggero left a 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.

@guggero
Copy link
Contributor

guggero commented Sep 23, 2022

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.

@positiveblue positiveblue changed the title feature: add support for zero conf channels feature: add support for unannounced and zero conf channels Sep 23, 2022
@guggero guggero self-requested a review September 27, 2022 16:38
@dstadulis
Copy link
Contributor

There are 3 new commits

Action Items:
@guggero needs to check these
when review is finished, tag roas for review

Copy link
Contributor

@guggero guggero left a 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 🎉

Copy link
Member

@Roasbeef Roasbeef left a 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.
@positiveblue positiveblue merged commit 9280be1 into master Oct 18, 2022
@guggero guggero deleted the zero_conf branch October 18, 2022 13:23
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.

6 participants