Skip to content

Commit

Permalink
Merge pull request #124 from ChainSafe/mkeil/rebuild-hubert-review
Browse files Browse the repository at this point in the history 
fix: addressing Huberts PR comments
  • Loading branch information
@matthewkeil
matthewkeil authored Mar 28, 2024
2 parents c0d69b1 + 1625ff0 commit 32567b0
Show file tree
Hide file tree
Showing 11 changed files with 660 additions and 568 deletions.
3 changes: 3 additions & 0 deletions rebuild/binding.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
'-Wpedantic',
],
'cflags_cc': [
'-std=c++17',
'-fexceptions',
'-Werror',
'-Wall',
Expand All @@ -41,6 +42,7 @@
'VCCLCompilerTool': {
'ExceptionHandling': 1,
'EnablePREfast': 'true',
'AdditionalOptions': [ '/std:c++17' ],
},
},
}
Expand All @@ -52,6 +54,7 @@
['OS=="mac"', {
'xcode_settings': {
'OTHER_CFLAGS': ['-fvisibility=hidden'],
'CLANG_CXX_LANGUAGE_STANDARD': 'c++17',
'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',
'CLANG_CXX_LIBRARY': 'libc++',
'MACOSX_DEPLOYMENT_TARGET': '12',
Expand Down
84 changes: 46 additions & 38 deletions rebuild/src/addon.cc
View file
Original file line number Diff line number Diff line change
@@ -1,61 +1,63 @@
#include "addon.h"

namespace blst_ts {
bool is_zero_bytes(
const uint8_t *data, const size_t start_byte, const size_t byte_length) {
for (size_t i = start_byte; i < byte_length; i++) {
if (data[i] != 0) {
return false;
}
const uint8_t *data,
const size_t starting_index,
const size_t byte_length) noexcept {
if (starting_index < byte_length) {
return std::all_of(
data + starting_index, data + byte_length, [](uint8_t x) {
return x == 0x00;
});
}
return true;
return false;
}

bool is_valid_length(
std::string &error_out,
size_t byte_length,
size_t length1,
size_t length2) {
[[nodiscard]] std::optional<std::string> is_valid_length(
size_t byte_length, size_t length1, size_t length2) noexcept {
if (byte_length == length1 || (length2 != 0 && byte_length == length2)) {
return true;
return std::nullopt;
}
error_out.append(" must be ");
std::string err_msg{" must be "};
if (length1 != 0) {
error_out.append(std::to_string(length1));
err_msg.append(std::to_string(length1));
};
if (length2 != 0) {
if (length1 != 0) {
error_out.append(" or ");
err_msg.append(" or ");
}
error_out.append(std::to_string(length2));
err_msg.append(std::to_string(length2));
};
error_out.append(" bytes long");
return false;
err_msg.append(" bytes long");
return err_msg;
}
} // namespace blst_ts

BlstTsAddon::BlstTsAddon(Napi::Env env, Napi::Object exports)
: _dst{"BLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_POP_"},
_blst_error_strings{
"BLST_SUCCESS",
"BLST_ERROR::BLST_BAD_ENCODING",
"BLST_ERROR::BLST_POINT_NOT_ON_CURVE",
"BLST_ERROR::BLST_POINT_NOT_IN_GROUP",
"BLST_ERROR::BLST_AGGR_TYPE_MISMATCH",
"BLST_ERROR::BLST_VERIFY_FAIL",
"BLST_ERROR::BLST_PK_IS_INFINITY",
"BLST_ERROR::BLST_BAD_SCALAR",
} {
: _blst_error_strings{
"BLST_SUCCESS",
"BLST_ERROR::BLST_BAD_ENCODING",
"BLST_ERROR::BLST_POINT_NOT_ON_CURVE",
"BLST_ERROR::BLST_POINT_NOT_IN_GROUP",
"BLST_ERROR::BLST_AGGR_TYPE_MISMATCH",
"BLST_ERROR::BLST_VERIFY_FAIL",
"BLST_ERROR::BLST_PK_IS_INFINITY",
"BLST_ERROR::BLST_BAD_SCALAR",
},
dst{"BLS_SIG_BLS12381G2_XMD:SHA-256_SSWU_RO_POP_"} {
Napi::Object js_constants = Napi::Object::New(env);
js_constants.Set(
Napi::String::New(env, "DST"), Napi::String::New(env, _dst));
Napi::String::New(env, "DST"), Napi::String::New(env, dst));
DefineAddon(
exports,
{
InstanceValue("BLST_CONSTANTS", js_constants, napi_enumerable),
});
SecretKey::Init(env, exports, this);
PublicKey::Init(env, exports, this);
Signature::Init(env, exports, this);
Functions::Init(env, exports);
blst_ts::SecretKey::Init(env, exports, this);
blst_ts::PublicKey::Init(env, exports, this);
blst_ts::Signature::Init(env, exports, this);
blst_ts_functions::init(env, exports);
env.SetInstanceData(this);

// Check that openssl PRNG is seeded
Expand All @@ -68,6 +70,14 @@ BlstTsAddon::BlstTsAddon(Napi::Env env, Napi::Object exports)
}

std::string BlstTsAddon::GetBlstErrorString(const blst::BLST_ERROR &err) {
size_t err_index = static_cast<size_t>(err);
// size of total array divided by size of one element minus 1 for 0 index
// basis
size_t max_index =
sizeof(_blst_error_strings) / sizeof(_blst_error_strings[0]) - 1;
if (err_index > max_index) {
return "BLST_ERROR::UNKNOWN_ERROR_CODE";
}
return _blst_error_strings[err];
}

Expand All @@ -77,10 +87,8 @@ bool BlstTsAddon::GetRandomBytes(blst::byte *bytes, size_t length) {
// [RandomBytesTraits::DeriveBits](https://github.com/nodejs/node/blob/4166d40d0873b6d8a0c7291872c8d20dc680b1d7/src/crypto/crypto_random.cc#L65)
// [CSPRNG](https://github.com/nodejs/node/blob/4166d40d0873b6d8a0c7291872c8d20dc680b1d7/src/crypto/crypto_util.cc#L63)
do {
if (1 == RAND_status()) {
if (1 == RAND_bytes(bytes, length)) {
return true;
}
if ((1 == RAND_status()) && (1 == RAND_bytes(bytes, length))) {
return true;
}
} while (1 == RAND_poll());

Expand Down
106 changes: 56 additions & 50 deletions rebuild/src/addon.h
View file
Original file line number Diff line number Diff line change
@@ -1,34 +1,26 @@
#ifndef BLST_TS_ADDON_H__
#define BLST_TS_ADDON_H__
#pragma once

#include <openssl/rand.h>

#include <algorithm>
#include <iostream>
#include <memory>
#include <sstream>
#include <optional>
#include <string>
#include <string_view>

#include "blst.hpp"
#include "napi.h"

// TODO: these should come out post PR review
using std::cout;
using std::endl;

namespace blst_ts {
#define BLST_TS_RANDOM_BYTES_LENGTH 8U

#define BLST_TS_FUNCTION_PREAMBLE(info, env, module) \
Napi::Env env = info.Env(); \
Napi::EscapableHandleScope scope(env); \
BlstTsAddon *module = env.GetInstanceData<BlstTsAddon>();

#define BLST_TS_IS_INFINITY \
Napi::Env env = info.Env(); \
Napi::EscapableHandleScope scope(env); \
return scope.Escape(Napi::Boolean::New(env, _point->IsInfinite()));

#define BLST_TS_SERIALIZE_POINT(macro_name) \
#define BLST_TS_SERIALIZE_POINT(snake_case_name) \
Napi::Env env = info.Env(); \
Napi::EscapableHandleScope scope(env); \
bool compressed{true}; \
Expand All @@ -37,12 +29,12 @@ using std::endl;
} \
Napi::Buffer<uint8_t> serialized = Napi::Buffer<uint8_t>::New( \
env, \
compressed ? BLST_TS_##macro_name##_LENGTH_COMPRESSED \
: BLST_TS_##macro_name##_LENGTH_UNCOMPRESSED); \
_point->Serialize(compressed, serialized.Data()); \
compressed ? snake_case_name##_length_compressed \
: snake_case_name##_length_uncompressed); \
point->Serialize(compressed, serialized.Data()); \
return scope.Escape(serialized);

#define BLST_TS_UNWRAP_UINT_8_ARRAY(value_name, arr_name, js_name) \
#define BLST_TS_UNWRAP_UINT_8_ARRAY(value_name, arr_name, js_name) \
if (!value_name.IsTypedArray()) { \
Napi::TypeError::New( \
env, "BLST_ERROR: " js_name " must be a BlstBuffer") \
Expand All @@ -59,79 +51,95 @@ using std::endl;
Napi::Uint8Array arr_name = \
arr_name##_array.As<Napi::TypedArrayOf<uint8_t>>();

#define BLST_TS_CLASS_UNWRAP_UINT_8_ARRAY(value_name, arr_name, js_name) \
#define BLST_TS_CLASS_UNWRAP_UINT_8_ARRAY(value_name, arr_name, js_name) \
if (!value_name.IsTypedArray()) { \
Napi::TypeError::New( \
env, "BLST_ERROR: " js_name " must be a BlstBuffer") \
.ThrowAsJavaScriptException(); \
m_has_error = true; \
has_error = true; \
return; \
} \
Napi::TypedArray arr_name##_array = value_name.As<Napi::TypedArray>(); \
if (arr_name##_array.TypedArrayType() != napi_uint8_array) { \
Napi::TypeError::New( \
env, "BLST_ERROR: " js_name " must be a BlstBuffer") \
.ThrowAsJavaScriptException(); \
m_has_error = true; \
has_error = true; \
return; \
} \
Napi::Uint8Array arr_name = \
arr_name##_array.As<Napi::TypedArrayOf<uint8_t>>();

class BlstTsAddon;

typedef enum { Affine, Jacobian } CoordType;

/**
* Checks a byte array to see if it is all zeros. Can pass start byte for the
* cases where the first byte is the tag (infinity point and
* compress/uncompressed).
* Checks if a specified range of bytes within a byte array consists only of
* zeros.
*
* @param data uint8_t*
* @param start_byte size_t
* @param byte_length size_t
* @param data A pointer to the first element of the byte array to be checked.
* @param starting_index The offset (index) from the beginning of the array
* where the check should start. (0 starts at *data)
* @param byte_length The total length of the data array
*
* @return bool
* @return Returns true if all bytes from start_byte to the end of the array are
* zeros. Returns false if the starting offset is beyond the array length or if
* any byte in the specified range is not zero.
*/
bool is_zero_bytes(
const uint8_t *data, const size_t start_byte, const size_t byte_length);
const uint8_t *data,
const size_t start_byte,
const size_t byte_length) noexcept;

/**
* Checks if a byte array is a valid length. If not, sets the error message and
* returns false. If valid returns true for use in conditional statements.
* Validates that a given byte length matches one of two specified lengths,
* returning an optional error message if the validation fails.
*
* @param[out] error_out &std::string - error message to set if invalid
* @param[in] byte_length size_t - length of the byte array to validate
* @param[in] length1 size_t - first valid length
* @param[in] length2 size_t - second valid length (optional)
* @param byte_length The length to be validated against length1 and length2.
* @param length1 The first valid length that byte_length can be. A value of 0
* is considered as not set and thus not compared.
* @param length2 The second valid length that byte_length can be. A value of 0
* is considered as not set and thus not compared.
*
* @return bool
* @return An std::optional<std::string> that contains an error message if
* byte_length does not match either length1 or (if length2 is not 0) length2.
* If byte_length is valid, std::nullopt is returned. The error message, if
* returned, specifies the valid lengths byte_length must match.
*
* @note If both length1 and length2 are provided (non-zero), and byte_length
* does not match, the error message indicates that the valid byte_length must
* be either length1 or length2. If only one length is provided (the other being
* 0), the error message will only reference the provided length. This function
* is marked with [[nodiscard]] to ensure that the caller handles the potential
* error message, promoting safer and more intentional error checking.
*/
bool is_valid_length(
std::string &error_out,
size_t byte_length,
size_t length1,
size_t length2 = 0);
[[nodiscard]] std::optional<std::string> is_valid_length(
size_t byte_length, size_t length1, size_t length2 = 0) noexcept;
} // namespace blst_ts

class BlstTsAddon;
/**
* Circular dependency if these are moved up to the top of the file.
*/
#include "functions.h"
#include "public_key.h"
#include "secret_key.h"
#include "signature.h"
namespace blst_ts_functions {
void init(const Napi::Env &env, Napi::Object &exports);
}

/**
* BlstTsAddon is the main entry point for the library. It is responsible
* for initialization and holding global values.
*/
class BlstTsAddon : public Napi::Addon<BlstTsAddon> {
public:
std::string _dst;
private:
std::string _blst_error_strings[8];
Napi::FunctionReference _secret_key_ctr;
Napi::FunctionReference _public_key_ctr;
Napi::FunctionReference _signature_ctr;

public:
std::string dst;
Napi::FunctionReference secret_key_ctr;
Napi::FunctionReference public_key_ctr;
Napi::FunctionReference signature_ctr;

/**
* BlstTsAddon::BlstTsAddon constructor used by Node.js to create an
Expand Down Expand Up @@ -173,5 +181,3 @@ class BlstTsAddon : public Napi::Addon<BlstTsAddon> {
*/
bool GetRandomBytes(blst::byte *ikm, size_t length);
};

#endif /* BLST_TS_ADDON_H__ */
Loading

0 comments on commit 32567b0

Please sign in to comment.