-
Notifications
You must be signed in to change notification settings - Fork 21
[Draft] Add create wallet handler, cmd, and API #492
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
[Draft] Add create wallet handler, cmd, and API #492
Conversation
|
you shouldn't need to RPC to lotus to generate keypairs, it's just a few calls to go-address/go-crypto, take a look in https://github.com/filecoin-project/lotus/blob/master/chain/wallet/key/key.go RPCing to a node only makes sense if the user is running their own, public nodes (or SPs) will not give you signing and other wallet-related functionality is already handled internally by singularity |
|
one thing to note is that after creating the keypair (by whatever means) the address will need to be initialized on-chain by sending a message referencing it, in most cases funding it with FIL or datacap this needs to happen from a different, already existing actor for obvious reasons (that is to say the state check here will likely fail after creation, however this is true even if the wallet is created via lotus RPC as no messages reference it at that point and therefore it won't be in chainstate -- my guess is that this is likely why wallet creation was not originally implemented inside singularity) |
|
hi @zachfedor I would suggest rebasing on #467 to get passing go-check |
- Add init function to create full current schema on brand new clean databases and run missing migrations on any databases using old auto-migrate strategy with existing data. - Add CLI commands to migrate up, down, or to a specified version by ID - Add utility functions to get list of migration IDs ran on current database, check if migration has run by ID, etc.
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.
@zachfedor I pulled this and tested and the wallet addresses were created successfully and saved in the DB!
I can also list them using singularity wallet list command.
To make the CLI documentation clear and user-friendly, you should explicitly document:
- What the [type] argument represents
- What values are supported
- What the default behavior is if not provided
My tests
jeffersonsankara@Jeffersons-MacBook-Pro singularity % ./singularity wallet create --help
NAME:
singularity wallet create - Create a new wallet
USAGE:
singularity wallet create [command options] [type]
OPTIONS:
--help, -h show help
jeffersonsankara@Jeffersons-MacBook-Pro singularity % ./singularity wallet create
ID Address
f16wmhtwtu4aib5xazb3fwxvewue7b4dmyansznta f16wmhtwtu4aib5xazb3fwxvewue7b4dmyansznta
jeffersonsankara@Jeffersons-MacBook-Pro singularity % ./singularity wallet create secp256k1
ID Address
f1uwurfy2rdfsey4dwdnuk4utwm62fcxkehdjp2ki f1uwurfy2rdfsey4dwdnuk4utwm62fcxkehdjp2ki
jeffersonsankara@Jeffersons-MacBook-Pro singularity % ./singularity wallet create bls
ID Address
f3qya2n6wg2wgu5nrhyix7serr2llo64mbtgqtedxrywa6xanfdsalmkk4hotxvhw4zct2y4dqgc3quugbduaa f3qya2n6wg2wgu5nrhyix7serr2llo64mbtgqtedxrywa6xanfdsalmkk4hotxvhw4zct2y4dqgc3quugbduaa
jeffersonsankara@Jeffersons-MacBook-Pro singularity %
jeffersonsankara@Jeffersons-MacBook-Pro singularity % ./singularity wallet list
ID Address
f03539246 f12syf7zd3lfsv43aj2kb454ymaqw7debhumjnbqa
f16wmhtwtu4aib5xazb3fwxvewue7b4dmyansznta f16wmhtwtu4aib5xazb3fwxvewue7b4dmyansznta
f1uwurfy2rdfsey4dwdnuk4utwm62fcxkehdjp2ki f1uwurfy2rdfsey4dwdnuk4utwm62fcxkehdjp2ki
f3qya2n6wg2wgu5nrhyix7serr2llo64mbtgqtedxrywa6xanfdsalmkk4hotxvhw4zct2y4dqgc3quugbduaa f3qya2n6wg2wgu5nrhyix7serr2llo64mbtgqtedxrywa6xanfdsalmkk4hotxvhw4zct2y4dqgc3quugbduaa
jeffersonsankara@Jeffersons-MacBook-Pro singularity %
jeffersonsankara@Jeffersons-MacBook-Pro singularity % ./singularity wallet remove --really-do-it f16wmhtwtu4aib5xazb3fwxvewue7b4dmyansznta
jeffersonsankara@Jeffersons-MacBook-Pro singularity % singularity wallet list
ID Address
f03539246 f12syf7zd3lfsv43aj2kb454ymaqw7debhumjnbqa
f1uwurfy2rdfsey4dwdnuk4utwm62fcxkehdjp2ki f1uwurfy2rdfsey4dwdnuk4utwm62fcxkehdjp2ki
f3qya2n6wg2wgu5nrhyix7serr2llo64mbtgqtedxrywa6xanfdsalmkk4hotxvhw4zct2y4dqgc3quugbduaa f3qya2n6wg2wgu5nrhyix7serr2llo64mbtgqtedxrywa6xanfdsalmkk4hotxvhw4zct2y4dqgc3quugbduaa
jeffersonsankara@Jeffersons-MacBook-Pro singularity %
Unit tests
I ran this and the wallet create logic works as expected under the SQLite setup used in unit tests. It failed for Postgres and I was expecting that based on our last conversation.
Sample description to add to documentation:
Creates a new Filecoin wallet.
You can optionally specify the wallet type to generate. Supported types:
secp256k1 (default)
bls
If no type is specified, a secp256k1 wallet will be created by default.Examples:
singularity wallet create
singularity wallet create secp256k1
singularity wallet create bls
|
@zachfedor do you have preferences for release workflow (merge feature branches to main and tag release, create release branch, something else?) additionally could you please PR with the new migration approach since it may affect how I'm handling min-piece-size and piece-type fields |
|
@Sankara-Jefferson and @parkan I'm closing this to reopen a new PR rebased on #467 as mentioned above. New PR is #496. It's got all the notes for migrations for you. And I'm flexible on release workflow, @parkan. Looks like you've been merging to main and tagging releases. That's totally fine by me! |
This PR adds the ability to create a new Filecoin wallet via CLI or API endpoint using the Lotus RPC client, saving the new wallet address and private key to the database as if it was imported.
Fixes #469