Merge pull request #450 from getlipa/feature/persist-payment-failure-…

…reason

Persist and expose payment fail reason

eel/src/data_store.rs

@@ -1,5 +1,5 @@
 use crate::config::TzConfig;
-use crate::errors::{Error, Result};
+use crate::errors::{Error, PayErrorCode, Result};
 use crate::interfaces::ExchangeRate;
 use crate::migrations::get_migrations;
 use crate::payment::{Payment, PaymentState, PaymentType, TzTime};
@@ -156,6 +156,27 @@ impl DataStore {
         self.fill_network_fees(hash, network_fees_msat)
     }
 
+    pub fn outgoing_payment_failed(&self, hash: &str, fail_reason: PayErrorCode) -> Result<()> {
+        self.db_conn
+            .execute(
+                "\
+                INSERT INTO events (payment_id, type, timezone_id, timezone_utc_offset_secs, fail_reason) \
+                VALUES (
+                    (SELECT payment_id FROM payments WHERE hash=?1), ?2, ?3, ?4, ?5)
+                ",
+                (
+                    hash,
+                    PaymentState::Failed as u8,
+                    &self.timezone_config.timezone_id,
+                    self.timezone_config.timezone_utc_offset_secs,
+                    fail_reason as u8,
+                ),
+            )
+            .map_to_invalid_input("Failed to add payment failed event to db")?;
+
+        Ok(())
+    }
+
     pub fn new_payment_state(&self, hash: &str, state: PaymentState) -> Result<()> {
         self.db_conn
             .execute(
@@ -171,7 +192,7 @@ impl DataStore {
                     self.timezone_config.timezone_utc_offset_secs,
                 ),
             )
-            .map_to_invalid_input("Failed to add payment retrying event to payments db")?;
+            .map_to_invalid_input("Failed to add payment event to db")?;
 
         Ok(())
     }
@@ -216,7 +237,7 @@ impl DataStore {
             lsp_fees_msat, invoice, metadata, recent_events.type as state, recent_events.inserted_at, \
             recent_events.timezone_id, recent_events.timezone_utc_offset_secs, description, \
             creation_events.inserted_at, creation_events.timezone_id, creation_events.timezone_utc_offset_secs, \
-            h.fiat_currency, h.rate, h.updated_at \
+            h.fiat_currency, h.rate, h.updated_at, recent_events.fail_reason \
             FROM payments \
             JOIN recent_events ON payments.payment_id=recent_events.payment_id \
             JOIN creation_events ON payments.payment_id=creation_events.payment_id \
@@ -248,7 +269,7 @@ impl DataStore {
             lsp_fees_msat, invoice, metadata, recent_events.type as state, recent_events.inserted_at, \
             recent_events.timezone_id, recent_events.timezone_utc_offset_secs, description, \
             creation_events.inserted_at, creation_events.timezone_id, creation_events.timezone_utc_offset_secs, \
-            h.fiat_currency, h.rate, h.updated_at \
+            h.fiat_currency, h.rate, h.updated_at, recent_events.fail_reason \
             FROM payments \
             JOIN recent_events ON payments.payment_id=recent_events.payment_id \
             JOIN creation_events ON payments.payment_id=creation_events.payment_id \
@@ -278,7 +299,7 @@ impl DataStore {
             lsp_fees_msat, invoice, metadata, recent_events.type as state, recent_events.inserted_at, \
             recent_events.timezone_id, recent_events.timezone_utc_offset_secs, description, \
             creation_events.inserted_at, creation_events.timezone_id, creation_events.timezone_utc_offset_secs, \
-            h.fiat_currency, h.rate, h.updated_at \
+            h.fiat_currency, h.rate, h.updated_at, recent_events.fail_reason \
             FROM payments \
             JOIN recent_events ON payments.payment_id=recent_events.payment_id \
             JOIN creation_events ON payments.payment_id=creation_events.payment_id \
@@ -475,9 +496,17 @@ fn payment_from_row(row: &Row) -> rusqlite::Result<Payment> {
     } else {
         None
     };
+    let fail_reason: Option<u8> = row.get(20)?;
+    let fail_reason = match fail_reason {
+        None => None,
+        Some(r) => Some(PayErrorCode::try_from(r).map_err(|e| {
+            rusqlite::Error::FromSqlConversionFailure(1, Type::Integer, Box::new(e))
+        })?),
+    };
     Ok(Payment {
         payment_type,
         payment_state,
+        fail_reason,
         hash,
         amount_msat,
         invoice,
@@ -526,6 +555,7 @@ mod tests {
     use crate::interfaces::ExchangeRate;
     use crate::payment::{FiatValues, PaymentState, PaymentType};
 
+    use crate::errors::PayErrorCode;
     use lightning::chain::keysinterface::SpendableOutputDescriptor;
     use lightning::ln::PaymentSecret;
     use lightning::util::ser::Readable;
@@ -711,12 +741,13 @@ mod tests {
         sleep(Duration::from_secs(1));
 
         data_store
-            .new_payment_state(hash, PaymentState::Failed)
+            .outgoing_payment_failed(hash, PayErrorCode::NoMoreRoutes)
             .unwrap();
         let payments = data_store.get_latest_payments(100).unwrap();
         assert_eq!(payments.len(), 2);
         let payment = payments.get(0).unwrap();
         assert_eq!(payment.payment_state, PaymentState::Failed);
+        assert_eq!(payment.fail_reason, Some(PayErrorCode::NoMoreRoutes));
         assert_eq!(payment.created_at.timezone_id, TEST_TZ_ID);
         assert_eq!(payment.created_at.timezone_utc_offset_secs, TEST_TZ_OFFSET);
         assert_eq!(payment.latest_state_change_at.timezone_id, TEST_TZ_ID);

eel/src/errors.rs

@@ -1,3 +1,5 @@
+use lightning::events::PaymentFailureReason;
+use num_enum::TryFromPrimitive;
 use std::fmt::{Display, Formatter};
 
 #[derive(Debug, PartialEq, Eq)]
@@ -18,7 +20,8 @@ impl Display for RuntimeErrorCode {
 pub type Error = perro::Error<RuntimeErrorCode>;
 pub type Result<T> = std::result::Result<T, Error>;
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(PartialEq, Eq, Debug, TryFromPrimitive, Clone)]
+#[repr(u8)]
 pub enum PayErrorCode {
     InvoiceExpired,
     AlreadyUsedInvoice,
@@ -27,6 +30,20 @@ pub enum PayErrorCode {
     RecipientRejected,
     RetriesExhausted,
     NoMoreRoutes,
+    UnexpectedError,
+}
+
+impl PayErrorCode {
+    pub(crate) fn from_failure_reason(reason: PaymentFailureReason) -> Self {
+        match reason {
+            PaymentFailureReason::RecipientRejected => Self::RecipientRejected,
+            PaymentFailureReason::UserAbandoned => Self::UnexpectedError,
+            PaymentFailureReason::RetriesExhausted => Self::RetriesExhausted,
+            PaymentFailureReason::PaymentExpired => Self::InvoiceExpired,
+            PaymentFailureReason::RouteNotFound => Self::NoMoreRoutes,
+            PaymentFailureReason::UnexpectedError => Self::UnexpectedError,
+        }
+    }
 }
 
 impl Display for PayErrorCode {

eel/src/event_handler.rs

@@ -1,5 +1,5 @@
 use crate::data_store::DataStore;
-use crate::errors::Result;
+use crate::errors::{PayErrorCode, Result};
 use crate::interfaces;
 use crate::payment::PaymentState;
 use crate::task_manager::TaskManager;
@@ -155,7 +155,14 @@ impl EventHandler for LipaEventHandler {
                     .data_store
                     .lock()
                     .unwrap()
-                    .new_payment_state(&payment_hash, PaymentState::Failed)
+                    .outgoing_payment_failed(
+                        &payment_hash,
+                        PayErrorCode::from_failure_reason(
+                            reason.unwrap_or(
+                                lightning::events::PaymentFailureReason::UnexpectedError,
+                            ),
+                        ), // `reason` could only be None if we deserialize events from old LDK versions
+                    )
                     .is_err()
                 {
                     error!("Failed to persist in the payment db that sending payment with hash {payment_hash} has failed");

eel/src/lib.rs

@@ -506,27 +506,38 @@ impl LightningNode {
         error: PaymentError,
         payment_hash: &str,
     ) -> PayResult<()> {
-        self.data_store
-            .lock()
-            .unwrap()
-            .new_payment_state(payment_hash, PaymentState::Failed)
-            .map_to_permanent_failure("Failed to persist payment result")?;
-        match error {
-            PaymentError::Invoice(e) => Err(invalid_input(format!("Invalid invoice - {e}"))),
+        let (error, error_code) = match error {
+            PaymentError::Invoice(e) => (
+                Err(invalid_input(format!("Invalid invoice - {e}"))),
+                PayErrorCode::UnexpectedError,
+            ),
             PaymentError::Sending(e) => match e {
-                RetryableSendFailure::PaymentExpired => Err(runtime_error(
+                RetryableSendFailure::PaymentExpired => (
+                    Err(runtime_error(
+                        PayErrorCode::InvoiceExpired,
+                        "Invoice has expired",
+                    )),
                     PayErrorCode::InvoiceExpired,
-                    "Invoice has expired",
-                )),
-                RetryableSendFailure::RouteNotFound => Err(runtime_error(
+                ),
+                RetryableSendFailure::RouteNotFound => (
+                    Err(runtime_error(
+                        PayErrorCode::NoRouteFound,
+                        "Failed to find any route",
+                    )),
                     PayErrorCode::NoRouteFound,
-                    "Failed to find any route",
-                )),
-                RetryableSendFailure::DuplicatePayment => {
-                    Err(permanent_failure("Duplicate payment"))
-                }
+                ),
+                RetryableSendFailure::DuplicatePayment => (
+                    Err(permanent_failure("Duplicate payment")),
+                    PayErrorCode::UnexpectedError,
+                ),
             },
-        }
+        };
+        self.data_store
+            .lock()
+            .unwrap()
+            .outgoing_payment_failed(payment_hash, error_code)
+            .map_to_permanent_failure("Failed to persist payment result")?;
+        error
     }
 
     fn validate_persist_new_outgoing_payment_attempt(

eel/src/migrations.rs

@@ -10,6 +10,7 @@ pub(crate) fn get_migrations() -> Vec<Migration> {
         migration_02_store_exchange_rates,
         migration_03_store_spendable_outputs,
         migration_04_store_exchange_rates_history,
+        migration_05_store_payment_failure_reason,
     ]
 }
 
@@ -116,3 +117,13 @@ fn migration_04_store_exchange_rates_history(connection: &Connection) -> Result<
         )
         .map_to_permanent_failure("Failed to set up local database")
 }
+
+fn migration_05_store_payment_failure_reason(connection: &Connection) -> Result<()> {
+    connection
+        .execute_batch(
+            "\
+            ALTER TABLE events ADD COLUMN fail_reason INTEGER NULL;
+        ",
+        )
+        .map_to_permanent_failure("Failed to set up local database")
+}

eel/src/payment.rs

@@ -1,6 +1,7 @@
 use crate::interfaces::ExchangeRate;
 use lightning_invoice::Invoice;
 
+use crate::errors::PayErrorCode;
 use num_enum::TryFromPrimitive;
 use std::time::SystemTime;
 
@@ -56,6 +57,7 @@ impl FiatValues {
 pub struct Payment {
     pub payment_type: PaymentType,
     pub payment_state: PaymentState,
+    pub fail_reason: Option<PayErrorCode>,
     pub hash: String,
     pub amount_msat: u64,
     pub invoice: Invoice,

eel/tests/data_store_test.rs

@@ -122,6 +122,7 @@ mod data_store_test {
         let payment_dummy = Payment {
             payment_type: PaymentType::Receiving,
             payment_state: PaymentState::Succeeded,
+            fail_reason: None,
             hash: "<unknown>".to_string(),
             amount_msat: TWENTY_K_SATS,
             invoice: invoice.clone(),
@@ -163,6 +164,7 @@ mod data_store_test {
         let mut payment_dummy = Payment {
             payment_type: PaymentType::Sending,
             payment_state: PaymentState::Created,
+            fail_reason: None,
             hash: "<unknown>".to_string(),
             amount_msat: TWO_K_SATS,
             invoice: Invoice::from_str(&invoice).unwrap(),

examples/3l-node/cli.rs

@@ -6,6 +6,7 @@ use bitcoin::secp256k1::PublicKey;
 use chrono::offset::FixedOffset;
 use chrono::{DateTime, Utc};
 use colored::Colorize;
+use eel::payment::PaymentState;
 use rustyline::config::{Builder, CompletionType};
 use rustyline::error::ReadlineError;
 use rustyline::history::DefaultHistory;
@@ -500,6 +501,9 @@ fn list_payments(node: &LightningNode) -> Result<(), String> {
             payment.latest_state_change_at.timezone_id
         );
         println!("      State:        {:?}", payment.payment_state);
+        if payment.payment_state == PaymentState::Failed {
+            println!("      Fail reason:  {:?}", payment.fail_reason);
+        }
         println!("      Amount:       {}", amount_to_string(payment.amount));
         println!(
             "      Network fees: {:?}",

src/lib.rs

@@ -84,6 +84,7 @@ pub struct ChannelsInfo {
 pub struct Payment {
     pub payment_type: PaymentType,
     pub payment_state: PaymentState,
+    pub fail_reason: Option<PayErrorCode>,
     pub hash: String,
     pub amount: Amount,
     pub invoice_details: InvoiceDetails,
@@ -338,6 +339,7 @@ fn to_payment(payment: eel::payment::Payment) -> Payment {
     Payment {
         payment_type: payment.payment_type,
         payment_state: payment.payment_state,
+        fail_reason: payment.fail_reason,
         hash: payment.hash,
         amount,
         invoice_details,

src/lipalightninglib.udl

@@ -292,6 +292,7 @@ interface MaxRoutingFeeMode {
 dictionary Payment {
     PaymentType payment_type;
     PaymentState payment_state;
+    PayErrorCode? fail_reason;
     string hash; // Hex representation of payment hash
     Amount amount; // Nominal amount specified in the invoice
     InvoiceDetails invoice_details;
@@ -479,4 +480,6 @@ enum PayErrorCode {
     "RecipientRejected", // The recipient has rejected the payment.
     "RetriesExhausted", // Retry attempts or timeout was reached.
     "NoMoreRoutes", // All possible routes failed.
+
+    "UnexpectedError", // An unexpected error occurred. This likely is a result of a bug within 3L/LDK and should be reported to lipa.
 };