From 221ef18afa268cd627172bc3d5a241b18c4d8848 Mon Sep 17 00:00:00 2001 From: Alec Chen Date: Wed, 3 Jul 2024 13:07:02 -0700 Subject: [PATCH] f - test enabling signer before reestablish + monitor completion, no double send --- lightning/src/ln/async_signer_tests.rs | 87 +++++++++++++++++++++----- 1 file changed, 73 insertions(+), 14 deletions(-) diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index a21f2fd3978..f9fd0eea043 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -273,7 +273,28 @@ fn test_async_commitment_signature_for_funding_signed_0conf() { } #[test] -fn test_async_commitment_signature_for_peer_disconnect() { +fn test_async_commitment_signature_peer_disconnect() { + do_test_async_commitment_signature_peer_disconnect(0); +} + +#[test] +fn test_async_commitment_signature_peer_disconnect_signer_restored_before_monitor_completion() { + // This tests that if we were pending a monitor update completion across a disconnect, + // and needed to send a CS, that if our signer becomes available before the monitor + // update completes, then we don't send duplicate messages upon calling `signer_unblocked` + // after the monitor update completes. + do_test_async_commitment_signature_peer_disconnect(1); +} + +#[test] +fn test_async_commitment_signature_peer_disconnect_signer_restored_before_reestablish() { + // This tests that if we tried to send a commitment_signed, but our signer was blocked, + // if we disconnect, reconnect, the signer becomes available, then handle channel_reestablish, + // that we don't send duplicate messages upon calling `signer_unblocked`. + do_test_async_commitment_signature_peer_disconnect(2); +} + +fn do_test_async_commitment_signature_peer_disconnect(test_case: u8) { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); @@ -299,34 +320,72 @@ fn test_async_commitment_signature_for_peer_disconnect() { dst.node.handle_update_add_htlc(&src.node.get_our_node_id(), &payment_event.msgs[0]); + if test_case == 1 { + // Fail to persist the monitor update when handling the commitment_signed. + chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + } + // Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a // `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`. dst.disable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg); check_added_monitors(dst, 1); - get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id()); + if test_case != 1 { + get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id()); + } // Now disconnect and reconnect the peers. src.node.peer_disconnected(&dst.node.get_our_node_id()); dst.node.peer_disconnected(&src.node.get_our_node_id()); - let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); - reconnect_args.send_channel_ready = (false, false); - reconnect_args.pending_raa = (true, false); - reconnect_nodes(reconnect_args); + + // do reestablish stuff + src.node.peer_connected(&dst.node.get_our_node_id(), &msgs::Init { + features: dst.node.init_features(), networks: None, remote_network_address: None + }, true).unwrap(); + let reestablish_1 = get_chan_reestablish_msgs!(src, dst); + assert_eq!(reestablish_1.len(), 1); + dst.node.peer_connected(&src.node.get_our_node_id(), &msgs::Init { + features: src.node.init_features(), networks: None, remote_network_address: None + }, false).unwrap(); + let reestablish_2 = get_chan_reestablish_msgs!(dst, src); + assert_eq!(reestablish_2.len(), 1); + + if test_case == 2 { + // Reenable the signer before the reestablish. + dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); + } + + dst.node.handle_channel_reestablish(&src.node.get_our_node_id(), &reestablish_1[0]); + + if test_case == 1 { + dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); + chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); + let (outpoint, latest_update, _) = dst.chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_id).unwrap().clone(); + dst.chain_monitor.chain_monitor.force_channel_monitor_updated(outpoint, latest_update); + check_added_monitors!(dst, 0); + } + + // Expect the RAA + let (_, revoke_and_ack, commitment_signed, _) = handle_chan_reestablish_msgs!(dst, src); + assert!(revoke_and_ack.is_some()); + if test_case == 0 { + assert!(commitment_signed.is_none()); + } else { + assert!(commitment_signed.is_some()); + } // Mark dst's signer as available and retry: we now expect to see dst's `commitment_signed`. dst.enable_channel_signer_op(&src.node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id))); - { - let events = dst.node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1, "expected one message, got {}", events.len()); - if let MessageSendEvent::UpdateHTLCs { ref node_id, .. } = events[0] { - assert_eq!(node_id, &src.node.get_our_node_id()); - } else { - panic!("expected UpdateHTLCs message, not {:?}", events[0]); - }; + if test_case == 0 { + let (_, _, commitment_signed, _) = handle_chan_reestablish_msgs!(dst, src); + assert!(commitment_signed.is_some()); + } else { + // Make sure we don't double send the CS. + let (_, _, commitment_signed, _) = handle_chan_reestablish_msgs!(dst, src); + assert!(commitment_signed.is_none()); } }