From edf1cfc9c5a45278252410a0eced6987a138566d Mon Sep 17 00:00:00 2001 From: Kaloyan Tanev Date: Thu, 18 Jul 2024 17:53:57 +0300 Subject: [PATCH] Fix all other revive linters --- .golangci.yml | 2 ++ app/log/log.go | 9 ++++++--- app/monitoringapi.go | 1 + app/version/version.go | 4 ++-- cluster/definition.go | 4 ++-- cluster/lock.go | 2 +- cluster/ssz.go | 16 ++++++++-------- core/consensus/msg.go | 10 ++++++++-- core/priority/component.go | 10 ++++++++-- core/validatorapi/router.go | 6 +++--- dkg/dkg.go | 8 ++++---- p2p/sender.go | 6 +++++- p2p/sender_internal_test.go | 3 ++- testutil/beaconmock/server.go | 7 ++++++- 14 files changed, 58 insertions(+), 30 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a3b1d943a..ccc541aed 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -86,6 +86,8 @@ linters-settings: disabled: true - name: comment-spacings disabled: true # Doesn't support latest go spec comments + - name: range-val-address + disabled: true # It is not an issue for go versions >=1.22 # Some configured revive rules - name: unhandled-error arguments: diff --git a/app/log/log.go b/app/log/log.go index 57dbfb0a0..d5d932d1c 100644 --- a/app/log/log.go +++ b/app/log/log.go @@ -51,13 +51,16 @@ func WithLogger(ctx context.Context, logger zapLogger) context.Context { } func fieldsFromCtx(ctx context.Context) []z.Field { - resp, _ := ctx.Value(ctxKey{}).([]z.Field) + resp, ok := ctx.Value(ctxKey{}).([]z.Field) + if !ok { + return []z.Field{} + } return resp } func metricsTopicFromCtx(ctx context.Context) string { - resp, _ := ctx.Value(topicKey{}).(string) - if resp == "" { + resp, ok := ctx.Value(topicKey{}).(string) + if resp == "" || !ok { return "unknown" } diff --git a/app/monitoringapi.go b/app/monitoringapi.go index 84671f5e4..82bfaa574 100644 --- a/app/monitoringapi.go +++ b/app/monitoringapi.go @@ -173,6 +173,7 @@ func startReadyChecker(ctx context.Context, tcpNode host.Host, eth2Cl eth2wrap.C } syncing, syncDistance, err := beaconNodeSyncing(ctx, eth2Cl) + //nolint:revive // skip max-control-nesting for monitoring if err != nil { err = errReadyBeaconNodeDown readyzGauge.Set(readyzBeaconNodeDown) diff --git a/app/version/version.go b/app/version/version.go index e30317580..4464a8d4b 100644 --- a/app/version/version.go +++ b/app/version/version.go @@ -139,9 +139,9 @@ func Compare(a, b SemVer) int { return 0 } else if a.patch < b.patch { return -1 - } else { - return 1 } + + return 1 } var semverRegex = regexp.MustCompile(`^v(\d+)\.(\d+)(?:\.(\d+))?(?:-(.+))?$`) diff --git a/cluster/definition.go b/cluster/definition.go index 687b79855..87e4c008a 100644 --- a/cluster/definition.go +++ b/cluster/definition.go @@ -354,7 +354,7 @@ func (d Definition) SetDefinitionHashes() (Definition, error) { return Definition{}, errors.Wrap(err, "config hash") } - d.ConfigHash = configHash[:] + d.ConfigHash = configHash[:] //nolint: revive // okay to assign to by-value receiver as we return the struct // Marshal definition hashDefinition defHash, err := hashDefinition(d, false) @@ -362,7 +362,7 @@ func (d Definition) SetDefinitionHashes() (Definition, error) { return Definition{}, errors.Wrap(err, "definition hashDefinition") } - d.DefinitionHash = defHash[:] + d.DefinitionHash = defHash[:] //nolint: revive // okay to assign to by-value receiver as we return the struct return d, nil } diff --git a/cluster/lock.go b/cluster/lock.go index 529179192..2fe6dcdb5 100644 --- a/cluster/lock.go +++ b/cluster/lock.go @@ -123,7 +123,7 @@ func (l Lock) SetLockHash() (Lock, error) { return Lock{}, err } - l.LockHash = lockHash[:] + l.LockHash = lockHash[:] //nolint: revive // okay to assign to by-value receiver as we return the struct return l, nil } diff --git a/cluster/ssz.go b/cluster/ssz.go index 8bdba545d..24f17cf23 100644 --- a/cluster/ssz.go +++ b/cluster/ssz.go @@ -37,9 +37,9 @@ func getDefinitionHashFunc(version string) (func(Definition, ssz.HashWalker, boo return hashDefinitionV1x5to7, nil } else if isAnyVersion(version, v1_8) { return hashDefinitionV1x8orLater, nil - } else { - return nil, errors.New("unknown version", z.Str("version", version)) } + + return nil, errors.New("unknown version", z.Str("version", version)) } // hashDefinition returns a config or definition hash. The config hash excludes operator ENRs and signatures @@ -607,9 +607,9 @@ func getValidatorHashFunc(version string) (func(DistValidator, ssz.HashWalker, s return hashValidatorV1x5to7, nil } else if isAnyVersion(version, v1_8) { return hashValidatorV1x8OrLater, nil - } else { - return nil, errors.New("unknown version", z.Str("version", version)) } + + return nil, errors.New("unknown version", z.Str("version", version)) } func hashValidatorPubsharesField(v DistValidator, hh ssz.HashWalker) error { @@ -799,9 +799,9 @@ func getDepositDataHashFunc(version string) (func(DepositData, ssz.HashWalker) e return hashDepositDataV1x6, nil } else if isAnyVersion(version, v1_7, v1_8) { return hashDepositDataV1x7OrLater, nil - } else { - return nil, errors.New("unknown version", z.Str("version", version)) } + + return nil, errors.New("unknown version", z.Str("version", version)) } // getRegistrationHashFunc returns the function to hash a BuilderRegistration based on the provided version. @@ -811,9 +811,9 @@ func getRegistrationHashFunc(version string) (func(BuilderRegistration, ssz.Hash return func(BuilderRegistration, ssz.HashWalker) error { return nil }, nil } else if isAnyVersion(version, v1_7, v1_8) { return hashBuilderRegistration, nil - } else { - return nil, errors.New("unknown version", z.Str("version", version)) } + + return nil, errors.New("unknown version", z.Str("version", version)) } // hashDepositDataV1x6 hashes the deposit data for version v1.6.0. diff --git a/core/consensus/msg.go b/core/consensus/msg.go index 2ebe6bc08..56ed5b9fd 100644 --- a/core/consensus/msg.go +++ b/core/consensus/msg.go @@ -152,7 +152,10 @@ func verifyMsgSig(msg *pbv1.QBFTMsg, pubkey *k1.PublicKey) (bool, error) { return false, errors.New("empty signature") } - clone := proto.Clone(msg).(*pbv1.QBFTMsg) + clone, ok := proto.Clone(msg).(*pbv1.QBFTMsg) + if !ok { + return false, errors.New("type assert qbft msg") + } clone.Signature = nil hash, err := hashProto(clone) if err != nil { @@ -169,7 +172,10 @@ func verifyMsgSig(msg *pbv1.QBFTMsg, pubkey *k1.PublicKey) (bool, error) { // signMsg returns a copy of the proto message with a populated signature signed by the provided private key. func signMsg(msg *pbv1.QBFTMsg, privkey *k1.PrivateKey) (*pbv1.QBFTMsg, error) { - clone := proto.Clone(msg).(*pbv1.QBFTMsg) + clone, ok := proto.Clone(msg).(*pbv1.QBFTMsg) + if !ok { + return nil, errors.New("type assert qbft msg") + } clone.Signature = nil hash, err := hashProto(clone) diff --git a/core/priority/component.go b/core/priority/component.go index 356b93e48..0780ef718 100644 --- a/core/priority/component.go +++ b/core/priority/component.go @@ -151,7 +151,10 @@ func (c *Component) Prioritise(ctx context.Context, duty core.Duty, proposals .. // signMsg returns a copy of the proto message with a populated signature signed by the provided private key. func signMsg(msg *pbv1.PriorityMsg, privkey *k1.PrivateKey) (*pbv1.PriorityMsg, error) { - clone := proto.Clone(msg).(*pbv1.PriorityMsg) + clone, ok := proto.Clone(msg).(*pbv1.PriorityMsg) + if !ok { + return nil, errors.New("type assert priority msg") + } clone.Signature = nil hash, err := hashProto(clone) @@ -173,7 +176,10 @@ func verifyMsgSig(msg *pbv1.PriorityMsg, pubkey *k1.PublicKey) (bool, error) { return false, errors.New("empty signature") } - clone := proto.Clone(msg).(*pbv1.PriorityMsg) + clone, ok := proto.Clone(msg).(*pbv1.PriorityMsg) + if !ok { + return false, errors.New("type assert priority msg") + } clone.Signature = nil hash, err := hashProto(clone) if err != nil { diff --git a/core/validatorapi/router.go b/core/validatorapi/router.go index 43dbf75bb..794efb3f9 100644 --- a/core/validatorapi/router.go +++ b/core/validatorapi/router.go @@ -25,7 +25,7 @@ import ( eth2v1 "github.com/attestantio/go-eth2-client/api/v1" eth2bellatrix "github.com/attestantio/go-eth2-client/api/v1/bellatrix" eth2capella "github.com/attestantio/go-eth2-client/api/v1/capella" - deneb "github.com/attestantio/go-eth2-client/api/v1/deneb" + eth2deneb "github.com/attestantio/go-eth2-client/api/v1/deneb" eth2spec "github.com/attestantio/go-eth2-client/spec" "github.com/attestantio/go-eth2-client/spec/altair" "github.com/attestantio/go-eth2-client/spec/bellatrix" @@ -791,7 +791,7 @@ func createProposeBlockResponse(proposal *eth2api.VersionedProposal) (*proposeBl func submitProposal(p eth2client.ProposalSubmitter) handlerFunc { return func(ctx context.Context, _ map[string]string, _ url.Values, typ contentType, body []byte) (any, http.Header, error) { - denebBlock := new(deneb.SignedBlockContents) + denebBlock := new(eth2deneb.SignedBlockContents) err := unmarshal(typ, body, denebBlock) if err == nil { block := ð2api.VersionedSignedProposal{ @@ -863,7 +863,7 @@ func submitProposal(p eth2client.ProposalSubmitter) handlerFunc { func submitBlindedBlock(p eth2client.BlindedProposalSubmitter) handlerFunc { return func(ctx context.Context, _ map[string]string, _ url.Values, typ contentType, body []byte) (any, http.Header, error) { // The blinded block maybe either bellatrix, capella or deneb. - denebBlock := new(deneb.SignedBlindedBeaconBlock) + denebBlock := new(eth2deneb.SignedBlindedBeaconBlock) err := unmarshal(typ, body, denebBlock) if err == nil { block := ð2api.VersionedSignedBlindedProposal{ diff --git a/dkg/dkg.go b/dkg/dkg.go index 28e8331a0..56444f82f 100644 --- a/dkg/dkg.go +++ b/dkg/dkg.go @@ -182,12 +182,12 @@ func Run(ctx context.Context, conf Config) (err error) { return errors.Wrap(err, "private key not matching definition file") } - peerIds, err := def.PeerIDs() + peerIDs, err := def.PeerIDs() if err != nil { return errors.Wrap(err, "get peer IDs") } - ex := newExchanger(tcpNode, nodeIdx.PeerIdx, peerIds, def.NumValidators, []sigType{ + ex := newExchanger(tcpNode, nodeIdx.PeerIdx, peerIDs, def.NumValidators, []sigType{ sigLock, sigDepositData, sigValidatorRegistration, @@ -203,7 +203,7 @@ func Run(ctx context.Context, conf Config) (err error) { peerMap[p.ID] = nodeIdx } - caster := bcast.New(tcpNode, peerIds, key) + caster := bcast.New(tcpNode, peerIDs, key) // register bcast callbacks for frostp2p tp, err := newFrostP2P(tcpNode, peerMap, caster, def.Threshold, def.NumValidators) @@ -219,7 +219,7 @@ func Run(ctx context.Context, conf Config) (err error) { // Improve UX of "context cancelled" errors when sync fails. ctx = errors.WithCtxErr(ctx, "p2p connection failed, please retry DKG") - nextStepSync, stopSync, err := startSyncProtocol(ctx, tcpNode, key, def.DefinitionHash, peerIds, cancel, conf.TestConfig) + nextStepSync, stopSync, err := startSyncProtocol(ctx, tcpNode, key, def.DefinitionHash, peerIDs, cancel, conf.TestConfig) if err != nil { return err } diff --git a/p2p/sender.go b/p2p/sender.go index 1030a2c16..2dfe252eb 100644 --- a/p2p/sender.go +++ b/p2p/sender.go @@ -98,7 +98,11 @@ type Sender struct { func (s *Sender) addResult(ctx context.Context, peerID peer.ID, err error) { state := &peerState{} if val, ok := s.states.Load(peerID); ok { - state = val.(*peerState) + state, ok = val.(*peerState) + if !ok { + log.Warn(ctx, "Type assertion peer state failing", err, z.Str("peer", PeerName(peerID))) + return + } } state.buffer.add(err) diff --git a/p2p/sender_internal_test.go b/p2p/sender_internal_test.go index f367cbe4c..efc55c7c7 100644 --- a/p2p/sender_internal_test.go +++ b/p2p/sender_internal_test.go @@ -32,7 +32,8 @@ func TestSenderAddResult(t *testing.T) { t.Helper() state := &peerState{} if val, ok := sender.states.Load(peerID); ok { - state = val.(*peerState) + state, ok = val.(*peerState) + require.True(t, ok) } require.Equal(t, expect, state.failing.Load()) } diff --git a/testutil/beaconmock/server.go b/testutil/beaconmock/server.go index 5a33c7a33..674947e87 100644 --- a/testutil/beaconmock/server.go +++ b/testutil/beaconmock/server.go @@ -206,7 +206,12 @@ func newHTTPMock(optionalHandlers map[string]http.HandlerFunc, overrides ...stat return nil, nil, errors.Wrap(err, "new http client") } - return cl.(HTTPMock), srv, nil + httpMock, ok := cl.(HTTPMock) + if !ok { + return nil, nil, errors.New("type assert http mock") + } + + return httpMock, srv, nil } // overrideResponse overrides the key in the raw response. If key is empty, it overrides the whole response.