Split Bls into per-type files; generate G1/G2 from a shared template - #10
Merged
Conversation
- Pairing.AsFp12 read past the end of the context buffer (it copied Pairing.Sz longs from the interior GT pointer instead of PT.Sz) and used the pointer after the span was unpinned; copy under fixed - new Pairing() bypassed the all-optional-parameter constructor and produced an instance backed by a null buffer that crashed the process on first use; add an explicit parameterless constructor - MultiMult passed npoints == 0 through to blst, which underflows and crashes; return infinity instead, and validate buffer lengths - Generator(Span)/PT.One(Span) copied into the caller's buffer before validating its length, corrupting memory for short buffers - Keygen*/DeriveMasterEip2333 silently produced an all-zero secret key for IKM shorter than 32 bytes (blst zeroes the output); throw instead - MapTo read past the end of inputs shorter than 48 bytes; validate - native library fallback path resolved runtimes/ against the current working directory instead of the application base directory - expose the error code on BlsException - fix RepositoryUrl typo (bls-bindings -> blst-bindings) Also readability: pool MSM scratch buffers instead of allocating per call, replace Buffer.MemoryCopy with bounds-checked span copies, use stackalloc pointer arrays for the pippenger wrappers, take MSM inputs as ReadOnlySpan, normalize modifier order, document the unvalidated decode paths and wrap-vs-parse constructors, drop dead code Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- sign/verify/aggregate roundtrips for min-pk signatures, including Pairing-based verification and rejection cases - EIP-2333 key derivation test vectors - RFC 9380 hash-to-curve test vectors for G1 and G2 - secret key range validation and encoding roundtrips - point serialization/compression roundtrips, infinity encoding, decode error cases, generator known-answer tests - scalar arithmetic identities and reduction - MSM vs single mult consistency, empty and invalid input handling - GT operations and Pairing.AsFp12 consistency Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BenchmarkDotNet project covering keygen, hash-to-curve, sign, verify, point arithmetic, decode/subgroup checks, pairings (including precomputed lines) and G1/G2 multi-scalar multiplication Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- PT.MillerLoopN exposes blst_miller_loop_n: computes the product of n Miller loops in one pass, sharing the Fp12 squarings across pairs (measured 1.14x-1.45x end-to-end for pairing checks of 2-16 pairs) - P1Affine/P2Affine raw affine Decode from field element bytes, so callers can build contiguous affine buffers without a Jacobian round trip - MultiMultAffine skips the Jacobian-to-affine batch conversion for callers that already hold affine points - bump version to 1.1.0 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The folded scalar's deeper-indented continuation line kept its newline, so -p:VersionSuffix=... executed as its own shell command whenever the preview input was set Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dotnet pack --no-build recomputes the package version, so without the suffix it packed and pushed a non-preview version Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pure move: each nested type and the native imports it uses now live in their own partial class file Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two point groups were near-identical copies that could drift apart silently. Bls.G1.cs and Bls.G2.cs are now rendered from PointGroup.template by the Nethermind.Crypto.Bls.Gen console project; the generated files stay checked in and reviewable, and CI verifies they match the template. Normalizations from unifying the two sides: P2 gains FromSk (parity with P1), P2.Mult(BigInteger) uses the same helper shape as P1, redundant readonly/scoped modifiers and comments are now consistent. No behavioral changes; all tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mments - add DefaultPairingConstructorInitializesContext regression test for the new Pairing() null-buffer crash - guard MSM/MillerLoopN length checks against int overflow ((long)npoints) - bump copyright to 2026 on files added in this PR - trim verbose comments to the load-bearing "why" Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merge review/fixes-tests-benchmarks (#9) into the point-group refactor and carry its improvements into the new template/split structure: - port the (long)npoints/npairs overflow-safe MSM/MillerLoopN guards into PointGroup.template and Bls.Gt.cs - keep #9's DefaultPairingConstructorInitializesContext regression test Review feedback on #10: - CI drift check stages first (git add -A && git diff --cached) so newly generated files are also compared - CI now enforces whitespace (dotnet format --verify-no-changes); fixed the one pre-existing violation in BlsTests.cs - drop the unused CopyToOutputDirectory in the generator csproj (the template is read from the source tree) - note in Program.cs that conditional blocks must be flat and paired Also: trim verbose comments and bump copyright to 2026 on the new files. Generation stays idempotent; all 58 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
benaadams
reviewed
Jul 16, 2026
Write the little-endian scalar magnitude straight to a stackalloc span via BigInteger.TryWriteBytes(isUnsigned) instead of ToByteArray(); a positive scalar now allocates nothing, and a negative one only the negated BigInteger. Drops the PrepareMult/GetSize helpers (the unsigned write needs no sign-byte trimming). Behaviour unchanged — covered by the One/Zero/MinusOne/large Mult tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # .github/CODEOWNERS # README.md # src/Nethermind.Crypto.Bls.slnx # src/Nethermind.Crypto.Bls/Bls.cs
rubo
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pure move:
Bls.cs(~1,900 lines) is split into per-type partial class files —Bls.cs(loader,ERROR,BlsException, shared constants),Bls.SecretKey.cs,Bls.Scalar.cs,Bls.G1.cs,Bls.G2.cs,Bls.Gt.cs,Bls.Pairing.cs— with each type's native imports next to it. No code changes; zero API impact since the class was alreadypartial.Generate G1/G2 from one template: the two point groups were ~95% duplicated code whose subtle intentional differences (fp ordering in
P2.TryDecode, coordinate counts, thesign_pk_in_*swap) could drift apart silently.Bls.G1.csandBls.G2.csare now rendered fromsrc/Nethermind.Crypto.Bls.Gen/PointGroup.templateby a small C# console project (dotnet run --project src/Nethermind.Crypto.Bls.Gen); the differences live in explicit//<<g1-only///<<g2-onlyblocks and named tokens. A new CI job regenerates and fails on drift.Why checked-in generation rather than a Roslyn source generator
For a crypto library the shipped logic should be auditable in the repo: generated-but-committed files mean reviewers see actual C# in every PR diff, IDE navigation/debugging work on real files, and the package build stays plain (no analyzer wiring). The CI drift check gives the same can't-diverge guarantee a source generator would.
Normalizations from unifying the two sides (no behavioral change)
P2gainsFromSk(parity withP1, additive API)P2.Mult(BigInteger)refactored to the samePrepareMult/GetSizeshape asP1readonlymember modifiers andscopedparameters across both groupsAll 57 tests pass; the generator is idempotent (regenerating on a clean tree produces no diff).