From: Valentine Wallace Date: Mon, 20 Sep 2021 16:18:49 +0000 (-0400) Subject: Rename PaymentFailed -> PaymentPathFailed X-Git-Tag: v0.0.101~6^2~1 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=bf16dfd15395b0e06703620325d0f90a8ba4b01d;p=rust-lightning Rename PaymentFailed -> PaymentPathFailed Since we don't want to imply to users that a payment has completely failed when it really has just partially failed --- diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index f85ac9b9d..4177b5027 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -827,7 +827,7 @@ pub fn do_test(data: &[u8], out: Out) { } }, events::Event::PaymentSent { .. } => {}, - events::Event::PaymentFailed { .. } => {}, + events::Event::PaymentPathFailed { .. } => {}, events::Event::PaymentForwarded { .. } if $node == 1 => {}, events::Event::PendingHTLCsForwardable { .. } => { nodes[$node].process_pending_htlc_forwards(); diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 36f226823..c3262f173 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -78,7 +78,7 @@ fn do_test_simple_monitor_permanent_update_fail(persister_fail: bool) { }; // TODO: Once we hit the chain with the failure transaction we should check that we get a - // PaymentFailed event + // PaymentPathFailed event assert_eq!(nodes[0].node.list_channels().len(), 0); check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() }); @@ -267,7 +267,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail check_closed_broadcast!(nodes[0], true); // TODO: Once we hit the chain with the failure transaction we should check that we get a - // PaymentFailed event + // PaymentPathFailed event assert_eq!(nodes[0].node.list_channels().len(), 0); check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed); @@ -1819,7 +1819,7 @@ fn test_monitor_update_on_pending_forwards() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 2); - if let Event::PaymentFailed { payment_hash, rejected_by_dest, .. } = events[0] { + if let Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } = events[0] { assert_eq!(payment_hash, payment_hash_1); assert!(rejected_by_dest); } else { panic!("Unexpected event!"); } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8ce19cc0c..a3b8715f7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -489,7 +489,7 @@ pub struct ChannelManager ChannelMana if let hash_map::Entry::Occupied(mut sessions) = outbounds.entry(mpp_id) { if sessions.get_mut().remove(&session_priv_bytes) { self.pending_events.lock().unwrap().push( - events::Event::PaymentFailed { + events::Event::PaymentPathFailed { payment_hash, rejected_by_dest: false, network_update: None, @@ -2951,7 +2951,7 @@ impl ChannelMana // process_onion_failure we should close that channel as it implies our // next-hop is needlessly blaming us! self.pending_events.lock().unwrap().push( - events::Event::PaymentFailed { + events::Event::PaymentPathFailed { payment_hash: payment_hash.clone(), rejected_by_dest: !payment_retryable, network_update, @@ -2977,7 +2977,7 @@ impl ChannelMana // TODO: For non-temporary failures, we really should be closing the // channel here as we apparently can't relay through them anyway. self.pending_events.lock().unwrap().push( - events::Event::PaymentFailed { + events::Event::PaymentPathFailed { payment_hash: payment_hash.clone(), rejected_by_dest: path.len() == 1, network_update: None, diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 5690c834e..e30046c99 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1073,7 +1073,7 @@ macro_rules! expect_payment_failed_with_update { let events = $node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, .. } => { + Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, ref network_update, ref error_code, ref error_data, .. } => { assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash"); assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value"); assert!(error_code.is_some(), "expected error_code.is_some() = true"); @@ -1102,7 +1102,7 @@ macro_rules! expect_payment_failed { let events = $node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, .. } => { + Event::PaymentPathFailed { ref payment_hash, rejected_by_dest, network_update: _, ref error_code, ref error_data, .. } => { assert_eq!(*payment_hash, $expected_payment_hash, "unexpected payment_hash"); assert_eq!(rejected_by_dest, $rejected_by_dest, "unexpected rejected_by_dest value"); assert!(error_code.is_some(), "expected error_code.is_some() = true"); @@ -1399,7 +1399,7 @@ pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expe let events = origin_node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentFailed { payment_hash, rejected_by_dest, all_paths_failed, .. } => { + Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, .. } => { assert_eq!(payment_hash, our_payment_hash); assert!(rejected_by_dest); assert_eq!(all_paths_failed, i == expected_paths.len() - 1); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index a7714796a..00dacc420 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -3020,7 +3020,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use _ => panic!("Unexepected event"), } match events[1] { - Event::PaymentFailed { ref payment_hash, .. } => { + Event::PaymentPathFailed { ref payment_hash, .. } => { assert_eq!(*payment_hash, fourth_payment_hash); }, _ => panic!("Unexpected event"), @@ -3076,7 +3076,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 3); match events[0] { - Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { + Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { assert!(failed_htlcs.insert(payment_hash.0)); // If we delivered B's RAA we got an unknown preimage error, not something // that we should update our routing table for. @@ -3087,14 +3087,14 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { + Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { assert!(failed_htlcs.insert(payment_hash.0)); assert!(network_update.is_some()); }, _ => panic!("Unexpected event"), } match events[2] { - Event::PaymentFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { + Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { assert!(failed_htlcs.insert(payment_hash.0)); assert!(network_update.is_some()); }, @@ -3190,7 +3190,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() { assert_eq!(events.len(), 2); // Check that Alice fails backward the pending HTLC from the second payment. match events[0] { - Event::PaymentFailed { payment_hash, .. } => { + Event::PaymentPathFailed { payment_hash, .. } => { assert_eq!(payment_hash, failed_payment_hash); }, _ => panic!("Unexpected event"), @@ -3392,7 +3392,7 @@ fn test_simple_peer_disconnect() { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => { + Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } => { assert_eq!(payment_hash, payment_hash_5); assert!(rejected_by_dest); }, @@ -4192,7 +4192,7 @@ fn test_dup_htlc_onchain_fails_on_reload() { // // If, due to an on-chain event, an HTLC is failed/claimed, and then we serialize the // ChannelManager, we generally expect there not to be a duplicate HTLC fail/claim (eg via a - // PaymentFailed event appearing). However, because we may not serialize the relevant + // PaymentPathFailed event appearing). However, because we may not serialize the relevant // ChannelMonitor at the same time, this isn't strictly guaranteed. In order to provide this // consistency, the ChannelManager explicitly tracks pending-onchain-resolution outbound HTLCs // and de-duplicates ChannelMonitor events. @@ -5518,7 +5518,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno let mut as_failds = HashSet::new(); let mut as_updates = 0; for event in as_events.iter() { - if let &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event { + if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event { assert!(as_failds.insert(*payment_hash)); if *payment_hash != payment_hash_2 { assert_eq!(*rejected_by_dest, deliver_last_raa); @@ -5543,7 +5543,7 @@ fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, anno let mut bs_failds = HashSet::new(); let mut bs_updates = 0; for event in bs_events.iter() { - if let &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event { + if let &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event { assert!(bs_failds.insert(*payment_hash)); if *payment_hash != payment_hash_1 && *payment_hash != payment_hash_5 { assert_eq!(*rejected_by_dest, deliver_last_raa); @@ -6068,7 +6068,7 @@ fn test_fail_holding_cell_htlc_upon_free() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match &events[0] { - &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => { + &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => { assert_eq!(our_payment_hash.clone(), *payment_hash); assert_eq!(*rejected_by_dest, false); assert_eq!(*all_paths_failed, true); @@ -6155,7 +6155,7 @@ fn test_free_and_fail_holding_cell_htlcs() { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match &events[0] { - &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => { + &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref error_code, ref error_data, ref all_paths_failed } => { assert_eq!(payment_hash_2.clone(), *payment_hash); assert_eq!(*rejected_by_dest, false); assert_eq!(*all_paths_failed, true); @@ -7107,12 +7107,12 @@ fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) { assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1); let events = nodes[0].node.get_and_clear_pending_events(); - // Only 2 PaymentFailed events should show up, over-dust HTLC has to be failed by timeout tx + // Only 2 PaymentPathFailed events should show up, over-dust HTLC has to be failed by timeout tx assert_eq!(events.len(), 2); let mut first_failed = false; for event in events { match event { - Event::PaymentFailed { payment_hash, .. } => { + Event::PaymentPathFailed { payment_hash, .. } => { if payment_hash == payment_hash_1 { assert!(!first_failed); first_failed = true; @@ -7202,14 +7202,14 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { assert_eq!(events.len(), 2); let first; match events[0] { - Event::PaymentFailed { payment_hash, .. } => { + Event::PaymentPathFailed { payment_hash, .. } => { if payment_hash == dust_hash { first = true; } else { first = false; } }, _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentFailed { payment_hash, .. } => { + Event::PaymentPathFailed { payment_hash, .. } => { if first { assert_eq!(payment_hash, non_dust_hash); } else { assert_eq!(payment_hash, dust_hash); } }, diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 70d5a0c0b..090200856 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -163,7 +163,7 @@ fn run_onion_failure_test_with_fail_intercept(_name: &str, test_case: let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); - if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _, ref all_paths_failed } = &events[0] { + if let &Event::PaymentPathFailed { payment_hash:_, ref rejected_by_dest, ref network_update, ref error_code, error_data: _, ref all_paths_failed } = &events[0] { assert_eq!(*rejected_by_dest, !expected_retryable); assert_eq!(*all_paths_failed, true); assert_eq!(*error_code, expected_error_code); diff --git a/lightning/src/routing/network_graph.rs b/lightning/src/routing/network_graph.rs index 16ff80189..c39dde7bf 100644 --- a/lightning/src/routing/network_graph.rs +++ b/lightning/src/routing/network_graph.rs @@ -113,7 +113,7 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate, impl EventHandler for NetGraphMsgHandler where C::Target: chain::Access, L::Target: Logger { fn handle_event(&self, event: &Event) { - if let Event::PaymentFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event { + if let Event::PaymentPathFailed { payment_hash: _, rejected_by_dest: _, network_update, .. } = event { if let Some(network_update) = network_update { self.handle_network_update(network_update); } @@ -127,7 +127,7 @@ where C::Target: chain::Access, L::Target: Logger { /// Provides interface to help with initial routing sync by /// serving historical announcements. /// -/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentFailed`] to the +/// Serves as an [`EventHandler`] for applying updates from [`Event::PaymentPathFailed`] to the /// [`NetworkGraph`]. pub struct NetGraphMsgHandler where C::Target: chain::Access, L::Target: Logger @@ -1725,7 +1725,7 @@ mod tests { assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_none()); - net_graph_msg_handler.handle_event(&Event::PaymentFailed { + net_graph_msg_handler.handle_event(&Event::PaymentPathFailed { payment_hash: PaymentHash([0; 32]), rejected_by_dest: false, all_paths_failed: true, @@ -1748,7 +1748,7 @@ mod tests { } }; - net_graph_msg_handler.handle_event(&Event::PaymentFailed { + net_graph_msg_handler.handle_event(&Event::PaymentPathFailed { payment_hash: PaymentHash([0; 32]), rejected_by_dest: false, all_paths_failed: true, @@ -1770,7 +1770,7 @@ mod tests { // Permanent closing deletes a channel { - net_graph_msg_handler.handle_event(&Event::PaymentFailed { + net_graph_msg_handler.handle_event(&Event::PaymentPathFailed { payment_hash: PaymentHash([0; 32]), rejected_by_dest: false, all_paths_failed: true, diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index be9296161..a2f48e060 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -170,8 +170,8 @@ pub enum Event { /// Indicates an outbound payment we made succeeded (i.e. it made it all the way to its target /// and we got back the payment preimage for it). /// - /// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentFailed` event. - /// In this situation, you SHOULD treat this payment as having succeeded. + /// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentPathFailed` + /// event. In this situation, you SHOULD treat this payment as having succeeded. PaymentSent { /// The preimage to the hash given to ChannelManager::send_payment. /// Note that this serves as a payment receipt, if you wish to have such a thing, you must @@ -180,7 +180,7 @@ pub enum Event { }, /// Indicates an outbound payment we made failed. Probably some intermediary node dropped /// something. You may wish to retry with a different route. - PaymentFailed { + PaymentPathFailed { /// The hash which was given to ChannelManager::send_payment. payment_hash: PaymentHash, /// Indicates the payment was rejected for some reason by the recipient. This implies that @@ -291,7 +291,7 @@ impl Writeable for Event { (0, payment_preimage, required), }); }, - &Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, + &Event::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, #[cfg(test)] ref error_code, #[cfg(test)] @@ -409,7 +409,7 @@ impl MaybeReadable for Event { (2, rejected_by_dest, required), (3, all_paths_failed, option), }); - Ok(Some(Event::PaymentFailed { + Ok(Some(Event::PaymentPathFailed { payment_hash, rejected_by_dest, network_update,