[AD-501] Move wallet restore logic to cardano modules #391
Conversation
epicallan
force-pushed
the
epicallan/ad501-move-wallet-restore-logic
branch
from
c551e97
to
dd169b6
Compare
epicallan
force-pushed
the
epicallan/ad501-move-wallet-restore-logic
branch
from
dd169b6
to
59ecd9e
Compare
|
Why did you open a new PR instead of closing the existing one (#385)? You should at least provide the reference to the old PR in the description, because the old PR has my comments. |
|
@gromakovsky its because you mentioned branch name wasn't conventional. I also noticed implementing your changes was close to a total rewrite of what I had done, so I didn't think it would be a big deal to re-do on a properly named branch. |
| import Ariadne.Wallet.Face (Mnemonic(..)) | ||
|
|
||
| newtype WrongMnemonic = WrongMnemonic Text | ||
| deriving (Eq, Show) |
There was a problem hiding this comment.
| deriving (Eq, Show) | |
| deriving (Eq, Show) |
|
|
||
|
|
||
| data WalletToRestore | ||
| = WalletToRestore |
There was a problem hiding this comment.
I believe that our code style says that constructor should be on the previous line in this case.
| -> PassPhrase | ||
| -> m EncryptedSecretKey | ||
| getKeyFromMnemonic isAriadneMnemonic mnemonicWords pp = | ||
| if |
There was a problem hiding this comment.
Guards can be used here.
| Right (sk, _) -> pure sk | ||
| | otherwise -> throwM $ WrongMnemonic "Unknown mnemonic type" | ||
|
|
||
| -- | reads daedalus wallet keys from file for wallet restoration |
There was a problem hiding this comment.
| -- | reads daedalus wallet keys from file for wallet restoration | |
| -- | Reads daedalus wallet keys from file for wallet restoration |
| Just bip44DerPath -> Just ((toHdAddressId bip44DerPath, addr), utxo) | ||
| {------------------------------------------------------------------------------- | ||
| Utilities | ||
| -------------------------------------------------------------------------------} |
There was a problem hiding this comment.
This block can be deleted safely, I believe. The module is small enough.
| "Failed to decode " <> path <> ": " <> show txt | ||
|
|
||
| data RestoreFrom | ||
| = RestoreFromMnemonic !Mnemonic |
There was a problem hiding this comment.
Hm, now I realized that what I proposed is not exactly what I want. Specifically, I want isAriadneMnemonic logic to still be outside Cardano Kernel. And I see why this type makes it impossible. I propose to change Mnemonic here to Either Mnemonic BackupPhrase.
epicallan
force-pushed
the
epicallan/ad501-move-wallet-restore-logic
branch
2 times, most recently
from
4396063
to
f173e0c
Compare
| getKeyFromMnemonic pp = \case | ||
| Left (Mnemonic mnemonic) -> | ||
| let mnemonicWords = words mnemonic | ||
| seed = mnemonicToSeedNoPassword (unwords $ Unsafe.init mnemonicWords) |
There was a problem hiding this comment.
I think wallet kernel shouldn't know about this Unsafe.init thing, let's do it outside this module.
epicallan
force-pushed
the
epicallan/ad501-move-wallet-restore-logic
branch
from
d639c80
to
845c09f
Compare
Problem: walletRestore and walletRestoreFromFile use a lot of Cardano-related stuff. solution: move walletRestore logic to passive wallet layer.
epicallan
force-pushed
the
epicallan/ad501-move-wallet-restore-logic
branch
from
845c09f
to
979dac7
Compare
Description:
Problem: walletRestore and walletRestoreFromFile use a lot of Cardano-related stuff.
solution: move walletRestore logic to passive wallet layer.
Refrences: #385
YT issue: https://issues.serokell.io/issue/AD-501
Checklist: