Skip to content

Avoid previously failed hops on PaymentFailure::AllFailedResendSafe #1969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tnull opened this issue Jan 18, 2023 · 0 comments · Fixed by #2014
Closed

Avoid previously failed hops on PaymentFailure::AllFailedResendSafe #1969

tnull opened this issue Jan 18, 2023 · 0 comments · Fixed by #2014
Assignees

Comments

@tnull
Copy link
Contributor

tnull commented Jan 18, 2023

Currently, only PaymentFailure::PartialFailure carry PaymentParameters, which is why InvoicePayer retries any PaymentFailure::AllFailedResendSafes with empty previously_failed_channels:

PaymentSendFailure::AllFailedResendSafe(_) => {
let mut payment_cache = self.payment_cache.lock().unwrap();
let payment_attempts = payment_cache.get_mut(&payment_hash).unwrap();
payment_attempts.count += 1;
if self.retry.is_retryable_now(payment_attempts) {
core::mem::drop(payment_cache);
Ok(self.pay_internal(params, payment_hash, send_payment)?)
} else {
Err(e)
}

As this may result in a loop where a failing route is retried over and over again until we run out of time or attempts, we probably want to consider any previously failed channels in the AllFailedResendSafe case also.

As InvoicePayer is pending deprecation anyways, this probably should be fixed as part of the migration of the retry logic to ChannelManager.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants