Skip to content

Wipe splice state upon failed interactive funding construction #4120

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

Merged
merged 1 commit into from
Oct 1, 2025
Merged

Wipe splice state upon failed interactive funding construction #4120

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
160 changes: 122 additions & 38 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1686,27 +1686,20 @@ where
let logger = WithChannelContext::from(logger, &self.context(), None);
log_info!(logger, "Failed interactive transaction negotiation: {reason}");

let _interactive_tx_constructor = match &mut self.phase {
match &mut self.phase {
ChannelPhase::Undefined => unreachable!(),
ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => None,
ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => {},
ChannelPhase::UnfundedV2(pending_v2_channel) => {
pending_v2_channel.interactive_tx_constructor.take()
pending_v2_channel.interactive_tx_constructor.take();
},
ChannelPhase::Funded(funded_channel) => {
if funded_channel.should_reset_pending_splice_funding_negotiation().unwrap_or(true)
{
funded_channel.reset_pending_splice_state();
} else {
debug_assert!(false, "We should never fail an interactive funding negotiation once we're exchanging tx_signatures");
}
},
ChannelPhase::Funded(funded_channel) => funded_channel
.pending_splice
.as_mut()
.and_then(|pending_splice| pending_splice.funding_negotiation.take())
.and_then(|funding_negotiation| {
if let FundingNegotiation::ConstructingTransaction {
interactive_tx_constructor,
..
} = funding_negotiation
{
Some(interactive_tx_constructor)
} else {
None
}
}),
};

reason.into_tx_abort_msg(self.context().channel_id)
Expand Down Expand Up @@ -1818,11 +1811,11 @@ where
where
L::Target: Logger,
{
// This checks for and resets the interactive negotiation state by `take()`ing it from the channel.
// The existence of the `tx_constructor` indicates that we have not moved into the signing
// phase for this interactively constructed transaction and hence we have not exchanged
// `tx_signatures`. Either way, we never close the channel upon receiving a `tx_abort`:
// https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L574-L576
// If we have not sent a `tx_abort` message for this negotiation previously, we need to echo
// back a tx_abort message according to the spec:
// https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L560-L561
// For rationale why we echo back `tx_abort`:
// https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L578-L580
let should_ack = match &mut self.phase {
ChannelPhase::Undefined => unreachable!(),
ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => {
Expand All @@ -1832,19 +1825,27 @@ where
ChannelPhase::UnfundedV2(pending_v2_channel) => {
pending_v2_channel.interactive_tx_constructor.take().is_some()
},
ChannelPhase::Funded(funded_channel) => funded_channel
.pending_splice
.as_mut()
.and_then(|pending_splice| pending_splice.funding_negotiation.take())
.is_some(),
ChannelPhase::Funded(funded_channel) => {
if let Some(should_reset) =
funded_channel.should_reset_pending_splice_funding_negotiation()
{
if should_reset {
// We may have still tracked the pending funding negotiation state, so we
// should ack with our own `tx_abort`.
funded_channel.reset_pending_splice_state()
} else {
return Err(ChannelError::close(
"Received tx_abort while awaiting tx_signatures exchange".to_owned(),
));
}
} else {
// We were not tracking the pending funding negotiation state anymore, likely
// due to a disconnection or already having sent our own `tx_abort`.
false
}
},
};

// NOTE: Since at this point we have not sent a `tx_abort` message for this negotiation
// previously (tx_constructor was `Some`), we need to echo back a tx_abort message according
// to the spec:
// https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L560-L561
// For rationale why we echo back `tx_abort`:
// https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L578-L580
Ok(should_ack.then(|| {
let logger = WithChannelContext::from(logger, &self.context(), None);
let reason =
Expand Down Expand Up @@ -2554,6 +2555,15 @@ impl FundingNegotiation {
}

impl PendingFunding {
fn can_abandon_funding_negotiation(&self) -> bool {
self.funding_negotiation
.as_ref()
.map(|funding_negotiation| {
!matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. })
})
.unwrap_or(true)
}

fn check_get_splice_locked<SP: Deref>(
&mut self, context: &ChannelContext<SP>, confirmed_funding_index: usize, height: u32,
) -> Option<msgs::SpliceLocked>
Expand Down Expand Up @@ -6739,6 +6749,45 @@ where
)
}

/// Returns `None` if there is no [`FundedChannel::pending_splice`], otherwise a boolean
/// indicating whether we should reset the splice's [`PendingFunding::funding_negotiation`].
fn should_reset_pending_splice_funding_negotiation(&self) -> Option<bool> {
self.pending_splice.as_ref().map(|pending_splice| {
if pending_splice.can_abandon_funding_negotiation() {
true
} else {
self.context
.interactive_tx_signing_session
.as_ref()
.map(|signing_session| !signing_session.has_received_commitment_signed())
.unwrap_or_else(|| {
debug_assert!(false);
false
})
}
})
}

fn should_reset_pending_splice_state(&self) -> bool {
self.should_reset_pending_splice_funding_negotiation().unwrap_or(true)
&& self.pending_funding().is_empty()
}

fn reset_pending_splice_state(&mut self) -> bool {
debug_assert!(self.should_reset_pending_splice_funding_negotiation().unwrap_or(true));
self.context.channel_state.clear_quiescent();
self.context.interactive_tx_signing_session.take();
let has_funding_negotiation = self
.pending_splice
.as_mut()
.and_then(|pending_splice| pending_splice.funding_negotiation.take())
.is_some();
if self.should_reset_pending_splice_state() {
self.pending_splice.take();
}
has_funding_negotiation
}

#[rustfmt::skip]
fn check_remote_fee<F: Deref, L: Deref>(
channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator<F>,
Expand Down Expand Up @@ -8864,7 +8913,14 @@ where
}
self.context.channel_state.clear_local_stfu_sent();
self.context.channel_state.clear_remote_stfu_sent();
self.context.channel_state.clear_quiescent();
if self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) {
// If we were in quiescence but a splice was never negotiated, or the negotiation
// failed due to disconnecting, we shouldn't be quiescent anymore upon reconnecting.
// If there was a pending splice negotiation that has failed due to disconnecting,
// we also take the opportunity to clean up our state.
self.reset_pending_splice_state();
debug_assert!(!self.context.channel_state.is_quiescent());
}
}

self.context.channel_state.set_peer_disconnected();
Expand Down Expand Up @@ -13875,7 +13931,12 @@ where
}
channel_state.clear_local_stfu_sent();
channel_state.clear_remote_stfu_sent();
channel_state.clear_quiescent();
if self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) {
// If we were in quiescence but a splice was never negotiated, or the
// negotiation failed due to disconnecting, we shouldn't be quiescent
// anymore upon reconnecting.
channel_state.clear_quiescent();
}
},
ChannelState::FundingNegotiated(_)
if self.context.interactive_tx_signing_session.is_some() => {},
Expand Down Expand Up @@ -14238,6 +14299,20 @@ where
let holder_commitment_point_next = self.holder_commitment_point.next_point();
let holder_commitment_point_pending_next = self.holder_commitment_point.pending_next_point;

let interactive_tx_signing_session =
if self.should_reset_pending_splice_funding_negotiation().unwrap_or(false) {
None
} else {
self.context.interactive_tx_signing_session.as_ref()
};
let pending_splice = if self.should_reset_pending_splice_state() {
None
} else {
// We don't have to worry about resetting the pending `FundingNegotiation` because we
// can only read `FundingNegotiation::AwaitingSignatures` variants anyway.
self.pending_splice.as_ref()
};

write_tlv_fields!(writer, {
(0, self.context.announcement_sigs, option),
// minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
Expand Down Expand Up @@ -14281,12 +14356,12 @@ where
(53, funding_tx_broadcast_safe_event_emitted, option), // Added in 0.0.124
(55, removed_htlc_attribution_data, optional_vec), // Added in 0.2
(57, holding_cell_attribution_data, optional_vec), // Added in 0.2
(58, self.context.interactive_tx_signing_session, option), // Added in 0.2
(58, interactive_tx_signing_session, option), // Added in 0.2
(59, self.funding.minimum_depth_override, option), // Added in 0.2
(60, self.context.historical_scids, optional_vec), // Added in 0.2
(61, fulfill_attribution_data, optional_vec), // Added in 0.2
(63, holder_commitment_point_current, option), // Added in 0.2
(64, self.pending_splice, option), // Added in 0.2
(64, pending_splice, option), // Added in 0.2
(65, self.quiescent_action, option), // Added in 0.2
(67, pending_outbound_held_htlc_flags, optional_vec), // Added in 0.2
(69, holding_cell_held_htlc_flags, optional_vec), // Added in 0.2
Expand Down Expand Up @@ -14950,6 +15025,15 @@ where
}
};

if let Some(funding_negotiation) = pending_splice
.as_ref()
.and_then(|pending_splice| pending_splice.funding_negotiation.as_ref())
{
if !matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) {
return Err(DecodeError::InvalidValue);
}
}

Ok(FundedChannel {
funding: FundingScope {
value_to_self_msat,
Expand Down
60 changes: 60 additions & 0 deletions lightning/src/ln/quiescence_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::ln::functional_test_utils::*;
use crate::ln::msgs;
use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, ErrorAction, MessageSendEvent};
use crate::util::errors::APIError;
use crate::util::ser::Writeable;
use crate::util::test_channel_signer::SignerOp;

#[test]
Expand Down Expand Up @@ -699,3 +700,62 @@ fn test_quiescence_during_disconnection() {
do_test_quiescence_during_disconnection(false, true);
do_test_quiescence_during_disconnection(true, true);
}

#[test]
fn test_quiescence_termination_on_disconnect() {
do_test_quiescence_termination_on_disconnect(false);
do_test_quiescence_termination_on_disconnect(true);
}

fn do_test_quiescence_termination_on_disconnect(reload: bool) {
// Tests that we terminate quiescence on disconnect if there's no quiescence protocol taking place.
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let (persister_0a, persister_1a);
let (chain_monitor_0a, chain_monitor_1a);
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
let (node_0a, node_1a);
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let node_id_0 = nodes[0].node.get_our_node_id();
let node_id_1 = nodes[1].node.get_our_node_id();

let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1);

nodes[0].node.maybe_propose_quiescence(&node_id_1, &channel_id).unwrap();

let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1);
nodes[1].node.handle_stfu(node_id_0, &stfu);
let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0);
nodes[0].node.handle_stfu(node_id_1, &stfu);

if reload {
let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode();
reload_node!(
nodes[0],
&nodes[0].node.encode(),
&[&encoded_monitor_0],
persister_0a,
chain_monitor_0a,
node_0a
);
let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode();
reload_node!(
nodes[1],
&nodes[1].node.encode(),
&[&encoded_monitor_1],
persister_1a,
chain_monitor_1a,
node_1a
);
} else {
nodes[0].node.peer_disconnected(node_id_1);
nodes[1].node.peer_disconnected(node_id_0);
}

let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
reconnect_args.send_channel_ready = (true, true);
reconnect_nodes(reconnect_args);

send_payment(&nodes[0], &[&nodes[1]], 1_000_000);
}
Loading