Move FundedChannel::pending_funding into PendingSplice
What changed, and why it matters
This is a code-cleanup refactor in a Bitcoin Lightning Network library. It moves a list of not-yet-confirmed funding transactions (pending_funding) from the general channel object into a splice-specific structure (PendingSplice), because that list is only meaningful during a splice. The change is mostly internal reorganization, but it also slightly reorders cleanup steps when a splice is finalized and adds helper methods to access the moved data. There is no direct evidence in the commit that this fixes a known security bug, but refactors around funding-state management can affect correctness of channel funds handling.
Treat as a normal code-quality and maintainability patch. Review the reordered cleanup in promote_splice_funding! to confirm no use-after-swap or double-promotion edge cases. No urgent security action is indicated by the commit itself.
Security signals we found
State-machine refactor for channel funding/splicing
Reordering of cleanup operations in promote_splice_funding! macro
Addition of debug_assert! bounds check in splice-locked generation
No explicit security bug or CVE referenced in commit message
Evidence from the diff
The commit relocates FundedChannel::pending_funding: Vec
Changed components
lightning/src/ln/channel.rsFundedChannel structPendingSplice structpromote_splice_funding! macroSplice locking and funding confirmation logicInspect captured patch +75 / −49
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 12c4bd8..1e7347d 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2037,7 +2037,6 @@ where
};
let mut funded_channel = FundedChannel {
funding: chan.funding,
- pending_funding: vec![],
context: chan.context,
interactive_tx_signing_session: chan.interactive_tx_signing_session,
holder_commitment_point,
@@ -2550,6 +2549,7 @@ impl AddSigned for u64 {
/// Info about a pending splice
struct PendingSplice {
funding_negotiation: Option<FundingNegotiation>,
+ pending_funding: Vec<FundingScope>,
/// The funding txid used in the `splice_locked` sent to the counterparty.
sent_funding_txid: Option<Txid>,
@@ -2576,11 +2576,14 @@ impl FundingNegotiation {
impl PendingSplice {
fn check_get_splice_locked<SP: Deref>(
- &mut self, context: &ChannelContext<SP>, funding: &FundingScope, height: u32,
+ &mut self, context: &ChannelContext<SP>, confirmed_funding_index: usize, height: u32,
) -> Option<msgs::SpliceLocked>
where
SP::Target: SignerProvider,
{
+ debug_assert!(confirmed_funding_index < self.pending_funding.len());
+
+ let funding = &self.pending_funding[confirmed_funding_index];
if !context.check_funding_meets_minimum_depth(funding, height) {
return None;
}
@@ -6632,7 +6635,6 @@ where
SP::Target: SignerProvider,
{
pub funding: FundingScope,
- pending_funding: Vec<FundingScope>,
pub context: ChannelContext<SP>,
/// The signing session for the current interactive tx construction, if any.
///
@@ -6654,19 +6656,16 @@ where
}
macro_rules! promote_splice_funding {
- ($self: expr, $funding: expr) => {{
+ ($self: expr, $pending_splice: expr, $funding: expr) => {{
let prev_funding_txid = $self.funding.get_funding_txid();
if let Some(scid) = $self.funding.short_channel_id {
$self.context.historical_scids.push(scid);
}
+
core::mem::swap(&mut $self.funding, $funding);
- $self.interactive_tx_signing_session = None;
- $self.pending_splice = None;
- $self.context.announcement_sigs = None;
- $self.context.announcement_sigs_state = AnnouncementSigsState::NotSent;
// The swap above places the previous `FundingScope` into `pending_funding`.
- let discarded_funding = $self
+ let discarded_funding = $pending_splice
.pending_funding
.drain(..)
.filter(|funding| funding.get_funding_txid() != prev_funding_txid)
@@ -6682,6 +6681,12 @@ macro_rules! promote_splice_funding {
})
})
.collect::<Vec<_>>();
+
+ $self.interactive_tx_signing_session = None;
+ $self.pending_splice = None;
+ $self.context.announcement_sigs = None;
+ $self.context.announcement_sigs_state = AnnouncementSigsState::NotSent;
+
discarded_funding
}};
}
@@ -6796,6 +6801,24 @@ where
})
}
+ fn pending_funding(&self) -> &[FundingScope] {
+ if let Some(pending_splice) = &self.pending_splice {
+ pending_splice.pending_funding.as_slice()
+ } else {
+ &[]
+ }
+ }
+
+ fn funding_and_pending_funding_iter_mut(&mut self) -> impl Iterator<Item = &mut FundingScope> {
+ core::iter::once(&mut self.funding).chain(
+ self.pending_splice
+ .as_mut()
+ .map(|pending_splice| pending_splice.pending_funding.as_mut_slice())
+ .unwrap_or(&mut [])
+ .iter_mut(),
+ )
+ }
+
#[rustfmt::skip]
fn check_remote_fee<F: Deref, L: Deref>(
channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator<F>,
@@ -7425,7 +7448,7 @@ where
}
core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.try_for_each(|funding| self.context.validate_update_add_htlc(funding, msg, fee_estimator))?;
// Now update local state:
@@ -7680,7 +7703,7 @@ where
{
self.commitment_signed_check_state()?;
- if !self.pending_funding.is_empty() {
+ if !self.pending_funding().is_empty() {
return Err(ChannelError::close(
"Got a single commitment_signed message when expecting a batch".to_owned(),
));
@@ -7747,9 +7770,9 @@ where
// Any commitment_signed not associated with a FundingScope is ignored below if a
// pending splice transaction has confirmed since receiving the batch.
- let mut commitment_txs = Vec::with_capacity(self.pending_funding.len() + 1);
+ let mut commitment_txs = Vec::with_capacity(self.pending_funding().len() + 1);
let mut htlc_data = None;
- for funding in core::iter::once(&self.funding).chain(self.pending_funding.iter()) {
+ for funding in core::iter::once(&self.funding).chain(self.pending_funding().iter()) {
let funding_txid =
funding.get_funding_txid().expect("Funding txid must be known for pending scope");
let msg = messages.get(&funding_txid).ok_or_else(|| {
@@ -8419,7 +8442,7 @@ where
}
}
- for funding in core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut()) {
+ for funding in self.funding_and_pending_funding_iter_mut() {
funding.value_to_self_msat =
(funding.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;
}
@@ -8749,7 +8772,7 @@ where
debug_assert!(!self.funding.get_channel_type().supports_anchor_zero_fee_commitments());
let can_send_update_fee = core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.all(|funding| self.context.can_send_update_fee(funding, feerate_per_kw, fee_estimator, logger));
if !can_send_update_fee {
return None;
@@ -9052,14 +9075,14 @@ where
}
core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.try_for_each(|funding| FundedChannel::<SP>::check_remote_fee(funding.get_channel_type(), fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger))?;
self.context.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced));
self.context.update_time_counter += 1;
core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.try_for_each(|funding| self.context.validate_update_fee(funding, fee_estimator, msg))
}
@@ -9584,7 +9607,7 @@ where
// - MUST process `my_current_funding_locked` as if it was receiving `splice_locked`
// for this `txid`.
let inferred_splice_locked = msg.my_current_funding_locked.as_ref().and_then(|funding_locked| {
- self.pending_funding
+ self.pending_funding()
.iter()
.find(|funding| funding.get_funding_txid() == Some(funding_locked.txid))
.and_then(|_| {
@@ -10358,7 +10381,7 @@ where
);
core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.try_for_each(|funding| self.context.can_accept_incoming_htlc(funding, dust_exposure_limiting_feerate, &logger))
}
@@ -10692,12 +10715,12 @@ where
let discarded_funding = {
// Scope `funding` since it is swapped within `promote_splice_funding` and we don't want
// to unintentionally use it.
- let funding = self
+ let funding = pending_splice
.pending_funding
.iter_mut()
.find(|funding| funding.get_funding_txid() == Some(splice_txid))
.unwrap();
- promote_splice_funding!(self, funding)
+ promote_splice_funding!(self, pending_splice, funding)
};
let funding_txo = self
@@ -10767,18 +10790,20 @@ where
let mut confirmed_funding_index = None;
let mut funding_already_confirmed = false;
- for (index, funding) in self.pending_funding.iter_mut().enumerate() {
- if self.context.check_for_funding_tx_confirmed(
- funding, block_hash, height, index_in_block, &mut confirmed_tx, logger,
- )? {
- if funding_already_confirmed || 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 {
+ for (index, funding) in pending_splice.pending_funding.iter_mut().enumerate() {
+ if self.context.check_for_funding_tx_confirmed(
+ funding, block_hash, height, index_in_block, &mut confirmed_tx, logger,
+ )? {
+ if funding_already_confirmed || 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);
- } else if funding.funding_tx_confirmation_height != 0 {
- funding_already_confirmed = true;
+ confirmed_funding_index = Some(index);
+ } else if funding.funding_tx_confirmation_height != 0 {
+ funding_already_confirmed = true;
+ }
}
}
@@ -10792,9 +10817,8 @@ where
return Err(ClosureReason::ProcessingError { err });
},
};
- let funding = self.pending_funding.get(confirmed_funding_index).unwrap();
- if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, funding, 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 {}",
@@ -10918,7 +10942,7 @@ where
}
let mut confirmed_funding_index = None;
- for (index, funding) in self.pending_funding.iter().enumerate() {
+ 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";
@@ -10939,7 +10963,7 @@ where
return Err(ClosureReason::ProcessingError { err });
},
};
- let funding = self.pending_funding.get_mut(confirmed_funding_index).unwrap();
+ let funding = &mut pending_splice.pending_funding[confirmed_funding_index];
// Check if the splice funding transaction was unconfirmed
if funding.get_funding_tx_confirmations(height) == 0 {
@@ -10958,8 +10982,11 @@ where
}
let pending_splice = self.pending_splice.as_mut().unwrap();
- let funding = self.pending_funding.get(confirmed_funding_index).unwrap();
- if let Some(splice_locked) = pending_splice.check_get_splice_locked(&self.context, funding, height) {
+ 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());
@@ -10989,7 +11016,7 @@ where
pub fn get_relevant_txids(&self) -> impl Iterator<Item = (Txid, u32, Option<BlockHash>)> + '_ {
core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.map(|funding| {
(
funding.get_funding_txid(),
@@ -11019,8 +11046,8 @@ where
where
L::Target: Logger,
{
- let unconfirmed_funding = core::iter::once(&mut self.funding)
- .chain(self.pending_funding.iter_mut())
+ let unconfirmed_funding = self
+ .funding_and_pending_funding_iter_mut()
.find(|funding| funding.get_funding_txid() == Some(*txid));
if let Some(funding) = unconfirmed_funding {
@@ -11515,6 +11542,7 @@ where
self.pending_splice = Some(PendingSplice {
funding_negotiation: Some(FundingNegotiation::AwaitingAck(funding_negotiation_context)),
+ pending_funding: vec![],
sent_funding_txid: None,
received_funding_txid: None,
});
@@ -11737,6 +11765,7 @@ where
splice_funding,
interactive_tx_constructor,
)),
+ pending_funding: Vec::new(),
received_funding_txid: None,
sent_funding_txid: None,
});
@@ -11924,7 +11953,7 @@ where
},
};
- if !self
+ if !pending_splice
.pending_funding
.iter()
.any(|funding| funding.get_funding_txid() == Some(msg.splice_txid))
@@ -12121,7 +12150,7 @@ where
F::Target: FeeEstimator,
{
core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.map(|funding| self.context.get_available_balances_for_scope(funding, fee_estimator))
.reduce(|acc, e| {
AvailableBalances {
@@ -12168,7 +12197,7 @@ where
}
self.context.resend_order = RAACommitmentOrder::RevokeAndACKFirst;
- let update = if self.pending_funding.is_empty() {
+ let update = if self.pending_funding().is_empty() {
let (htlcs_ref, counterparty_commitment_tx) =
self.build_commitment_no_state_update(&self.funding, logger);
let htlc_outputs = htlcs_ref.into_iter()
@@ -12191,7 +12220,7 @@ where
} else {
let mut htlc_data = None;
let commitment_txs = core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.map(|funding| {
let (htlcs_ref, counterparty_commitment_tx) =
self.build_commitment_no_state_update(funding, logger);
@@ -12264,7 +12293,7 @@ where
L::Target: Logger,
{
core::iter::once(&self.funding)
- .chain(self.pending_funding.iter())
+ .chain(self.pending_funding().iter())
.map(|funding| self.send_commitment_no_state_update_for_funding(funding, logger))
.collect::<Result<Vec<_>, ChannelError>>()
}
@@ -13064,7 +13093,6 @@ where
let mut channel = FundedChannel {
funding: self.funding,
- pending_funding: vec![],
context: self.context,
interactive_tx_signing_session: None,
holder_commitment_point,
@@ -13350,7 +13378,6 @@ where
// `ChannelMonitor`.
let mut channel = FundedChannel {
funding: self.funding,
- pending_funding: vec![],
context: self.context,
interactive_tx_signing_session: None,
holder_commitment_point,
@@ -14926,7 +14953,6 @@ where
short_channel_id,
minimum_depth_override,
},
- pending_funding: Vec::new(),
context: ChannelContext {
user_id,
Why this scored 26/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.