-
Notifications
You must be signed in to change notification settings - Fork 2
fix: mnemonic not found crash #678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ovitrif
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utAck
PS. Still need to resolve the nit review comments to be able to merge AFAIU, so I let myself choose Approve even though I want the remarks addressed 🙏🏻
This comment has been minimized.
This comment has been minimized.
|
@piotr-iohk to streamline this |
- Remove unused assertTrue import from VssBackupClientTest - Remove doc comments and inline comments from VssBackupClient - Refactor setup() to return Result<Boolean> - Add DSL-style setupWithRetry() with SetupRetryLogger builder - Move retry logic from BackupRepo to VssBackupClient - Update tests for Result<Boolean> return type Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…view fix: address PR review comments for mnemonic crash fix
This comment was marked as resolved.
This comment was marked as resolved.
jvsena42
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just left two suggestion that can be solved here or in other PR
Addressing here 🙏🏻 |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
jvsena42
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested:
- Lightning channels
- Install over RN
- recover from RN
- Reset and restore
Fixes: #553
Fixes: #691
OBS: #657 (comment)
Description
This PR fixes a crash that occurs when
VssBackupClient.setup()is called before the mnemonic is available in the keychain. This race condition can happen during wallet restoration whenBackupRepo.startObservingBackups()is triggered (onNodeLifecycleState.Running) before the mnemonic has been saved.Root cause:
Changes:
VssBackupClient.setup()now returnsBoolean(true = success, false = mnemonic not available yet)BackupReponow retriessetup()with linear backoff (1s, 2s, 3s... up to 10 attempts) when mnemonic is not yet availablePreview
N/A - crash fix, no UI changes
QA Notes
To reproduce the original crash: note: it is intermittent.
Unit tests added:
setup returns false when mnemonic is not availablesetup does not call vssStoreIdProvider when mnemonic is not availablesetup checks mnemonic before proceeding with vss initializationsetup can be called multiple times when mnemonic not availableRun with:
./gradlew testDevDebugUnitTest --tests "to.bitkit.data.backup.VssBackupClientTest"