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
feat: redesign passphrase backup [NMA-1484] #1106
Conversation
private fun decryptSeed(pin: String, password: String) { | ||
if (deriveKeyTask == null) { | ||
deriveKeyTask = object : DeriveKeyTask(backgroundHandler, scryptIterationsTarget) { | ||
|
||
override fun onSuccess(encryptionKey: KeyParameter, changed: Boolean) { | ||
deriveKeyTask = null | ||
if (decryptSeedTask == null) { | ||
decryptSeedTask = object : DecryptSeedTask(backgroundHandler) { | ||
|
||
override fun onBadPassphrase() { | ||
value = Resource.error("wrong password", Pair(wallet.keyChainSeed, pin)) | ||
decryptSeedTask = null | ||
} | ||
|
||
override fun onSuccess(seed : DeterministicSeed) { | ||
value = Resource.success(Pair(seed, pin)) | ||
decryptSeedTask = null | ||
} | ||
} | ||
value = Resource.loading(null) | ||
decryptSeedTask!!.decryptSeed(wallet.keyChainSeed, wallet.keyCrypter, encryptionKey) | ||
} | ||
} | ||
} | ||
value = Resource.loading(null) | ||
deriveKeyTask!!.deriveKey(wallet, password) | ||
} |
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.
I think it's better to have this logic as a couple of suspend
calls in the SecurityFunctions
class. That way, we can use it from anywhere - not just the UI thread.
if (showBackupWalletDialog) { | ||
BackupWalletDialogFragment.show(getSupportFragmentManager()); | ||
showBackupWalletDialog = false; | ||
} | ||
} |
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.
Looks like we don't need this anymore. showBackupWalletDialog
is never set to true
.
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.
Correct.
|
||
wipePasswords(); | ||
super.onDismiss(dialog); | ||
} | ||
|
||
@Override | ||
public void onCancel(final DialogInterface dialog) { | ||
activity.finish(); |
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.
Not sure why we were finishing SecurityActivity
on the dialog dismissal. Seems like just dismissing is fine.
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.
This was taken from Bitcoin Wallet, where I think an activity was created that displays the backup wallet dialog.
However, we are not creating an activity to only display this dialog:
class DecryptSeedWithPinDialog( | ||
private var onSuccessOrDismiss: ((DeterministicSeed?) -> Unit)? | ||
private var onSuccessOrDismiss: ((Array<String>) -> Unit)? |
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.
This is an interesting change. We must not need the DeterministicSeed
object, only the phrase.
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.
LGTM.
We want to remove an extra info screen from the "backup passphrase" flow and add a "success" screen.
Some redesign is also required.
Issue being fixed or feature implemented
VerifyActivity
to use a navigation controller.SecurityActivity
to aSecurityFragment
.ViewSeedActivity
andVerifyWriteDownFragment
since they are the same.DecryptSeedLiveData
to move this logic intoSecurityFunctions
.Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist: