bitcoinjs / tiny-secp256k1

A tiny secp256k1 native/JS wrapper
MIT License
92 stars 55 forks source link

Doesn't work on NodeJS 12 #38

Closed NatoBoram closed 5 years ago

NatoBoram commented 5 years ago

I specified NodeJS Stable on Travis CI, but yesterday the pipelines broke on tiny-secp256k1. More context on travis-ci.org/GetEpona/Epona-js.

[4/4] Building fresh packages...
error /home/travis/build/GetEpona/Epona-js/node_modules/tiny-secp256k1: Command failed.
Exit code: 1
Command: node-gyp rebuild
Arguments: 
Directory: /home/travis/build/GetEpona/Epona-js/node_modules/tiny-secp256k1
Output:
gyp info it worked if it ends with ok
gyp info using node-gyp@3.8.0
gyp info using node@12.0.0 | linux | x64
gyp http GET https://nodejs.org/download/release/v12.0.0/node-v12.0.0-headers.tar.gz
gyp http 200 https://nodejs.org/download/release/v12.0.0/node-v12.0.0-headers.tar.gz
gyp http GET https://nodejs.org/download/release/v12.0.0/SHASUMS256.txt
gyp http 200 https://nodejs.org/download/release/v12.0.0/SHASUMS256.txt
gyp info spawn /opt/pyenv/shims/python2
gyp info spawn args [
gyp info spawn args   '/home/travis/.nvm/versions/node/v12.0.0/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/home/travis/build/GetEpona/Epona-js/node_modules/tiny-secp256k1/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/travis/.nvm/versions/node/v12.0.0/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/travis/.node-gyp/12.0.0/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/home/travis/.node-gyp/12.0.0',
gyp info spawn args   '-Dnode_gyp_dir=/home/travis/.nvm/versions/node/v12.0.0/lib/node_modules/npm/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/home/travis/.node-gyp/12.0.0/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/home/travis/build/GetEpona/Epona-js/node_modules/tiny-secp256k1',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn make
gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
make: Entering directory '/home/travis/build/GetEpona/Epona-js/node_modules/tiny-secp256k1/build'
  CXX(target) Release/obj.target/secp256k1/native/addon.o
../native/addon.cpp: In function ‘Nan::NAN_METHOD_RETURN_TYPE ecdsaVerify(Nan::NAN_METHOD_ARGS_TYPE)’:
../native/addon.cpp:320:34: error: no matching function for call to ‘v8::Value::BooleanValue()’
   strict = info[3]->BooleanValue();
                                  ^
In file included from /home/travis/.node-gyp/12.0.0/include/node/node.h:63:0,
                 from ../../nan/nan.h:53,
                 from ../native/addon.cpp:4:
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2559:8: note: candidate: bool v8::Value::BooleanValue(v8::Isolate*) const
   bool BooleanValue(Isolate* isolate) const;
        ^
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2559:8: note:   candidate expects 1 argument, 0 provided
In file included from /home/travis/.node-gyp/12.0.0/include/node/v8-internal.h:14:0,
                 from /home/travis/.node-gyp/12.0.0/include/node/v8.h:25,
                 from /home/travis/.node-gyp/12.0.0/include/node/node.h:63,
                 from ../../nan/nan.h:53,
                 from ../native/addon.cpp:4:
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2562:51: note: candidate: v8::Maybe<bool> v8::Value::BooleanValue(v8::Local<v8::Context>) const
                 V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                   ^
/home/travis/.node-gyp/12.0.0/include/node/v8config.h:307:3: note: in definition of macro ‘V8_DEPRECATED’
   declarator __attribute__((deprecated(message)))
   ^
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2562:51: note:   candidate expects 1 argument, 0 provided
                 V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                   ^
/home/travis/.node-gyp/12.0.0/include/node/v8config.h:307:3: note: in definition of macro ‘V8_DEPRECATED’
   declarator __attribute__((deprecated(message)))
   ^
../native/addon.cpp: In instantiation of ‘unsigned int {anonymous}::assumeCompression(const I&, const A&) [with long unsigned int index = 2ul; I = Nan::FunctionCallbackInfo<v8::Value>; A = v8::Local<v8::Object>]’:
../native/addon.cpp:142:50:   required from here
../native/addon.cpp:80:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (info.Length() <= index) return __isPointCompressed(p) ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED;
                     ^
../native/addon.cpp:82:38: error: no matching function for call to ‘v8::Value::BooleanValue()’
   return info[index]->BooleanValue() ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED;
                                      ^
In file included from /home/travis/.node-gyp/12.0.0/include/node/node.h:63:0,
                 from ../../nan/nan.h:53,
                 from ../native/addon.cpp:4:
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2559:8: note: candidate: bool v8::Value::BooleanValue(v8::Isolate*) const
   bool BooleanValue(Isolate* isolate) const;
        ^
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2559:8: note:   candidate expects 1 argument, 0 provided
In file included from /home/travis/.node-gyp/12.0.0/include/node/v8-internal.h:14:0,
                 from /home/travis/.node-gyp/12.0.0/include/node/v8.h:25,
                 from /home/travis/.node-gyp/12.0.0/include/node/node.h:63,
                 from ../../nan/nan.h:53,
                 from ../native/addon.cpp:4:
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2562:51: note: candidate: v8::Maybe<bool> v8::Value::BooleanValue(v8::Local<v8::Context>) const
                 V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                   ^
/home/travis/.node-gyp/12.0.0/include/node/v8config.h:307:3: note: in definition of macro ‘V8_DEPRECATED’
   declarator __attribute__((deprecated(message)))
   ^
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2562:51: note:   candidate expects 1 argument, 0 provided
                 V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                   ^
/home/travis/.node-gyp/12.0.0/include/node/v8config.h:307:3: note: in definition of macro ‘V8_DEPRECATED’
   declarator __attribute__((deprecated(message)))
   ^
../native/addon.cpp: In instantiation of ‘unsigned int {anonymous}::assumeCompression(const I&, const A&) [with long unsigned int index = 1ul; I = Nan::FunctionCallbackInfo<v8::Value>; A = v8::Local<v8::Object>]’:
../native/addon.cpp:174:49:   required from here
../native/addon.cpp:80:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (info.Length() <= index) return __isPointCompressed(p) ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED;
                     ^
../native/addon.cpp:82:38: error: no matching function for call to ‘v8::Value::BooleanValue()’
   return info[index]->BooleanValue() ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED;
                                      ^
In file included from /home/travis/.node-gyp/12.0.0/include/node/node.h:63:0,
                 from ../../nan/nan.h:53,
                 from ../native/addon.cpp:4:
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2559:8: note: candidate: bool v8::Value::BooleanValue(v8::Isolate*) const
   bool BooleanValue(Isolate* isolate) const;
        ^
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2559:8: note:   candidate expects 1 argument, 0 provided
In file included from /home/travis/.node-gyp/12.0.0/include/node/v8-internal.h:14:0,
                 from /home/travis/.node-gyp/12.0.0/include/node/v8.h:25,
                 from /home/travis/.node-gyp/12.0.0/include/node/node.h:63,
                 from ../../nan/nan.h:53,
                 from ../native/addon.cpp:4:
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2562:51: note: candidate: v8::Maybe<bool> v8::Value::BooleanValue(v8::Local<v8::Context>) const
                 V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                   ^
/home/travis/.node-gyp/12.0.0/include/node/v8config.h:307:3: note: in definition of macro ‘V8_DEPRECATED’
   declarator __attribute__((deprecated(message)))
   ^
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2562:51: note:   candidate expects 1 argument, 0 provided
                 V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                   ^
/home/travis/.node-gyp/12.0.0/include/node/v8config.h:307:3: note: in definition of macro ‘V8_DEPRECATED’
   declarator __attribute__((deprecated(message)))
   ^
../native/addon.cpp: In instantiation of ‘unsigned int {anonymous}::assumeCompression(const I&) [with long unsigned int index = 1ul; I = Nan::FunctionCallbackInfo<v8::Value>]’:
../native/addon.cpp:189:46:   required from here
../native/addon.cpp:87:21: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (info.Length() <= index) return SECP256K1_EC_COMPRESSED;
                     ^
../native/addon.cpp:89:38: error: no matching function for call to ‘v8::Value::BooleanValue()’
   return info[index]->BooleanValue() ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED;
                                      ^
In file included from /home/travis/.node-gyp/12.0.0/include/node/node.h:63:0,
                 from ../../nan/nan.h:53,
                 from ../native/addon.cpp:4:
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2559:8: note: candidate: bool v8::Value::BooleanValue(v8::Isolate*) const
   bool BooleanValue(Isolate* isolate) const;
        ^
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2559:8: note:   candidate expects 1 argument, 0 provided
In file included from /home/travis/.node-gyp/12.0.0/include/node/v8-internal.h:14:0,
                 from /home/travis/.node-gyp/12.0.0/include/node/v8.h:25,
                 from /home/travis/.node-gyp/12.0.0/include/node/node.h:63,
                 from ../../nan/nan.h:53,
                 from ../native/addon.cpp:4:
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2562:51: note: candidate: v8::Maybe<bool> v8::Value::BooleanValue(v8::Local<v8::Context>) const
                 V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                   ^
/home/travis/.node-gyp/12.0.0/include/node/v8config.h:307:3: note: in definition of macro ‘V8_DEPRECATED’
   declarator __attribute__((deprecated(message)))
   ^
/home/travis/.node-gyp/12.0.0/include/node/v8.h:2562:51: note:   candidate expects 1 argument, 0 provided
                 V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
                                                   ^
/home/travis/.node-gyp/12.0.0/include/node/v8config.h:307:3: note: in definition of macro ‘V8_DEPRECATED’
   declarator __attribute__((deprecated(message)))
   ^
../native/addon.cpp: In function ‘Nan::NAN_METHOD_RETURN_TYPE eccPrivateSub(Nan::NAN_METHOD_ARGS_TYPE)’:
../native/addon.cpp:240:53: warning: ignoring return value of ‘int secp256k1_ec_privkey_negate(const secp256k1_context*, unsigned char*)’, declared with attribute warn_unused_result [-Wunused-result]
  secp256k1_ec_privkey_negate(context, tweak_negated); // returns 1 always
                                                     ^
secp256k1.target.mk:149: recipe for target 'Release/obj.target/secp256k1/native/addon.o' failed
make: Leaving directory '/home/travis/build/GetEpona/Epona-js/node_modules/tiny-secp256k1/build'
make: *** [Release/obj.target/secp256k1/native/addon.o] Error 1
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/home/travis/.nvm/versions/node/v12.0.0/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at ChildProcess.emit (events.js:196:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:256:12)
gyp ERR! System Linux 4.15.0-1028-gcp
gyp ERR! command "/home/travis/.nvm/versions/node/v12.0.0/bin/node" "/home/travis/.nvm/versions/node/v12.0.0/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /home/travis/build/GetEpona/Epona-js/node_modules/tiny-secp256k1
gyp ERR! node -v v12.0.0
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok
junderw commented 5 years ago

https://github.com/bcoin-org/bcrypto/issues/7

Looks like v12 finally removed a bunch of functionality that we make heavy use of...

For now, use a node version < v12.

v10 LTS will be supported until April 2021. But it's nice to have this here for priorities.

Thanks for the report.

junderw commented 5 years ago

I reported NaN issues to the NaN repo, we will need a fix from them before we can work on this:

https://github.com/nodejs/nan/issues/849#issuecomment-486899010

Here's the list of problems we can fix: Basically just stop using v8::Value::BooleanValue()

../native/addon.cpp: In function ‘Nan::NAN_METHOD_RETURN_TYPE ecdsaVerify(Nan::NAN_METHOD_ARGS_TYPE)’:
../native/addon.cpp:284:34: error: no matching function for call to ‘v8::Value::BooleanValue()’
   strict = info[3]->BooleanValue();

../native/addon.cpp:81:38: error: no matching function for call to ‘v8::Value::BooleanValue()’
   return info[index]->BooleanValue() ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED;

../native/addon.cpp:81:38: error: no matching function for call to ‘v8::Value::BooleanValue()’
   return info[index]->BooleanValue() ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED;

../native/addon.cpp:88:38: error: no matching function for call to ‘v8::Value::BooleanValue()’
   return info[index]->BooleanValue() ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED;
junderw commented 5 years ago

Also: #26 seems like a good idea... people will see the error logs, but using optionalDependencies we can at least pass your tests with the javascript implementation.

ghost commented 5 years ago

In the meantime can we set the max version to < 12 here?

https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/package.json#L8

junderw commented 5 years ago

Why there? Why not here?

How would this help if the result is a failed install either way?

ghost commented 5 years ago

Ooops, I'm sorry, I think was looking at a different repo when I copy and pasted that. Yeah I mean here.

I think for me, it wasn't pleasant hunting down why my heroku deployment failed though I have no problem setting node version of my package anyways. Just thought it might help those in the meantime if/when tiny-secp256k1 supports the changes introduced by node12.

you21979 commented 5 years ago

https://github.com/nodejs/node-gyp/blob/master/CHANGELOG.md#v400-2019-04-24 goodnews

carsonfarmer commented 5 years ago

Looks like secp256k1-node have a fix that could be ported here? https://github.com/cryptocoinjs/secp256k1-node/pull/143

Xmader commented 5 years ago

I fixed it.

39

junderw commented 5 years ago

Fixed in v1.1.1 thanks to @Xmader !

Thanks a lot!