Bypass channel monitor sync requests when no partition key given
What changed, and why it matters
This small patch changes how a Lightning node decides which channel monitors need to be re-synchronized with the blockchain after a restart. Previously, if the node's best-known block height was missing, the code treated that as height 0 and still computed a partition key, which could cause the wrong subset of monitors to sync. Now, if no best height is known, the partition key is treated as absent and the monitor is skipped unless it has pending claims. The fix is more about correctness and avoiding unnecessary/misdirected sync work than a direct theft-of-funds vulnerability, but in a Lightning context monitor-sync bugs can affect safety.
Review whether any production deployments could have run with best_height=None and therefore skipped monitors that should have synced. Consider adding a regression test that verifies sync behavior when best_height is absent. No immediate emergency response appears warranted, but treat as a correctness fix in safety-critical code.
Security signals we found
Missing/default value used as valid partition input (best_height unwrap_or_default)
Partition-based sync selection could select wrong monitor subset when chain tip unknown
Lightning channel monitor sync correctness affects fund safety
Patch is minimal and defensive
Evidence from the diff
In chainmonitor.rs, get_partition_key() previously did channel_id_u32.wrapping_add(best_height.unwrap_or_default()), so a missing best_height became 0 and always produced a deterministic key. The caller then checked get_partition_key(channel_id) % partition_factor == 0 to decide whether to sync this monitor. The patch makes get_partition_key return Option<u32>: best_height.map(|height| channel_id_u32.wrapping_add(height)), and the caller uses is_some_and(|key| key % partition_factor == 0). Thus when best_height is None the monitor is bypassed unless has_pending_claims is true. This prevents the sync-selection logic from running on a bogus/default height and avoids syncing the wrong partition of monitors.
Changed components
lightning/src/chain/chainmonitor.rsChannel monitor synchronization schedulerPartition-key based sync selectionInspect captured patch +2 / −2
diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs
index 8644301..0a790b3 100644
--- a/lightning/src/chain/chainmonitor.rs
+++ b/lightning/src/chain/chainmonitor.rs
@@ -555,7 +555,7 @@ where
channel_id_bytes[2],
channel_id_bytes[3],
]);
- channel_id_u32.wrapping_add(best_height.unwrap_or_default())
+ best_height.map(|height| channel_id_u32.wrapping_add(height))
};
let partition_factor = if channel_count < 15 {
@@ -565,7 +565,7 @@ where
};
let has_pending_claims = monitor_state.monitor.has_pending_claims();
- if has_pending_claims || get_partition_key(channel_id) % partition_factor == 0 {
+ if has_pending_claims || get_partition_key(channel_id).is_some_and(|key| key % partition_factor == 0) {
log_trace!(logger, "Syncing Channel Monitor");
// Even though we don't track monitor updates from chain-sync as pending, we still want
// updates per-channel to be well-ordered so that users don't see a
Why this scored 30/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.