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(golangci-lint): add unparam linter #6263

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ linters:
# - tparallel
- typecheck
# - unconvert
# - unparam
- unparam
- unused
# - usestdlibvars
# - varnamelen
Expand Down
14 changes: 6 additions & 8 deletions activation/activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,10 @@ func publishAtxV1(
nodeID types.NodeID,
posEpoch types.EpochID,
currLayer *types.LayerID,
buildNIPostLayerDuration uint32,
) *wire.ActivationTxV1 {
tb.Helper()
var watx wire.ActivationTxV1
publishAtx(tb, tab, nodeID, posEpoch, currLayer, buildNIPostLayerDuration,
publishAtx(tb, tab, nodeID, posEpoch, currLayer,
func(_ context.Context, _ string, got []byte) error {
return codec.Decode(got, &watx)
})
Expand All @@ -148,7 +147,6 @@ func publishAtx(
nodeID types.NodeID,
posEpoch types.EpochID,
currLayer *types.LayerID, // pointer to keep current layer consistent across calls
buildNIPostLayerDuration uint32,
onPublish func(context.Context, string, []byte) error,
) {
tb.Helper()
Expand All @@ -175,7 +173,7 @@ func publishAtx(
tab.mnipost.EXPECT().BuildNIPost(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
func(_ context.Context, _ *signing.EdSigner, _ types.Hash32, _ *types.NIPostChallenge,
) (*nipost.NIPostState, error) {
*currLayer = currLayer.Add(buildNIPostLayerDuration)
*currLayer = currLayer.Add(layersPerEpoch)
return newNIPostWithPoet(tb, types.RandomHash().Bytes()), nil
})
ch := make(chan struct{})
Expand Down Expand Up @@ -358,15 +356,15 @@ func TestBuilder_PublishActivationTx_HappyFlow(t *testing.T) {
// create and publish ATX
tab.mclock.EXPECT().CurrentLayer().Return(currLayer).Times(4)
tab.mValidator.EXPECT().VerifyChain(gomock.Any(), prevAtx.ID(), tab.goldenATXID, gomock.Any())
atx1 := publishAtxV1(t, tab, sig.NodeID(), posEpoch, &currLayer, layersPerEpoch)
atx1 := publishAtxV1(t, tab, sig.NodeID(), posEpoch, &currLayer)
require.NotNil(t, atx1)
require.Equal(t, prevAtx.ID(), atx1.PositioningATXID)

// create and publish another ATX
currLayer = (posEpoch + 1).FirstLayer()
tab.mclock.EXPECT().CurrentLayer().Return(currLayer).Times(4)
tab.mValidator.EXPECT().VerifyChain(gomock.Any(), atx1.ID(), tab.goldenATXID, gomock.Any())
atx2 := publishAtxV1(t, tab, sig.NodeID(), atx1.PublishEpoch, &currLayer, layersPerEpoch)
atx2 := publishAtxV1(t, tab, sig.NodeID(), atx1.PublishEpoch, &currLayer)
require.NotNil(t, atx2)
require.NotEqual(t, atx1, atx2)
require.Equal(t, atx1.PublishEpoch+1, atx2.PublishEpoch)
Expand Down Expand Up @@ -655,7 +653,7 @@ func TestBuilder_PublishActivationTx_RebuildNIPostWhenTargetEpochPassed(t *testi
tab.mclock.EXPECT().CurrentLayer().DoAndReturn(func() types.LayerID { return currLayer }).AnyTimes()
tab.mnipost.EXPECT().ResetState(sig.NodeID()).Return(nil)
tab.mValidator.EXPECT().VerifyChain(gomock.Any(), posAtx.ID(), tab.goldenATXID, gomock.Any())
built2 := publishAtxV1(t, tab, sig.NodeID(), posEpoch, &currLayer, layersPerEpoch)
built2 := publishAtxV1(t, tab, sig.NodeID(), posEpoch, &currLayer)
require.NotNil(t, built2)
require.NotEqual(t, built.NIPostChallengeV1, built2.NIPostChallengeV1)
require.Equal(t, posEpoch+1, built2.PublishEpoch)
Expand Down Expand Up @@ -694,7 +692,7 @@ func TestBuilder_PublishActivationTx_NoPrevATX(t *testing.T) {

// create and publish ATX
tab.mclock.EXPECT().CurrentLayer().Return(currLayer).AnyTimes()
atx := publishAtxV1(t, tab, sig.NodeID(), posEpoch, &currLayer, layersPerEpoch)
atx := publishAtxV1(t, tab, sig.NodeID(), posEpoch, &currLayer)
require.NotNil(t, atx)

// state is cleaned up
Expand Down
6 changes: 3 additions & 3 deletions activation/builder_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestBuilder_BuildsInitialAtxV2(t *testing.T) {
tab.mValidator.EXPECT().PostV2(gomock.Any(), sig.NodeID(), commitment, &initialPost, post.Challenge, post.NumUnits)

var atx wire.ActivationTxV2
publishAtx(t, tab, sig.NodeID(), posEpoch, &layer, layersPerEpoch,
publishAtx(t, tab, sig.NodeID(), posEpoch, &layer,
func(_ context.Context, _ string, got []byte) error {
require.NoError(t, codec.Decode(got, &atx))

Expand Down Expand Up @@ -86,7 +86,7 @@ func TestBuilder_SwitchesToBuildV2(t *testing.T) {

// create and publish ATX V1
tab.mclock.EXPECT().CurrentLayer().Return(layer).Times(4)
atx1 := publishAtxV1(t, tab, sig.NodeID(), posEpoch, &layer, layersPerEpoch)
atx1 := publishAtxV1(t, tab, sig.NodeID(), posEpoch, &layer)
require.NotNil(t, atx1)

// create and publish ATX V2
Expand All @@ -95,7 +95,7 @@ func TestBuilder_SwitchesToBuildV2(t *testing.T) {
tab.mclock.EXPECT().CurrentLayer().Return(layer).Times(4)
tab.mValidator.EXPECT().VerifyChain(gomock.Any(), atx1.ID(), tab.goldenATXID, gomock.Any())
var atx2 wire.ActivationTxV2
publishAtx(t, tab, sig.NodeID(), posEpoch, &layer, layersPerEpoch,
publishAtx(t, tab, sig.NodeID(), posEpoch, &layer,
func(_ context.Context, _ string, got []byte) error {
return codec.Decode(got, &atx2)
})
Expand Down
10 changes: 5 additions & 5 deletions activation/post_supervisor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func testSetupOpts(t *testing.T) PostSetupOpts {
return opts
}

func newPostManager(t *testing.T, cfg PostConfig, opts PostSetupOpts) *PostSetupManager {
func newPostManager(t *testing.T, cfg PostConfig) *PostSetupManager {
t.Helper()
ctrl := gomock.NewController(t)
validator := NewMocknipostValidator(ctrl)
Expand Down Expand Up @@ -159,7 +159,7 @@ func Test_PostSupervisor_StartsServiceCmd(t *testing.T) {
require.NoError(t, err)

ctrl := gomock.NewController(t)
mgr := newPostManager(t, postCfg, postOpts)
mgr := newPostManager(t, postCfg)
builder := NewMockAtxBuilder(ctrl)
builder.EXPECT().Register(sig)
ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
Expand Down Expand Up @@ -196,7 +196,7 @@ func Test_PostSupervisor_Restart_Possible(t *testing.T) {
require.NoError(t, err)

ctrl := gomock.NewController(t)
mgr := newPostManager(t, postCfg, postOpts)
mgr := newPostManager(t, postCfg)
builder := NewMockAtxBuilder(ctrl)
builder.EXPECT().Register(sig)
ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
Expand Down Expand Up @@ -227,7 +227,7 @@ func Test_PostSupervisor_LogFatalOnCrash(t *testing.T) {
require.NoError(t, err)

ctrl := gomock.NewController(t)
mgr := newPostManager(t, postCfg, postOpts)
mgr := newPostManager(t, postCfg)
builder := NewMockAtxBuilder(ctrl)
builder.EXPECT().Register(sig)
ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
Expand Down Expand Up @@ -260,7 +260,7 @@ func Test_PostSupervisor_LogFatalOnInvalidConfig(t *testing.T) {
require.NoError(t, err)

ctrl := gomock.NewController(t)
mgr := newPostManager(t, postCfg, postOpts)
mgr := newPostManager(t, postCfg)
builder := NewMockAtxBuilder(ctrl)
builder.EXPECT().Register(sig)
ps := NewPostSupervisor(log.Named("supervisor"), postCfg, provingOpts, mgr, builder)
Expand Down
4 changes: 2 additions & 2 deletions api/grpcserver/grpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2451,7 +2451,7 @@ func TestVMAccountUpdates(t *testing.T) {
require.Equal(t, len(accounts), i)
}

func createAtxs(tb testing.TB, epoch types.EpochID, atxids []types.ATXID) []*types.ActivationTx {
func createAtxs(epoch types.EpochID, atxids []types.ATXID) []*types.ActivationTx {
all := make([]*types.ActivationTx, 0, len(atxids))
for _, id := range atxids {
atx := &types.ActivationTx{
Expand Down Expand Up @@ -2487,7 +2487,7 @@ func TestMeshService_EpochStream(t *testing.T) {

epoch := types.EpochID(3)
atxids := types.RandomActiveSet(100)
all := createAtxs(t, epoch, atxids)
all := createAtxs(epoch, atxids)
var expected, got []types.ATXID
for i, vatx := range all {
require.NoError(t, atxs.Add(db, vatx, types.AtxBlob{}))
Expand Down
18 changes: 8 additions & 10 deletions api/grpcserver/post_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
func launchPostSupervisor(
tb testing.TB,
log *zap.Logger,
cfg Config,
serviceCfg activation.PostSupervisorConfig,
postOpts activation.PostSetupOpts,
) (types.NodeID, func()) {
Expand Down Expand Up @@ -74,7 +73,6 @@ func launchPostSupervisor(
func launchPostSupervisorTLS(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and launchPostSupervisor are basically identical, except that launchPostSupervisor doesn't yet use sql.InMemoryTest() and this function has the wrong name for the logger of the post manager.

I think launchPostSupervisorTLS can be removed and in the one place where it is called be replaced with launchPostSupervisor

tb testing.TB,
log *zap.Logger,
cfg Config,
serviceCfg activation.PostSupervisorConfig,
postOpts activation.PostSetupOpts,
) (types.NodeID, func()) {
Expand Down Expand Up @@ -102,7 +100,7 @@ func launchPostSupervisorTLS(
close(ch)
return ch
})
db := sql.InMemory()
db := sql.InMemoryTest(tb)
logger := log.Named("post supervisor")
mgr, err := activation.NewPostSetupManager(postCfg, logger, db, atxsdata.New(), goldenATXID, syncer, validator)
require.NoError(tb, err)
Expand Down Expand Up @@ -130,7 +128,7 @@ func Test_GenerateProof(t *testing.T) {
serviceCfg := activation.DefaultTestPostServiceConfig()
serviceCfg.NodeAddress = fmt.Sprintf("http://%s", cfg.PublicListener)

id, postCleanup := launchPostSupervisor(t, log.Named("supervisor"), cfg, serviceCfg, opts)
id, postCleanup := launchPostSupervisor(t, log.Named("supervisor"), serviceCfg, opts)
t.Cleanup(postCleanup)

var client activation.PostClient
Expand Down Expand Up @@ -181,7 +179,7 @@ func Test_GenerateProof_TLS(t *testing.T) {
serviceCfg.Cert = filepath.Join(certDir, clientCertName)
serviceCfg.Key = filepath.Join(certDir, clientKeyName)

id, postCleanup := launchPostSupervisorTLS(t, log.Named("supervisor"), cfg, serviceCfg, opts)
id, postCleanup := launchPostSupervisorTLS(t, log.Named("supervisor"), serviceCfg, opts)
t.Cleanup(postCleanup)

var client activation.PostClient
Expand Down Expand Up @@ -228,7 +226,7 @@ func Test_GenerateProof_Cancel(t *testing.T) {
serviceCfg := activation.DefaultTestPostServiceConfig()
serviceCfg.NodeAddress = fmt.Sprintf("http://%s", cfg.PublicListener)

id, postCleanup := launchPostSupervisor(t, log.Named("supervisor"), cfg, serviceCfg, opts)
id, postCleanup := launchPostSupervisor(t, log.Named("supervisor"), serviceCfg, opts)
t.Cleanup(postCleanup)

var client activation.PostClient
Expand Down Expand Up @@ -268,7 +266,7 @@ func Test_Metadata(t *testing.T) {
serviceCfg := activation.DefaultTestPostServiceConfig()
serviceCfg.NodeAddress = fmt.Sprintf("http://%s", cfg.PublicListener)

id, postCleanup := launchPostSupervisor(t, log.Named("supervisor"), cfg, serviceCfg, opts)
id, postCleanup := launchPostSupervisor(t, log.Named("supervisor"), serviceCfg, opts)
t.Cleanup(postCleanup)

var client activation.PostClient
Expand Down Expand Up @@ -313,15 +311,15 @@ func Test_GenerateProof_MultipleServices(t *testing.T) {
serviceCfg.NodeAddress = fmt.Sprintf("http://%s", cfg.PublicListener)

// all but one should not be able to register to the node (i.e. open a stream to it).
id, postCleanup := launchPostSupervisor(t, log.Named("supervisor1"), cfg, serviceCfg, opts)
id, postCleanup := launchPostSupervisor(t, log.Named("supervisor1"), serviceCfg, opts)
t.Cleanup(postCleanup)

opts.DataDir = t.TempDir()
_, postCleanup = launchPostSupervisor(t, log.Named("supervisor2"), cfg, serviceCfg, opts)
_, postCleanup = launchPostSupervisor(t, log.Named("supervisor2"), serviceCfg, opts)
t.Cleanup(postCleanup)

opts.DataDir = t.TempDir()
_, postCleanup = launchPostSupervisor(t, log.Named("supervisor3"), cfg, serviceCfg, opts)
_, postCleanup = launchPostSupervisor(t, log.Named("supervisor3"), serviceCfg, opts)
t.Cleanup(postCleanup)

var client activation.PostClient
Expand Down
16 changes: 5 additions & 11 deletions api/grpcserver/v2alpha1/layer.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@
dbChan := make(chan *spacemeshv2alpha1.Layer, 100)
errChan := make(chan error, 1)

ops, err := toLayerOperations(toLayerRequest(request))
if err != nil {
return status.Error(codes.InvalidArgument, err.Error())
}
ops := toLayerOperations(toLayerRequest(request))
// send db data to chan to avoid buffer overflow
go func() {
defer close(dbChan)
Expand Down Expand Up @@ -201,10 +198,7 @@
return nil, status.Error(codes.InvalidArgument, "limit must be set to <= 100")
}

ops, err := toLayerOperations(request)
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
ops := toLayerOperations(request)

rst := make([]*spacemeshv2alpha1.Layer, 0, request.Limit)
if err := layers.IterateLayersWithBlockOps(s.db, ops, func(layer *layers.Layer) bool {
Expand All @@ -217,10 +211,10 @@
return &spacemeshv2alpha1.LayerList{Layers: rst}, nil
}

func toLayerOperations(filter *spacemeshv2alpha1.LayerRequest) (builder.Operations, error) {
func toLayerOperations(filter *spacemeshv2alpha1.LayerRequest) builder.Operations {
ops := builder.Operations{}
if filter == nil {
return ops, nil
return ops

Check warning on line 217 in api/grpcserver/v2alpha1/layer.go

View check run for this annotation

Codecov / codecov/patch

api/grpcserver/v2alpha1/layer.go#L217

Added line #L217 was not covered by tests
}

if filter.StartLayer != 0 {
Expand Down Expand Up @@ -259,7 +253,7 @@
})
}

return ops, nil
return ops
}

func toLayer(layer *layers.Layer) *spacemeshv2alpha1.Layer {
Expand Down
19 changes: 9 additions & 10 deletions atxsdata/warmup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ import (
"github.com/spacemeshos/go-spacemesh/sql/mocks"
)

var nonce = types.VRFPostIndex(1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about inlining types.VRFPostIndex(1) in gatx() instead of creating a global?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also ideally it should not be the same value every time 😅


func gatx(
id types.ATXID,
epoch types.EpochID,
smesher types.NodeID,
nonce types.VRFPostIndex,
) types.ActivationTx {
atx := &types.ActivationTx{
NumUnits: 1,
Expand All @@ -40,14 +41,13 @@ func TestWarmup(t *testing.T) {
t.Run("sanity", func(t *testing.T) {
db := sql.InMemory()
applied := types.LayerID(10)
nonce := types.VRFPostIndex(1)
data := []types.ActivationTx{
gatx(types.ATXID{1, 1}, 1, types.NodeID{1}, nonce),
gatx(types.ATXID{1, 2}, 1, types.NodeID{2}, nonce),
gatx(types.ATXID{2, 1}, 2, types.NodeID{1}, nonce),
gatx(types.ATXID{2, 2}, 2, types.NodeID{2}, nonce),
gatx(types.ATXID{3, 2}, 3, types.NodeID{2}, nonce),
gatx(types.ATXID{3, 3}, 3, types.NodeID{3}, nonce),
gatx(types.ATXID{1, 1}, 1, types.NodeID{1}),
gatx(types.ATXID{1, 2}, 1, types.NodeID{2}),
gatx(types.ATXID{2, 1}, 2, types.NodeID{1}),
gatx(types.ATXID{2, 2}, 2, types.NodeID{2}),
gatx(types.ATXID{3, 2}, 3, types.NodeID{2}),
gatx(types.ATXID{3, 3}, 3, types.NodeID{3}),
}
for i := range data {
require.NoError(t, atxs.Add(db, &data[i], types.AtxBlob{}))
Expand All @@ -74,8 +74,7 @@ func TestWarmup(t *testing.T) {
})
t.Run("db failures", func(t *testing.T) {
db := sql.InMemory()
nonce := types.VRFPostIndex(1)
data := gatx(types.ATXID{1, 1}, 1, types.NodeID{1}, nonce)
data := gatx(types.ATXID{1, 1}, 1, types.NodeID{1})
require.NoError(t, atxs.Add(db, &data, types.AtxBlob{}))

exec := mocks.NewMockExecutor(gomock.NewController(t))
Expand Down
4 changes: 2 additions & 2 deletions beacon/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ func (pd *ProtocolDriver) HandleFollowingVotes(ctx context.Context, peer p2p.Pee
return errUntimelyMessage
}

nodeID, err := pd.verifyFollowingVotes(ctx, m)
nodeID, err := pd.verifyFollowingVotes(m)
if err != nil {
return err
}
Expand All @@ -434,7 +434,7 @@ func (pd *ProtocolDriver) HandleFollowingVotes(ctx context.Context, peer p2p.Pee
return nil
}

func (pd *ProtocolDriver) verifyFollowingVotes(ctx context.Context, m FollowingVotingMessage) (types.NodeID, error) {
func (pd *ProtocolDriver) verifyFollowingVotes(m FollowingVotingMessage) (types.NodeID, error) {
messageBytes := codec.MustEncode(&m.FollowingVotingMessageBody)
if !pd.edVerifier.Verify(signing.BEACON_FOLLOWUP_MSG, m.SmesherID, messageBytes, m.Signature) {
return types.EmptyNodeID, fmt.Errorf("[round %v] verify signature %s: failed", types.FirstRound, m.Signature)
Expand Down
Loading
Loading