HDWallet tests and strict constructor

[dcousens]

This pull request finally restricts the HDWallet constructor to a restrictive form that only accepts two prototypes (unfortunately not 1):

• HDWallet :: BigInteger -> Buffer -> Network, private
• HDWallet :: ECPointFp -> Buffer -> Network, public

It adds tests for these constructors, all the implied behaviour and overall adds tests for all the methods within HDWallet, including getFingerprint, getIdentifier, getAddress and fromSeed*.

Some of these features were tested in the HDWallet test vectors, but now these tests have been separated to better identify functionality, while still maintaining the same level of test coverage (if not more).

HDWallet.getKeyVersion was removed, it was returning the pubKeyHash and has no meaningful representation in the specification, so I saw no need to keep it.

I can't decide whether keeping from/toBuffer (and their *Hex wrappers) is a worthwhile decision, as I see no use case for it. The purpose of the Base58 export functions is that they have error checking and are human readable.

[dcousens]

The coveralls statistics are off because the last PR caused coveralls to think half the codebase was deleted (no idea why...)

[coveralls]

Coverage decreased (-5.2%) when pulling 4cec42a on dcousens:hdwallettests into 1bce662 on bitcoinjs:master.

[dcousens]

There are tests missing for extended public key derivation; I'll be adding
them on (no rebase) before this PR is good to go.
@weilu and I are looking into ways to avoid the boolean param for
toBase58 but still want to be able to get a neutered equivalent of the
HDKey, but avoiding an export/import.
On Jun 1, 2014 4:49 PM, "Coveralls" notifications@github.com wrote:

[image: Coverage Status] https://coveralls.io/builds/822716

Coverage decreased (-5.2%) when pulling 4cec42a
4cec42a
on dcousens:hdwallettests into 1bce662
1bce662
on bitcoinjs:master.

—
Reply to this email directly or view it on GitHub
#203 (comment)
.

[coveralls]

Coverage decreased (-4.93%) when pulling 64e307b on dcousens:hdwallettests into 1bce662 on bitcoinjs:master.

[dcousens]

I still haven't found the best way to neuter a HDNode, but, for now, there is tests showing how it can work at least.

[dcousens]

Good to go. @weilu @abrkn @kyledrake

[coveralls]

Coverage decreased (-5.13%) when pulling 15cc03a on dcousens:hdwallettests into c76d3ce on bitcoinjs:master.

[dcousens]

The failed build is just helloblock's API dying again... @sidazhang could you look into this? It's getting a bit frustrating.

We at least could use some kind of error message if it is our fault. @locksley

[sidazhang]

@dcousens How often does this happen?

https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/test/integration/p2sh.js#L13

Could you add debug: true. That way we have a better trace of what happened. It is a shame that it is showing Error: [object Object] which is not helpful at all. I wonder how we could make the error message show

[sidazhang]

If error was an object, does mocha print the content of the object?

https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/test/integration/p2sh.js#L37

[coveralls]

Coverage decreased (-5.13%) when pulling 56a88b8 on dcousens:hdwallettests into c76d3ce on bitcoinjs:master.

[dcousens]

@sidazhang I have found it rarely happens locally. It seems to be an async problem with the way the test is structured right now. If one request fails before another, it throws all requests thereafter.

[dcousens]

It appears that mocha does not print the contents of the object.

[sidazhang]

Right. But if one request fails, shouldn't the test just stops?

[dcousens]

Yes, I was just explaining in terms of a local test I have, where I was continuing despite the error.

[coveralls]

Coverage decreased (-5.13%) when pulling ee04826 on dcousens:hdwallettests into c76d3ce on bitcoinjs:master.