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

Integrate new wallet address manager package #147

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@davecgh
Member

davecgh commented Oct 29, 2014

This pull request converts the wallet to use the new secure hierarchical deterministic wallet address manager package.

The following is an overview of modified functionality:

  • The wallet must now be created before starting the executable
  • A new flag --create has been added to create the new wallet using wizard style question and answer prompts
  • Starting the process without an existing wallet will now display a message to run it with --create
  • Providing the --create flag with an existing wallet will simply show an error and return
  • When an existing legacy keystore exists, the --create flag will prompt you for the old private passphrase and automatic convert all old addresses to the new wallet being created

Github seems to have incorrectly closed the previous pull request #138.

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch 2 times, most recently from a715835 to 8f1cd24 Nov 10, 2014

@davecgh

This comment has been minimized.

Member

davecgh commented Nov 10, 2014

I've rebased this on top of #153.

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch 2 times, most recently from 77fd5f1 to d851527 Nov 10, 2014

@davecgh

This comment has been minimized.

Member

davecgh commented Nov 10, 2014

Rebased and updated for changes in #152.

@davecgh

This comment has been minimized.

Member

davecgh commented Nov 11, 2014

For anyone wondering about the status of this, it is being maintained until the current txstore is converted to use the new walletdb namespacing. However, it has been fully reviewed and is ready for the additional modifications that will be needed by the forthcoming txstore changes. Once those changes are in and reviewed, this pull request will land and thereby make the sweeping changes brought be all these other packages available in btcwallet.

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch 8 times, most recently from ca86ac1 to 2d8a8be Nov 11, 2014

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch from 2d8a8be to 4308841 Dec 11, 2014

@davecgh

This comment has been minimized.

Member

davecgh commented Dec 11, 2014

I've rebased this to the latest master and updated all go subrepo import paths for their new locations.

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch from 08f2a34 to bbeea62 Dec 16, 2014

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch 4 times, most recently from 6754fc5 to 9f9bd9c Jan 15, 2015

@davecgh

This comment has been minimized.

Member

davecgh commented Jan 17, 2015

This has been updated to include all of the new paths and rebased accordingly.

@jrick

This comment has been minimized.

Member

jrick commented Jan 23, 2015

Because accounts are created with 0 addresses, but manager.LastExternalAddress returns the next address without error (which is not yet watched or managed), the getaccountaddress RPC is currently broken. To fix, we will need to add Num{Internal,External}Addresses methods to the manager and check for 0 external addresses in the GetAccountAddress handler.

@davecgh

This comment has been minimized.

Member

davecgh commented Jan 23, 2015

Another option is to create the first address on the internal and external branches when an account is created.

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch from 9f9bd9c to 92122f8 Jan 30, 2015

@davecgh

This comment has been minimized.

Member

davecgh commented Jan 30, 2015

This has been updated for the btcscript -> btcd/txscript move and rebased accordingly.

davecgh and others added some commits Oct 29, 2014

Switch to new waddrmgr package
This commit converts the wallet to use the new secure hierarchical
deterministic wallet address manager package as well as the walletdb
package.

The following is an overview of modified functionality:

- The wallet must now be created before starting the executable
- A new flag --create has been added to create the new wallet using wizard
  style question and answer prompts
- Starting the process without an existing wallet will instruct now
  display a message to run it with --create
- Providing the --create flag with an existing wallet will simply show an
  error and return

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch from 92122f8 to 3e7cba0 Feb 1, 2015

Return the memory after scrypt to the OS.
Previously a runtime.GC was being invoked which forced it to release the
memory as far as the garbage collector is concerned, but the memory was
not released back to the OS immediatley.  This modification allows the
memory to be released immedately since it won't be needed again until the
next wallet unlock.

@davecgh davecgh force-pushed the davecgh:davec_intwaddrmgr branch from 3e7cba0 to 51e3d5f Feb 1, 2015

@jrick

This comment has been minimized.

Member

jrick commented Feb 5, 2015

I've pushed a intwaddrmgr branch to this repo. The keystore->waddrmgr replacement and integration here is in good shape but there are several other components (like txstore -> wtxmgr) that we want to complete before pushing this to master. PRs which depend on intwaddr should be rebased to this branch and submitted as a intwaddrmgr PR (not a PR to master).

#154
#155
#183
#185

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment