Add support for array format in account import and export #74
Conversation
moshthepitt
changed the title
Adds a switch to output the exported private key as a "raw" key that can be used from solana (~/.config/solana/id.json), among other places. Fixes: https://github.com/project-serum/spl-token-wallet/issues/66
moshthepitt
force-pushed
the
raw-key-support
branch
from
e147e5e to
cc5cb3d
Compare
|
Updated the wording to "array format"
|
|
|
||
| const keyOutput = isArrayFormat | ||
| ? `[${[].slice.call(wallet.provider.account.secretKey)}]` | ||
| : bs58.encode(wallet.provider.account.secretKey); |
There was a problem hiding this comment.
To both line 19 and 20, is it worth adding the optional chaining operator e.g. wallet?.provider?.account?.secretKey? I'm new to the Solana codebase, so maybe all of these keys are guaranteed by the ts interfaces, if so then disregard this comment.
There was a problem hiding this comment.
Not sure about this. @armaniferrante thoughts?
There was a problem hiding this comment.
I'm not the expert here, but I would be surprised if any of these fields were null or undefined.
|
Should there also be the "array format" option for importing? If you're not going to add an "array format" option for importing accounts, then for better UX I think there should be some kind of message along the lines of "Developer feature - array format cannot be imported directly" when exporting in the byte array format. The problem I'm foreseeing is non-technical users of the wallet accidentally exporting the array format instead of base58, then not being able to import their wallet later on another device / another browser. |
Rewrite the code to make it clearer. Context: project-serum#74 (review)
Added this notice
|
|
@evanpipta thanks for the comments. I made changes. |
Sweet! After I commented I noticed this open ticket to support different import formats including array: I might work on it, then the notice about the array format could be removed after that's implemented. |
|
As mentioned by @sconybeare, since no tools actually use the base58 private key format directly, the right thing to do here is to replace the base58 import/export with the array format. @moshthepitt do you want to extend this PR to do this? If not @evanpipta that'd be awesome if you wanted to take this. |
|
@evanpipta @armaniferrante please have a look. I have now changed it such that both importing and exporting only work with the array format.
|
moshthepitt
changed the title
|
Nice, I see you merged this, should we still work on #55 to have "legacy support" for accounts exported before this change? Or to make it easier to import keys from other wallets that might export in a different format? |
I think it's a good idea to import/export keys from other (relatively popular) wallets that are in different formats. Any in particular you had in mind? |
|
i cant import my private key with old format！！！！ |
|
@armaniferrante does it make sense to allow importing the old bs58 keys to support existing users at sollet.io and possibly elsewhere? |
|
@moshthepitt yes lets just import them both and automatically detect which format is being used. I see no need to require this elsewhere (other than sollet). |
|
Cool, will send a PR your way @armaniferrante |
This modified version can import private keys formated as: - base58 strings - raw arrays Context: project-serum#74 (comment)
|
@armaniferrante please have a look at #77 |
Changes things such that you can only import/export accounts using the array format used in Solana at
~/.config/solana/id.jsonand other places.