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

Key origin metadata, with HD wallet support #8471

Closed
wants to merge 3 commits into from

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented Aug 6, 2016

This upgrades #5916/#8132 to support HD wallets by adding a map<string,string> at the end of CKeyMetadata which can be used to store future-proof data, similar to CWalletTx's mapValue.

@fanquake
Copy link
Member

fanquake commented Aug 8, 2016

This needs a rebase.

READWRITE(this->nVersion);
nVersion = this->nVersion;
READWRITE(nCreateTime);
if (this->nVersion >= VERSION_WITH_HDDATA)
{
// Core/Knots 0.13+
Copy link
Member

Choose a reason for hiding this comment

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

Knots ?

@jonasschnelli
Copy link
Contributor

Needs rebase.

@luke-jr
Copy link
Member Author

luke-jr commented Aug 9, 2016

Rebased, and logic re-factored to deal with the optional wallet version bump for HD. Not sure if this is the best approach...

jonasschnelli and others added 3 commits November 16, 2016 13:23
An encrypted wallet can still hold keys which where created when the wallet was unencrypted.

This PR will add a 8bit-flags-int to the CKeyMetadata class.

`listreceivedbyaddress` will report whether the key was generated within a enctypted wallet or if it was imported throught `importprivkey`
@TheBlueMatt
Copy link
Contributor

Is this still relevant with the default-added hdKeypath metadata when we generate hd keys now?

@luke-jr
Copy link
Member Author

luke-jr commented Feb 22, 2018

I don't see why it wouldn't be - the HD keypath is kinda unrelated to this.

@@ -81,9 +81,12 @@ enum WalletFeature

FEATURE_WALLETCRYPT = 40000, // wallet encryption
FEATURE_COMPRPUBKEY = 60000, // compressed public keys
FEATURE_KEYFLAGS = 70000, // key metadata flags for storing informations like key origin
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo found by codespell: informations

@meshcollider
Copy link
Contributor

meshcollider commented Nov 16, 2018

Concept ACK but I'm not convinced about this approach. Seems like the keyflags and the origin metadata are a bit mixed here, my impression of the flags is that they were a more general thing (although perhaps unnecessary with the metadata map now). Why not just store origin in the metadata without using the flags (more than one origin isn't output by the RPC here anyway). IMO this also makes the wallet features messy, I really don't see by we need flags, meta, and hdmeta

@maflcko
Copy link
Member

maflcko commented Mar 5, 2019

There hasn't been much activity lately and the patch still needs rebase, so I am closing this for now. Please let me know when you want to continue working on this, so the pull request can be re-opened.

@Sjors
Copy link
Member

Sjors commented Mar 5, 2019

Key origin info has been added in #14021 (which upgrades existing HD wallets to add it).

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants