Skip to content

Exit quiescence when splice_init and tx_init_rbf are rejected #4495

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
jkczyz wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Exit quiescence when splice_init and tx_init_rbf are rejected #4495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12998ce
Exit quiescence when tx_init_rbf is rejected with Abort
jkczyz Mar 17, 2026
67e3076
Exit quiescence when splice_init is rejected with Abort
jkczyz Mar 17, 2026
f5bfedd
Return InteractiveTxMsgError from splice_init and tx_init_rbf
jkczyz Mar 17, 2026
3c55d3c
f - Defer resolve_queued_contribution until after validation
jkczyz Apr 2, 2026
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
148 changes: 78 additions & 70 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12686,9 +12686,7 @@ where
}

/// Checks during handling splice_init
pub fn validate_splice_init(
&self, msg: &msgs::SpliceInit, our_funding_contribution: SignedAmount,
) -> Result<FundingScope, ChannelError> {
pub fn validate_splice_init(&self, msg: &msgs::SpliceInit) -> Result<(), ChannelError> {
if self.holder_commitment_point.current_point().is_none() {
return Err(ChannelError::WarnAndDisconnect(format!(
"Channel {} commitment point needs to be advanced once before spliced",
Expand Down Expand Up @@ -12725,32 +12723,7 @@ where
)));
}

self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
.map_err(|e| ChannelError::WarnAndDisconnect(e))?;

// Rotate the pubkeys using the prev_funding_txid as a tweak
let prev_funding_txid = self.funding.get_funding_txid();
let funding_pubkey = match prev_funding_txid {
None => {
debug_assert!(false);
self.funding.get_holder_pubkeys().funding_pubkey
},
Some(prev_funding_txid) => self
.context
.holder_signer
.new_funding_pubkey(prev_funding_txid, &self.context.secp_ctx),
};
let mut new_keys = self.funding.get_holder_pubkeys().clone();
new_keys.funding_pubkey = funding_pubkey;

Ok(FundingScope::for_splice(
&self.funding,
&self.context,
our_funding_contribution,
their_funding_contribution,
msg.funding_pubkey,
new_keys,
))
Ok(())
}

fn validate_splice_contributions(
Expand Down Expand Up @@ -12890,17 +12863,46 @@ where
pub(crate) fn splice_init<ES: EntropySource, L: Logger>(
&mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey,
logger: &L,
) -> Result<msgs::SpliceAck, ChannelError> {
) -> Result<msgs::SpliceAck, InteractiveTxMsgError> {
self.validate_splice_init(msg).map_err(|e| self.quiescent_negotiation_err(e))?;

let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64);
let (our_funding_contribution, holder_balance) =
self.resolve_queued_contribution(feerate, logger)?;
let (queued_net_value, holder_balance) = self
.resolve_queued_contribution(feerate, logger)
.map_err(|e| self.quiescent_negotiation_err(e))?;

let splice_funding =
self.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))?;
let our_funding_contribution = queued_net_value.unwrap_or(SignedAmount::ZERO);
let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis);
self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
.map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?;

// Rotate the pubkeys using the prev_funding_txid as a tweak
let prev_funding_txid = self.funding.get_funding_txid();
let funding_pubkey = match prev_funding_txid {
None => {
debug_assert!(false);
self.funding.get_holder_pubkeys().funding_pubkey
},
Some(prev_funding_txid) => self
.context
.holder_signer
.new_funding_pubkey(prev_funding_txid, &self.context.secp_ctx),
};
let mut holder_pubkeys = self.funding.get_holder_pubkeys().clone();
holder_pubkeys.funding_pubkey = funding_pubkey;

let splice_funding = FundingScope::for_splice(
&self.funding,
&self.context,
our_funding_contribution,
their_funding_contribution,
msg.funding_pubkey,
holder_pubkeys,
);

// Adjust for the feerate and clone so we can store it for future RBF re-use.
let (adjusted_contribution, our_funding_inputs, our_funding_outputs) =
if our_funding_contribution.is_some() {
if queued_net_value.is_some() {
let adjusted_contribution = self
.take_queued_funding_contribution()
.expect("queued_funding_contribution was Some")
Expand All @@ -12911,7 +12913,6 @@ where
} else {
(None, Default::default(), Default::default())
};
let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);

log_info!(
logger,
Expand Down Expand Up @@ -12954,9 +12955,8 @@ where

/// Checks during handling tx_init_rbf for an existing splice
fn validate_tx_init_rbf<F: FeeEstimator>(
&self, msg: &msgs::TxInitRbf, our_funding_contribution: SignedAmount,
fee_estimator: &LowerBoundedFeeEstimator<F>,
) -> Result<FundingScope, ChannelError> {
&self, msg: &msgs::TxInitRbf, fee_estimator: &LowerBoundedFeeEstimator<F>,
) -> Result<(ChannelPublicKeys, PublicKey), ChannelError> {
if self.holder_commitment_point.current_point().is_none() {
return Err(ChannelError::WarnAndDisconnect(format!(
"Channel {} commitment point needs to be advanced once before RBF",
Expand Down Expand Up @@ -13022,36 +13022,26 @@ where
return Err(ChannelError::Abort(AbortReason::InsufficientRbfFeerate));
}

let their_funding_contribution = match msg.funding_output_contribution {
Some(value) => SignedAmount::from_sat(value),
None => SignedAmount::ZERO,
};

self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
.map_err(|e| ChannelError::WarnAndDisconnect(e))?;

// Reuse funding pubkeys from the last negotiated candidate since all RBF candidates
// for the same splice share the same funding output script.
let holder_pubkeys = last_candidate.get_holder_pubkeys().clone();
let counterparty_funding_pubkey = *last_candidate.counterparty_funding_pubkey();

Ok(FundingScope::for_splice(
&self.funding,
&self.context,
our_funding_contribution,
their_funding_contribution,
counterparty_funding_pubkey,
holder_pubkeys,
Ok((
last_candidate.get_holder_pubkeys().clone(),
*last_candidate.counterparty_funding_pubkey(),
))
}

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,
) -> Result<msgs::TxAckRbf, ChannelError> {
) -> Result<msgs::TxAckRbf, InteractiveTxMsgError> {
let (holder_pubkeys, counterparty_funding_pubkey) = self
.validate_tx_init_rbf(msg, fee_estimator)
.map_err(|e| self.quiescent_negotiation_err(e))?;

let feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64);
let (queued_net_value, holder_balance) =
self.resolve_queued_contribution(feerate, logger)?;
let (queued_net_value, holder_balance) = self
.resolve_queued_contribution(feerate, logger)
.map_err(|e| self.quiescent_negotiation_err(e))?;

// If no queued contribution, try prior contribution from previous negotiation.
// Failing here means the RBF would erase our splice — reject it.
Expand All @@ -13068,19 +13058,31 @@ where
prior
.net_value_for_acceptor_at_feerate(feerate, holder_balance)
.map_err(|_| ChannelError::Abort(AbortReason::InsufficientRbfFeerate))
})?;
})
.map_err(|e| self.quiescent_negotiation_err(e))?;
Some(net_value)
} else {
None
};

let our_funding_contribution = queued_net_value.or(prior_net_value);
let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);

let rbf_funding = self.validate_tx_init_rbf(
msg,
our_funding_contribution.unwrap_or(SignedAmount::ZERO),
fee_estimator,
)?;
let their_funding_contribution = match msg.funding_output_contribution {
Some(value) => SignedAmount::from_sat(value),
None => SignedAmount::ZERO,
};
self.validate_splice_contributions(our_funding_contribution, their_funding_contribution)
.map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?;

let rbf_funding = FundingScope::for_splice(
&self.funding,
&self.context,
our_funding_contribution,
their_funding_contribution,
counterparty_funding_pubkey,
holder_pubkeys,
);

// Consume the appropriate contribution source.
let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() {
Expand Down Expand Up @@ -13117,8 +13119,6 @@ where
Default::default()
};

let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO);

log_info!(
logger,
"Starting RBF funding negotiation for channel {} after receiving tx_init_rbf; channel value: {} sats",
Expand Down Expand Up @@ -14237,8 +14237,6 @@ where
Some(msgs::Stfu { channel_id: self.context.channel_id, initiator })
}

#[cfg(any(test, fuzzing, feature = "_test_utils"))]
#[rustfmt::skip]
pub fn exit_quiescence(&mut self) -> bool {
// Make sure we either finished the quiescence handshake and are quiescent, or we never
// attempted to initiate quiescence at all.
Expand All @@ -14251,6 +14249,16 @@ where
was_quiescent
}

fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError {
let exited_quiescence = if matches!(err, ChannelError::Abort(_)) {
debug_assert!(self.context.channel_state.is_quiescent());
self.exit_quiescence()
} else {
false
};
InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence }
}
Comment on lines +14252 to +14260
Copy link
Copy Markdown
Collaborator

@ldk-claude-review-bot ldk-claude-review-bot Apr 2, 2026

Choose a reason for hiding this comment

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

In splice_init, resolve_queued_contribution (which can return Abort(FeeRateTooHigh)) is called before validate_splice_init checks is_quiescent(). If a misbehaving peer sends splice_init during the STFU handshake (before quiescence is established), and the holder has a queued contribution with a low max_feerate, the Abort error would flow into quiescent_negotiation_err → exit_quiescence(), which has debug_assert!(!is_local_stfu_sent()) / debug_assert!(!is_remote_stfu_sent()).

Those asserts would fire because the STFU flags are still set (handshake incomplete). In release builds, exit_quiescence would harmlessly return false, but in debug/test builds this would panic.

Same issue applies to tx_init_rbf at line 12762 (and line 12780).

Consider guarding with an is_quiescent() check before calling exit_quiescence:

Suggested change
fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError {
let exited_quiescence =
if matches!(err, ChannelError::Abort(_)) { self.exit_quiescence() } else { false };
InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence }
}
fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError {
let exited_quiescence = if matches!(err, ChannelError::Abort(_))
&& self.context.channel_state.is_quiescent()
{
self.exit_quiescence()
} else {
false
};
InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence }
}

Copy link
Copy Markdown
Contributor Author

@jkczyz jkczyz Apr 2, 2026

Choose a reason for hiding this comment

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

Fixed, though I took the opportunity to have validate_splice_init and validate_ tx_init_rbf no longer call validate_splice_contributions and FundingScope::for_splice. Rather they are called at those call sites. This let's us check if we are quiescent before checking fee rates when calling resolve_queued_contribution.


pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> {
let end = self
.funding
Expand Down
Loading
Loading