Skip to content

Conversation

@dcousens
Copy link
Contributor

This pull request adds BIP32 style neutering of nodes by re-creating a node with all the private key information stripped.
This is useful for when you want to distinctly store or transport a HDnode without exporting it publicly then re-importing it as can be seen in af47371.

@dcousens
Copy link
Contributor Author

There are arguments for and against allowing the isPrivate flag on toBase58.
If we were going for a removal of the flag in 2.0.0, then an assert should be included for the transition IMHO.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling af47371 on dcousens:hdnode into a8c6f52 on bitcoinjs:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 903f412 on dcousens:hdnode into 04bcbad on bitcoinjs:master.

@weilu
Copy link
Contributor

weilu commented Jul 30, 2014

What we currently have:

  • private hdnode exports private: hd.toBase58()
  • public hdnode exports public: hd.toBase58()
  • private hdnode exports public: hd.toBase58(false)
  • public hdnode exports private: hd.toBase58(true) //error

With this PR + we drop the isPrivate flag on toBase58, we will have:

  • private hdnode exports private: hd.toBase58()
  • public hdnode exports public: hd.toBase58()
  • private hdnode exports public: hd.neutered().toBase58()
  • public hdnode exports private: // impossible because API is such

I agree that the latter is better because it gets rid of a boolean arg and elegantly makes the error case impossible. My vote goes to deprecating the boolean flag on toBase58 with this PR, bump minor version after this is merged. With 2.0.0, we remove the boolean arg, until then the error path is still possible.

@kyledrake
Copy link
Contributor

+1 with @weilu's deprecation proposal.

@dcousens
Copy link
Contributor Author

Rebased on HEAD and added deprecation warning.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 6b42949 on dcousens:hdnode into 5888ca5 on bitcoinjs:master.

weilu added a commit that referenced this pull request Jul 30, 2014
@weilu weilu merged commit 09455a6 into bitcoinjs:master Jul 30, 2014
@dcousens dcousens deleted the hdnode branch July 30, 2014 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants