Skip to content

Commit

Permalink
Remove PaymentPathFailed::retry
Browse files Browse the repository at this point in the history 
We now support automatic retries in ChannelManager and no longer support manual
retries, so the field is useless.
  • Loading branch information
@valentinewallace
valentinewallace committed Mar 1, 2023
1 parent 0b1a64f commit 5c76544
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 56 deletions.
4 changes: 2 additions & 2 deletions lightning/src/ln/channelmanager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3782,9 +3782,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(); }
},
Expand Down
9 changes: 1 addition & 8 deletions lightning/src/ln/functional_test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1818,20 +1818,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");
Expand Down
43 changes: 6 additions & 37 deletions lightning/src/ln/outbound_payment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,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);
Expand Down Expand Up @@ -790,7 +782,6 @@ impl OutboundPayments {
failure: events::PathFailure::InitialSend { err: e },
path,
short_channel_id: failed_scid,
retry: None,
#[cfg(test)]
error_code: None,
#[cfg(test)]
Expand Down Expand Up @@ -1120,8 +1111,8 @@ impl OutboundPayments {
pub(super) fn fail_htlc<L: Deref>(
&self, source: &HTLCSource, payment_hash: &PaymentHash, onion_error: &HTLCFailReason,
path: &Vec<RouteHop>, session_priv: &SecretKey, payment_id: &PaymentId,
payment_params: &Option<PaymentParameters>, probing_cookie_secret: [u8; 32],
secp_ctx: &Secp256k1<secp256k1::All>, pending_events: &Mutex<Vec<events::Event>>, logger: &L
probing_cookie_secret: [u8; 32], secp_ctx: &Secp256k1<secp256k1::All>,
pending_events: &Mutex<Vec<events::Event>>, 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);
Expand Down Expand Up @@ -1149,7 +1140,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));
Expand All @@ -1161,27 +1151,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;
}
Expand Down Expand Up @@ -1221,12 +1197,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 {
Expand All @@ -1240,7 +1210,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)]
Expand Down
11 changes: 2 additions & 9 deletions lightning/src/util/events.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<u64>,
/// Parameters used by LDK to compute a new [`Route`] when retrying the failed payment path.
///
/// [`Route`]: crate::routing::router::Route
retry: Option<RouteParameters>,
#[cfg(test)]
error_code: Option<u16>,
#[cfg(test)]
Expand Down Expand Up @@ -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)]
Expand All @@ -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::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
(11, payment_id, option),
(13, failure, required),
});
Expand Down Expand Up @@ -1227,7 +1223,6 @@ impl MaybeReadable for Event {
let mut network_update = None;
let mut path: Option<Vec<RouteHop>> = 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, {
Expand All @@ -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),
});
Expand All @@ -1248,7 +1242,6 @@ impl MaybeReadable for Event {
failure,
path: path.unwrap(),
short_channel_id,
retry,
#[cfg(test)]
error_code,
#[cfg(test)]
Expand Down

0 comments on commit 5c76544

Please sign in to comment.