Skip to content
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

Add accounts parameter to wallet_change_seed #775

Merged

Conversation

Projects
5 participants
@PlasmaPower
Copy link
Contributor

commented Apr 2, 2018

Right now this only affects the RPC, not the CLI. I'd like to figure out what to do with the CLI option before merging this. Untested (though it compiles).

Show resolved Hide resolved rai/node/wallet.cpp Outdated
Show resolved Hide resolved rai/node/wallet.hpp Outdated

@PlasmaPower PlasmaPower force-pushed the PlasmaPower:feature/wallet-change-seed-accounts branch from 7e929b5 to be44256 Apr 2, 2018

@renesq

This comment has been minimized.

Copy link

commented Apr 4, 2018

You might want to clarify that this ~~ reverts PR #764~~ (which by the way didn't seem to work for me anyway, and supposedly had some quirks if there were empty frontiers inbetween?) in such a way that the user can now simply specify the number of accounts that will be restored on wallet_change_seed.

@SergiySW

This comment has been minimized.

Copy link
Collaborator

commented Apr 4, 2018

@renesq thinking about casual desktop users, they won't specify count & want all their 10... 20... 50 accounts with nano to be restored (instead of clicking "Create account" 50 times). This auto-search wasn't intended to be used by "pro", but they are too lazy to listen usually

@PlasmaPower

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2018

Yeah the default is auto-restore.

@renesq

This comment has been minimized.

Copy link

commented Apr 4, 2018

Oh, I just realized that the supposedly "deleted" lines were just pushed downwards. So the feature is still there.
But to clairfy once and for all times: Does @SergiySW 's automatic account-adding feature work in: CLI, RPC and Qt wallet? When exactly will it stop? At the first 0-balance frontier?

@SergiySW

This comment has been minimized.

Copy link
Collaborator

commented Apr 4, 2018

QT: originally
CLI: #637
RPC: this
So now everywhere, huh

At last account with frontier (it doesn't depend on balance) or with pending block. With 64+(n/64) accounts check after last restored account.
So if there is gap more than 64+(n/64) between, some account won't be restored.

Actually after b67feea & #766 default count to check can be increased to several thousands

@zhyatt zhyatt added this to the V18.0 milestone Dec 31, 2018

@zhyatt zhyatt added this to Unscheduled in V18 Dec 31, 2018

@zhyatt zhyatt moved this from Unscheduled to CP 1 (2018-01-09) in V18 Jan 2, 2019

@zhyatt zhyatt requested a review from cryptocode Jan 3, 2019

SergiySW added some commits Jan 7, 2019

@cryptocode

This comment has been minimized.

Copy link
Collaborator

commented Jan 7, 2019

@SergiySW What do you think about adding a create_work_count parameter (default 2) instead of hard coding it to 2?

If that makes sense, then maybe change the "accounts" parameter name to "create_count" as well.

@SergiySW

This comment has been minimized.

Copy link
Collaborator

commented Jan 7, 2019

Probably better disable work generation here completely
What about "count" ?

SergiySW added some commits Jan 8, 2019

@zhyatt zhyatt merged commit ba6196c into nanocurrency:master Jan 14, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@zhyatt zhyatt added the enhancement label Jan 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.