Skip to content

test: include derivation test when private key has leading zeros#673

Merged
dcousens merged 1 commit into
bitcoinjs:masterfrom
braydonf:hd-zeros
Oct 21, 2016
Merged

test: include derivation test when private key has leading zeros#673
dcousens merged 1 commit into
bitcoinjs:masterfrom
braydonf:hd-zeros

Conversation

@braydonf

@braydonf braydonf commented Oct 20, 2016

Copy link
Copy Markdown
Contributor

@dcousens

Copy link
Copy Markdown
Contributor

Probably would add this as a test fixture instead, but sure? Was there any reason why this might not work?

@braydonf

Copy link
Copy Markdown
Contributor Author

All of the tests will currently pass if the 32 is removed from line
https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/hdnode.js#L217, and changed into:

this.keyPair.d.toBuffer().copy(data, 1)

Instead of:

this.keyPair.d.toBuffer(32).copy(data, 1)

So this test covers that case.

There may be one more at, that isn't covered: https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/hdnode.js#L193

Most of the fixtures have a seed, the seed is unknown here.

dcousens pushed a commit that referenced this pull request Oct 21, 2016
dcousens pushed a commit that referenced this pull request Oct 21, 2016
@dcousens

dcousens commented Oct 21, 2016

Copy link
Copy Markdown
Contributor

@braydonf just so we didn't need to change our testing fixtures dramatically with more branching (though, my refactoring did that anyway), I generated a fixture that had 5 leading leading zeros here

@dcousens dcousens merged commit 0783180 into bitcoinjs:master Oct 21, 2016
@dcousens

Copy link
Copy Markdown
Contributor

Whatever, we'll merge both

dcousens pushed a commit that referenced this pull request Oct 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants