Add electrum support to lightning-transaction-sync

[tnull]

Closes #2010.

This PR implements an ElectrumSyncClient that allows syncing LDK via the Confirm/Filter interfaces based on BDK's electrum-client crate.

We furthermore improve the logging in EsploraSyncClient and ElectrumSyncClient, and extend our test coverage.

Currently in draft as I want to do some further cleanup and experimentation.

[codecov-commenter]

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (70ea110) 88.50% compared to head (31ea90e) 88.53%.

Files	Patch %	Lines
lightning/src/chain/channelmonitor.rs	0.00%	6 Missing ⚠️
lightning/src/chain/onchaintx.rs	0.00%	5 Missing ⚠️
lightning/src/chain/chainmonitor.rs	0.00%	3 Missing ⚠️
lightning/src/ln/channel.rs	83.33%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2685      +/-   ##
==========================================
+ Coverage   88.50%   88.53%   +0.02%     
==========================================
  Files         113      113              
  Lines       89323    89334      +11     
  Branches    89323    89334      +11     
==========================================
+ Hits        79055    79090      +35     
+ Misses       7890     7875      -15     
+ Partials     2378     2369       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tnull]

Did some further cleanup, should be ready for review now.

[jkczyz]

Still have a bit to get through

[jkczyz]

Mostly looks good. Feel free to squash fixups. Will take a final pass after.

[tnull]

Squashed previous fixups and addressed the pending comments.

[jkczyz]

LGTM after a couple minor comments

[tnull]

Squashed fixups and included the following changes:

diff --git a/lightning-transaction-sync/src/electrum.rs b/lightning-transaction-sync/src/electrum.rs
index a37d017fd..a002fc8e2 100644
--- a/lightning-transaction-sync/src/electrum.rs
+++ b/lightning-transaction-sync/src/electrum.rs
@@ -320,13 +320,13 @@ where
                                                        if let Some(history) = script_history.iter()
                                                                .filter(|h| h.tx_hash == txid).max_by_key(|x| x.height)
-                                                               {
-                                                                       let prob_conf_height = history.height;
-                                                                       let block_header = self.client.block_header(
-                                                                               prob_conf_height as usize)?;
-                                                                       if block_header.block_hash() == block_hash {
-                                                                               // Skip if the tx is still confirmed in the block in question.
-                                                                               continue;
-                                                                       }
+                                                       {
+                                                               let prob_conf_height = history.height;
+                                                               let block_header = self.client.block_header(
+                                                                       prob_conf_height as usize)?;
+                                                               if block_header.block_hash() == block_hash {
+                                                                       // Skip if the tx is still confirmed in the block in question.
+                                                                       continue;
                                                                }
+                                                       }
                                                }
                                        }
@@ -404,11 +404,6 @@ where
                for mut bytes in merkle_res.merkle {
                        bytes.reverse();
-                       let next_hash = Sha256d::from_slice(&bytes).map_err(|_| {
-                               log_error!(self.logger,
-                                       "Failed due to the server sending us bogus transaction data. This should not happen. Please verify server integrity."
-                               );
-                               InternalError::Failed
-                       })?;
-
+                       // unwrap() safety: `bytes` has len 32 so `from_slice` can never fail.
+                       let next_hash = Sha256d::from_slice(&bytes).unwrap();
                        let (left, right) = if index % 2 == 0 {
                                (cur, next_hash)

[tnull]

Moved logging and updating SyncState back to the call sites and renamed the method:

> git diff-tree -U2 a48cf50b8 d609b5d37

diff --git a/lightning-transaction-sync/src/electrum.rs b/lightning-transaction-sync/src/electrum.rs
index 9d39ed87b..4dbd53609 100644
--- a/lightning-transaction-sync/src/electrum.rs
+++ b/lightning-transaction-sync/src/electrum.rs
@@ -113,7 +113,9 @@ where
                                                        // Double-check the tip hash. If it changed, a reorg happened since
                                                        // we started syncing and we need to restart last-minute.
-                                                       if self.check_has_chain_moved(&mut sync_state, &mut tip_header,
-                                                               &mut tip_height)?
+                                                       if self.check_update_tip(&mut tip_header, &mut tip_height)?
                                                        {
+                                                               log_debug!(self.logger,
+                                                                       "Encountered inconsistency during transaction sync, restarting.");
+                                                               sync_state.pending_sync = true;
                                                                continue;
                                                        }
@@ -145,7 +147,9 @@ where
                                                // Double-check the tip hash. If it changed, a reorg happened since
                                                // we started syncing and we need to restart last-minute.
-                                               if self.check_has_chain_moved(&mut sync_state, &mut tip_header,
-                                                       &mut tip_height)?
+                                               if self.check_update_tip(&mut tip_header, &mut tip_height)?
                                                {
+                                                       log_debug!(self.logger,
+                                                               "Encountered inconsistency during transaction sync, restarting.");
+                                                       sync_state.pending_sync = true;
                                                        continue;
                                                }
@@ -186,5 +190,7 @@ where
        }

-       fn check_has_chain_moved(&self, sync_state: &mut SyncState, cur_tip_header: &mut BlockHeader, cur_tip_height: &mut u32) -> Result<bool, InternalError> {
+       fn check_update_tip(&self, cur_tip_header: &mut BlockHeader, cur_tip_height: &mut u32)
+               -> Result<bool, InternalError>
+       {
                let check_notification = self.client.block_headers_subscribe()?;
                let check_tip_hash = check_notification.header.block_hash();
@@ -203,8 +209,4 @@ where
                        *cur_tip_header = check_notification.header;
                        *cur_tip_height = check_notification.height as u32;
-                       log_debug!(self.logger,
-                               "Encountered inconsistency during transaction sync, restarting.");
-                       sync_state.pending_sync = true;
-
                        Ok(true)
                } else {

[tnull]

Now included two more commits that set pending_sync when our last-minute tip checks failed and would have us error out (both for Esplora and Electrum).

[tnull]

I just double-checked that there are no version conflicts/notable API changes messing with this after the bumps in #2740. Have a rebased branch ready-to-go, so happy to land this after #2740.

[TheBlueMatt]

Should be good to go. I dunno if @jkczyz had any other thoughts, though.

[tnull]

Rebased on main after #2740 landed.

[tnull]

Introduced a small follow-up commit to the rebase. As we've now upgraded to use esplora-client 0.6, we can make use of the introduced async-https-rustls feature.