Merge pull request #6888 from onflow/jord/test-lint-upgrade

Upgrade golangci linter to 1.63
onflow · Jan 15, 2025 · 0d2b34f · 0d2b34f
2 parents b740fc0 + c396112
commit 0d2b34f
Show file tree

Hide file tree

Showing 16 changed files with 28 additions and 50 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -49,7 +49,7 @@ jobs:
       uses: golangci/golangci-lint-action@v3
       with:
         # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-        version: v1.54
+        version: v1.63
         args: -v
         working-directory: ${{ matrix.dir }}
         # https://github.com/golangci/golangci-lint-action/issues/244

diff --git a/cmd/util/cmd/common/node_info.go b/cmd/util/cmd/common/node_info.go
@@ -41,16 +41,16 @@ func ReadFullPartnerNodeInfos(log zerolog.Logger, partnerWeightsPath, partnerNod
 		}
 		err = ValidateNetworkPubKey(partner.NetworkPubKey)
 		if err != nil {
-			return nil, fmt.Errorf(fmt.Sprintf("invalid network public key: %s", partner.NetworkPubKey))
+			return nil, fmt.Errorf("invalid network public key: %s", partner.NetworkPubKey)
 		}
 		err = ValidateStakingPubKey(partner.StakingPubKey)
 		if err != nil {
-			return nil, fmt.Errorf(fmt.Sprintf("invalid staking public key: %s", partner.StakingPubKey))
+			return nil, fmt.Errorf("invalid staking public key: %s", partner.StakingPubKey)
 		}
 
 		weight := weights[partner.NodeID]
 		if valid := ValidateWeight(weight); !valid {
-			return nil, fmt.Errorf(fmt.Sprintf("invalid partner weight %v: %d", partner.NodeID, weight))
+			return nil, fmt.Errorf("invalid partner weight %v: %d", partner.NodeID, weight)
 		}
 
 		if weight != flow.DefaultInitialWeight {
@@ -143,12 +143,12 @@ func ReadFullInternalNodeInfos(log zerolog.Logger, internalNodePrivInfoDir, inte
 		// validate every single internal node
 		err := ValidateNodeID(internal.NodeID)
 		if err != nil {
-			return nil, fmt.Errorf(fmt.Sprintf("invalid internal node ID: %s", internal.NodeID))
+			return nil, fmt.Errorf("invalid internal node ID: %s", internal.NodeID)
 		}
 		weight := weights[internal.Address]
 
 		if valid := ValidateWeight(weight); !valid {
-			return nil, fmt.Errorf(fmt.Sprintf("invalid partner weight %v: %d", internal.NodeID, weight))
+			return nil, fmt.Errorf("invalid partner weight %v: %d", internal.NodeID, weight)
 		}
 		if weight != flow.DefaultInitialWeight {
 			log.Warn().Msgf("internal node (id=%x) has non-default weight (%d != %d)", internal.NodeID, weight, flow.DefaultInitialWeight)

diff --git a/engine/access/rpc/backend/backend_transactions.go b/engine/access/rpc/backend/backend_transactions.go
@@ -417,7 +417,7 @@ func (b *backendTransactions) getTransactionResultsByBlockIDFromExecutionNode(
 	)
 	if err != nil {
 		if IsInsufficientExecutionReceipts(err) {
-			return nil, status.Errorf(codes.NotFound, err.Error())
+			return nil, status.Error(codes.NotFound, err.Error())
 		}
 		return nil, rpc.ConvertError(err, "failed to retrieve result from any execution node", codes.Internal)
 	}
@@ -574,7 +574,7 @@ func (b *backendTransactions) getTransactionResultByIndexFromExecutionNode(
 	)
 	if err != nil {
 		if IsInsufficientExecutionReceipts(err) {
-			return nil, status.Errorf(codes.NotFound, err.Error())
+			return nil, status.Error(codes.NotFound, err.Error())
 		}
 		return nil, rpc.ConvertError(err, "failed to retrieve result from any execution node", codes.Internal)
 	}
@@ -762,7 +762,7 @@ func (b *backendTransactions) getTransactionResultFromExecutionNode(
 	if err != nil {
 		// if no execution receipt were found, return a NotFound GRPC error
 		if IsInsufficientExecutionReceipts(err) {
-			return nil, status.Errorf(codes.NotFound, err.Error())
+			return nil, status.Error(codes.NotFound, err.Error())
 		}
 		return nil, err
 	}
@@ -1002,7 +1002,7 @@ func (b *backendTransactions) LookupErrorMessageByTransactionID(
 	)
 	if err != nil {
 		if IsInsufficientExecutionReceipts(err) {
-			return "", status.Errorf(codes.NotFound, err.Error())
+			return "", status.Error(codes.NotFound, err.Error())
 		}
 		return "", rpc.ConvertError(err, "failed to select execution nodes", codes.Internal)
 	}
@@ -1057,7 +1057,7 @@ func (b *backendTransactions) LookupErrorMessageByIndex(
 	)
 	if err != nil {
 		if IsInsufficientExecutionReceipts(err) {
-			return "", status.Errorf(codes.NotFound, err.Error())
+			return "", status.Error(codes.NotFound, err.Error())
 		}
 		return "", rpc.ConvertError(err, "failed to select execution nodes", codes.Internal)
 	}
@@ -1117,7 +1117,7 @@ func (b *backendTransactions) LookupErrorMessagesByBlockID(
 	)
 	if err != nil {
 		if IsInsufficientExecutionReceipts(err) {
-			return nil, status.Errorf(codes.NotFound, err.Error())
+			return nil, status.Error(codes.NotFound, err.Error())
 		}
 		return nil, rpc.ConvertError(err, "failed to select execution nodes", codes.Internal)
 	}

diff --git a/engine/collection/compliance/core_test.go b/engine/collection/compliance/core_test.go
@@ -376,7 +376,7 @@ func (cs *CoreSuite) TestOnBlockProposal_FailsProtocolStateValidation() {
 		// make sure we fail to extend the state
 		*cs.state = clusterstate.MutableState{}
 		cs.state.On("Final").Return(func() clusterint.Snapshot { return cs.snapshot })
-		sentinelErr := state.NewInvalidExtensionError("")
+		sentinelErr := state.NewInvalidExtensionErrorf("")
 		cs.state.On("Extend", mock.Anything).Return(sentinelErr)
 		cs.proposalViolationNotifier.On("OnInvalidBlockDetected", mock.Anything).Run(func(args mock.Arguments) {
 			err := args.Get(0).(flow.Slashable[model.InvalidProposalError])
@@ -406,7 +406,7 @@ func (cs *CoreSuite) TestOnBlockProposal_FailsProtocolStateValidation() {
 		// make sure we fail to extend the state
 		*cs.state = clusterstate.MutableState{}
 		cs.state.On("Final").Return(func() clusterint.Snapshot { return cs.snapshot })
-		cs.state.On("Extend", mock.Anything).Return(state.NewOutdatedExtensionError(""))
+		cs.state.On("Extend", mock.Anything).Return(state.NewOutdatedExtensionErrorf(""))
 
 		// the expected error should be handled within the Core
 		err := cs.core.OnBlockProposal(flow.Slashable[*messages.ClusterBlockProposal]{

diff --git a/engine/common/grpc/forwarder/forwarder.go b/engine/common/grpc/forwarder/forwarder.go
@@ -75,7 +75,7 @@ func (f *Forwarder) reconnectingClient(i int) error {
 // FaultTolerantClient implements an upstream connection that reconnects on errors
 // a reasonable amount of time.
 func (f *Forwarder) FaultTolerantClient() (access.AccessAPIClient, io.Closer, error) {
-	if f.upstream == nil || len(f.upstream) == 0 {
+	if len(f.upstream) == 0 {
 		return nil, nil, status.Errorf(codes.Unimplemented, "method not implemented")
 	}
 
@@ -101,5 +101,5 @@ func (f *Forwarder) FaultTolerantClient() (access.AccessAPIClient, io.Closer, er
 		return f.upstream[f.roundRobin].client, f.upstream[f.roundRobin].closer, nil
 	}
 
-	return nil, nil, status.Errorf(codes.Unavailable, err.Error())
+	return nil, nil, status.Error(codes.Unavailable, err.Error())
 }
diff --git a/engine/consensus/compliance/core_test.go b/engine/consensus/compliance/core_test.go
@@ -459,7 +459,7 @@ func (cs *CoreSuite) TestOnBlockProposal_FailsProtocolStateValidation() {
 		// make sure we fail to extend the state
 		*cs.state = protocol.ParticipantState{}
 		cs.state.On("Final").Return(func() protint.Snapshot { return cs.snapshot })
-		sentinelErr := state.NewInvalidExtensionError("")
+		sentinelErr := state.NewInvalidExtensionErrorf("")
 		cs.state.On("Extend", mock.Anything, mock.Anything).Return(sentinelErr)
 		cs.proposalViolationNotifier.On("OnInvalidBlockDetected", mock.Anything).Run(func(args mock.Arguments) {
 			err := args.Get(0).(flow.Slashable[model.InvalidProposalError])
@@ -489,7 +489,7 @@ func (cs *CoreSuite) TestOnBlockProposal_FailsProtocolStateValidation() {
 		// make sure we fail to extend the state
 		*cs.state = protocol.ParticipantState{}
 		cs.state.On("Final").Return(func() protint.Snapshot { return cs.snapshot })
-		cs.state.On("Extend", mock.Anything, mock.Anything).Return(state.NewOutdatedExtensionError(""))
+		cs.state.On("Extend", mock.Anything, mock.Anything).Return(state.NewOutdatedExtensionErrorf(""))
 
 		// the expected error should be handled within the Core
 		err := cs.core.OnBlockProposal(flow.Slashable[*messages.BlockProposal]{

diff --git a/engine/consensus/matching/core_test.go b/engine/consensus/matching/core_test.go
@@ -166,7 +166,7 @@ func (ms *MatchingSuite) TestOnReceiptInvalid() {
 	)
 
 	// check that _expected_ failure case of invalid receipt is handled without error
-	ms.receiptValidator.On("Validate", receipt).Return(engine.NewInvalidInputError("")).Once()
+	ms.receiptValidator.On("Validate", receipt).Return(engine.NewInvalidInputErrorf("")).Once()
 	wasAdded, err := ms.core.processReceipt(receipt)
 	ms.Require().NoError(err, "invalid receipt should be dropped but not error")
 	ms.Require().False(wasAdded, "invalid receipt should not be added")

diff --git a/engine/errors.go b/engine/errors.go
@@ -22,10 +22,6 @@ type InvalidInputError struct {
 	err error
 }
 
-func NewInvalidInputError(msg string) error {
-	return NewInvalidInputErrorf(msg)
-}
-
 func NewInvalidInputErrorf(msg string, args ...interface{}) error {
 	return InvalidInputError{
 		err: fmt.Errorf(msg, args...),
@@ -64,12 +60,6 @@ func NewNetworkTransmissionErrorf(msg string, args ...interface{}) error {
 	}
 }
 
-func NewNetworkTransmissionError(msg string) error {
-	return NetworkTransmissionError{
-		err: fmt.Errorf(msg),
-	}
-}
-
 func (e NetworkTransmissionError) Unwrap() error {
 	return e.err
 }

diff --git a/integration/tests/access/cohort4/execution_data_pruning_test.go b/integration/tests/access/cohort4/execution_data_pruning_test.go
@@ -85,7 +85,7 @@ func (s *ExecutionDataPruningSuite) SetupTest() {
 		testnet.WithAdditionalFlagf("--event-query-mode=local-only"),
 		testnet.WithAdditionalFlagf("--execution-data-height-range-target=%d", s.heightRangeTarget),
 		testnet.WithAdditionalFlagf("--execution-data-height-range-threshold=%d", s.threshold),
-		testnet.WithAdditionalFlagf(fmt.Sprintf("--execution-data-pruning-interval=%s", s.pruningInterval)),
+		testnet.WithAdditionalFlagf("--execution-data-pruning-interval=%s", s.pruningInterval),
 	)
 
 	consensusConfigs := []func(config *testnet.NodeConfig){

diff --git a/module/mempool/errors.go b/module/mempool/errors.go
@@ -10,10 +10,6 @@ type UnknownExecutionResultError struct {
 	err error
 }
 
-func NewUnknownExecutionResultError(msg string) error {
-	return NewUnknownExecutionResultErrorf(msg)
-}
-
 func NewUnknownExecutionResultErrorf(msg string, args ...interface{}) error {
 	return UnknownExecutionResultError{
 		err: fmt.Errorf(msg, args...),

diff --git a/module/validation/seal_validator.go b/module/validation/seal_validator.go
@@ -131,7 +131,7 @@ func (s *sealValidator) Validate(candidate *flow.Block) (*flow.Seal, error) {
 		byBlock[seal.BlockID] = seal
 	}
 	if len(payload.Seals) != len(byBlock) {
-		return nil, engine.NewInvalidInputError("multiple seals for the same block")
+		return nil, engine.NewInvalidInputErrorf("multiple seals for the same block")
 	}
 
 	// incorporatedResults collects execution results that are incorporated in unsealed

diff --git a/state/errors.go b/state/errors.go
@@ -19,10 +19,6 @@ type InvalidExtensionError struct {
 	error
 }
 
-func NewInvalidExtensionError(msg string) error {
-	return NewInvalidExtensionErrorf(msg)
-}
-
 func NewInvalidExtensionErrorf(msg string, args ...interface{}) error {
 	return InvalidExtensionError{
 		error: fmt.Errorf(msg, args...),
@@ -46,10 +42,6 @@ type OutdatedExtensionError struct {
 	error
 }
 
-func NewOutdatedExtensionError(msg string) error {
-	return NewOutdatedExtensionErrorf(msg)
-}
-
 func NewOutdatedExtensionErrorf(msg string, args ...interface{}) error {
 	return OutdatedExtensionError{
 		error: fmt.Errorf(msg, args...),

diff --git a/state/protocol/badger/mutator.go b/state/protocol/badger/mutator.go
@@ -308,7 +308,7 @@ func (m *FollowerState) headerExtend(ctx context.Context, candidate *flow.Block,
 
 	// STEP 1: Check that the payload is consistent with the payload hash in the header
 	if candidate.Payload.Hash() != header.PayloadHash {
-		return state.NewInvalidExtensionError("payload integrity check failed")
+		return state.NewInvalidExtensionErrorf("payload integrity check failed")
 	}
 
 	// STEP 2: check whether the candidate (i) connects to the known block tree and

diff --git a/state/protocol/badger/mutator_test.go b/state/protocol/badger/mutator_test.go
@@ -719,7 +719,7 @@ func TestExtendReceiptsInvalid(t *testing.T) {
 		require.NoError(t, err)
 
 		// but receipt for block 2 is invalid, which the ParticipantState should reject with an InvalidExtensionError
-		validator.On("ValidatePayload", block3).Return(engine.NewInvalidInputError("")).Once()
+		validator.On("ValidatePayload", block3).Return(engine.NewInvalidInputErrorf("")).Once()
 		err = state.Extend(context.Background(), block3)
 		require.Error(t, err)
 		require.True(t, st.IsInvalidExtensionError(err), err)
@@ -2611,7 +2611,7 @@ func TestExtendInvalidSealsInBlock(t *testing.T) {
 				return seal
 			}, func(candidate *flow.Block) error {
 				if candidate.ID() == block3.ID() {
-					return engine.NewInvalidInputError("")
+					return engine.NewInvalidInputErrorf("")
 				}
 				_, err := all.Seals.HighestInFork(candidate.Header.ParentID)
 				return err

diff --git a/utils/grpcutils/grpc.go b/utils/grpcutils/grpc.go
@@ -141,7 +141,7 @@ func verifyPeerCertificateFunc(expectedPublicKey crypto.PublicKey) (func(rawCert
 		for i := 0; i < len(rawCerts); i++ {
 			cert, err := x509.ParseCertificate(rawCerts[i])
 			if err != nil {
-				return newServerAuthError(err.Error())
+				return newServerAuthError("failed to parse certificate: %s", err.Error())
 			}
 			chain[i] = cert
 		}
@@ -150,7 +150,7 @@ func verifyPeerCertificateFunc(expectedPublicKey crypto.PublicKey) (func(rawCert
 		// extension, extract the remote's public key and finally verifies the signature included in the certificate
 		actualLibP2PKey, err := libp2ptls.PubKeyFromCertChain(chain)
 		if err != nil {
-			return newServerAuthError(err.Error())
+			return newServerAuthError("could not convert certificate to libp2p public key: %s", err.Error())
 		}
 
 		// verify that the public key received is the one that is expected
@@ -170,7 +170,7 @@ func verifyPeerCertificateFunc(expectedPublicKey crypto.PublicKey) (func(rawCert
 func libP2PKeyToHexString(key lcrypto.PubKey) (string, *ServerAuthError) {
 	keyRaw, err := key.Raw()
 	if err != nil {
-		return "", newServerAuthError(err.Error())
+		return "", newServerAuthError("could not convert public key to hex string: %s", err.Error())
 	}
 	return hex.EncodeToString(keyRaw), nil
 }
diff --git a/utils/unittest/logging.go b/utils/unittest/logging.go
@@ -41,7 +41,7 @@ func LoggerWithWriterAndLevel(writer io.Writer, level zerolog.Level) zerolog.Log
 	return log
 }
 
-// go:noinline
+//go:noinline
 func LoggerForTest(t *testing.T, level zerolog.Level) zerolog.Logger {
 	_, file, _, ok := runtime.Caller(1)
 	if !ok {