x

[abrkn]

x

[empoweoqwj]

I re-implemented the BIP0032 wallet in vbuterin/bitcoinjs-lib and would like someone to review my work:

Source code: https://github.com/justcoin/bitcoinjs-lib/blob/master/src/hdwallet.js
Tests: https://github.com/justcoin/bitcoinjs-lib/blob/master/test/hdwallet.js

Is it customary to tip for code review?

If someone spends their time properly reviewing your code then I would certainly think a tip would be in order

[empoweoqwj]

Will pay a 0.1 BTC bounty

How many lines of js are we talking?

[pocesar]

You are missing tests for transaction.js, script.js and wallet.js and not taking in consideration third party modules (like jsbn and crypto-js).

You got some defined functions that arent being used (therefore tested if working), for example in index.js, rotl, rotr and endian.

You need to make tests cover exceptions being thrown (haven't found any in any file). since you are using qunit, assert.throws should do it.

On the derive of HDWallet, you are checking for this.priv, but the pub is not being covered (in case priv is false).

None of the base64 functions from convert.js are being covered.

This is what I got so far.

Btw, you should enable issues in your repo (since when you fork on github, it's not enabled by default)

[empoweoqwj]

You are missing tests for transaction.js, script.js and wallet.js and not taking in consideration third party modules (like jsbn and crypto-js).

You got some defined functions that arent being used (therefore tested if working), for example in index.js, rotl, rotr and endian.

You need to make tests cover exceptions being thrown (haven't found any in any file). since you are using qunit, assert.throws should do it.

On the derive of HDWallet, you are checking for this.priv, but the pub is not being covered (in case priv is false).

None of the base64 functions from convert.js are being covered.

This is what I got so far.

Btw, you should enable issues in your repo (since when you fork on github, it's not enabled by default)

Ouch - I think you should pay pocesar immediately