Skip to content

Enforce min_funding_satoshis after splices #4615

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.

Sign up for GitHub

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

Open
tankyleo wants to merge 1 commit into
base: main
Choose a base branch
from
+339 −13
Open

Enforce min_funding_satoshis after splices #4615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 42 additions & 12 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2778,7 +2778,7 @@ impl FundingScope {
fn for_splice<SP: SignerProvider>(
prev_funding: &Self, context: &ChannelContext<SP>, our_funding_contribution: SignedAmount,
their_funding_contribution: SignedAmount, counterparty_funding_pubkey: PublicKey,
our_new_holder_keys: ChannelPublicKeys,
our_new_holder_keys: ChannelPublicKeys, min_funding_satoshis: u64,
) -> Result<Self, String> {
if our_funding_contribution.unsigned_abs() > Amount::MAX_MONEY {
return Err(format!(
Expand Down Expand Up @@ -2817,13 +2817,27 @@ impl FundingScope {
),
)?;

let post_channel_value_sat = prev_funding.get_value_satoshis()
let post_channel_value_sat = prev_funding
.get_value_satoshis()
.checked_add_signed(our_funding_contribution.to_sat())
.and_then(|v| v.checked_add_signed(their_funding_contribution.to_sat()))
.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"
Copy link
Copy Markdown
Collaborator

@TheBlueMatt TheBlueMatt May 15, 2026

Choose a reason for hiding this comment

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

What's with the random reformatting here?

Copy link
Copy Markdown
Contributor Author

@tankyleo tankyleo May 15, 2026

Choose a reason for hiding this comment

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

haha trying to break some long lines myself since rust fmt doesn't help, will revisit

))?;
if post_channel_value_sat < MIN_CHANNEL_VALUE_SATOSHIS {
return Err(format!(
"Spliced channel value must be at least 1000 satoshis. It would be {post_channel_value_sat}",
"Spliced channel value must be at least 1000 satoshis. It would be \
{post_channel_value_sat}"
));
}
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 \
than the configured min_funding_satoshis {min_funding_satoshis}"
));
}

Expand Down Expand Up @@ -13048,6 +13062,7 @@ where
fn validate_splice_contributions(
&self, our_funding_contribution: SignedAmount, their_funding_contribution: SignedAmount,
counterparty_funding_pubkey: PublicKey, our_new_holder_keys: ChannelPublicKeys,
min_funding_satoshis: u64,
) -> Result<FundingScope, String> {
let candidate_scope = FundingScope::for_splice(
&self.funding,
Expand All @@ -13056,6 +13071,7 @@ where
their_funding_contribution,
counterparty_funding_pubkey,
our_new_holder_keys,
min_funding_satoshis,
)
.map_err(|e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e))?;

Expand Down Expand Up @@ -13166,7 +13182,7 @@ where

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,
Copy link
Copy Markdown
Collaborator

@TheBlueMatt TheBlueMatt May 15, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@tankyleo tankyleo May 15, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@tankyleo tankyleo May 15, 2026

Choose a reason for hiding this comment

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

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 \

Copy link
Copy Markdown
Collaborator

@TheBlueMatt TheBlueMatt May 15, 2026

Choose a reason for hiding this comment

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

Hmm, yea, maybe we just do that?

) -> Result<msgs::SpliceAck, InteractiveTxMsgError> {
self.validate_splice_init(msg).map_err(|e| self.quiescent_negotiation_err(e))?;

Expand Down Expand Up @@ -13199,6 +13215,7 @@ where
their_funding_contribution,
msg.funding_pubkey,
holder_pubkeys,
min_funding_satoshis,
)
.map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?;

Expand Down Expand Up @@ -13334,7 +13351,7 @@ where

pub(crate) fn tx_init_rbf<ES: EntropySource, F: FeeEstimator, L: Logger>(
&mut self, msg: &msgs::TxInitRbf, entropy_source: &ES, holder_node_id: &PublicKey,
fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
fee_estimator: &LowerBoundedFeeEstimator<F>, min_funding_satoshis: u64, logger: &L,
) -> Result<msgs::TxAckRbf, InteractiveTxMsgError> {
let (holder_pubkeys, counterparty_funding_pubkey) = self
.validate_tx_init_rbf(msg, fee_estimator)
Expand Down Expand Up @@ -13381,6 +13398,7 @@ where
their_funding_contribution,
counterparty_funding_pubkey,
holder_pubkeys,
min_funding_satoshis,
)
.map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?;

Expand Down Expand Up @@ -13452,7 +13470,9 @@ where
})
}

fn validate_tx_ack_rbf(&self, msg: &msgs::TxAckRbf) -> Result<FundingScope, ChannelError> {
fn validate_tx_ack_rbf(
&self, msg: &msgs::TxAckRbf, min_funding_satoshis: u64,
) -> Result<FundingScope, ChannelError> {
let pending_splice = self
.pending_splice
.as_ref()
Expand All @@ -13478,6 +13498,7 @@ where
their_funding_contribution,
counterparty_funding_pubkey,
holder_pubkeys,
min_funding_satoshis,
)
.map_err(|e| ChannelError::WarnAndDisconnect(e))?;

Expand All @@ -13486,9 +13507,9 @@ where

pub(crate) fn tx_ack_rbf<ES: EntropySource, L: Logger>(
&mut self, msg: &msgs::TxAckRbf, entropy_source: &ES, holder_node_id: &PublicKey,
logger: &L,
min_funding_satoshis: u64, logger: &L,
) -> Result<Option<InteractiveTxMessageSend>, ChannelError> {
let rbf_funding = self.validate_tx_ack_rbf(msg)?;
let rbf_funding = self.validate_tx_ack_rbf(msg, min_funding_satoshis)?;

log_info!(
logger,
Expand Down Expand Up @@ -13519,9 +13540,9 @@ where

pub(crate) fn splice_ack<ES: EntropySource, L: Logger>(
&mut self, msg: &msgs::SpliceAck, entropy_source: &ES, holder_node_id: &PublicKey,
logger: &L,
min_funding_satoshis: u64, logger: &L,
) -> Result<Option<InteractiveTxMessageSend>, ChannelError> {
let splice_funding = self.validate_splice_ack(msg)?;
let splice_funding = self.validate_splice_ack(msg, min_funding_satoshis)?;

log_info!(
logger,
Expand Down Expand Up @@ -13553,7 +13574,9 @@ where
Ok(tx_msg_opt)
}

fn validate_splice_ack(&self, msg: &msgs::SpliceAck) -> Result<FundingScope, ChannelError> {
fn validate_splice_ack(
&self, msg: &msgs::SpliceAck, min_funding_satoshis: u64,
) -> Result<FundingScope, ChannelError> {
// TODO(splicing): Add check that we are the splice (quiescence) initiator

let pending_splice = self
Expand All @@ -13576,6 +13599,7 @@ where
their_funding_contribution,
msg.funding_pubkey,
new_keys,
min_funding_satoshis,
)
.map_err(|e| ChannelError::WarnAndDisconnect(e))?;

Expand Down Expand Up @@ -13692,6 +13716,9 @@ where
SignedAmount::ZERO,
funding.counterparty_funding_pubkey().clone(),
funding.get_holder_pubkeys().clone(),
// When the counterparty's contribution is non-negative, we don't validate
// the post splice channel value against `min_funding_satoshis`
0,
)
.unwrap();
// Splice-out an additional satoshi, and validation fails!
Expand All @@ -13700,6 +13727,9 @@ where
SignedAmount::ZERO,
funding.counterparty_funding_pubkey().clone(),
funding.get_holder_pubkeys().clone(),
// When the counterparty's contribution is non-negative, we don't validate
// the post splice channel value against `min_funding_satoshis`
0,
)
Comment thread
tankyleo marked this conversation as resolved.
.unwrap_err();
}
Expand Down
4 changes: 4 additions & 0 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13384,6 +13384,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
msg,
&self.entropy_source,
&self.get_our_node_id(),
self.config.read().unwrap().channel_handshake_limits.min_funding_satoshis,
&self.logger,
) {
Ok(splice_ack_msg) => {
Expand Down Expand Up @@ -13441,6 +13442,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
&self.entropy_source,
&self.get_our_node_id(),
&self.fee_estimator,
self.config.read().unwrap().channel_handshake_limits.min_funding_satoshis,
&self.logger,
) {
Ok(tx_ack_rbf_msg) => {
Expand Down Expand Up @@ -13497,6 +13499,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
msg,
&self.entropy_source,
&self.get_our_node_id(),
self.config.read().unwrap().channel_handshake_limits.min_funding_satoshis,
&self.logger,
);
let tx_msg_opt =
Expand Down Expand Up @@ -13541,6 +13544,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
msg,
&self.entropy_source,
&self.get_our_node_id(),
self.config.read().unwrap().channel_handshake_limits.min_funding_satoshis,
&self.logger,
);
let tx_msg_opt =
Expand Down
Loading
Loading