Skip to content

Commit 2f087ab

Browse files
committed
Refactor send_payment_for_bolt12_invoice
The BOLT11 and BOLT12 outbound payment initiation code differ in that the latter re-uses the retry path (i.e., find_route_and_send_payment). The drawback of this is that Ok is returned even if there is an error finding a route. Refactor send_payment_for_bolt12_invoice such that it re-uses find_initial_route instead so that errors can be returned.
1 parent c8fd681 commit 2f087ab

File tree

1 file changed

+63
-32
lines changed

1 file changed

+63
-32
lines changed

lightning/src/ln/outbound_payment.rs

+63-32
Original file line numberDiff line numberDiff line change
@@ -800,20 +800,24 @@ impl OutboundPayments {
800800
{
801801
let payment_hash = invoice.payment_hash();
802802
let max_total_routing_fee_msat;
803+
let retry_strategy;
803804
match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
804805
hash_map::Entry::Occupied(entry) => match entry.get() {
805-
PendingOutboundPayment::AwaitingInvoice { retry_strategy, max_total_routing_fee_msat: max_total_fee, .. } => {
806+
PendingOutboundPayment::AwaitingInvoice {
807+
retry_strategy: retry, max_total_routing_fee_msat: max_total_fee, ..
808+
} => {
809+
retry_strategy = Some(*retry);
806810
max_total_routing_fee_msat = *max_total_fee;
807811
*entry.into_mut() = PendingOutboundPayment::InvoiceReceived {
808812
payment_hash,
809-
retry_strategy: *retry_strategy,
813+
retry_strategy: *retry,
810814
max_total_routing_fee_msat,
811815
};
812816
},
813817
_ => return Err(Bolt12PaymentError::DuplicateInvoice),
814818
},
815819
hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice),
816-
};
820+
}
817821

818822
let mut payment_params = PaymentParameters::from_bolt12_invoice(&invoice);
819823

@@ -839,25 +843,64 @@ impl OutboundPayments {
839843
let mut route_params = RouteParameters::from_payment_params_and_value(
840844
payment_params, amount_msat
841845
);
842-
onion_utils::set_max_path_length(
843-
&mut route_params, &RecipientOnionFields::spontaneous_empty(), None, best_block_height
844-
)
845-
.map_err(|()| {
846-
log_error!(logger, "Can't construct an onion packet without exceeding 1300-byte onion \
847-
hop_data length for payment with id {} and hash {}", payment_id, payment_hash);
848-
Bolt12PaymentError::SendingFailed(RetryableSendFailure::OnionPacketSizeExceeded)
849-
})?;
850846

851847
if let Some(max_fee_msat) = max_total_routing_fee_msat {
852848
route_params.max_total_routing_fee_msat = Some(max_fee_msat);
853849
}
854850

855-
self.find_route_and_send_payment(
856-
payment_hash, payment_id, route_params, router, first_hops, &inflight_htlcs,
857-
entropy_source, node_signer, best_block_height, logger, pending_events,
858-
&send_payment_along_path
851+
let recipient_onion = RecipientOnionFields {
852+
payment_secret: None,
853+
payment_metadata: None,
854+
custom_tlvs: vec![],
855+
};
856+
let route = match self.find_initial_route(
857+
payment_id, payment_hash, &recipient_onion, None, &mut route_params, router,
858+
&first_hops, &inflight_htlcs, node_signer, best_block_height, logger,
859+
) {
860+
Ok(route) => route,
861+
Err(e) => {
862+
let reason = match e {
863+
RetryableSendFailure::PaymentExpired => PaymentFailureReason::PaymentExpired,
864+
RetryableSendFailure::RouteNotFound => PaymentFailureReason::RouteNotFound,
865+
RetryableSendFailure::DuplicatePayment => PaymentFailureReason::UnexpectedError,
866+
RetryableSendFailure::OnionPacketSizeExceeded => PaymentFailureReason::UnexpectedError,
867+
};
868+
self.abandon_payment(payment_id, reason, pending_events);
869+
return Err(Bolt12PaymentError::SendingFailed(e));
870+
},
871+
};
872+
873+
let payment_params = Some(route_params.payment_params.clone());
874+
let (retryable_payment, onion_session_privs) = self.create_pending_payment(
875+
payment_hash, recipient_onion.clone(), None, &route,
876+
retry_strategy, payment_params, entropy_source, best_block_height
859877
);
878+
match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
879+
hash_map::Entry::Occupied(entry) => match entry.get() {
880+
PendingOutboundPayment::InvoiceReceived { .. } => {
881+
*entry.into_mut() = retryable_payment;
882+
},
883+
_ => return Err(Bolt12PaymentError::DuplicateInvoice),
884+
},
885+
hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice),
886+
}
860887

888+
let result = self.pay_route_internal(
889+
&route, payment_hash, &recipient_onion, None, payment_id,
890+
Some(route_params.final_value_msat), onion_session_privs, node_signer,
891+
best_block_height, &send_payment_along_path
892+
);
893+
log_info!(
894+
logger, "Sending payment with id {} and hash {} returned {:?}", payment_id,
895+
payment_hash, result
896+
);
897+
if let Err(e) = result {
898+
self.handle_pay_route_err(
899+
e, payment_id, payment_hash, route, route_params, router, first_hops,
900+
&inflight_htlcs, entropy_source, node_signer, best_block_height, logger,
901+
pending_events, &send_payment_along_path
902+
);
903+
}
861904
Ok(())
862905
}
863906

@@ -1134,21 +1177,9 @@ impl OutboundPayments {
11341177
log_error!(logger, "Payment not yet sent");
11351178
return
11361179
},
1137-
PendingOutboundPayment::InvoiceReceived { payment_hash, retry_strategy, .. } => {
1138-
let total_amount = route_params.final_value_msat;
1139-
let recipient_onion = RecipientOnionFields {
1140-
payment_secret: None,
1141-
payment_metadata: None,
1142-
custom_tlvs: vec![],
1143-
};
1144-
let retry_strategy = Some(*retry_strategy);
1145-
let payment_params = Some(route_params.payment_params.clone());
1146-
let (retryable_payment, onion_session_privs) = self.create_pending_payment(
1147-
*payment_hash, recipient_onion.clone(), None, &route,
1148-
retry_strategy, payment_params, entropy_source, best_block_height
1149-
);
1150-
*payment.into_mut() = retryable_payment;
1151-
(total_amount, recipient_onion, None, onion_session_privs)
1180+
PendingOutboundPayment::InvoiceReceived { .. } => {
1181+
log_error!(logger, "Payment already initiating");
1182+
return
11521183
},
11531184
PendingOutboundPayment::Fulfilled { .. } => {
11541185
log_error!(logger, "Payment already completed");
@@ -2290,7 +2321,7 @@ mod tests {
22902321
&&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events,
22912322
|_| panic!()
22922323
),
2293-
Ok(()),
2324+
Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::PaymentExpired)),
22942325
);
22952326
assert!(!outbound_payments.has_pending_payments());
22962327

@@ -2351,7 +2382,7 @@ mod tests {
23512382
&&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events,
23522383
|_| panic!()
23532384
),
2354-
Ok(()),
2385+
Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::RouteNotFound)),
23552386
);
23562387
assert!(!outbound_payments.has_pending_payments());
23572388

0 commit comments

Comments
 (0)