Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
478 changes: 430 additions & 48 deletions bindings/napi/blst.zig

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions bindings/napi/root.zig
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const EnvCleanup = struct {
config.state.deinit();
pubkeys.state.deinit();
pool.state.deinit();
if (blst.PairingPool.instance) |p| p.deinit();
}
}
};
Expand Down
150 changes: 150 additions & 0 deletions bindings/perf/blst.test.ts
Copy link
Copy Markdown
Collaborator Author

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,150 @@
//TODO(bing): The ts benchmarks are here really to ensure perf is up to par on the zig side.
// Remove once we are happy
import crypto from "node:crypto";
import {bench, describe} from "@chainsafe/benchmark";
import {
SecretKey as SecretKeyTS,
type Signature as SignatureTS,
aggregatePublicKeys as aggregatePublicKeysTS,
aggregateSignatures as aggregateSignaturesTS,
aggregateVerify as aggregateVerifyTS,
verifyMultipleAggregateSignatures as verifyTS,
} from "@chainsafe/blst";
import {
SecretKey as SecretKeyZig,
type Signature as SignatureZig,
aggregatePublicKeys as aggregatePublicKeysZig,
aggregateSignatures as aggregateSignaturesZig,
aggregateVerify as aggregateVerifyZig,
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("aggregatePublicKeys", () => {
for (const count of [1, 8, 32, 128, 256]) {
bench({
beforeEach: () => generateZigSets(count).map((s) => s.pk),
fn: (publicKeys) => {
aggregatePublicKeysZig(publicKeys);
},
id: `aggregatePublicKeys lodestar-z ${count} keys`,
});

bench({
beforeEach: () => generateTSSets(count).map((s) => s.pk),
fn: (publicKeys) => {
aggregatePublicKeysTS(publicKeys);
},
id: `aggregatePublicKeys @chainsafe/blst ${count} keys`,
});
}
});

describe("aggregateSignatures", () => {
for (const count of [1, 8, 32, 128, 256]) {
bench({
beforeEach: () => generateZigSets(count).map((s) => s.sig),
fn: (signatures) => {
aggregateSignaturesZig(signatures);
},
id: `aggregateSignatures lodestar-z ${count} sigs`,
});

bench({
beforeEach: () => generateTSSets(count).map((s) => s.sig),
fn: (signatures) => {
aggregateSignaturesTS(signatures);
},
id: `aggregateSignatures @chainsafe/blst ${count} sigs`,
});
}
});

describe("aggregateVerify", () => {
for (const count of [3, 8, 32, 64, 128]) {
bench({
beforeEach: () => {
const sets = generateZigSets(count);
return {
messages: sets.map((s) => s.msg),
publicKeys: sets.map((s) => s.pk),
signature: aggregateSignaturesZig(sets.map((s) => s.sig)),
};
},
fn: ({messages, publicKeys, signature}) => {
const isValid = aggregateVerifyZig(messages, publicKeys, signature);
if (!isValid) throw Error("Invalid");
},
id: `aggregateVerify lodestar-z ${count} sets`,
});

bench({
beforeEach: () => {
const sets = generateTSSets(count);
return {
messages: sets.map((s) => s.msg),
publicKeys: sets.map((s) => s.pk),
signature: aggregateSignaturesTS(sets.map((s) => s.sig)),
};
},
fn: ({messages, publicKeys, signature}) => {
const isValid = aggregateVerifyTS(messages, publicKeys, signature);
if (!isValid) throw Error("Invalid");
},
id: `aggregateVerify @chainsafe/blst ${count} sets`,
});
}
});

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
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#7a3267e33848677da6b14d7207a6321f632fd434",
.hash = "blst_z-0.0.0-td3FNBybAAA8-g-1U89fnsbLJlE4DkmiHwe4rxBvP-0b",
},
.hashtree = .{
.url = "git+https://github.com/chainsafe/hashtree-z#4ed9aebb6178662299ae799f6914499820ff0180",
Expand Down
4 changes: 3 additions & 1 deletion package.json
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

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

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