Bound sync loops in lightning-transaction-sync
What the developer wrote
Authored by Matt Corallo
Bound sync loops in lightning-transaction-sync
If we start syncing from an electrum or esplora server and find
that the chain moved during our sync, we reset and start fresh.
However, if that happens repeatedly, we probably shouldn't just
spin forever. Here we give up after ten attempts and just hope we
can sync properly later.
What changed, and why it matters
This commit fixes a potential denial-of-service (DoS) issue in the Lightning Dev Kit's transaction sync module. Previously, if the Bitcoin blockchain kept changing during a sync attempt, the code could loop forever trying to catch up. The patch limits this to 10 attempts before giving up and trying again later, preventing the node from getting stuck or wasting excessive resources.
Review whether 10 attempts is an appropriate bound under adversarial conditions, and consider adding exponential backoff or jitter before retrying. Monitor for any new error handling paths that downstream callers must accommodate.
Security signals we found
Unbounded loop replaced with bounded retry limit
Potential denial-of-service via infinite resync on chain tip churn
Error return path added after retry exhaustion
Sync state marked pending for later retry
Evidence from the diff
The patch modifies lightning-transaction-sync/src/electrum.rs and lightning-transaction-sync/src/esplora.rs to replace unbounded loop {} constructs with bounded for i in 0..100 loops that return TxSyncError::Failed after 10 iterations. When the chain tip changes during synchronization, the sync resets and restarts. Without a bound, a rapidly moving chain tip or a malicious/manipulated server could cause the client to spin indefinitely, consuming CPU, network bandwidth, and holding locks. The fix sets sync_state.pending_sync = true so a future sync attempt can recover.
Changed components
lightning-transaction-sync/src/electrum.rslightning-transaction-sync/src/esplora.rsElectrumSyncClientEsploraSyncClientInspect captured patch +14 / −2
diff --git a/lightning-transaction-sync/src/electrum.rs b/lightning-transaction-sync/src/electrum.rs
index 9d643f4..cb93724 100644
--- a/lightning-transaction-sync/src/electrum.rs
+++ b/lightning-transaction-sync/src/electrum.rs
@@ -96,7 +96,13 @@ impl<L: Logger> ElectrumSyncClient<L> {
let mut tip_header = tip_notification.header;
let mut tip_height = tip_notification.height as u32;
- loop {
+ for i in 0..100 {
+ if i >= 10 {
+ log_debug!(self.logger, "Giving up trying to sync transactions after 10 attempts.");
+ sync_state.pending_sync = true;
+ return Err(TxSyncError::Failed);
+ }
+
let pending_registrations = self.queue.lock().unwrap().process_queues(&mut sync_state);
let tip_is_new = Some(tip_header.block_hash()) != sync_state.last_sync_hash;
diff --git a/lightning-transaction-sync/src/esplora.rs b/lightning-transaction-sync/src/esplora.rs
index 7d3550d..52cfb39 100644
--- a/lightning-transaction-sync/src/esplora.rs
+++ b/lightning-transaction-sync/src/esplora.rs
@@ -100,7 +100,13 @@ impl<L: Logger> EsploraSyncClient<L> {
let mut tip_hash = maybe_await!(self.client.get_tip_hash())?;
- loop {
+ for i in 0..100 {
+ if i >= 10 {
+ log_debug!(self.logger, "Giving up trying to sync transactions after 10 attempts.");
+ sync_state.pending_sync = true;
+ return Err(TxSyncError::Failed);
+ }
+
let pending_registrations = self.queue.lock().unwrap().process_queues(&mut sync_state);
let tip_is_new = Some(tip_hash) != sync_state.last_sync_hash;
Why this scored 36/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.