[0.2] Backport of #4580#4686
Conversation
This was a drive-by fix in a4bf94a
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
Based on my prior thorough review (recorded in memory), I examined all changes in this PR: the No issues found. |
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
On the first commit, please provide a brief summary of conflicts resolved after the "Backport of..." line. On the second commit, given the counterparty dust limit is required to be between 354 and 546, should we just check that the channel value is at least 546?
| Some(get_v2_channel_reserve_satoshis(post_channel_value, MIN_CHAN_DUST_LIMIT_SATOSHIS)); | ||
| let counterparty_selected_channel_reserve_satoshis = Some( | ||
| get_v2_channel_reserve_satoshis(post_channel_value, context.holder_dust_limit_satoshis) | ||
| .expect("We ran this exact function in `validate_splice_contributions`"), |
There was a problem hiding this comment.
I admit I don't love adding expects in a backport that aren't there upstream. Can we just convert these to failures?
There was a problem hiding this comment.
Done below, I add an extra commit where I make for_splice fallible
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
The floor for *our* selected reserve is *their* dust limit. Backport of a3dded1
In preparation for an upcoming backport commit, we make `FundedChannel::for_splice` fallible. This will avoid introducing `expect` calls in the backport commit that aren't present in the upstream commit.
In 0FC channels, capping the reserve to the total value of the channel allowed a splice initiator to withdraw past their reserve in case the acceptor had no balance in the channel. This is because the post-splice value of the channel was equal to the initiator's post splice balance. Hence, this post splice balance always matched the reserve, even though the reserve was below the dust limit. The only thing that prevented the initiator from withdrawing all their balance was the script dust limit check in `interactivetxs::NegotiationContext::receive_tx_add_output`. In case the splice acceptor had any balance in the channel, or there were HTLCs in the channel, or the channel was not 0FC, the splice initiator's post-splice balance was always below the full channel value. Hence when the reserve was capped at the channel value, the post-splice balance was always below the reserve, and the splice was rejected. Also, in `validate_splice_contributions`, to determine the `counterparty_selected_channel_reserve`, we now read the holder's dust limit from the context, instead of the current global constant. Backport of 3835f84 Conflicts resolved in: * lightning/src/ln/channel.rs * lightning/src/ln/splicing_tests.rs * lightning/src/sign/tx_builder.rs
We made the same change to the calculation of the v2 reserve in the previous commit. Backport of 53e156a Conflicts resolved in: * lightning/src/ln/channel.rs * lightning/src/ln/channel_open_tests.rs * lightning/src/ln/functional_tests.rs * lightning/src/ln/htlc_reserve_unit_tests.rs * lightning/src/ln/update_fee_tests.rs Here their `dust_limit_satoshis` can never be greater than `MIN_THEIR_CHAN_RESERVE_SATOSHIS`, so by making sure that `channel_value_satoshis` is greater than `MIN_THEIR_CHAN_RESERVE_SATOSHIS`, we also make sure that `channel_value_satoshis` is greater than their `dust_limit_satoshis`.
tankyleo
force-pushed
the
2026-06-reserve-greater-than-channel-value
branch
from
5eb859d to
9755b2e
Compare
|
I added a backport of a3dded1
Hmm in that case we'd return a reserve of 1000sats for a channel value of 546? I think we should error in this case. |
4 participants
No description provided.