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

Reject decimal input #91

Closed
wants to merge 3 commits into from
Closed

Reject decimal input #91

wants to merge 3 commits into from

Conversation

axic
Copy link
Contributor

@axic axic commented Dec 19, 2015

As discussed in #90

@indutny
Copy link
Owner

indutny commented Dec 19, 2015

Does it affect benchmarks?

@axic
Copy link
Contributor Author

axic commented Jan 24, 2016

Seems like my comments didn't get posted here back in December. Unfortunately it does affect benchmarks so I was thinking about others ways to do it. Perhaps the best would be having two input functions: a safe (for "user input") and an unsafe one (for speed, e.g. the current implementation).

Any ideas about an API for the above?

@indutny
Copy link
Owner

indutny commented Jan 24, 2016

I'm not sure about it myself. Perhaps, BN.checkedFrom(string, 'hex')?

@axic axic mentioned this pull request Feb 1, 2016
@indutny
Copy link
Owner

indutny commented Feb 9, 2016

Preemptively added semver-major tag per your request.

@fanatid
Copy link
Collaborator

fanatid commented Mar 28, 2016

Why not land this PR in 4.x?
@axic I think only safe should be here

@indutny
Copy link
Owner

indutny commented Nov 29, 2017

Landed test in 9173bc2, the issue was fixed in different commit. Thank you!

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

Successfully merging this pull request may close these issues.

3 participants