Skip to content

Commit 816cf0e

Browse files
WondertanWondertan
authored
chore: major linting (#256)
- Fix dozens of integer overflow lint failures - add CI linting rules - updated to golangci-lint v2 - refine selected linters - remove go-acc
1 parent 8224f89 commit 816cf0e

39 files changed

+426
-212
lines changed

‎.github/workflows/go-ci.yml‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,25 @@ jobs:
1111
build:
1212
runs-on: ubuntu-latest
1313
steps:
14-
- uses: actions/checkout@v3
14+
- uses: actions/checkout@v4
1515

16-
- name: Set up Go
17-
uses: actions/setup-go@v3
18-
with:
19-
go-version: 1.22
16+
- name: Set up Go
17+
uses: actions/setup-go@v5
18+
with:
19+
go-version: stable
2020

21-
- name: Build
22-
run: go build -v ./...
21+
- name: Build
22+
run: go build -v ./...
2323

24-
- name: Test & Coverage
25-
run: |
26-
go install github.com/ory/[email protected]
27-
go-acc -o coverage.txt ./... -- -v --race
24+
- name: Test & Coverage
25+
run: go test -cover -coverprofile coverage.o ./... -- -v --race
26+
- uses: codecov/codecov-action@v3
27+
with:
28+
file: ./coverage.o
2829

29-
- uses: codecov/codecov-action@v3
30-
with:
31-
file: ./coverage.txt
30+
- name: Lint
31+
uses: golangci/golangci-lint-action@v7
32+
with:
33+
version: latest
34+
- name: Go Mod Tidy
35+
run: go mod tidy && git diff --exit-code

‎.golangci.yml‎

Lines changed: 50 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,75 @@
1-
run:
2-
timeout: 5m
3-
1+
version: "2"
42
linters:
53
enable:
4+
- asciicheck
65
- bidichk
76
- bodyclose
8-
# - depguard as of v1.54.2, the default config throws errors on our repo
7+
- copyloopvar
98
- dogsled
109
- dupl
11-
- errcheck
10+
- dupword
1211
- errorlint
13-
# - funlen
14-
# - gochecknoglobals
15-
# - gochecknoinits
16-
- exportloopref
1712
- gocheckcompilerdirectives
1813
- goconst
1914
- gocritic
20-
# - gocyclo
21-
# - godox
22-
- gofmt
23-
- gofumpt
24-
- goimports
25-
# - golint - deprecated since v1.41. revive will be used instead
26-
- revive
2715
- gosec
28-
- gosimple
29-
- govet
30-
- ineffassign
31-
# - interfacer
3216
- lll
3317
- loggercheck
3418
- misspell
35-
# - maligned
3619
- nakedret
3720
- nilerr
3821
- nilnil
3922
- nolintlint
4023
- prealloc
4124
- protogetter
42-
# - scopelint - deprecated since v1.39. exportloopref will be used instead
43-
- exportloopref
25+
- revive
4426
- staticcheck
45-
- stylecheck
46-
- typecheck
4727
- unconvert
48-
# - unparam
49-
- unused
50-
# - whitespace
51-
# - wsl
52-
# - gocognit
5328
- wastedassign
5429
- whitespace
55-
- nolintlint
56-
- asciicheck
57-
- dupword
58-
30+
settings:
31+
dogsled:
32+
max-blank-identifiers: 3
33+
dupl:
34+
threshold: 200
35+
misspell:
36+
locale: US
37+
exclusions:
38+
generated: lax
39+
presets:
40+
- comments
41+
- common-false-positives
42+
- legacy
43+
- std-error-handling
44+
rules:
45+
- linters:
46+
- gosec
47+
- revive
48+
path: _test\.go
49+
- linters:
50+
- lll
51+
source: https://
52+
paths:
53+
- third_party$
54+
- builtin$
55+
- examples$
5956
issues:
60-
exclude-rules:
61-
- path: _test\.go
62-
linters:
63-
- gosec
64-
- revive
65-
- linters:
66-
- lll
67-
source: "https://"
68-
max-same-issues: 50
69-
70-
linters-settings:
71-
dogsled:
72-
max-blank-identifiers: 3
73-
golint:
74-
min-confidence: 0
75-
maligned:
76-
suggest-new: true
77-
misspell:
78-
locale: US
79-
goimports:
80-
local-prefixes: github.com/celestiaorg
81-
dupl:
82-
threshold: 200
83-
gofumpt:
84-
extra-rules: true
57+
max-same-issues: 15
58+
formatters:
59+
enable:
60+
- gofmt
61+
- gofumpt
62+
- goimports
63+
- golines
64+
settings:
65+
gofumpt:
66+
extra-rules: true
67+
goimports:
68+
local-prefixes:
69+
- github.com/celestiaorg
70+
exclusions:
71+
generated: lax
72+
paths:
73+
- third_party$
74+
- builtin$
75+
- examples$

‎headertest/store.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func NewDummyStore(t *testing.T) *Store[*DummyHeader] {
2424
}
2525

2626
// NewStore creates a generic mock store supporting different type of Headers based on Generator.
27-
func NewStore[H header.Header[H]](t *testing.T, gen Generator[H], numHeaders int) *Store[H] {
27+
func NewStore[H header.Header[H]](_ *testing.T, gen Generator[H], numHeaders int) *Store[H] {
2828
store := &Store[H]{
2929
Headers: make(map[uint64]H),
3030
HeadHeight: 0,
@@ -44,14 +44,14 @@ func NewStore[H header.Header[H]](t *testing.T, gen Generator[H], numHeaders int
4444
func (m *Store[H]) Init(context.Context, H) error { return nil }
4545

4646
func (m *Store[H]) Height() uint64 {
47-
return uint64(m.HeadHeight)
47+
return m.HeadHeight
4848
}
4949

5050
func (m *Store[H]) Head(context.Context, ...header.HeadOption[H]) (H, error) {
5151
return m.Headers[m.HeadHeight], nil
5252
}
5353

54-
func (m *Store[H]) Get(ctx context.Context, hash header.Hash) (H, error) {
54+
func (m *Store[H]) Get(_ context.Context, hash header.Hash) (H, error) {
5555
for _, header := range m.Headers {
5656
if bytes.Equal(header.Hash(), hash) {
5757
return header, nil
@@ -61,7 +61,7 @@ func (m *Store[H]) Get(ctx context.Context, hash header.Hash) (H, error) {
6161
return zero, header.ErrNotFound
6262
}
6363

64-
func (m *Store[H]) GetByHeight(ctx context.Context, height uint64) (H, error) {
64+
func (m *Store[H]) GetByHeight(_ context.Context, height uint64) (H, error) {
6565
if header, exists := m.Headers[height]; exists {
6666
return header, nil
6767
}
@@ -79,7 +79,7 @@ func (m *Store[H]) GetRangeByHeight(ctx context.Context, fromHead H, to uint64)
7979
return m.getRangeByHeight(ctx, from, to)
8080
}
8181

82-
func (m *Store[H]) getRangeByHeight(ctx context.Context, from, to uint64) ([]H, error) {
82+
func (m *Store[H]) getRangeByHeight(_ context.Context, from, to uint64) ([]H, error) {
8383
if to <= from {
8484
return nil, fmt.Errorf("malformed range, from: %d, to: %d", from, to)
8585
}
@@ -107,7 +107,7 @@ func (m *Store[H]) HasAt(_ context.Context, height uint64) bool {
107107
return height != 0 && m.HeadHeight >= height
108108
}
109109

110-
func (m *Store[H]) Append(ctx context.Context, headers ...H) error {
110+
func (m *Store[H]) Append(_ context.Context, headers ...H) error {
111111
for _, header := range headers {
112112
m.Headers[header.Height()] = header
113113
// set head

‎headertest/subscriber.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func (mhs *Subscriber[H]) Subscribe() (header.Subscription[H], error) {
2222
return mhs, nil
2323
}
2424

25-
func (mhs *Subscriber[H]) NextHeader(ctx context.Context) (H, error) {
25+
func (mhs *Subscriber[H]) NextHeader(_ context.Context) (H, error) {
2626
defer func() {
2727
if len(mhs.Headers) > 1 {
2828
// pop the already-returned header
@@ -40,4 +40,5 @@ func (mhs *Subscriber[H]) NextHeader(ctx context.Context) (H, error) {
4040
}
4141

4242
func (mhs *Subscriber[H]) Stop(context.Context) error { return nil }
43-
func (mhs *Subscriber[H]) Cancel() {}
43+
44+
func (mhs *Subscriber[H]) Cancel() {}

‎internal/otelattr/attribute_uint64.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package otelattr
2+
3+
import (
4+
"fmt"
5+
"math"
6+
7+
"go.opentelemetry.io/otel/attribute"
8+
)
9+
10+
// Uint64 returns an attribute.KeyValue for the given uint64.
11+
// TODO(@Wondertan): Otel doesn't support uin64 for unknown reason while it should.
12+
//
13+
// This helper localizes the unsafe conversion from uint64 to int64 in a single place.
14+
func Uint64(key string, value uint64) attribute.KeyValue {
15+
if value > math.MaxInt64 {
16+
return attribute.String(key, fmt.Sprintf("%d overflows int64", value))
17+
}
18+
return attribute.Int64(key, int64(value))
19+
}

‎p2p/exchange.go‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"go.opentelemetry.io/otel/trace"
2121

2222
"github.com/celestiaorg/go-header"
23+
otelattr "github.com/celestiaorg/go-header/internal/otelattr"
2324
p2p_pb "github.com/celestiaorg/go-header/p2p/pb"
2425
)
2526

@@ -94,7 +95,7 @@ func NewExchange[H header.Header[H]](
9495
return ex, nil
9596
}
9697

97-
func (ex *Exchange[H]) Start(ctx context.Context) error {
98+
func (ex *Exchange[H]) Start(context.Context) error {
9899
ex.ctx, ex.cancel = context.WithCancel(context.Background())
99100
log.Infow("client: starting client", "protocol ID", ex.protocolID)
100101

@@ -255,7 +256,7 @@ func (ex *Exchange[H]) GetByHeight(ctx context.Context, height uint64) (H, error
255256
log.Debugw("requesting header", "height", height)
256257
ctx, span := tracerClient.Start(ctx, "get-by-height",
257258
trace.WithAttributes(
258-
attribute.Int64("height", int64(height)),
259+
otelattr.Uint64("height", height),
259260
))
260261
defer span.End()
261262
var zero H
@@ -288,17 +289,29 @@ func (ex *Exchange[H]) GetRangeByHeight(
288289
) ([]H, error) {
289290
ctx, span := tracerClient.Start(ctx, "get-range-by-height",
290291
trace.WithAttributes(
291-
attribute.Int64("from", int64(from.Height())),
292-
attribute.Int64("to", int64(to)),
293-
))
292+
otelattr.Uint64("from", from.Height()),
293+
otelattr.Uint64("to", to),
294+
),
295+
)
294296
defer span.End()
295297
session := newSession[H](
296-
ex.ctx, ex.host, ex.peerTracker, ex.protocolID, ex.Params.RequestTimeout, ex.metrics, withValidation(from),
298+
ex.ctx,
299+
ex.host,
300+
ex.peerTracker,
301+
ex.protocolID,
302+
ex.Params.RequestTimeout,
303+
ex.metrics,
304+
withValidation(from),
297305
)
298306
defer session.close()
299307
// we request the next header height that we don't have: `fromHead`+1
300308
amount := to - (from.Height() + 1)
301-
result, err := session.getRangeByHeight(ctx, from.Height()+1, amount, ex.Params.MaxHeadersPerRangeRequest)
309+
result, err := session.getRangeByHeight(
310+
ctx,
311+
from.Height()+1,
312+
amount,
313+
ex.Params.MaxHeadersPerRangeRequest,
314+
)
302315
if err != nil {
303316
span.SetStatus(codes.Error, err.Error())
304317
return nil, err
@@ -446,7 +459,9 @@ func bestHead[H header.Header[H]](result []H) (H, error) {
446459
return res, nil
447460
}
448461
}
449-
log.Debug("could not find latest header received from at least two peers, returning header with the max height")
462+
log.Debug(
463+
"could not find latest header received from at least two peers, returning header with the max height",
464+
)
450465
// otherwise return header with the max height
451466
return result[0], nil
452467
}

‎p2p/exchange_metrics.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,12 @@ func (m *exchangeMetrics) observeMetrics(_ context.Context, obs metric.Observer)
106106
return nil
107107
}
108108

109-
func (m *exchangeMetrics) head(ctx context.Context, duration time.Duration, headersReceived int, tp, status string) {
109+
func (m *exchangeMetrics) head(
110+
ctx context.Context,
111+
duration time.Duration,
112+
headersReceived int,
113+
tp, status string,
114+
) {
110115
m.observe(ctx, func(ctx context.Context) {
111116
m.headRequestTimeInst.Record(ctx,
112117
duration.Seconds(),
@@ -119,7 +124,12 @@ func (m *exchangeMetrics) head(ctx context.Context, duration time.Duration, head
119124
})
120125
}
121126

122-
func (m *exchangeMetrics) response(ctx context.Context, size uint64, duration time.Duration, err error) {
127+
func (m *exchangeMetrics) response(
128+
ctx context.Context,
129+
size int,
130+
duration time.Duration,
131+
err error,
132+
) {
123133
m.observe(ctx, func(ctx context.Context) {
124134
m.responseSizeInst.Record(ctx,
125135
int64(size),
@@ -145,7 +155,7 @@ func (m *exchangeMetrics) peersDisconnected(num int) {
145155
}
146156

147157
func (m *exchangeMetrics) peerBlocked() {
148-
m.observe(context.Background(), func(ctx context.Context) {
158+
m.observe(context.Background(), func(context.Context) {
149159
m.blockedPeersNum.Add(1)
150160
})
151161
}

0 commit comments

Comments
 (0)