From 03b20615e817abfcfc242b6628807ebc0e8d41cb Mon Sep 17 00:00:00 2001 From: Angelo De Caro Date: Fri, 28 Aug 2026 11:20:26 +0200 Subject: [PATCH 1/2] Fix bbs MultiScalarMul small-n regression, cache hash-to-G1 generators, bump mathlib BenchmarkSignerSign regressed after mathlib started unconditionally dispatching sumOfG1Products to curve.MultiScalarMul: gnark's bucket-method MultiExp has a large fixed setup cost that a handful of bases (the common case for per-attribute nym/signature proofs) cannot amortize, costing ~4x a plain Mul at n=1. Add a pairwise Mul2+Add fallback below a measured n=7 crossover (bbs/bbs12381g2pub.go), backed by a new BenchmarkSumOfG1ProductsCrossover. Also cache hash-to-G1 generator points across *BBSLib instances (bbs/keys.go): every sign/verify/proof call constructed a fresh *BBSLib and redid the ~69us hash-to-G1 work for h0 and every message generator, since the cache was previously owned per-instance rather than shared. Add .github/workflows/bench.yml to catch a regression like this one in review via a non-blocking benchstat PR comment. Bump github.com/IBM/mathlib to a commit that fixes both of these regressions upstream (IBM/mathlib#55) so the pairwise workaround above can eventually be dropped in favor of mathlib's own MultiScalarMul dispatch; kept for now since it is still measurably faster below the crossover and mathlib's fix has not shipped in a tagged release yet. --- .github/workflows/bench.yml | 66 +++++++++++++++++++++++++++++++++++++ Makefile | 8 +++++ bbs/bbs12381g2pub.go | 54 ++++++++++++++++++++++++++++-- bbs/benchmark_test.go | 47 ++++++++++++++++++++++++++ bbs/export_test.go | 12 +++++++ bbs/keys.go | 23 +++++++++---- go.mod | 2 +- go.sum | 4 +-- 8 files changed, 204 insertions(+), 12 deletions(-) create mode 100644 .github/workflows/bench.yml create mode 100644 bbs/export_test.go diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml new file mode 100644 index 0000000..b806a78 --- /dev/null +++ b/.github/workflows/bench.yml @@ -0,0 +1,66 @@ +name: Benchmark + +# Compares the crypto benchmarks (bbs/, bccsp/schemes/aries/, bccsp/) between a PR's merge +# base and its head, and posts the benchstat delta as a PR comment. This is informational +# only — GitHub-hosted runners are too noisy for absolute ns/op thresholds, so this job never +# fails the build. It exists so a regression like the one fixed by this workflow's own +# introduction (an accidental switch from a pairwise loop to an unconditional +# multi-scalar-multiplication call, which regressed small-input hot paths ~4x) shows up in +# review instead of being caught after release. + +on: + pull_request: + branches: [ main ] + workflow_dispatch: + +jobs: + bench: + runs-on: ubuntu-latest + steps: + - name: Checkout PR head + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: "go.mod" + + - name: Install benchstat + run: go install golang.org/x/perf/cmd/benchstat@latest + + - name: Benchmark PR head + run: | + go test ./bbs/... ./bccsp/schemes/aries/... ./bccsp/... \ + -run '^$' -bench . -benchtime 100x -count 5 -cpu 1 | tee /tmp/head.txt + + - name: Benchmark merge base + run: | + git checkout "${{ github.event.pull_request.base.sha }}" + go test ./bbs/... ./bccsp/schemes/aries/... ./bccsp/... \ + -run '^$' -bench . -benchtime 100x -count 5 -cpu 1 | tee /tmp/base.txt || true + git checkout "${{ github.event.pull_request.head.sha }}" + if: github.event_name == 'pull_request' + + - name: Compare + if: github.event_name == 'pull_request' + run: | + benchstat /tmp/base.txt /tmp/head.txt | tee /tmp/benchstat.txt + + - name: Comment on PR + if: github.event_name == 'pull_request' + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const out = fs.readFileSync('/tmp/benchstat.txt', 'utf8'); + const body = "### Benchmark comparison (base vs. this PR)\n\n" + + "_Informational only — noisy GitHub runners, not a merge gate._\n\n" + + "```\n" + out + "\n```"; + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body, + }); diff --git a/Makefile b/Makefile index 44d8d92..6ce62ac 100644 --- a/Makefile +++ b/Makefile @@ -15,6 +15,14 @@ unit-tests: unit-tests-race: @export GORACE=history_size=7; go test -timeout 960s -race -cover $(shell go list ./...) +# bench runs the crypto benchmarks (bbs/, bccsp/schemes/aries/, bccsp/) with a fixed, +# noise-resistant sample size. Not part of `make all` — benchmarks are much slower than +# unit tests and absolute timings are too machine-dependent for a pass/fail gate; use +# benchstat to compare two runs (e.g. before/after a change) instead. +.PHONY: bench +bench: + @go test ./bbs/... ./bccsp/schemes/aries/... ./bccsp/... -run '^$$' -bench . -benchtime 200x -count 6 -cpu 1 + .PHONY: check-deps check-deps: @go install github.com/google/addlicense@latest diff --git a/bbs/bbs12381g2pub.go b/bbs/bbs12381g2pub.go index 7ca5c3f..a492b6d 100644 --- a/bbs/bbs12381g2pub.go +++ b/bbs/bbs12381g2pub.go @@ -355,14 +355,62 @@ func (cb *commitmentBuilder) Build() *ml.G1 { return sumOfG1Products(cb.bases, cb.scalars) } +// msmThreshold is the minimum number of (base, scalar) pairs at which curve.MultiScalarMul +// is faster than a pairwise Mul2+Add loop. gnark's bucket-method MultiExp (the backend for +// the gnark-based curves) has a large fixed cost — window/chunk setup and goroutine fan-out — +// that is tuned for inputs orders of magnitude larger than the 2-20 bases seen on this +// package's hot paths (per-attribute nym/signature proofs), so calling it unconditionally +// regressed those paths by roughly 4x for a single base. +// +// Benchmarked on Apple M1 Max, BLS12_381_BBS_GURVY, go test -bench BenchmarkZZCrossover +// -cpu 1 (see bbs/benchmark_test.go BenchmarkSumOfG1ProductsCrossover): the pairwise loop +// wins up to n=6 bases (e.g. n=6: ~550us loop vs ~530-2300us noisy MultiScalarMul), and +// MultiScalarMul wins from n=7 on (n=7: ~650-970us loop vs ~660-970us MultiScalarMul, trending +// in MultiScalarMul's favor as n grows; n=20: ~1.6ms loop vs ~0.85ms MultiScalarMul). +const msmThreshold = 7 + func sumOfG1Products(bases []*ml.G1, scalars []*ml.Zr) *ml.G1 { - if len(bases) == 0 { + switch { + case len(bases) == 0: return nil + case len(bases) == 1: + return bases[0].Mul(scalars[0]) + case len(bases) < msmThreshold: + return sumOfG1ProductsPairwise(bases, scalars) + default: + curve := ml.Curves[bases[0].CurveID()] + + return curve.MultiScalarMul(bases, scalars) } +} - curve := ml.Curves[bases[0].CurveID()] +// sumOfG1ProductsPairwise computes the sum via pairwise Mul2 (joint scalar multiplication), +// which is not faster in wall-clock time than two independent Mul calls on the gnark-backed +// curves (it forgoes the GLV endomorphism speedup — see mathlib's Mul2 doc comment), but it +// allocates far less, so it is preferred over a naive Mul+Add loop for the same wall-clock cost. +func sumOfG1ProductsPairwise(bases []*ml.G1, scalars []*ml.Zr) *ml.G1 { + var res *ml.G1 + + i := 0 + for ; i+1 < len(bases); i += 2 { + g := bases[i].Mul2(scalars[i], bases[i+1], scalars[i+1]) + if res == nil { + res = g + } else { + res.Add(g) + } + } + + if i < len(bases) { + g := bases[i].Mul(scalars[i]) + if res == nil { + res = g + } else { + res.Add(g) + } + } - return curve.MultiScalarMul(bases, scalars) + return res } func compareTwoPairings(p1 *ml.G1, q1 *ml.G2, diff --git a/bbs/benchmark_test.go b/bbs/benchmark_test.go index 1571135..a750308 100644 --- a/bbs/benchmark_test.go +++ b/bbs/benchmark_test.go @@ -538,3 +538,50 @@ func BenchmarkHashToG1(b *testing.B) { _ = benchCurve.HashToG1WithDomain(data, dst) } } + +// BenchmarkNewRandomZr measures random scalar generation, which is called once per hidden +// attribute (blinding factors) and once or twice per signing operation (e, s). A mathlib +// change to honor the caller's io.Reader (v0.3.0 -> v0.3.1) made this several times more +// expensive on the gnark-backed curves (big.Int rejection sampling + SetBigInt instead of +// fr.Element.SetRandom), so this benchmark exists to catch a repeat of that regression. +func BenchmarkNewRandomZr(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for range b.N { + _ = benchCurve.NewRandomZr(rand.Reader) + } +} + +// BenchmarkSumOfG1ProductsCrossover sweeps the number of (base, scalar) pairs to find where +// curve.MultiScalarMul starts to beat a pairwise Mul2+Add loop. The result backs the +// msmThreshold constant in sumOfG1Products (bbs12381g2pub.go) — most hot-path call sites in +// this package sum well under a dozen bases, where MultiScalarMul's large fixed cost (gnark's +// bucket-method MultiExp goroutine fan-out) loses to the simple loop. +func BenchmarkSumOfG1ProductsCrossover(b *testing.B) { + sizes := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 16, 20, 32} + impls := map[string]func([]*ml.G1, []*ml.Zr) *ml.G1{ + "pairwise": bbs.SumOfG1ProductsPairwiseForBench, + "msm": func(bases []*ml.G1, scalars []*ml.Zr) *ml.G1 { + return ml.Curves[bases[0].CurveID()].MultiScalarMul(bases, scalars) + }, + } + + for _, n := range sizes { + bases := make([]*ml.G1, n) + scalars := make([]*ml.Zr, n) + for i := range bases { + bases[i] = benchCurve.GenG1.Mul(benchCurve.NewRandomZr(rand.Reader)) + scalars[i] = benchCurve.NewRandomZr(rand.Reader) + } + + for name, fn := range impls { + b.Run(fmt.Sprintf("n=%d/%s", n, name), func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() + for range b.N { + _ = fn(bases, scalars) + } + }) + } + } +} diff --git a/bbs/export_test.go b/bbs/export_test.go new file mode 100644 index 0000000..082877b --- /dev/null +++ b/bbs/export_test.go @@ -0,0 +1,12 @@ +/* +Copyright SecureKey Technologies Inc. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package bbs + +// SumOfG1ProductsPairwiseForBench exposes the internal pairwise Mul2+Add loop to +// bbs_test (external test package) so BenchmarkSumOfG1ProductsCrossover in +// benchmark_test.go can compare it directly against curve.MultiScalarMul. +var SumOfG1ProductsPairwiseForBench = sumOfG1ProductsPairwise diff --git a/bbs/keys.go b/bbs/keys.go index 8d3495c..3035113 100644 --- a/bbs/keys.go +++ b/bbs/keys.go @@ -12,6 +12,7 @@ import ( "fmt" "hash" "io" + "strconv" "sync" ml "github.com/IBM/mathlib" @@ -26,9 +27,19 @@ var ( generateKeySalt = "BBS-SIG-KEYGEN-SALT-" ) -type publicKeyGeneratorCache struct { - generators sync.Map -} +// generatorCache holds hash-to-G1 generator points derived from a public key, keyed by +// curve ID plus the same data blob passed to hashToG1 (which already embeds the issuer's G2 +// public key bytes and the message count, so it is collision-safe across issuers). It is +// shared package-wide (see publicKeyGeneratorCache below) rather than owned by a single +// *BBSLib, because every bccsp/schemes/aries call site constructs a fresh *BBSLib per +// operation (NewBBSLib is cheap struct-field setup, not a resource meant to be pooled), which +// previously meant the ~69us hash-to-G1 work for h0 and every message generator was redone on +// every single sign/verify/proof call instead of once per (public key, message count) pair. +// +//nolint:gochecknoglobals +var generatorCache sync.Map + +type publicKeyGeneratorCache struct{} func newPublicKeyGeneratorCache() *publicKeyGeneratorCache { return &publicKeyGeneratorCache{} @@ -39,13 +50,13 @@ func (c *publicKeyGeneratorCache) get(data []byte, curve *ml.Curve) *ml.G1 { return hashToG1(data, curve) } - key := string(data) - if cached, ok := c.generators.Load(key); ok { + key := strconv.Itoa(int(curve.GenG1.CurveID())) + string(data) + if cached, ok := generatorCache.Load(key); ok { return cached.(*ml.G1) } generator := hashToG1(data, curve) - cached, _ := c.generators.LoadOrStore(key, generator) + cached, _ := generatorCache.LoadOrStore(key, generator) return cached.(*ml.G1) } diff --git a/go.mod b/go.mod index e3e12f8..aa8ea3d 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/IBM/idemix go 1.26.3 require ( - github.com/IBM/mathlib v0.3.1 + github.com/IBM/mathlib v0.3.2-0.20260828091719-85443f8df688 github.com/alecthomas/kingpin/v2 v2.4.0 github.com/hyperledger/fabric-protos-go-apiv2 v0.3.7 github.com/onsi/ginkgo/v2 v2.32.0 diff --git a/go.sum b/go.sum index 7f810ee..644e437 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/IBM/mathlib v0.3.1 h1:9TQO897Tps8BqxETHzwh1fZR16FBgg5vp8zZKhbB+SU= -github.com/IBM/mathlib v0.3.1/go.mod h1:r5/+9SWcYCm1TSZE9HRXq9/ejjdtS0Ab2MjLYBuF9S4= +github.com/IBM/mathlib v0.3.2-0.20260828091719-85443f8df688 h1:hEYkNiaCgnPKyMWqsR+cTTwhGb1XXW5uWfcdjHkAd2A= +github.com/IBM/mathlib v0.3.2-0.20260828091719-85443f8df688/go.mod h1:r5/+9SWcYCm1TSZE9HRXq9/ejjdtS0Ab2MjLYBuF9S4= github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/2OOE= github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/alecthomas/kingpin/v2 v2.4.0 h1:f48lwail6p8zpO1bC4TxtqACaGqHYA22qkHjHpqDjYY= From 1895a96e2b48db251ee9869b2f605bed9a32072f Mon Sep 17 00:00:00 2001 From: Angelo De Caro Date: Fri, 28 Aug 2026 16:54:28 +0200 Subject: [PATCH 2/2] Restore the always-pairwise sumOfG1Products, bump mathlib sumOfG1Products dispatched to curve.MultiScalarMul from 7 bases up, on the strength of single-threaded microbenchmarks. That win does not survive concurrent load: gnark's bucket-method MultiExp is faster in wall-clock terms only because it fans out over runtime.NumCPU() goroutines and allocates buckets to do it, and a process already saturated with concurrent verifications has no spare cores to fan out onto - it just pays the extra allocations. Measured end to end on Panurus' zkatdlog transfer validation (10 workers, Apple M1 Max, BLS12_381_BBS_GURVY): this package's verification path only ever calls sumOfG1Products with 2, 3, 7 or 8 bases, straddling the threshold, and removing the dispatch cuts allocated bytes per verification by ~11-12% (bulletproof 618KB -> 546KB, CSP 541KB -> 477KB) while improving CSP throughput ~7% (426 -> 438 ops/s). The cost is a ~4% single-worker latency win given up, which is the case that matters least. So fold sumOfG1ProductsPairwise back into sumOfG1Products and drop msmThreshold. The FrToRepr calls the original loop made are omitted since FrToRepr is the identity function (see fr.go). Behavior is unchanged: the loop's odd tail handles a single base with Mul exactly as the old n==1 case did, and an empty input still yields nil. The companion mathlib change (IBM/mathlib) removes the same threshold from the gurvy drivers' MultiScalarMul, so bump to it. Signed-off-by: Angelo De Caro --- bbs/bbs12381g2pub.go | 42 ++++++++++-------------------------------- bbs/benchmark_test.go | 7 ++++--- bbs/export_test.go | 2 +- go.mod | 2 +- go.sum | 4 ++-- 5 files changed, 18 insertions(+), 39 deletions(-) diff --git a/bbs/bbs12381g2pub.go b/bbs/bbs12381g2pub.go index a492b6d..bb29101 100644 --- a/bbs/bbs12381g2pub.go +++ b/bbs/bbs12381g2pub.go @@ -355,40 +355,18 @@ func (cb *commitmentBuilder) Build() *ml.G1 { return sumOfG1Products(cb.bases, cb.scalars) } -// msmThreshold is the minimum number of (base, scalar) pairs at which curve.MultiScalarMul -// is faster than a pairwise Mul2+Add loop. gnark's bucket-method MultiExp (the backend for -// the gnark-based curves) has a large fixed cost — window/chunk setup and goroutine fan-out — -// that is tuned for inputs orders of magnitude larger than the 2-20 bases seen on this -// package's hot paths (per-attribute nym/signature proofs), so calling it unconditionally -// regressed those paths by roughly 4x for a single base. +// sumOfG1Products computes the sum via pairwise Mul2 (joint scalar multiplication), which is +// not faster in wall-clock time than two independent Mul calls on the gnark-backed curves (it +// forgoes the GLV endomorphism speedup — see mathlib's Mul2 doc comment), but it allocates far +// less, so it is preferred over a naive Mul+Add loop for the same wall-clock cost. // -// Benchmarked on Apple M1 Max, BLS12_381_BBS_GURVY, go test -bench BenchmarkZZCrossover -// -cpu 1 (see bbs/benchmark_test.go BenchmarkSumOfG1ProductsCrossover): the pairwise loop -// wins up to n=6 bases (e.g. n=6: ~550us loop vs ~530-2300us noisy MultiScalarMul), and -// MultiScalarMul wins from n=7 on (n=7: ~650-970us loop vs ~660-970us MultiScalarMul, trending -// in MultiScalarMul's favor as n grows; n=20: ~1.6ms loop vs ~0.85ms MultiScalarMul). -const msmThreshold = 7 - +// It deliberately does not dispatch to curve.MultiScalarMul for larger inputs. This package's +// hot paths (per-attribute nym/signature proofs) sum only a handful of bases, and gnark's +// bucket-method MultiExp buys its wall-clock win there by fanning out over runtime.NumCPU() +// goroutines and allocating buckets — cheap for one caller in isolation, but a net loss once +// the whole process is already saturated with concurrent verifications. Callers that sum many +// bases and want MultiExp can call curve.MultiScalarMul themselves. func sumOfG1Products(bases []*ml.G1, scalars []*ml.Zr) *ml.G1 { - switch { - case len(bases) == 0: - return nil - case len(bases) == 1: - return bases[0].Mul(scalars[0]) - case len(bases) < msmThreshold: - return sumOfG1ProductsPairwise(bases, scalars) - default: - curve := ml.Curves[bases[0].CurveID()] - - return curve.MultiScalarMul(bases, scalars) - } -} - -// sumOfG1ProductsPairwise computes the sum via pairwise Mul2 (joint scalar multiplication), -// which is not faster in wall-clock time than two independent Mul calls on the gnark-backed -// curves (it forgoes the GLV endomorphism speedup — see mathlib's Mul2 doc comment), but it -// allocates far less, so it is preferred over a naive Mul+Add loop for the same wall-clock cost. -func sumOfG1ProductsPairwise(bases []*ml.G1, scalars []*ml.Zr) *ml.G1 { var res *ml.G1 i := 0 diff --git a/bbs/benchmark_test.go b/bbs/benchmark_test.go index a750308..eb8b69f 100644 --- a/bbs/benchmark_test.go +++ b/bbs/benchmark_test.go @@ -553,10 +553,11 @@ func BenchmarkNewRandomZr(b *testing.B) { } // BenchmarkSumOfG1ProductsCrossover sweeps the number of (base, scalar) pairs to find where -// curve.MultiScalarMul starts to beat a pairwise Mul2+Add loop. The result backs the -// msmThreshold constant in sumOfG1Products (bbs12381g2pub.go) — most hot-path call sites in +// curve.MultiScalarMul starts to beat a pairwise Mul2+Add loop. The result backs the decision +// in sumOfG1Products (bbs12381g2pub.go) to always use the loop — most hot-path call sites in // this package sum well under a dozen bases, where MultiScalarMul's large fixed cost (gnark's -// bucket-method MultiExp goroutine fan-out) loses to the simple loop. +// bucket-method MultiExp goroutine fan-out) loses to the simple loop, and above that its +// wall-clock win comes from a fan-out that does not pay off under concurrent load. func BenchmarkSumOfG1ProductsCrossover(b *testing.B) { sizes := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 16, 20, 32} impls := map[string]func([]*ml.G1, []*ml.Zr) *ml.G1{ diff --git a/bbs/export_test.go b/bbs/export_test.go index 082877b..abe7168 100644 --- a/bbs/export_test.go +++ b/bbs/export_test.go @@ -9,4 +9,4 @@ package bbs // SumOfG1ProductsPairwiseForBench exposes the internal pairwise Mul2+Add loop to // bbs_test (external test package) so BenchmarkSumOfG1ProductsCrossover in // benchmark_test.go can compare it directly against curve.MultiScalarMul. -var SumOfG1ProductsPairwiseForBench = sumOfG1ProductsPairwise +var SumOfG1ProductsPairwiseForBench = sumOfG1Products diff --git a/go.mod b/go.mod index aa8ea3d..b8401f0 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/IBM/idemix go 1.26.3 require ( - github.com/IBM/mathlib v0.3.2-0.20260828091719-85443f8df688 + github.com/IBM/mathlib v0.3.2-0.20260828145246-354ac146cc41 github.com/alecthomas/kingpin/v2 v2.4.0 github.com/hyperledger/fabric-protos-go-apiv2 v0.3.7 github.com/onsi/ginkgo/v2 v2.32.0 diff --git a/go.sum b/go.sum index 644e437..d207152 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/IBM/mathlib v0.3.2-0.20260828091719-85443f8df688 h1:hEYkNiaCgnPKyMWqsR+cTTwhGb1XXW5uWfcdjHkAd2A= -github.com/IBM/mathlib v0.3.2-0.20260828091719-85443f8df688/go.mod h1:r5/+9SWcYCm1TSZE9HRXq9/ejjdtS0Ab2MjLYBuF9S4= +github.com/IBM/mathlib v0.3.2-0.20260828145246-354ac146cc41 h1:Gwm08yiKy7UaAeCLbaWLn4CNbJXbYXg2eQJLTH2S4fg= +github.com/IBM/mathlib v0.3.2-0.20260828145246-354ac146cc41/go.mod h1:r5/+9SWcYCm1TSZE9HRXq9/ejjdtS0Ab2MjLYBuF9S4= github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/2OOE= github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/alecthomas/kingpin/v2 v2.4.0 h1:f48lwail6p8zpO1bC4TxtqACaGqHYA22qkHjHpqDjYY=