session_privs: HashSet<[u8; 32]>,
},
Retryable {
- retry_strategy: Retry,
+ retry_strategy: Option<Retry>,
attempts: PaymentAttempts,
route_params: Option<RouteParameters>,
session_privs: HashSet<[u8; 32]>,
attempts.count += 1;
}
}
+ fn is_auto_retryable_now(&self) -> bool {
+ match self {
+ PendingOutboundPayment::Retryable { retry_strategy: Some(strategy), attempts, .. } => {
+ strategy.is_retryable_now(&attempts)
+ },
+ _ => false,
+ }
+ }
fn is_retryable_now(&self) -> bool {
- if let PendingOutboundPayment::Retryable { retry_strategy, attempts, .. } = self {
- return retry_strategy.is_retryable_now(&attempts)
+ match self {
+ PendingOutboundPayment::Retryable { retry_strategy: None, .. } => {
+ // We're handling retries manually, we can always retry.
+ true
+ },
+ PendingOutboundPayment::Retryable { retry_strategy: Some(strategy), attempts, .. } => {
+ strategy.is_retryable_now(&attempts)
+ },
+ _ => false,
}
- false
}
pub fn insert_previously_failed_scid(&mut self, scid: u64) {
if let PendingOutboundPayment::Retryable { route_params: Some(params), .. } = self {
pub enum Retry {
/// Max number of attempts to retry payment.
///
- /// Note that this is the number of *path* failures, not full payment retries. For multi-path
- /// payments, if this is less than the total number of paths, we will never even retry all of the
- /// payment's paths.
+ /// Each attempt may be multiple HTLCs along multiple paths if the router decides to split up a
+ /// retry, and may retry multiple failed HTLCs at once if they failed around the same time and
+ /// were retried along a route from a single call to [`Router::find_route`].
Attempts(usize),
#[cfg(not(feature = "no-std"))]
/// Time elapsed before abandoning retries for a payment.
F: Fn(&Vec<RouteHop>, &Option<PaymentParameters>, &PaymentHash, &Option<PaymentSecret>, u64,
u32, PaymentId, &Option<PaymentPreimage>, [u8; 32]) -> Result<(), APIError>
{
- let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, Retry::Attempts(0), None, entropy_source, best_block_height)?;
+ let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, route, None, None, entropy_source, best_block_height)?;
self.pay_route_internal(route, payment_hash, payment_secret, None, payment_id, None,
onion_session_privs, node_signer, best_block_height, &send_payment_along_path)
.map_err(|e| { self.remove_outbound_if_all_failed(payment_id, &e); e })
None => PaymentPreimage(entropy_source.get_secure_random_bytes()),
};
let payment_hash = PaymentHash(Sha256::hash(&preimage.0).into_inner());
- let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?;
+ let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, None, None, entropy_source, best_block_height)?;
match self.pay_route_internal(route, payment_hash, &None, Some(preimage), payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) {
Ok(()) => Ok(payment_hash),
let mut outbounds = self.pending_outbound_payments.lock().unwrap();
let mut retry_id_route_params = None;
for (pmt_id, pmt) in outbounds.iter_mut() {
- if pmt.is_retryable_now() {
+ if pmt.is_auto_retryable_now() {
if let PendingOutboundPayment::Retryable { pending_amt_msat, total_msat, route_params: Some(params), .. } = pmt {
if pending_amt_msat < total_msat {
retry_id_route_params = Some((*pmt_id, params.clone()));
- pmt.increment_attempts();
break
}
}
}))?;
let res = if let Some((payment_hash, payment_secret, retry_strategy)) = initial_send_info {
- let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, retry_strategy, Some(route_params.clone()), entropy_source, best_block_height)?;
+ let onion_session_privs = self.add_new_pending_payment(payment_hash, *payment_secret, payment_id, &route, Some(retry_strategy), Some(route_params.clone()), entropy_source, best_block_height)?;
self.pay_route_internal(&route, payment_hash, payment_secret, None, payment_id, None, onion_session_privs, node_signer, best_block_height, send_payment_along_path)
} else {
self.retry_payment_with_route(&route, payment_id, entropy_source, node_signer, best_block_height, send_payment_along_path)
};
match res {
Err(PaymentSendFailure::AllFailedResendSafe(_)) => {
- let mut outbounds = self.pending_outbound_payments.lock().unwrap();
- if let Some(payment) = outbounds.get_mut(&payment_id) {
- let retryable = payment.is_retryable_now();
- if retryable {
- payment.increment_attempts();
- } else { return res }
- } else { return res }
- core::mem::drop(outbounds);
let retry_res = self.pay_internal(payment_id, None, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, send_payment_along_path);
log_info!(logger, "Result retrying payment id {}: {:?}", log_bytes!(payment_id.0), retry_res);
+ if let Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError { err })) = &retry_res {
+ if err.starts_with("Retries exhausted ") { return res; }
+ }
retry_res
},
- Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, .. }) => {
- let mut outbounds = self.pending_outbound_payments.lock().unwrap();
- if let Some(payment) = outbounds.get_mut(&payment_id) {
- let retryable = payment.is_retryable_now();
- if retryable {
- payment.increment_attempts();
- } else { return Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, payment_id }) }
- } else { return Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), results, payment_id }) }
- core::mem::drop(outbounds);
-
+ Err(PaymentSendFailure::PartialFailure { failed_paths_retry: Some(retry), .. }) => {
// Some paths were sent, even if we failed to send the full MPP value our recipient may
// misbehave and claim the funds, at which point we have to consider the payment sent, so
// return `Ok()` here, ignoring any retry errors.
}));
},
};
+ if !payment.is_retryable_now() {
+ return Err(PaymentSendFailure::ParameterError(APIError::APIMisuseError {
+ err: format!("Retries exhausted for payment id {}", log_bytes!(payment_id.0)),
+ }))
+ }
+ payment.increment_attempts();
for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) {
assert!(payment.insert(*session_priv_bytes, path));
}
}
let route = Route { paths: vec![hops], payment_params: None };
- let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, Retry::Attempts(0), None, entropy_source, best_block_height)?;
+ let onion_session_privs = self.add_new_pending_payment(payment_hash, None, payment_id, &route, None, None, entropy_source, best_block_height)?;
match self.pay_route_internal(&route, payment_hash, &None, None, payment_id, None, onion_session_privs, node_signer, best_block_height, &send_payment_along_path) {
Ok(()) => Ok((payment_hash, payment_id)),
#[cfg(test)]
pub(super) fn test_add_new_pending_payment<ES: Deref>(
&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId,
- route: &Route, retry_strategy: Retry, entropy_source: &ES, best_block_height: u32
+ route: &Route, retry_strategy: Option<Retry>, entropy_source: &ES, best_block_height: u32
) -> Result<Vec<[u8; 32]>, PaymentSendFailure> where ES::Target: EntropySource {
self.add_new_pending_payment(payment_hash, payment_secret, payment_id, route, retry_strategy, None, entropy_source, best_block_height)
}
pub(super) fn add_new_pending_payment<ES: Deref>(
&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId,
- route: &Route, retry_strategy: Retry, route_params: Option<RouteParameters>,
+ route: &Route, retry_strategy: Option<Retry>, route_params: Option<RouteParameters>,
entropy_source: &ES, best_block_height: u32
) -> Result<Vec<[u8; 32]>, PaymentSendFailure> where ES::Target: EntropySource {
let mut onion_session_privs = Vec::with_capacity(route.paths.len());
log_trace!(logger, "Received failure of HTLC with payment_hash {} after payment completion", log_bytes!(payment_hash.0));
return
}
- let is_retryable_now = payment.get().is_retryable_now();
+ let is_retryable_now = payment.get().is_auto_retryable_now();
if let Some(scid) = short_channel_id {
payment.get_mut().insert_previously_failed_scid(scid);
}
(0, session_privs, required),
(1, pending_fee_msat, option),
(2, payment_hash, required),
- (not_written, retry_strategy, (static_value, Retry::Attempts(0))),
+ (not_written, retry_strategy, (static_value, None)),
(4, payment_secret, option),
(not_written, attempts, (static_value, PaymentAttempts::new())),
(6, total_msat, required),
let err = if on_retry {
outbound_payments.add_new_pending_payment(PaymentHash([0; 32]), None, PaymentId([0; 32]),
- &Route { paths: vec![], payment_params: None }, Retry::Attempts(1), Some(route_params.clone()),
+ &Route { paths: vec![], payment_params: None }, Some(Retry::Attempts(1)), Some(route_params.clone()),
&&keys_manager, 0).unwrap();
outbound_payments.pay_internal(
PaymentId([0; 32]), None, route_params, &&router, vec![], InFlightHtlcs::new(),
node_features: nodes[1].node.node_features(),
short_channel_id: chan_1_scid,
channel_features: nodes[1].node.channel_features(),
- fee_msat: 0,
+ fee_msat: 0, // nodes[1] will fail the payment as we don't pay its fee
cltv_expiry_delta: 100,
}, RouteHop {
pubkey: nodes[2].node.get_our_node_id(),
node_features: nodes[1].node.node_features(),
short_channel_id: chan_1_scid,
channel_features: nodes[1].node.channel_features(),
- fee_msat: 0,
+ fee_msat: 0, // nodes[1] will fail the payment as we don't pay its fee
cltv_expiry_delta: 100,
}, RouteHop {
pubkey: nodes[2].node.get_our_node_id(),
payment_params: Some(PaymentParameters::from_node_id(nodes[2].node.get_our_node_id(), TEST_FINAL_CLTV)),
};
nodes[0].router.expect_find_route(route_params.clone(), Ok(route.clone()));
- // On retry, we'll only be asked for one path
- route.paths.remove(1);
let mut second_payment_params = route_params.payment_params.clone();
second_payment_params.previously_failed_channels = vec![chan_2_scid, chan_2_scid];
+ // On retry, we'll only return one path
+ route.paths.remove(1);
+ route.paths[0][1].fee_msat = amt_msat;
nodes[0].router.expect_find_route(RouteParameters {
payment_params: second_payment_params,
final_value_msat: amt_msat, final_cltv_expiry_delta: TEST_FINAL_CLTV,
// At this point A has sent two HTLCs which both failed due to lack of fee. It now has two
// pending `PaymentPathFailed` events, one with `all_paths_failed` unset, and the second
- // with it set. The first event will use up the only retry we are allowed, with the second
- // `PaymentPathFailed` being passed up to the user (us, in this case). Previously, we'd
- // treated this as "HTLC complete" and dropped the retry counter, causing us to retry again
- // if the final HTLC failed.
+ // with it set.
+ //
+ // Previously, we retried payments in an event consumer, which would retry each
+ // `PaymentPathFailed` individually. In that setup, we had retried the payment in response to
+ // the first `PaymentPathFailed`, then seen the second `PaymentPathFailed` with
+ // `all_paths_failed` set and assumed the payment was completely failed. We ultimately fixed it
+ // by adding the `PaymentFailed` event.
+ //
+ // Because we now retry payments as a batch, we simply return a single-path route in the
+ // second, batched, request, have that fail, then complete the payment via `abandon_payment`.
let mut events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 4);
match events[0] {
projects / rust-lightning / commitdiff