From e3270aee5dbbd5f38b2f6806de5e38a7f10a1bcd Mon Sep 17 00:00:00 2001 From: Akash Khosla Date: Tue, 8 Jun 2021 03:07:11 -0400 Subject: [PATCH] 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 | 19 ++++++++-------- .golangci.yml | 5 ----- Makefile | 1 + client/testnet.go | 40 ---------------------------------- cmd/ethermintd/flags.go | 8 +++++-- cmd/ethermintd/genaccounts.go | 1 - cmd/ethermintd/root.go | 29 +----------------------- cmd/ethermintd/start.go | 9 ++++---- cmd/ethermintd/util.go | 10 +++++++-- ethereum/rpc/eth_api.go | 4 +--- ethereum/rpc/pubsub/pubsub.go | 20 +++++++---------- ethereum/rpc/websockets.go | 24 +++++++------------- tests/rpc/utils.go | 4 ++-- x/evm/client/cli/utils.go | 17 --------------- x/evm/client/cli/utils_test.go | 18 +++++++++++++++ x/evm/keeper/statedb.go | 2 +- x/evm/types/access_list.go | 12 +++++----- x/evm/types/journal.go | 32 --------------------------- x/evm/types/statedb.go | 24 +++++--------------- 19 files changed, 81 insertions(+), 198 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c718c838..ccca39c0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,15 +1,15 @@ -name: Lint -# Lint runs golangci-lint over the entire cosmos-sdk repository -# This workflow is run on every pull request and push to development -# The `golangci` will pass without running if no *.{go, mod, sum} files have been changed. +name: Run Lint +# Lint runs golangci-lint over the entire ethermint repository This workflow is +# run on every pull request and push to main The `golangci` will pass without +# 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" diff --git a/.golangci.yml b/.golangci.yml index e5419ed2..2d914549 100644 --- a/.golangci.yml +++ b/.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 diff --git a/Makefile b/Makefile index 1626fe5f..6d4b4624 100755 --- a/Makefile +++ b/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} diff --git a/client/testnet.go b/client/testnet.go index bdc51d7b..d9a5fd2e 100644 --- a/client/testnet.go +++ b/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 -} diff --git a/cmd/ethermintd/flags.go b/cmd/ethermintd/flags.go index 7c9afa56..eaab57be 100644 --- a/cmd/ethermintd/flags.go +++ b/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 } diff --git a/cmd/ethermintd/genaccounts.go b/cmd/ethermintd/genaccounts.go index a5663ba8..9d417a95 100644 --- a/cmd/ethermintd/genaccounts.go +++ b/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. diff --git a/cmd/ethermintd/root.go b/cmd/ethermintd/root.go index 119c3def..46dec4a1 100644 --- a/cmd/ethermintd/root.go +++ b/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) -} - -func addModuleInitFlags(startCmd *cobra.Command) { - crisis.AddModuleInitFlags(startCmd) + return rootCmd, encodingConfig } func queryCommand() *cobra.Command { diff --git a/cmd/ethermintd/start.go b/cmd/ethermintd/start.go index fc43604a..8c496b9e 100644 --- a/cmd/ethermintd/start.go +++ b/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 { diff --git a/cmd/ethermintd/util.go b/cmd/ethermintd/util.go index dfec959e..a5edc046 100644 --- a/cmd/ethermintd/util.go +++ b/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()) - rootViper.BindPFlags(cmd.PersistentFlags()) + err := rootViper.BindPFlags(cmd.Flags()) + if err != nil { + return err + } + err = rootViper.BindPFlags(cmd.PersistentFlags()) + if err != nil { + return err + } serverCtx := server.NewDefaultContext() config, err := interceptConfigs(serverCtx, rootViper) diff --git a/ethereum/rpc/eth_api.go b/ethereum/rpc/eth_api.go index 50e5d15e..dfdbf23d 100644 --- a/ethereum/rpc/eth_api.go +++ b/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)) } diff --git a/ethereum/rpc/pubsub/pubsub.go b/ethereum/rpc/pubsub/pubsub.go index 835d7a88..8ecf5ca7 100644 --- a/ethereum/rpc/pubsub/pubsub.go +++ b/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 { - case msg, ok := <-src: - if !ok { - m.closeAllSubscribers(name) - m.topicsMux.Lock() - delete(m.topics, name) - m.topicsMux.Unlock() - - return - } - - m.publishAllSubscribers(name, msg) + msg, ok := <-src + if !ok { + m.closeAllSubscribers(name) + m.topicsMux.Lock() + delete(m.topics, name) + m.topicsMux.Unlock() + return } + m.publishAllSubscribers(name, msg) } } diff --git a/ethereum/rpc/websockets.go b/ethereum/rpc/websockets.go index a9d20b40..1c783623 100644 --- a/ethereum/rpc/websockets.go +++ b/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 } diff --git a/tests/rpc/utils.go b/tests/rpc/utils.go index c2a1a795..b3bbab88 100644 --- a/tests/rpc/utils.go +++ b/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"` } diff --git a/x/evm/client/cli/utils.go b/x/evm/client/cli/utils.go index 389c3235..7277cdae 100644 --- a/x/evm/client/cli/utils.go +++ b/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) -} diff --git a/x/evm/client/cli/utils_test.go b/x/evm/client/cli/utils_test.go index 8ddb3102..70c718ba 100644 --- a/x/evm/client/cli/utils_test.go +++ b/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 diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index e23bfe96..a4c157f0 100644 --- a/x/evm/keeper/statedb.go +++ b/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) diff --git a/x/evm/types/access_list.go b/x/evm/types/access_list.go index 2e67ac93..4a0196df 100644 --- a/x/evm/types/access_list.go +++ b/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 ðAccessList } diff --git a/x/evm/types/journal.go b/x/evm/types/journal.go index 65a5b307..5952c841 100644 --- a/x/evm/types/journal.go +++ b/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 -} diff --git a/x/evm/types/statedb.go b/x/evm/types/statedb.go index fa8aefb6..d9eb44db 100644 --- a/x/evm/types/statedb.go +++ b/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) } // ----------------------------------------------------------------------------