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..bb29101 100644 --- a/bbs/bbs12381g2pub.go +++ b/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, diff --git a/bbs/benchmark_test.go b/bbs/benchmark_test.go index 1571135..eb8b69f 100644 --- a/bbs/benchmark_test.go +++ b/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) + } + }) + } + } +} diff --git a/bbs/export_test.go b/bbs/export_test.go new file mode 100644 index 0000000..abe7168 --- /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 = sumOfG1Products 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..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.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 diff --git a/go.sum b/go.sum index 7f810ee..d207152 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.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=