conservative linting (cosmos#820)

* gofumpt * gofumpt * automatic fixes from golangci-lint run ./... --fix * add source port to events.go * much more conservative lint * move the source port to the correct order * rever changes to democracy.go * thelper * allow machine to use all cores * Revert "allow machine to use all cores" This reverts commit e536add. * Revert "Revert "allow machine to use all cores"" This reverts commit 1bb8832. * gofumpt * use test helpers * lint after merging main * lint --------- Co-authored-by: Jehan <[email protected]>
neutron-org · Apr 5, 2023 · cd8c22c · cd8c22c
1 parent a313e2e
commit cd8c22c
Show file tree

Hide file tree

Showing 54 changed files with 249 additions and 144 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,5 +1,81 @@
+run:
+  tests: true
+  timeout: 10m
+  sort-results: true
+  allow-parallel-runners: true
+  exclude-dir: testutil/testdata
+
+linters:
+  disable-all: true
+  enable:
+    - depguard
+    - dogsled
+    - exportloopref
+    - goconst
+    - gocritic
+    - gofumpt
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - nakedret
+    - nolintlint
+    - staticcheck
+    - revive
+    - stylecheck
+    - typecheck
+    - thelper
+    - unconvert
+    - unused
+
+issues:
+  gofumpt:
+    # Module path which contains the source code being formatted.
+    # Default: ""
+    module-path: github.com/cosmos/interchain-security
+    # Choose whether to use the extra rules.
+    # Default: false
+    extra-rules: true
+  exclude-rules:
+    - text: "Use of weak random number generator"
+      linters:
+        - gosec
+    - text: "ST1003:"
+      linters:
+        - stylecheck
+    # FIXME: Disabled until golangci-lint updates stylecheck with this fix:
+    # https://github.com/dominikh/go-tools/issues/389
+    - text: "ST1016:"
+      linters:
+        - stylecheck
+    - path: "migrations"
+      text: "SA1019:"
+      linters:
+        - staticcheck
+    - text: "leading space"
+      linters:
+        - nolintlint
+
+  max-issues-per-linter: 10000
+  max-same-issues: 10000
+
 linters-settings:
-  staticcheck:
-    checks:
-      - all
-      - "-SA1019" # cosmosEd25519 allowed in tests
+  gocritic:
+    disabled-checks:
+      - appendAssign
+      - ifElseChain
+  dogsled:
+    max-blank-identifiers: 3
+  revive:
+    rules:
+      - name: var-naming
+        disabled: true
+  maligned:
+    # print struct with more effective memory layout or not, false by default
+    suggest-new: true
+  nolintlint:
+    allow-unused: false
+    allow-leading-space: true
+    require-explanation: false
+    require-specific: false
diff --git a/app/consumer/app.go b/app/consumer/app.go
@@ -157,7 +157,7 @@ type App struct { // nolint: golint
 	SlashingKeeper   slashingkeeper.Keeper
 
 	// NOTE the distribution keeper should either be removed
-	// from consumer chain or set to use an independant
+	// from consumer chain or set to use an independent
 	// different fee-pool from the consumer chain ConsumerKeeper
 
 	CrisisKeeper   crisiskeeper.Keeper

diff --git a/app/provider/app.go b/app/provider/app.go
@@ -196,7 +196,7 @@ type App struct { // nolint: golint
 	MintKeeper       mintkeeper.Keeper
 
 	// NOTE the distribution keeper should either be removed
-	// from consumer chain or set to use an independant
+	// from consumer chain or set to use an independent
 	// different fee-pool from the consumer chain ConsumerKeeper
 	DistrKeeper distrkeeper.Keeper
 

diff --git a/legacy_ibc_testing/simapp/test_helpers.go b/legacy_ibc_testing/simapp/test_helpers.go
@@ -90,6 +90,7 @@ func SignAndDeliver(
 	t *testing.T, txCfg client.TxConfig, app *bam.BaseApp, header tmproto.Header, msgs []sdk.Msg,
 	chainID string, accNums, accSeqs []uint64, expSimPass, expPass bool, priv ...cryptotypes.PrivKey,
 ) (sdk.GasInfo, *sdk.Result, error) {
+	t.Helper()
 	tx, err := helpers.GenTx(
 		txCfg,
 		msgs,

diff --git a/legacy_ibc_testing/testing/app.go b/legacy_ibc_testing/testing/app.go
@@ -61,6 +61,7 @@ type TestingApp interface {
 // of one consensus engine unit (10^6) in the default token of the simapp from first genesis
 // account. A Nop logger is set in SimApp.
 func SetupWithGenesisValSet(t *testing.T, appIniter AppIniter, valSet *tmtypes.ValidatorSet, genAccs []authtypes.GenesisAccount, chainID string, powerReduction sdk.Int, balances ...banktypes.Balance) TestingApp {
+	t.Helper()
 	app, genesisState := appIniter()
 
 	// set genesis accounts

diff --git a/legacy_ibc_testing/testing/chain.go b/legacy_ibc_testing/testing/chain.go
@@ -106,6 +106,7 @@ type TestChain struct {
 // CONTRACT: Validator array must be provided in the order expected by Tendermint.
 // i.e. sorted first by power and then lexicographically by address.
 func NewTestChainWithValSet(t *testing.T, coord *Coordinator, appIniter AppIniter, chainID string, valSet *tmtypes.ValidatorSet, signers map[string]tmtypes.PrivValidator) *TestChain {
+	t.Helper()
 	genAccs := []authtypes.GenesisAccount{}
 	genBals := []banktypes.Balance{}
 	senderAccs := []SenderAccount{}
@@ -171,6 +172,7 @@ func NewTestChainWithValSet(t *testing.T, coord *Coordinator, appIniter AppInite
 // NewTestChain initializes a new test chain with a default of 4 validators
 // Use this function if the tests do not need custom control over the validator set
 func NewTestChain(t *testing.T, coord *Coordinator, appIniter AppIniter, chainID string) *TestChain {
+	t.Helper()
 	// generate validators private/public key
 	var (
 		validatorsPerChain = 4
@@ -510,7 +512,7 @@ func (chain *TestChain) CreateTMClientHeader(chainID string, blockHeight int64,
 		AppHash:            chain.CurrentHeader.AppHash,
 		LastResultsHash:    tmhash.Sum([]byte("last_results_hash")),
 		EvidenceHash:       tmhash.Sum([]byte("evidence_hash")),
-		ProposerAddress:    tmValSet.Proposer.Address, //nolint:staticcheck
+		ProposerAddress:    tmValSet.Proposer.Address,
 	}
 
 	hhash := tmHeader.Hash()

diff --git a/legacy_ibc_testing/testing/coordinator.go b/legacy_ibc_testing/testing/coordinator.go
@@ -34,6 +34,7 @@ type Coordinator struct {
 
 // NewCoordinator initializes Coordinator with N TestChain's
 func NewCoordinator(t *testing.T, n int) *Coordinator {
+	t.Helper()
 	chains := make(map[string]*TestChain)
 	coord := &Coordinator{
 		T:           t,

diff --git a/legacy_ibc_testing/testing/utils.go b/legacy_ibc_testing/testing/utils.go
@@ -18,6 +18,7 @@ These files will be deprecated once ICS is able to upgrade to ibc-go v5.
 // ApplyValSetChanges takes in tmtypes.ValidatorSet and []abci.ValidatorUpdate and will return a new tmtypes.ValidatorSet which has the
 // provided validator updates applied to the provided validator set.
 func ApplyValSetChanges(t *testing.T, valSet *tmtypes.ValidatorSet, valUpdates []abci.ValidatorUpdate) *tmtypes.ValidatorSet {
+	t.Helper()
 	updates, err := tmtypes.PB2TM.ValidatorUpdates(valUpdates)
 	require.NoError(t, err)
 

diff --git a/tests/difference/core/driver/seed_gen_fuzzy_test.go b/tests/difference/core/driver/seed_gen_fuzzy_test.go
@@ -15,7 +15,7 @@ import (
 
 func GetPV(seed []byte) mock.PV {
 	//lint:ignore SA1019 We don't care because this is only a test.
-	return mock.PV{PrivKey: &cosmosEd25519.PrivKey{Key: cryptoEd25519.NewKeyFromSeed(seed)}}
+	return mock.PV{PrivKey: &cosmosEd25519.PrivKey{Key: cryptoEd25519.NewKeyFromSeed(seed)}} //nolint:staticcheck // SA1019: cosmosEd25519.PrivKey is deprecated: PrivKey defines a ed25519 private key. NOTE: ed25519 keys must not be used in SDK apps except in a tendermint validator context.
 }
 
 // getStakingKeyBytes takes seed bytes which can be be used to create

diff --git a/tests/difference/core/driver/setup.go b/tests/difference/core/driver/setup.go
@@ -102,7 +102,7 @@ func (b *Builder) consAddr(i int64) sdk.ConsAddress {
 // getValidatorPK returns the validator private key using the given seed index
 func (b *Builder) getValidatorPK(seedIx int) mock.PV {
 	seed := []byte(b.initState.PKSeeds[seedIx])
-	return mock.PV{PrivKey: &cosmosEd25519.PrivKey{Key: cryptoEd25519.NewKeyFromSeed(seed)}}
+	return mock.PV{PrivKey: &cosmosEd25519.PrivKey{Key: cryptoEd25519.NewKeyFromSeed(seed)}} //nolint:staticcheck // SA1019: cosmosEd25519.PrivKey is deprecated: PrivKey defines a ed25519 private key. NOTE: ed25519 keys must not be used in SDK apps except in a tendermint validator context.
 }
 
 func (b *Builder) getAppBytesAndSenders(

diff --git a/tests/difference/core/driver/setup_test.go b/tests/difference/core/driver/setup_test.go
@@ -123,8 +123,8 @@ func (s *CoreSuite) TestAssumptionsSetup() {
 	// The offset time is the last committed time, but the SUT is +1 block ahead
 	// because the currentHeader time is ahead of the last committed. Therefore sub
 	// the difference (duration of 1 block).
-	s.Require().Equal(int64(s.offsetTimeUnix), s.time(P).Add(-s.initState.BlockInterval).Unix())
-	s.Require().Equal(int64(s.offsetTimeUnix), s.time(C).Add(-s.initState.BlockInterval).Unix())
+	s.Require().Equal(s.offsetTimeUnix, s.time(P).Add(-s.initState.BlockInterval).Unix())
+	s.Require().Equal(s.offsetTimeUnix, s.time(C).Add(-s.initState.BlockInterval).Unix())
 
 	// The offset height is the last committed height, but the SUT is +1 because
 	// the currentHeader is +1 ahead of the last committed. Therefore sub 1.

diff --git a/tests/difference/core/driver/trace.go b/tests/difference/core/driver/trace.go
@@ -80,7 +80,7 @@ func LoadTraces(fn string) []TraceData {
 
 	var ret []TraceData
 
-	err = json.Unmarshal([]byte(byteValue), &ret)
+	err = json.Unmarshal(byteValue, &ret)
 
 	if err != nil {
 		panic(err)

diff --git a/tests/e2e/actions.go b/tests/e2e/actions.go
@@ -27,6 +27,8 @@ type SendTokensAction struct {
 	amount uint
 }
 
+const done = "done!!!!!!!!"
+
 func (tr TestRun) sendTokens(
 	action SendTokensAction,
 	verbose bool,
@@ -149,7 +151,7 @@ func (tr TestRun) startChain(
 		if verbose {
 			fmt.Println("startChain: " + out)
 		}
-		if out == "done!!!!!!!!" {
+		if out == done {
 			break
 		}
 	}
@@ -671,7 +673,7 @@ func (tr TestRun) addIbcConnection(
 		if verbose {
 			fmt.Println("addIbcConnection: " + out)
 		}
-		if out == "done!!!!!!!!" {
+		if out == done {
 			break
 		}
 	}
@@ -746,7 +748,7 @@ func (tr TestRun) addIbcChannel(
 		if verbose {
 			fmt.Println("addIBCChannel: " + out)
 		}
-		if out == "done!!!!!!!!" {
+		if out == done {
 			break
 		}
 	}
@@ -1098,7 +1100,7 @@ func (tr TestRun) unjailValidator(action unjailValidatorAction, verbose bool) {
 	}
 
 	// wait for 1 blocks to make sure that tx got included
-	// in a block and packets commited before proceeding
+	// in a block and packets committed before proceeding
 	tr.waitBlocks(action.provider, 1, time.Minute)
 }
 
@@ -1274,7 +1276,7 @@ func (tr TestRun) assignConsumerPubKey(action assignConsumerPubKeyAction, verbos
 			if verbose {
 				fmt.Println("assign key - reconfigure: " + out)
 			}
-			if out == "done!!!!!!!!" {
+			if out == done {
 				break
 			}
 		}
@@ -1315,7 +1317,8 @@ func (tr TestRun) waitForSlashThrottleDequeue(
 			fmt.Printf("waiting for packed queue size to reach: %d - current: %d\n", action.nextQueueSize, globalQueueSize)
 		}
 
-		if globalQueueSize == chainQueueSize && globalQueueSize == action.nextQueueSize {
+		// check if global queue size is equal to chain queue size
+		if globalQueueSize == chainQueueSize && globalQueueSize == action.nextQueueSize { //nolint:gocritic // this is the comparison that we want here.
 			break
 		}
 

diff --git a/tests/e2e/main.go b/tests/e2e/main.go
@@ -23,6 +23,7 @@ var (
 	localSdkPath         = flag.String("local-sdk-path", "",
 		"path of a local sdk version to build and reference in integration tests")
 )
+
 var (
 	useGaia = flag.Bool("use-gaia", false, "use gaia instead of ICS provider app")
 	gaiaTag = flag.String("gaia-tag", "", "gaia tag to use - default is latest")

diff --git a/tests/e2e/steps_downtime.go b/tests/e2e/steps_downtime.go
@@ -6,7 +6,7 @@ import "time"
 //
 // Note: These steps are not affected by slash packet throttling since
 // only one consumer initiated slash is implemented. Throttling is also
-// psuedo-disabled in this test by setting the slash meter replenish
+// pseudo-disabled in this test by setting the slash meter replenish
 // fraction to 1.0 in the config file.
 //
 // No slashing should occur for downtime slash initiated from the consumer chain

diff --git a/tests/e2e/steps_multi_consumer_double_sign.go b/tests/e2e/steps_multi_consumer_double_sign.go
@@ -5,7 +5,7 @@ package main
 // Note: These steps would be affected by slash packet throttling, since the
 // consumer-initiated slash steps are executed after consumer-initiated downtime
 // slashes have already occurred. However slash packet throttling is
-// psuedo-disabled in this test by setting the slash meter replenish
+// pseudo-disabled in this test by setting the slash meter replenish
 // fraction to 1.0 in the config file.
 //
 // only double sign on provider chain will cause slashing and tombstoning

diff --git a/tests/e2e/steps_multi_consumer_downtime.go b/tests/e2e/steps_multi_consumer_downtime.go
@@ -328,7 +328,7 @@ func stepsMultiConsumerDowntimeFromProvider(consumer1, consumer2 string) []Step
 					ValPowers: &map[validatorID]uint{
 						validatorID("alice"): 509,
 						validatorID("bob"):   500,
-						validatorID("carol"): 495, // slashed because infraction was commited on provider
+						validatorID("carol"): 495, // slashed because infraction was committed on provider
 					},
 				},
 				chainID(consumer1): ChainState{

diff --git a/tests/e2e/steps_start_chains.go b/tests/e2e/steps_start_chains.go
@@ -92,7 +92,7 @@ func stepsStartConsumerChain(consumerName string, proposalIndex, chainIndex uint
 			state: State{},
 		},
 		{
-			// op should fail - key allready assigned by another validator
+			// op should fail - key already assigned by another validator
 			action: assignConsumerPubKeyAction{
 				chain:     chainID(consumerName),
 				validator: validatorID("bob"),

diff --git a/tests/integration/common.go b/tests/integration/common.go
@@ -157,7 +157,7 @@ func delegateByIdx(s *CCVTestSuite, delAddr sdk.AccAddress, bondAmt sdk.Int, idx
 		delAddr,
 		bondAmt,
 		stakingtypes.Unbonded,
-		stakingtypes.Validator(validator),
+		validator,
 		true,
 	)
 	s.Require().NoError(err)
@@ -180,7 +180,7 @@ func undelegate(s *CCVTestSuite, delAddr sdk.AccAddress, valAddr sdk.ValAddress,
 // Executes a BeginRedelegation (unbonding and redelegation) operation
 // on the provider chain using delegated funds from delAddr
 func redelegate(s *CCVTestSuite, delAddr sdk.AccAddress, valSrcAddr sdk.ValAddress,
-	ValDstAddr sdk.ValAddress, sharesAmount sdk.Dec,
+	valDstAddr sdk.ValAddress, sharesAmount sdk.Dec,
 ) {
 	stakingKeeper := s.providerApp.GetTestStakingKeeper()
 	ctx := s.providerCtx()
@@ -190,7 +190,7 @@ func redelegate(s *CCVTestSuite, delAddr sdk.AccAddress, valSrcAddr sdk.ValAddre
 		ctx,
 		delAddr,
 		valSrcAddr,
-		ValDstAddr,
+		valDstAddr,
 		sharesAmount,
 	)
 	s.Require().NoError(err)

diff --git a/tests/integration/democracy.go b/tests/integration/democracy.go
@@ -39,12 +39,13 @@ func NewConsumerDemocracyTestSuite[T testutil.DemocConsumerApp](
 		*ibctesting.TestChain,
 		testutil.DemocConsumerApp,
 	) {
+		t.Helper()
 		// Instantiate the test coordinator
 		coordinator := ibctesting.NewCoordinator(t, 0)
 
 		// Add single democracy consumer to coordinator, store returned test chain and app.
 		democConsumer, democConsumerApp := icstestingutils.AddDemocracyConsumer[T](
-			coordinator, t, democConsumerAppIniter)
+			t, coordinator, democConsumerAppIniter)
 
 		// Pass variables to suite.
 		return coordinator, democConsumer, democConsumerApp

diff --git a/tests/integration/distribution.go b/tests/integration/distribution.go
@@ -47,7 +47,7 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
 	providerExpectedRewards := feePoolTokens.Sub(consumerExpectedRewards)
 	s.consumerChain.NextBlock()
 
-	// amount from the fee pool is devided between consumer redistribute address and address reserved for provider chain
+	// amount from the fee pool is divided between consumer redistribute address and address reserved for provider chain
 	feePoolTokens = consumerBankKeeper.GetAllBalances(s.consumerCtx(), consumerFeePoolAddr)
 	s.Require().Equal(0, len(feePoolTokens))
 	consumerRedistributeAddr := consumerAccountKeeper.GetModuleAccount(s.consumerCtx(), consumertypes.ConsumerRedistributeName).GetAddress()
@@ -242,7 +242,7 @@ func (s *CCVTestSuite) TestEndBlockRD() {
 	}
 }
 
-// getEscrowBalance gets the current balances in the escrow account holding the transfered tokens to the provider
+// getEscrowBalance gets the current balances in the escrow account holding the transferred tokens to the provider
 func (s CCVTestSuite) getEscrowBalance() sdk.Coins {
 	consumerBankKeeper := s.consumerApp.GetTestBankKeeper()
 	transChanID := s.consumerApp.GetConsumerKeeper().GetDistributionTransmissionChannel(s.consumerCtx())

diff --git a/tests/integration/normal_operations.go b/tests/integration/normal_operations.go
@@ -71,7 +71,7 @@ func (k CCVTestSuite) TestHistoricalInfo() {
 			expLen: 0,
 		},
 		{
-			height: initHeight + int64(consumertypes.DefaultHistoricalEntries) + 2,
+			height: initHeight + consumertypes.DefaultHistoricalEntries + 2,
 			found:  true,
 			expLen: initValsetLen + 2,
 		},

diff --git a/tests/integration/setup.go b/tests/integration/setup.go
@@ -73,12 +73,13 @@ func NewCCVTestSuite[Tp testutil.ProviderApp, Tc testutil.ConsumerApp](
 		*ibctesting.TestChain,
 		testutil.ProviderApp,
 	) {
+		t.Helper()
 		// Instantiate the test coordinator.
 		coordinator := ibctesting.NewCoordinator(t, 0)
 
 		// Add provider to coordinator, store returned test chain and app.
 		// Concrete provider app type is passed to the generic function here.
-		provider, providerApp := icstestingutils.AddProvider[Tp](coordinator, t, providerAppIniter)
+		provider, providerApp := icstestingutils.AddProvider[Tp](t, coordinator, providerAppIniter)
 
 		// Pass variables to suite.
 		return coordinator, provider, providerApp

diff --git a/tests/integration/slashing.go b/tests/integration/slashing.go
@@ -562,7 +562,7 @@ func (suite *CCVTestSuite) TestValidatorDoubleSigning() {
 	// construct slash packet data and get the expcted commit hash
 	packetData := ccv.NewSlashPacketData(
 		abci.Validator{Address: consAddr.Bytes(), Power: power},
-		// get VSC ID mapping to the infraction height with the TM delay substracted
+		// get VSC ID mapping to the infraction height with the TM delay subtracted
 		suite.consumerApp.GetConsumerKeeper().GetHeightValsetUpdateID(ctx, uint64(infractionHeight-sdk.ValidatorUpdateDelay)),
 		stakingtypes.DoubleSign,
 	)