-
Notifications
You must be signed in to change notification settings - Fork 1
Fix: Encrypted Json Flow #78
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
base: main
Are you sure you want to change the base?
Conversation
|
After looking at the import flow it seems that the encrypted file thing never worked as the secret keys were never added when importing accounts |
6ba0d3e to
47e3714
Compare
| const midenClientDbContent = decryptedWallet.midenClientDbContent; | ||
| const walletDbContent = decryptedWallet.walletDbContent; | ||
| const seedPhrase = decryptedWallet.seedPhrase; | ||
| const walletAccounts = decryptedWallet.accounts; |
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.
Old export files without the accounts field will cause import to fail. Need to add fallback: decryptedWallet.accounts ?? []
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.
But they would be useless anyways cause there is no way to get secret key without having accounts, I mean we could insert a single account assuming it would be a "On Chain" account but if it was not then that would be disastrous.
Also in lieu of upcoming upgrades the old accounts would be useless or will have zero values anyways
| vaultKey | ||
| ); | ||
| await savePlain(currentAccPubKeyStrgKey, newAccounts[0].publicKey); | ||
| await savePlain(currentAccPubKeyStrgKey, walletAccounts[0].publicKey); |
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.
walletAccounts[0].publicKey without checking if array is empty will crash if importing old export files
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.
Would add a error check here thanks
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.
On another thought the loop above would anyways throw error if it could not find the walletAccount.
Or could it be the case that there are no accounts in miden db and no wallet accounts also? Unlikely no
| console.log('importing wallet from client'); | ||
| await importWalletFromClient(password, seedPhraseFormatted, importedWalletAccounts); | ||
| } catch (e) { | ||
| console.error(e); |
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 we should show an error message, no?
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.
No UI for this currently; do you want me to address it in this one or separate one
importing from file
e5ce701 to
97e8b4a
Compare
Currently we are also importing faucet accounts from a encrypted json which have imported to client when doing metadata fetching.
While we are removing metadata fetching by importing faucet accounts with the release where this 0xMiden/miden-client#1638 is included (was not in
13.next.1) but until then this should do and I feel it is good to check otherwise also