Skip to content

Conversation

spiral-ladder
Copy link

@spiral-ladder spiral-ladder commented Sep 30, 2025

This PR implements the blst bun bindings as in ChainSafe/blst-z#49

What this PR does:

  • moved tests, backing TS code, and eth_c_abi.zig within that PR here as blst.zig
  • cleaned up some commented out / unused TS code in that PR

What this PR does NOT do:

  • include benchmark code. Left out test/perf for a separate PR

@spiral-ladder spiral-ladder self-assigned this Sep 30, 2025
@spiral-ladder spiral-ladder marked this pull request as ready for review September 30, 2025 17:14
@spiral-ladder spiral-ladder requested a review from a team as a code owner September 30, 2025 17:14
aggregateWithRandomness(
sets.concat({
pk: sets[0].pk,
//TODO: (@matthewkeil) this throws error "Public key is infinity" not signature because there is only one blst error
Copy link
Author

Choose a reason for hiding this comment

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

what does this TODO mean, is this still relevant? @matthewkeil

Copy link
Member

Choose a reason for hiding this comment

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

yes its still relevant. Check out the error type https://github.com/ChainSafe/blst-z/pull/49/files#diff-31ebad738598504e01f7313f21e76c7eb1c128f9cc636dca0fa8957b7ec15748R22 which wraps c.BLST_PK_IS_INFINITY, but no corresponding c.BLST_SIG_IS_INFINITY exists. The way the error is used is that it is returned whenever either p1 or p2 is infinity.

Copy link
Author

Choose a reason for hiding this comment

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

So it shouldn't be a TODO, but a note?

Copy link
Member

Choose a reason for hiding this comment

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

yeah

@wemeetagain
Copy link
Member

I just added a few benchmarks to sanity check, somehow looks quite bad?

  blst
    ✔ blst - keygen                                                       372023.8 ops/s    2.688000 us/op        -     510549 runs   1.62 s
    ✔ other - keygen                                                      376081.2 ops/s    2.659000 us/op        -     635747 runs   2.02 s
    ✔ blst - sign                                                         1688.576 ops/s    592.2150 us/op        -        509 runs  0.814 s
    ✔ other - sign                                                        1942.408 ops/s    514.8250 us/op        -        584 runs  0.808 s
    ✔ blst - verify                                                       463.1062 ops/s    2.159332 ms/op        -         71 runs  0.890 s
    ✔ other - verify                                                      579.2484 ops/s    1.726375 ms/op        -        213 runs   1.23 s
    ✔ blst - aggregateVerify 16                                           83.31737 ops/s    12.00230 ms/op        -         37 runs   2.05 s
    ✔ other - aggregateVerify 16                                          308.3152 ops/s    3.243434 ms/op        -        106 runs   4.02

@twoeths
Copy link

twoeths commented Oct 17, 2025

looks like we did not ultilize multi-threads? also need to make sure a Release build of zig, refer to ChainSafe/blst-bun#14 (comment)

aggregateVerify
      ✔ aggregateVerify - 1 sets                                            968.8627 ops/s    1.032138 ms/op   x1.038        197 runs  0.706 s
      ✔ aggregateVerify - 8 sets                                            468.5928 ops/s    2.134049 ms/op   x0.647        142 runs  0.812 s
      ✔ aggregateVerify - 32 sets                                           337.2375 ops/s    2.965269 ms/op   x0.259        474 runs   1.94 s
      ✔ aggregateVerify - 128 sets                                          128.2682 ops/s    7.796166 ms/op   x0.177         27 runs  0.737 s

@spiral-ladder
Copy link
Author

spiral-ladder commented Oct 17, 2025

@twoeths is right, the port did not contain the mt version. I've since ported it over in ChainSafe/blst-z#56, and is using it in a branch that uses this as base: #21

The aggregation numbers look better but is still underperforming even under zig build -Doptimize=ReleaseFast:

before:

bench/blst.bench.ts
    ✔ blst - aggregateVerify 16                                           144.0573 ops/s    6.941682 ms/op        -         29 runs   1.53 s
    ✔ other - aggregateVerify 16                                          846.1060 ops/s    1.181885 ms/op        -         55 runs   3.70 s

after:

bench/blst.bench.ts
    ✔ blst - aggregateVerify 16                                           498.5373 ops/s    2.005868 ms/op        -        116 runs   3.71 s
    ✔ other - aggregateVerify 16                                          841.3663 ops/s    1.188543 ms/op        -         81 runs   3.90 s

verify is also 2x slower on mac and that probably needs a separate look.

spiral-ladder added a commit to ChainSafe/blst.zig that referenced this pull request Oct 20, 2025
This missing c flag as well as the macro were causing some regressions
in performance within lodestar-bun versus production blst-ts 
in ChainSafe/lodestar-bun#18.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants