Skip to content

Commit e418d45

Browse files
committed
Wipe splice state upon failed interactive funding construction
An interactive funding construction can be considered failed upon a disconnect or a `tx_abort` message. So far, we've consumed the `InteractiveTxConstructor` in the latter case, but not the former. Additionally, we may have splice-specific state that needs to be consumed as well to allow us to negotiate another splice later on. This commit ensures that we properly consume all splice and interactive funding state whenever possible upon a disconnect or `tx_abort`. The interactive funding state is safe to consume as long as we have either yet to reach `AwaitingSignatures`, or we have but `tx_signatures` has not been sent/received. The splice state is safe to consume as long as we don't have a pending `FundingNegotiation::AwaitingSignatures` with a `tx_signatures` sent/received and we don't have any negotiated candidates. Note that until splice RBF is supported, it is not currently possible to have any negotiated candidates with a pending interactive funding transaction.
1 parent 025d49d commit e418d45

File tree

2 files changed

+192
-10
lines changed

2 files changed

+192
-10
lines changed

lightning/src/ln/channel.rs

Lines changed: 79 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,10 +1694,17 @@ where
16941694
},
16951695
ChannelPhase::Funded(funded_channel) => {
16961696
funded_channel.context.channel_state.clear_quiescent();
1697-
funded_channel
1698-
.pending_splice
1699-
.as_mut()
1700-
.and_then(|pending_splice| pending_splice.funding_negotiation.take());
1697+
if funded_channel.should_reset_pending_splice_funding_negotiation() {
1698+
funded_channel
1699+
.pending_splice
1700+
.as_mut()
1701+
.and_then(|pending_splice| pending_splice.funding_negotiation.take());
1702+
} else {
1703+
debug_assert!(false);
1704+
}
1705+
if funded_channel.should_reset_pending_splice_state() {
1706+
funded_channel.pending_splice.take();
1707+
}
17011708
},
17021709
};
17031710

@@ -1826,11 +1833,21 @@ where
18261833
},
18271834
ChannelPhase::Funded(funded_channel) => {
18281835
funded_channel.context.channel_state.clear_quiescent();
1829-
funded_channel
1830-
.pending_splice
1831-
.as_mut()
1832-
.and_then(|pending_splice| pending_splice.funding_negotiation.take())
1833-
.is_some()
1836+
let should_ack = if funded_channel.should_reset_pending_splice_funding_negotiation()
1837+
{
1838+
funded_channel
1839+
.pending_splice
1840+
.as_mut()
1841+
.and_then(|pending_splice| pending_splice.funding_negotiation.take())
1842+
.is_some()
1843+
} else {
1844+
debug_assert!(false);
1845+
false
1846+
};
1847+
if funded_channel.should_reset_pending_splice_state() {
1848+
funded_channel.pending_splice.take();
1849+
}
1850+
should_ack
18341851
},
18351852
};
18361853

@@ -6734,6 +6751,32 @@ where
67346751
)
67356752
}
67366753

6754+
fn should_reset_pending_splice_funding_negotiation(&self) -> bool {
6755+
self.pending_splice
6756+
.as_ref()
6757+
.and_then(|pending_splice| pending_splice.funding_negotiation.as_ref())
6758+
.map(|funding_negotiation| {
6759+
let is_awaiting_sigs =
6760+
matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. });
6761+
let sent_or_received_tx_signatures = self
6762+
.context
6763+
.interactive_tx_signing_session
6764+
.as_ref()
6765+
.map(|signing_session| {
6766+
signing_session.holder_tx_signatures().is_some()
6767+
|| signing_session.has_received_tx_signatures()
6768+
})
6769+
.unwrap_or(false);
6770+
!is_awaiting_sigs || !sent_or_received_tx_signatures
6771+
})
6772+
.unwrap_or(false)
6773+
}
6774+
6775+
fn should_reset_pending_splice_state(&self) -> bool {
6776+
(self.should_reset_pending_splice_funding_negotiation() || self.pending_splice.is_some())
6777+
&& self.pending_funding().is_empty()
6778+
}
6779+
67376780
#[rustfmt::skip]
67386781
fn check_remote_fee<F: Deref, L: Deref>(
67396782
channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator<F>,
@@ -8862,6 +8905,15 @@ where
88628905
self.context.channel_state.clear_quiescent();
88638906
}
88648907

8908+
if self.should_reset_pending_splice_funding_negotiation() {
8909+
self.pending_splice
8910+
.as_mut()
8911+
.and_then(|pending_splice| pending_splice.funding_negotiation.take());
8912+
}
8913+
if self.should_reset_pending_splice_state() {
8914+
self.pending_splice.take();
8915+
}
8916+
88658917
self.context.channel_state.set_peer_disconnected();
88668918
log_trace!(logger, "Peer disconnection resulted in {} remote-announced HTLC drops on channel {}", inbound_drop_count, &self.context.channel_id());
88678919
Ok(())
@@ -14233,6 +14285,14 @@ where
1423314285
let holder_commitment_point_next = self.holder_commitment_point.next_point();
1423414286
let holder_commitment_point_pending_next = self.holder_commitment_point.pending_next_point;
1423514287

14288+
let pending_splice = if self.should_reset_pending_splice_state() {
14289+
None
14290+
} else {
14291+
// We don't have to worry about resetting the pending `FundingNegotiation` because we
14292+
// can only read `FundingNegotiation::AwaitingSignatures` variants anyway.
14293+
self.pending_splice.as_ref()
14294+
};
14295+
1423614296
write_tlv_fields!(writer, {
1423714297
(0, self.context.announcement_sigs, option),
1423814298
// minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
@@ -14281,7 +14341,7 @@ where
1428114341
(60, self.context.historical_scids, optional_vec), // Added in 0.2
1428214342
(61, fulfill_attribution_data, optional_vec), // Added in 0.2
1428314343
(63, holder_commitment_point_current, option), // Added in 0.2
14284-
(64, self.pending_splice, option), // Added in 0.2
14344+
(64, pending_splice, option), // Added in 0.2
1428514345
(65, self.quiescent_action, option), // Added in 0.2
1428614346
(67, pending_outbound_held_htlc_flags, optional_vec), // Added in 0.2
1428714347
(69, holding_cell_held_htlc_flags, optional_vec), // Added in 0.2
@@ -14945,6 +15005,15 @@ where
1494515005
}
1494615006
};
1494715007

15008+
if let Some(funding_negotiation) = pending_splice
15009+
.as_ref()
15010+
.and_then(|pending_splice| pending_splice.funding_negotiation.as_ref())
15011+
{
15012+
if !matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) {
15013+
return Err(DecodeError::InvalidValue);
15014+
}
15015+
}
15016+
1494815017
Ok(FundedChannel {
1494915018
funding: FundingScope {
1495015019
value_to_self_msat,

lightning/src/ln/splicing_tests.rs

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use crate::ln::funding::{FundingTxInput, SpliceContribution};
1919
use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, MessageSendEvent};
2020
use crate::ln::types::ChannelId;
2121
use crate::util::errors::APIError;
22+
use crate::util::ser::Writeable;
2223

2324
use bitcoin::{Amount, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut};
2425

@@ -315,6 +316,118 @@ fn lock_splice_after_blocks<'a, 'b, 'c, 'd>(
315316
node_b.chain_source.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script);
316317
}
317318

319+
#[test]
320+
fn test_splice_state_reset_on_disconnect() {
321+
do_test_splice_state_reset_on_disconnect(false);
322+
do_test_splice_state_reset_on_disconnect(true);
323+
}
324+
325+
fn do_test_splice_state_reset_on_disconnect(reload: bool) {
326+
// Tests that we're able to forget our pending splice state after a disconnect such that we can
327+
// retry later.
328+
let chanmon_cfgs = create_chanmon_cfgs(2);
329+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
330+
let (persister_0a, persister_0b, persister_1a, persister_1b);
331+
let (chain_monitor_0a, chain_monitor_0b, chain_monitor_1a, chain_monitor_1b);
332+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
333+
let (node_0a, node_0b, node_1a, node_1b);
334+
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
335+
336+
let node_id_0 = nodes[0].node.get_our_node_id();
337+
let node_id_1 = nodes[1].node.get_our_node_id();
338+
339+
let (_, _, channel_id, _) =
340+
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 50_000_000);
341+
342+
let contribution = SpliceContribution::SpliceOut {
343+
outputs: vec![TxOut {
344+
value: Amount::from_sat(1_000),
345+
script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
346+
}],
347+
};
348+
nodes[0]
349+
.node
350+
.splice_channel(
351+
&channel_id,
352+
&node_id_1,
353+
contribution.clone(),
354+
FEERATE_FLOOR_SATS_PER_KW,
355+
None,
356+
)
357+
.unwrap();
358+
359+
let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1);
360+
nodes[1].node.handle_stfu(node_id_0, &stfu);
361+
let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0);
362+
nodes[0].node.handle_stfu(node_id_1, &stfu);
363+
364+
let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1);
365+
nodes[1].node.handle_splice_init(node_id_0, &splice_init);
366+
let _ = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0);
367+
368+
if reload {
369+
let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode();
370+
reload_node!(
371+
nodes[0],
372+
&nodes[0].node.encode(),
373+
&[&encoded_monitor_0],
374+
persister_0a,
375+
chain_monitor_0a,
376+
node_0a
377+
);
378+
let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode();
379+
reload_node!(
380+
nodes[1],
381+
&nodes[1].node.encode(),
382+
&[&encoded_monitor_1],
383+
persister_1a,
384+
chain_monitor_1a,
385+
node_1a
386+
);
387+
} else {
388+
nodes[0].node.peer_disconnected(node_id_1);
389+
nodes[1].node.peer_disconnected(node_id_0);
390+
}
391+
392+
let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
393+
reconnect_args.send_channel_ready = (true, true);
394+
reconnect_nodes(reconnect_args);
395+
396+
let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, contribution);
397+
398+
if reload {
399+
let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode();
400+
reload_node!(
401+
nodes[0],
402+
&nodes[0].node.encode(),
403+
&[&encoded_monitor_0],
404+
persister_0b,
405+
chain_monitor_0b,
406+
node_0b
407+
);
408+
let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode();
409+
reload_node!(
410+
nodes[1],
411+
&nodes[1].node.encode(),
412+
&[&encoded_monitor_1],
413+
persister_1b,
414+
chain_monitor_1b,
415+
node_1b
416+
);
417+
} else {
418+
nodes[0].node.peer_disconnected(node_id_1);
419+
nodes[1].node.peer_disconnected(node_id_0);
420+
}
421+
422+
let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
423+
reconnect_args.send_channel_ready = (true, true);
424+
reconnect_nodes(reconnect_args);
425+
426+
mine_transaction(&nodes[0], &splice_tx);
427+
mine_transaction(&nodes[1], &splice_tx);
428+
lock_splice_after_blocks(&nodes[0], &nodes[1], channel_id, ANTI_REORG_DELAY - 1);
429+
}
430+
318431
#[test]
319432
fn test_splice_in() {
320433
let chanmon_cfgs = create_chanmon_cfgs(2);

0 commit comments

Comments
 (0)