From c57bb42204a569fce5fb90d7f1b1367320124ae1 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 7 Sep 2022 20:58:05 +0000 Subject: [PATCH] Rename `rejected_by_dest` -> `payment_failed_permanently` The `rejected_by_dest` field of the `PaymentPathFailed` event has always been a bit of a misnomer, as its really more about retry than where a payment failed. Now is as good a time as any to rename it. --- lightning-invoice/src/payment.rs | 40 +++++++++---------- lightning/src/ln/chanmon_update_fail_tests.rs | 4 +- lightning/src/ln/channelmanager.rs | 6 +-- lightning/src/ln/functional_test_utils.rs | 22 +++++----- lightning/src/ln/functional_tests.rs | 30 +++++++------- lightning/src/ln/onion_route_tests.rs | 4 +- lightning/src/routing/gossip.rs | 6 +-- lightning/src/util/events.rs | 12 +++--- 8 files changed, 62 insertions(+), 62 deletions(-) diff --git a/lightning-invoice/src/payment.rs b/lightning-invoice/src/payment.rs index e7cc4350..a97bf2fc 100644 --- a/lightning-invoice/src/payment.rs +++ b/lightning-invoice/src/payment.rs @@ -717,7 +717,7 @@ where match event { Event::PaymentPathFailed { - payment_id, payment_hash, rejected_by_dest, path, short_channel_id, retry, .. + payment_id, payment_hash, payment_failed_permanently, path, short_channel_id, retry, .. } => { if let Some(short_channel_id) = short_channel_id { let path = path.iter().collect::>(); @@ -726,7 +726,7 @@ where if payment_id.is_none() { log_trace!(self.logger, "Payment {} has no id; not retrying", log_bytes!(payment_hash.0)); - } else if *rejected_by_dest { + } else if *payment_failed_permanently { log_trace!(self.logger, "Payment {} rejected by destination; not retrying", log_bytes!(payment_hash.0)); self.payer.abandon_payment(payment_id.unwrap()); } else if retry.is_none() { @@ -916,7 +916,7 @@ mod tests { payment_id, payment_hash, network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: TestRouter::path_for_value(final_value_msat), short_channel_id: None, @@ -981,7 +981,7 @@ mod tests { payment_id, payment_hash, network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: TestRouter::path_for_value(final_value_msat), short_channel_id: None, @@ -1028,7 +1028,7 @@ mod tests { payment_id, payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: true, path: TestRouter::path_for_value(final_value_msat), short_channel_id: None, @@ -1042,7 +1042,7 @@ mod tests { payment_id, payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: TestRouter::path_for_value(final_value_msat / 2), short_channel_id: None, @@ -1088,7 +1088,7 @@ mod tests { payment_id, payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: true, path: TestRouter::path_for_value(final_value_msat), short_channel_id: None, @@ -1128,7 +1128,7 @@ mod tests { payment_id, payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: vec![], short_channel_id: None, @@ -1189,7 +1189,7 @@ mod tests { payment_id, payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: vec![], short_channel_id: None, @@ -1226,7 +1226,7 @@ mod tests { payment_id, payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: TestRouter::path_for_value(final_value_msat / 2), short_channel_id: None, @@ -1260,7 +1260,7 @@ mod tests { payment_id, payment_hash: PaymentHash(invoice.payment_hash().clone().into_inner()), network_update: None, - rejected_by_dest: true, + payment_failed_permanently: true, all_paths_failed: false, path: vec![], short_channel_id: None, @@ -1309,7 +1309,7 @@ mod tests { payment_id, payment_hash, network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: vec![], short_channel_id: None, @@ -1444,7 +1444,7 @@ mod tests { payment_id, payment_hash, network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: vec![], short_channel_id: None, @@ -1490,7 +1490,7 @@ mod tests { payment_id, payment_hash, network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path, short_channel_id, @@ -1680,7 +1680,7 @@ mod tests { payment_id, payment_hash, network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: TestRouter::path_for_value(final_value_msat), short_channel_id: None, @@ -1692,7 +1692,7 @@ mod tests { payment_id, payment_hash, network_update: None, - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: false, path: TestRouter::path_for_value(final_value_msat / 2), short_channel_id: None, @@ -2393,8 +2393,8 @@ mod tests { // treated this as "HTLC complete" and dropped the retry counter, causing us to retry again // if the final HTLC failed. expected_events.borrow_mut().push_back(&|ev: &Event| { - if let Event::PaymentPathFailed { rejected_by_dest, all_paths_failed, .. } = ev { - assert!(!rejected_by_dest); + if let Event::PaymentPathFailed { payment_failed_permanently, all_paths_failed, .. } = ev { + assert!(!payment_failed_permanently); assert!(all_paths_failed); } else { panic!("Unexpected event"); } }); @@ -2411,8 +2411,8 @@ mod tests { commitment_signed_dance!(nodes[0], nodes[1], &bs_fail_update.commitment_signed, false, true); expected_events.borrow_mut().push_back(&|ev: &Event| { - if let Event::PaymentPathFailed { rejected_by_dest, all_paths_failed, .. } = ev { - assert!(!rejected_by_dest); + if let Event::PaymentPathFailed { payment_failed_permanently, all_paths_failed, .. } = ev { + assert!(!payment_failed_permanently); assert!(all_paths_failed); } else { panic!("Unexpected event"); } }); diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index f5977abd..794ac0db 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -1744,9 +1744,9 @@ 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::PaymentPathFailed { payment_hash, rejected_by_dest, .. } = events[0] { + if let Event::PaymentPathFailed { payment_hash, payment_failed_permanently, .. } = events[0] { assert_eq!(payment_hash, payment_hash_1); - assert!(rejected_by_dest); + assert!(payment_failed_permanently); } else { panic!("Unexpected event!"); } match events[1] { Event::PendingHTLCsForwardable { .. } => { }, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6f8ba7b3..2d1a3c7e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3844,7 +3844,7 @@ impl ChannelMana pending_events.push(events::Event::PaymentPathFailed { payment_id: Some(payment_id), payment_hash, - rejected_by_dest: false, + payment_failed_permanently: false, network_update: None, all_paths_failed: payment.get().remaining_parts() == 0, path: path.clone(), @@ -3959,7 +3959,7 @@ impl ChannelMana events::Event::PaymentPathFailed { payment_id: Some(payment_id), payment_hash: payment_hash.clone(), - rejected_by_dest: !payment_retryable, + payment_failed_permanently: !payment_retryable, network_update, all_paths_failed, path: path.clone(), @@ -3990,7 +3990,7 @@ impl ChannelMana events::Event::PaymentPathFailed { payment_id: Some(payment_id), payment_hash: payment_hash.clone(), - rejected_by_dest: false, + payment_failed_permanently: false, network_update: None, all_paths_failed, path: path.clone(), diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 39efab5f..11f61cad 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1580,9 +1580,9 @@ impl<'a> PaymentFailedConditions<'a> { #[cfg(test)] macro_rules! expect_payment_failed_with_update { - ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr, $scid: expr, $chan_closed: expr) => { + ($node: expr, $expected_payment_hash: expr, $payment_failed_permanently: expr, $scid: expr, $chan_closed: expr) => { $crate::ln::functional_test_utils::expect_payment_failed_conditions( - &$node, $expected_payment_hash, $rejected_by_dest, + &$node, $expected_payment_hash, $payment_failed_permanently, $crate::ln::functional_test_utils::PaymentFailedConditions::new() .blamed_scid($scid).blamed_chan_closed($chan_closed)); } @@ -1590,28 +1590,28 @@ macro_rules! expect_payment_failed_with_update { #[cfg(test)] macro_rules! expect_payment_failed { - ($node: expr, $expected_payment_hash: expr, $rejected_by_dest: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => { + ($node: expr, $expected_payment_hash: expr, $payment_failed_permanently: expr $(, $expected_error_code: expr, $expected_error_data: expr)*) => { #[allow(unused_mut)] let mut conditions = $crate::ln::functional_test_utils::PaymentFailedConditions::new(); $( conditions = conditions.expected_htlc_error_data($expected_error_code, &$expected_error_data); )* - $crate::ln::functional_test_utils::expect_payment_failed_conditions(&$node, $expected_payment_hash, $rejected_by_dest, conditions); + $crate::ln::functional_test_utils::expect_payment_failed_conditions(&$node, $expected_payment_hash, $payment_failed_permanently, conditions); }; } pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>( node: &'a Node<'b, 'c, 'd>, payment_failed_event: Event, expected_payment_hash: PaymentHash, - expected_rejected_by_dest: bool, conditions: PaymentFailedConditions<'e> + expected_payment_failed_permanently: bool, conditions: PaymentFailedConditions<'e> ) { let expected_payment_id = match payment_failed_event { - Event::PaymentPathFailed { payment_hash, rejected_by_dest, path, retry, payment_id, network_update, short_channel_id, + Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, network_update, short_channel_id, #[cfg(test)] error_code, #[cfg(test)] error_data, .. } => { assert_eq!(payment_hash, expected_payment_hash, "unexpected payment_hash"); - assert_eq!(rejected_by_dest, expected_rejected_by_dest, "unexpected rejected_by_dest value"); + assert_eq!(payment_failed_permanently, expected_payment_failed_permanently, "unexpected payment_failed_permanently value"); assert!(retry.is_some(), "expected retry.is_some()"); assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path"); assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path"); @@ -1668,12 +1668,12 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>( } pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>( - node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_rejected_by_dest: bool, + node: &'a Node<'b, 'c, 'd>, expected_payment_hash: PaymentHash, expected_payment_failed_permanently: bool, conditions: PaymentFailedConditions<'e> ) { let mut events = node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); - expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_rejected_by_dest, conditions); + expect_payment_failed_conditions_event(node, events.pop().unwrap(), expected_payment_hash, expected_payment_failed_permanently, conditions); } pub fn send_along_route_with_secret<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route, expected_paths: &[&[&Node<'a, 'b, 'c>]], recv_value: u64, our_payment_hash: PaymentHash, our_payment_secret: PaymentSecret) -> PaymentId { @@ -2017,9 +2017,9 @@ pub fn pass_failed_payment_back<'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); let expected_payment_id = match events[0] { - Event::PaymentPathFailed { payment_hash, rejected_by_dest, all_paths_failed, ref path, ref payment_id, .. } => { + Event::PaymentPathFailed { payment_hash, payment_failed_permanently, all_paths_failed, ref path, ref payment_id, .. } => { assert_eq!(payment_hash, our_payment_hash); - assert!(rejected_by_dest); + assert!(payment_failed_permanently); assert_eq!(all_paths_failed, i == expected_paths.len() - 1); for (idx, hop) in expected_route.iter().enumerate() { assert_eq!(hop.node.get_our_node_id(), path[idx].pubkey); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 826a2c31..214e7cda 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -3266,7 +3266,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::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { + Event::PaymentPathFailed { ref payment_hash, 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. @@ -3277,14 +3277,14 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { + Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => { assert!(failed_htlcs.insert(payment_hash.0)); assert!(network_update.is_some()); }, _ => panic!("Unexpected event"), } match events[2] { - Event::PaymentPathFailed { ref payment_hash, rejected_by_dest: _, ref network_update, .. } => { + Event::PaymentPathFailed { ref payment_hash, ref network_update, .. } => { assert!(failed_htlcs.insert(payment_hash.0)); assert!(network_update.is_some()); }, @@ -3614,9 +3614,9 @@ fn test_simple_peer_disconnect() { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentPathFailed { payment_hash, rejected_by_dest, .. } => { + Event::PaymentPathFailed { payment_hash, payment_failed_permanently, .. } => { assert_eq!(payment_hash, payment_hash_5); - assert!(rejected_by_dest); + assert!(payment_failed_permanently); }, _ => panic!("Unexpected event"), } @@ -5715,12 +5715,12 @@ 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::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event { + if let &Event::PaymentPathFailed { ref payment_hash, ref payment_failed_permanently, ref network_update, .. } = event { assert!(as_failds.insert(*payment_hash)); if *payment_hash != payment_hash_2 { - assert_eq!(*rejected_by_dest, deliver_last_raa); + assert_eq!(*payment_failed_permanently, deliver_last_raa); } else { - assert!(!rejected_by_dest); + assert!(!payment_failed_permanently); } if network_update.is_some() { as_updates += 1; @@ -5740,12 +5740,12 @@ 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::PaymentPathFailed { ref payment_hash, ref rejected_by_dest, ref network_update, .. } = event { + if let &Event::PaymentPathFailed { ref payment_hash, ref payment_failed_permanently, 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); + assert_eq!(*payment_failed_permanently, deliver_last_raa); } else { - assert!(!rejected_by_dest); + assert!(!payment_failed_permanently); } if network_update.is_some() { bs_updates += 1; @@ -6264,10 +6264,10 @@ 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::PaymentPathFailed { ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, ref error_data, .. } => { + &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, ref error_data, .. } => { assert_eq!(our_payment_id, *payment_id.as_ref().unwrap()); assert_eq!(our_payment_hash.clone(), *payment_hash); - assert_eq!(*rejected_by_dest, false); + assert_eq!(*payment_failed_permanently, false); assert_eq!(*all_paths_failed, true); assert_eq!(*network_update, None); assert_eq!(*short_channel_id, None); @@ -6350,10 +6350,10 @@ 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::PaymentPathFailed { ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, ref error_data, .. } => { + &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, ref error_data, .. } => { assert_eq!(payment_id_2, *payment_id.as_ref().unwrap()); assert_eq!(payment_hash_2.clone(), *payment_hash); - assert_eq!(*rejected_by_dest, false); + assert_eq!(*payment_failed_permanently, false); assert_eq!(*all_paths_failed, true); assert_eq!(*network_update, None); assert_eq!(*short_channel_id, None); diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index e2c64323..4223b4cb 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -167,8 +167,8 @@ 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::PaymentPathFailed { ref rejected_by_dest, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, .. } = &events[0] { - assert_eq!(*rejected_by_dest, !expected_retryable); + if let &Event::PaymentPathFailed { ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref short_channel_id, ref error_code, .. } = &events[0] { + assert_eq!(*payment_failed_permanently, !expected_retryable); assert_eq!(*all_paths_failed, true); assert_eq!(*error_code, expected_error_code); if expected_channel_update.is_some() { diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 637a8c04..28067838 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -2279,7 +2279,7 @@ mod tests { network_graph.handle_event(&Event::PaymentPathFailed { payment_id: None, payment_hash: PaymentHash([0; 32]), - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: true, path: vec![], network_update: Some(NetworkUpdate::ChannelUpdateMessage { @@ -2306,7 +2306,7 @@ mod tests { network_graph.handle_event(&Event::PaymentPathFailed { payment_id: None, payment_hash: PaymentHash([0; 32]), - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: true, path: vec![], network_update: Some(NetworkUpdate::ChannelFailure { @@ -2331,7 +2331,7 @@ mod tests { network_graph.handle_event(&Event::PaymentPathFailed { payment_id: None, payment_hash: PaymentHash([0; 32]), - rejected_by_dest: false, + payment_failed_permanently: false, all_paths_failed: true, path: vec![], network_update: Some(NetworkUpdate::ChannelFailure { diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index e86eae3c..47e0c88f 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -376,7 +376,7 @@ pub enum Event { /// Indicates the payment was rejected for some reason by the recipient. This implies that /// the payment has failed, not just the route in question. If this is not set, you may /// retry the payment via a different route. - rejected_by_dest: bool, + payment_failed_permanently: bool, /// Any failure information conveyed via the Onion return packet by a node along the failed /// payment route. /// @@ -643,7 +643,7 @@ impl Writeable for Event { }); }, &Event::PaymentPathFailed { - ref payment_id, ref payment_hash, ref rejected_by_dest, ref network_update, + ref payment_id, ref payment_hash, ref payment_failed_permanently, ref network_update, ref all_paths_failed, ref path, ref short_channel_id, ref retry, #[cfg(test)] ref error_code, @@ -658,7 +658,7 @@ impl Writeable for Event { write_tlv_fields!(writer, { (0, payment_hash, required), (1, network_update, option), - (2, rejected_by_dest, required), + (2, payment_failed_permanently, required), (3, all_paths_failed, required), (5, path, vec_type), (7, short_channel_id, option), @@ -827,7 +827,7 @@ impl MaybeReadable for Event { #[cfg(test)] let error_data = Readable::read(reader)?; let mut payment_hash = PaymentHash([0; 32]); - let mut rejected_by_dest = false; + let mut payment_failed_permanently = false; let mut network_update = None; let mut all_paths_failed = Some(true); let mut path: Option> = Some(vec![]); @@ -837,7 +837,7 @@ impl MaybeReadable for Event { read_tlv_fields!(reader, { (0, payment_hash, required), (1, network_update, ignorable), - (2, rejected_by_dest, required), + (2, payment_failed_permanently, required), (3, all_paths_failed, option), (5, path, vec_type), (7, short_channel_id, option), @@ -847,7 +847,7 @@ impl MaybeReadable for Event { Ok(Some(Event::PaymentPathFailed { payment_id, payment_hash, - rejected_by_dest, + payment_failed_permanently, network_update, all_paths_failed: all_paths_failed.unwrap(), path: path.unwrap(), -- 2.30.2