Skip to content

Conversation

@jvsena42
Copy link
Member

@jvsena42 jvsena42 commented Feb 2, 2026

Description

This PR fixes a critical bug where channel monitor retrieval failures during migration from the React Native app were silently dropped, potentially causing permanent loss of Lightning channel state.

  1. Adds retrieveChannelMonitorWithRetry method with configurable retry attempts and linear backoff
  2. Implements parallel retrieval with proper error tracking for each channel
  3. Logs individual failures with channel IDs for debugging
  4. Validates expected vs retrieved monitor count with warnings

This is the iOS equivalent of the Android fix: synonymdev/bitkit-android#760

Linked Issues/Tasks

Related: synonymdev/bitkit-android#760

Screenshot / Video

N/A - internal migration logic fix

@jvsena42
Copy link
Member Author

jvsena42 commented Feb 2, 2026

@jvsena42 jvsena42 self-assigned this Feb 2, 2026
@claude

This comment has been minimized.

@jvsena42 jvsena42 requested review from ben-kaufman and pwltr February 2, 2026 17:02
@pwltr
Copy link
Contributor

pwltr commented Feb 3, 2026

Is it intentional that we continue with restoring if one or more channel monitors fail to retrieve? Seems to me like we should only restore if everything goes right, and if not show the restore error.

@jvsena42
Copy link
Member Author

jvsena42 commented Feb 3, 2026

Is it intentional that we continue with restoring if one or more channel monitors fail to retrieve? Seems to me like we should only restore if everything goes right, and if not show the restore error.

I thought that if an error happens for some reason that we don't know yet, other than just connection, the user could be stuck outside the wallet

On the case that we got from support, the wallet migrate almost all the channels except one, that was force closed

But I agree, the restore error is also a valid solution, since the user could also try to restore from mnemonics if they are backed up

@pwltr
Copy link
Contributor

pwltr commented Feb 3, 2026

It might be okay to continue if the channel_manager restores even if all of the channel_monitors fail if that's what we want to do, at least then the channels will force close and the user will get their funds. But continuing without channel_manager seems problematic, since the user will not see their LN balance without any further information (I believe, haven't tested). The design also has UI for this case, which I think is not implemented:

image

Copy link
Contributor

@pwltr pwltr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK, comments above not applicable to this PR.

@jvsena42
Copy link
Member Author

jvsena42 commented Feb 3, 2026

But continuing without channel_manager seems problematic, since the user will not see their LN balance without any further information (I believe, haven't tested)

Indeed, that is what happened. I found it was a force close from the logs

@jvsena42
Copy link
Member Author

jvsena42 commented Feb 3, 2026

Waiting for the migration tests to succeed

https://github.com/synonymdev/bitkit-ios/actions/runs/21628335768

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants