Replace maze of BOLT11 payment utilities with parameter generators
authorMatt Corallo <git@bluematt.me>
Fri, 10 Nov 2023 19:23:21 +0000 (19:23 +0000)
committerMatt Corallo <git@bluematt.me>
Fri, 10 Nov 2023 19:23:21 +0000 (19:23 +0000)
`lightning-invoice` was historically responsible for actually
paying invoices, handling retries and everything. However, that
turned out to be buggy and hard to maintain, so the payment logic
was eventually moved into `ChannelManager`. However, the old
utilites remain.

Because our payment logic has a number of tunable parameters and
there are different ways to pay a BOLT11 invoice, we ended up with
six different methods to pay or probe a BOLT11 invoice, with more
requested as various options still were not exposed.

Instead, here, we replace all six methods with two simple ones
which return the arguments which need to be passed to
`ChannelManager`. Those arguments can be further tweaked before
passing them on, allowing more flexibility.

lightning-invoice/src/payment.rs

index 89842591fdec8a89693cbb62457389e9f4a46dc9..e51638f5120c2247a28ec0033bc90429f4755454 100644 (file)
@@ -14,233 +14,76 @@ use crate::prelude::*;
 
 use bitcoin_hashes::Hash;
 
-use lightning::chain;
-use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
-use lightning::sign::{NodeSigner, SignerProvider, EntropySource};
 use lightning::ln::PaymentHash;
-use lightning::ln::channelmanager::{AChannelManager, ChannelManager, PaymentId, Retry, RetryableSendFailure, RecipientOnionFields, ProbeSendFailure};
-use lightning::routing::router::{PaymentParameters, RouteParameters, Router};
-use lightning::util::logger::Logger;
+use lightning::ln::channelmanager::RecipientOnionFields;
+use lightning::routing::router::{PaymentParameters, RouteParameters};
 
-use core::fmt::Debug;
-use core::ops::Deref;
 use core::time::Duration;
 
-/// Pays the given [`Bolt11Invoice`], retrying if needed based on [`Retry`].
+/// Builds the necessary parameters to pay or pre-flight probe the given zero-amount
+/// [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or
+/// [`ChannelManager::send_preflight_probes`].
 ///
-/// [`Bolt11Invoice::payment_hash`] is used as the [`PaymentId`], which ensures idempotency as long
-/// as the payment is still pending. If the payment succeeds, you must ensure that a second payment
-/// with the same [`PaymentHash`] is never sent.
+/// Prior to paying, you must ensure that the [`Bolt11Invoice::payment_hash`] is unique and the
+/// same [`PaymentHash`] has never been paid before.
 ///
-/// If you wish to use a different payment idempotency token, see [`pay_invoice_with_id`].
-pub fn pay_invoice<C: Deref>(
-       invoice: &Bolt11Invoice, retry_strategy: Retry, channelmanager: C
-) -> Result<PaymentId, PaymentError>
-where C::Target: AChannelManager,
-{
-       let payment_id = PaymentId(invoice.payment_hash().into_inner());
-       pay_invoice_with_id(invoice, payment_id, retry_strategy, channelmanager.get_cm())
-               .map(|()| payment_id)
-}
-
-/// Pays the given [`Bolt11Invoice`] with a custom idempotency key, retrying if needed based on
-/// [`Retry`].
-///
-/// Note that idempotency is only guaranteed as long as the payment is still pending. Once the
-/// payment completes or fails, no idempotency guarantees are made.
-///
-/// You should ensure that the [`Bolt11Invoice::payment_hash`] is unique and the same
-/// [`PaymentHash`] has never been paid before.
-///
-/// See [`pay_invoice`] for a variant which uses the [`PaymentHash`] for the idempotency token.
-pub fn pay_invoice_with_id<C: Deref>(
-       invoice: &Bolt11Invoice, payment_id: PaymentId, retry_strategy: Retry, channelmanager: C
-) -> Result<(), PaymentError>
-where C::Target: AChannelManager,
-{
-       let amt_msat = invoice.amount_milli_satoshis().ok_or(PaymentError::Invoice("amount missing"))?;
-       pay_invoice_using_amount(invoice, amt_msat, payment_id, retry_strategy, channelmanager.get_cm())
-}
-
-/// Pays the given zero-value [`Bolt11Invoice`] using the given amount, retrying if needed based on
-/// [`Retry`].
-///
-/// [`Bolt11Invoice::payment_hash`] is used as the [`PaymentId`], which ensures idempotency as long
-/// as the payment is still pending. If the payment succeeds, you must ensure that a second payment
-/// with the same [`PaymentHash`] is never sent.
+/// Will always succeed unless the invoice has an amount specified, in which case
+/// [`payment_parameters_from_invoice`] should be used.
 ///
-/// If you wish to use a different payment idempotency token, see
-/// [`pay_zero_value_invoice_with_id`].
-pub fn pay_zero_value_invoice<C: Deref>(
-       invoice: &Bolt11Invoice, amount_msats: u64, retry_strategy: Retry, channelmanager: C
-) -> Result<PaymentId, PaymentError>
-where C::Target: AChannelManager,
-{
-       let payment_id = PaymentId(invoice.payment_hash().into_inner());
-       pay_zero_value_invoice_with_id(invoice, amount_msats, payment_id, retry_strategy,
-               channelmanager)
-               .map(|()| payment_id)
+/// [`ChannelManager::send_payment`]: lightning::ln::channelmanager::ChannelManager::send_payment
+/// [`ChannelManager::send_preflight_probes`]: lightning::ln::channelmanager::ChannelManager::send_preflight_probes
+pub fn payment_parameters_from_zero_amount_invoice(invoice: &Bolt11Invoice, amount_msat: u64)
+-> Result<(PaymentHash, RecipientOnionFields, RouteParameters), ()> {
+       if invoice.amount_milli_satoshis().is_some() {
+               Err(())
+       } else {
+               Ok(params_from_invoice(invoice, amount_msat))
+       }
 }
 
-/// Pays the given zero-value [`Bolt11Invoice`] using the given amount and custom idempotency key,
-/// retrying if needed based on [`Retry`].
+/// Builds the necessary parameters to pay or pre-flight probe the given [`Bolt11Invoice`] using
+/// [`ChannelManager::send_payment`] or [`ChannelManager::send_preflight_probes`].
 ///
-/// Note that idempotency is only guaranteed as long as the payment is still pending. Once the
-/// payment completes or fails, no idempotency guarantees are made.
+/// Prior to paying, you must ensure that the [`Bolt11Invoice::payment_hash`] is unique and the
+/// same [`PaymentHash`] has never been paid before.
 ///
-/// You should ensure that the [`Bolt11Invoice::payment_hash`] is unique and the same
-/// [`PaymentHash`] has never been paid before.
+/// Will always succeed unless the invoice has no amount specified, in which case
+/// [`payment_parameters_from_zero_amount_invoice`] should be used.
 ///
-/// See [`pay_zero_value_invoice`] for a variant which uses the [`PaymentHash`] for the
-/// idempotency token.
-pub fn pay_zero_value_invoice_with_id<C: Deref>(
-       invoice: &Bolt11Invoice, amount_msats: u64, payment_id: PaymentId, retry_strategy: Retry,
-       channelmanager: C
-) -> Result<(), PaymentError>
-where C::Target: AChannelManager,
-{
-       if invoice.amount_milli_satoshis().is_some() {
-               Err(PaymentError::Invoice("amount unexpected"))
+/// [`ChannelManager::send_payment`]: lightning::ln::channelmanager::ChannelManager::send_payment
+/// [`ChannelManager::send_preflight_probes`]: lightning::ln::channelmanager::ChannelManager::send_preflight_probes
+pub fn payment_parameters_from_invoice(invoice: &Bolt11Invoice)
+-> Result<(PaymentHash, RecipientOnionFields, RouteParameters), ()> {
+       if let Some(amount_msat) = invoice.amount_milli_satoshis() {
+               Ok(params_from_invoice(invoice, amount_msat))
        } else {
-               pay_invoice_using_amount(invoice, amount_msats, payment_id, retry_strategy,
-                       channelmanager.get_cm())
+               Err(())
        }
 }
 
-fn pay_invoice_using_amount<P: Deref>(
-       invoice: &Bolt11Invoice, amount_msats: u64, payment_id: PaymentId, retry_strategy: Retry,
-       payer: P
-) -> Result<(), PaymentError> where P::Target: Payer {
+fn expiry_time_from_unix_epoch(invoice: &Bolt11Invoice) -> Duration {
+       invoice.signed_invoice.raw_invoice.data.timestamp.0 + invoice.expiry_time()
+}
+
+fn params_from_invoice(invoice: &Bolt11Invoice, amount_msat: u64)
+-> (PaymentHash, RecipientOnionFields, RouteParameters) {
        let payment_hash = PaymentHash((*invoice.payment_hash()).into_inner());
+
        let mut recipient_onion = RecipientOnionFields::secret_only(*invoice.payment_secret());
        recipient_onion.payment_metadata = invoice.payment_metadata().map(|v| v.clone());
-       let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
-               invoice.min_final_cltv_expiry_delta() as u32)
-               .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs())
-               .with_route_hints(invoice.route_hints()).unwrap();
-       if let Some(features) = invoice.features() {
-               payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
-       }
-       let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msats);
-
-       payer.send_payment(payment_hash, recipient_onion, payment_id, route_params, retry_strategy)
-}
-
-/// Sends payment probes over all paths of a route that would be used to pay the given invoice.
-///
-/// See [`ChannelManager::send_preflight_probes`] for more information.
-pub fn preflight_probe_invoice<C: Deref>(
-       invoice: &Bolt11Invoice, channelmanager: C, liquidity_limit_multiplier: Option<u64>,
-) -> Result<Vec<(PaymentHash, PaymentId)>, ProbingError>
-where C::Target: AChannelManager,
-{
-       let amount_msat = if let Some(invoice_amount_msat) = invoice.amount_milli_satoshis() {
-               invoice_amount_msat
-       } else {
-               return Err(ProbingError::Invoice("Failed to send probe as no amount was given in the invoice."));
-       };
 
        let mut payment_params = PaymentParameters::from_node_id(
-               invoice.recover_payee_pub_key(),
-               invoice.min_final_cltv_expiry_delta() as u32,
-       )
-       .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs())
-       .with_route_hints(invoice.route_hints())
-       .unwrap();
-
+                       invoice.recover_payee_pub_key(),
+                       invoice.min_final_cltv_expiry_delta() as u32
+               )
+               .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs())
+               .with_route_hints(invoice.route_hints()).unwrap();
        if let Some(features) = invoice.features() {
                payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
        }
-       let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
-
-       channelmanager.get_cm().send_preflight_probes(route_params, liquidity_limit_multiplier)
-               .map_err(ProbingError::Sending)
-}
-
-/// Sends payment probes over all paths of a route that would be used to pay the given zero-value
-/// invoice using the given amount.
-///
-/// See [`ChannelManager::send_preflight_probes`] for more information.
-pub fn preflight_probe_zero_value_invoice<C: Deref>(
-       invoice: &Bolt11Invoice, amount_msat: u64, channelmanager: C,
-       liquidity_limit_multiplier: Option<u64>,
-) -> Result<Vec<(PaymentHash, PaymentId)>, ProbingError>
-where C::Target: AChannelManager,
-{
-       if invoice.amount_milli_satoshis().is_some() {
-               return Err(ProbingError::Invoice("amount unexpected"));
-       }
-
-       let mut payment_params = PaymentParameters::from_node_id(
-               invoice.recover_payee_pub_key(),
-               invoice.min_final_cltv_expiry_delta() as u32,
-       )
-       .with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs())
-       .with_route_hints(invoice.route_hints())
-       .unwrap();
 
-       if let Some(features) = invoice.features() {
-               payment_params = payment_params.with_bolt11_features(features.clone()).unwrap();
-       }
        let route_params = RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
-
-       channelmanager.get_cm().send_preflight_probes(route_params, liquidity_limit_multiplier)
-               .map_err(ProbingError::Sending)
-}
-
-fn expiry_time_from_unix_epoch(invoice: &Bolt11Invoice) -> Duration {
-       invoice.signed_invoice.raw_invoice.data.timestamp.0 + invoice.expiry_time()
-}
-
-/// An error that may occur when making a payment.
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub enum PaymentError {
-       /// An error resulting from the provided [`Bolt11Invoice`] or payment hash.
-       Invoice(&'static str),
-       /// An error occurring when sending a payment.
-       Sending(RetryableSendFailure),
-}
-
-/// An error that may occur when sending a payment probe.
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub enum ProbingError {
-       /// An error resulting from the provided [`Bolt11Invoice`].
-       Invoice(&'static str),
-       /// An error occurring when sending a payment probe.
-       Sending(ProbeSendFailure),
-}
-
-/// A trait defining behavior of a [`Bolt11Invoice`] payer.
-///
-/// Useful for unit testing internal methods.
-trait Payer {
-       /// Sends a payment over the Lightning Network using the given [`Route`].
-       ///
-       /// [`Route`]: lightning::routing::router::Route
-       fn send_payment(
-               &self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
-               payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry
-       ) -> Result<(), PaymentError>;
-}
-
-impl<M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Deref, R: Deref, L: Deref> Payer for ChannelManager<M, T, ES, NS, SP, F, R, L>
-where
-               M::Target: chain::Watch<<SP::Target as SignerProvider>::Signer>,
-               T::Target: BroadcasterInterface,
-               ES::Target: EntropySource,
-               NS::Target: NodeSigner,
-               SP::Target: SignerProvider,
-               F::Target: FeeEstimator,
-               R::Target: Router,
-               L::Target: Logger,
-{
-       fn send_payment(
-               &self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
-               payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry
-       ) -> Result<(), PaymentError> {
-               self.send_payment(payment_hash, recipient_onion, payment_id, route_params, retry_strategy)
-                       .map_err(PaymentError::Sending)
-       }
+       (payment_hash, recipient_onion, route_params)
 }
 
 #[cfg(test)]
@@ -249,64 +92,14 @@ mod tests {
        use crate::{InvoiceBuilder, Currency};
        use bitcoin_hashes::sha256::Hash as Sha256;
        use lightning::events::Event;
+       use lightning::ln::channelmanager::{Retry, PaymentId};
        use lightning::ln::msgs::ChannelMessageHandler;
-       use lightning::ln::{PaymentPreimage, PaymentSecret};
+       use lightning::ln::PaymentSecret;
        use lightning::ln::functional_test_utils::*;
-       use secp256k1::{SecretKey, Secp256k1};
-       use std::collections::VecDeque;
+       use lightning::routing::router::Payee;
+       use secp256k1::{SecretKey, PublicKey, Secp256k1};
        use std::time::{SystemTime, Duration};
 
-       struct TestPayer {
-               expectations: core::cell::RefCell<VecDeque<Amount>>,
-       }
-
-       impl TestPayer {
-               fn new() -> Self {
-                       Self {
-                               expectations: core::cell::RefCell::new(VecDeque::new()),
-                       }
-               }
-
-               fn expect_send(self, value_msat: Amount) -> Self {
-                       self.expectations.borrow_mut().push_back(value_msat);
-                       self
-               }
-
-               fn check_value_msats(&self, actual_value_msats: Amount) {
-                       let expected_value_msats = self.expectations.borrow_mut().pop_front();
-                       if let Some(expected_value_msats) = expected_value_msats {
-                               assert_eq!(actual_value_msats, expected_value_msats);
-                       } else {
-                               panic!("Unexpected amount: {:?}", actual_value_msats);
-                       }
-               }
-       }
-
-       #[derive(Clone, Debug, PartialEq, Eq)]
-       struct Amount(u64); // msat
-
-       impl Payer for TestPayer {
-               fn send_payment(
-                       &self, _payment_hash: PaymentHash, _recipient_onion: RecipientOnionFields,
-                       _payment_id: PaymentId, route_params: RouteParameters, _retry_strategy: Retry
-               ) -> Result<(), PaymentError> {
-                       self.check_value_msats(Amount(route_params.final_value_msat));
-                       Ok(())
-               }
-       }
-
-       impl Drop for TestPayer {
-               fn drop(&mut self) {
-                       if std::thread::panicking() {
-                               return;
-                       }
-
-                       if !self.expectations.borrow().is_empty() {
-                               panic!("Unsatisfied payment expectations: {:?}", self.expectations.borrow());
-                       }
-               }
-       }
-
        fn duration_since_epoch() -> Duration {
                #[cfg(feature = "std")]
                let duration_since_epoch =
@@ -316,11 +109,14 @@ mod tests {
                duration_since_epoch
        }
 
-       fn invoice(payment_preimage: PaymentPreimage) -> Bolt11Invoice {
-               let payment_hash = Sha256::hash(&payment_preimage.0);
+       #[test]
+       fn invoice_test() {
+               let payment_hash = Sha256::hash(&[0; 32]);
                let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+               let secp_ctx = Secp256k1::new();
+               let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
 
-               InvoiceBuilder::new(Currency::Bitcoin)
+               let invoice = InvoiceBuilder::new(Currency::Bitcoin)
                        .description("test".into())
                        .payment_hash(payment_hash)
                        .payment_secret(PaymentSecret([0; 32]))
@@ -328,63 +124,53 @@ mod tests {
                        .min_final_cltv_expiry_delta(144)
                        .amount_milli_satoshis(128)
                        .build_signed(|hash| {
-                               Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key)
+                               secp_ctx.sign_ecdsa_recoverable(hash, &private_key)
                        })
-                       .unwrap()
+                       .unwrap();
+
+               assert!(payment_parameters_from_zero_amount_invoice(&invoice, 42).is_err());
+
+               let (hash, onion, params) = payment_parameters_from_invoice(&invoice).unwrap();
+               assert_eq!(&hash.0[..], &payment_hash[..]);
+               assert_eq!(onion.payment_secret, Some(PaymentSecret([0; 32])));
+               assert_eq!(params.final_value_msat, 128);
+               match params.payment_params.payee {
+                       Payee::Clear { node_id, .. } => {
+                               assert_eq!(node_id, public_key);
+                       },
+                       _ => panic!(),
+               }
        }
 
-       fn zero_value_invoice(payment_preimage: PaymentPreimage) -> Bolt11Invoice {
-               let payment_hash = Sha256::hash(&payment_preimage.0);
+       #[test]
+       fn zero_value_invoice_test() {
+               let payment_hash = Sha256::hash(&[0; 32]);
                let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
+               let secp_ctx = Secp256k1::new();
+               let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
 
-               InvoiceBuilder::new(Currency::Bitcoin)
+               let invoice = InvoiceBuilder::new(Currency::Bitcoin)
                        .description("test".into())
                        .payment_hash(payment_hash)
                        .payment_secret(PaymentSecret([0; 32]))
                        .duration_since_epoch(duration_since_epoch())
                        .min_final_cltv_expiry_delta(144)
                        .build_signed(|hash| {
-                               Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key)
+                               secp_ctx.sign_ecdsa_recoverable(hash, &private_key)
                        })
-               .unwrap()
-       }
+               .unwrap();
 
-       #[test]
-       fn pays_invoice() {
-               let payment_id = PaymentId([42; 32]);
-               let payment_preimage = PaymentPreimage([1; 32]);
-               let invoice = invoice(payment_preimage);
-               let final_value_msat = invoice.amount_milli_satoshis().unwrap();
-
-               let payer = TestPayer::new().expect_send(Amount(final_value_msat));
-               pay_invoice_using_amount(&invoice, final_value_msat, payment_id, Retry::Attempts(0), &payer).unwrap();
-       }
+               assert!(payment_parameters_from_invoice(&invoice).is_err());
 
-       #[test]
-       fn pays_zero_value_invoice() {
-               let payment_id = PaymentId([42; 32]);
-               let payment_preimage = PaymentPreimage([1; 32]);
-               let invoice = zero_value_invoice(payment_preimage);
-               let amt_msat = 10_000;
-
-               let payer = TestPayer::new().expect_send(Amount(amt_msat));
-               pay_invoice_using_amount(&invoice, amt_msat, payment_id, Retry::Attempts(0), &payer).unwrap();
-       }
-
-       #[test]
-       fn fails_paying_zero_value_invoice_with_amount() {
-               let chanmon_cfgs = create_chanmon_cfgs(1);
-               let node_cfgs = create_node_cfgs(1, &chanmon_cfgs);
-               let node_chanmgrs = create_node_chanmgrs(1, &node_cfgs, &[None]);
-               let nodes = create_network(1, &node_cfgs, &node_chanmgrs);
-
-               let payment_preimage = PaymentPreimage([1; 32]);
-               let invoice = invoice(payment_preimage);
-               let amt_msat = 10_000;
-
-               match pay_zero_value_invoice(&invoice, amt_msat, Retry::Attempts(0), nodes[0].node) {
-                       Err(PaymentError::Invoice("amount unexpected")) => {},
-                       _ => panic!()
+               let (hash, onion, params) = payment_parameters_from_zero_amount_invoice(&invoice, 42).unwrap();
+               assert_eq!(&hash.0[..], &payment_hash[..]);
+               assert_eq!(onion.payment_secret, Some(PaymentSecret([0; 32])));
+               assert_eq!(params.final_value_msat, 42);
+               match params.payment_params.payee {
+                       Payee::Clear { node_id, .. } => {
+                               assert_eq!(node_id, public_key);
+                       },
+                       _ => panic!(),
                }
        }
 
@@ -418,7 +204,8 @@ mod tests {
                        })
                        .unwrap();
 
-               pay_invoice(&invoice, Retry::Attempts(0), nodes[0].node).unwrap();
+               let (hash, onion, params) = payment_parameters_from_invoice(&invoice).unwrap();
+               nodes[0].node.send_payment(hash, onion, PaymentId(hash.0), params, Retry::Attempts(0)).unwrap();
                check_added_monitors(&nodes[0], 1);
                let send_event = SendEvent::from_node(&nodes[0]);
                nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send_event.msgs[0]);