Skip to content
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

Bump max inflight HTLC value to 100% if we're an LSPS2 client #262

Merged

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented Feb 26, 2024

When we're an LSPS2 client, the LSP might open the channel to us with a capacity only barely more than the initial payment size. We therefore need to be able to receive close to 100% of the channel size in a single HTLC.

Here we therefore bump the default max HTLC value inflight config to 100%. We should eventually be able to set this on a per-channel basis, but for now we just bump or default for all channels.

@tnull tnull requested a review from jkczyz February 26, 2024 08:15
@tnull tnull force-pushed the 2024-02-bump-inflight-when-lsps-client branch 3 times, most recently from 997c571 to 7a64a76 Compare February 26, 2024 08:19
@tnull tnull mentioned this pull request Feb 26, 2024
src/builder.rs Outdated Show resolved Hide resolved
src/builder.rs Outdated
Comment on lines 707 to 709
// FIXME: When we're an LSPS2 client, set maximum allowed inbound HTLC value in flight
// to 100%. We should eventually be able to set this on a per-channel basis, but for
// now we just bump or default for all channels.
Copy link

Choose a reason for hiding this comment

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

Should we note somewhere in user docs that this affects non-JIT channels?

Copy link
Collaborator Author

@tnull tnull Feb 26, 2024

Choose a reason for hiding this comment

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

Hmm, I'd rather not honestly, as communicating what risks this entails requires to get super technical and arguably the inflight value is much too low for non-forwarding nodes anyways (and we can probably assume a node using LSPS2 JIT invoices won't be a central routing node in the network). Still, I want to get this fixed soon, hopefully even by 0.0.122.

@tnull tnull force-pushed the 2024-02-bump-inflight-when-lsps-client branch from 7a64a76 to 1723d09 Compare February 26, 2024 17:57
When we're an LSPS2 client, the LSP might open the channel to us with a
capacity only barely more than the initial payment size. We therefore
need to be able to receive close to 100% of the channel size in a single
HTLC.

Here we therefore bump the default max HTLC value inflight config to
100%. We should eventually be able to set this on a per-channel basis,
but for now we just bump the default for all channels.
@tnull tnull force-pushed the 2024-02-bump-inflight-when-lsps-client branch from 1723d09 to 2a85149 Compare February 26, 2024 17:57
@tnull
Copy link
Collaborator Author

tnull commented Feb 26, 2024

Force pushed with the following change:

diff --git a/src/builder.rs b/src/builder.rs
index cfc6c59..cc0c9ad 100644
--- a/src/builder.rs
+++ b/src/builder.rs
@@ -707,5 +707,5 @@ fn build_with_store_internal<K: KVStore + Sync + Send + 'static>(
                // FIXME: When we're an LSPS2 client, set maximum allowed inbound HTLC value in flight
                // to 100%. We should eventually be able to set this on a per-channel basis, but for
-               // now we just bump or default for all channels.
+               // now we just bump the default for all channels.
                user_config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel =
                        100;

@tnull tnull merged commit 87c0266 into lightningdevkit:main Feb 26, 2024
7 of 13 checks passed
This pull request was closed.
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.

2 participants