-
Notifications
You must be signed in to change notification settings - Fork 113
Expand docs on LSPS2ServiceConfig::client_trusts_lsp field
#707
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
Expand docs on LSPS2ServiceConfig::client_trusts_lsp field
#707
Conversation
Previously the docs have been a bit sparse. Now that we actually implement the client-trusts-LSP flow, we should expand a bit on what the bool actually does.
|
👋 Thanks for assigning @joostjager as a reviewer! |
| /// When set, the service will delay *broadcasting* the JIT channel's funding transaction until | ||
| /// the client claimed sufficient HTLC parts to pay for the channel open. | ||
| /// | ||
| /// Note this will render the flow incompatible with clients utilizing the 'LSP-trust-client' |
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.
Perhaps make it more clear that this bool switches between the two models, and that it should be false for LSP-trust-client. Maybe an enum would have been better here?
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.
Well, the thing is that if we set this to false the client can do whatever (wait or immediately claim). While technically there are two modes, no client that I'm aware of will actually run LSP-trusts-client model. I even attempted to get it removed from the spec, but there are .. reasons .. why we can't at this point. Really, any client should just claim the HTLCs ASAP. So this flag is really only concerning the LSP, allowing to determine how strict/safe it wants to be.
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.
Why offer the boolean then if there is no demand yet and you think it shouldn't even be in the spec?
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.
Because the LSP might opt to be less strict. Some require to support 'LSP-trusts-client' from their (legal) side, but none actually expects the client to be strict and implement mempool-checking-before-claiming.
Previously the docs have been a bit sparse. Now that we actually implement the client-trusts-LSP flow, we should expand a bit on what the bool actually does.