Fix bbs MultiScalarMul small-n regression and cache hash-to-G1 generators

.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,
+            });

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

bbs/bbs12381g2pub.go

@@ -355,14 +355,40 @@ func (cb *commitmentBuilder) Build() *ml.G1 {
 	return sumOfG1Products(cb.bases, cb.scalars)
 }
 
+// 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.
+//
+// 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 {
-	if len(bases) == 0 {
-		return nil
+	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)
+		}
 	}
 
-	curve := ml.Curves[bases[0].CurveID()]
+	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,

bbs/benchmark_test.go

@@ -538,3 +538,51 @@ 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 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, 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{
+		"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)
+				}
+			})
+		}
+	}
+}

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 = sumOfG1Products

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)
 }

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.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

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.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=