`ElectrumSyncClient`: Skip unconfirmed `get_history` entries
What the developer wrote
Authored by Elias Rohrer
`ElectrumSyncClient`: Skip unconfirmed `get_history` entries
Electrum's `blockchain.scripthash.get_history` will return the
*confirmed* history for any scripthash, but will then also append any
matching entries from the mempool, with respective `height` fields set
to 0 or -1 (depending on whether all inputs are confirmed or not).
Unfortunately we previously only included a filter for confirmed
`get_history` entries in the watched output case, and forgot to add such
a check also when checking for watched transactions. This would have us
treat the entry as confirmed, then failing on the `get_merkle` step
which of course couldn't prove block inclusion. Here we simply fix this
omission and skip entries that are still unconfirmed (e.g., unconfirmed
funding transactions from 0conf channels).
Signed-off-by: Elias Rohrer <dev@tnull.de>
What changed, and why it matters
This patch fixes a bug in rust-lightning's Electrum-based transaction sync. When checking whether watched transactions were confirmed, the code previously treated unconfirmed mempool entries from Electrum's get_history as if they were already confirmed. It would then try to fetch a Merkle proof of block inclusion, which cannot exist for an unconfirmed transaction, causing a failure. The fix skips any history entry whose height is 0 or -1 (unconfirmed) in the watched-transaction path, matching what was already done for watched outputs. A debug assertion was also replaced with a proper error log and return if a server returns duplicate history entries.
Apply the patch. Users relying on ElectrumSyncClient should upgrade to avoid spurious sync failures when watched transactions are still unconfirmed (e.g., 0conf channel funding). No immediate key-rotation or emergency response is indicated; this is a robustness fix rather than a key-compromise vulnerability.
Security signals we found
Denial-of-service via unconfirmed transaction handling: a malicious or buggy Electrum server, or simply mempool state, could return unconfirmed history entries that caused local sync failure.
Incorrect confirmation state assumption: mempool entries were processed as confirmed, leading to impossible get_merkle requests.
Inconsistent filtering between watched outputs and watched transactions paths.
Replacement of debug-only assertion with runtime error handling for duplicate history entries.
Evidence from the diff
In lightning-transaction-sync/src/electrum.rs, the loop over watched transactions (txids) now filters script_history entries with height <= 0 before calling get_confirmed_tx and get_merkle. Previously this filter existed only in the watched_output loop. The change prevents treating mempool-only transactions (height 0 or -1 from Electrum’s blockchain.scripthash.get_history) as confirmed. Additionally, the debug_assert!(filtered_history.next().is_none()) for duplicate history entries is replaced with an explicit error log and InternalError::Failed return.
Changed components
lightning-transaction-sync/src/electrum.rsElectrumSyncClientwatched transaction confirmation logicInspect captured patch +13 / −1
diff --git a/lightning-transaction-sync/src/electrum.rs b/lightning-transaction-sync/src/electrum.rs
index 47489df..1162b9c 100644
--- a/lightning-transaction-sync/src/electrum.rs
+++ b/lightning-transaction-sync/src/electrum.rs
@@ -336,10 +336,21 @@ where
script_history.iter().filter(|h| h.tx_hash == **txid);
if let Some(history) = filtered_history.next() {
let prob_conf_height = history.height as u32;
+ if prob_conf_height <= 0 {
+ // Skip if it's a an unconfirmed entry.
+ continue;
+ }
let confirmed_tx = self.get_confirmed_tx(tx, prob_conf_height)?;
confirmed_txs.push(confirmed_tx);
}
- debug_assert!(filtered_history.next().is_none());
+ if filtered_history.next().is_some() {
+ log_error!(
+ self.logger,
+ "Failed due to server returning multiple history entries for Tx {}.",
+ txid
+ );
+ return Err(InternalError::Failed);
+ }
}
for (watched_output, script_history) in
@@ -347,6 +358,7 @@ where
{
for possible_output_spend in script_history {
if possible_output_spend.height <= 0 {
+ // Skip if it's a an unconfirmed entry.
continue;
}
Why this scored 49/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.