Skip to content

Commit a5b3bd2

Browse files
committed
Allow counterparty pending monitor update within quiescence handshake
Previously, if we were negotiating quiescence, and we had already sent our `stfu`, we'd disconnect upon receiving the counterparty's `stfu` if we had a pending monitor update. This could result from processing a counterparty's final `revoke_and_ack` to an update, and immediately processing their `stfu` (which is valid from their point of view) without complete the monitor update. This was unintended, as we are able to track the quiescent and pending monitor update flags at the same time. Note that this commit still considers whether our signer owes any messages, as these are indicative of a channel update still pending.
1 parent 5688166 commit a5b3bd2

File tree

2 files changed

+60
-2
lines changed

2 files changed

+60
-2
lines changed

lightning/src/ln/channel.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9675,10 +9675,14 @@ impl<SP: Deref> FundedChannel<SP> where
96759675
self.mark_response_received();
96769676

96779677
if self.context.is_waiting_on_peer_pending_channel_update()
9678-
|| self.context.is_monitor_or_signer_pending_channel_update()
9678+
|| self.context.signer_pending_revoke_and_ack
9679+
|| self.context.signer_pending_commitment_update
96799680
{
96809681
// Since we've already sent `stfu`, it should not be possible for one of our updates to
9681-
// be pending, so anything pending currently must be from a counterparty update.
9682+
// be pending, so anything pending currently must be from a counterparty update. We may
9683+
// have a monitor update pending if we've processed a message from the counterparty, but
9684+
// we don't consider this when becoming quiescent since the states are not mutually
9685+
// exclusive.
96829686
return Err(ChannelError::WarnAndDisconnect(
96839687
"Received counterparty stfu while having pending counterparty updates".to_owned()
96849688
));

lightning/src/ln/quiescence_tests.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,60 @@ fn test_quiescence_waits_for_async_signer_and_monitor_update() {
276276
send_payment(&nodes[0], &[&nodes[1]], payment_amount);
277277
}
278278

279+
#[test]
280+
fn test_quiescence_on_final_revoke_and_ack_pending_monitor_update() {
281+
// Test that we do not let a pending monitor update for a final `revoke_and_ack` prevent us from
282+
// entering quiescence. This was caught by the fuzzer, reported as #3805.
283+
let chanmon_cfgs = create_chanmon_cfgs(2);
284+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
285+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
286+
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
287+
let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
288+
289+
let node_id_0 = nodes[0].node.get_our_node_id();
290+
let node_id_1 = nodes[1].node.get_our_node_id();
291+
292+
let payment_amount = 1_000_000;
293+
let (route, payment_hash, _, payment_secret) =
294+
get_route_and_payment_hash!(&nodes[0], &nodes[1], payment_amount);
295+
let onion = RecipientOnionFields::secret_only(payment_secret);
296+
let payment_id = PaymentId(payment_hash.0);
297+
nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
298+
check_added_monitors(&nodes[0], 1);
299+
300+
nodes[1].node.maybe_propose_quiescence(&node_id_0, &chan_id).unwrap();
301+
let stfu = get_event_msg!(&nodes[1], MessageSendEvent::SendStfu, node_id_0);
302+
nodes[0].node.handle_stfu(node_id_1, &stfu);
303+
304+
let update_add = get_htlc_update_msgs!(&nodes[0], node_id_1);
305+
nodes[1].node.handle_update_add_htlc(node_id_0, &update_add.update_add_htlcs[0]);
306+
nodes[1].node.handle_commitment_signed_batch_test(node_id_0, &update_add.commitment_signed);
307+
check_added_monitors(&nodes[1], 1);
308+
309+
let (revoke_and_ack, commit_sig) = get_revoke_commit_msgs!(&nodes[1], node_id_0);
310+
nodes[0].node.handle_revoke_and_ack(node_id_1, &revoke_and_ack);
311+
check_added_monitors(&nodes[0], 1);
312+
nodes[0].node.handle_commitment_signed_batch_test(node_id_1, &commit_sig);
313+
check_added_monitors(&nodes[0], 1);
314+
315+
chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
316+
let msgs = nodes[0].node.get_and_clear_pending_msg_events();
317+
if let MessageSendEvent::SendRevokeAndACK { msg, .. } = &msgs[0] {
318+
nodes[1].node.handle_revoke_and_ack(node_id_0, &msg);
319+
check_added_monitors(&nodes[1], 1);
320+
} else {
321+
panic!();
322+
}
323+
if let MessageSendEvent::SendStfu { msg, .. } = &msgs[1] {
324+
nodes[1].node.handle_stfu(node_id_0, &msg);
325+
} else {
326+
panic!();
327+
}
328+
329+
nodes[0].node.exit_quiescence(&node_id_1, &chan_id).unwrap();
330+
nodes[1].node.exit_quiescence(&node_id_0, &chan_id).unwrap();
331+
}
332+
279333
#[test]
280334
fn test_quiescence_updates_go_to_holding_cell() {
281335
quiescence_updates_go_to_holding_cell(false);

0 commit comments

Comments
 (0)