Skip to content

feat: mt verify with benchmarks #231

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

Closed
spiral-ladder wants to merge 18 commits into from
Closed

feat: mt verify with benchmarks #231

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
b69a81a
mt verify
spiral-ladder Mar 6, 2026
e3947c5
feat: use mt verify
spiral-ladder Mar 3, 2026
900d77a
use ReleaseFast
spiral-ladder Mar 4, 2026
0cccfd3
new api
spiral-ladder Mar 5, 2026
9b03f38
update blst-z
spiral-ladder Mar 5, 2026
8f01ec6
bench: blst-ts vs blst-z
spiral-ladder Mar 5, 2026
f4e7aa9
use pairing pools
spiral-ladder Mar 6, 2026
36b5d38
add aggregateVerify bench
spiral-ladder Mar 6, 2026
846333c
add aggregatePublicKeys and aggregateSignatures
spiral-ladder Mar 6, 2026
9b191fc
fmt
spiral-ladder Mar 6, 2026
ed21a61
address comments
spiral-ladder Mar 6, 2026
76447b9
streamline aggregateVerify
spiral-ladder Mar 8, 2026
e923570
handle pks.len = 1 separately for aggregatePulbicKeys
spiral-ladder Mar 8, 2026
d7760d2
add note
spiral-ladder Mar 9, 2026
80d2833
align
spiral-ladder Mar 9, 2026
e5c9294
explicit sizing (u32) for n_workers
spiral-ladder Mar 9, 2026
ebb9def
add comments + rename variables
spiral-ladder Mar 9, 2026
1ec9d3f
teardown for blst.PairingPool
spiral-ladder Mar 9, 2026
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
12 changes: 5 additions & 7 deletions bindings/napi/blst.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ pub fn blst_aggregateVerify(

const result = sig.aggregateVerify(
sig_groupcheck,
&pairing_buf,
@alignCast(&pairing_buf),
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 6, 2026

Choose a reason for hiding this comment

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

security-medium medium

The use of @alignCast on &pairing_buf is unsafe because pairing_buf is declared as a byte array [Pairing.sizeOf()]u8 (line 549), which has a default alignment of 1. If the aggregateVerify function expects a pointer with a higher alignment (e.g., aligned to the Pairing struct), this results in Undefined Behavior if the buffer is not correctly aligned at runtime. This can lead to crashes on architectures with strict alignment requirements or when using SIMD instructions. To fix this, the declaration of pairing_buf should be updated to include an explicit alignment: var pairing_buf: [Pairing.sizeOf()]u8 align(@alignOf(Pairing)) = undefined;

msgs,
DST,
pks,
Expand Down Expand Up @@ -598,7 +598,7 @@ pub fn blst_fastAggregateVerify(env: napi.Env, cb: napi.CallbackInfo(4)) !napi.V

var pairing_buf: [Pairing.sizeOf()]u8 = undefined;
Comment thread
spiral-ladder marked this conversation as resolved.
Outdated
// `pks_validate` is always false here since we assume proof of possession for public keys.
const result = sig.fastAggregateVerify(sigs_groupcheck, &pairing_buf, msg_info.data[0..32], DST, pks, false) catch {
const result = sig.fastAggregateVerify(sigs_groupcheck, @alignCast(&pairing_buf), msg_info.data[0..32], DST, pks, false) catch {
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 6, 2026

Choose a reason for hiding this comment

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

security-medium medium

The use of @alignCast on &pairing_buf is unsafe because pairing_buf is declared as a byte array [Pairing.sizeOf()]u8 (line 599), which has a default alignment of 1. If the fastAggregateVerify function expects a pointer with a higher alignment, this results in Undefined Behavior. To fix this, the declaration of pairing_buf should be updated to include an explicit alignment: var pairing_buf: [Pairing.sizeOf()]u8 align(@alignOf(Pairing)) = undefined;

return try env.getBoolean(false);
};

Expand Down Expand Up @@ -632,10 +632,10 @@ pub fn blst_verifyMultipleAggregateSignatures(env: napi.Env, cb: napi.CallbackIn
const msgs = try allocator.alloc([32]u8, n_elems);
defer allocator.free(msgs);

const pks = try allocator.alloc(*PublicKey, n_elems);
const pks = try allocator.alloc(*const PublicKey, n_elems);
defer allocator.free(pks);

const sigs = try allocator.alloc(*Signature, n_elems);
const sigs = try allocator.alloc(*const Signature, n_elems);
defer allocator.free(sigs);

const rands = try allocator.alloc([32]u8, n_elems);
Expand All @@ -662,17 +662,15 @@ pub fn blst_verifyMultipleAggregateSignatures(env: napi.Env, cb: napi.CallbackIn
rand.bytes(&rands[i]);
}

var pairing_buf: [Pairing.sizeOf()]u8 = undefined;
const result = blst.verifyMultipleAggregateSignatures(
&pairing_buf,
n_elems,
msgs,
DST,
pks,
pks_validate,
sigs,
sigs_groupcheck,
rands,
allocator,
) catch {
return try env.getBoolean(false);
};
Expand Down
3 changes: 3 additions & 0 deletions bindings/napi/root.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const config = @import("./config.zig");
const shuffle = @import("./shuffle.zig");
const BeaconStateView = @import("./BeaconStateView.zig");
const blst = @import("./blst.zig");
const blst_z = @import("blst");
Comment thread
spiral-ladder marked this conversation as resolved.
Outdated
const state_transition = @import("./state_transition.zig");

comptime {
Expand All @@ -21,6 +22,7 @@ const EnvCleanup = struct {
fn hook(_: *EnvCleanup) void {
if (env_refcount.fetchSub(1, .acq_rel) == 1) {
// Last environment — tear down shared state.
blst_z.thread_pool.deinitializeThreadPool();
config.state.deinit();
pubkeys.state.deinit();
pool.state.deinit();
Expand All @@ -36,6 +38,7 @@ fn register(env: napi.Env, exports: napi.Value) !void {
try pool.state.init();
try pubkeys.state.init();
config.state.init();
try blst_z.thread_pool.initializeThreadPool(null);
}

try env.addEnvCleanupHook(EnvCleanup, &env_cleanup, EnvCleanup.hook);
Expand Down
66 changes: 66 additions & 0 deletions bindings/perf/blst.test.ts
View file
Open in desktop
Copy link
Copy Markdown
Collaborator Author

@spiral-ladder spiral-ladder Mar 9, 2026

Choose a reason for hiding this comment

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

In this test file we have both benchmarks for blst-z and blst-ts but this is really just for the PRs sake, we should remove the typescript benchmarks once we're happy with results.

Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import crypto from "node:crypto";
import { bench, describe } from "@chainsafe/benchmark";
import {
SecretKey as SecretKeyTS,
type Signature as SignatureTS,
verifyMultipleAggregateSignatures as verifyTS,
} from "@chainsafe/blst";
import {
SecretKey as SecretKeyZig,
type Signature as SignatureZig,
verifyMultipleAggregateSignatures as verifyZig,
} from "../src/blst.js";

interface SignatureSetZig {
msg: Uint8Array;
pk: InstanceType<typeof SecretKeyZig> extends { toPublicKey(): infer P } ? P : never;
sig: InstanceType<typeof SignatureZig>;
}

interface SignatureSetTS {
msg: Uint8Array;
pk: ReturnType<InstanceType<typeof SecretKeyTS>["toPublicKey"]>;
sig: InstanceType<typeof SignatureTS>;
}

function generateZigSets(count: number): SignatureSetZig[] {
return Array.from({ length: count }, () => {
const msg = crypto.randomBytes(32);
const sk = SecretKeyZig.fromKeygen(crypto.randomBytes(32));
const pk = sk.toPublicKey();
const sig = sk.sign(msg);
return { msg, pk, sig };
});
}

function generateTSSets(count: number): SignatureSetTS[] {
return Array.from({ length: count }, () => {
const msg = crypto.randomBytes(32);
const sk = SecretKeyTS.fromKeygen(crypto.randomBytes(32));
const pk = sk.toPublicKey();
const sig = sk.sign(msg);
return { msg, pk, sig };
});
}

describe("verifyMultipleAggregateSignatures", () => {
for (const count of [3, 8, 32, 64, 128]) {
bench({
beforeEach: () => generateZigSets(count),
fn: (sets) => {
const isValid = verifyZig(sets);
if (!isValid) throw Error("Invalid");
},
id: `lodestar-z ${count} sets`,
});

bench({
beforeEach: () => generateTSSets(count),
fn: (sets) => {
const isValid = verifyTS(sets);
if (!isValid) throw Error("Invalid");
},
id: `@chainsafe/blst ${count} sets`,
});
}
});
4 changes: 2 additions & 2 deletions build.zig.zon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
.minimum_zig_version = "0.14.1",
.dependencies = .{
.blst = .{
.url = "git+https://github.com/chainsafe/blst-z#7c135f5049d99f603856a1c8728321cb69682788",
.hash = "blst_z-0.0.0-td3FNDuaAAA1nbeXzEihbQm-w9sJWrGpkrEjioonUnrV",
.url = "git+https://github.com/chainsafe/blst-z?ref=bing/mt-verify#2d8044a4fee9555e2b91caeb4d1282f1aadcf86e",
.hash = "blst_z-0.0.0-td3FNMzsAAApEwBlroOu7ooXu8S-ubHh2m8_zAgRoQ3W",
},
.hashtree = .{
.url = "git+https://github.com/chainsafe/hashtree-z#4ed9aebb6178662299ae799f6914499820ff0180",
Expand Down
4 changes: 3 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
}
},
"scripts": {
"prepare": "zig build build-lib:bindings -Doptimize=ReleaseSafe",
"prepare": "zig build build-lib:bindings -Doptimize=ReleaseFast",
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 6, 2026

Choose a reason for hiding this comment

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

critical

Changing the optimization level to ReleaseFast disables all runtime safety checks in Zig, which can lead to undefined behavior (e.g., from out-of-bounds memory access) going undetected. The style guide (line 5) explicitly prioritizes safety over performance. For a cryptographic library, sacrificing these safety checks for performance is a critical risk. Please revert this to ReleaseSafe to maintain the project's safety guarantees. If ReleaseFast is deemed essential for performance, this trade-off should be explicitly justified and ideally confined to benchmark-specific build configurations, not the default prepare script.

Suggested change
"prepare": "zig build build-lib:bindings -Doptimize=ReleaseFast",
"prepare": "zig build build-lib:bindings -Doptimize=ReleaseSafe",
References
  1. The style guide prioritizes safety over performance: "Our design goals are safety, performance, and developer experience. In that order." The change to ReleaseFast contradicts this by disabling runtime safety checks. (link)

Copy link
Copy Markdown
Collaborator Author

@spiral-ladder spiral-ladder Mar 9, 2026

Choose a reason for hiding this comment

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

The underlying blst library uses -O2 for the cryptographic operations, but the blst-ts library uses -O3. So we should do the same

Comment thread
spiral-ladder marked this conversation as resolved.
"lint": "biome check",
"test": "NODE_OPTIONS='--expose-gc' vitest run bindings/test/*.test.ts"
},
Expand All @@ -47,7 +47,9 @@
},
"devDependencies": {
"@biomejs/biome": "^2.3.11",
"@chainsafe/benchmark": "^2.0.1",
"@chainsafe/biomejs-config": "^1.0.0",
"@chainsafe/blst": "^2.2.0",
"@lodestar/config": "^1.39.1",
"@lodestar/era": "^1.39.1",
"@lodestar/state-transition": "^1.39.1",
Expand Down
Loading
Loading