Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci(lint): golangci-lint and fieldalignment #132

Closed
wants to merge 13 commits into from
41 changes: 39 additions & 2 deletions .github/workflows/go-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,54 @@ on:
branches:
- main
pull_request:

jobs:
setup:
name: Setup
runs-on: ubuntu-latest
outputs:
go-version: ${{ steps.go-version.outputs.go-version }}

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Read .go-version file
id: go-version
run: |
echo "go-version=$(cat .go-version)" >> $GITHUB_OUTPUT

lint:
needs: [setup]
name: Lint
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- uses: actions/setup-go@v4
with:
go-version: ${{ needs.setup.outputs.go-version }}

- name: golangci-lint
uses: golangci/[email protected]
with:
args: --timeout 10m
version: v1.55
skip-pkg-cache: true
skip-build-cache: true

build:
needs: [setup, lint]
name: Build
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- name: Set up Go
uses: actions/setup-go@v3
uses: actions/setup-go@v4
with:
go-version: 1.21
go-version: ${{ needs.setup.outputs.go-version }}

- name: Build
run: go build -v ./...
Expand Down
1 change: 1 addition & 0 deletions .go-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1.21
72 changes: 72 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
run:
timeout: 5m

linters:
enable:
- bodyclose
# - depguard as of v1.54.2, the default config throws errors on our repo
- dogsled
- dupl
- errcheck
# - funlen
# - gochecknoglobals
# - gochecknoinits
- goconst
- gocritic
# - gocyclo
# - godox
- gofmt
- goimports
# - golint - deprecated since v1.41. revive will be used instead
- revive
- gosec
- gosimple
- govet
- ineffassign
# - interfacer
- lll
- misspell
# - maligned
- nakedret
- prealloc
# - scopelint - deprecated since v1.39. exportloopref will be used instead
- exportloopref
- staticcheck
- stylecheck
- typecheck
- unconvert
# - unparam
- unused
# - whitespace
# - wsl
# - gocognit
- nolintlint
- asciicheck

issues:
exclude-rules:
- path: _test\.go
linters:
- gosec
- govet
- linters:
- lll
source: "https://"
max-same-issues: 50

linters-settings:
dogsled:
max-blank-identifiers: 3
golint:
min-confidence: 0
maligned:
suggest-new: true
misspell:
locale: US
goimports:
local-prefixes: github.com/celestiaorg/go-header
dupl:
threshold: 200
govet:
enable:
- fieldalignment
6 changes: 4 additions & 2 deletions headertest/dummy_header.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ import (
var ErrDummyVerify = errors.New("dummy verify error")

type DummyHeader struct {
Timestamp time.Time

Chainid string
PreviousHash header.Hash
HeightI uint64
Timestamp time.Time

hash header.Hash

HeightI uint64

// VerifyFailure allows for testing scenarios where a header would fail
// verification. When set to true, it forces a failure.
VerifyFailure bool
Expand Down
12 changes: 6 additions & 6 deletions headertest/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func NewDummyStore(t *testing.T) *Store[*DummyHeader] {
}

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

func (m *Store[H]) Height() uint64 {
return uint64(m.HeadHeight)
return m.HeadHeight
}

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

func (m *Store[H]) Get(ctx context.Context, hash header.Hash) (H, error) {
func (m *Store[H]) Get(_ context.Context, hash header.Hash) (H, error) {
for _, header := range m.Headers {
if bytes.Equal(header.Hash(), hash) {
return header, nil
Expand All @@ -60,7 +60,7 @@ func (m *Store[H]) Get(ctx context.Context, hash header.Hash) (H, error) {
return zero, header.ErrNotFound
}

func (m *Store[H]) GetByHeight(ctx context.Context, height uint64) (H, error) {
func (m *Store[H]) GetByHeight(_ context.Context, height uint64) (H, error) {
return m.Headers[height], nil
}

Expand All @@ -74,7 +74,7 @@ func (m *Store[H]) GetRangeByHeight(ctx context.Context, fromHead H, to uint64)
return m.getRangeByHeight(ctx, from, to)
}

func (m *Store[H]) getRangeByHeight(ctx context.Context, from, to uint64) ([]H, error) {
func (m *Store[H]) getRangeByHeight(_ context.Context, from, to uint64) ([]H, error) {
amount := to - from
headers := make([]H, amount)

Expand All @@ -99,7 +99,7 @@ func (m *Store[H]) HasAt(_ context.Context, height uint64) bool {
return height != 0 && m.HeadHeight >= height
}

func (m *Store[H]) Append(ctx context.Context, headers ...H) error {
func (m *Store[H]) Append(_ context.Context, headers ...H) error {
for _, header := range headers {
m.Headers[header.Height()] = header
// set head
Expand Down
2 changes: 1 addition & 1 deletion headertest/subscriber.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (mhs *Subscriber[H]) Subscribe() (header.Subscription[H], error) {
return mhs, nil
}

func (mhs *Subscriber[H]) NextHeader(ctx context.Context) (H, error) {
func (mhs *Subscriber[H]) NextHeader(_ context.Context) (H, error) {
defer func() {
if len(mhs.Headers) > 1 {
// pop the already-returned header
Expand Down
2 changes: 1 addition & 1 deletion p2p/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ var maxUntrustedHeadRequests = 4
// Exchange enables sending outbound HeaderRequests to the network as well as
// handling inbound HeaderRequests from the network.
type Exchange[H header.Header[H]] struct {
ctx context.Context
cancel context.CancelFunc
ctx context.Context

protocolID protocol.ID
host host.Host
Expand Down
9 changes: 6 additions & 3 deletions p2p/exchange_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,20 @@ type exchangeMetrics struct {
responseSizeInst metric.Int64Histogram
responseTimeInst metric.Float64Histogram

trackerPeersNum atomic.Int64
trackedPeersNumInst metric.Int64ObservableGauge
trackedPeersNumReg metric.Registration

disconnectedPeersNum atomic.Int64
disconnectedPeersNumInst metric.Int64ObservableGauge
disconnectedPeersNumReg metric.Registration

blockedPeersNum atomic.Int64
blockedPeersNumInst metric.Int64ObservableGauge
blockedPeersNumReg metric.Registration

trackerPeersNum atomic.Int64

disconnectedPeersNum atomic.Int64

blockedPeersNum atomic.Int64
}

func newExchangeMetrics() (m *exchangeMetrics, err error) {
Expand Down
23 changes: 18 additions & 5 deletions p2p/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ func TestExchange_RequestHead(t *testing.T) {
})

tests := []struct {
requestFromTrusted bool
lastHeader *headertest.DummyHeader
expectedHeight uint64
expectedHash header.Hash
expectedHeight uint64
requestFromTrusted bool
}{
// routes to trusted peer only
{
Expand Down Expand Up @@ -607,7 +607,12 @@ func quicHosts(t *testing.T, n int) []libhost.Host {
return hosts
}

func client(ctx context.Context, t *testing.T, host libhost.Host, trusted []peer.ID) *Exchange[*headertest.DummyHeader] {
func client(
ctx context.Context,
t *testing.T,
host libhost.Host,
trusted []peer.ID,
) *Exchange[*headertest.DummyHeader] {
client, err := NewExchange[*headertest.DummyHeader](host, trusted, nil)
require.NoError(t, err)

Expand All @@ -621,7 +626,12 @@ func client(ctx context.Context, t *testing.T, host libhost.Host, trusted []peer
return client
}

func server(ctx context.Context, t *testing.T, host libhost.Host, store header.Store[*headertest.DummyHeader]) *ExchangeServer[*headertest.DummyHeader] {
func server(
ctx context.Context,
t *testing.T,
host libhost.Host,
store header.Store[*headertest.DummyHeader],
) *ExchangeServer[*headertest.DummyHeader] {
server, err := NewExchangeServer[*headertest.DummyHeader](host, store)
require.NoError(t, err)
err = server.Start(ctx)
Expand All @@ -643,7 +653,10 @@ func (t *timedOutStore) HasAt(_ context.Context, _ uint64) bool {
return true
}

func (t *timedOutStore) Head(context.Context, ...header.HeadOption[*headertest.DummyHeader]) (*headertest.DummyHeader, error) {
func (t *timedOutStore) Head(
context.Context,
...header.HeadOption[*headertest.DummyHeader],
) (*headertest.DummyHeader, error) {
time.Sleep(t.timeout)
return nil, header.ErrNoHead
}
26 changes: 12 additions & 14 deletions p2p/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ type Option[T parameters] func(*T)

// ServerParameters is the set of parameters that must be configured for the exchange.
type ServerParameters struct {
// networkID is a network that will be used to create a protocol.ID
// Is empty by default
networkID string
// WriteDeadline sets the timeout for sending messages to the stream
WriteDeadline time.Duration
// ReadDeadline sets the timeout for reading messages from the stream
ReadDeadline time.Duration
// RangeRequestTimeout defines a timeout after which the session will try to re-request headers
// from another peer.
RangeRequestTimeout time.Duration
// networkID is a network that will be used to create a protocol.ID
// Is empty by default
networkID string
// metrics is a flag that enables metrics collection.
metrics bool
}
Expand Down Expand Up @@ -57,7 +57,7 @@ func (p *ServerParameters) Validate() error {

func WithMetrics[T parameters]() Option[T] {
return func(p *T) {
switch t := any(p).(type) { //nolint:gocritic
switch t := any(p).(type) {
case *ServerParameters:
t.metrics = true
case *ClientParameters:
Expand Down Expand Up @@ -123,19 +123,19 @@ func WithParams[T parameters](params T) Option[T] {

// ClientParameters is the set of parameters that must be configured for the exchange.
type ClientParameters struct {
// pidstore is an optional interface used to periodically dump peers
pidstore PeerIDStore
// networkID is a network that will be used to create a protocol.ID
networkID string
// chainID is an identifier of the chain.
chainID string
// MaxHeadersPerRangeRequest defines the max amount of headers that can be requested per 1 request.
MaxHeadersPerRangeRequest uint64
// RangeRequestTimeout defines a timeout after which the session will try to re-request headers
// from another peer.
RangeRequestTimeout time.Duration
// networkID is a network that will be used to create a protocol.ID
networkID string
// chainID is an identifier of the chain.
chainID string
// metrics is a flag that enables metrics collection.
metrics bool
// pidstore is an optional interface used to periodically dump peers
pidstore PeerIDStore
}

// DefaultClientParameters returns the default params to configure the store.
Expand Down Expand Up @@ -178,8 +178,7 @@ func WithMaxHeadersPerRangeRequest[T ClientParameters](amount uint64) Option[T]
// `chainID` parameter.
func WithChainID[T ClientParameters](chainID string) Option[T] {
return func(p *T) {
switch t := any(p).(type) { //nolint:gocritic
case *ClientParameters:
if t, ok := any(p).(*ClientParameters); ok {
t.chainID = chainID
}
}
Expand All @@ -189,8 +188,7 @@ func WithChainID[T ClientParameters](chainID string) Option[T] {
// inside the peerTracker.
func WithPeerIDStore[T ClientParameters](pidstore PeerIDStore) Option[T] {
return func(p *T) {
switch t := any(p).(type) { //nolint:gocritic
case *ClientParameters:
if t, ok := any(p).(*ClientParameters); ok {
t.pidstore = pidstore
}
}
Expand Down
Loading