Skip to content

Commit 44061d0

Browse files
authored
fix(galoisd): correct signature/validator count check in Poll (#5471)
fixes #5428 ## Summary both `Poll` implementations had two input-validation guards comparing each commit's `Signatures` slice length against itself, so the condition was always false and the "More signatures than validators" error never fired. a malformed request with more signatures than validators would skip the check entirely and reach the prover. this fixes both comparisons to use `Validators` on the right-hand side. the same copy paste bug was in both server files; both are patched in this commit ## Changes - `galoisd/grpc/server.go`: compare `Signatures` against `Validators` for both the trusted and untrusted commits - `galoisd/grpc/bls12381_server.go`: same fix (the guard was copy pasted from `server.go` with the same typo) - `galoisd/grpc/server_test.go`: regression test that `Poll` rejects a commit with more signatures than validators, covering both server types and both commit fields (4 sub-cases total) ## Testing `go test ./grpc/...` in `galoisd\`. the new test fails before the fix (the dead guard is skipped so a zero-value server hits the job-slot check and returns `busy_building`), and passes after. also ran `go build ./grpc/...` and `go vet ./grpc/...`
2 parents 773eb5d + 897da74 commit 44061d0

3 files changed

Lines changed: 53 additions & 4 deletions

File tree

galoisd/grpc/bls12381_server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,10 @@ func (p *proverServerBls12381) Poll(ctx context.Context, pollReq *grpc.PollReque
105105
if len(req.UntrustedCommit.Validators) > lightclient.MaxVal {
106106
return nil, fmt.Errorf("The circuit can handle a maximum of %d validators", lightclient.MaxVal)
107107
}
108-
if len(req.TrustedCommit.Signatures) > len(req.TrustedCommit.Signatures) {
108+
if len(req.TrustedCommit.Signatures) > len(req.TrustedCommit.Validators) {
109109
return nil, fmt.Errorf("More signatures than validators")
110110
}
111-
if len(req.UntrustedCommit.Signatures) > len(req.UntrustedCommit.Signatures) {
111+
if len(req.UntrustedCommit.Signatures) > len(req.UntrustedCommit.Validators) {
112112
return nil, fmt.Errorf("More signatures than validators")
113113
}
114114

galoisd/grpc/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,10 +305,10 @@ func (p *proverServer) Poll(ctx context.Context, pollReq *grpc.PollRequest) (*gr
305305
if len(req.UntrustedCommit.Validators) > lightclient.MaxVal {
306306
return nil, fmt.Errorf("The circuit can handle a maximum of %d validators", lightclient.MaxVal)
307307
}
308-
if len(req.TrustedCommit.Signatures) > len(req.TrustedCommit.Signatures) {
308+
if len(req.TrustedCommit.Signatures) > len(req.TrustedCommit.Validators) {
309309
return nil, fmt.Errorf("More signatures than validators")
310310
}
311-
if len(req.UntrustedCommit.Signatures) > len(req.UntrustedCommit.Signatures) {
311+
if len(req.UntrustedCommit.Signatures) > len(req.UntrustedCommit.Validators) {
312312
return nil, fmt.Errorf("More signatures than validators")
313313
}
314314

galoisd/grpc/server_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package grpc
2+
3+
import (
4+
"context"
5+
"strings"
6+
"testing"
7+
8+
v3 "galois/grpc/api/v3"
9+
10+
types "github.com/cometbft/cometbft/api/cometbft/types/v1"
11+
)
12+
13+
type pollServer interface {
14+
Poll(context.Context, *v3.PollRequest) (*v3.PollResponse, error)
15+
}
16+
17+
// Regression test for #5428: Poll must reject any commit with more signatures than
18+
// validators. The guard fires before proving, so a zero-value server (maxJobs == 0) works.
19+
func TestPollSignatureValidatorGuard(t *testing.T) {
20+
const wantErr = "More signatures than validators"
21+
22+
oneVal := []*types.SimpleValidator{{}}
23+
okCommit := func() *v3.ValidatorSetCommit {
24+
return &v3.ValidatorSetCommit{Validators: oneVal, Signatures: [][]byte{{}}}
25+
}
26+
badCommit := func() *v3.ValidatorSetCommit {
27+
return &v3.ValidatorSetCommit{Validators: oneVal, Signatures: [][]byte{{}, {}}}
28+
}
29+
30+
servers := map[string]pollServer{
31+
"bn254": &proverServer{},
32+
"bls12381": &proverServerBls12381{},
33+
}
34+
35+
for sName, srv := range servers {
36+
reqs := map[string]*v3.ProveRequest{
37+
"trusted": {TrustedCommit: badCommit(), UntrustedCommit: okCommit()},
38+
"untrusted": {TrustedCommit: okCommit(), UntrustedCommit: badCommit()},
39+
}
40+
for rName, proveReq := range reqs {
41+
t.Run(sName+"/"+rName, func(t *testing.T) {
42+
_, err := srv.Poll(context.Background(), &v3.PollRequest{Request: proveReq})
43+
if err == nil || !strings.Contains(err.Error(), wantErr) {
44+
t.Fatalf("expected error containing %q, got: %v", wantErr, err)
45+
}
46+
})
47+
}
48+
}
49+
}

0 commit comments

Comments
 (0)