Skip to content

Conversation

@glorat
Copy link

@glorat glorat commented Oct 15, 2014

Per IRC discussion with @dcousens , an integration test to cover Brainwallet signature capability with latest API. Test case covers the previous discussed Ecurve dependency issue, which is now fixed in master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling ec7bd92 on glorat:brainwallet_test into cfcfe59 on bitcoinjs:master.

@glorat
Copy link
Author

glorat commented Oct 15, 2014

Self-appraisal

  • This is the test that would have failed in issue ecdsa.recoverPubKey takes a "curve" parameter that is private #255 but now resolved. (Thanks for resolving!)
  • Apologies this test case doesn't conform to test style. I may get around to cleaning this up in the future if my focus switches back here - or just take this and tidy :)
  • Feedback welcome - I may or may not be able to action
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8819204 on glorat:brainwallet_test into cfcfe59 on bitcoinjs:master.

@dcousens
Copy link
Contributor

I think we definitely need more integration tests; but I'm not sure what example this is covering.
In any case, as mentioned, it does need to adhere to the testing style and use mocha so we can actually run it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) when pulling 05b6849 on glorat:brainwallet_test into cfcfe59 on bitcoinjs:master.

@glorat
Copy link
Author

glorat commented Oct 17, 2014

It specifically covers the previous inability to make use of the recoverPubKey method (since there was previously no way to get the ecurve).

It more generally covers the ability to sign and verify arbitrary messages (in this case a hash from the usual Bitcoin.Message). This may be very trivial but actually I found the use of this library is lacking in examples of how to do trivial small functions... I tried to learn from brainwallet but they were against 0.13. Hence this is the same Brainwallet code example... but against latest code.

And I've tried to conform to Mocha. (Never used it before so hope I've done okay)

@dcousens
Copy link
Contributor

As discussed in IRC, I moved this to a branch which we'll eventually merge.
This particular example is covered by caa43d9.

Thanks @glorat.

@dcousens dcousens closed this Oct 18, 2014
@dcousens dcousens deleted the brainwallet_test branch October 18, 2014 14:20
@dcousens
Copy link
Contributor

Please don't hesitate to add more examples to the above branch using the same style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants