diff --git a/lightning-background-processor/src/lib.rs b/lightning-background-processor/src/lib.rs index f64f1e7651e..e407c4fdb82 100644 --- a/lightning-background-processor/src/lib.rs +++ b/lightning-background-processor/src/lib.rs @@ -1369,7 +1369,6 @@ mod tests { failure: PathFailure::OnPath { network_update: None }, path: path.clone(), short_channel_id: Some(scored_scid), - retry: None, }); let event = receiver .recv_timeout(Duration::from_secs(EVENT_DEADLINE)) @@ -1389,7 +1388,6 @@ mod tests { failure: PathFailure::OnPath { network_update: None }, path: path.clone(), short_channel_id: None, - retry: None, }); let event = receiver .recv_timeout(Duration::from_secs(EVENT_DEADLINE)) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9af0f6379e5..4aca2343f80 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7118,7 +7118,6 @@ mod tests { first_hop_htlc_msat: 548, payment_id: PaymentId([42; 32]), payment_secret: None, - payment_params: None, } }); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6e0cd170980..c4ebc8bc3fa 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -275,11 +275,6 @@ pub(crate) enum HTLCSource { first_hop_htlc_msat: u64, payment_id: PaymentId, payment_secret: Option, - /// Note that this is now "deprecated" - we write it for forwards (and read it for - /// backwards) compatibility reasons, but prefer to use the data in the - /// [`super::outbound_payment`] module, which stores per-payment data once instead of in - /// each HTLC. - payment_params: Option, }, } #[allow(clippy::derive_hash_xor_eq)] // Our Hash is faithful to the data, we just don't have SecretKey::hash @@ -290,14 +285,13 @@ impl core::hash::Hash for HTLCSource { 0u8.hash(hasher); prev_hop_data.hash(hasher); }, - HTLCSource::OutboundRoute { path, session_priv, payment_id, payment_secret, first_hop_htlc_msat, payment_params } => { + HTLCSource::OutboundRoute { path, session_priv, payment_id, payment_secret, first_hop_htlc_msat } => { 1u8.hash(hasher); path.hash(hasher); session_priv[..].hash(hasher); payment_id.hash(hasher); payment_secret.hash(hasher); first_hop_htlc_msat.hash(hasher); - payment_params.hash(hasher); }, } } @@ -312,7 +306,6 @@ impl HTLCSource { first_hop_htlc_msat: 0, payment_id: PaymentId([2; 32]), payment_secret: None, - payment_params: None, } } } @@ -2467,12 +2460,12 @@ where } #[cfg(test)] - pub(crate) fn test_send_payment_along_path(&self, path: &Vec, payment_params: &Option, payment_hash: &PaymentHash, payment_secret: &Option, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option, session_priv_bytes: [u8; 32]) -> Result<(), APIError> { + pub(crate) fn test_send_payment_along_path(&self, path: &Vec, payment_hash: &PaymentHash, payment_secret: &Option, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option, session_priv_bytes: [u8; 32]) -> Result<(), APIError> { let _lck = self.total_consistency_lock.read().unwrap(); - self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv_bytes) + self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv_bytes) } - fn send_payment_along_path(&self, path: &Vec, payment_params: &Option, payment_hash: &PaymentHash, payment_secret: &Option, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option, session_priv_bytes: [u8; 32]) -> Result<(), APIError> { + fn send_payment_along_path(&self, path: &Vec, payment_hash: &PaymentHash, payment_secret: &Option, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option, session_priv_bytes: [u8; 32]) -> Result<(), APIError> { // The top-level caller should hold the total_consistency_lock read lock. debug_assert!(self.total_consistency_lock.try_write().is_err()); @@ -2511,7 +2504,6 @@ where first_hop_htlc_msat: htlc_msat, payment_id, payment_secret: payment_secret.clone(), - payment_params: payment_params.clone(), }, onion_packet, &self.logger); match break_chan_entry!(self, send_res, chan) { Some(monitor_update) => { @@ -2617,8 +2609,8 @@ where let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); self.pending_outbound_payments .send_payment_with_route(route, payment_hash, payment_secret, payment_id, &self.entropy_source, &self.node_signer, best_block_height, - |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + |path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) } /// Similar to [`ChannelManager::send_payment`], but will automatically find a route based on @@ -2631,8 +2623,8 @@ where &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height, &self.logger, &self.pending_events, - |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + |path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) } #[cfg(test)] @@ -2640,8 +2632,8 @@ where let best_block_height = self.best_block.read().unwrap().height(); let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); self.pending_outbound_payments.test_send_payment_internal(route, payment_hash, payment_secret, keysend_preimage, payment_id, recv_value_msat, onion_session_privs, &self.node_signer, best_block_height, - |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + |path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) } #[cfg(test)] @@ -2693,8 +2685,8 @@ where self.pending_outbound_payments.send_spontaneous_payment_with_route( route, payment_preimage, payment_id, &self.entropy_source, &self.node_signer, best_block_height, - |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + |path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) } /// Similar to [`ChannelManager::send_spontaneous_payment`], but will automatically find a route @@ -2711,8 +2703,8 @@ where retry_strategy, route_params, &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height, &self.logger, &self.pending_events, - |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + |path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) } /// Send a payment that is probing the given route for liquidity. We calculate the @@ -2722,8 +2714,8 @@ where let best_block_height = self.best_block.read().unwrap().height(); let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); self.pending_outbound_payments.send_probe(hops, self.probing_cookie_secret, &self.entropy_source, &self.node_signer, best_block_height, - |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) + |path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)) } /// Returns whether a payment with the given [`PaymentHash`] and [`PaymentId`] is, in fact, a @@ -3447,8 +3439,8 @@ where self.pending_outbound_payments.check_retry_payments(&self.router, || self.list_usable_channels(), || self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, best_block_height, &self.pending_events, &self.logger, - |path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| - self.send_payment_along_path(path, payment_params, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)); + |path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv| + self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv)); for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) { self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination); @@ -3834,9 +3826,9 @@ where // from block_connected which may run during initialization prior to the chain_monitor // being fully configured. See the docs for `ChannelManagerReadArgs` for more. match source { - HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, ref payment_params, .. } => { + HTLCSource::OutboundRoute { ref path, ref session_priv, ref payment_id, .. } => { if self.pending_outbound_payments.fail_htlc(source, payment_hash, onion_error, path, - session_priv, payment_id, payment_params, self.probing_cookie_secret, &self.secp_ctx, + session_priv, payment_id, self.probing_cookie_secret, &self.secp_ctx, &self.pending_events, &self.logger) { self.push_pending_forwards_ev(); } }, @@ -6858,7 +6850,6 @@ impl Readable for HTLCSource { path, payment_id: payment_id.unwrap(), payment_secret, - payment_params, }) } 1 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)), @@ -6870,7 +6861,7 @@ impl Readable for HTLCSource { impl Writeable for HTLCSource { fn write(&self, writer: &mut W) -> Result<(), crate::io::Error> { match self { - HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, payment_secret, payment_params } => { + HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, payment_id, payment_secret } => { 0u8.write(writer)?; let payment_id_opt = Some(payment_id); write_tlv_fields!(writer, { @@ -6879,7 +6870,7 @@ impl Writeable for HTLCSource { (2, first_hop_htlc_msat, required), (3, payment_secret, option), (4, *path, vec_type), - (5, payment_params, option), + (5, None::, option), // payment_params in LDK versions prior to 0.0.115 }); } HTLCSource::PreviousHopData(ref field) => { @@ -7959,7 +7950,7 @@ mod tests { // indicates there are more HTLCs coming. let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match. let session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash, Some(payment_secret), payment_id, &mpp_route).unwrap(); - nodes[0].node.test_send_payment_along_path(&mpp_route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[0]).unwrap(); + nodes[0].node.test_send_payment_along_path(&mpp_route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[0]).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); @@ -7989,7 +7980,7 @@ mod tests { expect_payment_failed!(nodes[0], our_payment_hash, true); // Send the second half of the original MPP payment. - nodes[0].node.test_send_payment_along_path(&mpp_route.paths[1], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[1]).unwrap(); + nodes[0].node.test_send_payment_along_path(&mpp_route.paths[1], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[1]).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 1e99a475fe9..c8d3beb3cba 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1868,20 +1868,13 @@ pub fn expect_payment_failed_conditions_event<'a, 'b, 'c, 'd, 'e>( ) { if conditions.expected_mpp_parts_remain { assert_eq!(payment_failed_events.len(), 1); } else { assert_eq!(payment_failed_events.len(), 2); } let expected_payment_id = match &payment_failed_events[0] { - Event::PaymentPathFailed { payment_hash, payment_failed_permanently, path, retry, payment_id, failure, short_channel_id, + Event::PaymentPathFailed { payment_hash, payment_failed_permanently, payment_id, failure, #[cfg(test)] error_code, #[cfg(test)] error_data, .. } => { assert_eq!(*payment_hash, expected_payment_hash, "unexpected payment_hash"); 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"); - if let Some(scid) = short_channel_id { - assert!(retry.as_ref().unwrap().payment_params.previously_failed_channels.contains(&scid)); - } - #[cfg(test)] { assert!(error_code.is_some(), "expected error_code.is_some() = true"); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 454fbe2b781..4fe5d041028 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4083,7 +4083,7 @@ fn do_test_htlc_timeout(send_partial_mpp: bool) { let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match. let payment_id = PaymentId([42; 32]); let session_privs = nodes[0].node.test_add_new_pending_payment(our_payment_hash, Some(payment_secret), payment_id, &route).unwrap(); - nodes[0].node.test_send_payment_along_path(&route.paths[0], &route.payment_params, &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[0]).unwrap(); + nodes[0].node.test_send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, payment_id, &None, session_privs[0]).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); @@ -9130,7 +9130,6 @@ fn test_inconsistent_mpp_params() { if path_a[0].pubkey == nodes[1].node.get_our_node_id() { core::cmp::Ordering::Less } else { core::cmp::Ordering::Greater } }); - let payment_params_opt = Some(payment_params); let (our_payment_preimage, our_payment_hash, our_payment_secret) = get_payment_preimage_hash!(&nodes[3]); @@ -9144,7 +9143,7 @@ fn test_inconsistent_mpp_params() { dup_route.paths.push(route.paths[1].clone()); nodes[0].node.test_add_new_pending_payment(our_payment_hash, Some(our_payment_secret), payment_id, &dup_route).unwrap() }; - nodes[0].node.test_send_payment_along_path(&route.paths[0], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None, session_privs[0]).unwrap(); + nodes[0].node.test_send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None, session_privs[0]).unwrap(); check_added_monitors!(nodes[0], 1); { @@ -9154,7 +9153,7 @@ fn test_inconsistent_mpp_params() { } assert!(nodes[3].node.get_and_clear_pending_events().is_empty()); - nodes[0].node.test_send_payment_along_path(&route.paths[1], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 14_000_000, cur_height, payment_id, &None, session_privs[1]).unwrap(); + nodes[0].node.test_send_payment_along_path(&route.paths[1], &our_payment_hash, &Some(our_payment_secret), 14_000_000, cur_height, payment_id, &None, session_privs[1]).unwrap(); check_added_monitors!(nodes[0], 1); { @@ -9200,7 +9199,7 @@ fn test_inconsistent_mpp_params() { expect_payment_failed_conditions(&nodes[0], our_payment_hash, true, PaymentFailedConditions::new().mpp_parts_remain()); - nodes[0].node.test_send_payment_along_path(&route.paths[1], &payment_params_opt, &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None, session_privs[2]).unwrap(); + nodes[0].node.test_send_payment_along_path(&route.paths[1], &our_payment_hash, &Some(our_payment_secret), 15_000_000, cur_height, payment_id, &None, session_privs[2]).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index fdf9cdd0e25..6ede956b7d5 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -79,7 +79,9 @@ impl PendingOutboundPayment { } fn is_auto_retryable_now(&self) -> bool { match self { - PendingOutboundPayment::Retryable { retry_strategy: Some(strategy), attempts, .. } => { + PendingOutboundPayment::Retryable { + retry_strategy: Some(strategy), attempts, payment_params: Some(_), .. + } => { strategy.is_retryable_now(&attempts) }, _ => false, @@ -97,14 +99,6 @@ impl PendingOutboundPayment { _ => false, } } - fn payment_parameters(&mut self) -> Option<&mut PaymentParameters> { - match self { - PendingOutboundPayment::Retryable { payment_params: Some(ref mut params), .. } => { - Some(params) - }, - _ => None, - } - } pub fn insert_previously_failed_scid(&mut self, scid: u64) { if let PendingOutboundPayment::Retryable { payment_params: Some(params), .. } = self { params.previously_failed_channels.push(scid); @@ -436,8 +430,8 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError>, + SP: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError>, { self.send_payment_internal(payment_id, payment_hash, payment_secret, None, retry_strategy, route_params, router, first_hops, &compute_inflight_htlcs, entropy_source, node_signer, @@ -452,8 +446,8 @@ impl OutboundPayments { where ES::Target: EntropySource, NS::Target: NodeSigner, - F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError> { let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, None, route, None, None, entropy_source, best_block_height)?; self.pay_route_internal(route, payment_hash, payment_secret, None, payment_id, None, @@ -474,8 +468,8 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError>, + SP: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError>, { let preimage = payment_preimage .unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes())); @@ -493,8 +487,8 @@ impl OutboundPayments { where ES::Target: EntropySource, NS::Target: NodeSigner, - F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError> { let preimage = payment_preimage .unwrap_or_else(|| PaymentPreimage(entropy_source.get_secure_random_bytes())); @@ -519,8 +513,8 @@ impl OutboundPayments { R::Target: Router, ES::Target: EntropySource, NS::Target: NodeSigner, - SP: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError>, + SP: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError>, IH: Fn() -> InFlightHtlcs, FH: Fn() -> Vec, L::Target: Logger, @@ -539,7 +533,7 @@ impl OutboundPayments { })); break } - } + } else { debug_assert!(false); } } } core::mem::drop(outbounds); @@ -588,8 +582,8 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> + SP: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError> { #[cfg(feature = "std")] { if has_expired(&route_params) { @@ -629,8 +623,8 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> + SP: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError> { #[cfg(feature = "std")] { if has_expired(&route_params) { @@ -744,8 +738,8 @@ impl OutboundPayments { NS::Target: NodeSigner, L::Target: Logger, IH: Fn() -> InFlightHtlcs, - SP: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> + SP: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError> { match err { PaymentSendFailure::AllFailedResendSafe(errs) => { @@ -798,7 +792,6 @@ impl OutboundPayments { failure: events::PathFailure::InitialSend { err: e }, path, short_channel_id: failed_scid, - retry: None, #[cfg(test)] error_code: None, #[cfg(test)] @@ -815,8 +808,8 @@ impl OutboundPayments { where ES::Target: EntropySource, NS::Target: NodeSigner, - F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError> { let payment_id = PaymentId(entropy_source.get_secure_random_bytes()); @@ -893,8 +886,8 @@ impl OutboundPayments { ) -> Result<(), PaymentSendFailure> where NS::Target: NodeSigner, - F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError> { if route.paths.len() < 1 { return Err(PaymentSendFailure::ParameterError(APIError::InvalidRoute{err: "There must be at least one path to send over".to_owned()})); @@ -931,7 +924,7 @@ impl OutboundPayments { let mut results = Vec::new(); debug_assert_eq!(route.paths.len(), onion_session_privs.len()); for (path, session_priv) in route.paths.iter().zip(onion_session_privs.into_iter()) { - let mut path_res = send_payment_along_path(&path, &route.payment_params, &payment_hash, payment_secret, total_value, cur_height, payment_id, &keysend_preimage, session_priv); + let mut path_res = send_payment_along_path(&path, &payment_hash, payment_secret, total_value, cur_height, payment_id, &keysend_preimage, session_priv); match path_res { Ok(_) => {}, Err(APIError::MonitorUpdateInProgress) => { @@ -998,8 +991,8 @@ impl OutboundPayments { ) -> Result<(), PaymentSendFailure> where NS::Target: NodeSigner, - F: Fn(&Vec, &Option, &PaymentHash, &Option, u64, - u32, PaymentId, &Option, [u8; 32]) -> Result<(), APIError> + F: Fn(&Vec, &PaymentHash, &Option, u64, u32, PaymentId, + &Option, [u8; 32]) -> Result<(), APIError> { self.pay_route_internal(route, payment_hash, payment_secret, keysend_preimage, payment_id, recv_value_msat, onion_session_privs, node_signer, best_block_height, @@ -1128,8 +1121,8 @@ impl OutboundPayments { pub(super) fn fail_htlc( &self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason, path: &Vec, session_priv: &SecretKey, payment_id: &PaymentId, - payment_params: &Option, probing_cookie_secret: [u8; 32], - secp_ctx: &Secp256k1, pending_events: &Mutex>, logger: &L + probing_cookie_secret: [u8; 32], secp_ctx: &Secp256k1, + pending_events: &Mutex>, logger: &L ) -> bool where L::Target: Logger { #[cfg(test)] let (network_update, short_channel_id, payment_retryable, onion_error_code, onion_error_data) = onion_error.decode_onion_failure(secp_ctx, logger, &source); @@ -1157,7 +1150,6 @@ impl OutboundPayments { let mut full_failure_ev = None; let mut pending_retry_ev = false; - let mut retry = None; let attempts_remaining = if let hash_map::Entry::Occupied(mut payment) = outbounds.entry(*payment_id) { if !payment.get_mut().remove(&session_priv_bytes, Some(&path)) { log_trace!(logger, "Received duplicative fail for HTLC with payment_hash {}", log_bytes!(payment_hash.0)); @@ -1169,27 +1161,13 @@ impl OutboundPayments { } let mut is_retryable_now = payment.get().is_auto_retryable_now(); if let Some(scid) = short_channel_id { + // TODO: If we decided to blame ourselves (or one of our channels) in + // process_onion_failure we should close that channel as it implies our + // next-hop is needlessly blaming us! payment.get_mut().insert_previously_failed_scid(scid); } - // We want to move towards only using the `PaymentParameters` in the outbound payments - // map. However, for backwards-compatibility, we still need to support passing the - // `PaymentParameters` data that was shoved in the HTLC (and given to us via - // `payment_params`) back to the user. - let path_last_hop = path.last().expect("Outbound payments must have had a valid path"); - if let Some(params) = payment.get_mut().payment_parameters() { - retry = Some(RouteParameters { - payment_params: params.clone(), - final_value_msat: path_last_hop.fee_msat, - }); - } else if let Some(params) = payment_params { - retry = Some(RouteParameters { - payment_params: params.clone(), - final_value_msat: path_last_hop.fee_msat, - }); - } - - if payment_is_probe || !is_retryable_now || !payment_retryable || retry.is_none() { + if payment_is_probe || !is_retryable_now || !payment_retryable { let _ = payment.get_mut().mark_abandoned(); // we'll only Err if it's a legacy payment is_retryable_now = false; } @@ -1229,12 +1207,6 @@ impl OutboundPayments { } } } else { - // TODO: If we decided to blame ourselves (or one of our channels) in - // process_onion_failure we should close that channel as it implies our - // next-hop is needlessly blaming us! - if let Some(scid) = short_channel_id { - retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid)); - } // If we miss abandoning the payment above, we *must* generate an event here or else the // payment will sit in our outbounds forever. if attempts_remaining && !already_awaiting_retry { @@ -1248,7 +1220,6 @@ impl OutboundPayments { failure: events::PathFailure::OnPath { network_update }, path: path.clone(), short_channel_id, - retry, #[cfg(test)] error_code: onion_error_code, #[cfg(test)] @@ -1387,7 +1358,7 @@ mod tests { outbound_payments.retry_payment_internal( PaymentHash([0; 32]), PaymentId([0; 32]), expired_route_params, &&router, vec![], &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, &|_, _, _, _, _, _, _, _, _| Ok(())); + &pending_events, &|_, _, _, _, _, _, _, _| Ok(())); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 1); if let Event::PaymentFailed { .. } = events[0] { } else { panic!("Unexpected event"); } @@ -1395,7 +1366,7 @@ mod tests { let err = outbound_payments.send_payment( PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), expired_route_params, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err(); + &pending_events, |_, _, _, _, _, _, _, _| Ok(())).unwrap_err(); if let RetryableSendFailure::PaymentExpired = err { } else { panic!("Unexpected error"); } } } @@ -1431,7 +1402,7 @@ mod tests { outbound_payments.retry_payment_internal( PaymentHash([0; 32]), PaymentId([0; 32]), route_params, &&router, vec![], &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, &|_, _, _, _, _, _, _, _, _| Ok(())); + &pending_events, &|_, _, _, _, _, _, _, _| Ok(())); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 1); if let Event::PaymentFailed { .. } = events[0] { } else { panic!("Unexpected event"); } @@ -1439,7 +1410,7 @@ mod tests { let err = outbound_payments.send_payment( PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, |_, _, _, _, _, _, _, _, _| Ok(())).unwrap_err(); + &pending_events, |_, _, _, _, _, _, _, _| Ok(())).unwrap_err(); if let RetryableSendFailure::RouteNotFound = err { } else { panic!("Unexpected error"); } } @@ -1488,7 +1459,7 @@ mod tests { PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, - |_, _, _, _, _, _, _, _, _| Err(APIError::ChannelUnavailable { err: "test".to_owned() })) + |_, _, _, _, _, _, _, _| Err(APIError::ChannelUnavailable { err: "test".to_owned() })) .unwrap(); let mut events = pending_events.lock().unwrap(); assert_eq!(events.len(), 2); @@ -1506,7 +1477,7 @@ mod tests { outbound_payments.send_payment( PaymentHash([0; 32]), &None, PaymentId([0; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, - &pending_events, |_, _, _, _, _, _, _, _, _| Err(APIError::MonitorUpdateInProgress)) + &pending_events, |_, _, _, _, _, _, _, _| Err(APIError::MonitorUpdateInProgress)) .unwrap(); { let events = pending_events.lock().unwrap(); @@ -1518,7 +1489,7 @@ mod tests { PaymentHash([0; 32]), &None, PaymentId([1; 32]), Retry::Attempts(0), route_params.clone(), &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, - |_, _, _, _, _, _, _, _, _| Err(APIError::APIMisuseError { err: "test".to_owned() })) + |_, _, _, _, _, _, _, _| Err(APIError::APIMisuseError { err: "test".to_owned() })) .unwrap(); let events = pending_events.lock().unwrap(); assert_eq!(events.len(), 2); diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 5bc8a5084f2..8c981fd1c48 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -701,10 +701,6 @@ pub enum Event { /// If this is `Some`, then the corresponding channel should be avoided when the payment is /// retried. May be `None` for older [`Event`] serializations. short_channel_id: Option, - /// Parameters used by LDK to compute a new [`Route`] when retrying the failed payment path. - /// - /// [`Route`]: crate::routing::router::Route - retry: Option, #[cfg(test)] error_code: Option, #[cfg(test)] @@ -992,7 +988,7 @@ impl Writeable for Event { }, &Event::PaymentPathFailed { ref payment_id, ref payment_hash, ref payment_failed_permanently, ref failure, - ref path, ref short_channel_id, ref retry, + ref path, ref short_channel_id, #[cfg(test)] ref error_code, #[cfg(test)] @@ -1010,7 +1006,7 @@ impl Writeable for Event { (3, false, required), // all_paths_failed in LDK versions prior to 0.0.114 (5, *path, vec_type), (7, short_channel_id, option), - (9, retry, option), + (9, None::, option), // retry in LDK versions prior to 0.0.115 (11, payment_id, option), (13, failure, required), }); @@ -1227,7 +1223,6 @@ impl MaybeReadable for Event { let mut network_update = None; let mut path: Option> = Some(vec![]); let mut short_channel_id = None; - let mut retry = None; let mut payment_id = None; let mut failure_opt = None; read_tlv_fields!(reader, { @@ -1236,7 +1231,6 @@ impl MaybeReadable for Event { (2, payment_failed_permanently, required), (5, path, vec_type), (7, short_channel_id, option), - (9, retry, option), (11, payment_id, option), (13, failure_opt, upgradable_option), }); @@ -1248,7 +1242,6 @@ impl MaybeReadable for Event { failure, path: path.unwrap(), short_channel_id, - retry, #[cfg(test)] error_code, #[cfg(test)] diff --git a/pending_changelog/2063.txt b/pending_changelog/2063.txt new file mode 100644 index 00000000000..8ac52a416af --- /dev/null +++ b/pending_changelog/2063.txt @@ -0,0 +1,4 @@ +## API Updates + +- `Event::PaymentPathFailed::retry` will always be `None` if we initiate a payment on 0.0.115 + then downgrade to an earlier version (#2063)