fix(spec-test): pass eth_aggregate_pubkeys_x40_pubkey #100
Conversation
- Create ErrorHandler - Extends ErrorHandler to DRY rest of classes
…to check in spec test
| * so if P1 points are aggregated often (Eth2.0) you want to keep the point | ||
| * cached in jacobian coordinates. | ||
| */ | ||
| export enum CoordType { |
There was a problem hiding this comment.
can we move all types/interfaces to a types.d.ts or something
|
|
||
| /** | ||
| * | ||
| * |
There was a problem hiding this comment.
can we chunk out these empty comment lines
| */ | ||
| bool BlstBase::IsZeroBytes(const uint8_t *data, size_t start_byte, size_t byte_length) | ||
| { | ||
| for (size_t i = start_byte; i < byte_length; i++) |
There was a problem hiding this comment.
can we use spaces instead of tabs
| Setup(); | ||
| if (HasError()) | ||
| { | ||
| goto out_err; |
There was a problem hiding this comment.
why do we need t o use goto? and where is the label out_err ?
| return ret_val; | ||
| WORKER_TRY_CATCH_END("RunSync"); | ||
| }; | ||
| Napi::Value BlstAsyncWorker::Run() |
There was a problem hiding this comment.
add a new line after each fn/class declaration
| Napi::Value BlstAsyncWorker::Run() | ||
| { | ||
| WORKER_TRY_CATCH_BEGIN | ||
| _use_deferred = true; |
There was a problem hiding this comment.
could you add comment what _use_deferred does
| * | ||
| * | ||
| * Uint8ArrayArgArray | ||
| * |
There was a problem hiding this comment.
can we move this to a separate file, also js-docs kind of input output description
| return true; | ||
| } | ||
| } | ||
| } while (1 == RAND_poll()); |
There was a problem hiding this comment.
can't we just import something rather than copy over?
| try \ | ||
| { | ||
|
|
||
| #define WORKER_TRY_CATCH_END(name) \ |
There was a problem hiding this comment.
is using the macro to try catch standard way of doing this?
| } | ||
| else if (_has_affine) | ||
| { | ||
| if (_affine->is_inf()) |
| } | ||
| } | ||
|
|
||
| set_error: |
There was a problem hiding this comment.
not in favor of goto at all unless you are writing a stack implementation
goto is hard to prove correctness
| public: | ||
| PublicKeyArg(Napi::Env env); | ||
| PublicKeyArg(Napi::Env env, Napi::Value raw_arg); | ||
| PublicKeyArg(const PublicKeyArg &source) = delete; |
There was a problem hiding this comment.
what is this supposed to do here?
|
closed with preference for macros instead of class based constructs. see |
This PR is related to #88 and covers spec testing of the
aggregatePublicKeysspec test that was failing.Inclusions
aggregatePublicKeysHow to test
NOTE: to build and test copy the
blstfolder intorebuild/deps/blst. This will go away when we merge but for nownode-gypgets heartburn when building deps in folder above thebinding.gypfileUnit tests are provided and should have 100% coverage. If you see an edge that may not be covered please feel free to bring it up and I will add the test case
cd rebuild yarn yarn build yarn test:spec