From 2d69ba469d7347c687f439864b9f6225891a2251 Mon Sep 17 00:00:00 2001 From: Jesse de Wit Date: Thu, 5 Dec 2024 14:06:52 +0100 Subject: [PATCH] ensure pseudo payments are deleted on sync Before sending out a payment, the sdk inserts a pseudo payment. This ensures there is always a pending payment shown on clients directly when paying. If the call to send_payment never resulted in in actual payment being sent, these payments would sit in the database forever and never be deleted. Pseudo payments are now marked as pseudo in the database. On sync pseudo payments are deleted before inserting the updated payments from the node. Existing payments and sync state are dropped in case the client's database already contained pending payments because of this issue. --- libs/sdk-core/src/breez_services.rs | 74 ++++++++++++----------- libs/sdk-core/src/persist/migrations.rs | 4 ++ libs/sdk-core/src/persist/transactions.rs | 29 +++++++-- libs/sdk-core/src/swap_in/swap.rs | 4 +- 4 files changed, 69 insertions(+), 42 deletions(-) diff --git a/libs/sdk-core/src/breez_services.rs b/libs/sdk-core/src/breez_services.rs index a5d9ccf5a..85b57c040 100644 --- a/libs/sdk-core/src/breez_services.rs +++ b/libs/sdk-core/src/breez_services.rs @@ -1196,7 +1196,8 @@ impl BreezServices { // update both closed channels and lightning transaction payments let mut payments = closed_channel_payments; payments.extend(new_data.payments.clone()); - self.persister.insert_or_update_payments(&payments)?; + self.persister.delete_pseudo_payments()?; + self.persister.insert_or_update_payments(&payments, false)?; let duration = start.elapsed(); info!("Sync duration: {:?}", duration); @@ -1253,37 +1254,40 @@ impl BreezServices { amount_msat: u64, label: Option, ) -> Result<(), SendPaymentError> { - self.persister.insert_or_update_payments(&[Payment { - id: invoice.payment_hash.clone(), - payment_type: PaymentType::Sent, - payment_time: SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs() as i64, - amount_msat, - fee_msat: 0, - status: PaymentStatus::Pending, - error: None, - description: invoice.description.clone(), - details: PaymentDetails::Ln { - data: LnPaymentDetails { - payment_hash: invoice.payment_hash.clone(), - label: label.unwrap_or_default(), - destination_pubkey: invoice.payee_pubkey.clone(), - payment_preimage: String::new(), - keysend: false, - bolt11: invoice.bolt11.clone(), - lnurl_success_action: None, - lnurl_pay_domain: None, - lnurl_pay_comment: None, - ln_address: None, - lnurl_metadata: None, - lnurl_withdraw_endpoint: None, - swap_info: None, - reverse_swap_info: None, - pending_expiration_block: None, - open_channel_bolt11: None, + self.persister.insert_or_update_payments( + &[Payment { + id: invoice.payment_hash.clone(), + payment_type: PaymentType::Sent, + payment_time: SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs() as i64, + amount_msat, + fee_msat: 0, + status: PaymentStatus::Pending, + error: None, + description: invoice.description.clone(), + details: PaymentDetails::Ln { + data: LnPaymentDetails { + payment_hash: invoice.payment_hash.clone(), + label: label.unwrap_or_default(), + destination_pubkey: invoice.payee_pubkey.clone(), + payment_preimage: String::new(), + keysend: false, + bolt11: invoice.bolt11.clone(), + lnurl_success_action: None, + lnurl_pay_domain: None, + lnurl_pay_comment: None, + ln_address: None, + lnurl_metadata: None, + lnurl_withdraw_endpoint: None, + swap_info: None, + reverse_swap_info: None, + pending_expiration_block: None, + open_channel_bolt11: None, + }, }, - }, - metadata: None, - }])?; + metadata: None, + }], + true, + )?; self.persister.insert_payment_external_info( &invoice.payment_hash, @@ -1644,7 +1648,9 @@ impl BreezServices { let mut payment: Option = p.clone().try_into().ok(); if let Some(ref p) = payment { - let res = cloned.persister.insert_or_update_payments(&vec![p.clone()]); + let res = cloned + .persister + .insert_or_update_payments(&vec![p.clone()], false); debug!("paid invoice was added to payments list {res:?}"); if let Ok(Some(mut node_info)) = cloned.persister.get_node_state() { node_info.channels_balance_msat += p.amount_msat; @@ -3113,7 +3119,7 @@ pub(crate) mod tests { let test_config = create_test_config(); let persister = Arc::new(create_test_persister(test_config.clone())); persister.init()?; - persister.insert_or_update_payments(&dummy_transactions)?; + persister.insert_or_update_payments(&dummy_transactions, false)?; persister.insert_payment_external_info( payment_hash_with_lnurl_success_action, PaymentExternalInfo { @@ -3334,7 +3340,7 @@ pub(crate) mod tests { let test_config = create_test_config(); let persister = Arc::new(create_test_persister(test_config.clone())); persister.init()?; - persister.insert_or_update_payments(&known_payments)?; + persister.insert_or_update_payments(&known_payments, false)?; persister.set_lsp(MockBreezServer {}.lsp_id(), None)?; let mut builder = BreezServicesBuilder::new(test_config.clone()); diff --git a/libs/sdk-core/src/persist/migrations.rs b/libs/sdk-core/src/persist/migrations.rs index f24235448..59ace04d0 100644 --- a/libs/sdk-core/src/persist/migrations.rs +++ b/libs/sdk-core/src/persist/migrations.rs @@ -482,6 +482,10 @@ pub(crate) fn current_migrations() -> Vec<&'static str> { DELETE FROM cached_items WHERE key = 'sync_state'; ", + "ALTER TABLE payments ADD COLUMN is_pseudo INTEGER DEFAULT 0 NOT NULL; + DELETE FROM payments; + DELETE FROM cached_items WHERE key = 'sync_state'; + " ] } diff --git a/libs/sdk-core/src/persist/transactions.rs b/libs/sdk-core/src/persist/transactions.rs index 9d3cbec06..681d03a44 100644 --- a/libs/sdk-core/src/persist/transactions.rs +++ b/libs/sdk-core/src/persist/transactions.rs @@ -20,7 +20,11 @@ impl SqliteStorage { /// Note that, if a payment has details of type [LnPaymentDetails] which contain a [SuccessActionProcessed], /// then the [LnPaymentDetails] will NOT be persisted. In that case, the [SuccessActionProcessed] /// can be inserted separately via [SqliteStorage::insert_payment_external_info]. - pub fn insert_or_update_payments(&self, transactions: &[Payment]) -> PersistResult<()> { + pub fn insert_or_update_payments( + &self, + transactions: &[Payment], + is_pseudo: bool, + ) -> PersistResult<()> { let con = self.get_connection()?; let mut prep_statement = con.prepare( " @@ -32,9 +36,10 @@ impl SqliteStorage { fee_msat, status, description, - details + details, + is_pseudo ) - VALUES (?1,?2,?3,?4,?5,?6,?7,?8) + VALUES (?1,?2,?3,?4,?5,?6,?7,?8,?9) ", )?; @@ -48,11 +53,23 @@ impl SqliteStorage { &ln_tx.status, &ln_tx.description, &ln_tx.details, + &is_pseudo, ))?; } Ok(()) } + pub fn delete_pseudo_payments(&self) -> PersistResult<()> { + let con = self.get_connection()?; + let mut stmt = con.prepare("DELETE FROM payments where is_pseudo=1")?; + let res = stmt.execute([])?; + if res > 0 { + debug!("deleted {} pseudo-payments", res); + } + + Ok(()) + } + /// Inserts metadata associated with this payment pub fn insert_payment_external_info( &self, @@ -759,8 +776,8 @@ fn test_ln_transactions() -> PersistResult<(), Box> { }]; let storage = SqliteStorage::new(test_utils::create_test_sql_dir()); storage.init()?; - storage.insert_or_update_payments(&txs)?; - storage.insert_or_update_payments(&failed_txs)?; + storage.insert_or_update_payments(&txs, false)?; + storage.insert_or_update_payments(&failed_txs, false)?; storage.insert_payment_external_info( payment_hash_with_lnurl_success_action, PaymentExternalInfo { @@ -851,7 +868,7 @@ fn test_ln_transactions() -> PersistResult<(), Box> { matches!( &retrieve_txs[1].details, PaymentDetails::Ln {data: LnPaymentDetails {swap_info: swap, ..}} if swap == &Some(swap_info)) ); - storage.insert_or_update_payments(&txs)?; + storage.insert_or_update_payments(&txs, false)?; let retrieve_txs = storage.list_payments(ListPaymentsRequest::default())?; assert_eq!(retrieve_txs.len(), 5); assert_eq!(retrieve_txs, txs); diff --git a/libs/sdk-core/src/swap_in/swap.rs b/libs/sdk-core/src/swap_in/swap.rs index 56b9aa330..95e7bee05 100644 --- a/libs/sdk-core/src/swap_in/swap.rs +++ b/libs/sdk-core/src/swap_in/swap.rs @@ -1162,7 +1162,7 @@ mod tests { }, metadata: None, }; - persister.insert_or_update_payments(&vec![payment.clone()])?; + persister.insert_or_update_payments(&vec![payment.clone()], false)?; // We test the case that a confirmed transaction was detected on chain that // sent funds to this address. @@ -1193,7 +1193,7 @@ mod tests { // paid_amount of the swap. let mut payment = payment.clone(); payment.amount_msat = 2_000; - persister.insert_or_update_payments(&vec![payment])?; + persister.insert_or_update_payments(&vec![payment], false)?; swapper .on_event(BreezEvent::InvoicePaid { details: crate::InvoicePaidDetails {