Drop max_inbound_htlc_value_in_flight_percent_of_channel #878
Drop max_inbound_htlc_value_in_flight_percent_of_channel #878jkczyz wants to merge 4 commits intolightningdevkit:mainfrom
max_inbound_htlc_value_in_flight_percent_of_channel #878Conversation
|
👋 Hi! This PR is now in draft status. |
jkczyz
force-pushed
the
2026-04-lsp-max-in-flight
branch
2 times, most recently
from
54b2aa3 to
e09fd86
Compare
jkczyz
force-pushed
the
2026-04-lsp-max-in-flight
branch
from
e09fd86 to
e8f3575
Compare
Update to use the new HeaderCache type instead of implementing the Cache trait, pass BestBlock instead of BlockHash to synchronize_listeners, and pass HeaderCache by value to SpvClient::new. Also adapt to BestBlock gaining a previous_blocks field and ChannelManager deserialization returning BestBlock instead of BlockHash. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
UniFFI cannot represent the fixed-size array that upstream's BestBlock carries via `previous_blocks`, so NodeStatus.current_best_block was unusable from Swift, Kotlin, and Python once upstream added that field. Introduce a small ldk-node BestBlock with just hash and height — the pieces bindings can handle — and expose it in place of the upstream type on the public API. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The max_inbound_htlc_value_in_flight_percent_of_channel config setting was used when acting as an LSPS2 service in order to forward the initial payment. However, upstream divided the config setting into two for announced and unannounced channels, the latter defaulting to 100%.
tankyleo
left a comment
tankyleo
left a comment
There was a problem hiding this comment.
Thank you I am excited to land this PR soon, a few nits
| .unannounced_channel_max_inbound_htlc_value_in_flight_percentage, | ||
| 100 | ||
| ); | ||
| debug_assert!(config.accept_forwards_to_priv_channels); |
There was a problem hiding this comment.
nit: can we add a debug_assert here on announce_for_forwarding = false ? IIUC right now we inherit announce_for_forwarding = false here because of the default in rust-lightning, but would be good to confirm this here I think.
This is to make sure that the 100% cap actually gets applied to the channel opened by the LSPS2 service.
| // If we act as an LSPS2 service, set the HTLC-value-in-flight to 100% of the channel value | ||
| // to ensure we can forward the initial payment. | ||
| user_config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = | ||
| 100; |
There was a problem hiding this comment.
To confirm here I don't think setting 100% here would have helped as that maximum only applies to the inbound HTLCs on that channel. To make sure that 100% of the value can be allocated to outbound HTLCs from the LSP's perspective as the comment describes, the user needs to set their max_inbound to 100%.
So seems to me the intention of the code did not match the comment ?
There was a problem hiding this comment.
Hmm... yeah it does seem that way.
| let mut user_config = default_user_config(&self.config); | ||
| user_config.channel_handshake_config.announce_for_forwarding = announce_for_forwarding; | ||
| user_config.channel_config = (channel_config.unwrap_or_default()).clone().into(); | ||
| // We set the max inflight to 100% for private channels. |
There was a problem hiding this comment.
nit: similar here, would it be worth adding a debug_assert to guard against any changes away from this behavior ?
| accept_underpaying_htlcs: Some(true), | ||
| ..Default::default() | ||
| }), | ||
| ..Default::default() |
There was a problem hiding this comment.
nit: as elsewhere, maybe a quick debug_assert here on the 100% value ? Up to @tnull
jkczyz
force-pushed
the
2026-04-lsp-max-in-flight
branch
from
e8f3575 to
4fda093
Compare
Per review feedback on PR lightningdevkit#878. The forwarding behavior for LSPS2 channels relies on LDK's default of 100% for the unannounced-channel inbound HTLC value-in-flight cap. Catch upstream changes to that default with debug asserts at the three sites that depend on it. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| // If we act as an LSPS2 service, set the HTLC-value-in-flight to 100% of the channel value | ||
| // to ensure we can forward the initial payment. | ||
| user_config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = | ||
| 100; |
There was a problem hiding this comment.
Hmm... yeah it does seem that way.
4 participants
The
max_inbound_htlc_value_in_flight_percent_of_channelconfig setting was used when acting as an LSPS2 service in order to forward the initial payment. However, upstream divided the config setting into two for announced and unannounced channels, the latter defaulting to 100%.Based on #874.