test_utils: parameterize TestRouter by TestScorer
[rust-lightning] / lightning / src / ln / channelmanager.rs
index 17dcab9fe73323e9cea6f5cb04a7c16945591c7d..cfb462b0120cac784c2393e4e27450c14747a6bc 100644 (file)
@@ -45,7 +45,7 @@ use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, No
 #[cfg(any(feature = "_test_utils", test))]
 use crate::ln::features::InvoiceFeatures;
 use crate::routing::gossip::NetworkGraph;
-use crate::routing::router::{DefaultRouter, InFlightHtlcs, PaymentParameters, Route, RouteHop, RoutePath, Router};
+use crate::routing::router::{DefaultRouter, InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router};
 use crate::routing::scoring::ProbabilisticScorer;
 use crate::ln::msgs;
 use crate::ln::onion_utils;
@@ -53,7 +53,7 @@ use crate::ln::onion_utils::HTLCFailReason;
 use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
 #[cfg(test)]
 use crate::ln::outbound_payment;
-use crate::ln::outbound_payment::{OutboundPayments, PendingOutboundPayment};
+use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment};
 use crate::ln::wire::Encode;
 use crate::chain::keysinterface::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, ChannelSigner};
 use crate::util::config::{UserConfig, ChannelConfig};
@@ -76,7 +76,7 @@ use core::time::Duration;
 use core::ops::Deref;
 
 // Re-export this for use in the public API.
-pub use crate::ln::outbound_payment::PaymentSendFailure;
+pub use crate::ln::outbound_payment::{PaymentSendFailure, Retry};
 
 // We hold various information about HTLC relay in the HTLC objects in Channel itself:
 //
@@ -245,6 +245,10 @@ pub(crate) enum HTLCSource {
                first_hop_htlc_msat: u64,
                payment_id: PaymentId,
                payment_secret: Option<PaymentSecret>,
+               /// 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<PaymentParameters>,
        },
 }
@@ -289,6 +293,25 @@ struct ReceiveError {
        msg: &'static str,
 }
 
+/// This enum is used to specify which error data to send to peers when failing back an HTLC
+/// using [`ChannelManager::fail_htlc_backwards_with_reason`].
+///
+/// For more info on failure codes, see <https://github.com/lightning/bolts/blob/master/04-onion-routing.md#failure-messages>.
+#[derive(Clone, Copy)]
+pub enum FailureCode {
+       /// We had a temporary error processing the payment. Useful if no other error codes fit
+       /// and you want to indicate that the payer may want to retry.
+       TemporaryNodeFailure             = 0x2000 | 2,
+       /// We have a required feature which was not in this onion. For example, you may require
+       /// some additional metadata that was not provided with this payment.
+       RequiredNodeFeatureMissing       = 0x4000 | 0x2000 | 3,
+       /// You may wish to use this when a `payment_preimage` is unknown, or the CLTV expiry of
+       /// the HTLC is too close to the current block height for safe handling.
+       /// Using this failure code in [`ChannelManager::fail_htlc_backwards_with_reason`] is
+       /// equivalent to calling [`ChannelManager::fail_htlc_backwards`].
+       IncorrectOrUnknownPaymentDetails = 0x4000 | 15,
+}
+
 type ShutdownResult = (Option<(OutPoint, ChannelMonitorUpdate)>, Vec<(HTLCSource, PaymentHash, PublicKey, [u8; 32])>);
 
 /// Error type returned across the peer_state mutex boundary. When an Err is generated for a
@@ -388,7 +411,7 @@ impl MsgHandleErrInternal {
 /// Event::PendingHTLCsForwardable for the API guidelines indicating how long should be waited).
 /// This provides some limited amount of privacy. Ideally this would range from somewhere like one
 /// second to 30 seconds, but people expect lightning to be, you know, kinda fast, sadly.
-const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u64 = 100;
+pub(super) const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u64 = 100;
 
 /// For events which result in both a RevokeAndACK and a CommitmentUpdate, by default they should
 /// be sent in the order they appear in the return value, however sometimes the order needs to be
@@ -1131,6 +1154,36 @@ impl ChannelDetails {
        }
 }
 
+/// Used by [`ChannelManager::list_recent_payments`] to express the status of recent payments.
+/// These include payments that have yet to find a successful path, or have unresolved HTLCs.
+#[derive(Debug, PartialEq)]
+pub enum RecentPaymentDetails {
+       /// When a payment is still being sent and awaiting successful delivery.
+       Pending {
+               /// Hash of the payment that is currently being sent but has yet to be fulfilled or
+               /// abandoned.
+               payment_hash: PaymentHash,
+               /// Total amount (in msat, excluding fees) across all paths for this payment,
+               /// not just the amount currently inflight.
+               total_msat: u64,
+       },
+       /// When a pending payment is fulfilled, we continue tracking it until all pending HTLCs have
+       /// been resolved. Upon receiving [`Event::PaymentSent`], we delay for a few minutes before the
+       /// payment is removed from tracking.
+       Fulfilled {
+               /// Hash of the payment that was claimed. `None` for serializations of [`ChannelManager`]
+               /// made before LDK version 0.0.104.
+               payment_hash: Option<PaymentHash>,
+       },
+       /// After a payment is explicitly abandoned by calling [`ChannelManager::abandon_payment`], it
+       /// is marked as abandoned until an [`Event::PaymentFailed`] is generated. A payment could also
+       /// be marked as abandoned if pathfinding fails repeatedly or retries have been exhausted.
+       Abandoned {
+               /// Hash of the payment that we have given up trying to send.
+               payment_hash: PaymentHash,
+       },
+}
+
 /// Route hints used in constructing invoices for [phantom node payents].
 ///
 /// [phantom node payments]: crate::chain::keysinterface::PhantomKeysManager
@@ -1668,6 +1721,34 @@ where
                self.list_channels_with_filter(|&(_, ref channel)| channel.is_live())
        }
 
+       /// Returns in an undefined order recent payments that -- if not fulfilled -- have yet to find a
+       /// successful path, or have unresolved HTLCs.
+       ///
+       /// This can be useful for payments that may have been prepared, but ultimately not sent, as a
+       /// result of a crash. If such a payment exists, is not listed here, and an
+       /// [`Event::PaymentSent`] has not been received, you may consider retrying the payment.
+       ///
+       /// [`Event::PaymentSent`]: events::Event::PaymentSent
+       pub fn list_recent_payments(&self) -> Vec<RecentPaymentDetails> {
+               self.pending_outbound_payments.pending_outbound_payments.lock().unwrap().iter()
+                       .filter_map(|(_, pending_outbound_payment)| match pending_outbound_payment {
+                               PendingOutboundPayment::Retryable { payment_hash, total_msat, .. } => {
+                                       Some(RecentPaymentDetails::Pending {
+                                               payment_hash: *payment_hash,
+                                               total_msat: *total_msat,
+                                       })
+                               },
+                               PendingOutboundPayment::Abandoned { payment_hash, .. } => {
+                                       Some(RecentPaymentDetails::Abandoned { payment_hash: *payment_hash })
+                               },
+                               PendingOutboundPayment::Fulfilled { payment_hash, .. } => {
+                                       Some(RecentPaymentDetails::Fulfilled { payment_hash: *payment_hash })
+                               },
+                               PendingOutboundPayment::Legacy { .. } => None
+                       })
+                       .collect()
+       }
+
        /// Helper function that issues the channel close events
        fn issue_channel_close_events(&self, channel: &Channel<<SP::Target as SignerProvider>::Signer>, closure_reason: ClosureReason) {
                let mut pending_events_lock = self.pending_events.lock().unwrap();
@@ -2392,9 +2473,14 @@ where
 
        /// Sends a payment along a given route.
        ///
-       /// Value parameters are provided via the last hop in route, see documentation for RouteHop
+       /// Value parameters are provided via the last hop in route, see documentation for [`RouteHop`]
        /// fields for more info.
        ///
+       /// May generate SendHTLCs message(s) event on success, which should be relayed (e.g. via
+       /// [`PeerManager::process_events`]).
+       ///
+       /// # Avoiding Duplicate Payments
+       ///
        /// If a pending payment is currently in-flight with the same [`PaymentId`] provided, this
        /// method will error with an [`APIError::InvalidRoute`]. Note, however, that once a payment
        /// is no longer pending (either via [`ChannelManager::abandon_payment`], or handling of an
@@ -2407,12 +2493,16 @@ where
        /// consider using the [`PaymentHash`] as the key for tracking payments. In that case, the
        /// [`PaymentId`] should be a copy of the [`PaymentHash`] bytes.
        ///
-       /// May generate SendHTLCs message(s) event on success, which should be relayed (e.g. via
-       /// [`PeerManager::process_events`]).
+       /// Additionally, in the scenario where we begin the process of sending a payment, but crash
+       /// before `send_payment` returns (or prior to [`ChannelMonitorUpdate`] persistence if you're
+       /// using [`ChannelMonitorUpdateStatus::InProgress`]), the payment may be lost on restart. See
+       /// [`ChannelManager::list_recent_payments`] for more information.
+       ///
+       /// # Possible Error States on [`PaymentSendFailure`]
        ///
        /// Each path may have a different return value, and PaymentSendValue may return a Vec with
        /// each entry matching the corresponding-index entry in the route paths, see
-       /// PaymentSendFailure for more info.
+       /// [`PaymentSendFailure`] for more info.
        ///
        /// In general, a path may raise:
        ///  * [`APIError::InvalidRoute`] when an invalid route or forwarding parameter (cltv_delta, fee,
@@ -2427,18 +2517,21 @@ where
        /// irrevocably committed to on our end. In such a case, do NOT retry the payment with a
        /// different route unless you intend to pay twice!
        ///
-       /// payment_secret is unrelated to payment_hash (or PaymentPreimage) and exists to authenticate
-       /// the sender to the recipient and prevent payment-probing (deanonymization) attacks. For
-       /// newer nodes, it will be provided to you in the invoice. If you do not have one, the Route
-       /// must not contain multiple paths as multi-path payments require a recipient-provided
-       /// payment_secret.
+       /// # A caution on `payment_secret`
+       ///
+       /// `payment_secret` is unrelated to `payment_hash` (or [`PaymentPreimage`]) and exists to
+       /// authenticate the sender to the recipient and prevent payment-probing (deanonymization)
+       /// attacks. For newer nodes, it will be provided to you in the invoice. If you do not have one,
+       /// the [`Route`] must not contain multiple paths as multi-path payments require a
+       /// recipient-provided `payment_secret`.
        ///
-       /// If a payment_secret *is* provided, we assume that the invoice had the payment_secret feature
-       /// bit set (either as required or as available). If multiple paths are present in the Route,
-       /// we assume the invoice had the basic_mpp feature set.
+       /// If a `payment_secret` *is* provided, we assume that the invoice had the payment_secret
+       /// feature bit set (either as required or as available). If multiple paths are present in the
+       /// [`Route`], we assume the invoice had the basic_mpp feature set.
        ///
        /// [`Event::PaymentSent`]: events::Event::PaymentSent
        /// [`PeerManager::process_events`]: crate::ln::peer_handler::PeerManager::process_events
+       /// [`ChannelMonitorUpdateStatus::InProgress`]: crate::chain::ChannelMonitorUpdateStatus::InProgress
        pub fn send_payment(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
                let best_block_height = self.best_block.read().unwrap().height();
                self.pending_outbound_payments
@@ -2447,6 +2540,18 @@ where
                                self.send_payment_along_path(path, payment_params, 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
+       /// `route_params` and retry failed payment paths based on `retry_strategy`.
+       pub fn send_payment_with_retry(&self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<(), PaymentSendFailure> {
+               let best_block_height = self.best_block.read().unwrap().height();
+               self.pending_outbound_payments
+                       .send_payment(payment_hash, payment_secret, payment_id, 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,
+                               |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))
+       }
+
        #[cfg(test)]
        fn test_send_payment_internal(&self, route: &Route, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>, keysend_preimage: Option<PaymentPreimage>, payment_id: PaymentId, recv_value_msat: Option<u64>, onion_session_privs: Vec<[u8; 32]>) -> Result<(), PaymentSendFailure> {
                let best_block_height = self.best_block.read().unwrap().height();
@@ -2458,7 +2563,7 @@ where
        #[cfg(test)]
        pub(crate) fn test_add_new_pending_payment(&self, payment_hash: PaymentHash, payment_secret: Option<PaymentSecret>, payment_id: PaymentId, route: &Route) -> Result<Vec<[u8; 32]>, PaymentSendFailure> {
                let best_block_height = self.best_block.read().unwrap().height();
-               self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, &self.entropy_source, best_block_height)
+               self.pending_outbound_payments.test_add_new_pending_payment(payment_hash, payment_secret, payment_id, route, None, &self.entropy_source, best_block_height)
        }
 
 
@@ -2523,7 +2628,26 @@ where
        /// [`send_payment`]: Self::send_payment
        pub fn send_spontaneous_payment(&self, route: &Route, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId) -> Result<PaymentHash, PaymentSendFailure> {
                let best_block_height = self.best_block.read().unwrap().height();
-               self.pending_outbound_payments.send_spontaneous_payment(route, payment_preimage, payment_id, &self.entropy_source, &self.node_signer, best_block_height,
+               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))
+       }
+
+       /// Similar to [`ChannelManager::send_spontaneous_payment`], but will automatically find a route
+       /// based on `route_params` and retry failed payment paths based on `retry_strategy`.
+       ///
+       /// See [`PaymentParameters::for_keysend`] for help in constructing `route_params` for spontaneous
+       /// payments.
+       ///
+       /// [`PaymentParameters::for_keysend`]: crate::routing::router::PaymentParameters::for_keysend
+       pub fn send_spontaneous_payment_with_retry(&self, payment_preimage: Option<PaymentPreimage>, payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry) -> Result<PaymentHash, PaymentSendFailure> {
+               let best_block_height = self.best_block.read().unwrap().height();
+               self.pending_outbound_payments.send_spontaneous_payment(payment_preimage, payment_id,
+                       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,
                        |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))
        }
@@ -3261,6 +3385,12 @@ where
                        }
                }
 
+               let best_block_height = self.best_block.read().unwrap().height();
+               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.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));
+
                for (htlc_source, payment_hash, failure_reason, destination) in failed_forwards.drain(..) {
                        self.fail_htlc_backwards_internal(&htlc_source, &payment_hash, &failure_reason, destination);
                }
@@ -3483,21 +3613,40 @@ where
        /// [`events::Event::PaymentClaimed`] events even for payments you intend to fail, especially on
        /// startup during which time claims that were in-progress at shutdown may be replayed.
        pub fn fail_htlc_backwards(&self, payment_hash: &PaymentHash) {
+               self.fail_htlc_backwards_with_reason(payment_hash, &FailureCode::IncorrectOrUnknownPaymentDetails);
+       }
+
+       /// This is a variant of [`ChannelManager::fail_htlc_backwards`] that allows you to specify the
+       /// reason for the failure.
+       ///
+       /// See [`FailureCode`] for valid failure codes.
+       pub fn fail_htlc_backwards_with_reason(&self, payment_hash: &PaymentHash, failure_code: &FailureCode) {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
                let removed_source = self.claimable_payments.lock().unwrap().claimable_htlcs.remove(payment_hash);
                if let Some((_, mut sources)) = removed_source {
                        for htlc in sources.drain(..) {
-                               let mut htlc_msat_height_data = htlc.value.to_be_bytes().to_vec();
-                               htlc_msat_height_data.extend_from_slice(&self.best_block.read().unwrap().height().to_be_bytes());
+                               let reason = self.get_htlc_fail_reason_from_failure_code(failure_code, &htlc);
                                let source = HTLCSource::PreviousHopData(htlc.prev_hop);
-                               let reason = HTLCFailReason::reason(0x4000 | 15, htlc_msat_height_data);
                                let receiver = HTLCDestination::FailedPayment { payment_hash: *payment_hash };
                                self.fail_htlc_backwards_internal(&source, &payment_hash, &reason, receiver);
                        }
                }
        }
 
+       /// Gets error data to form an [`HTLCFailReason`] given a [`FailureCode`] and [`ClaimableHTLC`].
+       fn get_htlc_fail_reason_from_failure_code(&self, failure_code: &FailureCode, htlc: &ClaimableHTLC) -> HTLCFailReason {
+               match failure_code {
+                       FailureCode::TemporaryNodeFailure => HTLCFailReason::from_failure_code(*failure_code as u16),
+                       FailureCode::RequiredNodeFeatureMissing => HTLCFailReason::from_failure_code(*failure_code as u16),
+                       FailureCode::IncorrectOrUnknownPaymentDetails => {
+                               let mut htlc_msat_height_data = htlc.value.to_be_bytes().to_vec();
+                               htlc_msat_height_data.extend_from_slice(&self.best_block.read().unwrap().height().to_be_bytes());
+                               HTLCFailReason::reason(*failure_code as u16, htlc_msat_height_data)
+                       }
+               }
+       }
+
        /// Gets an HTLC onion failure code and error data for an `UPDATE` error, given the error code
        /// that we want to return and a channel.
        ///
@@ -4895,7 +5044,7 @@ where
                                        ), chan),
                                        // Note that announcement_signatures fails if the channel cannot be announced,
                                        // so get_channel_update_for_broadcast will never fail by the time we get here.
-                                       update_msg: self.get_channel_update_for_broadcast(chan.get()).unwrap(),
+                                       update_msg: Some(self.get_channel_update_for_broadcast(chan.get()).unwrap()),
                                });
                        },
                        hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id))
@@ -5269,7 +5418,8 @@ where
        /// [`PaymentHash`] and [`PaymentPreimage`] for you.
        ///
        /// The [`PaymentPreimage`] will ultimately be returned to you in the [`PaymentClaimable`], which
-       /// will have the [`PaymentClaimable::payment_preimage`] field filled in. That should then be
+       /// will have the [`PaymentClaimable::purpose`] be [`PaymentPurpose::InvoicePayment`] with
+       /// its [`PaymentPurpose::InvoicePayment::payment_preimage`] field filled in. That should then be
        /// passed directly to [`claim_funds`].
        ///
        /// See [`create_inbound_payment_for_hash`] for detailed documentation on behavior and requirements.
@@ -5289,7 +5439,9 @@ where
        ///
        /// [`claim_funds`]: Self::claim_funds
        /// [`PaymentClaimable`]: events::Event::PaymentClaimable
-       /// [`PaymentClaimable::payment_preimage`]: events::Event::PaymentClaimable::payment_preimage
+       /// [`PaymentClaimable::purpose`]: events::Event::PaymentClaimable::purpose
+       /// [`PaymentPurpose::InvoicePayment`]: events::PaymentPurpose::InvoicePayment
+       /// [`PaymentPurpose::InvoicePayment::payment_preimage`]: events::PaymentPurpose::InvoicePayment::payment_preimage
        /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
        pub fn create_inbound_payment(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32,
                min_final_cltv_expiry_delta: Option<u16>) -> Result<(PaymentHash, PaymentSecret), ()> {
@@ -5464,6 +5616,12 @@ where
                events.into_inner()
        }
 
+       #[cfg(feature = "_test_utils")]
+       pub fn push_pending_event(&self, event: events::Event) {
+               let mut events = self.pending_events.lock().unwrap();
+               events.push(event);
+       }
+
        #[cfg(test)]
        pub fn pop_pending_event(&self) -> Option<events::Event> {
                let mut events = self.pending_events.lock().unwrap();
@@ -5730,8 +5888,8 @@ where
                        let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                        let peer_state = &mut *peer_state_lock;
                        for chan in peer_state.channel_by_id.values() {
-                               if let (Some(funding_txo), block_hash) = (chan.get_funding_txo(), chan.get_funding_tx_confirmed_in()) {
-                                       res.push((funding_txo.txid, block_hash));
+                               if let (Some(funding_txo), Some(block_hash)) = (chan.get_funding_txo(), chan.get_funding_tx_confirmed_in()) {
+                                       res.push((funding_txo.txid, Some(block_hash)));
                                }
                        }
                }
@@ -5815,7 +5973,7 @@ where
                                                                                msg: announcement,
                                                                                // Note that announcement_signatures fails if the channel cannot be announced,
                                                                                // so get_channel_update_for_broadcast will never fail by the time we get here.
-                                                                               update_msg: self.get_channel_update_for_broadcast(channel).unwrap(),
+                                                                               update_msg: Some(self.get_channel_update_for_broadcast(channel).unwrap()),
                                                                        });
                                                                }
                                                        }
@@ -6131,6 +6289,7 @@ where
                                                &events::MessageSendEvent::SendChannelAnnouncement { .. } => false,
                                                &events::MessageSendEvent::BroadcastChannelAnnouncement { .. } => true,
                                                &events::MessageSendEvent::BroadcastChannelUpdate { .. } => true,
+                                               &events::MessageSendEvent::BroadcastNodeAnnouncement { .. } => true,
                                                &events::MessageSendEvent::SendChannelUpdate { .. } => false,
                                                &events::MessageSendEvent::HandleError { .. } => false,
                                                &events::MessageSendEvent::SendChannelRangeQuery { .. } => false,
@@ -7301,9 +7460,13 @@ where
                                                                hash_map::Entry::Vacant(entry) => {
                                                                        let path_fee = path.get_path_fees();
                                                                        entry.insert(PendingOutboundPayment::Retryable {
+                                                                               retry_strategy: None,
+                                                                               attempts: PaymentAttempts::new(),
+                                                                               payment_params: None,
                                                                                session_privs: [session_priv_bytes].iter().map(|a| *a).collect(),
                                                                                payment_hash: htlc.payment_hash,
                                                                                payment_secret,
+                                                                               keysend_preimage: None, // only used for retries, and we'll never retry on startup
                                                                                pending_amt_msat: path_amt,
                                                                                pending_fee_msat: Some(path_fee),
                                                                                total_msat: path_amt,
@@ -7803,7 +7966,7 @@ mod tests {
                let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
                let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
                create_announced_chan_between_nodes(&nodes, 0, 1);
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
 
                // To start (1), send a regular payment but don't claim it.
@@ -7812,7 +7975,7 @@ mod tests {
 
                // Next, attempt a keysend payment and make sure it fails.
                let route_params = RouteParameters {
-                       payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id()),
+                       payment_params: PaymentParameters::for_keysend(expected_route.last().unwrap().node.get_our_node_id(), TEST_FINAL_CLTV),
                        final_value_msat: 100_000,
                        final_cltv_expiry_delta: TEST_FINAL_CLTV,
                };
@@ -7905,13 +8068,13 @@ mod tests {
 
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
                let route_params = RouteParameters {
-                       payment_params: PaymentParameters::for_keysend(payee_pubkey),
+                       payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
                        final_value_msat: 10_000,
                        final_cltv_expiry_delta: 40,
                };
                let network_graph = nodes[0].network_graph.clone();
                let first_hops = nodes[0].node.list_usable_channels();
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
                let route = find_route(
                        &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
@@ -7950,13 +8113,13 @@ mod tests {
 
                let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
                let route_params = RouteParameters {
-                       payment_params: PaymentParameters::for_keysend(payee_pubkey),
+                       payment_params: PaymentParameters::for_keysend(payee_pubkey, 40),
                        final_value_msat: 10_000,
                        final_cltv_expiry_delta: 40,
                };
                let network_graph = nodes[0].network_graph.clone();
                let first_hops = nodes[0].node.list_usable_channels();
-               let scorer = test_utils::TestScorer::with_penalty(0);
+               let scorer = test_utils::TestScorer::new();
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
                let route = find_route(
                        &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
@@ -8426,7 +8589,8 @@ pub mod bench {
                let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
                let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
                let logger_a = test_utils::TestLogger::with_id("node a".to_owned());
-               let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(genesis_hash, &logger_a)));
+               let scorer = Mutex::new(test_utils::TestScorer::new());
+               let router = test_utils::TestRouter::new(Arc::new(NetworkGraph::new(genesis_hash, &logger_a)), &scorer);
 
                let mut config: UserConfig = Default::default();
                config.channel_handshake_config.minimum_depth = 1;
@@ -8515,9 +8679,9 @@ pub mod bench {
                macro_rules! send_payment {
                        ($node_a: expr, $node_b: expr) => {
                                let usable_channels = $node_a.list_usable_channels();
-                               let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id())
+                               let payment_params = PaymentParameters::from_node_id($node_b.get_our_node_id(), TEST_FINAL_CLTV)
                                        .with_features($node_b.invoice_features());
-                               let scorer = test_utils::TestScorer::with_penalty(0);
+                               let scorer = test_utils::TestScorer::new();
                                let seed = [3u8; 32];
                                let keys_manager = KeysManager::new(&seed, 42, 42);
                                let random_seed_bytes = keys_manager.get_secure_random_bytes();