Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion bindings/napi/BeaconStateView.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const std = @import("std");
const napi = @import("zapi:napi");
const napi = @import("zapi").napi;
const c = @import("config");
const fork_types = @import("fork_types");
const st = @import("state_transition");
Expand Down Expand Up @@ -37,6 +37,7 @@ pub fn BeaconStateView_ctor(env: napi.Env, cb: napi.CallbackInfo(0)) !napi.Value
cached_state,
BeaconStateView_finalize,
null,
null,
);

return cb.this();
Expand Down
8 changes: 4 additions & 4 deletions bindings/napi/blst.zig
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Contains the necessary bindings for blst operations in lodestar-ts.
const std = @import("std");
const napi = @import("zapi:napi");
const napi = @import("zapi").napi;
const bls = @import("bls");
const builtin = @import("builtin");
const getter = @import("napi_property_descriptor.zig").getter;
Expand Down Expand Up @@ -108,7 +108,7 @@ pub fn PublicKey_finalize(_: napi.Env, pk: *PublicKey, _: ?*anyopaque) void {
pub fn PublicKey_ctor(env: napi.Env, cb: napi.CallbackInfo(0)) !napi.Value {
const pk = try allocator.create(PublicKey);
errdefer allocator.destroy(pk);
_ = try env.wrap(cb.this(), PublicKey, pk, PublicKey_finalize, null);
_ = try env.wrap(cb.this(), PublicKey, pk, PublicKey_finalize, null, null);
return cb.this();
}

Expand Down Expand Up @@ -218,7 +218,7 @@ pub fn Signature_finalize(_: napi.Env, sig: *Signature, _: ?*anyopaque) void {
pub fn Signature_ctor(env: napi.Env, cb: napi.CallbackInfo(0)) !napi.Value {
const sig = try allocator.create(Signature);
errdefer allocator.destroy(sig);
_ = try env.wrap(cb.this(), Signature, sig, Signature_finalize, null);
_ = try env.wrap(cb.this(), Signature, sig, Signature_finalize, null, null);
return cb.this();
}

Expand Down Expand Up @@ -327,7 +327,7 @@ pub fn SecretKey_finalize(_: napi.Env, sk: *SecretKey, _: ?*anyopaque) void {
pub fn SecretKey_ctor(env: napi.Env, cb: napi.CallbackInfo(0)) !napi.Value {
const sk = try allocator.create(SecretKey);
errdefer allocator.destroy(sk);
_ = try env.wrap(cb.this(), SecretKey, sk, SecretKey_finalize, null);
_ = try env.wrap(cb.this(), SecretKey, sk, SecretKey_finalize, null, null);
return cb.this();
}

Expand Down
35 changes: 13 additions & 22 deletions bindings/napi/config.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const std = @import("std");
const napi = @import("zapi:napi");
const napi = @import("zapi").napi;
const js = @import("zapi").js;
const active_preset = @import("preset").active_preset;
const c = @import("config");
const BeaconConfig = @import("config").BeaconConfig;
Expand Down Expand Up @@ -41,29 +42,31 @@ pub const State = struct {

pub var state: State = .{};

pub fn Config_set(env: napi.Env, cb: napi.CallbackInfo(2)) !napi.Value {
/// JS: config.set(chainConfigObj, genesisValidatorsRoot)
pub fn set(obj: js.Value, genesis_root: js.Uint8Array) !void {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This function lacks assertions. The repository style guide requires that all function arguments, return values, pre/postconditions, and invariants be asserted. Furthermore, the assertion density must average a minimum of two assertions per function.

References
  1. Assert all function arguments and return values, pre/postconditions and invariants. The assertion density of the code must average a minimum of two assertions per function. (link)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguments are asserted inside the functions.

if (!state.initialized) {
return error.ConfigNotInitialized;
}

const obj = try cb.arg(0).coerceToObject();
const chain_config = try chainConfigFromObject(env, obj);
const genesis_validators_root_info = try cb.arg(1).getTypedarrayInfo();
if (genesis_validators_root_info.data.len != 32) {
// Drop to low-level for the complex object parsing
const e = js.env();
const raw_obj = try obj.toValue().coerceToObject();
const chain_config = try chainConfigFromObject(e, raw_obj);
Comment thread
nazarhussain marked this conversation as resolved.
Outdated

const root_slice = try genesis_root.toSlice();
if (root_slice.len != 32) {
return error.InvalidGenesisValidatorsRootLength;
}

state.config = BeaconConfig.init(
chain_config,
genesis_validators_root_info.data[0..32].*,
root_slice[0..32].*,
);

state.initialized = true;

return env.getUndefined();
}

pub fn chainConfigFromObject(env: napi.Env, obj: napi.Value) !ChainConfig {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the pub is removed, this function will not be exposed to binding.

fn chainConfigFromObject(env: napi.Env, obj: napi.Value) !ChainConfig {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function chainConfigFromObject exceeds the hard limit of 70 lines (it is currently 80 lines long). Consider refactoring this function by splitting the complex object parsing into smaller, specialized helper functions.

References
  1. Restrict the length of function bodies to reduce the probability of poorly structured code. We enforce a hard limit of 70 lines per function. (link)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope from this PR.

var chain_config: ChainConfig = undefined;
inline for (std.meta.fields(ChainConfig)) |field| {
const field_value = obj.getNamedProperty(field.name) catch |err| {
Expand Down Expand Up @@ -143,15 +146,3 @@ pub fn chainConfigFromObject(env: napi.Env, obj: napi.Value) !ChainConfig {
}
return chain_config;
}

pub fn register(env: napi.Env, exports: napi.Value) !void {
const config_obj = try env.createObject();
try config_obj.setNamedProperty("set", try env.createFunction(
"set",
2,
Config_set,
null,
));

try exports.setNamedProperty("config", config_obj);
}
19 changes: 4 additions & 15 deletions bindings/napi/metrics.zig
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const std = @import("std");
const builtin = @import("builtin");
const napi = @import("zapi:napi");
const js = @import("zapi").js;
const state_transition = @import("state_transition");

var gpa: std.heap.DebugAllocator(.{}) = .init;
Expand All @@ -11,27 +11,16 @@ else

var initialized: bool = false;

pub fn Metrics_scrapeMetrics(env: napi.Env, _: napi.CallbackInfo(0)) !napi.Value {
/// JS: metrics.scrapeMetrics() → string
pub fn scrapeMetrics() !js.String {
var buf = std.ArrayList(u8).init(allocator);
defer buf.deinit();
try state_transition.metrics.write(buf.writer());
return env.createStringUtf8(buf.items);
return js.String.from(buf.items);
}

pub fn deinit() void {
if (!initialized) return;
state_transition.metrics.state_transition.deinit();
initialized = false;
}

pub fn register(env: napi.Env, exports: napi.Value) !void {
const metrics_obj = try env.createObject();

try metrics_obj.setNamedProperty("scrapeMetrics", try env.createFunction(
"scrapeMetrics",
0,
Metrics_scrapeMetrics,
null,
));
try exports.setNamedProperty("metrics", metrics_obj);
}
2 changes: 1 addition & 1 deletion bindings/napi/napi_property_descriptor.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const std = @import("std");
const napi = @import("zapi:napi");
const napi = @import("zapi").napi;

/// Extracts a function name, without prefix, from a napi binding function.
///
Expand Down
25 changes: 7 additions & 18 deletions bindings/napi/pool.zig
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const std = @import("std");
const napi = @import("zapi:napi");
const js = @import("zapi").js;
const Node = @import("persistent_merkle_tree").Node;

/// Pool uses page allocator for internal allocations.
Expand Down Expand Up @@ -27,27 +27,16 @@ pub const State = struct {

pub var state: State = .{};

pub fn Pool_ensureCapacity(env: napi.Env, cb: napi.CallbackInfo(1)) !napi.Value {
/// JS: pool.ensureCapacity(newSize)
pub fn ensureCapacity(new_size: js.Number) !void {
if (!state.initialized) {
return error.PoolNotInitialized;
}

const requested = new_size.assertU32();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see that the DSL contract enforces that new_size is a number? so using assertXX here is not ideal and can cause a crash (when it should just throw an error).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type new_size: js.Number enforces it's a number, but it can be a uint or int, as JS number support both. So we need to assert here.

This is same as std.debug.assert for function arguments.

const old_size = state.pool.nodes.capacity;
const new_size = try cb.arg(0).getValueUint32();
if (new_size <= old_size) {
return env.getUndefined();
if (requested <= old_size) {
return;
}
try state.pool.preheat(@intCast(new_size - state.pool.nodes.capacity));
return env.getUndefined();
}

pub fn register(env: napi.Env, exports: napi.Value) !void {
const pool_obj = try env.createObject();
try pool_obj.setNamedProperty("ensureCapacity", try env.createFunction(
"ensureCapacity",
1,
Pool_ensureCapacity,
null,
));
try exports.setNamedProperty("pool", pool_obj);
try state.pool.preheat(@intCast(requested - state.pool.nodes.capacity));
}
Loading
Loading