Skip to content

Commit 6ebb128

Browse files
rach-idrach-id
authored
chore: use the signature set for signatures ordering (#188)
## Overview Mainly addresses the feedback in #187
1 parent eb003c6 commit 6ebb128

File tree

5 files changed

+130
-72
lines changed

5 files changed

+130
-72
lines changed

‎fibre/client_upload.go‎

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/hex"
66
"fmt"
7-
"sync"
87
"sync/atomic"
98

109
"github.com/celestiaorg/celestia-app-fibre/v6/fibre/validator"
@@ -81,9 +80,6 @@ func (c *Client) Upload(ctx context.Context, ns share.Namespace, blob *Blob) (re
8180
requests := makeUploadRequests(shardMap, promise.ToProto(), blob.RLCCoeffs())
8281
sigSet := valSet.NewSignatureSet(c.cfg.SafetyThreshold, validatorSignBytes)
8382

84-
// concurrent-safe map to track validator address → signature for ordering
85-
var sigMap sync.Map
86-
8783
c.log.DebugContext(ctx, "initiating blob upload",
8884
"promise_hash", hex.EncodeToString(promiseHash),
8985
"promise_height", promise.Height,
@@ -94,39 +90,31 @@ func (c *Client) Upload(ctx context.Context, ns share.Namespace, blob *Blob) (re
9490
)
9591

9692
// 3) upload data
97-
if err = c.uploadShards(ctx, requests, blob, sigSet, &sigMap); err != nil {
93+
if err = c.uploadShards(ctx, requests, blob, sigSet); err != nil {
9894
span.RecordError(err)
9995
span.SetStatus(codes.Error, "failed to upload")
10096
return result, err
10197
}
10298

10399
// 5) collect signatures
104-
if _, err := sigSet.Signatures(); err != nil {
100+
sigs, err := sigSet.Signatures()
101+
if err != nil {
105102
span.RecordError(err)
106103
span.SetStatus(codes.Error, "failed to collect signatures")
107104
return result, err
108105
}
109106

110-
// Build ordered signatures matching the validator set order.
111-
// Validators that didn't sign get nil entries.
112-
orderedSigs := make([][]byte, len(valSet.Validators))
113-
for i, val := range valSet.Validators {
114-
if sig, ok := sigMap.Load(val.Address.String()); ok {
115-
orderedSigs[i] = sig.([]byte)
116-
}
117-
}
118-
119107
c.log.DebugContext(ctx, "blob upload completed",
120108
"promise_hash", hex.EncodeToString(promiseHash),
121109
"blob_commitment", promise.Commitment.String(),
122110
"upload_size", promise.UploadSize,
123-
"signatures_collected", len(orderedSigs),
111+
"signatures_collected", len(sigs),
124112
)
125113

126114
span.SetStatus(codes.Ok, "")
127115
return SignedPaymentPromise{
128116
PaymentPromise: promise,
129-
ValidatorSignatures: orderedSigs,
117+
ValidatorSignatures: sigs,
130118
}, nil
131119
}
132120

@@ -191,7 +179,6 @@ func (c *Client) uploadTo(
191179
req *types.UploadShardRequest,
192180
blob *Blob,
193181
sigSet *validator.SignatureSet,
194-
sigMap *sync.Map,
195182
) bool {
196183
ctx, cancel := context.WithCancel(ctx) // GRPC calls require context cancelling upon completion
197184
defer cancel()
@@ -262,9 +249,6 @@ func (c *Client) uploadTo(
262249
return false
263250
}
264251

265-
// track address → signature for ordering later
266-
sigMap.Store(val.Address.String(), signature)
267-
268252
log.DebugContext(ctx, "successfully uploaded to validator")
269253
span.AddEvent("signature_verified")
270254
span.SetStatus(codes.Ok, "")
@@ -279,7 +263,6 @@ func (c *Client) uploadShards(
279263
requests map[*core.Validator]*types.UploadShardRequest,
280264
blob *Blob,
281265
sigSet *validator.SignatureSet,
282-
sigMap *sync.Map,
283266
) error {
284267
var (
285268
responses atomic.Uint32 // tracks finished responses
@@ -314,7 +297,7 @@ func (c *Client) uploadShards(
314297
c.closeWg.Done()
315298
}()
316299

317-
isDone := c.uploadTo(ctx, val, req, blob, sigSet, sigMap)
300+
isDone := c.uploadTo(ctx, val, req, blob, sigSet)
318301
if isDone && sigsCollectedOnce.CompareAndSwap(false, true) {
319302
close(sigsCollectedCh)
320303
}

‎fibre/validator/signature_set.go‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@ import (
1111

1212
// SignatureSet collects and validates signatures from validators.
1313
// It is safe for concurrent use.
14+
// Signatures are returned in validator set order by [Signatures],
15+
// with nil entries for validators that did not sign.
1416
type SignatureSet struct {
1517
requiredBytesSigned []byte
1618
minRequiredVotingPower int64
19+
validators []*core.Validator
1720

1821
mu sync.Mutex
1922
votingPower int64
20-
signatures [][]byte
23+
signatures map[string][]byte
2124
}
2225

2326
// NewSignatureSet creates a new [SignatureSet] for collecting and validating signatures.
@@ -27,7 +30,8 @@ func (s Set) NewSignatureSet(targetVotingPower cmtmath.Fraction, requiredBytesSi
2730
return &SignatureSet{
2831
requiredBytesSigned: requiredBytesSigned,
2932
minRequiredVotingPower: minRequiredVotingPower,
30-
signatures: make([][]byte, 0, s.Size()),
33+
validators: s.Validators,
34+
signatures: make(map[string][]byte, s.Size()),
3135
}
3236
}
3337

@@ -46,29 +50,36 @@ func (ss *SignatureSet) Add(val *core.Validator, signature []byte) (bool, error)
4650

4751
// add to collection
4852
ss.votingPower += val.VotingPower
49-
ss.signatures = append(ss.signatures, signature)
53+
ss.signatures[val.Address.String()] = signature
5054

5155
// check if thresholds are met
5256
return ss.votingPower >= ss.minRequiredVotingPower, nil
5357
}
5458

55-
// Signatures returns all collected signatures if thresholds are met.
56-
// Returns [NotEnoughSignaturesError] if either count or voting power threshold is not met.
59+
// Signatures returns collected signatures ordered by validator set position if thresholds are met.
60+
// Validators that did not sign have nil entries.
61+
// Returns [NotEnoughSignaturesError] if voting power threshold is not met.
5762
// The error contains the partially collected signatures and threshold information.
5863
func (ss *SignatureSet) Signatures() ([][]byte, error) {
5964
ss.mu.Lock()
6065
defer ss.mu.Unlock()
6166

62-
powerNotMet := ss.votingPower < ss.minRequiredVotingPower
63-
if powerNotMet {
67+
ordered := make([][]byte, len(ss.validators))
68+
for i, val := range ss.validators {
69+
if sig, ok := ss.signatures[val.Address.String()]; ok {
70+
ordered[i] = sig
71+
}
72+
}
73+
74+
if ss.votingPower < ss.minRequiredVotingPower {
6475
return nil, &NotEnoughSignaturesError{
65-
Collected: ss.signatures,
76+
Collected: ordered,
6677
CollectedPower: ss.votingPower,
6778
RequiredPower: ss.minRequiredVotingPower,
6879
}
6980
}
7081

71-
return ss.signatures, nil
82+
return ordered, nil
7283
}
7384

7485
// NotEnoughSignaturesError indicates that signature collection did not meet the required thresholds.

‎fibre/validator/signature_set_test.go‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,14 @@ func TestSignatureSet(t *testing.T) {
7373

7474
var sigErr *validator.NotEnoughSignaturesError
7575
require.ErrorAs(t, err, &sigErr)
76-
require.Len(t, sigErr.Collected, 3)
76+
require.Len(t, sigErr.Collected, 5)
77+
nonNil := 0
78+
for _, sig := range sigErr.Collected {
79+
if sig != nil {
80+
nonNil++
81+
}
82+
}
83+
require.Equal(t, 3, nonNil)
7784
require.Equal(t, int64(30), sigErr.CollectedPower)
7885
require.Equal(t, int64(33), sigErr.RequiredPower)
7986
require.Contains(t, err.Error(), "not enough voting power")
@@ -97,7 +104,14 @@ func TestSignatureSet(t *testing.T) {
97104

98105
var sigErr *validator.NotEnoughSignaturesError
99106
require.ErrorAs(t, err, &sigErr)
100-
require.Len(t, sigErr.Collected, 2)
107+
require.Len(t, sigErr.Collected, 5)
108+
nonNil := 0
109+
for _, sig := range sigErr.Collected {
110+
if sig != nil {
111+
nonNil++
112+
}
113+
}
114+
require.Equal(t, 2, nonNil)
101115
require.Equal(t, int64(20), sigErr.CollectedPower)
102116
require.Equal(t, int64(33), sigErr.RequiredPower)
103117
require.Contains(t, err.Error(), "not enough voting power")
@@ -116,7 +130,14 @@ func TestSignatureSet(t *testing.T) {
116130

117131
sigs, err := s.sigSet.Signatures()
118132
require.NoError(t, err)
119-
require.Len(t, sigs, 4)
133+
require.Len(t, sigs, 5)
134+
nonNil := 0
135+
for _, sig := range sigs {
136+
if sig != nil {
137+
nonNil++
138+
}
139+
}
140+
require.Equal(t, 4, nonNil)
120141
})
121142

122143
t.Run("SuccessConcurrent", func(t *testing.T) {
@@ -166,6 +187,13 @@ func TestSignatureSet(t *testing.T) {
166187

167188
sigs, err := s.sigSet.Signatures()
168189
require.NoError(t, err)
169-
require.Len(t, sigs, 3)
190+
require.Len(t, sigs, 5)
191+
nonNil := 0
192+
for _, sig := range sigs {
193+
if sig != nil {
194+
nonNil++
195+
}
196+
}
197+
require.Equal(t, 3, nonNil)
170198
})
171199
}

‎test/docker-e2e/go.mod‎

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,18 @@ require (
1414
github.com/moby/moby v28.4.0+incompatible
1515
github.com/stretchr/testify v1.11.1
1616
go.uber.org/zap v1.27.0
17-
golang.org/x/sync v0.17.0
17+
golang.org/x/sync v0.19.0
1818
)
1919

2020
require (
2121
cel.dev/expr v0.24.0 // indirect
22-
cloud.google.com/go v0.120.0 // indirect
23-
cloud.google.com/go/auth v0.16.4 // indirect
22+
cloud.google.com/go v0.123.0 // indirect
23+
cloud.google.com/go/auth v0.18.1 // indirect
2424
cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect
25-
cloud.google.com/go/compute/metadata v0.8.0 // indirect
26-
cloud.google.com/go/iam v1.5.2 // indirect
27-
cloud.google.com/go/monitoring v1.24.2 // indirect
28-
cloud.google.com/go/storage v1.50.0 // indirect
25+
cloud.google.com/go/compute/metadata v0.9.0 // indirect
26+
cloud.google.com/go/iam v1.5.3 // indirect
27+
cloud.google.com/go/monitoring v1.24.3 // indirect
28+
cloud.google.com/go/storage v1.56.0 // indirect
2929
cosmossdk.io/api v0.7.6 // indirect
3030
cosmossdk.io/client/v2 v2.0.0-beta.8 // indirect
3131
cosmossdk.io/collections v0.4.0 // indirect
@@ -46,9 +46,9 @@ require (
4646
github.com/BurntSushi/toml v1.5.0 // indirect
4747
github.com/DataDog/datadog-go v4.8.3+incompatible // indirect
4848
github.com/DataDog/zstd v1.5.7 // indirect
49-
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.29.0 // indirect
50-
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.50.0 // indirect
51-
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.50.0 // indirect
49+
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0 // indirect
50+
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.53.0 // indirect
51+
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.53.0 // indirect
5252
github.com/Microsoft/go-winio v0.6.2 // indirect
5353
github.com/avast/retry-go/v4 v4.6.1 // indirect
5454
github.com/aws/aws-sdk-go v1.55.5 // indirect
@@ -89,7 +89,7 @@ require (
8989
github.com/cespare/xxhash/v2 v2.3.0 // indirect
9090
github.com/chzyer/readline v1.5.1 // indirect
9191
github.com/cloudwego/base64x v0.1.6 // indirect
92-
github.com/cncf/xds/go v0.0.0-20250501225837-2ac532fd4443 // indirect
92+
github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f // indirect
9393
github.com/cockroachdb/apd/v2 v2.0.2 // indirect
9494
github.com/cockroachdb/errors v1.11.3 // indirect
9595
github.com/cockroachdb/fifo v0.0.0-20240816210425-c5d0cb0b6fc0 // indirect
@@ -123,15 +123,15 @@ require (
123123
github.com/dustin/go-humanize v1.0.1 // indirect
124124
github.com/dvsekhvalnov/jose2go v1.7.0 // indirect
125125
github.com/emicklei/dot v1.6.2 // indirect
126-
github.com/envoyproxy/go-control-plane/envoy v1.32.4 // indirect
126+
github.com/envoyproxy/go-control-plane/envoy v1.35.0 // indirect
127127
github.com/envoyproxy/protoc-gen-validate v1.2.1 // indirect
128128
github.com/ethereum/go-ethereum v1.16.3 // indirect
129129
github.com/fatih/color v1.18.0 // indirect
130130
github.com/felixge/httpsnoop v1.0.4 // indirect
131131
github.com/filecoin-project/go-clock v0.1.0 // indirect
132132
github.com/fsnotify/fsnotify v1.9.0 // indirect
133133
github.com/getsentry/sentry-go v0.35.0 // indirect
134-
github.com/go-jose/go-jose/v4 v4.1.1 // indirect
134+
github.com/go-jose/go-jose/v4 v4.1.3 // indirect
135135
github.com/go-kit/kit v0.13.0 // indirect
136136
github.com/go-kit/log v0.2.1 // indirect
137137
github.com/go-logfmt/logfmt v0.6.0 // indirect
@@ -153,8 +153,8 @@ require (
153153
github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db // indirect
154154
github.com/google/s2a-go v0.1.9 // indirect
155155
github.com/google/uuid v1.6.0 // indirect
156-
github.com/googleapis/enterprise-certificate-proxy v0.3.6 // indirect
157-
github.com/googleapis/gax-go/v2 v2.15.0 // indirect
156+
github.com/googleapis/enterprise-certificate-proxy v0.3.11 // indirect
157+
github.com/googleapis/gax-go/v2 v2.17.0 // indirect
158158
github.com/gorilla/handlers v1.5.2 // indirect
159159
github.com/gorilla/mux v1.8.1 // indirect
160160
github.com/gorilla/websocket v1.5.3 // indirect
@@ -229,7 +229,7 @@ require (
229229
github.com/spf13/cobra v1.9.1 // indirect
230230
github.com/spf13/pflag v1.0.10 // indirect
231231
github.com/spf13/viper v1.21.0 // indirect
232-
github.com/spiffe/go-spiffe/v2 v2.5.0 // indirect
232+
github.com/spiffe/go-spiffe/v2 v2.6.0 // indirect
233233
github.com/subosito/gotenv v1.6.0 // indirect
234234
github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect
235235
github.com/tendermint/go-amino v0.16.0 // indirect
@@ -242,33 +242,33 @@ require (
242242
github.com/zondax/ledger-go v1.0.1 // indirect
243243
go.etcd.io/bbolt v1.4.0 // indirect
244244
go.opencensus.io v0.24.0 // indirect
245-
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
246-
go.opentelemetry.io/contrib/detectors/gcp v1.36.0 // indirect
245+
go.opentelemetry.io/auto/sdk v1.2.1 // indirect
246+
go.opentelemetry.io/contrib/detectors/gcp v1.38.0 // indirect
247247
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 // indirect
248248
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.62.0 // indirect
249-
go.opentelemetry.io/otel v1.38.0 // indirect
249+
go.opentelemetry.io/otel v1.39.0 // indirect
250250
go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.37.0 // indirect
251-
go.opentelemetry.io/otel/metric v1.38.0 // indirect
252-
go.opentelemetry.io/otel/sdk v1.37.0 // indirect
253-
go.opentelemetry.io/otel/sdk/metric v1.37.0 // indirect
254-
go.opentelemetry.io/otel/trace v1.38.0 // indirect
251+
go.opentelemetry.io/otel/metric v1.39.0 // indirect
252+
go.opentelemetry.io/otel/sdk v1.39.0 // indirect
253+
go.opentelemetry.io/otel/sdk/metric v1.39.0 // indirect
254+
go.opentelemetry.io/otel/trace v1.39.0 // indirect
255255
go.uber.org/multierr v1.11.0 // indirect
256256
go.yaml.in/yaml/v3 v3.0.4 // indirect
257257
golang.org/x/arch v0.15.0 // indirect
258-
golang.org/x/crypto v0.41.0 // indirect
258+
golang.org/x/crypto v0.47.0 // indirect
259259
golang.org/x/exp v0.0.0-20250911091902-df9299821621 // indirect
260-
golang.org/x/net v0.43.0 // indirect
261-
golang.org/x/oauth2 v0.31.0 // indirect
262-
golang.org/x/sys v0.35.0 // indirect
263-
golang.org/x/term v0.34.0 // indirect
264-
golang.org/x/text v0.28.0 // indirect
265-
golang.org/x/time v0.12.0 // indirect
266-
google.golang.org/api v0.247.0 // indirect
267-
google.golang.org/genproto v0.0.0-20250603155806-513f23925822 // indirect
268-
google.golang.org/genproto/googleapis/api v0.0.0-20250707201910-8d1bb00bc6a7 // indirect
269-
google.golang.org/genproto/googleapis/rpc v0.0.0-20250818200422-3122310a409c // indirect
270-
google.golang.org/grpc v1.75.1 // indirect
271-
google.golang.org/protobuf v1.36.10 // indirect
260+
golang.org/x/net v0.49.0 // indirect
261+
golang.org/x/oauth2 v0.35.0 // indirect
262+
golang.org/x/sys v0.40.0 // indirect
263+
golang.org/x/term v0.39.0 // indirect
264+
golang.org/x/text v0.33.0 // indirect
265+
golang.org/x/time v0.14.0 // indirect
266+
google.golang.org/api v0.266.0 // indirect
267+
google.golang.org/genproto v0.0.0-20260128011058-8636f8732409 // indirect
268+
google.golang.org/genproto/googleapis/api v0.0.0-20260128011058-8636f8732409 // indirect
269+
google.golang.org/genproto/googleapis/rpc v0.0.0-20260203192932-546029d2fa20 // indirect
270+
google.golang.org/grpc v1.78.0 // indirect
271+
google.golang.org/protobuf v1.36.11 // indirect
272272
gopkg.in/yaml.v2 v2.4.0 // indirect
273273
gopkg.in/yaml.v3 v3.0.1 // indirect
274274
gotest.tools/v3 v3.5.2 // indirect

0 commit comments

Comments
 (0)