From 87b625ed508a22dafa314b28205b591f5837615f Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Mon, 16 Mar 2020 19:53:24 -0300 Subject: [PATCH] Tools configuration and linters (#215) * tools config * lint * lint * codecov --- .circleci/config.yml | 21 ++++++++++++ .codecov.yml | 58 +++++++++++++++++++++++++++++++++ .golangci.yml | 38 ++++++++++++++++----- cmd/emintd/main.go | 21 ++++++------ importer/importer_test.go | 1 + rpc/eth_api.go | 3 +- rpc/net_api.go | 2 +- types/account.go | 14 ++++---- x/evm/types/genesis.go | 4 +-- x/evm/types/msg.go | 1 + x/evm/types/msg_encoding.go | 25 ++++++++------ x/evm/types/msg_test.go | 17 +++++----- x/evm/types/state_object.go | 14 +++----- x/evm/types/state_transition.go | 14 ++++---- x/evm/types/statedb.go | 5 +-- x/evm/types/statedb_test.go | 3 +- x/evm/types/utils.go | 2 +- 17 files changed, 172 insertions(+), 71 deletions(-) create mode 100644 .codecov.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index 5593191a..306291d0 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -59,6 +59,27 @@ jobs: key: go-mod-v1-{{ checksum "go.sum" }} paths: - "/go/pkg/mod" + + upload-coverage: + docker: # run the steps with Docker + # CircleCI Go images available at: https://hub.docker.com/r/circleci/golang/ + - image: circleci/golang:1.13 # + working_directory: /go/src/github.com/cosmos/ethermint + steps: + - checkout + - run: + name: gather + command: | + for d in $(go list ./... | grep -v vendor); do + go test -race -coverprofile=profile.out -covermode=atomic "$d" + if [ -f profile.out ]; then + cat profile.out >> coverage.txt + rm profile.out + fi + done + - run: + name: upload + command: bash <(curl -s https://codecov.io/bash) -f coverage.txt workflows: version: 2 diff --git a/.codecov.yml b/.codecov.yml new file mode 100644 index 00000000..2517bd53 --- /dev/null +++ b/.codecov.yml @@ -0,0 +1,58 @@ +# +# This codecov.yml is the default configuration for +# all repositories on Codecov. You may adjust the settings +# below in your own codecov.yml in your repository. +# +coverage: + precision: 2 + round: down + range: 70...100 + + status: + # Learn more at https://docs.codecov.io/docs/commit-status + project: + default: + threshold: 1% # allow this much decrease on project + app: + target: 70% + flags: app + modules: + target: 50% + flags: modules + core: + target: 50% + flags: core + clients: + flags: clients + changes: false + +comment: + layout: "reach, diff, files" + behavior: default # update if exists else create new + require_changes: true + +flags: + app: + paths: + - "app/" + modules: + paths: + - "x/" + - "!x/**/client/" # ignore client package + core: + paths: + - "core/" + - "crypto/" + - "types/" + clients: + paths: + - "rpc/" + - "client/" + - "x/**/client/" + +ignore: + - "docs" + - "*.md" + - "**/*.pb.go" + - "types/*.pb.go" + - "x/**/*.pb.go" diff --git a/.golangci.yml b/.golangci.yml index ccdc3b2c..550f8344 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,11 +1,9 @@ -# This file configures github.com/golangci/golangci-lint. - -run: - timeout: 2m - tests: true - skip-dirs-use-default: true +# run: +# # timeout for analysis, e.g. 30s, 5m, default is 1m +# timeout: 5m linters: + disable-all: true enable: - bodyclose - deadcode @@ -13,7 +11,7 @@ linters: - dogsled - errcheck - goconst - - gocyclo + - gocritic - gofmt - goimports - golint @@ -22,7 +20,10 @@ linters: - govet - ineffassign - interfacer + - maligned - misspell + - nakedret + - prealloc - scopelint - staticcheck - structcheck @@ -30,13 +31,32 @@ linters: - typecheck - unconvert - unused + - misspell - varcheck issues: exclude-rules: + - text: "Use of weak random number generator" + linters: + - gosec - text: "comment on exported var" linters: - golint - - text: "ST1005:" + - text: "don't use an underscore in package name" linters: - - stylecheck \ No newline at end of file + - golint + - 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 + +linters-settings: + dogsled: + max-blank-identifiers: 3 + maligned: + # print struct with more effective memory layout or not, false by default + suggest-new: true diff --git a/cmd/emintd/main.go b/cmd/emintd/main.go index 08d87312..38930a6d 100644 --- a/cmd/emintd/main.go +++ b/cmd/emintd/main.go @@ -23,7 +23,6 @@ import ( "github.com/spf13/viper" "github.com/cosmos/ethermint/app" - emintapp "github.com/cosmos/ethermint/app" "github.com/cosmos/ethermint/client/genaccounts" emintcrypto "github.com/cosmos/ethermint/crypto" @@ -38,7 +37,7 @@ import ( func main() { cobra.EnableCommandSorting = false - cdc := emintapp.MakeCodec() + cdc := app.MakeCodec() tmamino.RegisterKeyType(emintcrypto.PubKeySecp256k1{}, emintcrypto.PubKeyAminoName) tmamino.RegisterKeyType(emintcrypto.PrivKeySecp256k1{}, emintcrypto.PrivKeyAminoName) @@ -63,12 +62,12 @@ func main() { } // CLI commands to initialize the chain rootCmd.AddCommand( - withChainIDValidation(genutilcli.InitCmd(ctx, cdc, emintapp.ModuleBasics, emintapp.DefaultNodeHome)), - genutilcli.CollectGenTxsCmd(ctx, cdc, auth.GenesisAccountIterator{}, emintapp.DefaultNodeHome), + withChainIDValidation(genutilcli.InitCmd(ctx, cdc, app.ModuleBasics, app.DefaultNodeHome)), + genutilcli.CollectGenTxsCmd(ctx, cdc, auth.GenesisAccountIterator{}, app.DefaultNodeHome), genutilcli.GenTxCmd( - ctx, cdc, emintapp.ModuleBasics, staking.AppModuleBasic{}, auth.GenesisAccountIterator{}, emintapp.DefaultNodeHome, emintapp.DefaultCLIHome, + ctx, cdc, app.ModuleBasics, staking.AppModuleBasic{}, auth.GenesisAccountIterator{}, app.DefaultNodeHome, app.DefaultCLIHome, ), - genutilcli.ValidateGenesisCmd(ctx, cdc, emintapp.ModuleBasics), + genutilcli.ValidateGenesisCmd(ctx, cdc, app.ModuleBasics), // AddGenesisAccountCmd allows users to add accounts to the genesis file genaccounts.AddGenesisAccountCmd(ctx, cdc, app.DefaultNodeHome, app.DefaultCLIHome), @@ -78,7 +77,7 @@ func main() { server.AddCommands(ctx, cdc, rootCmd, newApp, exportAppStateAndTMValidators) // prepare and add flags - executor := cli.PrepareBaseCmd(rootCmd, "EM", emintapp.DefaultNodeHome) + executor := cli.PrepareBaseCmd(rootCmd, "EM", app.DefaultNodeHome) err := executor.Execute() if err != nil { panic(err) @@ -86,7 +85,7 @@ func main() { } func newApp(logger tmlog.Logger, db dbm.DB, traceStore io.Writer) abci.Application { - return emintapp.NewEthermintApp(logger, db, true, 0, + return app.NewEthermintApp(logger, db, true, 0, baseapp.SetPruning(store.NewPruningOptionsFromString(viper.GetString("pruning")))) } @@ -95,7 +94,7 @@ func exportAppStateAndTMValidators( ) (json.RawMessage, []tmtypes.GenesisValidator, error) { if height != -1 { - emintApp := emintapp.NewEthermintApp(logger, db, true, 0) + emintApp := app.NewEthermintApp(logger, db, true, 0) err := emintApp.LoadHeight(height) if err != nil { return nil, nil, err @@ -103,7 +102,7 @@ func exportAppStateAndTMValidators( return emintApp.ExportAppStateAndValidators(forZeroHeight, jailWhiteList) } - emintApp := emintapp.NewEthermintApp(logger, db, true, 0) + emintApp := app.NewEthermintApp(logger, db, true, 0) return emintApp.ExportAppStateAndValidators(forZeroHeight, jailWhiteList) } @@ -121,7 +120,7 @@ func withChainIDValidation(baseCmd *cobra.Command) *cobra.Command { _, ok := new(big.Int).SetString(chainIDFlag, 10) if !ok { return fmt.Errorf( - fmt.Sprintf("Invalid chainID: %s, must be base-10 integer format", chainIDFlag)) + fmt.Sprintf("invalid chainID: %s, must be base-10 integer format", chainIDFlag)) } return baseRunE(cmd, args) diff --git a/importer/importer_test.go b/importer/importer_test.go index 26cd6656..847c66f0 100644 --- a/importer/importer_test.go +++ b/importer/importer_test.go @@ -199,6 +199,7 @@ func TestImportBlocks(t *testing.T) { blockchainInput, err := os.Open(flagBlockchain) require.Nil(t, err) + // nolint: gosec defer blockchainInput.Close() // ethereum mainnet config diff --git a/rpc/eth_api.go b/rpc/eth_api.go index b4c2e969..502c9a67 100644 --- a/rpc/eth_api.go +++ b/rpc/eth_api.go @@ -12,7 +12,6 @@ import ( emintcrypto "github.com/cosmos/ethermint/crypto" params "github.com/cosmos/ethermint/rpc/args" emint "github.com/cosmos/ethermint/types" - etypes "github.com/cosmos/ethermint/types" "github.com/cosmos/ethermint/utils" "github.com/cosmos/ethermint/version" "github.com/cosmos/ethermint/x/evm" @@ -885,7 +884,7 @@ func (e *PublicEthAPI) generateFromArgs(args params.SendTxArgs) (msg *types.Ethe // Set default gas price // TODO: Change to min gas price from context once available through server/daemon - gasPrice = big.NewInt(etypes.DefaultGasPrice) + gasPrice = big.NewInt(emint.DefaultGasPrice) } if args.Nonce == nil { diff --git a/rpc/net_api.go b/rpc/net_api.go index 058fd35b..83c6cab4 100644 --- a/rpc/net_api.go +++ b/rpc/net_api.go @@ -20,7 +20,7 @@ func NewPublicNetAPI(cliCtx context.CLIContext) *PublicNetAPI { // parse the chainID from a integer string intChainID, err := strconv.ParseUint(chainID, 0, 64) if err != nil { - panic(fmt.Sprintf("Invalid chainID: %s, must be integer format", chainID)) + panic(fmt.Sprintf("invalid chainID: %s, must be integer format", chainID)) } return &PublicNetAPI{ diff --git a/types/account.go b/types/account.go index 4b760dfe..110426e1 100644 --- a/types/account.go +++ b/types/account.go @@ -59,15 +59,17 @@ func (acc Account) Balance() sdk.Int { func (acc Account) SetBalance(amt sdk.Int) { coins := acc.GetCoins() diff := amt.Sub(coins.AmountOf(DenomDefault)) - if diff.IsZero() { - return - } else if diff.IsPositive() { + switch { + case diff.IsPositive(): // Increase coins to amount - coins = coins.Add(sdk.Coins{sdk.NewCoin(DenomDefault, diff)}) - } else { + coins = coins.Add(sdk.NewCoins(sdk.NewCoin(DenomDefault, diff))) + case diff.IsNegative(): // Decrease coins to amount - coins = coins.Sub(sdk.Coins{sdk.NewCoin(DenomDefault, diff.Neg())}) + coins = coins.Sub(sdk.NewCoins(sdk.NewCoin(DenomDefault, diff.Neg()))) + default: + return } + if err := acc.SetCoins(coins); err != nil { panic(fmt.Sprintf("Could not set coins for address %s", acc.GetAddress())) } diff --git a/x/evm/types/genesis.go b/x/evm/types/genesis.go index 10feb666..30320a39 100644 --- a/x/evm/types/genesis.go +++ b/x/evm/types/genesis.go @@ -28,10 +28,10 @@ type ( func ValidateGenesis(data GenesisState) error { for _, acct := range data.Accounts { if len(acct.Address.Bytes()) == 0 { - return fmt.Errorf("Invalid GenesisAccount Error: Missing Address") + return fmt.Errorf("invalid GenesisAccount Error: Missing Address") } if acct.Balance == nil { - return fmt.Errorf("Invalid GenesisAccount Error: Missing Balance") + return fmt.Errorf("invalid GenesisAccount Error: Missing Balance") } } return nil diff --git a/x/evm/types/msg.go b/x/evm/types/msg.go index eecbd39b..980bdd6f 100644 --- a/x/evm/types/msg.go +++ b/x/evm/types/msg.go @@ -340,6 +340,7 @@ func TxDecoder(cdc *codec.Codec) sdk.TxDecoder { // returns the sender or an error. // // Ref: Ethereum Yellow Paper (BYZANTIUM VERSION 69351d5) Appendix F +// nolint: gocritic func recoverEthSig(R, S, Vb *big.Int, sigHash ethcmn.Hash) (ethcmn.Address, error) { if Vb.BitLen() > 8 { return ethcmn.Address{}, errors.New("invalid signature") diff --git a/x/evm/types/msg_encoding.go b/x/evm/types/msg_encoding.go index 5e62e434..1284d3a5 100644 --- a/x/evm/types/msg_encoding.go +++ b/x/evm/types/msg_encoding.go @@ -42,7 +42,7 @@ func marshalAmino(td EncodableTxData) (string, error) { return string(bz), err } -func unmarshalAmino(td *EncodableTxData, text string) (err error) { +func unmarshalAmino(td *EncodableTxData, text string) error { return cdc.UnmarshalBinaryBare([]byte(text), td) } @@ -67,11 +67,11 @@ func (td TxData) MarshalAmino() (string, error) { } // UnmarshalAmino defines custom decoding scheme for TxData -func (td *TxData) UnmarshalAmino(text string) (err error) { +func (td *TxData) UnmarshalAmino(text string) error { e := new(EncodableTxData) - err = unmarshalAmino(e, text) + err := unmarshalAmino(e, text) if err != nil { - return + return err } td.AccountNonce = e.AccountNonce @@ -82,8 +82,9 @@ func (td *TxData) UnmarshalAmino(text string) (err error) { price, err := utils.UnmarshalBigInt(e.Price) if err != nil { - return + return err } + if td.Price != nil { td.Price.Set(price) } else { @@ -92,8 +93,9 @@ func (td *TxData) UnmarshalAmino(text string) (err error) { amt, err := utils.UnmarshalBigInt(e.Amount) if err != nil { - return + return err } + if td.Amount != nil { td.Amount.Set(amt) } else { @@ -102,8 +104,9 @@ func (td *TxData) UnmarshalAmino(text string) (err error) { v, err := utils.UnmarshalBigInt(e.V) if err != nil { - return + return err } + if td.V != nil { td.V.Set(v) } else { @@ -112,8 +115,9 @@ func (td *TxData) UnmarshalAmino(text string) (err error) { r, err := utils.UnmarshalBigInt(e.R) if err != nil { - return + return err } + if td.R != nil { td.R.Set(r) } else { @@ -122,15 +126,16 @@ func (td *TxData) UnmarshalAmino(text string) (err error) { s, err := utils.UnmarshalBigInt(e.S) if err != nil { - return + return err } + if td.S != nil { td.S.Set(s) } else { td.S = s } - return + return nil } // TODO: Implement JSON marshaling/ unmarshaling for this type diff --git a/x/evm/types/msg_test.go b/x/evm/types/msg_test.go index 3ec12649..a7654d24 100644 --- a/x/evm/types/msg_test.go +++ b/x/evm/types/msg_test.go @@ -9,7 +9,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/ethermint/crypto" "github.com/cosmos/ethermint/utils" - "github.com/ethereum/go-ethereum/common" ethcmn "github.com/ethereum/go-ethereum/common" ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/rlp" @@ -36,12 +35,12 @@ func TestMsgEthereumTx(t *testing.T) { func TestMsgEthereumTxValidation(t *testing.T) { testCases := []struct { + payload []byte + amount *big.Int + gasPrice *big.Int + gasLimit uint64 nonce uint64 to ethcmn.Address - amount *big.Int - gasLimit uint64 - gasPrice *big.Int - payload []byte expectPass bool }{ {amount: big.NewInt(100), gasPrice: big.NewInt(100000), expectPass: true}, @@ -183,13 +182,13 @@ func TestMarshalAndUnmarshalLogs(t *testing.T) { logs := []*ethtypes.Log{ { - Address: common.BytesToAddress([]byte{0x11}), - TxHash: common.HexToHash("0x01"), + Address: ethcmn.BytesToAddress([]byte{0x11}), + TxHash: ethcmn.HexToHash("0x01"), // May need to find workaround since Topics is required to unmarshal from JSON - Topics: []common.Hash{}, + Topics: []ethcmn.Hash{}, Removed: true, }, - {Address: common.BytesToAddress([]byte{0x01, 0x11}), Topics: []common.Hash{}}, + {Address: ethcmn.BytesToAddress([]byte{0x01, 0x11}), Topics: []ethcmn.Hash{}}, } raw, err := codec.MarshalJSONIndent(cdc, logs) diff --git a/x/evm/types/state_object.go b/x/evm/types/state_object.go index 5130fe30..2ae43d28 100644 --- a/x/evm/types/state_object.go +++ b/x/evm/types/state_object.go @@ -51,22 +51,18 @@ type ( // Account values can be accessed and modified through the object. // Finally, call CommitTrie to write the modified storage trie into a database. stateObject struct { - address ethcmn.Address - stateDB *CommitStateDB - account *types.Account - + code types.Code // contract bytecode, which gets set when code is loaded // DB error. // State objects are used by the consensus core and VM which are // unable to deal with database-level errors. Any error that occurs // during a database read is memoized here and will eventually be returned // by StateDB.Commit. - dbErr error - - code types.Code // contract bytecode, which gets set when code is loaded - + dbErr error + stateDB *CommitStateDB + account *types.Account originStorage types.Storage // Storage cache of original entries to dedup rewrites dirtyStorage types.Storage // Storage entries that need to be flushed to disk - + address ethcmn.Address // cache flags // // When an object is marked suicided it will be delete from the trie during diff --git a/x/evm/types/state_transition.go b/x/evm/types/state_transition.go index 79047f1f..058d9d57 100644 --- a/x/evm/types/state_transition.go +++ b/x/evm/types/state_transition.go @@ -14,16 +14,16 @@ import ( // StateTransition defines data to transitionDB in evm type StateTransition struct { - Sender common.Address - AccountNonce uint64 - Price *big.Int - GasLimit uint64 - Recipient *common.Address - Amount *big.Int Payload []byte - Csdb *CommitStateDB + Recipient *common.Address + AccountNonce uint64 + GasLimit uint64 + Price *big.Int + Amount *big.Int ChainID *big.Int + Csdb *CommitStateDB THash *common.Hash + Sender common.Address Simulate bool } diff --git a/x/evm/types/statedb.go b/x/evm/types/statedb.go index d58e89c8..763c83ef 100644 --- a/x/evm/types/statedb.go +++ b/x/evm/types/statedb.go @@ -294,6 +294,7 @@ func (csdb *CommitStateDB) GetLogs(hash ethcmn.Hash) []*ethtypes.Log { // Logs returns all the current logs in the state. func (csdb *CommitStateDB) Logs() []*ethtypes.Log { + // nolint: prealloc var logs []*ethtypes.Log for _, lgs := range csdb.logs { logs = append(logs, lgs...) @@ -337,7 +338,7 @@ func (csdb *CommitStateDB) StorageTrie(addr ethcmn.Address) ethstate.Trie { // in the cache, it will either be removed, or have it's code set and/or it's // state (storage) updated. In addition, the state object (account) itself will // be written. Finally, the root hash (version) will be returned. -func (csdb *CommitStateDB) Commit(deleteEmptyObjects bool) (root ethcmn.Hash, err error) { +func (csdb *CommitStateDB) Commit(deleteEmptyObjects bool) (ethcmn.Hash, error) { defer csdb.clearJournalAndRefund() // remove dirty state object entries based on the journal @@ -372,7 +373,7 @@ func (csdb *CommitStateDB) Commit(deleteEmptyObjects bool) (root ethcmn.Hash, er // NOTE: Ethereum returns the trie merkle root here, but as commitment // actually happens in the BaseApp at EndBlocker, we do not know the root at // this time. - return + return ethcmn.Hash{}, nil } // Finalise finalizes the state objects (accounts) state by setting their state, diff --git a/x/evm/types/statedb_test.go b/x/evm/types/statedb_test.go index c122f1e1..9cf624bb 100644 --- a/x/evm/types/statedb_test.go +++ b/x/evm/types/statedb_test.go @@ -12,7 +12,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/params" "github.com/stretchr/testify/require" - "github.com/ethereum/go-ethereum/common" ethcmn "github.com/ethereum/go-ethereum/common" ethtypes "github.com/ethereum/go-ethereum/core/types" @@ -79,7 +78,7 @@ func TestBloomFilter(t *testing.T) { // Prepare db for logs tHash := ethcmn.BytesToHash([]byte{0x1}) - stateDB.Prepare(tHash, common.Hash{}, 0) + stateDB.Prepare(tHash, ethcmn.Hash{}, 0) contractAddress := ethcmn.BigToAddress(big.NewInt(1)) diff --git a/x/evm/types/utils.go b/x/evm/types/utils.go index ab3822a1..706b2c0d 100644 --- a/x/evm/types/utils.go +++ b/x/evm/types/utils.go @@ -67,7 +67,7 @@ func DecodeReturnData(bytes []byte) (addr ethcmn.Address, bloom ethtypes.Bloom, bloom = ethtypes.BytesToBloom(bytes[bloomIdx:returnIdx]) ret = bytes[returnIdx:] } else { - err = fmt.Errorf("Invalid format for encoded data, message must be an EVM state transition") + err = fmt.Errorf("invalid format for encoded data, message must be an EVM state transition") } return