Address some shortcomings with wallet.js

[robby-d]

I've made two commits that enhance Bitcoin.Wallet in two ways:
#1 - The Wallet constructor now takes a "network" parameter, which can be "Bitcoin" or "BitcoinTest". This allows the wallet to generate addresses for testnet, for instance.
#2 - the Wallet constructor also takes a "derivationMethod" parameter, which allows one to specify whether they want public key child derivation, or private key child derivation (https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#child-key-derivation-functions).

This is important, because (to quote: https://bitcointalk.org/index.php?topic=405179.msg4415254#msg4415254): "One weakness that may not be immediately obvious, is that knowledge of the extended public key + a private key descending from it is equivalent to knowing the extended private key (i.e., every private and public key) in case public derivation is used. This means that extended public keys must be treated more carefully than regular public keys. This is the reason why accounts at the first level of the default wallet layout use private derivation, so a leak of account-specific (or below) private key never risks compromising the master."

Because of the above, a default of 'private' was chosen. Also, I put this on the constructor (instead of a param on generateAddress) because it didn't seem to make sense to allow the setting to change on a per-address basis, for a given wallet.

Comments on either of these commits welcome. I can tweak something if need be.

[kyledrake]

This is great, thanks!

There's a bit of a network name inconsistency going on. Address takes prod or testnet, HDWallet takes Bitcoin or Test. It would be nice to unify the network value, or do something like testnet: true.

Am I understanding it right that you're not actually adding BitcoinTest here? Or is something accepting that as a network variable?

[kyledrake]

@weilu could you look at this PR? I'm not very familiar with wallet.js and I know you were looking into changes here.

[weilu]

@kyledrake good thinking on unifying network names. I like testnet: true

@xnova thanks for the PR. Great stuff =)

[robby-d]

Thanks for accepting the PR guys! I totally agree with you regarding the inconsistencies around testnet between HDWallet, Address, etc. I noticed the exact same thing, but was wary of including it in a PR due to it breaking backwards compatibility. Would love to see this change made.

Also, I'm developing a deterministic web wallet and was using electrum.js (from Carbonwallet) for the HD functionality. HDWallet looks like a much more robust solution (it's BIP32, plus it has unit tests, etc). Would you guys say it's ready for prime time in this regard, or is it "not fully cooked" yet? If the latter, what needs to happen for it to be ready to be used in a production scenario?

[abrkn]

I'd love to start using it in production. Some work is being done on network/versions by Wei, which might be a good idea to complete first.

[robby-d]

BTW, thanks for the work on hdwallet.js, Andreas. The project we will be using it in is Counterwallet, which is a web wallet for Counterparty (see www.counterparty.co ...basically a layer on top of Bitcoin for financial applications). We are paying Sergio Demian-Lerner to perform a security audit of counterpartyd (the python-based reference client) shortly. I was thinking after that, I'd have him audit minimally hdwallet.js and perhaps some critical parts of bitcoinjs-lib.

Question to @kyledrake and @weilu: Has bitcoinjs-lib ever been professionally audited? If so, I'll aim for at least getting Sergio to look at it to some degree. If you have any recommended areas you'd want him to look at, let me know.