[Wallet] Simple HD/BIP32 support by jonasschnelli · Pull Request #7273 · bitcoin/bitcoin

jonasschnelli · 2016-01-02T16:04:13Z

Most simplest HD feature.

Use BIP32/hd key derivation by default for new wallets (m/0'/0')
Only private key derivation is supported (no pubkey-only-wallets for now)
No on the fly generation of private keys (keypools get still refilled, all derived private keys are touching the database, but deterministic)
Internal support for multiple hd keychains (potential allows simple key rotation feature)
Supports encrypted wallets
RPC- and unit-tests included
Currently now way to create a hd keychain for existing wallets (upcoming feature if/once this got merged)

ment as a starting point, have concrete plans to extend this, but don't want to overload this PR

getnewaddress "" true (last parameter true = "show details") returns an object if the new address was hd derived:

{
  "address": "n1EU7TC4YqVYGQYy5eav1APHhS3z3Jrgf4",
  "keypath": "m/0'/230'"
}

(same for getaddressesbyaccount)

jonasschnelli · 2016-01-02T16:13:01Z

The main benefits of this PR:

A wallet.dat backup, regardless of it's age, can be used to recover all private keys.
Basic groundwork for detaching the private-keys from the wallet (allow signing over hardware wallets, etc.)

maflcko · 2016-01-02T18:01:35Z

qa/rpc-tests/hdwallet.py

Nit: Is there a compelling reason to do this?

Encrypting the wallet does stop the process,... i think this is required.

jgarzik · 2016-01-02T18:21:43Z

concept ACK

luke-jr · 2016-01-15T18:00:20Z

src/wallet/walletdb.h

Typo? Achive -> Active?

luke-jr · 2016-01-15T18:12:32Z

Note to test: If encrypting wallet fails for any reason at any late stage, the wallet should retain all unencrypted data.

luke-jr · 2016-01-15T18:18:18Z

src/wallet/wallet.cpp

Return value is ignored.

pointbiz · 2016-01-22T21:10:19Z

Concept ACK

instagibbs · 2016-02-01T15:12:55Z

src/init.cpp

Probably should make this clear it only happens upon first run of the wallet.

Probably should make this clear it only happens upon first run of the wallet.

I agree, -hdseed as startup parameter is somehow not ideal. Not seeding the wallet at first start, would require an additional rpc command (something generatehdwallet, seedwallet, etc.).

Or an additional "bitcoin-wallet" tool that could allow generating and manipulating wallet.dat files.

But because I fear lack of reviewer, I don't want to make this PR more complex for now.

jonasschnelli · 2016-02-22T12:38:59Z

Rebased and fixed reported nits.

instagibbs · 2016-03-11T21:45:20Z

src/wallet/walletdb.cpp

+        return false;
+
+    Erase(std::make_pair(std::string("hdmasterseed"), hash));
+    Erase(std::make_pair(std::string("hdmasterseed"), hash));


double deletion?

- master seeds are kept seperated and can be encrypted

jonasschnelli · 2016-03-14T08:57:30Z

Rebased and fixed some nits.

jonasschnelli added the Wallet label Jan 2, 2016

jonasschnelli added this to the 0.13.0 milestone Jan 2, 2016

maflcko reviewed Jan 2, 2016
View reviewed changes

luke-jr reviewed Jan 15, 2016
View reviewed changes

src/wallet/walletdb.h Outdated

Copy link

Member

luke-jr Jan 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? Achive -> Active?

luke-jr reviewed Jan 15, 2016
View reviewed changes

instagibbs reviewed Feb 1, 2016
View reviewed changes

jonasschnelli force-pushed the 2016/01/hdsimple branch from 42d2a1a to 915b69d Compare February 22, 2016 12:37

instagibbs reviewed Mar 11, 2016
View reviewed changes

jonasschnelli added 3 commits March 14, 2016 09:42

[Wallet] Add hdkeystore

6a65143

- master seeds are kept seperated and can be encrypted

[Wallet] Extend walletdb with some hd related records

b717b42

[Wallet] use HD by default for all new wallets

3494217

jonasschnelli force-pushed the 2016/01/hdsimple branch from 915b69d to 3494217 Compare March 14, 2016 08:56

Conversation

jonasschnelli commented Jan 2, 2016

Uh oh!

jonasschnelli commented Jan 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgarzik commented Jan 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luke-jr commented Jan 15, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pointbiz commented Jan 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Feb 22, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Mar 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants