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.

Sign up for GitHub

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

Jump to bottom

typo #312

Closed
wants to merge 1 commit into from
Closed

typo #312

wants to merge 1 commit into from

Conversation

thehobbit85
Copy link
Contributor

@thehobbit85 thehobbit85 commented Sep 12, 2017 •
edited by bucko13
Loading

so because of a typo, the native-node version of Secp256k1 was never in use

@bucko13 bucko13 added the quick Can be fixed quickly, code change less than 10 lines label Oct 12, 2017
@bucko13 bucko13 requested review from chjj and BluSyn October 16, 2017 18:57
@chjj
Copy link
Member

chjj commented Oct 18, 2017

That's not a typo. /bindings is used because secp256k1-node has its own fallback now and we don't want to be tricked by it when we fail a build.

It's likely your build failed and you're just falling back to secp256k1-node's JS implementation when you change that. Try a fresh npm install and see if the build works.

@chjj
Copy link
Member

chjj commented Oct 19, 2017

Closing. This would do better as a separate issue for secp256k1-node build issues.

@chjj chjj closed this Oct 19, 2017
@thehobbit85
Copy link
Contributor Author

I still have the same issue that I only get the fallback js version.
I will check the secp256k1-node build.

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

@chjj chjj Awaiting requested review from chjj

@BluSyn BluSyn Awaiting requested review from BluSyn

Assignees
No one assigned
Labels
quick Can be fixed quickly, code change less than 10 lines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thehobbit85 @chjj @bucko13