Skip to content

Commit c608592

Browse files
committed
Trigger disconnect on pending holder stfu
If our counterparty proposes quiescence and sends their `stfu`, we may not be able to respond back with our own immediately if we have a an async monitor update/signer request still pending. To ensure the channel can continue making progress, we trigger a disconnect if we're in this state for too long.
1 parent c4d23bc commit c608592

File tree

2 files changed

+63
-22
lines changed

2 files changed

+63
-22
lines changed

lightning/src/ln/channel.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7415,6 +7415,8 @@ impl<SP: Deref> FundedChannel<SP> where
74157415
self.context.channel_state.is_peer_disconnected()
74167416
// Cleared upon receiving `stfu`.
74177417
|| self.context.channel_state.is_local_stfu_sent()
7418+
// Cleared upon becoming quiescent.
7419+
|| self.context.channel_state.is_remote_stfu_sent()
74187420
// Cleared upon receiving a message that triggers the end of quiescence.
74197421
|| self.context.channel_state.is_quiescent()
74207422
// Cleared upon receiving `revoke_and_ack`.

lightning/src/ln/quiescence_tests.rs

Lines changed: 61 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -445,18 +445,62 @@ fn test_quiescence_timeout() {
445445
}
446446

447447
let f = |event| {
448-
if let MessageSendEvent::HandleError { action, .. } = event {
449-
if let msgs::ErrorAction::DisconnectPeerWithWarning { .. } = action {
450-
Some(())
451-
} else {
452-
None
453-
}
454-
} else {
455-
None
456-
}
448+
matches!(
449+
event, MessageSendEvent::HandleError { action , .. }
450+
if matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })
451+
)
457452
};
458-
assert!(nodes[0].node.get_and_clear_pending_msg_events().into_iter().find_map(f).is_some());
459-
assert!(nodes[1].node.get_and_clear_pending_msg_events().into_iter().find_map(f).is_some());
453+
assert!(nodes[0].node.get_and_clear_pending_msg_events().into_iter().any(f));
454+
assert!(nodes[1].node.get_and_clear_pending_msg_events().into_iter().any(f));
455+
}
456+
457+
#[test]
458+
fn test_quiescence_timeout_while_waiting_for_holder_stfu() {
459+
// Test that we'll disconnect if the holder does not send their stfu within a reasonable
460+
// time if the counterparty already sent one.
461+
let chanmon_cfgs = create_chanmon_cfgs(2);
462+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
463+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
464+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
465+
let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
466+
467+
let node_id_0 = nodes[0].node.get_our_node_id();
468+
let node_id_1 = nodes[1].node.get_our_node_id();
469+
470+
nodes[1].node.maybe_propose_quiescence(&node_id_0, &chan_id).unwrap();
471+
let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0);
472+
473+
// Don't allow nodes[1] to complete monitor updates, such that it cannot send `stfu` after
474+
// receiving one because it has a pending channel update.
475+
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
476+
477+
let (route, payment_hash, _, payment_secret) =
478+
get_route_and_payment_hash!(&nodes[0], &nodes[1], 1_000_000);
479+
let onion = RecipientOnionFields::secret_only(payment_secret);
480+
let payment_id = PaymentId(payment_hash.0);
481+
nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
482+
check_added_monitors(&nodes[0], 1);
483+
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
484+
485+
nodes[0].node.handle_stfu(node_id_1, &stfu);
486+
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
487+
488+
for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS {
489+
nodes[0].node.timer_tick_occurred();
490+
nodes[1].node.timer_tick_occurred();
491+
}
492+
493+
// Both nodes should trigger a disconnect:
494+
// - nodes[0] because they were not able to complete the monitor update in time
495+
// - nodes[1] because they did not receive a `stfu` in time
496+
let f = |event| {
497+
matches!(
498+
event, MessageSendEvent::HandleError { action , .. }
499+
if matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })
500+
)
501+
};
502+
assert!(nodes[0].node.get_and_clear_pending_msg_events().into_iter().any(f));
503+
assert!(nodes[1].node.get_and_clear_pending_msg_events().into_iter().any(f));
460504
}
461505

462506
#[test]
@@ -487,16 +531,11 @@ fn test_quiescence_timeout_while_waiting_for_counterparty_stfu() {
487531
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
488532

489533
// nodes[1] didn't receive nodes[0]'s stfu within the timeout so it'll disconnect.
490-
let f = |&ref event| {
491-
if let MessageSendEvent::HandleError { action, .. } = event {
492-
if let msgs::ErrorAction::DisconnectPeerWithWarning { .. } = action {
493-
Some(())
494-
} else {
495-
None
496-
}
497-
} else {
498-
None
499-
}
534+
let f = |event| {
535+
matches!(
536+
event, MessageSendEvent::HandleError { action , .. }
537+
if matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })
538+
)
500539
};
501-
assert!(nodes[1].node.get_and_clear_pending_msg_events().iter().find_map(f).is_some());
540+
assert!(nodes[1].node.get_and_clear_pending_msg_events().into_iter().any(f));
502541
}

0 commit comments

Comments
 (0)