[Wallet] Ensure <0.13 clients can't open HD wallets #8343
Conversation
|
Concept ACK |
|
quickly tested ACK 51a4ee8 |
| @@ -78,7 +78,8 @@ enum WalletFeature | |||
| FEATURE_WALLETCRYPT = 40000, // wallet encryption | |||
| FEATURE_COMPRPUBKEY = 60000, // compressed public keys | |||
|
|
|||
| FEATURE_LATEST = 60000 | |||
| FEATURE_HD = 129900, // Hierarchical key derivation after BIP32 (HD Wallet) | |||
There was a problem hiding this comment.
Strictly speaking not all 12.99's support HD, so we may want to adjust to 130000 after the branch off.
There was a problem hiding this comment.
This isn't derived from the actual version, is it? It's just a marker constant, the exact value doesn't matter as long as it is higher than all other FEATURE_*. So if you want it 130000, just make it 130000 already.
Bah, it is checked against the CLIENT_VERSION, I was wrong.
Ok, let's do this after the release branch.
There was a problem hiding this comment.
Yes. We can try to merge this once we split of the masterbranch and had increase the version.
There was a problem hiding this comment.
Yes using the client version here is not very nice, if we had a dedicated wallet version constant this problem could have been avoided, as it could have been bumped at the moment HD support was introduced instead of arbitrary delineation based on release-process constraints.
But for now that's the best solution.
There was a problem hiding this comment.
@jonasschnelli Could you already bump this so we don't lose time later? (In case you are not around when the branch off happens)
There was a problem hiding this comment.
@MarcoFalke
If I bump this to 130000 now, HD would no longer work on current master IMO.
For <0.13 incompatibility, we need to stick to the current check based on CLIENT_VERSION.
https://github.com/bitcoin/bitcoin/blob/master/src/wallet/walletdb.cpp#L637
Once we change the wallet database format, we could introduce a set of strings about the features the wallet database requires/supports.
There was a problem hiding this comment.
If I bump this to 130000 now, HD would no longer work on current master IMO.
True, but I think the plan was to merge this after the bump to 13.99 into master?
No strong opinion, though.
There was a problem hiding this comment.
Version have been bumped, you can update this now
Fill in the header, and move items to the appropriate part of the release notes structure.
|
ut ACK 51a4ee8 |
| @@ -197,6 +197,8 @@ You can't disable HD key generation once you have created a HD wallet. | |||
|
|
|||
| There is no distinction between internal (change) and external keys. | |||
|
|
|||
| HD wallets are incompatible with older versions of Bitcoin Core. | |||
There was a problem hiding this comment.
I think you need to open a separate pull against .13 now that the release notes were moved.
jonasschnelli
force-pushed
the
2016/07/hd_minversion
branch
from
51a4ee8
to
8c56d30
Compare
jonasschnelli
force-pushed
the
2016/07/hd_minversion
branch
from
8c56d30
to
3b38a6a
Compare
|
Sorry. Have to close and reopen (new PR) this because use this local branch for #8366. |
|
Was a replacement PR opened? |
In current master, you could run a 0.13 HD wallet in a non HD compatible 0.12 client, resulting in a mix of HD and non HD key. You could even refill the wallets keypool running a 0.12 client and use it on 0.13 which would lead to a
getnewaddresscall in 0.13 responding a non HD address.This PR ensures that HD wallets will at least require version 0.12.99.
This only affects newly created wallets and only if the user had not passed
-usehd=0(old <0.13 created wallets will still run on <0.13 clients once they where opened in 0.13).