X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchanmon_update_fail_tests.rs;h=5b821bfb868df77523a3ba8ec32a55ab9db5a680;hb=920d96edb6595289902f287419de2d002e2dc2ee;hp=af827b8cebbc3a60cfe2a46ba02ad37cc0d59abf;hpb=50eba2624f8f6f6eeb02690f70a6ac725bf44d81;p=rust-lightning diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index af827b8c..5b821bfb 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -14,7 +14,7 @@ use bitcoin::blockdata::constants::genesis_block; use bitcoin::hash_types::BlockHash; -use bitcoin::network::constants::Network; +use bitcoin::network::Network; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor}; use crate::chain::transaction::OutPoint; use crate::chain::{ChannelMonitorUpdateStatus, Listen, Watch}; @@ -22,6 +22,7 @@ use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentP use crate::ln::channelmanager::{RAACommitmentOrder, PaymentSendFailure, PaymentId, RecipientOnionFields}; use crate::ln::channel::{AnnouncementSigsState, ChannelPhase}; use crate::ln::msgs; +use crate::ln::types::ChannelId; use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler}; use crate::util::test_channel_signer::TestChannelSigner; use crate::util::errors::APIError; @@ -173,11 +174,11 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); assert_eq!(via_channel_id, Some(channel_id)); match &purpose { - PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); assert_eq!(payment_secret_1, *payment_secret); }, - _ => panic!("expected PaymentPurpose::InvoicePayment") + _ => panic!("expected PaymentPurpose::Bolt11InvoicePayment") } }, _ => panic!("Unexpected event"), @@ -206,7 +207,8 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) { } // ...and make sure we can force-close a frozen channel - nodes[0].node.force_close_broadcasting_latest_txn(&channel_id, &nodes[1].node.get_our_node_id()).unwrap(); + let error_message = "Channel force-closed"; + nodes[0].node.force_close_broadcasting_latest_txn(&channel_id, &nodes[1].node.get_our_node_id(), error_message.to_string()).unwrap(); check_added_monitors!(nodes[0], 1); check_closed_broadcast!(nodes[0], true); @@ -554,11 +556,11 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) { assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); assert_eq!(via_channel_id, Some(channel_id)); match &purpose { - PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); assert_eq!(payment_secret_2, *payment_secret); }, - _ => panic!("expected PaymentPurpose::InvoicePayment") + _ => panic!("expected PaymentPurpose::Bolt11InvoicePayment") } }, _ => panic!("Unexpected event"), @@ -672,11 +674,11 @@ fn test_monitor_update_fail_cs() { assert_eq!(receiver_node_id.unwrap(), nodes[1].node.get_our_node_id()); assert_eq!(via_channel_id, Some(channel_id)); match &purpose { - PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); assert_eq!(our_payment_secret, *payment_secret); }, - _ => panic!("expected PaymentPurpose::InvoicePayment") + _ => panic!("expected PaymentPurpose::Bolt11InvoicePayment") } }, _ => panic!("Unexpected event"), @@ -1683,11 +1685,11 @@ fn test_monitor_update_fail_claim() { assert_eq!(via_channel_id, Some(channel_id)); assert_eq!(via_user_channel_id, Some(42)); match &purpose { - PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); assert_eq!(payment_secret_2, *payment_secret); }, - _ => panic!("expected PaymentPurpose::InvoicePayment") + _ => panic!("expected PaymentPurpose::Bolt11InvoicePayment") } }, _ => panic!("Unexpected event"), @@ -1699,11 +1701,11 @@ fn test_monitor_update_fail_claim() { assert_eq!(receiver_node_id.unwrap(), nodes[0].node.get_our_node_id()); assert_eq!(via_channel_id, Some(channel_id)); match &purpose { - PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => { + PaymentPurpose::Bolt11InvoicePayment { payment_preimage, payment_secret, .. } => { assert!(payment_preimage.is_none()); assert_eq!(payment_secret_3, *payment_secret); }, - _ => panic!("expected PaymentPurpose::InvoicePayment") + _ => panic!("expected PaymentPurpose::Bolt11InvoicePayment") } }, _ => panic!("Unexpected event"), @@ -1861,7 +1863,7 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); - let channel_id = OutPoint { txid: funding_created_msg.funding_txid, index: funding_created_msg.funding_output_index }.to_channel_id(); + let channel_id = ChannelId::v1_from_funding_outpoint(OutPoint { txid: funding_created_msg.funding_txid, index: funding_created_msg.funding_output_index }); nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg); check_added_monitors!(nodes[1], 1); @@ -1955,8 +1957,8 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: send_payment(&nodes[0], &[&nodes[1]], 8000000); close_channel(&nodes[0], &nodes[1], &channel_id, funding_tx, true); - check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); + check_closed_event!(nodes[0], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); } #[test] @@ -2024,7 +2026,9 @@ fn test_path_paused_mpp() { assert_eq!(events.len(), 1); pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash.clone(), Some(payment_secret), events.pop().unwrap(), true, None); - claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage); + claim_payment_along_route( + ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], payment_preimage) + ); } #[test] @@ -2634,8 +2638,8 @@ fn test_temporary_error_during_shutdown() { assert_eq!(txn_a, txn_b); assert_eq!(txn_a.len(), 1); check_spends!(txn_a[0], funding_tx); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); - check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); + check_closed_event!(nodes[0], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); } #[test] @@ -3221,17 +3225,18 @@ fn do_test_durable_preimages_on_closed_channel(close_chans_before_reload: bool, let _ = get_revoke_commit_msgs!(nodes[1], nodes[2].node.get_our_node_id()); let mon_bc = get_monitor!(nodes[1], chan_id_bc).encode(); + let error_message = "Channel force-closed"; if close_chans_before_reload { if !close_only_a { chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); - nodes[1].node.force_close_broadcasting_latest_txn(&chan_id_bc, &nodes[2].node.get_our_node_id()).unwrap(); + nodes[1].node.force_close_broadcasting_latest_txn(&chan_id_bc, &nodes[2].node.get_our_node_id(), error_message.to_string()).unwrap(); check_closed_broadcast(&nodes[1], 1, true); check_closed_event(&nodes[1], 1, ClosureReason::HolderForceClosed, false, &[nodes[2].node.get_our_node_id()], 100000); } chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); - nodes[1].node.force_close_broadcasting_latest_txn(&chan_id_ab, &nodes[0].node.get_our_node_id()).unwrap(); + nodes[1].node.force_close_broadcasting_latest_txn(&chan_id_ab, &nodes[0].node.get_our_node_id(), error_message.to_string()).unwrap(); check_closed_broadcast(&nodes[1], 1, true); check_closed_event(&nodes[1], 1, ClosureReason::HolderForceClosed, false, &[nodes[0].node.get_our_node_id()], 100000); } @@ -3252,8 +3257,9 @@ fn do_test_durable_preimages_on_closed_channel(close_chans_before_reload: bool, assert_eq!(bs_close_txn.len(), 3); } } + let error_message = "Channel force-closed"; - nodes[0].node.force_close_broadcasting_latest_txn(&chan_id_ab, &nodes[1].node.get_our_node_id()).unwrap(); + nodes[0].node.force_close_broadcasting_latest_txn(&chan_id_ab, &nodes[1].node.get_our_node_id(), error_message.to_string()).unwrap(); check_closed_event(&nodes[0], 1, ClosureReason::HolderForceClosed, false, &[nodes[1].node.get_our_node_id()], 100000); let as_closing_tx = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); assert_eq!(as_closing_tx.len(), 1); @@ -3331,6 +3337,93 @@ fn test_durable_preimages_on_closed_channel() { do_test_durable_preimages_on_closed_channel(false, false, false); } +#[test] +fn test_sync_async_persist_doesnt_hang() { + // Previously, we checked if a channel was a candidate for making forward progress based on if + // the `MonitorEvent::Completed` matched the channel's latest monitor update id. However, this + // could lead to a rare race when `ChannelMonitor`s were being persisted both synchronously and + // asynchronously leading to channel hangs. + // + // To hit this case, we need to generate a `MonitorEvent::Completed` prior to a new channel + // update, but which is only processed after the channel update. + 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]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let chan_id_ab = create_announced_chan_between_nodes(&nodes, 0, 1).2; + + // Send two payments from A to B, then claim the first, marking the very last + // ChannelMonitorUpdate as InProgress... + let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000); + let (payment_preimage_2, payment_hash_2, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000); + + nodes[1].node.claim_funds(payment_preimage_1); + check_added_monitors(&nodes[1], 1); + expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000); + + let bs_updates = get_htlc_update_msgs(&nodes[1], &nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.update_fulfill_htlcs[0]); + expect_payment_sent(&nodes[0], payment_preimage_1, None, false, false); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_updates.commitment_signed); + check_added_monitors(&nodes[0], 1); + let (as_raa, as_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_raa); + check_added_monitors(&nodes[1], 1); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_cs); + check_added_monitors(&nodes[1], 1); + + let bs_final_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress); + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_final_raa); + check_added_monitors(&nodes[0], 1); + + // Immediately complete the monitor update, but before the ChannelManager has a chance to see + // the MonitorEvent::Completed, create a channel update by receiving a claim on the second + // payment. + let (outpoint, _, ab_update_id) = nodes[0].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_id_ab).unwrap().clone(); + nodes[0].chain_monitor.chain_monitor.channel_monitor_updated(outpoint, ab_update_id).unwrap(); + + nodes[1].node.claim_funds(payment_preimage_2); + check_added_monitors(&nodes[1], 1); + expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000); + + let bs_updates = get_htlc_update_msgs(&nodes[1], &nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.update_fulfill_htlcs[0]); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_updates.commitment_signed); + check_added_monitors(&nodes[0], 1); + + // At this point, we have completed an extra `ChannelMonitorUpdate` but the `ChannelManager` + // hasn't yet seen our `MonitorEvent::Completed`. When we call + // `get_and_clear_pending_msg_events` here, the `ChannelManager` finally sees that event and + // should return the channel to normal operation. + let (as_raa, as_cs) = get_revoke_commit_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + + // Now that we've completed our test, process the events we have queued up (which we were not + // able to check until now as they would have caused the `ChannelManager` to look at the + // pending `MonitorEvent`s). + let pending_events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(pending_events.len(), 2); + if let Event::PaymentPathSuccessful { ref payment_hash, ..} = pending_events[1] { + assert_eq!(payment_hash.unwrap(), payment_hash_1); + } else { panic!(); } + if let Event::PaymentSent { ref payment_hash, ..} = pending_events[0] { + assert_eq!(*payment_hash, payment_hash_2); + } else { panic!(); } + + // Finally, complete the claiming of the second payment + nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_raa); + check_added_monitors(&nodes[1], 1); + nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_cs); + check_added_monitors(&nodes[1], 1); + + let bs_final_raa = get_event_msg!(nodes[1], MessageSendEvent::SendRevokeAndACK, nodes[0].node.get_our_node_id()); + nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_final_raa); + check_added_monitors(&nodes[0], 1); + expect_payment_path_successful!(nodes[0]); +} + fn do_test_reload_mon_update_completion_actions(close_during_reload: bool) { // Test that if a `ChannelMonitorUpdate` completes but a `ChannelManager` isn't serialized // before restart we run the monitor update completion action on startup. @@ -3390,10 +3483,11 @@ fn do_test_reload_mon_update_completion_actions(close_during_reload: bool) { let manager_b = nodes[1].node.encode(); reload_node!(nodes[1], &manager_b, &[&mon_ab, &mon_bc], persister, new_chain_monitor, nodes_1_deserialized); + let error_message = "Channel force-closed"; if close_during_reload { // Test that we still free the B<->C channel if the A<->B channel closed while we reloaded // (as learned about during the on-reload block connection). - nodes[0].node.force_close_broadcasting_latest_txn(&chan_id_ab, &nodes[1].node.get_our_node_id()).unwrap(); + nodes[0].node.force_close_broadcasting_latest_txn(&chan_id_ab, &nodes[1].node.get_our_node_id(), error_message.to_string()).unwrap(); check_added_monitors!(nodes[0], 1); check_closed_broadcast!(nodes[0], true); check_closed_event(&nodes[0], 1, ClosureReason::HolderForceClosed, false, &[nodes[1].node.get_our_node_id()], 100_000); @@ -3404,7 +3498,8 @@ fn do_test_reload_mon_update_completion_actions(close_during_reload: bool) { let bc_update_id = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_id_bc).unwrap().2; let mut events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), if close_during_reload { 2 } else { 1 }); - expect_payment_forwarded(events.pop().unwrap(), &nodes[1], &nodes[0], &nodes[2], Some(1000), close_during_reload, false); + expect_payment_forwarded(events.pop().unwrap(), &nodes[1], &nodes[0], &nodes[2], Some(1000), + None, close_during_reload, false, false); if close_during_reload { match events[0] { Event::ChannelClosed { .. } => {},