Enforce min_funding_satoshis after splices#4615
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
No issues found. The PR is clean. Summary of review:
No new issues found beyond what was flagged in prior review passes. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4615 +/- ##
==========================================
+ Coverage 86.40% 86.42% +0.01%
==========================================
Files 158 158
Lines 109293 109324 +31
Branches 109293 109324 +31
==========================================
+ Hits 94439 94487 +48
+ Misses 12309 12297 -12
+ Partials 2545 2540 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tankyleo
force-pushed
the
2026-05-splice-min-funding-satoshis
branch
from
861efec to
6a86bd5
Compare
| .ok_or(format!("The sum of contributions {our_funding_contribution} and {their_funding_contribution} is greater than the channel's value"))?; | ||
| .ok_or(format!( | ||
| "The sum of contributions {our_funding_contribution} and \ | ||
| {their_funding_contribution} is greater than the channel's value" |
There was a problem hiding this comment.
What's with the random reformatting here?
There was a problem hiding this comment.
haha trying to break some long lines myself since rust fmt doesn't help, will revisit
| pub(crate) fn splice_init<ES: EntropySource, L: Logger>( | ||
| &mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey, | ||
| logger: &L, | ||
| min_funding_satoshis: u64, logger: &L, |
There was a problem hiding this comment.
Hmm, should we maybe move the configured minimum to the channel somehow? Its kinda weird that we apply the global channel minimum to splice-outs on potentially-outbound channels. Someone might use that config to limit inbound channels to only 1 BTC + (or something very high to prevent rando nodes from opening channels) but be perfectly fine with a peer taking funds out of an existing channel. I'm generally not super convinced that re-applying this config knob to splices makes sense for uses outside of "don't allow absurdly broken channels", which is basically the use-case for the default value, but might not be for all uses of it.
There was a problem hiding this comment.
The case I had in mind is rando opens a fat channel to you, then withdraws past your funding limit. But agreed it's not a very strong case, can drop the PR if this use-case is not compelling.
There was a problem hiding this comment.
Could also apply this diff to enforce this on just inbound channels
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index f7c7fcff7..fde56a9ad 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2831,7 +2831,9 @@
{post_channel_value_sat}"
));
}
- if post_channel_value_sat < min_funding_satoshis && their_funding_contribution.is_negative()
+ if post_channel_value_sat < min_funding_satoshis
+ && their_funding_contribution.is_negative()
+ && !prev_funding.is_outbound()
{
return Err(format!(
"Spliced channel value {post_channel_value_sat} would be smaller \There was a problem hiding this comment.
Hmm, yea, maybe we just do that?
In inbound channels, we already enforce this minimum at channel open, so it makes sense to also enforce this minimum on any splices in which the counterparty's contribution is negative. Codex wrote the tests.
tankyleo
force-pushed
the
2026-05-splice-min-funding-satoshis
branch
from
6a86bd5 to
6c2090d
Compare
4 participants
We already enforce this minimum for inbound channels, so it makes sense to also enforce this minimum on any splices in which the counterparty's contribution is negative.