fix: prevent RN migration from overwriting local channel monitors#495
fix: prevent RN migration from overwriting local channel monitors#495
Conversation
Required ldk-node changeThis PR prevents the bug on the Swift side, but ldk-node also needs a fix to prevent Recommended: Migration-aware write in
|
Safest recovery path for already-affected usersThe safest approach is to accept stale monitors and cooperative-close the affected channels, rather than wiping data or force-closing with revoked commitments. Why this is the safest option
How it works
Required changesrust-lightning ( The stale check and // Instead of returning Err(DecodeError::DangerousValue) when monitor is stale:
if args.accept_stale_channel_monitors {
log_warn!(logger,
"Accepting stale monitor for channel {} (monitor update_id {} vs manager update_id {}). \
Cooperative close recommended.",
channel_id, monitor_update_id, manager_update_id
);
stale_channels.push(channel_id);
// Continue deserialization instead of returning error
} else {
return Err(DecodeError::DangerousValue); // existing behavior
}ldk-node ( Expose the flag via the builder API: pub fn set_accept_stale_channel_monitors(&mut self, accept: bool) -> &mut Self {
self.accept_stale_monitors = accept;
self
}After the node starts, auto-close stale channels: for channel_id in stale_channels {
node.close_channel(&channel_id, counterparty_node_id)?;
}Swift side (after ldk-node change): On the first startup failure due to stale monitors, retry with the flag enabled: // In LightningService.setup() or WalletViewModel.start()
// If node fails with stale monitor error, rebuild with:
builder.setAcceptStaleChannelMonitors(true)
// Node starts → stale channels are auto-closed cooperativelyCombine with migration-aware write (prevention)For defense-in-depth, also add the migration-aware write from the previous comment to |
Impact on displayed balanceThe balance values ( When the node fails to start due to the stale monitor:
Result: The user sees a plausible-looking but completely frozen balance — the last known values from before the app update. It's not zero, so it doesn't immediately look broken. But:
|
Description
This PR prevents old RN channel monitors from overwriting newer local ones during orphaned channel recovery, which caused LDK to refuse to start and display stale balances.
The root cause: PR #462 introduced orphaned channel monitor recovery that re-fetches all RN monitors from the remote backup. LDK's
setChannelDataMigration()blindly writes migration monitors to storage, overwriting existing ones -- including monitors that have advanced far beyond the old RN version. PR #480 fixed the channel manager overwrite but not the monitor overwrite.The fix adds a filtering step: before applying RN migration monitors, the app checks which channels already have local monitors and skips those. This prevents stale RN monitors from replacing up-to-date local ones.
Note: recovery for already-affected users requires ldk-node changes (migration-aware writes or force-close of stale channels). This PR only addresses prevention.
Logs from the bug
bitkit_logs_2026-03-17_14-16-30.zip
Linked Issues/Tasks
Related PRs:
Screenshot / Video
N/A - backend logic change