Merge pull request #520 from breez/ok300-add-txid-closed-channel-paym…

…ents

Add closing_txid to Channel, ClosedChannelPaymentDetails

libs/sdk-bindings/src/breez_sdk.udl

@@ -200,6 +200,7 @@ dictionary ClosedChannelPaymentDetails {
     string short_channel_id;
     ChannelState state;
     string funding_txid;
+    string? closing_txid;
 };
 
 enum ChannelState {

libs/sdk-core/src/breez_services.rs

@@ -22,7 +22,7 @@ use tonic::{Request, Status};
 
 use crate::backup::{BackupRequest, BackupTransport, BackupWatcher};
 use crate::boltzswap::BoltzApi;
-use crate::chain::{ChainService, MempoolSpace, RecommendedFees};
+use crate::chain::{ChainService, MempoolSpace, Outspend, RecommendedFees};
 use crate::error::{SdkError, SdkResult};
 use crate::fiat::{FiatCurrency, Rate};
 use crate::greenlight::{GLBackupTransport, Greenlight};
@@ -1226,55 +1226,77 @@ impl BreezServices {
         Ok(())
     }
 
+    async fn lookup_chain_service_closing_outspend(
+        &self,
+        channel: crate::models::Channel,
+    ) -> Result<Option<Outspend>> {
+        match channel.funding_outnum {
+            None => Ok(None),
+            Some(outnum) => {
+                // Find the output tx that was used to fund the channel
+                let outspends = self
+                    .chain_service
+                    .transaction_outspends(channel.funding_txid.clone())
+                    .await?;
+
+                Ok(outspends.get(outnum as usize).cloned())
+            }
+        }
+    }
+
+    /// Chain service lookup of relevant channel closing fields (closed_at, closing_txid).
+    ///
+    /// Should be used sparingly because it involves a network lookup.
+    async fn lookup_channel_closing_data(
+        &self,
+        channel: &crate::models::Channel,
+    ) -> Result<(Option<u64>, Option<String>)> {
+        let maybe_outspend = self
+            .lookup_chain_service_closing_outspend(channel.clone())
+            .await?;
+
+        let maybe_closed_at = maybe_outspend
+            .clone()
+            .and_then(|outspend| outspend.status)
+            .and_then(|s| s.block_time);
+        let maybe_closing_txid = maybe_outspend.and_then(|outspend| outspend.txid);
+
+        Ok((maybe_closed_at, maybe_closing_txid))
+    }
+
     async fn closed_channel_to_transaction(
         &self,
         channel: crate::models::Channel,
     ) -> Result<Payment> {
-        let now_epoch_sec = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs();
-
-        let channel_closed_at = match channel.closed_at {
-            Some(closed_at) => closed_at,
-            None => {
-                // If we don't have it, we look it up from the channel closing tx
-                let looked_up_channel_closed_at = match channel.funding_outnum {
+        let (payment_time, closing_txid) = match (channel.closed_at, channel.closing_txid.clone()) {
+            (Some(closed_at), Some(closing_txid)) => (closed_at as i64, Some(closing_txid)),
+            (_, _) => {
+                // If any of the two closing-related fields are empty, we look them up and persist them
+                let (maybe_closed_at, maybe_closing_txid) =
+                    self.lookup_channel_closing_data(&channel).await?;
+
+                let processed_closed_at = match maybe_closed_at {
                     None => {
-                        warn!("No founding_outnum found for the closing tx, defaulting closed_at to epoch time");
-                        now_epoch_sec
-                    }
-                    Some(outnum) => {
-                        // Find the output tx that was used to fund the channel
-                        let outspends = self
-                            .chain_service
-                            .transaction_outspends(channel.funding_txid.clone())
-                            .await?;
-                        let maybe_block_time = outspends
-                            .get(outnum as usize)
-                            .and_then(|outspend| outspend.status.as_ref())
-                            .and_then(|status| status.block_time);
-
-                        match maybe_block_time {
-                            None => {
-                                warn!("Blocktime could not be determined for funding_outnum {outnum}, defaulting closed_at to epoch time");
-                                now_epoch_sec
-                            }
-                            Some(block_time) => block_time,
-                        }
+                        warn!("Blocktime could not be determined for from closing outspend, defaulting closed_at to epoch time");
+                        SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs()
                     }
+                    Some(block_time) => block_time,
                 };
 
-                // Persist closed_at, if we had to look it up
                 let mut updated_channel = channel.clone();
-                updated_channel.closed_at = Some(looked_up_channel_closed_at);
+                updated_channel.closed_at = Some(processed_closed_at);
+                // If no closing txid found, we persist it as None, so it will be looked-up next time
+                updated_channel.closing_txid = maybe_closing_txid.clone();
                 self.persister.insert_or_update_channel(updated_channel)?;
 
-                looked_up_channel_closed_at
+                (processed_closed_at as i64, maybe_closing_txid)
             }
         };
 
         Ok(Payment {
             id: channel.funding_txid.clone(),
             payment_type: PaymentType::ClosedChannel,
-            payment_time: channel_closed_at as i64,
+            payment_time,
             amount_msat: channel.spendable_msat,
             fee_msat: 0,
             status: match channel.state {
@@ -1287,6 +1309,7 @@ impl BreezServices {
                     short_channel_id: channel.short_channel_id,
                     state: channel.state,
                     funding_txid: channel.funding_txid,
+                    closing_txid,
                 },
             },
         })

libs/sdk-core/src/bridge_generated.rs

@@ -898,6 +898,7 @@ impl support::IntoDart for ClosedChannelPaymentDetails {
             self.short_channel_id.into_dart(),
             self.state.into_dart(),
             self.funding_txid.into_dart(),
+            self.closing_txid.into_dart(),
         ]
         .into_dart()
     }

libs/sdk-core/src/greenlight/node_api.rs

@@ -1219,6 +1219,7 @@ impl From<cln::ListpeersPeers> for Peer {
     }
 }
 
+/// Conversion for an open channel
 impl From<cln::ListpeersPeersChannels> for Channel {
     fn from(c: cln::ListpeersPeersChannels) -> Self {
         let state = match c.state() {
@@ -1245,10 +1246,12 @@ impl From<cln::ListpeersPeersChannels> for Channel {
             funding_outnum: c.funding_outnum,
             alias_remote,
             alias_local,
+            closing_txid: None,
         }
     }
 }
 
+/// Conversion for a closed channel
 impl TryFrom<ListclosedchannelsClosedchannels> for Channel {
     type Error = anyhow::Error;
 
@@ -1257,6 +1260,9 @@ impl TryFrom<ListclosedchannelsClosedchannels> for Channel {
             Some(a) => (a.remote, a.local),
             None => (None, None),
         };
+
+        // To keep the conversion simple and fast, some closing-related fields (closed_at, closing_txid)
+        // are left empty here in the conversion, but populated later (via chain service lookup, or DB lookup)
         Ok(Channel {
             short_channel_id: c
                 .short_channel_id
@@ -1268,10 +1274,11 @@ impl TryFrom<ListclosedchannelsClosedchannels> for Channel {
                 .ok_or(anyhow!("final_to_us_msat is missing"))?
                 .msat,
             receivable_msat: 0,
-            closed_at: None, // Don't fill at his time, because it involves a chain_service lookup
+            closed_at: None,
             funding_outnum: Some(c.funding_outnum),
             alias_remote,
             alias_local,
+            closing_txid: None,
         })
     }
 }

libs/sdk-core/src/models.rs

@@ -718,6 +718,8 @@ pub struct ClosedChannelPaymentDetails {
     pub short_channel_id: String,
     pub state: ChannelState,
     pub funding_txid: String,
+    /// Can be empty for older closed channels.
+    pub closing_txid: Option<String>,
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -990,6 +992,10 @@ pub struct Channel {
     pub funding_outnum: Option<u32>,
     pub alias_local: Option<String>,
     pub alias_remote: Option<String>,
+    /// Only set for closed channels.
+    ///
+    /// This may be empty for older closed channels, if it was not possible to retrieve the closing txid.
+    pub closing_txid: Option<String>,
 }
 
 /// State of a Lightning channel

libs/sdk-core/src/persist/channels.rs

@@ -5,6 +5,12 @@ use anyhow::Result;
 use std::str::FromStr;
 
 impl SqliteStorage {
+    /// Expects a full list of (non-closed) channels.
+    ///
+    /// Any known channel that is missing from the list, will be marked as closed. When doing so, the
+    /// closing-related fields `closed_at` and `closing_txid` are not set, because doing so would require
+    /// a chain service lookup. Instead, they will be set on first lookup in
+    /// [BreezServices::closed_channel_to_transaction]
     pub(crate) fn update_channels(&self, channels: &[Channel]) -> Result<()> {
         // insert all channels
         for c in channels.iter().cloned() {
@@ -17,14 +23,13 @@ impl SqliteStorage {
             .map(|c| format!("'{}'", c.funding_txid))
             .collect();
 
-        // close channels not in list
+        // Close channels not in the list provided
         self.get_connection()?.execute(
             format!(
                 "
                  UPDATE channels 
                  SET 
-                  state=?1, 
-                  closed_at = case when closed_at is null then unixepoch() else closed_at end 
+                  state=?1
                  where funding_txid not in ({})
                 ",
                 funding_txs.join(",")
@@ -49,7 +54,8 @@ impl SqliteStorage {
                 closed_at,
                 funding_outnum,
                 alias_local,
-                alias_remote
+                alias_remote,
+                closing_txid
                FROM channels             
              ",
         )?;
@@ -67,6 +73,7 @@ impl SqliteStorage {
                     funding_outnum: row.get(6)?,
                     alias_local: row.get(7)?,
                     alias_remote: row.get(8)?,
+                    closing_txid: row.get(9)?,
                 })
             })?
             .map(|i| i.unwrap())
@@ -86,9 +93,10 @@ impl SqliteStorage {
                    closed_at,
                    funding_outnum,                   
                    alias_local,
-                   alias_remote
+                   alias_remote,
+                   closing_txid
                   )
-                  VALUES (?1,?2,?3,?4,?5,?6,?7,?8,?9)
+                  VALUES (?1,?2,?3,?4,?5,?6,?7,?8,?9,?10)
                ",
             (
                 c.funding_txid,
@@ -103,6 +111,7 @@ impl SqliteStorage {
                 c.funding_outnum,
                 c.alias_local,
                 c.alias_remote,
+                c.closing_txid,
             ),
         )?;
         Ok(())
@@ -127,6 +136,7 @@ fn test_simple_sync_channels() {
             funding_outnum: None,
             alias_local: None,
             alias_remote: None,
+            closing_txid: None,
         },
         Channel {
             funding_txid: "456".to_string(),
@@ -138,6 +148,7 @@ fn test_simple_sync_channels() {
             funding_outnum: None,
             alias_local: None,
             alias_remote: None,
+            closing_txid: None,
         },
     ];
 
@@ -168,7 +179,9 @@ fn test_sync_closed_channels() {
             funding_outnum: None,
             alias_local: None,
             alias_remote: None,
+            closing_txid: None,
         },
+        // Simulate closed channel that was persisted with closed_at and closing_txid
         Channel {
             funding_txid: "456".to_string(),
             short_channel_id: "13x14x15".to_string(),
@@ -179,6 +192,7 @@ fn test_sync_closed_channels() {
             funding_outnum: None,
             alias_local: None,
             alias_remote: None,
+            closing_txid: Some("a".into()),
         },
     ];
 
@@ -187,6 +201,7 @@ fn test_sync_closed_channels() {
     assert_eq!(2, queried_channels.len());
     assert_eq!(channels[0], queried_channels[0]);
     assert!(queried_channels[1].closed_at.is_some());
+    assert!(queried_channels[1].closing_txid.is_some());
 
     storage.update_channels(&channels).unwrap();
     let queried_channels = storage.list_channels().unwrap();
@@ -206,6 +221,7 @@ fn test_sync_closed_channels() {
             funding_outnum: None,
             alias_local: None,
             alias_remote: None,
+            closing_txid: None,
         },
         Channel {
             funding_txid: "456".to_string(),
@@ -217,11 +233,18 @@ fn test_sync_closed_channels() {
             funding_outnum: None,
             alias_local: None,
             alias_remote: None,
+            closing_txid: None,
         },
     ];
     assert_eq!(expected.len(), queried_channels.len());
-    assert!(queried_channels[0].closed_at.is_some());
+    // For channel inserted WITHOUT closed_at and closing_txid,
+    // the closing-related fields are empty on channel queried directly from DB
+    assert!(queried_channels[0].closed_at.is_none());
+    assert!(queried_channels[0].closing_txid.is_none());
+    // For channel inserted WITH closed_at and closing_txid (as for example after a chain service lookup),
+    // the closing-related fields are not empty on channel queried directly from DB
     assert!(queried_channels[1].closed_at.is_some());
+    assert!(queried_channels[1].closing_txid.is_some());
 
     // test dedup channels in db
     storage.update_channels(&channels).unwrap();

libs/sdk-core/src/persist/migrations.rs

@@ -414,7 +414,8 @@ pub(crate) fn current_migrations() -> Vec<&'static str> {
        ",
        "SELECT 1;", // Placeholder statement, to avoid that column is added twice (from sync fn below and here)
        "ALTER TABLE channels ADD COLUMN alias_local TEXT;",
-       "ALTER TABLE channels ADD COLUMN alias_remote TEXT;"
+       "ALTER TABLE channels ADD COLUMN alias_remote TEXT;",
+       "ALTER TABLE channels ADD COLUMN closing_txid TEXT;"
     ]
 }
 

libs/sdk-flutter/lib/bridge_generated.dart

@@ -402,10 +402,14 @@ class ClosedChannelPaymentDetails {
   final ChannelState state;
   final String fundingTxid;
 
+  /// Can be empty for older closed channels.
+  final String? closingTxid;
+
   const ClosedChannelPaymentDetails({
     required this.shortChannelId,
     required this.state,
     required this.fundingTxid,
+    this.closingTxid,
   });
 }
 
@@ -2408,11 +2412,12 @@ class BreezSdkCoreImpl implements BreezSdkCore {
 
   ClosedChannelPaymentDetails _wire2api_closed_channel_payment_details(dynamic raw) {
     final arr = raw as List<dynamic>;
-    if (arr.length != 3) throw Exception('unexpected arr length: expect 3 but see ${arr.length}');
+    if (arr.length != 4) throw Exception('unexpected arr length: expect 4 but see ${arr.length}');
     return ClosedChannelPaymentDetails(
       shortChannelId: _wire2api_String(arr[0]),
       state: _wire2api_channel_state(arr[1]),
       fundingTxid: _wire2api_String(arr[2]),
+      closingTxid: _wire2api_opt_String(arr[3]),
     );
   }