diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index fab1a3cab8e..9678bcdcab4 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -332,20 +332,33 @@ fn test_async_commitment_signature_for_peer_disconnect() { #[test] fn test_async_commitment_signature_ordering_reestablish() { - do_test_async_commitment_signature_ordering(false, false); + do_test_async_commitment_signature_ordering(false, false, false); } #[test] fn test_async_commitment_signature_ordering_monitor_restored() { - do_test_async_commitment_signature_ordering(true, false); + do_test_async_commitment_signature_ordering(true, false, false); } #[test] -fn test_async_commitment_signature_ordering_monitor_restored_signer_restored_early() { - do_test_async_commitment_signature_ordering(true, true); +fn test_async_commitment_signature_ordering_monitor_restored_signer_restored_before_completion() { + // This tests that if we were pending a monitor update completion across a restart, + // and needed to send a CS then RAA, that if our signer is unblocked 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_ordering(true, true, false); } -fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, enable_signer_before_monitor_completion: bool) { +#[test] +fn test_async_commitment_signature_ordering_signer_restored_before_reestablish() { + // This tests that if we tried to send a commitment_signed, but our signer was blocked, + // if we disconnect, reconnect, unblock the signer, then handle channel_reestablish, + // that we don't send duplicate messages upon calling `signer_unblocked`. + do_test_async_commitment_signature_ordering(false, false, true); +} + +fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, + enable_signer_before_monitor_completion: bool, enable_signer_before_reestablish: bool) { // Across disconnects we may end up in a situation where we need to send a // commitment_signed and then revoke_and_ack. We need to make sure that if // the signer is pending for commitment_signed but not revoke_and_ack, we don't @@ -365,6 +378,12 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000); + if enable_signer_before_reestablish { + // When the node goes to sign the second commitment update, it will set that + // it's pending a commitment update, which will still be set when the peer + // reconnects. + nodes[0].disable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); + } // Start to send the second update_add_htlc + commitment_signed, but don't actually make it // to the peer. let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000); @@ -372,7 +391,11 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap(); check_added_monitors!(nodes[0], 1); - get_htlc_update_msgs(&nodes[0], &nodes[1].node.get_our_node_id()); + if enable_signer_before_reestablish { + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + } else { + get_htlc_update_msgs(&nodes[0], &nodes[1].node.get_our_node_id()); + } // Send back update_fulfill_htlc + commitment_signed for the first payment. nodes[1].node.claim_funds(payment_preimage_1); @@ -391,7 +414,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); } nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), commitment_signed); - if monitor_update_failure { + if monitor_update_failure || enable_signer_before_reestablish { assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); } else { let _ = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id()); @@ -421,14 +444,30 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena // With a fully working signer, here we would send a commitment_signed, // and then revoke_and_ack. With commitment_signed disabled, since // our ordering is CS then RAA, we should make sure we don't send the RAA. - nodes[0].disable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); + if enable_signer_before_reestablish { + nodes[0].enable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); + } else { + nodes[0].disable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); + } nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]); - let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]); - assert!(as_resp.0.is_none()); - assert!(as_resp.1.is_none()); - assert!(as_resp.2.is_none()); - let as_resp = if monitor_update_failure && enable_signer_before_monitor_completion { + let as_resp = if enable_signer_before_reestablish { + // We should send the CS + RAA now, and not again on `signer_unblocked`. + let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]); + + nodes[0].node.signer_unblocked(Some((nodes[1].node.get_our_node_id(), chan_id))); + let as_resp_empty = handle_chan_reestablish_msgs!(nodes[0], nodes[1]); + assert!(as_resp_empty.0.is_none()); + assert!(as_resp_empty.1.is_none()); + assert!(as_resp_empty.2.is_none()); + + as_resp + } else if monitor_update_failure && enable_signer_before_monitor_completion { + let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]); + assert!(as_resp.0.is_none()); + assert!(as_resp.1.is_none()); + assert!(as_resp.2.is_none()); + nodes[0].enable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment); chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); @@ -436,7 +475,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(outpoint, latest_update); check_added_monitors!(nodes[0], 0); - // We should send the CS now, and not send it again on `signer_unblocked`. + // We should send the CS + RAA now, and not again on `signer_unblocked`. let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]); nodes[0].node.signer_unblocked(Some((nodes[1].node.get_our_node_id(), chan_id))); @@ -446,6 +485,11 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena assert!(as_resp_empty.2.is_none()); as_resp } else { + let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]); + assert!(as_resp.0.is_none()); + assert!(as_resp.1.is_none()); + assert!(as_resp.2.is_none()); + if monitor_update_failure { chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); let (outpoint, latest_update, _) = nodes[0].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_id).unwrap().clone();