Remove unnecessary FundedChannel::pending_splice checks
What changed, and why it matters
This commit is a routine internal cleanup in the Lightning channel code. It removes redundant checks for splice funding transactions now that the data structure already stores the negotiated candidates directly. There is no indication this fixes a security bug or changes security-critical behavior.
No security action required. Treat as normal code maintenance.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors two functions in lightning/src/ln/channel.rs that handle splice funding confirmation. Previously, the code iterated over self.pending_funding() to find a confirmed funding index, then separately matched self.pending_splice to access negotiated_candidates. After the change, the code directly iterates over pending_splice.negotiated_candidates inside an if-let Some(pending_splice) block, removing the now-unnecessary fallback match arm that contained a debug_assert(false) and a ProcessingError. The commit message explicitly frames this as removing unnecessary checks because PendingFunding now contains the negotiated candidates. No new validation is added and no security-sensitive behavior appears altered.
Changed components
lightning/src/ln/channel.rssplice funding confirmation handlingInspect captured patch +77 / −88
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 970d498..0e38eec 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -10793,9 +10793,10 @@ where
}
}
- let mut confirmed_funding_index = None;
- let mut funding_already_confirmed = false;
if let Some(pending_splice) = &mut self.pending_splice {
+ let mut confirmed_funding_index = None;
+ let mut funding_already_confirmed = false;
+
for (index, funding) in pending_splice.negotiated_candidates.iter_mut().enumerate() {
if self.context.check_for_funding_tx_confirmed(
funding, block_hash, height, index_in_block, &mut confirmed_tx, logger,
@@ -10810,40 +10811,36 @@ where
funding_already_confirmed = true;
}
}
- }
- if let Some(confirmed_funding_index) = confirmed_funding_index {
- let pending_splice = match self.pending_splice.as_mut() {
- Some(pending_splice) => pending_splice,
- None => {
- // TODO: Move pending_funding into pending_splice
- debug_assert!(false);
- let err = "expected a pending splice".to_string();
- return Err(ClosureReason::ProcessingError { err });
- },
- };
+ if let Some(confirmed_funding_index) = confirmed_funding_index {
+ if let Some(splice_locked) = pending_splice.check_get_splice_locked(
+ &self.context,
+ confirmed_funding_index,
+ height,
+ ) {
- if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, confirmed_funding_index, height) {
- log_info!(
- logger,
- "Sending splice_locked txid {} to our peer for channel {}",
- splice_locked.splice_txid,
- &self.context.channel_id,
- );
-
- let (funding_txo, monitor_update, announcement_sigs, discarded_funding) =
- self.maybe_promote_splice_funding(
- node_signer, chain_hash, user_config, height, logger,
- ).map(|splice_promotion| (
- Some(splice_promotion.funding_txo),
- splice_promotion.monitor_update,
- splice_promotion.announcement_sigs,
- splice_promotion.discarded_funding,
- )).unwrap_or((None, None, None, Vec::new()));
+ log_info!(
+ logger,
+ "Sending splice_locked txid {} to our peer for channel {}",
+ splice_locked.splice_txid,
+ &self.context.channel_id,
+ );
- return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo, monitor_update, discarded_funding)), announcement_sigs));
+ let (funding_txo, monitor_update, announcement_sigs, discarded_funding) =
+ self.maybe_promote_splice_funding(
+ node_signer, chain_hash, user_config, height, logger,
+ ).map(|splice_promotion| (
+ Some(splice_promotion.funding_txo),
+ splice_promotion.monitor_update,
+ splice_promotion.announcement_sigs,
+ splice_promotion.discarded_funding,
+ )).unwrap_or((None, None, None, Vec::new()));
+
+ return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo, monitor_update, discarded_funding)), announcement_sigs));
+ }
}
}
+
}
Ok((None, None))
@@ -10946,70 +10943,62 @@ where
return Err(ClosureReason::FundingTimedOut);
}
- let mut confirmed_funding_index = None;
- for (index, funding) in self.pending_funding().iter().enumerate() {
- if funding.funding_tx_confirmation_height != 0 {
- if confirmed_funding_index.is_some() {
- let err_reason = "splice tx of another pending funding already confirmed";
- return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
- }
+ if let Some(pending_splice) = &mut self.pending_splice {
+ let mut confirmed_funding_index = None;
+
+ for (index, funding) in pending_splice.negotiated_candidates.iter().enumerate() {
+ if funding.funding_tx_confirmation_height != 0 {
+ if confirmed_funding_index.is_some() {
+ let err_reason = "splice tx of another pending funding already confirmed";
+ return Err(ClosureReason::ProcessingError { err: err_reason.to_owned() });
+ }
- confirmed_funding_index = Some(index);
+ confirmed_funding_index = Some(index);
+ }
}
- }
- if let Some(confirmed_funding_index) = confirmed_funding_index {
- let pending_splice = match self.pending_splice.as_mut() {
- Some(pending_splice) => pending_splice,
- None => {
- // TODO: Move pending_funding into pending_splice
- debug_assert!(false);
- let err = "expected a pending splice".to_string();
- return Err(ClosureReason::ProcessingError { err });
- },
- };
- let funding = &mut pending_splice.negotiated_candidates[confirmed_funding_index];
-
- // Check if the splice funding transaction was unconfirmed
- if funding.get_funding_tx_confirmations(height) == 0 {
- funding.funding_tx_confirmation_height = 0;
- if let Some(sent_funding_txid) = pending_splice.sent_funding_txid {
- if Some(sent_funding_txid) == funding.get_funding_txid() {
- log_warn!(
- logger,
- "Unconfirming sent splice_locked txid {} for channel {}",
- sent_funding_txid,
- &self.context.channel_id,
- );
- pending_splice.sent_funding_txid = None;
+ if let Some(confirmed_funding_index) = confirmed_funding_index {
+ let funding = &mut pending_splice.negotiated_candidates[confirmed_funding_index];
+
+ // Check if the splice funding transaction was unconfirmed
+ if funding.get_funding_tx_confirmations(height) == 0 {
+ funding.funding_tx_confirmation_height = 0;
+ if let Some(sent_funding_txid) = pending_splice.sent_funding_txid {
+ if Some(sent_funding_txid) == funding.get_funding_txid() {
+ log_warn!(
+ logger,
+ "Unconfirming sent splice_locked txid {} for channel {}",
+ sent_funding_txid,
+ &self.context.channel_id,
+ );
+ pending_splice.sent_funding_txid = None;
+ }
}
}
- }
- let pending_splice = self.pending_splice.as_mut().unwrap();
- if let Some(splice_locked) = pending_splice.check_get_splice_locked(
- &self.context,
- confirmed_funding_index,
- height,
- ) {
- log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id);
- debug_assert!(chain_node_signer.is_some());
-
- let (funding_txo, monitor_update, announcement_sigs, discarded_funding) = chain_node_signer
- .and_then(|(chain_hash, node_signer, user_config)| {
- // We can only promote on blocks connected, which is when we expect
- // `chain_node_signer` to be `Some`.
- self.maybe_promote_splice_funding(node_signer, chain_hash, user_config, height, logger)
- })
- .map(|splice_promotion| (
- Some(splice_promotion.funding_txo),
- splice_promotion.monitor_update,
- splice_promotion.announcement_sigs,
- splice_promotion.discarded_funding,
- ))
- .unwrap_or((None, None, None, Vec::new()));
+ if let Some(splice_locked) = pending_splice.check_get_splice_locked(
+ &self.context,
+ confirmed_funding_index,
+ height,
+ ) {
+ log_info!(logger, "Sending a splice_locked to our peer for channel {}", &self.context.channel_id);
- return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo, monitor_update, discarded_funding)), timed_out_htlcs, announcement_sigs));
+ let (funding_txo, monitor_update, announcement_sigs, discarded_funding) = chain_node_signer
+ .and_then(|(chain_hash, node_signer, user_config)| {
+ // We can only promote on blocks connected, which is when we expect
+ // `chain_node_signer` to be `Some`.
+ self.maybe_promote_splice_funding(node_signer, chain_hash, user_config, height, logger)
+ })
+ .map(|splice_promotion| (
+ Some(splice_promotion.funding_txo),
+ splice_promotion.monitor_update,
+ splice_promotion.announcement_sigs,
+ splice_promotion.discarded_funding,
+ ))
+ .unwrap_or((None, None, None, Vec::new()));
+
+ return Ok((Some(FundingConfirmedMessage::Splice(splice_locked, funding_txo, monitor_update, discarded_funding)), timed_out_htlcs, announcement_sigs));
+ }
}
}
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.