lint: fix many broken lint issues (#49)

* lint: fix many broken lint issues

* more lint issues resolved

* more lint issues resolved

* all actions issues fixed

* pin variables for websocket

* Update .github/workflows/lint.yml

* more fixes

* fix lint issues in pubsub, rpc, types

* fix lint issues in statedb, journal, pubsub, cli

* fix comment

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

.github/workflows/lint.yml

@@ -1,15 +1,15 @@
-name: Lint
+name: Run Lint
-# Lint runs golangci-lint over the entire cosmos-sdk repository
+# Lint runs golangci-lint over the entire ethermint repository This workflow is
-# This workflow is run on every pull request and push to development
+# run on every pull request and push to main The `golangci` will pass without
-# The `golangci` will pass without running if no *.{go, mod, sum} files have been changed.
+# running if no *.{go, mod, sum} files have been changed.
 on:
   pull_request:
   push:
     branches:
-      - development
+      - main
 jobs:
   golangci:
-    name: golangci-lint
+    name: Run golangci-lint
     runs-on: ubuntu-latest
     timeout-minutes: 10
     steps:
@@ -20,10 +20,11 @@ jobs:
             .go
             .mod
             .sum
-      - uses: golangci/golangci-lint-action@master
+      - uses: golangci/golangci-lint-action@v2.5.2
         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.28
+          version: v1.29
           args: --timeout 10m
           github-token: ${{ secrets.github_token }}
-        if: "env.GIT_DIFF != ''"
+        # Check only if there are differences in the source code
+        if: "env.GIT_DIFF"

.golangci.yml

@@ -52,11 +52,6 @@ issues:
     - 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
   max-issues-per-linter: 10000
   max-same-issues: 10000
 

Makefile

@@ -152,6 +152,7 @@ clean:
 .PHONY: install clean
 
 docker-build:
+	# TODO replace with kaniko
 	docker build -t ${DOCKER_IMAGE}:${DOCKER_TAG} .
 	docker tag ${DOCKER_IMAGE}:${DOCKER_TAG} ${DOCKER_IMAGE}:latest
 	# docker tag ${DOCKER_IMAGE}:${DOCKER_TAG} ${DOCKER_IMAGE}:${COMMIT_HASH}

client/testnet.go

@@ -7,7 +7,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"log"
 	"net"
 	"os"
 	"path/filepath"
@@ -478,42 +477,3 @@ func writeFile(name string, dir string, contents []byte) error {
 
 	return nil
 }
-
-func appendToFile(name string, dir string, contents []byte) error {
-	writePath := filepath.Join(dir)
-	file := filepath.Join(writePath, name)
-
-	err := tmos.EnsureDir(writePath, 0755)
-	if err != nil {
-		return err
-	}
-
-	if _, err = os.Stat(file); err == nil {
-		err = os.Chmod(file, 0777)
-		if err != nil {
-			fmt.Println(err)
-			return err
-		}
-	}
-
-	f, err := os.OpenFile(file, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
-	if err != nil {
-		log.Println(err)
-		return err
-	}
-	defer f.Close()
-
-	_, err = f.Write(contents)
-	if err != nil {
-		log.Println(err)
-		return err
-	}
-
-	f.Write([]byte("\n"))
-	if err != nil {
-		log.Println(err)
-		return err
-	}
-
-	return nil
-}

cmd/ethermintd/flags.go

@@ -72,9 +72,13 @@ func addTxFlags(cmd *cobra.Command) *cobra.Command {
 	//))
 
 	//viper.BindPFlag(flags.FlagTrustNode, cmd.Flags().Lookup(flags.FlagTrustNode))
-	viper.BindPFlag(flags.FlagNode, cmd.Flags().Lookup(flags.FlagNode))
-	viper.BindPFlag(flags.FlagKeyringBackend, cmd.Flags().Lookup(flags.FlagKeyringBackend))
 
+	// TODO: we need to handle the errors for these, decide if we should return error upward and handle
+	// nolint: errcheck
+	viper.BindPFlag(flags.FlagNode, cmd.Flags().Lookup(flags.FlagNode))
+	// nolint: errcheck
+	viper.BindPFlag(flags.FlagKeyringBackend, cmd.Flags().Lookup(flags.FlagKeyringBackend))
+	// nolint: errcheck
 	cmd.MarkFlagRequired(flags.FlagChainID)
 	return cmd
 }

cmd/ethermintd/genaccounts.go

@@ -29,7 +29,6 @@ const (
 	flagVestingStart = "vesting-start-time"
 	flagVestingEnd   = "vesting-end-time"
 	flagVestingAmt   = "vesting-amount"
-	flagKeyring      = "pass"
 )
 
 // AddGenesisAccountCmd returns add-genesis-account cobra Command.

cmd/ethermintd/root.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"context"
 	"io"
 	"math/big"
 	"os"
@@ -11,7 +10,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/simapp/params"
 	"github.com/cosmos/cosmos-sdk/snapshots"
 	"github.com/cosmos/cosmos-sdk/version"
-	"github.com/cosmos/cosmos-sdk/x/crisis"
 
 	"github.com/spf13/cast"
 	"github.com/spf13/cobra"
@@ -67,28 +65,6 @@ func NewRootCmd() (*cobra.Command, params.EncodingConfig) {
 		},
 	}
 
-	initRootCmd(rootCmd, encodingConfig)
-
-	return rootCmd, encodingConfig
-}
-
-// Execute executes the root command.
-func Execute(rootCmd *cobra.Command) error {
-	// Create and set a client.Context on the command's Context. During the pre-run
-	// of the root command, a default initialized client.Context is provided to
-	// seed child command execution with values such as AccountRetriver, Keyring,
-	// and a Tendermint RPC. This requires the use of a pointer reference when
-	// getting and setting the client.Context. Ideally, we utilize
-	// https://github.com/spf13/cobra/pull/1118.
-	ctx := context.Background()
-	ctx = context.WithValue(ctx, client.ClientContextKey, &client.Context{})
-	ctx = context.WithValue(ctx, sdkserver.ServerContextKey, sdkserver.NewDefaultContext())
-
-	executor := tmcli.PrepareBaseCmd(rootCmd, "", app.DefaultNodeHome)
-	return executor.ExecuteContext(ctx)
-}
-
-func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
 	authclient.Codec = encodingConfig.Marshaler
 	sdk.PowerReduction = sdk.NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(18), nil))
 
@@ -137,10 +113,7 @@ func initRootCmd(rootCmd *cobra.Command, encodingConfig params.EncodingConfig) {
 	)
 	rootCmd = addTxFlags(rootCmd)
 
-}
+	return rootCmd, encodingConfig
-
-func addModuleInitFlags(startCmd *cobra.Command) {
-	crisis.AddModuleInitFlags(startCmd)
 }
 
 func queryCommand() *cobra.Command {

cmd/ethermintd/start.go

@@ -45,7 +45,6 @@ import (
 
 // Tendermint full-node start flags
 const (
-	flagWithTendermint     = "with-tendermint"
 	flagAddress            = "address"
 	flagTransport          = "transport"
 	flagTraceStore         = "trace-store"
@@ -62,7 +61,6 @@ const (
 	FlagPruningKeepRecent = "pruning-keep-recent"
 	FlagPruningKeepEvery  = "pruning-keep-every"
 	FlagPruningInterval   = "pruning-interval"
-	FlagIndexEvents       = "index-events"
 	FlagMinRetainBlocks   = "min-retain-blocks"
 )
 
@@ -114,9 +112,12 @@ which accepts a path for the resulting pprof file.
 
 			// Bind flags to the Context's Viper so the app construction can set
 			// options accordingly.
-			serverCtx.Viper.BindPFlags(cmd.Flags())
+			err := serverCtx.Viper.BindPFlags(cmd.Flags())
+			if err != nil {
+				return err
+			}
 
-			_, err := server.GetPruningOptionsFromFlags(serverCtx.Viper)
+			_, err = server.GetPruningOptionsFromFlags(serverCtx.Viper)
 			return err
 		},
 		RunE: func(cmd *cobra.Command, _ []string) error {

cmd/ethermintd/util.go

@@ -28,8 +28,14 @@ import (
 // to get the Tendermint configuration or to get access to Viper.
 func InterceptConfigsPreRunHandler(cmd *cobra.Command) error {
 	rootViper := viper.New()
-	rootViper.BindPFlags(cmd.Flags())
+	err := rootViper.BindPFlags(cmd.Flags())
-	rootViper.BindPFlags(cmd.PersistentFlags())
+	if err != nil {
+		return err
+	}
+	err = rootViper.BindPFlags(cmd.PersistentFlags())
+	if err != nil {
+		return err
+	}
 
 	serverCtx := server.NewDefaultContext()
 	config, err := interceptConfigs(serverCtx, rootViper)

ethereum/rpc/eth_api.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"math/big"
 	"strings"
-	"sync"
 
 	"github.com/gogo/protobuf/jsonpb"
 	"github.com/pkg/errors"
@@ -45,7 +44,6 @@ type PublicEthAPI struct {
 	logger       log.Logger
 	backend      Backend
 	nonceLock    *rpctypes.AddrLocker
-	keyringLock  sync.Mutex
 }
 
 // NewPublicEthAPI creates an instance of the public ETH Web3 API.
@@ -736,7 +734,7 @@ func (e *PublicEthAPI) GetTransactionReceipt(hash common.Hash) (map[string]inter
 		return nil, err
 	}
 
-	cumulativeGasUsed := uint64(tx.Receipt.Result.GasUsed)
+	cumulativeGasUsed := tx.Receipt.Result.GasUsed
 	if tx.Receipt.Index != 0 {
 		cumulativeGasUsed += rpctypes.GetBlockCumulativeGas(e.clientCtx, block.Block, int(tx.Receipt.Index))
 	}

ethereum/rpc/pubsub/pubsub.go

@@ -86,19 +86,15 @@ func (m *memEventBus) Subscribe(name string) (<-chan coretypes.ResultEvent, erro
 
 func (m *memEventBus) publishTopic(name string, src <-chan coretypes.ResultEvent) {
 	for {
-		select {
+		msg, ok := <-src
-		case msg, ok := <-src:
+		if !ok {
-			if !ok {
+			m.closeAllSubscribers(name)
-				m.closeAllSubscribers(name)
+			m.topicsMux.Lock()
-				m.topicsMux.Lock()
+			delete(m.topics, name)
-				delete(m.topics, name)
+			m.topicsMux.Unlock()
-				m.topicsMux.Unlock()
+			return
-
-				return
-			}
-
-			m.publishAllSubscribers(name, msg)
 		}
+		m.publishAllSubscribers(name, msg)
 	}
 }
 

ethereum/rpc/websockets.go

@@ -65,7 +65,7 @@ type websocketsServer struct {
 	logger  log.Logger
 }
 
-func NewWebsocketsServer(tmWSClient *rpcclient.WSClient, rpcAddr, wsAddr string) *websocketsServer {
+func NewWebsocketsServer(tmWSClient *rpcclient.WSClient, rpcAddr, wsAddr string) WebsocketsServer {
 	return &websocketsServer{
 		rpcAddr: rpcAddr,
 		wsAddr:  wsAddr,
@@ -174,7 +174,7 @@ func (s *websocketsServer) readLoop(wsConn *wsConn) {
 			continue
 		}
 
-		connId := msg["id"].(float64)
+		connID := msg["id"].(float64)
 		if method == "eth_subscribe" {
 			params := msg["params"].([]interface{})
 			if len(params) == 0 {
@@ -190,7 +190,7 @@ func (s *websocketsServer) readLoop(wsConn *wsConn) {
 
 			res := &SubscriptionResponseJSON{
 				Jsonrpc: "2.0",
-				ID:      connId,
+				ID:      connID,
 				Result:  id,
 			}
 
@@ -210,7 +210,7 @@ func (s *websocketsServer) readLoop(wsConn *wsConn) {
 			ok = s.api.unsubscribe(rpc.ID(ids[0].(string)))
 			res := &SubscriptionResponseJSON{
 				Jsonrpc: "2.0",
-				ID:      connId,
+				ID:      connID,
 				Result:  ok,
 			}
 
@@ -325,18 +325,6 @@ func (api *pubSubAPI) unsubscribe(id rpc.ID) bool {
 	return true
 }
 
-func (api *pubSubAPI) getSubscriptionByQuery(query string) *Subscription {
-	api.filtersMu.Lock()
-	defer api.filtersMu.Unlock()
-
-	for _, wsSub := range api.filters {
-		if wsSub.query == query {
-			return wsSub.sub
-		}
-	}
-	return nil
-}
-
 func (api *pubSubAPI) subscribeNewHeads(wsConn *wsConn) (rpc.ID, error) {
 	var query = "subscribeNewHeads"
 	var subID = rpc.NewID()
@@ -378,6 +366,8 @@ func (api *pubSubAPI) subscribeNewHeads(wsConn *wsConn) (rpc.ID, error) {
 
 				api.filtersMu.RLock()
 				for subID, wsSub := range api.filters {
+					subID := subID
+					wsSub := wsSub
 					if wsSub.query != query {
 						continue
 					}
@@ -696,6 +686,8 @@ func (api *pubSubAPI) subscribePendingTransactions(wsConn *wsConn) (rpc.ID, erro
 
 				api.filtersMu.RLock()
 				for subID, wsSub := range api.filters {
+					subID := subID
+					wsSub := wsSub
 					if wsSub.query != query {
 						continue
 					}

tests/rpc/utils.go

@@ -23,14 +23,14 @@ type Request struct {
 	ID      int         `json:"id"`
 }
 
-type RPCError struct {
+type Error struct {
 	Code    int         `json:"code"`
 	Message string      `json:"message"`
 	Data    interface{} `json:"data,omitempty"`
 }
 
 type Response struct {
-	Error  *RPCError       `json:"error"`
+	Error  *Error          `json:"error"`
 	ID     int             `json:"id"`
 	Result json.RawMessage `json:"result,omitempty"`
 }

x/evm/client/cli/utils.go

@@ -45,20 +45,3 @@ func formatKeyToHash(key string) string {
 
 	return ethkey.Hex()
 }
-
-// nolint: deadcode
-func cosmosAddressFromArg(addr string) (sdk.AccAddress, error) {
-	if strings.HasPrefix(addr, sdk.GetConfig().GetBech32AccountAddrPrefix()) {
-		// Check to see if address is Cosmos bech32 formatted
-		toAddr, err := sdk.AccAddressFromBech32(addr)
-		if err != nil {
-			return nil, errors.Wrap(err, "invalid bech32 formatted address")
-		}
-		return toAddr, nil
-	}
-
-	// Strip 0x prefix if exists
-	addr = strings.TrimPrefix(addr, "0x")
-
-	return sdk.AccAddressFromHex(addr)
-}

x/evm/client/cli/utils_test.go

@@ -1,8 +1,10 @@
 package cli
 
 import (
+	"strings"
 	"testing"
 
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -10,6 +12,22 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 )
 
+func cosmosAddressFromArg(addr string) (sdk.AccAddress, error) {
+	if strings.HasPrefix(addr, sdk.GetConfig().GetBech32AccountAddrPrefix()) {
+		// Check to see if address is Cosmos bech32 formatted
+		toAddr, err := sdk.AccAddressFromBech32(addr)
+		if err != nil {
+			return nil, errors.Wrap(err, "invalid bech32 formatted address")
+		}
+		return toAddr, nil
+	}
+
+	// Strip 0x prefix if exists
+	addr = strings.TrimPrefix(addr, "0x")
+
+	return sdk.AccAddressFromHex(addr)
+}
+
 func TestAddressFormats(t *testing.T) {
 	testCases := []struct {
 		name        string

x/evm/keeper/statedb.go

@@ -340,7 +340,7 @@ func (k *Keeper) GetCommittedState(addr common.Address, hash common.Hash) common
 	return common.BytesToHash(value)
 }
 
-// GetState returns the commited state for the given key hash, as all changes are commited directly
+// GetState returns the committed state for the given key hash, as all changes are committed directly
 // to the KVStore.
 func (k *Keeper) GetState(addr common.Address, hash common.Hash) common.Hash {
 	return k.GetCommittedState(addr, hash)

x/evm/types/access_list.go

@@ -16,7 +16,7 @@ func NewAccessList(ethAccessList *ethtypes.AccessList) AccessList {
 		return nil
 	}
 
-	var AccessListMappings AccessList
+	al := AccessList{}
 	for _, tuple := range *ethAccessList {
 		storageKeys := make([]string, len(tuple.StorageKeys))
 
@@ -24,19 +24,19 @@ func NewAccessList(ethAccessList *ethtypes.AccessList) AccessList {
 			storageKeys[i] = tuple.StorageKeys[i].String()
 		}
 
-		AccessListMappings = append(AccessListMappings, AccessTuple{
+		al = append(al, AccessTuple{
 			Address:     tuple.Address.String(),
 			StorageKeys: storageKeys,
 		})
 	}
 
-	return AccessListMappings
+	return al
 }
 
 // ToEthAccessList is an utility function to convert the protobuf compatible
 // AccessList to eth core AccessList from go-ethereum
 func (al AccessList) ToEthAccessList() *ethtypes.AccessList {
-	var AccessListMappings ethtypes.AccessList
+	var ethAccessList ethtypes.AccessList
 
 	for _, tuple := range al {
 		storageKeys := make([]ethcmn.Hash, len(tuple.StorageKeys))
@@ -45,11 +45,11 @@ func (al AccessList) ToEthAccessList() *ethtypes.AccessList {
 			storageKeys[i] = ethcmn.HexToHash(tuple.StorageKeys[i])
 		}
 
-		AccessListMappings = append(AccessListMappings, ethtypes.AccessTuple{
+		ethAccessList = append(ethAccessList, ethtypes.AccessTuple{
 			Address:     ethcmn.HexToAddress(tuple.Address),
 			StorageKeys: storageKeys,
 		})
 	}
 
-	return &AccessListMappings
+	return &ethAccessList
 }

x/evm/types/journal.go

@@ -191,13 +191,6 @@ type (
 		// prev      bool
 		// prevDirty bool
 	}
-	accessListAddAccountChange struct {
-		address *ethcmn.Address
-	}
-	accessListAddSlotChange struct {
-		address *ethcmn.Address
-		slot    *ethcmn.Hash
-	}
 )
 
 func (ch createObjectChange) revert(s *CommitStateDB) {
@@ -353,28 +346,3 @@ func (ch addPreimageChange) revert(s *CommitStateDB) {
 func (ch addPreimageChange) dirtied() *ethcmn.Address {
 	return nil
 }
-
-func (ch accessListAddAccountChange) revert(s *CommitStateDB) {
-	/*
-		One important invariant here, is that whenever a (addr, slot) is added, if the
-		addr is not already present, the add causes two journal entries:
-		- one for the address,
-		- one for the (address,slot)
-		Therefore, when unrolling the change, we can always blindly delete the
-		(addr) at this point, since no storage adds can remain when come upon
-		a single (addr) change.
-	*/
-	// s.accessList.DeleteAddress(*ch.address)
-}
-
-func (ch accessListAddAccountChange) dirtied() *ethcmn.Address {
-	return nil
-}
-
-func (ch accessListAddSlotChange) revert(s *CommitStateDB) {
-	// s.accessList.DeleteSlot(*ch.address, *ch.slot)
-}
-
-func (ch accessListAddSlotChange) dirtied() *ethcmn.Address {
-	return nil
-}

x/evm/types/statedb.go

@@ -127,49 +127,37 @@ func (csdb *CommitStateDB) SetParams(params Params) {
 // SetBalance sets the balance of an account.
 func (csdb *CommitStateDB) SetBalance(addr ethcmn.Address, amount *big.Int) {
 	so := csdb.GetOrNewStateObject(addr)
-	if so != nil {
+	so.SetBalance(amount)
-		so.SetBalance(amount)
-	}
 }
 
 // AddBalance adds amount to the account associated with addr.
 func (csdb *CommitStateDB) AddBalance(addr ethcmn.Address, amount *big.Int) {
 	so := csdb.GetOrNewStateObject(addr)
-	if so != nil {
+	so.AddBalance(amount)
-		so.AddBalance(amount)
-	}
 }
 
 // SubBalance subtracts amount from the account associated with addr.
 func (csdb *CommitStateDB) SubBalance(addr ethcmn.Address, amount *big.Int) {
 	so := csdb.GetOrNewStateObject(addr)
-	if so != nil {
+	so.SubBalance(amount)
-		so.SubBalance(amount)
-	}
 }
 
 // SetNonce sets the nonce (sequence number) of an account.
 func (csdb *CommitStateDB) SetNonce(addr ethcmn.Address, nonce uint64) {
 	so := csdb.GetOrNewStateObject(addr)
-	if so != nil {
+	so.SetNonce(nonce)
-		so.SetNonce(nonce)
-	}
 }
 
 // SetState sets the storage state with a key, value pair for an account.
 func (csdb *CommitStateDB) SetState(addr ethcmn.Address, key, value ethcmn.Hash) {
 	so := csdb.GetOrNewStateObject(addr)
-	if so != nil {
+	so.SetState(nil, key, value)
-		so.SetState(nil, key, value)
-	}
 }
 
 // SetCode sets the code for a given account.
 func (csdb *CommitStateDB) SetCode(addr ethcmn.Address, code []byte) {
 	so := csdb.GetOrNewStateObject(addr)
-	if so != nil {
+	so.SetCode(ethcrypto.Keccak256Hash(code), code)
-		so.SetCode(ethcrypto.Keccak256Hash(code), code)
-	}
 }
 
 // ----------------------------------------------------------------------------