Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation Fixes #12

Merged
merged 2 commits into from
Jun 7, 2014
Merged

Validation Fixes #12

merged 2 commits into from
Jun 7, 2014

Conversation

dcousens
Copy link
Contributor

@dcousens dcousens commented Jun 7, 2014

This pull request makes some mild progress towards improving the quality of the point validation functions.
Most of the important issues have been discussed on IRC, but some key points where things will need to change in the future (but are not implemented in this pull request) are:

  • Change params.curve.params to be curve.params, that is, return a curve object from names.js, not a parameters object.
  • Move all the group operations to this new Curve object and remove the Point object completely.
    It is just bloat, and there should never be a case where two points are not in the same curve context and group operations performed. So it is useless storage and a possibility for error.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.82%) when pulling 29dde30 on dcousens:validatepass into 2721aba on cryptocoinjs:master.

jprichardson added a commit that referenced this pull request Jun 7, 2014
@jprichardson jprichardson merged commit 0c66274 into cryptocoinjs:master Jun 7, 2014
@dcousens dcousens deleted the validatepass branch June 7, 2014 07:14
@jprichardson
Copy link
Member

@dcousens Practically, this seems pretty good. Good enough? Is this an edge case that you think will really matter or probabilistically speaking, ever be a concern?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants