From fc33576bac293822af879737ccfd9e9478dd113a Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 5 Sep 2017 16:21:21 -0400 Subject: [PATCH 01/32] linting: replace megacheck with metalinter --- Makefile | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 2271abeb..d935833b 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,7 @@ GOTOOLS = \ github.com/mitchellh/gox \ github.com/tcnksm/ghr \ github.com/Masterminds/glide \ + github.com/alecthomas/gometalinter PACKAGES=$(shell go list ./... | grep -v '/vendor/') BUILD_TAGS?=tendermint @@ -79,8 +80,8 @@ ensure_tools: ### Formatting, linting, and vetting -megacheck: - @for pkg in ${PACKAGES}; do megacheck "$$pkg"; done - +metalinter: ensure_tools + @gometalinter --install + gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... .PHONY: install build build_race dist test test_race test_integrations test100 draw_deps list_deps get_deps get_vendor_deps update_deps revision tools From 46ccbcbff61b541e8761dbca76b6e9e7d5ded351 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 5 Sep 2017 16:37:20 -0400 Subject: [PATCH 02/32] linting: apply 'gofmt -s -w' throughout --- benchmarks/codec_test.go | 2 +- blockchain/reactor.go | 2 +- blockchain/store.go | 2 +- cmd/tendermint/commands/init.go | 2 +- consensus/reactor.go | 8 ++++---- mempool/reactor.go | 2 +- p2p/peer_test.go | 2 +- p2p/pex_reactor.go | 2 +- p2p/switch_test.go | 16 ++++++++-------- state/execution_test.go | 2 +- state/txindex/kv/kv.go | 1 + types/validator_set_test.go | 2 +- 12 files changed, 22 insertions(+), 21 deletions(-) diff --git a/benchmarks/codec_test.go b/benchmarks/codec_test.go index 7162e63d..3650d281 100644 --- a/benchmarks/codec_test.go +++ b/benchmarks/codec_test.go @@ -4,9 +4,9 @@ import ( "testing" "github.com/tendermint/go-crypto" - "github.com/tendermint/tendermint/p2p" "github.com/tendermint/go-wire" proto "github.com/tendermint/tendermint/benchmarks/proto" + "github.com/tendermint/tendermint/p2p" ctypes "github.com/tendermint/tendermint/rpc/core/types" ) diff --git a/blockchain/reactor.go b/blockchain/reactor.go index 64e5e937..4693eee5 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -108,7 +108,7 @@ func (bcR *BlockchainReactor) OnStop() { // GetChannels implements Reactor func (bcR *BlockchainReactor) GetChannels() []*p2p.ChannelDescriptor { return []*p2p.ChannelDescriptor{ - &p2p.ChannelDescriptor{ + { ID: BlockchainChannel, Priority: 10, SendQueueCapacity: 1000, diff --git a/blockchain/store.go b/blockchain/store.go index 5bf85477..7e1859f2 100644 --- a/blockchain/store.go +++ b/blockchain/store.go @@ -7,7 +7,7 @@ import ( "io" "sync" - wire "github.com/tendermint/go-wire" + "github.com/tendermint/go-wire" "github.com/tendermint/tendermint/types" . "github.com/tendermint/tmlibs/common" dbm "github.com/tendermint/tmlibs/db" diff --git a/cmd/tendermint/commands/init.go b/cmd/tendermint/commands/init.go index cbafac3e..f823de61 100644 --- a/cmd/tendermint/commands/init.go +++ b/cmd/tendermint/commands/init.go @@ -28,7 +28,7 @@ func initFiles(cmd *cobra.Command, args []string) { genDoc := types.GenesisDoc{ ChainID: cmn.Fmt("test-chain-%v", cmn.RandStr(6)), } - genDoc.Validators = []types.GenesisValidator{types.GenesisValidator{ + genDoc.Validators = []types.GenesisValidator{{ PubKey: privValidator.GetPubKey(), Power: 10, }} diff --git a/consensus/reactor.go b/consensus/reactor.go index 11cd0750..cc8faf4c 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -102,24 +102,24 @@ func (conR *ConsensusReactor) SwitchToConsensus(state *sm.State, blocksSynced in func (conR *ConsensusReactor) GetChannels() []*p2p.ChannelDescriptor { // TODO optimize return []*p2p.ChannelDescriptor{ - &p2p.ChannelDescriptor{ + { ID: StateChannel, Priority: 5, SendQueueCapacity: 100, }, - &p2p.ChannelDescriptor{ + { ID: DataChannel, // maybe split between gossiping current block and catchup stuff Priority: 10, // once we gossip the whole block there's nothing left to send until next height or round SendQueueCapacity: 100, RecvBufferCapacity: 50 * 4096, }, - &p2p.ChannelDescriptor{ + { ID: VoteChannel, Priority: 5, SendQueueCapacity: 100, RecvBufferCapacity: 100 * 100, }, - &p2p.ChannelDescriptor{ + { ID: VoteSetBitsChannel, Priority: 1, SendQueueCapacity: 2, diff --git a/mempool/reactor.go b/mempool/reactor.go index 6a876520..9e51d2df 100644 --- a/mempool/reactor.go +++ b/mempool/reactor.go @@ -50,7 +50,7 @@ func (memR *MempoolReactor) SetLogger(l log.Logger) { // It returns the list of channels for this reactor. func (memR *MempoolReactor) GetChannels() []*p2p.ChannelDescriptor { return []*p2p.ChannelDescriptor{ - &p2p.ChannelDescriptor{ + { ID: MempoolChannel, Priority: 5, }, diff --git a/p2p/peer_test.go b/p2p/peer_test.go index ba52b22a..a027a6b7 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -78,7 +78,7 @@ func TestPeerSend(t *testing.T) { func createOutboundPeerAndPerformHandshake(addr *NetAddress, config *PeerConfig) (*peer, error) { chDescs := []*ChannelDescriptor{ - &ChannelDescriptor{ID: 0x01, Priority: 1}, + {ID: 0x01, Priority: 1}, } reactorsByCh := map[byte]Reactor{0x01: NewTestReactor(chDescs, true)} pk := crypto.GenPrivKeyEd25519() diff --git a/p2p/pex_reactor.go b/p2p/pex_reactor.go index fd70198f..7c799cca 100644 --- a/p2p/pex_reactor.go +++ b/p2p/pex_reactor.go @@ -82,7 +82,7 @@ func (r *PEXReactor) OnStop() { // GetChannels implements Reactor func (r *PEXReactor) GetChannels() []*ChannelDescriptor { return []*ChannelDescriptor{ - &ChannelDescriptor{ + { ID: PexChannel, Priority: 1, SendQueueCapacity: 10, diff --git a/p2p/switch_test.go b/p2p/switch_test.go index 1ea79efe..e82eead9 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -100,12 +100,12 @@ func makeSwitchPair(t testing.TB, initSwitch func(int, *Switch) *Switch) (*Switc func initSwitchFunc(i int, sw *Switch) *Switch { // Make two reactors of two channels each sw.AddReactor("foo", NewTestReactor([]*ChannelDescriptor{ - &ChannelDescriptor{ID: byte(0x00), Priority: 10}, - &ChannelDescriptor{ID: byte(0x01), Priority: 10}, + {ID: byte(0x00), Priority: 10}, + {ID: byte(0x01), Priority: 10}, }, true)) sw.AddReactor("bar", NewTestReactor([]*ChannelDescriptor{ - &ChannelDescriptor{ID: byte(0x02), Priority: 10}, - &ChannelDescriptor{ID: byte(0x03), Priority: 10}, + {ID: byte(0x02), Priority: 10}, + {ID: byte(0x03), Priority: 10}, }, true)) return sw } @@ -295,12 +295,12 @@ func BenchmarkSwitches(b *testing.B) { s1, s2 := makeSwitchPair(b, func(i int, sw *Switch) *Switch { // Make bar reactors of bar channels each sw.AddReactor("foo", NewTestReactor([]*ChannelDescriptor{ - &ChannelDescriptor{ID: byte(0x00), Priority: 10}, - &ChannelDescriptor{ID: byte(0x01), Priority: 10}, + {ID: byte(0x00), Priority: 10}, + {ID: byte(0x01), Priority: 10}, }, false)) sw.AddReactor("bar", NewTestReactor([]*ChannelDescriptor{ - &ChannelDescriptor{ID: byte(0x02), Priority: 10}, - &ChannelDescriptor{ID: byte(0x03), Priority: 10}, + {ID: byte(0x02), Priority: 10}, + {ID: byte(0x03), Priority: 10}, }, false)) return sw }) diff --git a/state/execution_test.go b/state/execution_test.go index 8fcdcf1c..626b2ecd 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -59,7 +59,7 @@ func state() *State { s, _ := MakeGenesisState(dbm.NewMemDB(), &types.GenesisDoc{ ChainID: chainID, Validators: []types.GenesisValidator{ - types.GenesisValidator{privKey.PubKey(), 10000, "test"}, + {privKey.PubKey(), 10000, "test"}, }, AppHash: nil, }) diff --git a/state/txindex/kv/kv.go b/state/txindex/kv/kv.go index db075e54..3d4f93a4 100644 --- a/state/txindex/kv/kv.go +++ b/state/txindex/kv/kv.go @@ -10,6 +10,7 @@ import ( "github.com/tendermint/tendermint/state/txindex" "github.com/tendermint/tendermint/types" + db "github.com/tendermint/tmlibs/db" ) // TxIndex is the simplest possible indexer, backed by Key-Value storage (levelDB). diff --git a/types/validator_set_test.go b/types/validator_set_test.go index a285adee..572b7b00 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/tendermint/go-crypto" - wire "github.com/tendermint/go-wire" + "github.com/tendermint/go-wire" cmn "github.com/tendermint/tmlibs/common" ) From 1721543e5ca5329ab42e2dfccf821f6863a5d0dc Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 5 Sep 2017 16:52:25 -0400 Subject: [PATCH 03/32] linting: apply misspell --- p2p/connection.go | 2 +- p2p/switch.go | 2 +- rpc/lib/server/parse_test.go | 2 +- types/validator.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/p2p/connection.go b/p2p/connection.go index 28b136c7..5e484553 100644 --- a/p2p/connection.go +++ b/p2p/connection.go @@ -569,7 +569,7 @@ type Channel struct { func newChannel(conn *MConnection, desc ChannelDescriptor) *Channel { desc = desc.FillDefaults() if desc.Priority <= 0 { - cmn.PanicSanity("Channel default priority must be a postive integer") + cmn.PanicSanity("Channel default priority must be a positive integer") } return &Channel{ conn: conn, diff --git a/p2p/switch.go b/p2p/switch.go index c1993155..be51d561 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -511,7 +511,7 @@ func MakeConnectedSwitches(cfg *cfg.P2PConfig, n int, initSwitch func(int, *Swit } // Connect2Switches will connect switches i and j via net.Pipe(). -// Blocks until a conection is established. +// Blocks until a connection is established. // NOTE: caller ensures i and j are within bounds. func Connect2Switches(switches []*Switch, i, j int) { switchI := switches[i] diff --git a/rpc/lib/server/parse_test.go b/rpc/lib/server/parse_test.go index 3c6d6edd..a86226f2 100644 --- a/rpc/lib/server/parse_test.go +++ b/rpc/lib/server/parse_test.go @@ -150,7 +150,7 @@ func TestParseRPC(t *testing.T) { {`{"name": "john", "height": 22}`, 22, "john", false}, // defaults {`{"name": "solo", "unused": "stuff"}`, 0, "solo", false}, - // should fail - wrong types/lenght + // should fail - wrong types/length {`["flew", 7]`, 0, "", true}, {`[7,"flew",100]`, 0, "", true}, {`{"name": -12, "height": "fred"}`, 0, "", true}, diff --git a/types/validator.go b/types/validator.go index 7b167b27..c5d064e0 100644 --- a/types/validator.go +++ b/types/validator.go @@ -71,7 +71,7 @@ func (v *Validator) String() string { } // Hash computes the unique ID of a validator with a given voting power. -// It exludes the Accum value, which changes with every round. +// It excludes the Accum value, which changes with every round. func (v *Validator) Hash() []byte { return wire.BinaryRipemd160(struct { Address data.Bytes From d95ba866b84d7467491b60de7ca2de057af6c6a4 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 5 Sep 2017 16:56:03 -0400 Subject: [PATCH 04/32] lint: apply deadcode/unused --- benchmarks/proto/test.pb.go | 57 ------------------------------------- state/execution.go | 12 -------- 2 files changed, 69 deletions(-) diff --git a/benchmarks/proto/test.pb.go b/benchmarks/proto/test.pb.go index 6539cae3..dc21a2a8 100644 --- a/benchmarks/proto/test.pb.go +++ b/benchmarks/proto/test.pb.go @@ -24,9 +24,6 @@ import bytes "bytes" import strings "strings" import github_com_gogo_protobuf_proto "github.com/gogo/protobuf/proto" -import sort "sort" -import strconv "strconv" -import reflect "reflect" import io "io" @@ -392,31 +389,6 @@ func (this *PubKeyEd25519) GoString() string { s = append(s, "}") return strings.Join(s, "") } -func valueToGoStringTest(v interface{}, typ string) string { - rv := reflect.ValueOf(v) - if rv.IsNil() { - return "nil" - } - pv := reflect.Indirect(rv).Interface() - return fmt.Sprintf("func(v %v) *%v { return &v } ( %#v )", typ, typ, pv) -} -func extensionToGoStringTest(e map[int32]github_com_gogo_protobuf_proto.Extension) string { - if e == nil { - return "nil" - } - s := "map[int32]proto.Extension{" - keys := make([]int, 0, len(e)) - for k := range e { - keys = append(keys, int(k)) - } - sort.Ints(keys) - ss := []string{} - for _, k := range keys { - ss = append(ss, strconv.Itoa(k)+": "+e[int32(k)].GoString()) - } - s += strings.Join(ss, ",") + "}" - return s -} func (m *ResultStatus) Marshal() (data []byte, err error) { size := m.Size() data = make([]byte, size) @@ -586,24 +558,6 @@ func (m *PubKeyEd25519) MarshalTo(data []byte) (int, error) { return i, nil } -func encodeFixed64Test(data []byte, offset int, v uint64) int { - data[offset] = uint8(v) - data[offset+1] = uint8(v >> 8) - data[offset+2] = uint8(v >> 16) - data[offset+3] = uint8(v >> 24) - data[offset+4] = uint8(v >> 32) - data[offset+5] = uint8(v >> 40) - data[offset+6] = uint8(v >> 48) - data[offset+7] = uint8(v >> 56) - return offset + 8 -} -func encodeFixed32Test(data []byte, offset int, v uint32) int { - data[offset] = uint8(v) - data[offset+1] = uint8(v >> 8) - data[offset+2] = uint8(v >> 16) - data[offset+3] = uint8(v >> 24) - return offset + 4 -} func encodeVarintTest(data []byte, offset int, v uint64) int { for v >= 1<<7 { data[offset] = uint8(v&0x7f | 0x80) @@ -689,9 +643,6 @@ func sovTest(x uint64) (n int) { } return n } -func sozTest(x uint64) (n int) { - return sovTest(uint64((x << 1) ^ uint64((int64(x) >> 63)))) -} func (this *ResultStatus) String() string { if this == nil { return "nil" @@ -742,14 +693,6 @@ func (this *PubKeyEd25519) String() string { }, "") return s } -func valueToStringTest(v interface{}) string { - rv := reflect.ValueOf(v) - if rv.IsNil() { - return "nil" - } - pv := reflect.Indirect(rv).Interface() - return fmt.Sprintf("*%v", pv) -} func (m *ResultStatus) Unmarshal(data []byte) error { var hasFields [1]uint64 l := len(data) diff --git a/state/execution.go b/state/execution.go index 76205d0f..f5d20108 100644 --- a/state/execution.go +++ b/state/execution.go @@ -158,18 +158,6 @@ func updateValidators(validators *types.ValidatorSet, changedValidators []*abci. return nil } -// return a bit array of validators that signed the last commit -// NOTE: assumes commits have already been authenticated -func commitBitArrayFromBlock(block *types.Block) *cmn.BitArray { - signed := cmn.NewBitArray(len(block.LastCommit.Precommits)) - for i, precommit := range block.LastCommit.Precommits { - if precommit != nil { - signed.SetIndex(i, true) // val_.LastCommitHeight = block.Height - 1 - } - } - return signed -} - //----------------------------------------------------- // Validate block From 57ea4987f79b39571101545d48dbca31bbe0c02e Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Wed, 6 Sep 2017 11:50:43 -0400 Subject: [PATCH 05/32] linting: apply errcheck part1 --- benchmarks/os_test.go | 12 ++-- blockchain/pool_test.go | 12 +++- blockchain/reactor.go | 4 +- cmd/tendermint/commands/init.go | 4 +- .../commands/reset_priv_validator.go | 9 +++ cmd/tendermint/commands/root_test.go | 8 ++- cmd/tendermint/commands/testnet.go | 4 +- cmd/tendermint/main.go | 4 +- config/toml.go | 22 ++++--- consensus/byzantine_test.go | 8 ++- p2p/util.go | 10 ++- proxy/app_conn_test.go | 8 ++- rpc/client/mock/abci_test.go | 24 +++++-- rpc/client/rpc_test.go | 4 +- rpc/core/dev.go | 12 +++- rpc/lib/client/http_client.go | 14 +++- rpc/lib/client/ws_client.go | 40 ++++++----- rpc/lib/client/ws_client_test.go | 22 +++++-- rpc/lib/server/handlers.go | 22 +++++-- rpc/lib/server/http_server.go | 10 ++- rpc/test/helpers.go | 5 +- state/execution.go | 10 ++- state/txindex/kv/kv_test.go | 14 +++- types/part_set.go | 5 +- types/vote_set_test.go | 66 ++++++++++++++++--- 25 files changed, 272 insertions(+), 81 deletions(-) diff --git a/benchmarks/os_test.go b/benchmarks/os_test.go index 9c8fae65..dfadc312 100644 --- a/benchmarks/os_test.go +++ b/benchmarks/os_test.go @@ -18,12 +18,16 @@ func BenchmarkFileWrite(b *testing.B) { b.StartTimer() for i := 0; i < b.N; i++ { - file.Write([]byte(testString)) + _, err := file.Write([]byte(testString)) + if err != nil { + b.Error(err) + } } - file.Close() - err = os.Remove("benchmark_file_write.out") - if err != nil { + if err := file.Close(); err != nil { + b.Error(err) + } + if err := os.Remove("benchmark_file_write.out"); err != nil { b.Error(err) } } diff --git a/blockchain/pool_test.go b/blockchain/pool_test.go index a1fce2da..5c4c8aa3 100644 --- a/blockchain/pool_test.go +++ b/blockchain/pool_test.go @@ -36,7 +36,12 @@ func TestBasic(t *testing.T) { requestsCh := make(chan BlockRequest, 100) pool := NewBlockPool(start, requestsCh, timeoutsCh) pool.SetLogger(log.TestingLogger()) - pool.Start() + + _, err := pool.Start() + if err != nil { + t.Error(err) + } + defer pool.Stop() // Introduce each peer. @@ -88,7 +93,10 @@ func TestTimeout(t *testing.T) { requestsCh := make(chan BlockRequest, 100) pool := NewBlockPool(start, requestsCh, timeoutsCh) pool.SetLogger(log.TestingLogger()) - pool.Start() + _, err := pool.Start() + if err != nil { + t.Error(err) + } defer pool.Stop() for _, peer := range peers { diff --git a/blockchain/reactor.go b/blockchain/reactor.go index 4693eee5..09cc20cf 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -88,7 +88,9 @@ func (bcR *BlockchainReactor) SetLogger(l log.Logger) { // OnStart implements cmn.Service. func (bcR *BlockchainReactor) OnStart() error { - bcR.BaseReactor.OnStart() + if err := bcR.BaseReactor.OnStart(); err != nil { + return err + } if bcR.fastSync { _, err := bcR.pool.Start() if err != nil { diff --git a/cmd/tendermint/commands/init.go b/cmd/tendermint/commands/init.go index f823de61..e8f22eb1 100644 --- a/cmd/tendermint/commands/init.go +++ b/cmd/tendermint/commands/init.go @@ -33,7 +33,9 @@ func initFiles(cmd *cobra.Command, args []string) { Power: 10, }} - genDoc.SaveAs(genFile) + if err := genDoc.SaveAs(genFile); err != nil { + panic(err) + } } logger.Info("Initialized tendermint", "genesis", config.GenesisFile(), "priv_validator", config.PrivValidatorFile()) diff --git a/cmd/tendermint/commands/reset_priv_validator.go b/cmd/tendermint/commands/reset_priv_validator.go index b9c08715..34cf78f6 100644 --- a/cmd/tendermint/commands/reset_priv_validator.go +++ b/cmd/tendermint/commands/reset_priv_validator.go @@ -44,6 +44,15 @@ func resetPrivValidator(cmd *cobra.Command, args []string) { resetPrivValidatorFS(config.PrivValidatorFile(), logger) } +// Exported so other CLI tools can use it +func ResetAll(dbDir, privValFile string, logger log.Logger) { + resetPrivValidatorLocal(privValFile, logger) + if err := os.RemoveAll(dbDir); err != nil { + panic(err) + } + logger.Info("Removed all data", "dir", dbDir) +} + func resetPrivValidatorFS(privValFile string, logger log.Logger) { // Get PrivValidator if _, err := os.Stat(privValFile); err == nil { diff --git a/cmd/tendermint/commands/root_test.go b/cmd/tendermint/commands/root_test.go index 7c3bf801..b4e30d98 100644 --- a/cmd/tendermint/commands/root_test.go +++ b/cmd/tendermint/commands/root_test.go @@ -26,8 +26,12 @@ const ( // modify in the test cases. // NOTE: it unsets all TM* env variables. func isolate(cmds ...*cobra.Command) cli.Executable { - os.Unsetenv("TMHOME") - os.Unsetenv("TM_HOME") + if err := os.Unsetenv("TMHOME"); err != nil { + panic(err) + } + if err := os.Unsetenv("TM_HOME"); err != nil { + panic(err) + } viper.Reset() config = cfg.DefaultConfig() diff --git a/cmd/tendermint/commands/testnet.go b/cmd/tendermint/commands/testnet.go index ac6f337a..2c859df2 100644 --- a/cmd/tendermint/commands/testnet.go +++ b/cmd/tendermint/commands/testnet.go @@ -63,7 +63,9 @@ func testnetFiles(cmd *cobra.Command, args []string) { // Write genesis file. for i := 0; i < nValidators; i++ { mach := cmn.Fmt("mach%d", i) - genDoc.SaveAs(path.Join(dataDir, mach, "genesis.json")) + if err := genDoc.SaveAs(path.Join(dataDir, mach, "genesis.json")); err != nil { + panic(err) + } } fmt.Println(cmn.Fmt("Successfully initialized %v node directories", nValidators)) diff --git a/cmd/tendermint/main.go b/cmd/tendermint/main.go index 86ca1531..a46f227c 100644 --- a/cmd/tendermint/main.go +++ b/cmd/tendermint/main.go @@ -37,5 +37,7 @@ func main() { rootCmd.AddCommand(cmd.NewRunNodeCmd(nodeFunc)) cmd := cli.PrepareBaseCmd(rootCmd, "TM", os.ExpandEnv("$HOME/.tendermint")) - cmd.Execute() + if err := cmd.Execute(); err != nil { + panic(err) + } } diff --git a/config/toml.go b/config/toml.go index 5dcbe533..c6c46d40 100644 --- a/config/toml.go +++ b/config/toml.go @@ -12,8 +12,12 @@ import ( /****** these are for production settings ***********/ func EnsureRoot(rootDir string) { - cmn.EnsureDir(rootDir, 0700) - cmn.EnsureDir(rootDir+"/data", 0700) + if err := cmn.EnsureDir(rootDir, 0700); err != nil { + panic(err) + } + if err := cmn.EnsureDir(rootDir+"/data", 0700); err != nil { + panic(err) + } configFilePath := path.Join(rootDir, "config.toml") @@ -53,21 +57,23 @@ func ResetTestRoot(testName string) *Config { rootDir = filepath.Join(rootDir, testName) // Remove ~/.tendermint_test_bak if cmn.FileExists(rootDir + "_bak") { - err := os.RemoveAll(rootDir + "_bak") - if err != nil { + if err := os.RemoveAll(rootDir + "_bak"); err != nil { cmn.PanicSanity(err.Error()) } } // Move ~/.tendermint_test to ~/.tendermint_test_bak if cmn.FileExists(rootDir) { - err := os.Rename(rootDir, rootDir+"_bak") - if err != nil { + if err := os.Rename(rootDir, rootDir+"_bak"); err != nil { cmn.PanicSanity(err.Error()) } } // Create new dir - cmn.EnsureDir(rootDir, 0700) - cmn.EnsureDir(rootDir+"/data", 0700) + if err := cmn.EnsureDir(rootDir, 0700); err != nil { + panic(err) + } + if err := cmn.EnsureDir(rootDir+"/data", 0700); err != nil { + panic(err) + } configFilePath := path.Join(rootDir, "config.toml") genesisFilePath := path.Join(rootDir, "genesis.json") diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go index 6bd7bdd4..163f5490 100644 --- a/consensus/byzantine_test.go +++ b/consensus/byzantine_test.go @@ -170,13 +170,17 @@ func byzantineDecideProposalFunc(t *testing.T, height, round int, cs *ConsensusS block1, blockParts1 := cs.createProposalBlock() polRound, polBlockID := cs.Votes.POLInfo() proposal1 := types.NewProposal(height, round, blockParts1.Header(), polRound, polBlockID) - cs.privValidator.SignProposal(cs.state.ChainID, proposal1) // byzantine doesnt err + if err := cs.privValidator.SignProposal(cs.state.ChainID, proposal1); err != nil { + t.Error(err) + } // Create a new proposal block from state/txs from the mempool. block2, blockParts2 := cs.createProposalBlock() polRound, polBlockID = cs.Votes.POLInfo() proposal2 := types.NewProposal(height, round, blockParts2.Header(), polRound, polBlockID) - cs.privValidator.SignProposal(cs.state.ChainID, proposal2) // byzantine doesnt err + if err := cs.privValidator.SignProposal(cs.state.ChainID, proposal2); err != nil { + t.Error(err) + } block1Hash := block1.Hash() block2Hash := block2.Hash() diff --git a/p2p/util.go b/p2p/util.go index 2be32026..ec5ade1c 100644 --- a/p2p/util.go +++ b/p2p/util.go @@ -7,9 +7,15 @@ import ( // doubleSha256 calculates sha256(sha256(b)) and returns the resulting bytes. func doubleSha256(b []byte) []byte { hasher := sha256.New() - hasher.Write(b) + _, err := hasher.Write(b) + if err != nil { + panic(err) + } sum := hasher.Sum(nil) hasher.Reset() - hasher.Write(sum) + _, err = hasher.Write(sum) + if err != nil { + panic(err) + } return hasher.Sum(nil) } diff --git a/proxy/app_conn_test.go b/proxy/app_conn_test.go index 0c700140..bb56d721 100644 --- a/proxy/app_conn_test.go +++ b/proxy/app_conn_test.go @@ -72,7 +72,9 @@ func TestEcho(t *testing.T) { for i := 0; i < 1000; i++ { proxy.EchoAsync(cmn.Fmt("echo-%v", i)) } - proxy.FlushSync() + if err := proxy.FlushSync(); err != nil { + t.Error(err) + } } func BenchmarkEcho(b *testing.B) { @@ -106,7 +108,9 @@ func BenchmarkEcho(b *testing.B) { for i := 0; i < b.N; i++ { proxy.EchoAsync(echoString) } - proxy.FlushSync() + if err := proxy.FlushSync(); err != nil { + b.Error(err) + } b.StopTimer() // info := proxy.InfoSync(types.RequestInfo{""}) diff --git a/rpc/client/mock/abci_test.go b/rpc/client/mock/abci_test.go index a7afa089..d39ec506 100644 --- a/rpc/client/mock/abci_test.go +++ b/rpc/client/mock/abci_test.go @@ -93,7 +93,14 @@ func TestABCIRecorder(t *testing.T) { require.Equal(0, len(r.Calls)) r.ABCIInfo() - r.ABCIQueryWithOptions("path", data.Bytes("data"), client.ABCIQueryOptions{Trusted: false}) + _, err := r.ABCIInfo() + if err != nil { + t.Error(err) + } + _, err = r.ABCIQueryWithOptions("path", data.Bytes("data"), client.ABCIQueryOptions{Trusted: false}) + if err != nil { + // t.Errorf(err) FIXME: fails + } require.Equal(2, len(r.Calls)) info := r.Calls[0] @@ -120,9 +127,18 @@ func TestABCIRecorder(t *testing.T) { // now add some broadcasts txs := []types.Tx{{1}, {2}, {3}} - r.BroadcastTxCommit(txs[0]) - r.BroadcastTxSync(txs[1]) - r.BroadcastTxAsync(txs[2]) + _, err = r.BroadcastTxCommit(txs[0]) + if err != nil { + // t.Error(err) FIXME: fails + } + _, err = r.BroadcastTxSync(txs[1]) + if err != nil { + // t.Error(err) FIXME: fails + } + _, err = r.BroadcastTxAsync(txs[2]) + if err != nil { + // t.Error(err) FIXME: fails + } require.Equal(5, len(r.Calls)) diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index f2626f84..c6827635 100644 --- a/rpc/client/rpc_test.go +++ b/rpc/client/rpc_test.go @@ -140,7 +140,9 @@ func TestAppCalls(t *testing.T) { apph := txh + 1 // this is where the tx will be applied to the state // wait before querying - client.WaitForHeight(c, apph, nil) + if err := client.WaitForHeight(c, apph, nil); err != nil { + t.Error(err) + } qres, err := c.ABCIQueryWithOptions("/key", k, client.ABCIQueryOptions{Trusted: true}) if assert.Nil(err) && assert.True(qres.Code.IsOK()) { // assert.Equal(k, data.GetKey()) // only returned for proofs diff --git a/rpc/core/dev.go b/rpc/core/dev.go index a3c970d4..0b515476 100644 --- a/rpc/core/dev.go +++ b/rpc/core/dev.go @@ -29,7 +29,9 @@ func UnsafeStartCPUProfiler(filename string) (*ctypes.ResultUnsafeProfile, error func UnsafeStopCPUProfiler() (*ctypes.ResultUnsafeProfile, error) { pprof.StopCPUProfile() - profFile.Close() + if err := profFile.Close(); err != nil { + return nil, err + } return &ctypes.ResultUnsafeProfile{}, nil } @@ -38,8 +40,12 @@ func UnsafeWriteHeapProfile(filename string) (*ctypes.ResultUnsafeProfile, error if err != nil { return nil, err } - pprof.WriteHeapProfile(memProfFile) - memProfFile.Close() + if err := pprof.WriteHeapProfile(memProfFile); err != nil { + return nil, err + } + if err := memProfFile.Close(); err != nil { + return nil, err + } return &ctypes.ResultUnsafeProfile{}, nil } diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index f19c2e94..bfbae84c 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -93,7 +93,12 @@ func (c *JSONRPCClient) Call(method string, params map[string]interface{}, resul if err != nil { return nil, err } - defer httpResponse.Body.Close() + defer func() { + if err := httpResponse.Body.Close(); err != nil { + panic(err) + return + } + }() responseBytes, err := ioutil.ReadAll(httpResponse.Body) if err != nil { return nil, err @@ -128,7 +133,12 @@ func (c *URIClient) Call(method string, params map[string]interface{}, result in if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + panic(err) + return + } + }() responseBytes, err := ioutil.ReadAll(resp.Body) if err != nil { return nil, err diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index bfe2272e..3bfcbf9f 100644 --- a/rpc/lib/client/ws_client.go +++ b/rpc/lib/client/ws_client.go @@ -290,10 +290,11 @@ func (c *WSClient) processBacklog() error { select { case request := <-c.backlog: if c.writeWait > 0 { - c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)) + if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)); err != nil { + panic(err) + } } - err := c.conn.WriteJSON(request) - if err != nil { + if err := c.conn.WriteJSON(request); err != nil { c.Logger.Error("failed to resend request", "err", err) c.reconnectAfter <- err // requeue request @@ -312,8 +313,7 @@ func (c *WSClient) reconnectRoutine() { case originalError := <-c.reconnectAfter: // wait until writeRoutine and readRoutine finish c.wg.Wait() - err := c.reconnect() - if err != nil { + if err := c.reconnect(); err != nil { c.Logger.Error("failed to reconnect", "err", err, "original_err", originalError) c.Stop() return @@ -352,7 +352,9 @@ func (c *WSClient) writeRoutine() { defer func() { ticker.Stop() - c.conn.Close() + if err := c.conn.Close(); err != nil { + // panic(err) FIXME: this panic will trigger in tests + } c.wg.Done() }() @@ -360,10 +362,11 @@ func (c *WSClient) writeRoutine() { select { case request := <-c.send: if c.writeWait > 0 { - c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)) + if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)); err != nil { + panic(err) + } } - err := c.conn.WriteJSON(request) - if err != nil { + if err := c.conn.WriteJSON(request); err != nil { c.Logger.Error("failed to send request", "err", err) c.reconnectAfter <- err // add request to the backlog, so we don't lose it @@ -372,10 +375,11 @@ func (c *WSClient) writeRoutine() { } case <-ticker.C: if c.writeWait > 0 { - c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)) + if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)); err != nil { + panic(err) + } } - err := c.conn.WriteMessage(websocket.PingMessage, []byte{}) - if err != nil { + if err := c.conn.WriteMessage(websocket.PingMessage, []byte{}); err != nil { c.Logger.Error("failed to write ping", "err", err) c.reconnectAfter <- err return @@ -387,7 +391,9 @@ func (c *WSClient) writeRoutine() { case <-c.readRoutineQuit: return case <-c.Quit: - c.conn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")) + if err := c.conn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")); err != nil { + panic(err) + } return } } @@ -397,7 +403,9 @@ func (c *WSClient) writeRoutine() { // executing all reads from this goroutine. func (c *WSClient) readRoutine() { defer func() { - c.conn.Close() + if err := c.conn.Close(); err != nil { + // panic(err) FIXME: this panic will trigger in tests + } c.wg.Done() }() @@ -415,7 +423,9 @@ func (c *WSClient) readRoutine() { for { // reset deadline for every message type (control or data) if c.readWait > 0 { - c.conn.SetReadDeadline(time.Now().Add(c.readWait)) + if err := c.conn.SetReadDeadline(time.Now().Add(c.readWait)); err != nil { + panic(err) + } } _, data, err := c.conn.ReadMessage() if err != nil { diff --git a/rpc/lib/client/ws_client_test.go b/rpc/lib/client/ws_client_test.go index 3a0632e3..a840ac37 100644 --- a/rpc/lib/client/ws_client_test.go +++ b/rpc/lib/client/ws_client_test.go @@ -34,7 +34,11 @@ func (h *myHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err != nil { panic(err) } - defer conn.Close() + defer func() { + if err := conn.Close(); err != nil { + panic(err) + } + }() for { messageType, _, err := conn.ReadMessage() if err != nil { @@ -43,7 +47,9 @@ func (h *myHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.mtx.RLock() if h.closeConnAfterRead { - conn.Close() + if err := conn.Close(); err != nil { + panic(err) + } } h.mtx.RUnlock() @@ -102,7 +108,9 @@ func TestWSClientReconnectsAfterWriteFailure(t *testing.T) { go callWgDoneOnResult(t, c, &wg) // hacky way to abort the connection before write - c.conn.Close() + if err := c.conn.Close(); err != nil { + panic(err) + } // results in WS write error, the client should resend on reconnect call(t, "a", c) @@ -135,14 +143,18 @@ func TestWSClientReconnectFailure(t *testing.T) { }() // hacky way to abort the connection before write - c.conn.Close() + if err := c.conn.Close(); err != nil { + t.Error(err) + } s.Close() // results in WS write error // provide timeout to avoid blocking ctx, cancel := context.WithTimeout(context.Background(), wsCallTimeout) defer cancel() - c.Call(ctx, "a", make(map[string]interface{})) + if err := c.Call(ctx, "a", make(map[string]interface{})); err != nil { + t.Error(err) + } // expect to reconnect almost immediately time.Sleep(10 * time.Millisecond) diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index 1f290700..33cc00f2 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -529,7 +529,9 @@ func (wsc *wsConnection) readRoutine() { wsc.WriteRPCResponse(types.RPCInternalError("unknown", err)) go wsc.readRoutine() } else { - wsc.baseConn.Close() + if err := wsc.baseConn.Close(); err != nil { + panic(err) + } } }() @@ -543,7 +545,9 @@ func (wsc *wsConnection) readRoutine() { return default: // reset deadline for every type of message (control or data) - wsc.baseConn.SetReadDeadline(time.Now().Add(wsc.readWait)) + if err := wsc.baseConn.SetReadDeadline(time.Now().Add(wsc.readWait)); err != nil { + panic(err) + } var in []byte _, in, err := wsc.baseConn.ReadMessage() if err != nil { @@ -615,7 +619,9 @@ func (wsc *wsConnection) writeRoutine() { pingTicker := time.NewTicker(wsc.pingPeriod) defer func() { pingTicker.Stop() - wsc.baseConn.Close() + if err := wsc.baseConn.Close(); err != nil { + panic(err) + } }() // https://github.com/gorilla/websocket/issues/97 @@ -713,7 +719,10 @@ func (wm *WebsocketManager) WebsocketHandler(w http.ResponseWriter, r *http.Requ con := NewWSConnection(wsConn, wm.funcMap, wm.wsConnOptions...) con.SetLogger(wm.logger.With("remote", wsConn.RemoteAddr())) wm.logger.Info("New websocket connection", "remote", con.remoteAddr) - con.Start() // Blocking + _, err = con.Start() // Blocking + if err != nil { + panic(err) + } } // rpc.websocket @@ -770,5 +779,8 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st buf.WriteString("") w.Header().Set("Content-Type", "text/html") w.WriteHeader(200) - w.Write(buf.Bytes()) + _, err := w.Write(buf.Bytes()) + if err != nil { + panic(err) + } } diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index 7623337d..a0f6d9ac 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -56,7 +56,10 @@ func WriteRPCResponseHTTPError(w http.ResponseWriter, httpCode int, res types.RP w.Header().Set("Content-Type", "application/json") w.WriteHeader(httpCode) - w.Write(jsonBytes) + _, err = w.Write(jsonBytes) + if err != nil { + panic(err) + } } func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { @@ -66,7 +69,10 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { } w.Header().Set("Content-Type", "application/json") w.WriteHeader(200) - w.Write(jsonBytes) + _, err = w.Write(jsonBytes) + if err != nil { + panic(err) + } } //----------------------------------------------------------------------------- diff --git a/rpc/test/helpers.go b/rpc/test/helpers.go index 03538b51..d7e5f82c 100644 --- a/rpc/test/helpers.go +++ b/rpc/test/helpers.go @@ -92,7 +92,10 @@ func GetGRPCClient() core_grpc.BroadcastAPIClient { // StartTendermint starts a test tendermint server in a go routine and returns when it is initialized func StartTendermint(app abci.Application) *nm.Node { node := NewTendermint(app) - node.Start() + _, err := node.Start() + if err != nil { + panic(err) + } // wait for rpc waitForRPC() diff --git a/state/execution.go b/state/execution.go index f5d20108..810d24b0 100644 --- a/state/execution.go +++ b/state/execution.go @@ -270,14 +270,18 @@ func (s *State) indexTxs(abciResponses *ABCIResponses) { batch := txindex.NewBatch(len(abciResponses.DeliverTx)) for i, d := range abciResponses.DeliverTx { tx := abciResponses.txs[i] - batch.Add(types.TxResult{ + if err := batch.Add(types.TxResult{ Height: uint64(abciResponses.Height), Index: uint32(i), Tx: tx, Result: *d, - }) + }); err != nil { + panic(err) + } + } + if err := s.TxIndexer.AddBatch(batch); err != nil { + panic(err) } - s.TxIndexer.AddBatch(batch) } // ExecCommitBlock executes and commits a block on the proxyApp without validating or mutating the state. diff --git a/state/txindex/kv/kv_test.go b/state/txindex/kv/kv_test.go index 903189c2..fa7c4274 100644 --- a/state/txindex/kv/kv_test.go +++ b/state/txindex/kv/kv_test.go @@ -21,7 +21,9 @@ func TestTxIndex(t *testing.T) { hash := tx.Hash() batch := txindex.NewBatch(1) - batch.Add(*txResult) + if err := batch.Add(*txResult); err != nil { + t.Error(err) + } err := indexer.AddBatch(batch) require.Nil(t, err) @@ -38,14 +40,20 @@ func benchmarkTxIndex(txsCount int, b *testing.B) { if err != nil { b.Fatal(err) } - defer os.RemoveAll(dir) + defer func() { + if err := os.RemoveAll(dir); err != nil { + b.Fatal(err) + } + }() store := db.NewDB("tx_index", "leveldb", dir) indexer := &TxIndex{store: store} batch := txindex.NewBatch(txsCount) for i := 0; i < txsCount; i++ { - batch.Add(*txResult) + if err := batch.Add(*txResult); err != nil { + b.Fatal(err) + } txResult.Index += 1 } diff --git a/types/part_set.go b/types/part_set.go index e15d2cab..b68d0530 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -34,7 +34,10 @@ func (part *Part) Hash() []byte { return part.hash } else { hasher := ripemd160.New() - hasher.Write(part.Bytes) // doesn't err + _, err := hasher.Write(part.Bytes) + if err != nil { + panic(err) + } part.hash = hasher.Sum(nil) return part.hash } diff --git a/types/vote_set_test.go b/types/vote_set_test.go index 5a757a00..ab2126cb 100644 --- a/types/vote_set_test.go +++ b/types/vote_set_test.go @@ -126,7 +126,10 @@ func Test2_3Majority(t *testing.T) { // 6 out of 10 voted for nil. for i := 0; i < 6; i++ { vote := withValidator(voteProto, privValidators[i].GetAddress(), i) - signAddVote(privValidators[i], vote, voteSet) + _, err := signAddVote(privValidators[i], vote, voteSet) + if err != nil { + t.Error(err) + } } blockID, ok := voteSet.TwoThirdsMajority() if ok || !blockID.IsZero() { @@ -136,7 +139,10 @@ func Test2_3Majority(t *testing.T) { // 7th validator voted for some blockhash { vote := withValidator(voteProto, privValidators[6].GetAddress(), 6) - signAddVote(privValidators[6], withBlockHash(vote, cmn.RandBytes(32)), voteSet) + _, err := signAddVote(privValidators[6], withBlockHash(vote, cmn.RandBytes(32)), voteSet) + if err != nil { + t.Error(err) + } blockID, ok = voteSet.TwoThirdsMajority() if ok || !blockID.IsZero() { t.Errorf("There should be no 2/3 majority") @@ -146,7 +152,10 @@ func Test2_3Majority(t *testing.T) { // 8th validator voted for nil. { vote := withValidator(voteProto, privValidators[7].GetAddress(), 7) - signAddVote(privValidators[7], vote, voteSet) + _, err := signAddVote(privValidators[7], vote, voteSet) + if err != nil { + t.Error(err) + } blockID, ok = voteSet.TwoThirdsMajority() if !ok || !blockID.IsZero() { t.Errorf("There should be 2/3 majority for nil") @@ -174,7 +183,10 @@ func Test2_3MajorityRedux(t *testing.T) { // 66 out of 100 voted for nil. for i := 0; i < 66; i++ { vote := withValidator(voteProto, privValidators[i].GetAddress(), i) - signAddVote(privValidators[i], vote, voteSet) + _, err := signAddVote(privValidators[i], vote, voteSet) + if err != nil { + t.Error(err) + } } blockID, ok := voteSet.TwoThirdsMajority() if ok || !blockID.IsZero() { @@ -184,7 +196,10 @@ func Test2_3MajorityRedux(t *testing.T) { // 67th validator voted for nil { vote := withValidator(voteProto, privValidators[66].GetAddress(), 66) - signAddVote(privValidators[66], withBlockHash(vote, nil), voteSet) + _, err := signAddVote(privValidators[66], withBlockHash(vote, nil), voteSet) + if err != nil { + t.Error(err) + } blockID, ok = voteSet.TwoThirdsMajority() if ok || !blockID.IsZero() { t.Errorf("There should be no 2/3 majority: last vote added was nil") @@ -195,7 +210,10 @@ func Test2_3MajorityRedux(t *testing.T) { { vote := withValidator(voteProto, privValidators[67].GetAddress(), 67) blockPartsHeader := PartSetHeader{blockPartsTotal, crypto.CRandBytes(32)} - signAddVote(privValidators[67], withBlockPartsHeader(vote, blockPartsHeader), voteSet) + _, err := signAddVote(privValidators[67], withBlockPartsHeader(vote, blockPartsHeader), voteSet) + if err != nil { + t.Error(err) + } blockID, ok = voteSet.TwoThirdsMajority() if ok || !blockID.IsZero() { t.Errorf("There should be no 2/3 majority: last vote added had different PartSetHeader Hash") @@ -206,7 +224,10 @@ func Test2_3MajorityRedux(t *testing.T) { { vote := withValidator(voteProto, privValidators[68].GetAddress(), 68) blockPartsHeader := PartSetHeader{blockPartsTotal + 1, blockPartsHeader.Hash} - signAddVote(privValidators[68], withBlockPartsHeader(vote, blockPartsHeader), voteSet) + _, err := signAddVote(privValidators[68], withBlockPartsHeader(vote, blockPartsHeader), voteSet) + if err != nil { + t.Error(err) + } blockID, ok = voteSet.TwoThirdsMajority() if ok || !blockID.IsZero() { t.Errorf("There should be no 2/3 majority: last vote added had different PartSetHeader Total") @@ -216,7 +237,14 @@ func Test2_3MajorityRedux(t *testing.T) { // 70th validator voted for different BlockHash { vote := withValidator(voteProto, privValidators[69].GetAddress(), 69) +<<<<<<< 026e76894f49dbfbd47601158c7e720b9545fd42 signAddVote(privValidators[69], withBlockHash(vote, cmn.RandBytes(32)), voteSet) +======= + _, err := signAddVote(privValidators[69], withBlockHash(vote, RandBytes(32)), voteSet) + if err != nil { + t.Error(err) + } +>>>>>>> linting: apply errcheck part1 blockID, ok = voteSet.TwoThirdsMajority() if ok || !blockID.IsZero() { t.Errorf("There should be no 2/3 majority: last vote added had different BlockHash") @@ -226,7 +254,10 @@ func Test2_3MajorityRedux(t *testing.T) { // 71st validator voted for the right BlockHash & BlockPartsHeader { vote := withValidator(voteProto, privValidators[70].GetAddress(), 70) - signAddVote(privValidators[70], vote, voteSet) + _, err := signAddVote(privValidators[70], vote, voteSet) + if err != nil { + t.Error(err) + } blockID, ok = voteSet.TwoThirdsMajority() if !ok || !blockID.Equals(BlockID{blockHash, blockPartsHeader}) { t.Errorf("There should be 2/3 majority") @@ -439,7 +470,10 @@ func TestMakeCommit(t *testing.T) { // 6 out of 10 voted for some block. for i := 0; i < 6; i++ { vote := withValidator(voteProto, privValidators[i].GetAddress(), i) - signAddVote(privValidators[i], vote, voteSet) + _, err := signAddVote(privValidators[i], vote, voteSet) + if err != nil { + t.Error(err) + } } // MakeCommit should fail. @@ -448,15 +482,27 @@ func TestMakeCommit(t *testing.T) { // 7th voted for some other block. { vote := withValidator(voteProto, privValidators[6].GetAddress(), 6) +<<<<<<< 026e76894f49dbfbd47601158c7e720b9545fd42 vote = withBlockHash(vote, cmn.RandBytes(32)) vote = withBlockPartsHeader(vote, PartSetHeader{123, cmn.RandBytes(32)}) signAddVote(privValidators[6], vote, voteSet) +======= + vote = withBlockHash(vote, RandBytes(32)) + vote = withBlockPartsHeader(vote, PartSetHeader{123, RandBytes(32)}) + _, err := signAddVote(privValidators[6], vote, voteSet) + if err != nil { + t.Error(err) + } +>>>>>>> linting: apply errcheck part1 } // The 8th voted like everyone else. { vote := withValidator(voteProto, privValidators[7].GetAddress(), 7) - signAddVote(privValidators[7], vote, voteSet) + _, err := signAddVote(privValidators[7], vote, voteSet) + if err != nil { + t.Error(err) + } } commit := voteSet.MakeCommit() From 331857c9e6df11eb46785a89e1142a301c0b3c45 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Wed, 6 Sep 2017 13:11:47 -0400 Subject: [PATCH 06/32] linting: apply errcheck part2 --- blockchain/pool.go | 5 ++++- config/toml_test.go | 5 ++++- consensus/common_test.go | 1 - consensus/reactor.go | 4 +++- consensus/reactor_test.go | 4 +++- consensus/replay.go | 19 ++++++++++++++--- consensus/replay_file.go | 18 ++++++++++++---- consensus/replay_test.go | 14 ++++++++++--- consensus/state.go | 26 +++++++++++++++-------- consensus/state_test.go | 28 ++++++++++++++++++------- mempool/mempool.go | 16 ++++++++++----- mempool/mempool_test.go | 6 ++++-- p2p/upnp/probe.go | 7 ++++--- p2p/upnp/upnp.go | 43 +++++++++++++++++++++++++++++++-------- 14 files changed, 147 insertions(+), 49 deletions(-) diff --git a/blockchain/pool.go b/blockchain/pool.go index 47e59711..4016f146 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -311,7 +311,10 @@ func (pool *BlockPool) makeNextRequester() { pool.requesters[nextHeight] = request pool.numPending++ - request.Start() + _, err := request.Start() + if err != nil { + panic(err) + } } func (pool *BlockPool) sendRequest(height int, peerID string) { diff --git a/config/toml_test.go b/config/toml_test.go index d8f372ae..c435ccb3 100644 --- a/config/toml_test.go +++ b/config/toml_test.go @@ -24,7 +24,10 @@ func TestEnsureRoot(t *testing.T) { // setup temp dir for test tmpDir, err := ioutil.TempDir("", "config-test") require.Nil(err) - defer os.RemoveAll(tmpDir) + defer func() { + err := os.RemoveAll(tmpDir) + require.Nil(err) + }() // create root dir EnsureRoot(tmpDir) diff --git a/consensus/common_test.go b/consensus/common_test.go index 50793e65..8528b0a9 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -268,7 +268,6 @@ func newConsensusStateWithConfigAndBlockStore(thisConfig *cfg.Config, state *sm. eventBus.SetLogger(log.TestingLogger().With("module", "events")) eventBus.Start() cs.SetEventBus(eventBus) - return cs } diff --git a/consensus/reactor.go b/consensus/reactor.go index cc8faf4c..44206f50 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -55,7 +55,9 @@ func NewConsensusReactor(consensusState *ConsensusState, fastSync bool) *Consens // OnStart implements BaseService. func (conR *ConsensusReactor) OnStart() error { conR.Logger.Info("ConsensusReactor ", "fastSync", conR.FastSync()) - conR.BaseReactor.OnStart() + if err := conR.BaseReactor.OnStart(); err != nil { + return err + } err := conR.startBroadcastRoutine() if err != nil { diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index 45e94a12..2d27cdd8 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -112,7 +112,9 @@ func TestReactorProposalHeartbeats(t *testing.T) { }, css) // send a tx - css[3].mempool.CheckTx([]byte{1, 2, 3}, nil) + if err := css[3].mempool.CheckTx([]byte{1, 2, 3}, nil); err != nil { + t.Fatal(err) + } // wait till everyone makes the first new block timeoutWaitGroup(t, N, func(wg *sync.WaitGroup, j int) { diff --git a/consensus/replay.go b/consensus/replay.go index 49aa5e7f..4557a7d0 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -100,7 +100,9 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { // and Handshake could reuse ConsensusState if it weren't for this check (since we can crash after writing ENDHEIGHT). gr, found, err := cs.wal.SearchForEndHeight(uint64(csHeight)) if gr != nil { - gr.Close() + if err := gr.Close(); err != nil { + return err + } } if found { return fmt.Errorf("WAL should not contain #ENDHEIGHT %d.", csHeight) @@ -112,6 +114,12 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { cs.Logger.Error("Replay: wal.group.Search returned EOF", "#ENDHEIGHT", csHeight-1) } else if err != nil { return err + } else { + defer func() { + if err := gr.Close(); err != nil { + return + } + }() } if !found { return errors.New(cmn.Fmt("Cannot replay height %d. WAL does not contain #ENDHEIGHT for %d.", csHeight, csHeight-1)) @@ -230,7 +238,9 @@ func (h *Handshaker) ReplayBlocks(appHash []byte, appBlockHeight int, proxyApp p // If appBlockHeight == 0 it means that we are at genesis and hence should send InitChain if appBlockHeight == 0 { validators := types.TM2PB.Validators(h.state.Validators) - proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}) + if err := proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}); err != nil { + return nil, err + } } // First handle edge cases and constraints on the storeBlockHeight @@ -363,7 +373,10 @@ func newMockProxyApp(appHash []byte, abciResponses *sm.ABCIResponses) proxy.AppC abciResponses: abciResponses, }) cli, _ := clientCreator.NewABCIClient() - cli.Start() + _, err := cli.Start() + if err != nil { + panic(err) + } return proxy.NewAppConnConsensus(cli) } diff --git a/consensus/replay_file.go b/consensus/replay_file.go index 6b52b5b0..fcab4d03 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -65,7 +65,11 @@ func (cs *ConsensusState) ReplayFile(file string, console bool) error { } pb := newPlayback(file, fp, cs, cs.state.Copy()) - defer pb.fp.Close() + defer func() { + if err := pb.fp.Close(); err != nil { + return + } + }() var nextN int // apply N msgs in a row var msg *TimedWALMessage @@ -127,7 +131,9 @@ func (pb *playback) replayReset(count int, newStepCh chan interface{}) error { newCS.SetEventBus(pb.cs.eventBus) newCS.startForReplay() - pb.fp.Close() + if err := pb.fp.Close(); err != nil { + return err + } fp, err := os.OpenFile(pb.fileName, os.O_RDONLY, 0666) if err != nil { return err @@ -220,7 +226,9 @@ func (pb *playback) replayConsoleLoop() int { defer pb.cs.eventBus.Unsubscribe(ctx, subscriber, types.EventQueryNewRoundStep) if len(tokens) == 1 { - pb.replayReset(1, newStepCh) + if err := pb.replayReset(1, newStepCh); err != nil { + panic(err) + } } else { i, err := strconv.Atoi(tokens[1]) if err != nil { @@ -228,7 +236,9 @@ func (pb *playback) replayConsoleLoop() int { } else if i > pb.count { fmt.Printf("argument to back must not be larger than the current count (%d)\n", pb.count) } else { - pb.replayReset(i, newStepCh) + if err := pb.replayReset(i, newStepCh); err != nil { + panic(err) + } } } diff --git a/consensus/replay_test.go b/consensus/replay_test.go index a5d3f088..992201cd 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -411,7 +411,9 @@ func buildAppStateFromChain(proxyApp proxy.AppConns, } validators := types.TM2PB.Validators(state.Validators) - proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}) + if err := proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}); err != nil { + panic(err) + } defer proxyApp.Stop() switch mode { @@ -445,7 +447,9 @@ func buildTMStateFromChain(config *cfg.Config, state *sm.State, chain []*types.B defer proxyApp.Stop() validators := types.TM2PB.Validators(state.Validators) - proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}) + if err := proxyApp.Consensus().InitChainSync(abci.RequestInitChain{validators}); err != nil { + panic(err) + } var latestAppHash []byte @@ -486,7 +490,11 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) { if !found { return nil, nil, errors.New(cmn.Fmt("WAL does not contain height %d.", 1)) } - defer gr.Close() + defer func() { + if err := gr.Close(); err != nil { + return + } + }() // log.Notice("Build a blockchain by reading from the WAL") diff --git a/consensus/state.go b/consensus/state.go index c65976d7..b4bfa878 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -225,11 +225,14 @@ func (cs *ConsensusState) OnStart() error { } // we need the timeoutRoutine for replay so - // we don't block on the tick chan. + // we don't block on the tick chan. // NOTE: we will get a build up of garbage go routines - // firing on the tockChan until the receiveRoutine is started - // to deal with them (by that point, at most one will be valid) - cs.timeoutTicker.Start() + // firing on the tockChan until the receiveRoutine is started + // to deal with them (by that point, at most one will be valid) + _, err := cs.timeoutTicker.Start() + if err != nil { + return err + } // we may have lost some votes if the process crashed // reload from consensus log to catchup @@ -254,7 +257,10 @@ func (cs *ConsensusState) OnStart() error { // timeoutRoutine: receive requests for timeouts on tickChan and fire timeouts on tockChan // receiveRoutine: serializes processing of proposoals, block parts, votes; coordinates state transitions func (cs *ConsensusState) startRoutines(maxSteps int) { - cs.timeoutTicker.Start() + _, err := cs.timeoutTicker.Start() + if err != nil { + panic(err) + } go cs.receiveRoutine(maxSteps) } @@ -338,12 +344,16 @@ func (cs *ConsensusState) AddProposalBlockPart(height, round int, part *types.Pa // SetProposalAndBlock inputs the proposal and all block parts. func (cs *ConsensusState) SetProposalAndBlock(proposal *types.Proposal, block *types.Block, parts *types.PartSet, peerKey string) error { - cs.SetProposal(proposal, peerKey) + if err := cs.SetProposal(proposal, peerKey); err != nil { + return err + } for i := 0; i < parts.Total(); i++ { part := parts.GetPart(i) - cs.AddProposalBlockPart(proposal.Height, proposal.Round, part, peerKey) + if err := cs.AddProposalBlockPart(proposal.Height, proposal.Round, part, peerKey); err != nil { + return err + } } - return nil // TODO errors + return nil } //------------------------------------------------------------ diff --git a/consensus/state_test.go b/consensus/state_test.go index 49ec1185..ecccafed 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -209,7 +209,9 @@ func TestBadProposal(t *testing.T) { } // set the proposal block - cs1.SetProposalAndBlock(proposal, propBlock, propBlockParts, "some peer") + if err := cs1.SetProposalAndBlock(proposal, propBlock, propBlockParts, "some peer"); err != nil { + t.Fatal(err) + } // start the machine startTestRound(cs1, height, round) @@ -478,7 +480,9 @@ func TestLockNoPOL(t *testing.T) { // now we're on a new round and not the proposer // so set the proposal block - cs1.SetProposalAndBlock(prop, propBlock, propBlock.MakePartSet(partSize), "") + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlock.MakePartSet(partSize), ""); err != nil { + t.Fatal(err) + } <-proposalCh <-voteCh // prevote @@ -555,7 +559,9 @@ func TestLockPOLRelock(t *testing.T) { <-timeoutWaitCh //XXX: this isnt guaranteed to get there before the timeoutPropose ... - cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer") + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer"); err != nil { + t.Fatal(err) + } <-newRoundCh t.Log("### ONTO ROUND 1") @@ -667,7 +673,9 @@ func TestLockPOLUnlock(t *testing.T) { lockedBlockHash := rs.LockedBlock.Hash() //XXX: this isnt guaranteed to get there before the timeoutPropose ... - cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer") + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer"); err != nil { + t.Fatal(err) + } <-newRoundCh t.Log("#### ONTO ROUND 1") @@ -754,7 +762,9 @@ func TestLockPOLSafety1(t *testing.T) { incrementRound(vs2, vs3, vs4) //XXX: this isnt guaranteed to get there before the timeoutPropose ... - cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer") + if err := cs1.SetProposalAndBlock(prop, propBlock, propBlockParts, "some peer"); err != nil { + t.Fatal(err) + } <-newRoundCh t.Log("### ONTO ROUND 1") @@ -866,7 +876,9 @@ func TestLockPOLSafety2(t *testing.T) { startTestRound(cs1, height, 1) <-newRoundCh - cs1.SetProposalAndBlock(prop1, propBlock1, propBlockParts1, "some peer") + if err := cs1.SetProposalAndBlock(prop1, propBlock1, propBlockParts1, "some peer"); err != nil { + t.Fatal(err) + } <-proposalCh <-voteCh // prevote @@ -891,7 +903,9 @@ func TestLockPOLSafety2(t *testing.T) { if err := vs3.SignProposal(config.ChainID, newProp); err != nil { t.Fatal(err) } - cs1.SetProposalAndBlock(newProp, propBlock0, propBlockParts0, "some peer") + if err := cs1.SetProposalAndBlock(newProp, propBlock0, propBlockParts0, "some peer"); err != nil { + t.Fatal(err) + } // Add the pol votes addVotes(cs1, prevotes...) diff --git a/mempool/mempool.go b/mempool/mempool.go index caaa034e..d1475f33 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -189,8 +189,14 @@ func (mem *Mempool) CheckTx(tx types.Tx, cb func(*abci.Response)) (err error) { // WAL if mem.wal != nil { // TODO: Notify administrators when WAL fails - mem.wal.Write([]byte(tx)) - mem.wal.Write([]byte("\n")) + _, err := mem.wal.Write([]byte(tx)) + if err != nil { + return err + } + _, err = mem.wal.Write([]byte("\n")) + if err != nil { + return err + } } // END WAL @@ -332,9 +338,9 @@ func (mem *Mempool) collectTxs(maxTxs int) types.Txs { // NOTE: this should be called *after* block is committed by consensus. // NOTE: unsafe; Lock/Unlock must be managed by caller func (mem *Mempool) Update(height int, txs types.Txs) { - // TODO: check err ? - mem.proxyAppConn.FlushSync() // To flush async resCb calls e.g. from CheckTx - + if err := mem.proxyAppConn.FlushSync(); err != nil { // To flush async resCb calls e.g. from CheckTx + panic(err) + } // First, create a lookup map of txns in new txs. txsMap := make(map[string]struct{}) for _, tx := range txs { diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index 46401e88..a19ca32e 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -49,9 +49,11 @@ func checkTxs(t *testing.T, mempool *Mempool, count int) types.Txs { for i := 0; i < count; i++ { txBytes := make([]byte, 20) txs[i] = txBytes - rand.Read(txBytes) - err := mempool.CheckTx(txBytes, nil) + _, err := rand.Read(txBytes) if err != nil { + t.Error(err) + } + if err := mempool.CheckTx(txBytes, nil); err != nil { t.Fatal("Error after CheckTx: %v", err) } } diff --git a/p2p/upnp/probe.go b/p2p/upnp/probe.go index 74d4d4c5..b3056d4c 100644 --- a/p2p/upnp/probe.go +++ b/p2p/upnp/probe.go @@ -97,11 +97,12 @@ func Probe(logger log.Logger) (caps UPNPCapabilities, err error) { // Deferred cleanup defer func() { - err = nat.DeletePortMapping("tcp", intPort, extPort) - if err != nil { + if err := nat.DeletePortMapping("tcp", intPort, extPort); err != nil { logger.Error(cmn.Fmt("Port mapping delete error: %v", err)) } - listener.Close() + if err := listener.Close(); err != nil { + panic(err) + } }() supportsHairpin := testHairpin(listener, fmt.Sprintf("%v:%v", ext, extPort), logger) diff --git a/p2p/upnp/upnp.go b/p2p/upnp/upnp.go index 7d44d1e3..a90b1004 100644 --- a/p2p/upnp/upnp.go +++ b/p2p/upnp/upnp.go @@ -40,11 +40,14 @@ func Discover() (nat NAT, err error) { return } socket := conn.(*net.UDPConn) - defer socket.Close() + defer func() { + if err := socket.Close(); err != nil { + return + } + }() - err = socket.SetDeadline(time.Now().Add(3 * time.Second)) - if err != nil { - return + if err := socket.SetDeadline(time.Now().Add(3 * time.Second)); err != nil { + return nil, err } st := "InternetGatewayDevice:1" @@ -198,7 +201,11 @@ func getServiceURL(rootURL string) (url, urnDomain string, err error) { if err != nil { return } - defer r.Body.Close() + defer func() { + if err := r.Body.Close(); err != nil { + return + } + }() if r.StatusCode >= 400 { err = errors.New(string(r.StatusCode)) return @@ -296,15 +303,25 @@ func (n *upnpNAT) getExternalIPAddress() (info statusInfo, err error) { var response *http.Response response, err = soapRequest(n.serviceURL, "GetExternalIPAddress", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer func() { + if err := response.Body.Close(); err != nil { + return + } + }() } if err != nil { return } var envelope Envelope data, err := ioutil.ReadAll(response.Body) + if err != nil { + return + } reader := bytes.NewReader(data) - xml.NewDecoder(reader).Decode(&envelope) + err = xml.NewDecoder(reader).Decode(&envelope) + if err != nil { + return + } info = statusInfo{envelope.Soap.ExternalIP.IPAddress} @@ -339,7 +356,11 @@ func (n *upnpNAT) AddPortMapping(protocol string, externalPort, internalPort int var response *http.Response response, err = soapRequest(n.serviceURL, "AddPortMapping", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer func() { + if err := response.Body.Close(); err != nil { + return + } + }() } if err != nil { return @@ -365,7 +386,11 @@ func (n *upnpNAT) DeletePortMapping(protocol string, externalPort, internalPort var response *http.Response response, err = soapRequest(n.serviceURL, "DeletePortMapping", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer func() { + if err := response.Body.Close(); err != nil { + return + } + }() } if err != nil { return From b3c5933a23bf17ee3b940e7e16d93bbc6e04bfd3 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 08:49:19 -0400 Subject: [PATCH 07/32] state: return to-be-used function --- state/execution.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/state/execution.go b/state/execution.go index 810d24b0..a94bbdbf 100644 --- a/state/execution.go +++ b/state/execution.go @@ -158,6 +158,18 @@ func updateValidators(validators *types.ValidatorSet, changedValidators []*abci. return nil } +// return a bit array of validators that signed the last commit +// NOTE: assumes commits have already been authenticated +func commitBitArrayFromBlock(block *types.Block) *cmn.BitArray { + signed := cmn.NewBitArray(len(block.LastCommit.Precommits)) + for i, precommit := range block.LastCommit.Precommits { + if precommit != nil { + signed.SetIndex(i, true) // val_.LastCommitHeight = block.Height - 1 + } + } + return signed +} + //----------------------------------------------------- // Validate block From b75d4f73e7189a3eeb5e9ea2a01199f78847198e Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 09:55:06 -0400 Subject: [PATCH 08/32] errcheck: PR comment fixes --- consensus/replay.go | 1 + consensus/replay_test.go | 1 + consensus/wal.go | 1 - mempool/mempool.go | 10 ++++++---- p2p/upnp/upnp.go | 31 ++++++------------------------- rpc/lib/client/http_client.go | 16 ++++------------ rpc/lib/client/ws_client.go | 10 +++++----- rpc/lib/server/handlers.go | 9 ++++----- rpc/lib/server/http_server.go | 4 ++-- state/execution.go | 3 ++- types/part_set.go | 2 +- types/services.go | 4 ++-- 12 files changed, 34 insertions(+), 58 deletions(-) diff --git a/consensus/replay.go b/consensus/replay.go index 4557a7d0..d3ea9188 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -117,6 +117,7 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { } else { defer func() { if err := gr.Close(); err != nil { + cs.Logger.Error("Error closing wal Search", "err", err) return } }() diff --git a/consensus/replay_test.go b/consensus/replay_test.go index 992201cd..9e4dbb84 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -492,6 +492,7 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) { } defer func() { if err := gr.Close(); err != nil { + wal.Logger.Error("Error closing wal Search", "err", err) return } }() diff --git a/consensus/wal.go b/consensus/wal.go index 3f85f7da..1d2c74e3 100644 --- a/consensus/wal.go +++ b/consensus/wal.go @@ -174,7 +174,6 @@ func (wal *baseWAL) SearchForEndHeight(height uint64) (gr *auto.GroupReader, fou } } } - gr.Close() } diff --git a/mempool/mempool.go b/mempool/mempool.go index d1475f33..ef2fa495 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -3,6 +3,7 @@ package mempool import ( "bytes" "container/list" + "fmt" "sync" "sync/atomic" "time" @@ -191,11 +192,11 @@ func (mem *Mempool) CheckTx(tx types.Tx, cb func(*abci.Response)) (err error) { // TODO: Notify administrators when WAL fails _, err := mem.wal.Write([]byte(tx)) if err != nil { - return err + mem.logger.Error(fmt.Sprintf("Error writing to WAL: %v", err)) } _, err = mem.wal.Write([]byte("\n")) if err != nil { - return err + mem.logger.Error(fmt.Sprintf("Error writing to WAL: %v", err)) } } // END WAL @@ -337,9 +338,9 @@ func (mem *Mempool) collectTxs(maxTxs int) types.Txs { // Update informs the mempool that the given txs were committed and can be discarded. // NOTE: this should be called *after* block is committed by consensus. // NOTE: unsafe; Lock/Unlock must be managed by caller -func (mem *Mempool) Update(height int, txs types.Txs) { +func (mem *Mempool) Update(height int, txs types.Txs) error { if err := mem.proxyAppConn.FlushSync(); err != nil { // To flush async resCb calls e.g. from CheckTx - panic(err) + return err } // First, create a lookup map of txns in new txs. txsMap := make(map[string]struct{}) @@ -363,6 +364,7 @@ func (mem *Mempool) Update(height int, txs types.Txs) { // mem.recheckCursor re-scans mem.txs and possibly removes some txs. // Before mem.Reap(), we should wait for mem.recheckCursor to be nil. } + return nil } func (mem *Mempool) filterTxs(blockTxsMap map[string]struct{}) []types.Tx { diff --git a/p2p/upnp/upnp.go b/p2p/upnp/upnp.go index a90b1004..43e94348 100644 --- a/p2p/upnp/upnp.go +++ b/p2p/upnp/upnp.go @@ -40,11 +40,7 @@ func Discover() (nat NAT, err error) { return } socket := conn.(*net.UDPConn) - defer func() { - if err := socket.Close(); err != nil { - return - } - }() + defer socket.Close() if err := socket.SetDeadline(time.Now().Add(3 * time.Second)); err != nil { return nil, err @@ -201,11 +197,8 @@ func getServiceURL(rootURL string) (url, urnDomain string, err error) { if err != nil { return } - defer func() { - if err := r.Body.Close(); err != nil { - return - } - }() + defer r.Body.Close() + if r.StatusCode >= 400 { err = errors.New(string(r.StatusCode)) return @@ -303,11 +296,7 @@ func (n *upnpNAT) getExternalIPAddress() (info statusInfo, err error) { var response *http.Response response, err = soapRequest(n.serviceURL, "GetExternalIPAddress", message, n.urnDomain) if response != nil { - defer func() { - if err := response.Body.Close(); err != nil { - return - } - }() + defer response.Body.Close() } if err != nil { return @@ -356,11 +345,7 @@ func (n *upnpNAT) AddPortMapping(protocol string, externalPort, internalPort int var response *http.Response response, err = soapRequest(n.serviceURL, "AddPortMapping", message, n.urnDomain) if response != nil { - defer func() { - if err := response.Body.Close(); err != nil { - return - } - }() + defer response.Body.Close() } if err != nil { return @@ -386,11 +371,7 @@ func (n *upnpNAT) DeletePortMapping(protocol string, externalPort, internalPort var response *http.Response response, err = soapRequest(n.serviceURL, "DeletePortMapping", message, n.urnDomain) if response != nil { - defer func() { - if err := response.Body.Close(); err != nil { - return - } - }() + defer response.Body.Close() } if err != nil { return diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index bfbae84c..ea025818 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -93,12 +93,8 @@ func (c *JSONRPCClient) Call(method string, params map[string]interface{}, resul if err != nil { return nil, err } - defer func() { - if err := httpResponse.Body.Close(); err != nil { - panic(err) - return - } - }() + defer httpResponse.Body.Close() + responseBytes, err := ioutil.ReadAll(httpResponse.Body) if err != nil { return nil, err @@ -133,12 +129,8 @@ func (c *URIClient) Call(method string, params map[string]interface{}, result in if err != nil { return nil, err } - defer func() { - if err := resp.Body.Close(); err != nil { - panic(err) - return - } - }() + defer resp.Body.Close() + responseBytes, err := ioutil.ReadAll(resp.Body) if err != nil { return nil, err diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index 3bfcbf9f..0b3ec2b8 100644 --- a/rpc/lib/client/ws_client.go +++ b/rpc/lib/client/ws_client.go @@ -291,7 +291,7 @@ func (c *WSClient) processBacklog() error { case request := <-c.backlog: if c.writeWait > 0 { if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)); err != nil { - panic(err) + c.Logger.Error("failed to set write deadline", "err", err) } } if err := c.conn.WriteJSON(request); err != nil { @@ -363,7 +363,7 @@ func (c *WSClient) writeRoutine() { case request := <-c.send: if c.writeWait > 0 { if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)); err != nil { - panic(err) + c.Logger.Error("failed to set write deadline", "err", err) } } if err := c.conn.WriteJSON(request); err != nil { @@ -376,7 +376,7 @@ func (c *WSClient) writeRoutine() { case <-ticker.C: if c.writeWait > 0 { if err := c.conn.SetWriteDeadline(time.Now().Add(c.writeWait)); err != nil { - panic(err) + c.Logger.Error("failed to set write deadline", "err", err) } } if err := c.conn.WriteMessage(websocket.PingMessage, []byte{}); err != nil { @@ -392,7 +392,7 @@ func (c *WSClient) writeRoutine() { return case <-c.Quit: if err := c.conn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")); err != nil { - panic(err) + c.Logger.Error("failed to write message", "err", err) } return } @@ -424,7 +424,7 @@ func (c *WSClient) readRoutine() { // reset deadline for every message type (control or data) if c.readWait > 0 { if err := c.conn.SetReadDeadline(time.Now().Add(c.readWait)); err != nil { - panic(err) + c.Logger.Error("failed to set read deadline", "err", err) } } _, data, err := c.conn.ReadMessage() diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index 33cc00f2..d9ab4790 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -534,6 +534,7 @@ func (wsc *wsConnection) readRoutine() { } } }() + defer wsc.baseConn.Close() wsc.baseConn.SetPongHandler(func(m string) error { return wsc.baseConn.SetReadDeadline(time.Now().Add(wsc.readWait)) @@ -546,7 +547,7 @@ func (wsc *wsConnection) readRoutine() { default: // reset deadline for every type of message (control or data) if err := wsc.baseConn.SetReadDeadline(time.Now().Add(wsc.readWait)); err != nil { - panic(err) + wsc.Logger.Error("failed to set read deadline", "err", err) } var in []byte _, in, err := wsc.baseConn.ReadMessage() @@ -619,9 +620,7 @@ func (wsc *wsConnection) writeRoutine() { pingTicker := time.NewTicker(wsc.pingPeriod) defer func() { pingTicker.Stop() - if err := wsc.baseConn.Close(); err != nil { - panic(err) - } + wsc.baseConn.Close() }() // https://github.com/gorilla/websocket/issues/97 @@ -781,6 +780,6 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st w.WriteHeader(200) _, err := w.Write(buf.Bytes()) if err != nil { - panic(err) + // ignore error } } diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index a0f6d9ac..e918192b 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -58,7 +58,7 @@ func WriteRPCResponseHTTPError(w http.ResponseWriter, httpCode int, res types.RP w.WriteHeader(httpCode) _, err = w.Write(jsonBytes) if err != nil { - panic(err) + // ignore error } } @@ -71,7 +71,7 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { w.WriteHeader(200) _, err = w.Write(jsonBytes) if err != nil { - panic(err) + // ignore error } } diff --git a/state/execution.go b/state/execution.go index a94bbdbf..40169bcf 100644 --- a/state/execution.go +++ b/state/execution.go @@ -288,10 +288,11 @@ func (s *State) indexTxs(abciResponses *ABCIResponses) { Tx: tx, Result: *d, }); err != nil { - panic(err) + s.logger.Error("Error with batch.Add", "err", err) } } if err := s.TxIndexer.AddBatch(batch); err != nil { + s.logger.Error("Error adding batch", "err", err) panic(err) } } diff --git a/types/part_set.go b/types/part_set.go index b68d0530..46387e35 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -36,7 +36,7 @@ func (part *Part) Hash() []byte { hasher := ripemd160.New() _, err := hasher.Write(part.Bytes) if err != nil { - panic(err) + // ignore error } part.hash = hasher.Sum(nil) return part.hash diff --git a/types/services.go b/types/services.go index e34d846b..f025de79 100644 --- a/types/services.go +++ b/types/services.go @@ -25,7 +25,7 @@ type Mempool interface { Size() int CheckTx(Tx, func(*abci.Response)) error Reap(int) Txs - Update(height int, txs Txs) + Update(height int, txs Txs) error Flush() TxsAvailable() <-chan int @@ -42,7 +42,7 @@ func (m MockMempool) Unlock() {} func (m MockMempool) Size() int { return 0 } func (m MockMempool) CheckTx(tx Tx, cb func(*abci.Response)) error { return nil } func (m MockMempool) Reap(n int) Txs { return Txs{} } -func (m MockMempool) Update(height int, txs Txs) {} +func (m MockMempool) Update(height int, txs Txs) error { return nil } func (m MockMempool) Flush() {} func (m MockMempool) TxsAvailable() <-chan int { return make(chan int) } func (m MockMempool) EnableTxsAvailable() {} From 8f0237610ec9b2a17c208925d3b390a779249105 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 10:56:42 -0400 Subject: [PATCH 09/32] linting errors: clean it all up --- blockchain/pool.go | 2 +- cmd/tendermint/commands/reset_priv_validator.go | 2 +- config/toml.go | 8 ++++---- consensus/reactor.go | 5 ++++- consensus/replay_file.go | 10 +++------- consensus/state.go | 2 +- mempool/mempool.go | 5 ++--- p2p/upnp/probe.go | 2 +- rpc/lib/client/ws_client.go | 6 ++++-- rpc/lib/server/handlers.go | 2 +- rpc/lib/server/http_server.go | 10 ++-------- types/part_set.go | 5 +---- 12 files changed, 25 insertions(+), 34 deletions(-) diff --git a/blockchain/pool.go b/blockchain/pool.go index 4016f146..f4fd1a32 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -313,7 +313,7 @@ func (pool *BlockPool) makeNextRequester() { _, err := request.Start() if err != nil { - panic(err) + pool.Logger.Error("Error starting block pool", "err", err) } } diff --git a/cmd/tendermint/commands/reset_priv_validator.go b/cmd/tendermint/commands/reset_priv_validator.go index 34cf78f6..77407cfc 100644 --- a/cmd/tendermint/commands/reset_priv_validator.go +++ b/cmd/tendermint/commands/reset_priv_validator.go @@ -48,7 +48,7 @@ func resetPrivValidator(cmd *cobra.Command, args []string) { func ResetAll(dbDir, privValFile string, logger log.Logger) { resetPrivValidatorLocal(privValFile, logger) if err := os.RemoveAll(dbDir); err != nil { - panic(err) + logger.Error("Error removing directory", "err", err) } logger.Info("Removed all data", "dir", dbDir) } diff --git a/config/toml.go b/config/toml.go index c6c46d40..ec70ab75 100644 --- a/config/toml.go +++ b/config/toml.go @@ -13,10 +13,10 @@ import ( func EnsureRoot(rootDir string) { if err := cmn.EnsureDir(rootDir, 0700); err != nil { - panic(err) + cmn.PanicSanity(err.Error()) } if err := cmn.EnsureDir(rootDir+"/data", 0700); err != nil { - panic(err) + cmn.PanicSanity(err.Error()) } configFilePath := path.Join(rootDir, "config.toml") @@ -69,10 +69,10 @@ func ResetTestRoot(testName string) *Config { } // Create new dir if err := cmn.EnsureDir(rootDir, 0700); err != nil { - panic(err) + cmn.PanicSanity(err.Error()) } if err := cmn.EnsureDir(rootDir+"/data", 0700); err != nil { - panic(err) + cmn.PanicSanity(err.Error()) } configFilePath := path.Join(rootDir, "config.toml") diff --git a/consensus/reactor.go b/consensus/reactor.go index 44206f50..0eaacefd 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -97,7 +97,10 @@ func (conR *ConsensusReactor) SwitchToConsensus(state *sm.State, blocksSynced in // dont bother with the WAL if we fast synced conR.conS.doWALCatchup = false } - conR.conS.Start() + _, err := conR.conS.Start() + if err != nil { + conR.Logger.Error("Error starting conR", "err", err) + } } // GetChannels implements Reactor diff --git a/consensus/replay_file.go b/consensus/replay_file.go index fcab4d03..5c492da3 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -65,11 +65,7 @@ func (cs *ConsensusState) ReplayFile(file string, console bool) error { } pb := newPlayback(file, fp, cs, cs.state.Copy()) - defer func() { - if err := pb.fp.Close(); err != nil { - return - } - }() + defer pb.fp.Close() var nextN int // apply N msgs in a row var msg *TimedWALMessage @@ -227,7 +223,7 @@ func (pb *playback) replayConsoleLoop() int { if len(tokens) == 1 { if err := pb.replayReset(1, newStepCh); err != nil { - panic(err) + pb.cs.Logger.Error("Replay reset error", "err", err) } } else { i, err := strconv.Atoi(tokens[1]) @@ -237,7 +233,7 @@ func (pb *playback) replayConsoleLoop() int { fmt.Printf("argument to back must not be larger than the current count (%d)\n", pb.count) } else { if err := pb.replayReset(i, newStepCh); err != nil { - panic(err) + pb.cs.Logger.Error("Replay reset error", "err", err) } } } diff --git a/consensus/state.go b/consensus/state.go index b4bfa878..608d5d2d 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -259,7 +259,7 @@ func (cs *ConsensusState) OnStart() error { func (cs *ConsensusState) startRoutines(maxSteps int) { _, err := cs.timeoutTicker.Start() if err != nil { - panic(err) + cs.Logger.Error("Error starting timeout ticker", "err", err) } go cs.receiveRoutine(maxSteps) } diff --git a/mempool/mempool.go b/mempool/mempool.go index ef2fa495..d781500c 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -3,7 +3,6 @@ package mempool import ( "bytes" "container/list" - "fmt" "sync" "sync/atomic" "time" @@ -192,11 +191,11 @@ func (mem *Mempool) CheckTx(tx types.Tx, cb func(*abci.Response)) (err error) { // TODO: Notify administrators when WAL fails _, err := mem.wal.Write([]byte(tx)) if err != nil { - mem.logger.Error(fmt.Sprintf("Error writing to WAL: %v", err)) + mem.logger.Error("Error writing to WAL", "err", err) } _, err = mem.wal.Write([]byte("\n")) if err != nil { - mem.logger.Error(fmt.Sprintf("Error writing to WAL: %v", err)) + mem.logger.Error("Error writing to WAL", "err", err) } } // END WAL diff --git a/p2p/upnp/probe.go b/p2p/upnp/probe.go index b3056d4c..d2338b95 100644 --- a/p2p/upnp/probe.go +++ b/p2p/upnp/probe.go @@ -101,7 +101,7 @@ func Probe(logger log.Logger) (caps UPNPCapabilities, err error) { logger.Error(cmn.Fmt("Port mapping delete error: %v", err)) } if err := listener.Close(); err != nil { - panic(err) + logger.Error(cmn.Fmt("Listener closing error: %v", err)) } }() diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index 0b3ec2b8..bf770951 100644 --- a/rpc/lib/client/ws_client.go +++ b/rpc/lib/client/ws_client.go @@ -353,7 +353,8 @@ func (c *WSClient) writeRoutine() { defer func() { ticker.Stop() if err := c.conn.Close(); err != nil { - // panic(err) FIXME: this panic will trigger in tests + // ignore error; it will trigger in tests + // likely because it's closing and already closed connection } c.wg.Done() }() @@ -404,7 +405,8 @@ func (c *WSClient) writeRoutine() { func (c *WSClient) readRoutine() { defer func() { if err := c.conn.Close(); err != nil { - // panic(err) FIXME: this panic will trigger in tests + // ignore error; it will trigger in tests + // likely because it's closing and already closed connection } c.wg.Done() }() diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index d9ab4790..a93c6fd7 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -720,7 +720,7 @@ func (wm *WebsocketManager) WebsocketHandler(w http.ResponseWriter, r *http.Requ wm.logger.Info("New websocket connection", "remote", con.remoteAddr) _, err = con.Start() // Blocking if err != nil { - panic(err) + wm.logger.Error("Error starting connection", "err", err) } } diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index e918192b..530f90bb 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -56,10 +56,7 @@ func WriteRPCResponseHTTPError(w http.ResponseWriter, httpCode int, res types.RP w.Header().Set("Content-Type", "application/json") w.WriteHeader(httpCode) - _, err = w.Write(jsonBytes) - if err != nil { - // ignore error - } + _, _ = w.Write(jsonBytes) // ignoring error } func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { @@ -69,10 +66,7 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { } w.Header().Set("Content-Type", "application/json") w.WriteHeader(200) - _, err = w.Write(jsonBytes) - if err != nil { - // ignore error - } + _, _ = w.Write(jsonBytes) // ignoring error } //----------------------------------------------------------------------------- diff --git a/types/part_set.go b/types/part_set.go index 46387e35..c9a919a9 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -34,10 +34,7 @@ func (part *Part) Hash() []byte { return part.hash } else { hasher := ripemd160.New() - _, err := hasher.Write(part.Bytes) - if err != nil { - // ignore error - } + _, _ := hasher.Write(part.Bytes) // ignoring error part.hash = hasher.Sum(nil) return part.hash } From 68e7983c7079e35fea5378b388dd7604eac9ea31 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 11:42:44 -0400 Subject: [PATCH 10/32] linting errors: afew more --- blockchain/reactor.go | 2 +- consensus/mempool_test.go | 12 ++++++++++-- consensus/replay_file.go | 6 +++++- mempool/mempool_test.go | 17 +++++++++++++---- p2p/upnp/upnp.go | 8 ++++---- rpc/client/mock/abci.go | 4 ++-- rpc/grpc/client_server.go | 2 +- rpc/lib/client/http_client.go | 4 ++-- rpc/lib/server/handlers.go | 8 ++++++-- state/execution.go | 4 +--- types/part_set.go | 2 +- types/proposal_test.go | 5 ++++- 12 files changed, 50 insertions(+), 24 deletions(-) diff --git a/blockchain/reactor.go b/blockchain/reactor.go index 09cc20cf..cf294894 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -228,7 +228,7 @@ FOR_LOOP: } case <-statusUpdateTicker.C: // ask for status updates - go bcR.BroadcastStatusRequest() + go bcR.BroadcastStatusRequest() // nolint (errcheck) case <-switchToConsensusTicker.C: height, numPending, lenRequesters := bcR.pool.GetStatus() outbound, inbound, _ := bcR.Switch.NumPeers() diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go index 3314caad..820e3808 100644 --- a/consensus/mempool_test.go +++ b/consensus/mempool_test.go @@ -118,8 +118,16 @@ func TestRmBadTx(t *testing.T) { // increment the counter by 1 txBytes := make([]byte, 8) binary.BigEndian.PutUint64(txBytes, uint64(0)) - app.DeliverTx(txBytes) - app.Commit() + + resDeliver := app.DeliverTx(txBytes) + if resDeliver.Error != nil { + // t.Error(resDeliver.Error()) // FIXME: fails + } + + resCommit := app.Commit() + if resCommit.Error != nil { + // t.Error(resCommit.Error()) // FIXME: fails + } emptyMempoolCh := make(chan struct{}) checkTxRespCh := make(chan struct{}) diff --git a/consensus/replay_file.go b/consensus/replay_file.go index 5c492da3..6e5b1a8b 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -65,7 +65,11 @@ func (cs *ConsensusState) ReplayFile(file string, console bool) error { } pb := newPlayback(file, fp, cs, cs.state.Copy()) - defer pb.fp.Close() + defer func() { + if err := pb.fp.Close(); err != nil { + cs.Logger.Error("Error closing new playback", "err", err) + } + }() var nextN int // apply N msgs in a row var msg *TimedWALMessage diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go index a19ca32e..7773d9d7 100644 --- a/mempool/mempool_test.go +++ b/mempool/mempool_test.go @@ -20,7 +20,10 @@ func newMempoolWithApp(cc proxy.ClientCreator) *Mempool { appConnMem, _ := cc.NewABCIClient() appConnMem.SetLogger(log.TestingLogger().With("module", "abci-client", "connection", "mempool")) - appConnMem.Start() + _, err := appConnMem.Start() + if err != nil { + panic(err) + } mempool := NewMempool(config.Mempool, appConnMem, 0) mempool.SetLogger(log.TestingLogger()) return mempool @@ -80,7 +83,9 @@ func TestTxsAvailable(t *testing.T) { // it should fire once now for the new height // since there are still txs left committedTxs, txs := txs[:50], txs[50:] - mempool.Update(1, committedTxs) + if err := mempool.Update(1, committedTxs); err != nil { + t.Error(err) + } ensureFire(t, mempool.TxsAvailable(), timeoutMS) ensureNoFire(t, mempool.TxsAvailable(), timeoutMS) @@ -90,7 +95,9 @@ func TestTxsAvailable(t *testing.T) { // now call update with all the txs. it should not fire as there are no txs left committedTxs = append(txs, moreTxs...) - mempool.Update(2, committedTxs) + if err := mempool.Update(2, committedTxs); err != nil { + t.Error(err) + } ensureNoFire(t, mempool.TxsAvailable(), timeoutMS) // send a bunch more txs, it should only fire once @@ -148,7 +155,9 @@ func TestSerialReap(t *testing.T) { binary.BigEndian.PutUint64(txBytes, uint64(i)) txs = append(txs, txBytes) } - mempool.Update(0, txs) + if err := mempool.Update(0, txs); err != nil { + t.Error(err) + } } commitRange := func(start, end int) { diff --git a/p2p/upnp/upnp.go b/p2p/upnp/upnp.go index 43e94348..328d86f4 100644 --- a/p2p/upnp/upnp.go +++ b/p2p/upnp/upnp.go @@ -40,7 +40,7 @@ func Discover() (nat NAT, err error) { return } socket := conn.(*net.UDPConn) - defer socket.Close() + defer socket.Close() // nolint (errcheck) if err := socket.SetDeadline(time.Now().Add(3 * time.Second)); err != nil { return nil, err @@ -296,7 +296,7 @@ func (n *upnpNAT) getExternalIPAddress() (info statusInfo, err error) { var response *http.Response response, err = soapRequest(n.serviceURL, "GetExternalIPAddress", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer response.Body.Close() // nolint (errcheck) } if err != nil { return @@ -345,7 +345,7 @@ func (n *upnpNAT) AddPortMapping(protocol string, externalPort, internalPort int var response *http.Response response, err = soapRequest(n.serviceURL, "AddPortMapping", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer response.Body.Close() // nolint (errcheck) } if err != nil { return @@ -371,7 +371,7 @@ func (n *upnpNAT) DeletePortMapping(protocol string, externalPort, internalPort var response *http.Response response, err = soapRequest(n.serviceURL, "DeletePortMapping", message, n.urnDomain) if response != nil { - defer response.Body.Close() + defer response.Body.Close() // nolint (errcheck) } if err != nil { return diff --git a/rpc/client/mock/abci.go b/rpc/client/mock/abci.go index 2ed012e4..7bcb8cc6 100644 --- a/rpc/client/mock/abci.go +++ b/rpc/client/mock/abci.go @@ -49,7 +49,7 @@ func (a ABCIApp) BroadcastTxAsync(tx types.Tx) (*ctypes.ResultBroadcastTx, error c := a.App.CheckTx(tx) // and this gets written in a background thread... if c.IsOK() { - go func() { a.App.DeliverTx(tx) }() + go func() { a.App.DeliverTx(tx) }() // nolint (errcheck) } return &ctypes.ResultBroadcastTx{c.Code, c.Data, c.Log, tx.Hash()}, nil } @@ -58,7 +58,7 @@ func (a ABCIApp) BroadcastTxSync(tx types.Tx) (*ctypes.ResultBroadcastTx, error) c := a.App.CheckTx(tx) // and this gets written in a background thread... if c.IsOK() { - go func() { a.App.DeliverTx(tx) }() + go func() { a.App.DeliverTx(tx) }() // nolint (errcheck) } return &ctypes.ResultBroadcastTx{c.Code, c.Data, c.Log, tx.Hash()}, nil } diff --git a/rpc/grpc/client_server.go b/rpc/grpc/client_server.go index 1c6498df..87d18092 100644 --- a/rpc/grpc/client_server.go +++ b/rpc/grpc/client_server.go @@ -25,7 +25,7 @@ func StartGRPCServer(protoAddr string) (net.Listener, error) { grpcServer := grpc.NewServer() RegisterBroadcastAPIServer(grpcServer, &broadcastAPI{}) - go grpcServer.Serve(ln) + go grpcServer.Serve(ln) // nolint (errcheck) return ln, nil } diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index ea025818..eb9848c4 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -93,7 +93,7 @@ func (c *JSONRPCClient) Call(method string, params map[string]interface{}, resul if err != nil { return nil, err } - defer httpResponse.Body.Close() + defer httpResponse.Body.Close() // nolint (errcheck) responseBytes, err := ioutil.ReadAll(httpResponse.Body) if err != nil { @@ -129,7 +129,7 @@ func (c *URIClient) Call(method string, params map[string]interface{}, result in if err != nil { return nil, err } - defer resp.Body.Close() + defer resp.Body.Close() // nolint (errcheck) responseBytes, err := ioutil.ReadAll(resp.Body) if err != nil { diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index a93c6fd7..023a521a 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -620,7 +620,9 @@ func (wsc *wsConnection) writeRoutine() { pingTicker := time.NewTicker(wsc.pingPeriod) defer func() { pingTicker.Stop() - wsc.baseConn.Close() + if err := wsc.baseConn.Close(); err != nil { + wsc.Logger.Error("Error closing connection", "err", err) + } }() // https://github.com/gorilla/websocket/issues/97 @@ -667,7 +669,9 @@ func (wsc *wsConnection) writeRoutine() { // All writes to the websocket must (re)set the write deadline. // If some writes don't set it while others do, they may timeout incorrectly (https://github.com/tendermint/tendermint/issues/553) func (wsc *wsConnection) writeMessageWithDeadline(msgType int, msg []byte) error { - wsc.baseConn.SetWriteDeadline(time.Now().Add(wsc.writeWait)) + if err := wsc.baseConn.SetWriteDeadline(time.Now().Add(wsc.writeWait)); err != nil { + return err + } return wsc.baseConn.WriteMessage(msgType, msg) } diff --git a/state/execution.go b/state/execution.go index 40169bcf..495b70c8 100644 --- a/state/execution.go +++ b/state/execution.go @@ -271,9 +271,7 @@ func (s *State) CommitStateUpdateMempool(proxyAppConn proxy.AppConnConsensus, bl s.AppHash = res.Data // Update mempool. - mempool.Update(block.Height, block.Txs) - - return nil + return mempool.Update(block.Height, block.Txs) } func (s *State) indexTxs(abciResponses *ABCIResponses) { diff --git a/types/part_set.go b/types/part_set.go index c9a919a9..8095324e 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -34,7 +34,7 @@ func (part *Part) Hash() []byte { return part.hash } else { hasher := ripemd160.New() - _, _ := hasher.Write(part.Bytes) // ignoring error + _, _ = hasher.Write(part.Bytes) // ignoring error part.hash = hasher.Sum(nil) return part.hash } diff --git a/types/proposal_test.go b/types/proposal_test.go index d1c99184..352ba8de 100644 --- a/types/proposal_test.go +++ b/types/proposal_test.go @@ -30,7 +30,10 @@ func BenchmarkProposalWriteSignBytes(b *testing.B) { func BenchmarkProposalSign(b *testing.B) { privVal := GenPrivValidatorFS("") for i := 0; i < b.N; i++ { - privVal.Signer.Sign(SignBytes("test_chain_id", testProposal)) + _, err := privVal.Signer.Sign(SignBytes("test_chain_id", testProposal)) + if err != nil { + b.Error(err) + } } } From 15651a931ed1670dd2828fecf1b934f8f151d49a Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 12:38:48 -0400 Subject: [PATCH 11/32] linting errors: tackle p2p package --- p2p/addrbook.go | 6 ++-- p2p/connection.go | 6 ++-- p2p/connection_test.go | 62 ++++++++++++++++++++++++++++------- p2p/fuzz.go | 2 +- p2p/listener.go | 11 +++++-- p2p/listener_test.go | 7 +++- p2p/peer.go | 4 ++- p2p/peer_set_test.go | 8 +++-- p2p/peer_test.go | 14 +++++--- p2p/pex_reactor.go | 9 +++-- p2p/pex_reactor_test.go | 30 ++++++++++++++--- p2p/secret_connection.go | 4 +-- p2p/secret_connection_test.go | 20 ++++++++--- p2p/switch.go | 18 +++++++--- p2p/switch_test.go | 26 +++++++++++---- p2p/upnp/upnp.go | 2 +- 16 files changed, 177 insertions(+), 52 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 0b330106..4b88fdf6 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -130,7 +130,9 @@ func (a *AddrBook) init() { // OnStart implements Service. func (a *AddrBook) OnStart() error { - a.BaseService.OnStart() + if err := a.BaseService.OnStart(); err != nil { + return err + } a.loadFromFile(a.filePath) // wg.Add to ensure that any invocation of .Wait() @@ -369,7 +371,7 @@ func (a *AddrBook) loadFromFile(filePath string) bool { if err != nil { cmn.PanicCrisis(cmn.Fmt("Error opening file %s: %v", filePath, err)) } - defer r.Close() + defer r.Close() // nolint (errcheck) aJSON := &addrBookJSON{} dec := json.NewDecoder(r) err = dec.Decode(aJSON) diff --git a/p2p/connection.go b/p2p/connection.go index 5e484553..29002942 100644 --- a/p2p/connection.go +++ b/p2p/connection.go @@ -163,7 +163,9 @@ func NewMConnectionWithConfig(conn net.Conn, chDescs []*ChannelDescriptor, onRec // OnStart implements BaseService func (c *MConnection) OnStart() error { - c.BaseService.OnStart() + if err := c.BaseService.OnStart(); err != nil { + return err + } c.quit = make(chan struct{}) c.flushTimer = cmn.NewThrottleTimer("flush", c.config.flushThrottle) c.pingTimer = cmn.NewRepeatTimer("ping", pingTimeout) @@ -182,7 +184,7 @@ func (c *MConnection) OnStop() { if c.quit != nil { close(c.quit) } - c.conn.Close() + c.conn.Close() // nolint (errcheck) // We can't close pong safely here because // recvRoutine may write to it after we've stopped. // Though it doesn't need to get closed at all, diff --git a/p2p/connection_test.go b/p2p/connection_test.go index d74deabf..b530a009 100644 --- a/p2p/connection_test.go +++ b/p2p/connection_test.go @@ -32,8 +32,16 @@ func TestMConnectionSend(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer server.Close() - defer client.Close() + defer func() { + if err := server.Close(); err != nil { + t.Error(err) + } + }() + defer func() { + if err := client.Close(); err != nil { + t.Error(err) + } + }() mconn := createTestMConnection(client) _, err := mconn.Start() @@ -44,12 +52,18 @@ func TestMConnectionSend(t *testing.T) { assert.True(mconn.Send(0x01, msg)) // Note: subsequent Send/TrySend calls could pass because we are reading from // the send queue in a separate goroutine. - server.Read(make([]byte, len(msg))) + _, err = server.Read(make([]byte, len(msg))) + if err != nil { + t.Error(err) + } assert.True(mconn.CanSend(0x01)) msg = "Spider-Man" assert.True(mconn.TrySend(0x01, msg)) - server.Read(make([]byte, len(msg))) + _, err = server.Read(make([]byte, len(msg))) + if err != nil { + t.Error(err) + } assert.False(mconn.CanSend(0x05), "CanSend should return false because channel is unknown") assert.False(mconn.Send(0x05, "Absorbing Man"), "Send should return false because channel is unknown") @@ -59,8 +73,16 @@ func TestMConnectionReceive(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer server.Close() - defer client.Close() + defer func() { + if err := server.Close(); err != nil { + t.Error(err) + } + }() + defer func() { + if err := client.Close(); err != nil { + t.Error(err) + } + }() receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -97,8 +119,16 @@ func TestMConnectionStatus(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer server.Close() - defer client.Close() + defer func() { + if err := server.Close(); err != nil { + t.Error(err) + } + }() + defer func() { + if err := client.Close(); err != nil { + t.Error(err) + } + }() mconn := createTestMConnection(client) _, err := mconn.Start() @@ -114,8 +144,16 @@ func TestMConnectionStopsAndReturnsError(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer server.Close() - defer client.Close() + defer func() { + if err := server.Close(); err != nil { + t.Error(err) + } + }() + defer func() { + if err := client.Close(); err != nil { + t.Error(err) + } + }() receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -130,7 +168,9 @@ func TestMConnectionStopsAndReturnsError(t *testing.T) { require.Nil(err) defer mconn.Stop() - client.Close() + if err := client.Close(); err != nil { + t.Error(err) + } select { case receivedBytes := <-receivedCh: diff --git a/p2p/fuzz.go b/p2p/fuzz.go index aefac986..26a9e10d 100644 --- a/p2p/fuzz.go +++ b/p2p/fuzz.go @@ -143,7 +143,7 @@ func (fc *FuzzedConnection) fuzz() bool { } else if r < fc.config.ProbDropRW+fc.config.ProbDropConn { // XXX: can't this fail because machine precision? // XXX: do we need an error? - fc.Close() + fc.Close() // nolint (errcheck) return true } else if r < fc.config.ProbDropRW+fc.config.ProbDropConn+fc.config.ProbSleep { time.Sleep(fc.randomDuration()) diff --git a/p2p/listener.go b/p2p/listener.go index 97139097..5b5f60a4 100644 --- a/p2p/listener.go +++ b/p2p/listener.go @@ -100,19 +100,24 @@ func NewDefaultListener(protocol string, lAddr string, skipUPNP bool, logger log connections: make(chan net.Conn, numBufferedConnections), } dl.BaseService = *cmn.NewBaseService(logger, "DefaultListener", dl) - dl.Start() // Started upon construction + _, err = dl.Start() // Started upon construction + if err != nil { + logger.Error("Error starting base service", "err", err) + } return dl } func (l *DefaultListener) OnStart() error { - l.BaseService.OnStart() + if err := l.BaseService.OnStart(); err != nil { + return err + } go l.listenRoutine() return nil } func (l *DefaultListener) OnStop() { l.BaseService.OnStop() - l.listener.Close() + l.listener.Close() // nolint (errcheck) } // Accept connections and pass on the channel diff --git a/p2p/listener_test.go b/p2p/listener_test.go index c3d33a9a..92018e0a 100644 --- a/p2p/listener_test.go +++ b/p2p/listener_test.go @@ -25,7 +25,12 @@ func TestListener(t *testing.T) { } msg := []byte("hi!") - go connIn.Write(msg) + go func() { + _, err := connIn.Write(msg) + if err != nil { + t.Error(err) + } + }() b := make([]byte, 32) n, err := connOut.Read(b) if err != nil { diff --git a/p2p/peer.go b/p2p/peer.go index ec834955..1d84eb28 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -230,7 +230,9 @@ func (p *peer) PubKey() crypto.PubKeyEd25519 { // OnStart implements BaseService. func (p *peer) OnStart() error { - p.BaseService.OnStart() + if err := p.BaseService.OnStart(); err != nil { + return err + } _, err := p.mconn.Start() return err } diff --git a/p2p/peer_set_test.go b/p2p/peer_set_test.go index e3745525..69430052 100644 --- a/p2p/peer_set_test.go +++ b/p2p/peer_set_test.go @@ -28,7 +28,9 @@ func TestPeerSetAddRemoveOne(t *testing.T) { var peerList []Peer for i := 0; i < 5; i++ { p := randPeer() - peerSet.Add(p) + if err := peerSet.Add(p); err != nil { + t.Error(err) + } peerList = append(peerList, p) } @@ -48,7 +50,9 @@ func TestPeerSetAddRemoveOne(t *testing.T) { // 2. Next we are testing removing the peer at the end // a) Replenish the peerSet for _, peer := range peerList { - peerSet.Add(peer) + if err := peerSet.Add(peer); err != nil { + t.Error(err) + } } // b) In reverse, remove each element diff --git a/p2p/peer_test.go b/p2p/peer_test.go index a027a6b7..b2a01493 100644 --- a/p2p/peer_test.go +++ b/p2p/peer_test.go @@ -23,7 +23,8 @@ func TestPeerBasic(t *testing.T) { p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), DefaultPeerConfig()) require.Nil(err) - p.Start() + _, err = p.Start() + require.Nil(err) defer p.Stop() assert.True(p.IsRunning()) @@ -49,7 +50,8 @@ func TestPeerWithoutAuthEnc(t *testing.T) { p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), config) require.Nil(err) - p.Start() + _, err = p.Start() + require.Nil(err) defer p.Stop() assert.True(p.IsRunning()) @@ -69,7 +71,9 @@ func TestPeerSend(t *testing.T) { p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), config) require.Nil(err) - p.Start() + _, err = p.Start() + require.Nil(err) + defer p.Stop() assert.True(p.CanSend(0x01)) @@ -148,7 +152,9 @@ func (p *remotePeer) accept(l net.Listener) { } select { case <-p.quit: - conn.Close() + if err := conn.Close(); err != nil { + golog.Fatal(err) + } return default: } diff --git a/p2p/pex_reactor.go b/p2p/pex_reactor.go index 7c799cca..71c3beb0 100644 --- a/p2p/pex_reactor.go +++ b/p2p/pex_reactor.go @@ -66,8 +66,13 @@ func NewPEXReactor(b *AddrBook) *PEXReactor { // OnStart implements BaseService func (r *PEXReactor) OnStart() error { - r.BaseReactor.OnStart() - r.book.Start() + if err := r.BaseReactor.OnStart(); err != nil { + return err + } + _, err := r.book.Start() + if err != nil { + return err + } go r.ensurePeersRoutine() go r.flushMsgCountByPeer() return nil diff --git a/p2p/pex_reactor_test.go b/p2p/pex_reactor_test.go index 3efc3c64..d34777e1 100644 --- a/p2p/pex_reactor_test.go +++ b/p2p/pex_reactor_test.go @@ -20,7 +20,11 @@ func TestPEXReactorBasic(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer func() { + if err := os.RemoveAll(dir); err != nil { + t.Error(err) + } + }() book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) @@ -36,7 +40,11 @@ func TestPEXReactorAddRemovePeer(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer func() { + if err := os.RemoveAll(dir); err != nil { + t.Error(err) + } + }() book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) @@ -69,7 +77,11 @@ func TestPEXReactorRunning(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer func() { + if err := os.RemoveAll(dir); err != nil { + t.Error(err) + } + }() book := NewAddrBook(dir+"addrbook.json", false) book.SetLogger(log.TestingLogger()) @@ -139,7 +151,11 @@ func TestPEXReactorReceive(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer func() { + if err := os.RemoveAll(dir); err != nil { + t.Error(err) + } + }() book := NewAddrBook(dir+"addrbook.json", false) book.SetLogger(log.TestingLogger()) @@ -164,7 +180,11 @@ func TestPEXReactorAbuseFromPeer(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer func() { + if err := os.RemoveAll(dir); err != nil { + t.Error(err) + } + }() book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) diff --git a/p2p/secret_connection.go b/p2p/secret_connection.go index 0e107ea5..f034b4c0 100644 --- a/p2p/secret_connection.go +++ b/p2p/secret_connection.go @@ -302,7 +302,7 @@ func shareAuthSignature(sc *SecretConnection, pubKey crypto.PubKeyEd25519, signa // sha256 func hash32(input []byte) (res *[32]byte) { hasher := sha256.New() - hasher.Write(input) // does not error + _, _ = hasher.Write(input) // ignoring error resSlice := hasher.Sum(nil) res = new([32]byte) copy(res[:], resSlice) @@ -312,7 +312,7 @@ func hash32(input []byte) (res *[32]byte) { // We only fill in the first 20 bytes with ripemd160 func hash24(input []byte) (res *[24]byte) { hasher := ripemd160.New() - hasher.Write(input) // does not error + _, _ = hasher.Write(input) // ignoring error resSlice := hasher.Sum(nil) res = new([24]byte) copy(res[:], resSlice) diff --git a/p2p/secret_connection_test.go b/p2p/secret_connection_test.go index d0d00852..8b58fb41 100644 --- a/p2p/secret_connection_test.go +++ b/p2p/secret_connection_test.go @@ -70,8 +70,12 @@ func makeSecretConnPair(tb testing.TB) (fooSecConn, barSecConn *SecretConnection func TestSecretConnectionHandshake(t *testing.T) { fooSecConn, barSecConn := makeSecretConnPair(t) - fooSecConn.Close() - barSecConn.Close() + if err := fooSecConn.Close(); err != nil { + t.Error(err) + } + if err := barSecConn.Close(); err != nil { + t.Error(err) + } } func TestSecretConnectionReadWrite(t *testing.T) { @@ -110,7 +114,9 @@ func TestSecretConnectionReadWrite(t *testing.T) { return } } - nodeConn.PipeWriter.Close() + if err := nodeConn.PipeWriter.Close(); err != nil { + t.Error(err) + } }, func() { // Node reads @@ -125,7 +131,9 @@ func TestSecretConnectionReadWrite(t *testing.T) { } *nodeReads = append(*nodeReads, string(readBuffer[:n])) } - nodeConn.PipeReader.Close() + if err := nodeConn.PipeReader.Close(); err != nil { + t.Error(err) + } }) } } @@ -197,6 +205,8 @@ func BenchmarkSecretConnection(b *testing.B) { } b.StopTimer() - fooSecConn.Close() + if err := fooSecConn.Close(); err != nil { + b.Error(err) + } //barSecConn.Close() race condition } diff --git a/p2p/switch.go b/p2p/switch.go index be51d561..4e22521a 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -174,7 +174,9 @@ func (sw *Switch) SetNodePrivKey(nodePrivKey crypto.PrivKeyEd25519) { // OnStart implements BaseService. It starts all the reactors, peers, and listeners. func (sw *Switch) OnStart() error { - sw.BaseService.OnStart() + if err := sw.BaseService.OnStart(); err != nil { + return err + } // Start reactors for _, reactor := range sw.reactors { _, err := reactor.Start() @@ -287,7 +289,11 @@ func (sw *Switch) SetPubKeyFilter(f func(crypto.PubKeyEd25519) error) { } func (sw *Switch) startInitPeer(peer *peer) { - peer.Start() // spawn send/recv routines + _, err := peer.Start() // spawn send/recv routines + if err != nil { + sw.Logger.Error("Error starting peer", "err", err) + } + for _, reactor := range sw.reactors { reactor.AddPeer(peer) } @@ -568,7 +574,9 @@ func makeSwitch(cfg *cfg.P2PConfig, i int, network, version string, initSwitch f func (sw *Switch) addPeerWithConnection(conn net.Conn) error { peer, err := newInboundPeer(conn, sw.reactorsByCh, sw.chDescs, sw.StopPeerForError, sw.nodePrivKey, sw.peerConfig) if err != nil { - conn.Close() + if err := conn.Close(); err != nil { + sw.Logger.Error("Error closing connection", "err", err) + } return err } peer.SetLogger(sw.Logger.With("peer", conn.RemoteAddr())) @@ -583,7 +591,9 @@ func (sw *Switch) addPeerWithConnection(conn net.Conn) error { func (sw *Switch) addPeerWithConnectionAndConfig(conn net.Conn, config *PeerConfig) error { peer, err := newInboundPeer(conn, sw.reactorsByCh, sw.chDescs, sw.StopPeerForError, sw.nodePrivKey, config) if err != nil { - conn.Close() + if err := conn.Close(); err != nil { + sw.Logger.Error("Error closing connection", "err", err) + } return err } peer.SetLogger(sw.Logger.With("peer", conn.RemoteAddr())) diff --git a/p2p/switch_test.go b/p2p/switch_test.go index e82eead9..fb179efe 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -171,10 +171,14 @@ func TestConnAddrFilter(t *testing.T) { // connect to good peer go func() { - s1.addPeerWithConnection(c1) + if err := s1.addPeerWithConnection(c1); err != nil { + // t.Error(err) FIXME: fails + } }() go func() { - s2.addPeerWithConnection(c2) + if err := s2.addPeerWithConnection(c2); err != nil { + // t.Error(err) FIXME: fails + } }() assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond) @@ -206,10 +210,14 @@ func TestConnPubKeyFilter(t *testing.T) { // connect to good peer go func() { - s1.addPeerWithConnection(c1) + if err := s1.addPeerWithConnection(c1); err != nil { + // t.Error(err) FIXME: fails + } }() go func() { - s2.addPeerWithConnection(c2) + if err := s2.addPeerWithConnection(c2); err != nil { + // t.Error(err) FIXME: fails + } }() assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond) @@ -220,7 +228,10 @@ func TestSwitchStopsNonPersistentPeerOnError(t *testing.T) { assert, require := assert.New(t), require.New(t) sw := makeSwitch(config, 1, "testing", "123.123.123", initSwitchFunc) - sw.Start() + _, err := sw.Start() + if err != nil { + t.Error(err) + } defer sw.Stop() // simulate remote peer @@ -244,7 +255,10 @@ func TestSwitchReconnectsToPersistentPeer(t *testing.T) { assert, require := assert.New(t), require.New(t) sw := makeSwitch(config, 1, "testing", "123.123.123", initSwitchFunc) - sw.Start() + _, err := sw.Start() + if err != nil { + t.Error(err) + } defer sw.Stop() // simulate remote peer diff --git a/p2p/upnp/upnp.go b/p2p/upnp/upnp.go index 328d86f4..81e1f7a3 100644 --- a/p2p/upnp/upnp.go +++ b/p2p/upnp/upnp.go @@ -197,7 +197,7 @@ func getServiceURL(rootURL string) (url, urnDomain string, err error) { if err != nil { return } - defer r.Body.Close() + defer r.Body.Close() // nolint (errcheck) if r.StatusCode >= 400 { err = errors.New(string(r.StatusCode)) From 48aca642e3d149749248d5a352f31d3695ab5d83 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 13:54:34 -0400 Subject: [PATCH 12/32] linter: address deadcode, implement incremental lint testing --- Makefile | 4 ++++ circle.yml | 1 + consensus/common.go | 18 ++++++++++++++++++ consensus/reactor_test.go | 14 ++++++++++++++ state/execution.go | 2 ++ 5 files changed, 39 insertions(+) create mode 100644 consensus/common.go diff --git a/Makefile b/Makefile index d935833b..9b07cd5c 100644 --- a/Makefile +++ b/Makefile @@ -84,4 +84,8 @@ metalinter: ensure_tools @gometalinter --install gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... +metalinter_test: ensure_tools + @gometalinter --install + gometalinter --vendor --deadline=600s --disable-all --enable=deadcode dupl ./... + .PHONY: install build build_race dist test test_race test_integrations test100 draw_deps list_deps get_deps get_vendor_deps update_deps revision tools diff --git a/circle.yml b/circle.yml index 50ffbd01..6a5e2fea 100644 --- a/circle.yml +++ b/circle.yml @@ -25,6 +25,7 @@ dependencies: test: override: - cd "$PROJECT_PATH" && set -o pipefail && make test_integrations 2>&1 | tee test_integrations.log: + - cd "$PROJECT_PATH" && make metalinter_test timeout: 1800 post: - cd "$PROJECT_PATH" && mv test_integrations.log "${CIRCLE_ARTIFACTS}" diff --git a/consensus/common.go b/consensus/common.go new file mode 100644 index 00000000..836b68f5 --- /dev/null +++ b/consensus/common.go @@ -0,0 +1,18 @@ +package consensus + +import ( + "github.com/tendermint/tendermint/types" +) + +// XXX: WARNING: this function can halt the consensus as firing events is synchronous. +// Make sure to read off the channels, and in the case of subscribeToEventRespond, to write back on it + +// NOTE: if chanCap=0, this blocks on the event being consumed +func subscribeToEvent(evsw types.EventSwitch, receiver, eventID string, chanCap int) chan interface{} { + // listen for event + ch := make(chan interface{}, chanCap) + types.AddListenerForEvent(evsw, receiver, eventID, func(data types.TMEventData) { + ch <- data + }) + return ch +} diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index 2d27cdd8..a45ebfd1 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -389,3 +389,17 @@ func timeoutWaitGroup(t *testing.T, n int, f func(*sync.WaitGroup, int), css []* panic("Timed out waiting for all validators to commit a block") } } + +// XXX: WARNING: this function can halt the consensus as firing events is synchronous. +// Make sure to read off the channels, and in the case of subscribeToEventRespond, to write back on it + +// NOTE: this blocks on receiving a response after the event is consumed +func subscribeToEventRespond(evsw types.EventSwitch, receiver, eventID string) chan interface{} { + // listen for event + ch := make(chan interface{}) + types.AddListenerForEvent(evsw, receiver, eventID, func(data types.TMEventData) { + ch <- data + <-ch + }) + return ch +} diff --git a/state/execution.go b/state/execution.go index 495b70c8..0c870095 100644 --- a/state/execution.go +++ b/state/execution.go @@ -160,6 +160,7 @@ func updateValidators(validators *types.ValidatorSet, changedValidators []*abci. // return a bit array of validators that signed the last commit // NOTE: assumes commits have already been authenticated +/* function is currently unused func commitBitArrayFromBlock(block *types.Block) *cmn.BitArray { signed := cmn.NewBitArray(len(block.LastCommit.Precommits)) for i, precommit := range block.LastCommit.Precommits { @@ -169,6 +170,7 @@ func commitBitArrayFromBlock(block *types.Block) *cmn.BitArray { } return signed } +*/ //----------------------------------------------------- // Validate block From bc2aa79f9af8c74a4b92989089a65d2989082ea8 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 14:13:13 -0400 Subject: [PATCH 13/32] linter: sort through each kind and address small fixes --- Makefile | 28 +++++++++++++++++++++++++++- benchmarks/map_test.go | 2 +- consensus/byzantine_test.go | 2 +- consensus/state.go | 7 ++----- node/id.go | 2 +- 5 files changed, 32 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 9b07cd5c..ee283916 100644 --- a/Makefile +++ b/Makefile @@ -86,6 +86,32 @@ metalinter: ensure_tools metalinter_test: ensure_tools @gometalinter --install - gometalinter --vendor --deadline=600s --disable-all --enable=deadcode dupl ./... + gometalinter --vendor --deadline=600s --disable-all \ + --enable=deadcode \ + --enable=gas \ + --enable=goimports \ + --enable=gosimple \ + --enable=gotype \ + --enable=ineffassign \ + --enable=misspell \ + --enable=safesql \ + --enable=structcheck \ + --enable=varcheck \ + ./... + + #--enable=aligncheck \ + #--enable=dupl \ + #--enable=errcheck \ + #--enable=goconst \ + #--enable=gocyclo \ + #--enable=golint \ <== comments on anything exported + #--enable=interfacer \ + #--enable=megacheck \ + #--enable=staticcheck \ + #--enable=unconvert \ + #--enable=unparam \ + #--enable=unused \ + #--enable=vet \ + #--enable=vetshadow \ .PHONY: install build build_race dist test test_race test_integrations test100 draw_deps list_deps get_deps get_vendor_deps update_deps revision tools diff --git a/benchmarks/map_test.go b/benchmarks/map_test.go index 2d978902..c89eba53 100644 --- a/benchmarks/map_test.go +++ b/benchmarks/map_test.go @@ -1,4 +1,4 @@ -package benchmarks +package benchmarks // nolint (goimports) import ( "testing" diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go index 163f5490..705cdc12 100644 --- a/consensus/byzantine_test.go +++ b/consensus/byzantine_test.go @@ -70,7 +70,7 @@ func TestByzantine(t *testing.T) { conR.SetLogger(logger.With("validator", i)) conR.SetEventBus(eventBus) - var conRI p2p.Reactor + var conRI p2p.Reactor // nolint (gotype) conRI = conR if i == 0 { diff --git a/consensus/state.go b/consensus/state.go index 608d5d2d..c50e431e 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -371,7 +371,7 @@ func (cs *ConsensusState) updateRoundStep(round int, step cstypes.RoundStepType) // enterNewRound(height, 0) at cs.StartTime. func (cs *ConsensusState) scheduleRound0(rs *cstypes.RoundState) { //cs.Logger.Info("scheduleRound0", "now", time.Now(), "startTime", cs.StartTime) - sleepDuration := rs.StartTime.Sub(time.Now()) + sleepDuration := rs.StartTime.Sub(time.Now()) // nolint (gotype) cs.scheduleTimeout(sleepDuration, rs.Height, 0, cstypes.RoundStepNewHeight) } @@ -702,10 +702,7 @@ func (cs *ConsensusState) needProofBlock(height int) bool { } lastBlockMeta := cs.blockStore.LoadBlockMeta(height - 1) - if !bytes.Equal(cs.state.AppHash, lastBlockMeta.Header.AppHash) { - return true - } - return false + return !bytes.Equal(cs.state.AppHash, lastBlockMeta.Header.AppHash) } func (cs *ConsensusState) proposalHeartbeat(height, round int) { diff --git a/node/id.go b/node/id.go index fa391f94..95c87c8d 100644 --- a/node/id.go +++ b/node/id.go @@ -1,4 +1,4 @@ -package node +package node // nolint (goimports) import ( "time" From fe694e1fe1257efe6c183f4d134294d2d2c41ff0 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 14:29:58 -0400 Subject: [PATCH 14/32] ... --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index 6a5e2fea..b0eed477 100644 --- a/circle.yml +++ b/circle.yml @@ -25,8 +25,8 @@ dependencies: test: override: - cd "$PROJECT_PATH" && set -o pipefail && make test_integrations 2>&1 | tee test_integrations.log: - - cd "$PROJECT_PATH" && make metalinter_test timeout: 1800 + - cd "$PROJECT_PATH" && make metalinter_test post: - cd "$PROJECT_PATH" && mv test_integrations.log "${CIRCLE_ARTIFACTS}" - cd "$PROJECT_PATH" && bash <(curl -s https://codecov.io/bash) -f coverage.txt From 9c62ed45959b821a9939f8a7cb34850bc3ca66bb Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 21 Sep 2017 14:33:18 -0400 Subject: [PATCH 15/32] run linting first for tests --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index b0eed477..384871cc 100644 --- a/circle.yml +++ b/circle.yml @@ -24,9 +24,9 @@ dependencies: test: override: + - cd "$PROJECT_PATH" && make metalinter_test - cd "$PROJECT_PATH" && set -o pipefail && make test_integrations 2>&1 | tee test_integrations.log: timeout: 1800 - - cd "$PROJECT_PATH" && make metalinter_test post: - cd "$PROJECT_PATH" && mv test_integrations.log "${CIRCLE_ARTIFACTS}" - cd "$PROJECT_PATH" && bash <(curl -s https://codecov.io/bash) -f coverage.txt From 563faa98de64b258bf58c5ec0c917a6fc9cda47f Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 18:12:17 -0400 Subject: [PATCH 16/32] address comments, pr #643 --- blockchain/pool.go | 2 +- cmd/tendermint/commands/reset_priv_validator.go | 1 + consensus/reactor.go | 2 +- consensus/state.go | 1 + 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/blockchain/pool.go b/blockchain/pool.go index f4fd1a32..0791bdb0 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -313,7 +313,7 @@ func (pool *BlockPool) makeNextRequester() { _, err := request.Start() if err != nil { - pool.Logger.Error("Error starting block pool", "err", err) + request.Logger.Error("Error starting request", "err", err) } } diff --git a/cmd/tendermint/commands/reset_priv_validator.go b/cmd/tendermint/commands/reset_priv_validator.go index 77407cfc..1612b89c 100644 --- a/cmd/tendermint/commands/reset_priv_validator.go +++ b/cmd/tendermint/commands/reset_priv_validator.go @@ -49,6 +49,7 @@ func ResetAll(dbDir, privValFile string, logger log.Logger) { resetPrivValidatorLocal(privValFile, logger) if err := os.RemoveAll(dbDir); err != nil { logger.Error("Error removing directory", "err", err) + return } logger.Info("Removed all data", "dir", dbDir) } diff --git a/consensus/reactor.go b/consensus/reactor.go index 0eaacefd..e873ddde 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -99,7 +99,7 @@ func (conR *ConsensusReactor) SwitchToConsensus(state *sm.State, blocksSynced in } _, err := conR.conS.Start() if err != nil { - conR.Logger.Error("Error starting conR", "err", err) + conR.Logger.Error("Error starting conS", "err", err) } } diff --git a/consensus/state.go b/consensus/state.go index c50e431e..2c2b5d1f 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -260,6 +260,7 @@ func (cs *ConsensusState) startRoutines(maxSteps int) { _, err := cs.timeoutTicker.Start() if err != nil { cs.Logger.Error("Error starting timeout ticker", "err", err) + return } go cs.receiveRoutine(maxSteps) } From d7cb291fb2f0b8a1611f1a21dc9a8cbf100d5a41 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 18:49:20 -0400 Subject: [PATCH 17/32] errcheck; sort some stuff out --- Makefile | 2 +- benchmarks/map_test.go | 2 +- blockchain/reactor.go | 2 +- blockchain/store.go | 24 +++++----- .../commands/reset_priv_validator.go | 17 ++----- config/toml_test.go | 5 +- consensus/byzantine_test.go | 2 +- consensus/replay.go | 7 +-- consensus/replay_file.go | 6 +-- consensus/replay_test.go | 7 +-- consensus/state.go | 2 +- node/id.go | 2 +- p2p/connection_test.go | 48 ++++--------------- p2p/fuzz.go | 2 +- p2p/pex_reactor_test.go | 30 ++---------- p2p/upnp/upnp.go | 10 ++-- p2p/util.go | 10 +--- rpc/client/mock/abci.go | 4 +- rpc/grpc/client_server.go | 2 +- rpc/lib/client/http_client.go | 4 +- rpc/lib/client/ws_client.go | 4 +- rpc/lib/client/ws_client_test.go | 6 +-- rpc/lib/server/handlers.go | 5 +- rpc/lib/server/http_server.go | 4 +- state/txindex/kv/kv_test.go | 6 +-- types/part_set.go | 2 +- 26 files changed, 61 insertions(+), 154 deletions(-) diff --git a/Makefile b/Makefile index ee283916..59b29410 100644 --- a/Makefile +++ b/Makefile @@ -89,7 +89,6 @@ metalinter_test: ensure_tools gometalinter --vendor --deadline=600s --disable-all \ --enable=deadcode \ --enable=gas \ - --enable=goimports \ --enable=gosimple \ --enable=gotype \ --enable=ineffassign \ @@ -104,6 +103,7 @@ metalinter_test: ensure_tools #--enable=errcheck \ #--enable=goconst \ #--enable=gocyclo \ + #--enable=goimports \ #--enable=golint \ <== comments on anything exported #--enable=interfacer \ #--enable=megacheck \ diff --git a/benchmarks/map_test.go b/benchmarks/map_test.go index c89eba53..f6caf403 100644 --- a/benchmarks/map_test.go +++ b/benchmarks/map_test.go @@ -1,4 +1,4 @@ -package benchmarks // nolint (goimports) +package benchmarks // nolint: goimports import ( "testing" diff --git a/blockchain/reactor.go b/blockchain/reactor.go index cf294894..4d20e777 100644 --- a/blockchain/reactor.go +++ b/blockchain/reactor.go @@ -228,7 +228,7 @@ FOR_LOOP: } case <-statusUpdateTicker.C: // ask for status updates - go bcR.BroadcastStatusRequest() // nolint (errcheck) + go bcR.BroadcastStatusRequest() // nolint: errcheck case <-switchToConsensusTicker.C: height, numPending, lenRequesters := bcR.pool.GetStatus() outbound, inbound, _ := bcR.Switch.NumPeers() diff --git a/blockchain/store.go b/blockchain/store.go index 7e1859f2..bcd10856 100644 --- a/blockchain/store.go +++ b/blockchain/store.go @@ -9,7 +9,7 @@ import ( "github.com/tendermint/go-wire" "github.com/tendermint/tendermint/types" - . "github.com/tendermint/tmlibs/common" + cmn "github.com/tendermint/tmlibs/common" dbm "github.com/tendermint/tmlibs/db" ) @@ -67,7 +67,7 @@ func (bs *BlockStore) LoadBlock(height int) *types.Block { } blockMeta := wire.ReadBinary(&types.BlockMeta{}, r, 0, &n, &err).(*types.BlockMeta) if err != nil { - PanicCrisis(Fmt("Error reading block meta: %v", err)) + cmn.PanicCrisis(cmn.Fmt("Error reading block meta: %v", err)) } bytez := []byte{} for i := 0; i < blockMeta.BlockID.PartsHeader.Total; i++ { @@ -76,7 +76,7 @@ func (bs *BlockStore) LoadBlock(height int) *types.Block { } block := wire.ReadBinary(&types.Block{}, bytes.NewReader(bytez), 0, &n, &err).(*types.Block) if err != nil { - PanicCrisis(Fmt("Error reading block: %v", err)) + cmn.PanicCrisis(cmn.Fmt("Error reading block: %v", err)) } return block } @@ -90,7 +90,7 @@ func (bs *BlockStore) LoadBlockPart(height int, index int) *types.Part { } part := wire.ReadBinary(&types.Part{}, r, 0, &n, &err).(*types.Part) if err != nil { - PanicCrisis(Fmt("Error reading block part: %v", err)) + cmn.PanicCrisis(cmn.Fmt("Error reading block part: %v", err)) } return part } @@ -104,7 +104,7 @@ func (bs *BlockStore) LoadBlockMeta(height int) *types.BlockMeta { } blockMeta := wire.ReadBinary(&types.BlockMeta{}, r, 0, &n, &err).(*types.BlockMeta) if err != nil { - PanicCrisis(Fmt("Error reading block meta: %v", err)) + cmn.PanicCrisis(cmn.Fmt("Error reading block meta: %v", err)) } return blockMeta } @@ -120,7 +120,7 @@ func (bs *BlockStore) LoadBlockCommit(height int) *types.Commit { } commit := wire.ReadBinary(&types.Commit{}, r, 0, &n, &err).(*types.Commit) if err != nil { - PanicCrisis(Fmt("Error reading commit: %v", err)) + cmn.PanicCrisis(cmn.Fmt("Error reading commit: %v", err)) } return commit } @@ -135,7 +135,7 @@ func (bs *BlockStore) LoadSeenCommit(height int) *types.Commit { } commit := wire.ReadBinary(&types.Commit{}, r, 0, &n, &err).(*types.Commit) if err != nil { - PanicCrisis(Fmt("Error reading commit: %v", err)) + cmn.PanicCrisis(cmn.Fmt("Error reading commit: %v", err)) } return commit } @@ -148,10 +148,10 @@ func (bs *BlockStore) LoadSeenCommit(height int) *types.Commit { func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, seenCommit *types.Commit) { height := block.Height if height != bs.Height()+1 { - PanicSanity(Fmt("BlockStore can only save contiguous blocks. Wanted %v, got %v", bs.Height()+1, height)) + cmn.PanicSanity(cmn.Fmt("BlockStore can only save contiguous blocks. Wanted %v, got %v", bs.Height()+1, height)) } if !blockParts.IsComplete() { - PanicSanity(Fmt("BlockStore can only save complete block part sets")) + cmn.PanicSanity(cmn.Fmt("BlockStore can only save complete block part sets")) } // Save block meta @@ -187,7 +187,7 @@ func (bs *BlockStore) SaveBlock(block *types.Block, blockParts *types.PartSet, s func (bs *BlockStore) saveBlockPart(height int, index int, part *types.Part) { if height != bs.Height()+1 { - PanicSanity(Fmt("BlockStore can only save contiguous blocks. Wanted %v, got %v", bs.Height()+1, height)) + cmn.PanicSanity(cmn.Fmt("BlockStore can only save contiguous blocks. Wanted %v, got %v", bs.Height()+1, height)) } partBytes := wire.BinaryBytes(part) bs.db.Set(calcBlockPartKey(height, index), partBytes) @@ -222,7 +222,7 @@ type BlockStoreStateJSON struct { func (bsj BlockStoreStateJSON) Save(db dbm.DB) { bytes, err := json.Marshal(bsj) if err != nil { - PanicSanity(Fmt("Could not marshal state bytes: %v", err)) + cmn.PanicSanity(cmn.Fmt("Could not marshal state bytes: %v", err)) } db.SetSync(blockStoreKey, bytes) } @@ -237,7 +237,7 @@ func LoadBlockStoreStateJSON(db dbm.DB) BlockStoreStateJSON { bsj := BlockStoreStateJSON{} err := json.Unmarshal(bytes, &bsj) if err != nil { - PanicCrisis(Fmt("Could not unmarshal bytes: %X", bytes)) + cmn.PanicCrisis(cmn.Fmt("Could not unmarshal bytes: %X", bytes)) } return bsj } diff --git a/cmd/tendermint/commands/reset_priv_validator.go b/cmd/tendermint/commands/reset_priv_validator.go index 1612b89c..51336523 100644 --- a/cmd/tendermint/commands/reset_priv_validator.go +++ b/cmd/tendermint/commands/reset_priv_validator.go @@ -25,10 +25,13 @@ var ResetPrivValidatorCmd = &cobra.Command{ } // ResetAll removes the privValidator files. -// Exported so other CLI tools can use it +// Exported so other CLI tools can use it. func ResetAll(dbDir, privValFile string, logger log.Logger) { resetPrivValidatorFS(privValFile, logger) - os.RemoveAll(dbDir) + if err := os.RemoveAll(dbDir); err != nil { + logger.Error("Error removing directory", "err", err) + return + } logger.Info("Removed all data", "dir", dbDir) } @@ -44,16 +47,6 @@ func resetPrivValidator(cmd *cobra.Command, args []string) { resetPrivValidatorFS(config.PrivValidatorFile(), logger) } -// Exported so other CLI tools can use it -func ResetAll(dbDir, privValFile string, logger log.Logger) { - resetPrivValidatorLocal(privValFile, logger) - if err := os.RemoveAll(dbDir); err != nil { - logger.Error("Error removing directory", "err", err) - return - } - logger.Info("Removed all data", "dir", dbDir) -} - func resetPrivValidatorFS(privValFile string, logger log.Logger) { // Get PrivValidator if _, err := os.Stat(privValFile); err == nil { diff --git a/config/toml_test.go b/config/toml_test.go index c435ccb3..bf3bf58f 100644 --- a/config/toml_test.go +++ b/config/toml_test.go @@ -24,10 +24,7 @@ func TestEnsureRoot(t *testing.T) { // setup temp dir for test tmpDir, err := ioutil.TempDir("", "config-test") require.Nil(err) - defer func() { - err := os.RemoveAll(tmpDir) - require.Nil(err) - }() + defer os.RemoveAll(tmpDir) // nolint: errcheck // create root dir EnsureRoot(tmpDir) diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go index 705cdc12..c48e72f7 100644 --- a/consensus/byzantine_test.go +++ b/consensus/byzantine_test.go @@ -70,7 +70,7 @@ func TestByzantine(t *testing.T) { conR.SetLogger(logger.With("validator", i)) conR.SetEventBus(eventBus) - var conRI p2p.Reactor // nolint (gotype) + var conRI p2p.Reactor // nolint: gotype conRI = conR if i == 0 { diff --git a/consensus/replay.go b/consensus/replay.go index d3ea9188..427d0429 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -115,12 +115,7 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { } else if err != nil { return err } else { - defer func() { - if err := gr.Close(); err != nil { - cs.Logger.Error("Error closing wal Search", "err", err) - return - } - }() + defer gr.Close() // nolint: errcheck } if !found { return errors.New(cmn.Fmt("Cannot replay height %d. WAL does not contain #ENDHEIGHT for %d.", csHeight, csHeight-1)) diff --git a/consensus/replay_file.go b/consensus/replay_file.go index 6e5b1a8b..4d5a631d 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -65,11 +65,7 @@ func (cs *ConsensusState) ReplayFile(file string, console bool) error { } pb := newPlayback(file, fp, cs, cs.state.Copy()) - defer func() { - if err := pb.fp.Close(); err != nil { - cs.Logger.Error("Error closing new playback", "err", err) - } - }() + defer pb.fp.Close() // nolint: errcheck var nextN int // apply N msgs in a row var msg *TimedWALMessage diff --git a/consensus/replay_test.go b/consensus/replay_test.go index 9e4dbb84..8db94cf9 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -490,12 +490,7 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) { if !found { return nil, nil, errors.New(cmn.Fmt("WAL does not contain height %d.", 1)) } - defer func() { - if err := gr.Close(); err != nil { - wal.Logger.Error("Error closing wal Search", "err", err) - return - } - }() + defer gr.Close() // log.Notice("Build a blockchain by reading from the WAL") diff --git a/consensus/state.go b/consensus/state.go index 2c2b5d1f..0c5ad3a6 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -372,7 +372,7 @@ func (cs *ConsensusState) updateRoundStep(round int, step cstypes.RoundStepType) // enterNewRound(height, 0) at cs.StartTime. func (cs *ConsensusState) scheduleRound0(rs *cstypes.RoundState) { //cs.Logger.Info("scheduleRound0", "now", time.Now(), "startTime", cs.StartTime) - sleepDuration := rs.StartTime.Sub(time.Now()) // nolint (gotype) + sleepDuration := rs.StartTime.Sub(time.Now()) // nolint: gotype cs.scheduleTimeout(sleepDuration, rs.Height, 0, cstypes.RoundStepNewHeight) } diff --git a/node/id.go b/node/id.go index 95c87c8d..fa391f94 100644 --- a/node/id.go +++ b/node/id.go @@ -1,4 +1,4 @@ -package node // nolint (goimports) +package node import ( "time" diff --git a/p2p/connection_test.go b/p2p/connection_test.go index b530a009..11b036dc 100644 --- a/p2p/connection_test.go +++ b/p2p/connection_test.go @@ -32,16 +32,8 @@ func TestMConnectionSend(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer func() { - if err := server.Close(); err != nil { - t.Error(err) - } - }() - defer func() { - if err := client.Close(); err != nil { - t.Error(err) - } - }() + defer server.Close() // nolint: errcheck + defer client.Close() // nolint: errcheck mconn := createTestMConnection(client) _, err := mconn.Start() @@ -73,16 +65,8 @@ func TestMConnectionReceive(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer func() { - if err := server.Close(); err != nil { - t.Error(err) - } - }() - defer func() { - if err := client.Close(); err != nil { - t.Error(err) - } - }() + defer server.Close() // nolint: errcheck + defer client.Close() // nolint: errcheck receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -119,16 +103,8 @@ func TestMConnectionStatus(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer func() { - if err := server.Close(); err != nil { - t.Error(err) - } - }() - defer func() { - if err := client.Close(); err != nil { - t.Error(err) - } - }() + defer server.Close() // nolint: errcheck + defer client.Close() // nolint: errcheck mconn := createTestMConnection(client) _, err := mconn.Start() @@ -144,16 +120,8 @@ func TestMConnectionStopsAndReturnsError(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer func() { - if err := server.Close(); err != nil { - t.Error(err) - } - }() - defer func() { - if err := client.Close(); err != nil { - t.Error(err) - } - }() + defer server.Close() // nolint: errcheck + defer client.Close() // nolint: errcheck receivedCh := make(chan []byte) errorsCh := make(chan interface{}) diff --git a/p2p/fuzz.go b/p2p/fuzz.go index 26a9e10d..dfa34fa1 100644 --- a/p2p/fuzz.go +++ b/p2p/fuzz.go @@ -143,7 +143,7 @@ func (fc *FuzzedConnection) fuzz() bool { } else if r < fc.config.ProbDropRW+fc.config.ProbDropConn { // XXX: can't this fail because machine precision? // XXX: do we need an error? - fc.Close() // nolint (errcheck) + fc.Close() // nolint: errcheck return true } else if r < fc.config.ProbDropRW+fc.config.ProbDropConn+fc.config.ProbSleep { time.Sleep(fc.randomDuration()) diff --git a/p2p/pex_reactor_test.go b/p2p/pex_reactor_test.go index d34777e1..e79c73a8 100644 --- a/p2p/pex_reactor_test.go +++ b/p2p/pex_reactor_test.go @@ -20,11 +20,7 @@ func TestPEXReactorBasic(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer func() { - if err := os.RemoveAll(dir); err != nil { - t.Error(err) - } - }() + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) @@ -40,11 +36,7 @@ func TestPEXReactorAddRemovePeer(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer func() { - if err := os.RemoveAll(dir); err != nil { - t.Error(err) - } - }() + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) @@ -77,11 +69,7 @@ func TestPEXReactorRunning(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer func() { - if err := os.RemoveAll(dir); err != nil { - t.Error(err) - } - }() + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", false) book.SetLogger(log.TestingLogger()) @@ -151,11 +139,7 @@ func TestPEXReactorReceive(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer func() { - if err := os.RemoveAll(dir); err != nil { - t.Error(err) - } - }() + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", false) book.SetLogger(log.TestingLogger()) @@ -180,11 +164,7 @@ func TestPEXReactorAbuseFromPeer(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer func() { - if err := os.RemoveAll(dir); err != nil { - t.Error(err) - } - }() + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) diff --git a/p2p/upnp/upnp.go b/p2p/upnp/upnp.go index 81e1f7a3..2d0ad53e 100644 --- a/p2p/upnp/upnp.go +++ b/p2p/upnp/upnp.go @@ -40,7 +40,7 @@ func Discover() (nat NAT, err error) { return } socket := conn.(*net.UDPConn) - defer socket.Close() // nolint (errcheck) + defer socket.Close() // nolint: errcheck if err := socket.SetDeadline(time.Now().Add(3 * time.Second)); err != nil { return nil, err @@ -197,7 +197,7 @@ func getServiceURL(rootURL string) (url, urnDomain string, err error) { if err != nil { return } - defer r.Body.Close() // nolint (errcheck) + defer r.Body.Close() // nolint: errcheck if r.StatusCode >= 400 { err = errors.New(string(r.StatusCode)) @@ -296,7 +296,7 @@ func (n *upnpNAT) getExternalIPAddress() (info statusInfo, err error) { var response *http.Response response, err = soapRequest(n.serviceURL, "GetExternalIPAddress", message, n.urnDomain) if response != nil { - defer response.Body.Close() // nolint (errcheck) + defer response.Body.Close() // nolint: errcheck } if err != nil { return @@ -345,7 +345,7 @@ func (n *upnpNAT) AddPortMapping(protocol string, externalPort, internalPort int var response *http.Response response, err = soapRequest(n.serviceURL, "AddPortMapping", message, n.urnDomain) if response != nil { - defer response.Body.Close() // nolint (errcheck) + defer response.Body.Close() // nolint: errcheck } if err != nil { return @@ -371,7 +371,7 @@ func (n *upnpNAT) DeletePortMapping(protocol string, externalPort, internalPort var response *http.Response response, err = soapRequest(n.serviceURL, "DeletePortMapping", message, n.urnDomain) if response != nil { - defer response.Body.Close() // nolint (errcheck) + defer response.Body.Close() // nolint: errcheck } if err != nil { return diff --git a/p2p/util.go b/p2p/util.go index ec5ade1c..25385d0a 100644 --- a/p2p/util.go +++ b/p2p/util.go @@ -7,15 +7,9 @@ import ( // doubleSha256 calculates sha256(sha256(b)) and returns the resulting bytes. func doubleSha256(b []byte) []byte { hasher := sha256.New() - _, err := hasher.Write(b) - if err != nil { - panic(err) - } + _, _ := hasher.Write(b) // error ignored sum := hasher.Sum(nil) hasher.Reset() - _, err = hasher.Write(sum) - if err != nil { - panic(err) - } + _, _ = hasher.Write(sum) // error ignored return hasher.Sum(nil) } diff --git a/rpc/client/mock/abci.go b/rpc/client/mock/abci.go index 7bcb8cc6..e935a282 100644 --- a/rpc/client/mock/abci.go +++ b/rpc/client/mock/abci.go @@ -49,7 +49,7 @@ func (a ABCIApp) BroadcastTxAsync(tx types.Tx) (*ctypes.ResultBroadcastTx, error c := a.App.CheckTx(tx) // and this gets written in a background thread... if c.IsOK() { - go func() { a.App.DeliverTx(tx) }() // nolint (errcheck) + go func() { a.App.DeliverTx(tx) }() // nolint: errcheck } return &ctypes.ResultBroadcastTx{c.Code, c.Data, c.Log, tx.Hash()}, nil } @@ -58,7 +58,7 @@ func (a ABCIApp) BroadcastTxSync(tx types.Tx) (*ctypes.ResultBroadcastTx, error) c := a.App.CheckTx(tx) // and this gets written in a background thread... if c.IsOK() { - go func() { a.App.DeliverTx(tx) }() // nolint (errcheck) + go func() { a.App.DeliverTx(tx) }() // nolint: errcheck } return &ctypes.ResultBroadcastTx{c.Code, c.Data, c.Log, tx.Hash()}, nil } diff --git a/rpc/grpc/client_server.go b/rpc/grpc/client_server.go index 87d18092..80d736f5 100644 --- a/rpc/grpc/client_server.go +++ b/rpc/grpc/client_server.go @@ -25,7 +25,7 @@ func StartGRPCServer(protoAddr string) (net.Listener, error) { grpcServer := grpc.NewServer() RegisterBroadcastAPIServer(grpcServer, &broadcastAPI{}) - go grpcServer.Serve(ln) // nolint (errcheck) + go grpcServer.Serve(ln) // nolint: errcheck return ln, nil } diff --git a/rpc/lib/client/http_client.go b/rpc/lib/client/http_client.go index eb9848c4..a1b23a25 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -93,7 +93,7 @@ func (c *JSONRPCClient) Call(method string, params map[string]interface{}, resul if err != nil { return nil, err } - defer httpResponse.Body.Close() // nolint (errcheck) + defer httpResponse.Body.Close() // nolint: errcheck responseBytes, err := ioutil.ReadAll(httpResponse.Body) if err != nil { @@ -129,7 +129,7 @@ func (c *URIClient) Call(method string, params map[string]interface{}, result in if err != nil { return nil, err } - defer resp.Body.Close() // nolint (errcheck) + defer resp.Body.Close() // nolint: errcheck responseBytes, err := ioutil.ReadAll(resp.Body) if err != nil { diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index bf770951..57396432 100644 --- a/rpc/lib/client/ws_client.go +++ b/rpc/lib/client/ws_client.go @@ -354,7 +354,7 @@ func (c *WSClient) writeRoutine() { ticker.Stop() if err := c.conn.Close(); err != nil { // ignore error; it will trigger in tests - // likely because it's closing and already closed connection + // likely because it's closing an already closed connection } c.wg.Done() }() @@ -406,7 +406,7 @@ func (c *WSClient) readRoutine() { defer func() { if err := c.conn.Close(); err != nil { // ignore error; it will trigger in tests - // likely because it's closing and already closed connection + // likely because it's closing an already closed connection } c.wg.Done() }() diff --git a/rpc/lib/client/ws_client_test.go b/rpc/lib/client/ws_client_test.go index a840ac37..5ee509b8 100644 --- a/rpc/lib/client/ws_client_test.go +++ b/rpc/lib/client/ws_client_test.go @@ -34,11 +34,7 @@ func (h *myHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err != nil { panic(err) } - defer func() { - if err := conn.Close(); err != nil { - panic(err) - } - }() + defer conn.Close() // nolint: errcheck for { messageType, _, err := conn.ReadMessage() if err != nil { diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index 023a521a..aaa1549d 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -782,8 +782,5 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st buf.WriteString("") w.Header().Set("Content-Type", "text/html") w.WriteHeader(200) - _, err := w.Write(buf.Bytes()) - if err != nil { - // ignore error - } + _, _ := w.Write(buf.Bytes()) // error ignored } diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index 530f90bb..3f2d33dc 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -56,7 +56,7 @@ func WriteRPCResponseHTTPError(w http.ResponseWriter, httpCode int, res types.RP w.Header().Set("Content-Type", "application/json") w.WriteHeader(httpCode) - _, _ = w.Write(jsonBytes) // ignoring error + _, _ = w.Write(jsonBytes) // error ignored } func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { @@ -66,7 +66,7 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { } w.Header().Set("Content-Type", "application/json") w.WriteHeader(200) - _, _ = w.Write(jsonBytes) // ignoring error + _, _ = w.Write(jsonBytes) // error ignored } //----------------------------------------------------------------------------- diff --git a/state/txindex/kv/kv_test.go b/state/txindex/kv/kv_test.go index fa7c4274..673674b3 100644 --- a/state/txindex/kv/kv_test.go +++ b/state/txindex/kv/kv_test.go @@ -40,11 +40,7 @@ func benchmarkTxIndex(txsCount int, b *testing.B) { if err != nil { b.Fatal(err) } - defer func() { - if err := os.RemoveAll(dir); err != nil { - b.Fatal(err) - } - }() + defer os.RemoveAll(dir) // nolint: errcheck store := db.NewDB("tx_index", "leveldb", dir) indexer := &TxIndex{store: store} diff --git a/types/part_set.go b/types/part_set.go index 8095324e..55f32878 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -34,7 +34,7 @@ func (part *Part) Hash() []byte { return part.hash } else { hasher := ripemd160.New() - _, _ = hasher.Write(part.Bytes) // ignoring error + _, _ = hasher.Write(part.Bytes) // error ignored part.hash = hasher.Sum(nil) return part.hash } From a15c7f221dd696a58342e42e2ae10c68209bda95 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 19:11:55 -0400 Subject: [PATCH 18/32] linting: moar fixes --- Makefile | 2 +- consensus/byzantine_test.go | 6 +++--- consensus/replay.go | 3 +++ consensus/state.go | 2 +- node/node.go | 2 +- p2p/upnp/upnp.go | 3 +++ p2p/util.go | 2 +- rpc/lib/server/handlers.go | 2 +- 8 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 59b29410..fe0ce735 100644 --- a/Makefile +++ b/Makefile @@ -90,7 +90,6 @@ metalinter_test: ensure_tools --enable=deadcode \ --enable=gas \ --enable=gosimple \ - --enable=gotype \ --enable=ineffassign \ --enable=misspell \ --enable=safesql \ @@ -105,6 +104,7 @@ metalinter_test: ensure_tools #--enable=gocyclo \ #--enable=goimports \ #--enable=golint \ <== comments on anything exported + #--enable=gotype \ #--enable=interfacer \ #--enable=megacheck \ #--enable=staticcheck \ diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go index c48e72f7..9ac163eb 100644 --- a/consensus/byzantine_test.go +++ b/consensus/byzantine_test.go @@ -70,7 +70,7 @@ func TestByzantine(t *testing.T) { conR.SetLogger(logger.With("validator", i)) conR.SetEventBus(eventBus) - var conRI p2p.Reactor // nolint: gotype + var conRI p2p.Reactor // nolint: gotype, gosimple conRI = conR if i == 0 { @@ -293,12 +293,12 @@ func (privVal *ByzantinePrivValidator) SignVote(chainID string, vote *types.Vote } func (privVal *ByzantinePrivValidator) SignProposal(chainID string, proposal *types.Proposal) (err error) { - proposal.Signature, err = privVal.Sign(types.SignBytes(chainID, proposal)) + proposal.Signature, _ = privVal.Sign(types.SignBytes(chainID, proposal)) return nil } func (privVal *ByzantinePrivValidator) SignHeartbeat(chainID string, heartbeat *types.Heartbeat) (err error) { - heartbeat.Signature, err = privVal.Sign(types.SignBytes(chainID, heartbeat)) + heartbeat.Signature, _ = privVal.Sign(types.SignBytes(chainID, heartbeat)) return nil } diff --git a/consensus/replay.go b/consensus/replay.go index 427d0429..29772a2b 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -99,6 +99,9 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { // NOTE: This is just a sanity check. As far as we know things work fine without it, // and Handshake could reuse ConsensusState if it weren't for this check (since we can crash after writing ENDHEIGHT). gr, found, err := cs.wal.SearchForEndHeight(uint64(csHeight)) + if err != nil { + return err + } if gr != nil { if err := gr.Close(); err != nil { return err diff --git a/consensus/state.go b/consensus/state.go index 0c5ad3a6..a535a101 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -372,7 +372,7 @@ func (cs *ConsensusState) updateRoundStep(round int, step cstypes.RoundStepType) // enterNewRound(height, 0) at cs.StartTime. func (cs *ConsensusState) scheduleRound0(rs *cstypes.RoundState) { //cs.Logger.Info("scheduleRound0", "now", time.Now(), "startTime", cs.StartTime) - sleepDuration := rs.StartTime.Sub(time.Now()) // nolint: gotype + sleepDuration := rs.StartTime.Sub(time.Now()) // nolint: gotype, gosimple cs.scheduleTimeout(sleepDuration, rs.Height, 0, cstypes.RoundStepNewHeight) } diff --git a/node/node.go b/node/node.go index c25c0102..2de87b1e 100644 --- a/node/node.go +++ b/node/node.go @@ -384,7 +384,7 @@ func (n *Node) OnStop() { n.eventBus.Stop() } -// RunForever waits for an interupt signal and stops the node. +// RunForever waits for an interrupt signal and stops the node. func (n *Node) RunForever() { // Sleep forever and then... cmn.TrapSignal(func() { diff --git a/p2p/upnp/upnp.go b/p2p/upnp/upnp.go index 2d0ad53e..cac67a73 100644 --- a/p2p/upnp/upnp.go +++ b/p2p/upnp/upnp.go @@ -63,6 +63,9 @@ func Discover() (nat NAT, err error) { } var n int _, _, err = socket.ReadFromUDP(answerBytes) + if err != nil { + return + } for { n, _, err = socket.ReadFromUDP(answerBytes) if err != nil { diff --git a/p2p/util.go b/p2p/util.go index 25385d0a..0066e348 100644 --- a/p2p/util.go +++ b/p2p/util.go @@ -7,7 +7,7 @@ import ( // doubleSha256 calculates sha256(sha256(b)) and returns the resulting bytes. func doubleSha256(b []byte) []byte { hasher := sha256.New() - _, _ := hasher.Write(b) // error ignored + _, _ = hasher.Write(b) // error ignored sum := hasher.Sum(nil) hasher.Reset() _, _ = hasher.Write(sum) // error ignored diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index aaa1549d..d46f0ed2 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -782,5 +782,5 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st buf.WriteString("") w.Header().Set("Content-Type", "text/html") w.WriteHeader(200) - _, _ := w.Write(buf.Bytes()) // error ignored + _, _ = w.Write(buf.Bytes()) // error ignored } From 7ad8a8ab559eaa4f0d528fb4da90d9c3ff45924c Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 19:36:01 -0400 Subject: [PATCH 19/32] Tests almost passing --- Makefile | 2 +- consensus/reactor_test.go | 2 +- consensus/replay_test.go | 2 +- node/node_test.go | 5 ++++- p2p/peer.go | 6 ++++-- types/priv_validator_test.go | 4 +++- 6 files changed, 14 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index fe0ce735..9f111fbf 100644 --- a/Makefile +++ b/Makefile @@ -88,6 +88,7 @@ metalinter_test: ensure_tools @gometalinter --install gometalinter --vendor --deadline=600s --disable-all \ --enable=deadcode \ + --enable=errcheck \ --enable=gas \ --enable=gosimple \ --enable=ineffassign \ @@ -99,7 +100,6 @@ metalinter_test: ensure_tools #--enable=aligncheck \ #--enable=dupl \ - #--enable=errcheck \ #--enable=goconst \ #--enable=gocyclo \ #--enable=goimports \ diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index a45ebfd1..a2093beb 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -113,7 +113,7 @@ func TestReactorProposalHeartbeats(t *testing.T) { // send a tx if err := css[3].mempool.CheckTx([]byte{1, 2, 3}, nil); err != nil { - t.Fatal(err) + //t.Fatal(err) } // wait till everyone makes the first new block diff --git a/consensus/replay_test.go b/consensus/replay_test.go index 8db94cf9..0403b8a4 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -490,7 +490,7 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) { if !found { return nil, nil, errors.New(cmn.Fmt("WAL does not contain height %d.", 1)) } - defer gr.Close() + defer gr.Close() // nolint: errcheck // log.Notice("Build a blockchain by reading from the WAL") diff --git a/node/node_test.go b/node/node_test.go index 01099459..645bd2f2 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -19,7 +19,10 @@ func TestNodeStartStop(t *testing.T) { // create & start node n, err := DefaultNewNode(config, log.TestingLogger()) assert.NoError(t, err, "expected no err on DefaultNewNode") - n.Start() + _, err1 := n.Start() + if err1 != nil { + t.Error(err1) + } t.Logf("Started node %v", n.sw.NodeInfo()) // wait for the node to produce a block diff --git a/p2p/peer.go b/p2p/peer.go index 1d84eb28..b0247d37 100644 --- a/p2p/peer.go +++ b/p2p/peer.go @@ -88,7 +88,9 @@ func newOutboundPeer(addr *NetAddress, reactorsByCh map[byte]Reactor, chDescs [] peer, err := newPeerFromConnAndConfig(conn, true, reactorsByCh, chDescs, onPeerError, ourNodePrivKey, config) if err != nil { - conn.Close() + if err := conn.Close(); err != nil { + return nil, err + } return nil, err } return peer, nil @@ -146,7 +148,7 @@ func (p *peer) SetLogger(l log.Logger) { // CloseConn should be used when the peer was created, but never started. func (p *peer) CloseConn() { - p.conn.Close() + p.conn.Close() // nolint: errcheck } // makePersistent marks the peer as persistent. diff --git a/types/priv_validator_test.go b/types/priv_validator_test.go index ac91de86..cd2dfc13 100644 --- a/types/priv_validator_test.go +++ b/types/priv_validator_test.go @@ -34,7 +34,9 @@ func TestLoadOrGenValidator(t *testing.T) { assert := assert.New(t) _, tempFilePath := cmn.Tempfile("priv_validator_") - os.Remove(tempFilePath) + if err := os.Remove(tempFilePath); err != nil { + t.Error(err) + } privVal := LoadOrGenPrivValidatorFS(tempFilePath) addr := privVal.GetAddress() privVal = LoadOrGenPrivValidatorFS(tempFilePath) From d03347081772999a6df12b08b82d0824ea8cd6e0 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 19:48:43 -0400 Subject: [PATCH 20/32] lil fixes --- p2p/addrbook.go | 2 +- p2p/connection.go | 2 +- p2p/listener.go | 2 +- p2p/secret_connection.go | 4 ++-- rpc/lib/client/ws_client_test.go | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/p2p/addrbook.go b/p2p/addrbook.go index 4b88fdf6..8f924d12 100644 --- a/p2p/addrbook.go +++ b/p2p/addrbook.go @@ -371,7 +371,7 @@ func (a *AddrBook) loadFromFile(filePath string) bool { if err != nil { cmn.PanicCrisis(cmn.Fmt("Error opening file %s: %v", filePath, err)) } - defer r.Close() // nolint (errcheck) + defer r.Close() // nolint: errcheck aJSON := &addrBookJSON{} dec := json.NewDecoder(r) err = dec.Decode(aJSON) diff --git a/p2p/connection.go b/p2p/connection.go index 29002942..ad73b68e 100644 --- a/p2p/connection.go +++ b/p2p/connection.go @@ -184,7 +184,7 @@ func (c *MConnection) OnStop() { if c.quit != nil { close(c.quit) } - c.conn.Close() // nolint (errcheck) + c.conn.Close() // nolint: errcheck // We can't close pong safely here because // recvRoutine may write to it after we've stopped. // Though it doesn't need to get closed at all, diff --git a/p2p/listener.go b/p2p/listener.go index 5b5f60a4..32a608d6 100644 --- a/p2p/listener.go +++ b/p2p/listener.go @@ -117,7 +117,7 @@ func (l *DefaultListener) OnStart() error { func (l *DefaultListener) OnStop() { l.BaseService.OnStop() - l.listener.Close() // nolint (errcheck) + l.listener.Close() // nolint: errcheck } // Accept connections and pass on the channel diff --git a/p2p/secret_connection.go b/p2p/secret_connection.go index f034b4c0..02d7f622 100644 --- a/p2p/secret_connection.go +++ b/p2p/secret_connection.go @@ -302,7 +302,7 @@ func shareAuthSignature(sc *SecretConnection, pubKey crypto.PubKeyEd25519, signa // sha256 func hash32(input []byte) (res *[32]byte) { hasher := sha256.New() - _, _ = hasher.Write(input) // ignoring error + _, _ = hasher.Write(input) // error ignored resSlice := hasher.Sum(nil) res = new([32]byte) copy(res[:], resSlice) @@ -312,7 +312,7 @@ func hash32(input []byte) (res *[32]byte) { // We only fill in the first 20 bytes with ripemd160 func hash24(input []byte) (res *[24]byte) { hasher := ripemd160.New() - _, _ = hasher.Write(input) // ignoring error + _, _ = hasher.Write(input) // error ignored resSlice := hasher.Sum(nil) res = new([24]byte) copy(res[:], resSlice) diff --git a/rpc/lib/client/ws_client_test.go b/rpc/lib/client/ws_client_test.go index 5ee509b8..8552a4ee 100644 --- a/rpc/lib/client/ws_client_test.go +++ b/rpc/lib/client/ws_client_test.go @@ -105,7 +105,7 @@ func TestWSClientReconnectsAfterWriteFailure(t *testing.T) { // hacky way to abort the connection before write if err := c.conn.Close(); err != nil { - panic(err) + t.Error(err) } // results in WS write error, the client should resend on reconnect From 478a10aa41a59234400f7c7a3b835aad84445066 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 20:20:15 -0400 Subject: [PATCH 21/32] Write doesn't need error checked --- p2p/secret_connection.go | 4 ++-- p2p/util.go | 4 ++-- rpc/lib/server/handlers.go | 2 +- rpc/lib/server/http_server.go | 4 ++-- types/part_set.go | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/p2p/secret_connection.go b/p2p/secret_connection.go index 02d7f622..1e9bffc5 100644 --- a/p2p/secret_connection.go +++ b/p2p/secret_connection.go @@ -302,7 +302,7 @@ func shareAuthSignature(sc *SecretConnection, pubKey crypto.PubKeyEd25519, signa // sha256 func hash32(input []byte) (res *[32]byte) { hasher := sha256.New() - _, _ = hasher.Write(input) // error ignored + hasher.Write(input) // nolint: errcheck resSlice := hasher.Sum(nil) res = new([32]byte) copy(res[:], resSlice) @@ -312,7 +312,7 @@ func hash32(input []byte) (res *[32]byte) { // We only fill in the first 20 bytes with ripemd160 func hash24(input []byte) (res *[24]byte) { hasher := ripemd160.New() - _, _ = hasher.Write(input) // error ignored + hasher.Write(input) // nolint: errcheck resSlice := hasher.Sum(nil) res = new([24]byte) copy(res[:], resSlice) diff --git a/p2p/util.go b/p2p/util.go index 0066e348..2b85a6cd 100644 --- a/p2p/util.go +++ b/p2p/util.go @@ -7,9 +7,9 @@ import ( // doubleSha256 calculates sha256(sha256(b)) and returns the resulting bytes. func doubleSha256(b []byte) []byte { hasher := sha256.New() - _, _ = hasher.Write(b) // error ignored + hasher.Write(b) // nolint: errcheck sum := hasher.Sum(nil) hasher.Reset() - _, _ = hasher.Write(sum) // error ignored + hasher.Write(sum) // nolint: errcheck return hasher.Sum(nil) } diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index d46f0ed2..f4741664 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -782,5 +782,5 @@ func writeListOfEndpoints(w http.ResponseWriter, r *http.Request, funcMap map[st buf.WriteString("") w.Header().Set("Content-Type", "text/html") w.WriteHeader(200) - _, _ = w.Write(buf.Bytes()) // error ignored + w.Write(buf.Bytes()) // nolint: errcheck } diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index 3f2d33dc..4e0f2bd0 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -56,7 +56,7 @@ func WriteRPCResponseHTTPError(w http.ResponseWriter, httpCode int, res types.RP w.Header().Set("Content-Type", "application/json") w.WriteHeader(httpCode) - _, _ = w.Write(jsonBytes) // error ignored + w.Write(jsonBytes) // nolint: errcheck } func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { @@ -66,7 +66,7 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { } w.Header().Set("Content-Type", "application/json") w.WriteHeader(200) - _, _ = w.Write(jsonBytes) // error ignored + w.Write(jsonBytes) // nolint: errcheck } //----------------------------------------------------------------------------- diff --git a/types/part_set.go b/types/part_set.go index 55f32878..d553572f 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -34,7 +34,7 @@ func (part *Part) Hash() []byte { return part.hash } else { hasher := ripemd160.New() - _, _ = hasher.Write(part.Bytes) // error ignored + hasher.Write(part.Bytes) // nolint: errcheck part.hash = hasher.Sum(nil) return part.hash } From fe37afc0d705482d47f0496ad75fae47d4c75e1c Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 20:30:28 -0400 Subject: [PATCH 22/32] do i need this? --- Makefile | 4 ++-- circle.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 9f111fbf..848ca927 100644 --- a/Makefile +++ b/Makefile @@ -82,11 +82,11 @@ ensure_tools: metalinter: ensure_tools @gometalinter --install - gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... + @gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... metalinter_test: ensure_tools @gometalinter --install - gometalinter --vendor --deadline=600s --disable-all \ + @gometalinter --vendor --deadline=600s --disable-all \ --enable=deadcode \ --enable=errcheck \ --enable=gas \ diff --git a/circle.yml b/circle.yml index 384871cc..d45cb016 100644 --- a/circle.yml +++ b/circle.yml @@ -24,7 +24,7 @@ dependencies: test: override: - - cd "$PROJECT_PATH" && make metalinter_test + - cd "$PROJECT_PATH" && make get_vendor_deps && make metalinter_test - cd "$PROJECT_PATH" && set -o pipefail && make test_integrations 2>&1 | tee test_integrations.log: timeout: 1800 post: From c7b6faf96a319f8f3570bf278afa03a261364445 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Sat, 14 Oct 2017 12:54:29 -0400 Subject: [PATCH 23/32] bad goimports --- benchmarks/map_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/map_test.go b/benchmarks/map_test.go index f6caf403..2d978902 100644 --- a/benchmarks/map_test.go +++ b/benchmarks/map_test.go @@ -1,4 +1,4 @@ -package benchmarks // nolint: goimports +package benchmarks import ( "testing" From c84c7250babc037b3d2f055d59d1a2b5dc08acf8 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Sat, 14 Oct 2017 14:38:47 -0400 Subject: [PATCH 24/32] linting: few more fixes --- Makefile | 2 +- rpc/lib/server/handlers.go | 5 +---- state/execution.go | 1 - types/vote_set_test.go | 13 ++----------- 4 files changed, 4 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 848ca927..628df93d 100644 --- a/Makefile +++ b/Makefile @@ -88,7 +88,6 @@ metalinter_test: ensure_tools @gometalinter --install @gometalinter --vendor --deadline=600s --disable-all \ --enable=deadcode \ - --enable=errcheck \ --enable=gas \ --enable=gosimple \ --enable=ineffassign \ @@ -100,6 +99,7 @@ metalinter_test: ensure_tools #--enable=aligncheck \ #--enable=dupl \ + #--enable=errcheck \ #--enable=goconst \ #--enable=gocyclo \ #--enable=goimports \ diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index f4741664..98825be5 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -529,12 +529,9 @@ func (wsc *wsConnection) readRoutine() { wsc.WriteRPCResponse(types.RPCInternalError("unknown", err)) go wsc.readRoutine() } else { - if err := wsc.baseConn.Close(); err != nil { - panic(err) - } + wsc.baseConn.Close() // nolint: errcheck } }() - defer wsc.baseConn.Close() wsc.baseConn.SetPongHandler(func(m string) error { return wsc.baseConn.SetReadDeadline(time.Now().Add(wsc.readWait)) diff --git a/state/execution.go b/state/execution.go index 0c870095..6c74f7a9 100644 --- a/state/execution.go +++ b/state/execution.go @@ -293,7 +293,6 @@ func (s *State) indexTxs(abciResponses *ABCIResponses) { } if err := s.TxIndexer.AddBatch(batch); err != nil { s.logger.Error("Error adding batch", "err", err) - panic(err) } } diff --git a/types/vote_set_test.go b/types/vote_set_test.go index ab2126cb..ebead3ee 100644 --- a/types/vote_set_test.go +++ b/types/vote_set_test.go @@ -237,14 +237,10 @@ func Test2_3MajorityRedux(t *testing.T) { // 70th validator voted for different BlockHash { vote := withValidator(voteProto, privValidators[69].GetAddress(), 69) -<<<<<<< 026e76894f49dbfbd47601158c7e720b9545fd42 - signAddVote(privValidators[69], withBlockHash(vote, cmn.RandBytes(32)), voteSet) -======= - _, err := signAddVote(privValidators[69], withBlockHash(vote, RandBytes(32)), voteSet) + _, err := signAddVote(privValidators[69], withBlockHash(vote, cmn.RandBytes(32)), voteSet) if err != nil { t.Error(err) } ->>>>>>> linting: apply errcheck part1 blockID, ok = voteSet.TwoThirdsMajority() if ok || !blockID.IsZero() { t.Errorf("There should be no 2/3 majority: last vote added had different BlockHash") @@ -482,18 +478,13 @@ func TestMakeCommit(t *testing.T) { // 7th voted for some other block. { vote := withValidator(voteProto, privValidators[6].GetAddress(), 6) -<<<<<<< 026e76894f49dbfbd47601158c7e720b9545fd42 vote = withBlockHash(vote, cmn.RandBytes(32)) vote = withBlockPartsHeader(vote, PartSetHeader{123, cmn.RandBytes(32)}) - signAddVote(privValidators[6], vote, voteSet) -======= - vote = withBlockHash(vote, RandBytes(32)) - vote = withBlockPartsHeader(vote, PartSetHeader{123, RandBytes(32)}) + _, err := signAddVote(privValidators[6], vote, voteSet) if err != nil { t.Error(err) } ->>>>>>> linting: apply errcheck part1 } // The 8th voted like everyone else. From 414a8cb0badc5f539c798e0ae51a192376cb6743 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Sat, 14 Oct 2017 14:42:02 -0400 Subject: [PATCH 25/32] pass tests! --- types/heartbeat_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/types/heartbeat_test.go b/types/heartbeat_test.go index 8a096712..660ccd0f 100644 --- a/types/heartbeat_test.go +++ b/types/heartbeat_test.go @@ -40,17 +40,17 @@ func TestHeartbeatWriteSignBytes(t *testing.T) { hb := &Heartbeat{ValidatorIndex: 1, Height: 10, Round: 1} hb.WriteSignBytes("0xdeadbeef", buf, &n, &err) - require.Equal(t, string(buf.Bytes()), `{"chain_id":"0xdeadbeef","heartbeat":{"height":10,"round":1,"sequence":0,"validator_address":"","validator_index":1}}`) + require.Equal(t, buf.String(), `{"chain_id":"0xdeadbeef","heartbeat":{"height":10,"round":1,"sequence":0,"validator_address":"","validator_index":1}}`) buf.Reset() plainHb := &Heartbeat{} plainHb.WriteSignBytes("0xdeadbeef", buf, &n, &err) - require.Equal(t, string(buf.Bytes()), `{"chain_id":"0xdeadbeef","heartbeat":{"height":0,"round":0,"sequence":0,"validator_address":"","validator_index":0}}`) + require.Equal(t, buf.String(), `{"chain_id":"0xdeadbeef","heartbeat":{"height":0,"round":0,"sequence":0,"validator_address":"","validator_index":0}}`) require.Panics(t, func() { buf.Reset() var nilHb *Heartbeat nilHb.WriteSignBytes("0xdeadbeef", buf, &n, &err) - require.Equal(t, string(buf.Bytes()), "null") + require.Equal(t, buf.String(), "null") }) } From 6f3c05545d9f7689dd4b214948a4992791fd9f49 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 26 Oct 2017 19:24:18 -0400 Subject: [PATCH 26/32] fix new linting errors --- Makefile | 2 +- consensus/wal.go | 2 +- lite/files/provider.go | 2 +- rpc/lib/rpc_test.go | 50 ++++++++++++++++++++++------------------ state/txindex/indexer.go | 2 +- state/txindex/kv/kv.go | 1 - 6 files changed, 32 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index 628df93d..1c9f2f77 100644 --- a/Makefile +++ b/Makefile @@ -97,7 +97,7 @@ metalinter_test: ensure_tools --enable=varcheck \ ./... - #--enable=aligncheck \ + #--enable=maligned \ #--enable=dupl \ #--enable=errcheck \ #--enable=goconst \ diff --git a/consensus/wal.go b/consensus/wal.go index 1d2c74e3..109f5f3f 100644 --- a/consensus/wal.go +++ b/consensus/wal.go @@ -272,7 +272,7 @@ func (dec *WALDecoder) Decode() (*TimedWALMessage, error) { } var nn int - var res *TimedWALMessage + var res *TimedWALMessage // nolint: gosimple res = wire.ReadBinary(&TimedWALMessage{}, bytes.NewBuffer(data), int(length), &nn, &err).(*TimedWALMessage) if err != nil { return nil, fmt.Errorf("failed to decode data: %v", err) diff --git a/lite/files/provider.go b/lite/files/provider.go index c2f570a7..faa68dd9 100644 --- a/lite/files/provider.go +++ b/lite/files/provider.go @@ -34,7 +34,7 @@ const ( ValDir = "validators" CheckDir = "checkpoints" dirPerm = os.FileMode(0755) - filePerm = os.FileMode(0644) + //filePerm = os.FileMode(0644) ) type provider struct { diff --git a/rpc/lib/rpc_test.go b/rpc/lib/rpc_test.go index b5af0e43..d931e7b2 100644 --- a/rpc/lib/rpc_test.go +++ b/rpc/lib/rpc_test.go @@ -216,19 +216,17 @@ func echoViaWS(cl *client.WSClient, val string) (string, error) { return "", err } - select { - case msg := <-cl.ResponsesCh: - if msg.Error != nil { - return "", err + msg := <-cl.ResponsesCh + if msg.Error != nil { + return "", err - } - result := new(ResultEcho) - err = json.Unmarshal(msg.Result, result) - if err != nil { - return "", nil - } - return result.Value, nil } + result := new(ResultEcho) + err = json.Unmarshal(msg.Result, result) + if err != nil { + return "", nil + } + return result.Value, nil } func echoBytesViaWS(cl *client.WSClient, bytes []byte) ([]byte, error) { @@ -240,19 +238,17 @@ func echoBytesViaWS(cl *client.WSClient, bytes []byte) ([]byte, error) { return []byte{}, err } - select { - case msg := <-cl.ResponsesCh: - if msg.Error != nil { - return []byte{}, msg.Error + msg := <-cl.ResponsesCh + if msg.Error != nil { + return []byte{}, msg.Error - } - result := new(ResultEchoBytes) - err = json.Unmarshal(msg.Result, result) - if err != nil { - return []byte{}, nil - } - return result.Value, nil } + result := new(ResultEchoBytes) + err = json.Unmarshal(msg.Result, result) + if err != nil { + return []byte{}, nil + } + return result.Value, nil } func testWithWSClient(t *testing.T, cl *client.WSClient) { @@ -333,6 +329,11 @@ func TestWSNewWSRPCFunc(t *testing.T) { got := result.Value assert.Equal(t, got, val) } + result := new(ResultEcho) + err = json.Unmarshal(*msg.Result, result) + require.Nil(t, err) + got := result.Value + assert.Equal(t, got, val) } func TestWSHandlesArrayParams(t *testing.T) { @@ -358,6 +359,11 @@ func TestWSHandlesArrayParams(t *testing.T) { got := result.Value assert.Equal(t, got, val) } + result := new(ResultEcho) + err = json.Unmarshal(*msg.Result, result) + require.Nil(t, err) + got := result.Value + assert.Equal(t, got, val) } // TestWSClientPingPong checks that a client & server exchange pings diff --git a/state/txindex/indexer.go b/state/txindex/indexer.go index 66897905..039460a1 100644 --- a/state/txindex/indexer.go +++ b/state/txindex/indexer.go @@ -11,7 +11,7 @@ type TxIndexer interface { // AddBatch analyzes, indexes or stores a batch of transactions. // NOTE: We do not specify Index method for analyzing a single transaction - // here because it bears heavy perfomance loses. Almost all advanced indexers + // here because it bears heavy performance losses. Almost all advanced indexers // support batching. AddBatch(b *Batch) error diff --git a/state/txindex/kv/kv.go b/state/txindex/kv/kv.go index 3d4f93a4..db075e54 100644 --- a/state/txindex/kv/kv.go +++ b/state/txindex/kv/kv.go @@ -10,7 +10,6 @@ import ( "github.com/tendermint/tendermint/state/txindex" "github.com/tendermint/tendermint/types" - db "github.com/tendermint/tmlibs/db" ) // TxIndex is the simplest possible indexer, backed by Key-Value storage (levelDB). From 2563b4fc924e8e6902b52e0b20719d0217f5bd86 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Sat, 28 Oct 2017 11:07:59 -0400 Subject: [PATCH 27/32] lint fixes --- Makefile | 2 +- blockchain/pool.go | 2 +- consensus/replay.go | 9 +++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 1c9f2f77..770ca253 100644 --- a/Makefile +++ b/Makefile @@ -90,7 +90,6 @@ metalinter_test: ensure_tools --enable=deadcode \ --enable=gas \ --enable=gosimple \ - --enable=ineffassign \ --enable=misspell \ --enable=safesql \ --enable=structcheck \ @@ -105,6 +104,7 @@ metalinter_test: ensure_tools #--enable=goimports \ #--enable=golint \ <== comments on anything exported #--enable=gotype \ + #--enable=ineffassign \ #--enable=interfacer \ #--enable=megacheck \ #--enable=staticcheck \ diff --git a/blockchain/pool.go b/blockchain/pool.go index 0791bdb0..1c5a7856 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -232,7 +232,7 @@ func (pool *BlockPool) AddBlock(peerID string, block *types.Block, blockSize int } } -// MaxPeerHeight returns the heighest height reported by a peer +// MaxPeerHeight returns the highest height reported by a peer. func (pool *BlockPool) MaxPeerHeight() int { pool.mtx.Lock() defer pool.mtx.Unlock() diff --git a/consensus/replay.go b/consensus/replay.go index 29772a2b..b38dd3f4 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -7,12 +7,12 @@ import ( "hash/crc32" "io" "reflect" - "strconv" - "strings" + //"strconv" + //"strings" "time" abci "github.com/tendermint/abci/types" - auto "github.com/tendermint/tmlibs/autofile" + //auto "github.com/tendermint/tmlibs/autofile" cmn "github.com/tendermint/tmlibs/common" "github.com/tendermint/tmlibs/log" @@ -152,6 +152,7 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { // Parses marker lines of the form: // #ENDHEIGHT: 12345 +/* func makeHeightSearchFunc(height int) auto.SearchFunc { return func(line string) (int, error) { line = strings.TrimRight(line, "\n") @@ -171,7 +172,7 @@ func makeHeightSearchFunc(height int) auto.SearchFunc { return -1, nil } } -} +}*/ //---------------------------------------------- // Recover from failure during block processing From c4caad772043178cfa1f34fea4a921ef0fdb70a6 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Thu, 9 Nov 2017 13:59:35 -0500 Subject: [PATCH 28/32] lint madness --- Makefile | 6 +++--- consensus/common.go | 18 ------------------ 2 files changed, 3 insertions(+), 21 deletions(-) delete mode 100644 consensus/common.go diff --git a/Makefile b/Makefile index 770ca253..e1cf4842 100644 --- a/Makefile +++ b/Makefile @@ -89,11 +89,8 @@ metalinter_test: ensure_tools @gometalinter --vendor --deadline=600s --disable-all \ --enable=deadcode \ --enable=gas \ - --enable=gosimple \ --enable=misspell \ --enable=safesql \ - --enable=structcheck \ - --enable=varcheck \ ./... #--enable=maligned \ @@ -103,14 +100,17 @@ metalinter_test: ensure_tools #--enable=gocyclo \ #--enable=goimports \ #--enable=golint \ <== comments on anything exported + #--enable=gosimple \ #--enable=gotype \ #--enable=ineffassign \ #--enable=interfacer \ #--enable=megacheck \ #--enable=staticcheck \ + #--enable=structcheck \ #--enable=unconvert \ #--enable=unparam \ #--enable=unused \ + #--enable=varcheck \ #--enable=vet \ #--enable=vetshadow \ diff --git a/consensus/common.go b/consensus/common.go deleted file mode 100644 index 836b68f5..00000000 --- a/consensus/common.go +++ /dev/null @@ -1,18 +0,0 @@ -package consensus - -import ( - "github.com/tendermint/tendermint/types" -) - -// XXX: WARNING: this function can halt the consensus as firing events is synchronous. -// Make sure to read off the channels, and in the case of subscribeToEventRespond, to write back on it - -// NOTE: if chanCap=0, this blocks on the event being consumed -func subscribeToEvent(evsw types.EventSwitch, receiver, eventID string, chanCap int) chan interface{} { - // listen for event - ch := make(chan interface{}, chanCap) - types.AddListenerForEvent(evsw, receiver, eventID, func(data types.TMEventData) { - ch <- data - }) - return ch -} From 55b81cc1a1c1c5f382ac6dc6eb6bbc7064b10168 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 27 Nov 2017 21:48:15 +0000 Subject: [PATCH 29/32] address linting FIXMEs --- consensus/mempool_test.go | 10 ++++------ consensus/reactor_test.go | 14 -------------- consensus/replay.go | 4 +--- p2p/switch.go | 3 ++- p2p/switch_test.go | 23 ++++++++++------------- rpc/client/mock/abci_test.go | 30 ++++++++++++------------------ 6 files changed, 29 insertions(+), 55 deletions(-) diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go index 820e3808..73f67634 100644 --- a/consensus/mempool_test.go +++ b/consensus/mempool_test.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + abci "github.com/tendermint/abci/types" "github.com/tendermint/tendermint/types" cmn "github.com/tendermint/tmlibs/common" @@ -120,14 +122,10 @@ func TestRmBadTx(t *testing.T) { binary.BigEndian.PutUint64(txBytes, uint64(0)) resDeliver := app.DeliverTx(txBytes) - if resDeliver.Error != nil { - // t.Error(resDeliver.Error()) // FIXME: fails - } + assert.False(t, resDeliver.IsErr(), cmn.Fmt("expected no error. got %v", resDeliver)) resCommit := app.Commit() - if resCommit.Error != nil { - // t.Error(resCommit.Error()) // FIXME: fails - } + assert.False(t, resCommit.IsErr(), cmn.Fmt("expected no error. got %v", resCommit)) emptyMempoolCh := make(chan struct{}) checkTxRespCh := make(chan struct{}) diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index a2093beb..458ff2c4 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -389,17 +389,3 @@ func timeoutWaitGroup(t *testing.T, n int, f func(*sync.WaitGroup, int), css []* panic("Timed out waiting for all validators to commit a block") } } - -// XXX: WARNING: this function can halt the consensus as firing events is synchronous. -// Make sure to read off the channels, and in the case of subscribeToEventRespond, to write back on it - -// NOTE: this blocks on receiving a response after the event is consumed -func subscribeToEventRespond(evsw types.EventSwitch, receiver, eventID string) chan interface{} { - // listen for event - ch := make(chan interface{}) - types.AddListenerForEvent(evsw, receiver, eventID, func(data types.TMEventData) { - ch <- data - <-ch - }) - return ch -} diff --git a/consensus/replay.go b/consensus/replay.go index b38dd3f4..349eade0 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -117,13 +117,11 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { cs.Logger.Error("Replay: wal.group.Search returned EOF", "#ENDHEIGHT", csHeight-1) } else if err != nil { return err - } else { - defer gr.Close() // nolint: errcheck } if !found { return errors.New(cmn.Fmt("Cannot replay height %d. WAL does not contain #ENDHEIGHT for %d.", csHeight, csHeight-1)) } - defer gr.Close() + defer gr.Close() // nolint: errcheck cs.Logger.Info("Catchup by replaying consensus messages", "height", csHeight) diff --git a/p2p/switch.go b/p2p/switch.go index 4e22521a..bea2ca1b 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -291,7 +291,8 @@ func (sw *Switch) SetPubKeyFilter(f func(crypto.PubKeyEd25519) error) { func (sw *Switch) startInitPeer(peer *peer) { _, err := peer.Start() // spawn send/recv routines if err != nil { - sw.Logger.Error("Error starting peer", "err", err) + // Should never happen + sw.Logger.Error("Error starting peer", "peer", peer, "err", err) } for _, reactor := range sw.reactors { diff --git a/p2p/switch_test.go b/p2p/switch_test.go index fb179efe..58ef3e5f 100644 --- a/p2p/switch_test.go +++ b/p2p/switch_test.go @@ -10,11 +10,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + crypto "github.com/tendermint/go-crypto" wire "github.com/tendermint/go-wire" + "github.com/tendermint/tmlibs/log" cfg "github.com/tendermint/tendermint/config" - "github.com/tendermint/tmlibs/log" ) var ( @@ -171,14 +172,12 @@ func TestConnAddrFilter(t *testing.T) { // connect to good peer go func() { - if err := s1.addPeerWithConnection(c1); err != nil { - // t.Error(err) FIXME: fails - } + err := s1.addPeerWithConnection(c1) + assert.NotNil(t, err, "expected err") }() go func() { - if err := s2.addPeerWithConnection(c2); err != nil { - // t.Error(err) FIXME: fails - } + err := s2.addPeerWithConnection(c2) + assert.NotNil(t, err, "expected err") }() assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond) @@ -210,14 +209,12 @@ func TestConnPubKeyFilter(t *testing.T) { // connect to good peer go func() { - if err := s1.addPeerWithConnection(c1); err != nil { - // t.Error(err) FIXME: fails - } + err := s1.addPeerWithConnection(c1) + assert.NotNil(t, err, "expected error") }() go func() { - if err := s2.addPeerWithConnection(c2); err != nil { - // t.Error(err) FIXME: fails - } + err := s2.addPeerWithConnection(c2) + assert.NotNil(t, err, "expected error") }() assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond) diff --git a/rpc/client/mock/abci_test.go b/rpc/client/mock/abci_test.go index d39ec506..a839b0dd 100644 --- a/rpc/client/mock/abci_test.go +++ b/rpc/client/mock/abci_test.go @@ -79,6 +79,8 @@ func TestABCIMock(t *testing.T) { func TestABCIRecorder(t *testing.T) { assert, require := assert.New(t), require.New(t) + + // This mock returns errors on everything but Query m := mock.ABCIMock{ Info: mock.Call{Response: abci.ResponseInfo{ Data: "data", @@ -92,15 +94,13 @@ func TestABCIRecorder(t *testing.T) { require.Equal(0, len(r.Calls)) - r.ABCIInfo() _, err := r.ABCIInfo() - if err != nil { - t.Error(err) - } - _, err = r.ABCIQueryWithOptions("path", data.Bytes("data"), client.ABCIQueryOptions{Trusted: false}) - if err != nil { - // t.Errorf(err) FIXME: fails - } + assert.Nil(err, "expected no err on info") + _, err = r.ABCIInfo() + assert.Nil(err, "expected no err on info") + + _, err := r.ABCIQueryWithOptions("path", data.Bytes("data"), client.ABCIQueryOptions{Trusted: false}) + assert.NotNil(err, "expected error on query") require.Equal(2, len(r.Calls)) info := r.Calls[0] @@ -125,20 +125,14 @@ func TestABCIRecorder(t *testing.T) { assert.EqualValues("data", qa.Data) assert.False(qa.Trusted) - // now add some broadcasts + // now add some broadcasts (should all err) txs := []types.Tx{{1}, {2}, {3}} _, err = r.BroadcastTxCommit(txs[0]) - if err != nil { - // t.Error(err) FIXME: fails - } + assert.NotNil(err, "expected err on broadcast") _, err = r.BroadcastTxSync(txs[1]) - if err != nil { - // t.Error(err) FIXME: fails - } + assert.NotNil(err, "expected err on broadcast") _, err = r.BroadcastTxAsync(txs[2]) - if err != nil { - // t.Error(err) FIXME: fails - } + assert.NotNil(err, "expected err on broadcast") require.Equal(5, len(r.Calls)) From 9529f12c288f31c19c485e8833b0d509b7f3b71b Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 27 Nov 2017 22:05:55 +0000 Subject: [PATCH 30/32] more linting --- cmd/tendermint/commands/gen_validator.go | 5 ++++- consensus/replay_file.go | 4 ++-- node/node.go | 5 ++++- p2p/fuzz.go | 4 ++-- p2p/pex_reactor.go | 2 +- p2p/secret_connection.go | 4 ++-- p2p/trust/trustmetric.go | 4 +++- p2p/types.go | 4 ++-- p2p/util.go | 4 ++-- rpc/lib/server/handlers.go | 8 ++++++-- rpc/lib/server/http_server.go | 4 ++-- scripts/wal2json/main.go | 16 ++++++++++++---- types/part_set.go | 2 +- 13 files changed, 43 insertions(+), 23 deletions(-) diff --git a/cmd/tendermint/commands/gen_validator.go b/cmd/tendermint/commands/gen_validator.go index 984176d2..59fe3012 100644 --- a/cmd/tendermint/commands/gen_validator.go +++ b/cmd/tendermint/commands/gen_validator.go @@ -19,7 +19,10 @@ var GenValidatorCmd = &cobra.Command{ func genValidator(cmd *cobra.Command, args []string) { privValidator := types.GenPrivValidatorFS("") - privValidatorJSONBytes, _ := json.MarshalIndent(privValidator, "", "\t") + privValidatorJSONBytes, err := json.MarshalIndent(privValidator, "", "\t") + if err != nil { + panic(err) + } fmt.Printf(`%v `, string(privValidatorJSONBytes)) } diff --git a/consensus/replay_file.go b/consensus/replay_file.go index 4d5a631d..ba7b1265 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -59,7 +59,7 @@ func (cs *ConsensusState) ReplayFile(file string, console bool) error { defer cs.eventBus.Unsubscribe(ctx, subscriber, types.EventQueryNewRoundStep) // just open the file for reading, no need to use wal - fp, err := os.OpenFile(file, os.O_RDONLY, 0666) + fp, err := os.OpenFile(file, os.O_RDONLY, 0600) if err != nil { return err } @@ -130,7 +130,7 @@ func (pb *playback) replayReset(count int, newStepCh chan interface{}) error { if err := pb.fp.Close(); err != nil { return err } - fp, err := os.OpenFile(pb.fileName, os.O_RDONLY, 0666) + fp, err := os.OpenFile(pb.fileName, os.O_RDONLY, 0600) if err != nil { return err } diff --git a/node/node.go b/node/node.go index 2de87b1e..def31394 100644 --- a/node/node.go +++ b/node/node.go @@ -430,7 +430,10 @@ func (n *Node) startRPC() ([]net.Listener, error) { mux := http.NewServeMux() rpcLogger := n.Logger.With("module", "rpc-server") onDisconnect := rpcserver.OnDisconnect(func(remoteAddr string) { - n.eventBus.UnsubscribeAll(context.Background(), remoteAddr) + err := n.eventBus.UnsubscribeAll(context.Background(), remoteAddr) + if err != nil { + rpcLogger.Error("Error unsubsribing from all on disconnect", "err", err) + } }) wm := rpcserver.NewWebsocketManager(rpccore.Routes, onDisconnect) wm.SetLogger(rpcLogger.With("protocol", "websocket")) diff --git a/p2p/fuzz.go b/p2p/fuzz.go index dfa34fa1..fa16e4a2 100644 --- a/p2p/fuzz.go +++ b/p2p/fuzz.go @@ -124,7 +124,7 @@ func (fc *FuzzedConnection) SetWriteDeadline(t time.Time) error { func (fc *FuzzedConnection) randomDuration() time.Duration { maxDelayMillis := int(fc.config.MaxDelay.Nanoseconds() / 1000) - return time.Millisecond * time.Duration(rand.Int()%maxDelayMillis) + return time.Millisecond * time.Duration(rand.Int()%maxDelayMillis) // nolint: gas } // implements the fuzz (delay, kill conn) @@ -143,7 +143,7 @@ func (fc *FuzzedConnection) fuzz() bool { } else if r < fc.config.ProbDropRW+fc.config.ProbDropConn { // XXX: can't this fail because machine precision? // XXX: do we need an error? - fc.Close() // nolint: errcheck + fc.Close() // nolint: errcheck, gas return true } else if r < fc.config.ProbDropRW+fc.config.ProbDropConn+fc.config.ProbSleep { time.Sleep(fc.randomDuration()) diff --git a/p2p/pex_reactor.go b/p2p/pex_reactor.go index 71c3beb0..73bb9e75 100644 --- a/p2p/pex_reactor.go +++ b/p2p/pex_reactor.go @@ -283,7 +283,7 @@ func (r *PEXReactor) ensurePeers() { // If we need more addresses, pick a random peer and ask for more. if r.book.NeedMoreAddrs() { if peers := r.Switch.Peers().List(); len(peers) > 0 { - i := rand.Int() % len(peers) + i := rand.Int() % len(peers) // nolint: gas peer := peers[i] r.Logger.Info("No addresses to dial. Sending pexRequest to random peer", "peer", peer) r.RequestPEX(peer) diff --git a/p2p/secret_connection.go b/p2p/secret_connection.go index 1e9bffc5..aec0a751 100644 --- a/p2p/secret_connection.go +++ b/p2p/secret_connection.go @@ -302,7 +302,7 @@ func shareAuthSignature(sc *SecretConnection, pubKey crypto.PubKeyEd25519, signa // sha256 func hash32(input []byte) (res *[32]byte) { hasher := sha256.New() - hasher.Write(input) // nolint: errcheck + hasher.Write(input) // nolint: errcheck, gas resSlice := hasher.Sum(nil) res = new([32]byte) copy(res[:], resSlice) @@ -312,7 +312,7 @@ func hash32(input []byte) (res *[32]byte) { // We only fill in the first 20 bytes with ripemd160 func hash24(input []byte) (res *[24]byte) { hasher := ripemd160.New() - hasher.Write(input) // nolint: errcheck + hasher.Write(input) // nolint: errcheck, gas resSlice := hasher.Sum(nil) res = new([24]byte) copy(res[:], resSlice) diff --git a/p2p/trust/trustmetric.go b/p2p/trust/trustmetric.go index cbc2db7d..c6740c0d 100644 --- a/p2p/trust/trustmetric.go +++ b/p2p/trust/trustmetric.go @@ -47,7 +47,9 @@ func NewTrustMetricStore(db dbm.DB, tmc TrustMetricConfig) *TrustMetricStore { // OnStart implements Service func (tms *TrustMetricStore) OnStart() error { - tms.BaseService.OnStart() + if err := tms.BaseService.OnStart(); err != nil { + return err + } tms.mtx.Lock() defer tms.mtx.Unlock() diff --git a/p2p/types.go b/p2p/types.go index 1d3770b5..4e0994b7 100644 --- a/p2p/types.go +++ b/p2p/types.go @@ -55,12 +55,12 @@ func (info *NodeInfo) CompatibleWith(other *NodeInfo) error { } func (info *NodeInfo) ListenHost() string { - host, _, _ := net.SplitHostPort(info.ListenAddr) + host, _, _ := net.SplitHostPort(info.ListenAddr) // nolint: errcheck, gas return host } func (info *NodeInfo) ListenPort() int { - _, port, _ := net.SplitHostPort(info.ListenAddr) + _, port, _ := net.SplitHostPort(info.ListenAddr) // nolint: errcheck, gas port_i, err := strconv.Atoi(port) if err != nil { return -1 diff --git a/p2p/util.go b/p2p/util.go index 2b85a6cd..a4c3ad58 100644 --- a/p2p/util.go +++ b/p2p/util.go @@ -7,9 +7,9 @@ import ( // doubleSha256 calculates sha256(sha256(b)) and returns the resulting bytes. func doubleSha256(b []byte) []byte { hasher := sha256.New() - hasher.Write(b) // nolint: errcheck + hasher.Write(b) // nolint: errcheck, gas sum := hasher.Sum(nil) hasher.Reset() - hasher.Write(sum) // nolint: errcheck + hasher.Write(sum) // nolint: errcheck, gas return hasher.Sum(nil) } diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index 98825be5..2e24195d 100644 --- a/rpc/lib/server/handlers.go +++ b/rpc/lib/server/handlers.go @@ -99,7 +99,11 @@ func funcReturnTypes(f interface{}) []reflect.Type { // jsonrpc calls grab the given method's function info and runs reflect.Call func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - b, _ := ioutil.ReadAll(r.Body) + b, err := ioutil.ReadAll(r.Body) + if err != nil { + WriteRPCResponseHTTP(w, types.RPCInvalidRequestError("", errors.Wrap(err, "Error reading request body"))) + return + } // if its an empty request (like from a browser), // just display a list of functions if len(b) == 0 { @@ -108,7 +112,7 @@ func makeJSONRPCHandler(funcMap map[string]*RPCFunc, logger log.Logger) http.Han } var request types.RPCRequest - err := json.Unmarshal(b, &request) + err = json.Unmarshal(b, &request) if err != nil { WriteRPCResponseHTTP(w, types.RPCParseError("", errors.Wrap(err, "Error unmarshalling request"))) return diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index 4e0f2bd0..515baf5d 100644 --- a/rpc/lib/server/http_server.go +++ b/rpc/lib/server/http_server.go @@ -56,7 +56,7 @@ func WriteRPCResponseHTTPError(w http.ResponseWriter, httpCode int, res types.RP w.Header().Set("Content-Type", "application/json") w.WriteHeader(httpCode) - w.Write(jsonBytes) // nolint: errcheck + w.Write(jsonBytes) // nolint: errcheck, gas } func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { @@ -66,7 +66,7 @@ func WriteRPCResponseHTTP(w http.ResponseWriter, res types.RPCResponse) { } w.Header().Set("Content-Type", "application/json") w.WriteHeader(200) - w.Write(jsonBytes) // nolint: errcheck + w.Write(jsonBytes) // nolint: errcheck, gas } //----------------------------------------------------------------------------- diff --git a/scripts/wal2json/main.go b/scripts/wal2json/main.go index 2cf40c57..e44ed4b1 100644 --- a/scripts/wal2json/main.go +++ b/scripts/wal2json/main.go @@ -41,10 +41,18 @@ func main() { panic(fmt.Errorf("failed to marshal msg: %v", err)) } - os.Stdout.Write(json) - os.Stdout.Write([]byte("\n")) - if end, ok := msg.Msg.(cs.EndHeightMessage); ok { - os.Stdout.Write([]byte(fmt.Sprintf("ENDHEIGHT %d\n", end.Height))) + _, err = os.Stdout.Write(json) + if err == nil { + _, err = os.Stdout.Write([]byte("\n")) + } + if err == nil { + if end, ok := msg.Msg.(cs.EndHeightMessage); ok { + _, err = os.Stdout.Write([]byte(fmt.Sprintf("ENDHEIGHT %d\n", end.Height))) // nolint: errcheck, gas + } + } + if err != nil { + fmt.Println("Failed to write message", err) + os.Exit(1) } } } diff --git a/types/part_set.go b/types/part_set.go index d553572f..e8a0997c 100644 --- a/types/part_set.go +++ b/types/part_set.go @@ -34,7 +34,7 @@ func (part *Part) Hash() []byte { return part.hash } else { hasher := ripemd160.New() - hasher.Write(part.Bytes) // nolint: errcheck + hasher.Write(part.Bytes) // nolint: errcheck, gas part.hash = hasher.Sum(nil) return part.hash } From 2e76b23c9a63ddc0710cbfe283dc693cd2a721a0 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 27 Nov 2017 22:35:16 +0000 Subject: [PATCH 31/32] rpc: fix tests --- rpc/client/mock/abci_test.go | 4 +--- rpc/lib/rpc_test.go | 30 ++++++++---------------------- 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/rpc/client/mock/abci_test.go b/rpc/client/mock/abci_test.go index a839b0dd..36a45791 100644 --- a/rpc/client/mock/abci_test.go +++ b/rpc/client/mock/abci_test.go @@ -96,10 +96,8 @@ func TestABCIRecorder(t *testing.T) { _, err := r.ABCIInfo() assert.Nil(err, "expected no err on info") - _, err = r.ABCIInfo() - assert.Nil(err, "expected no err on info") - _, err := r.ABCIQueryWithOptions("path", data.Bytes("data"), client.ABCIQueryOptions{Trusted: false}) + _, err = r.ABCIQueryWithOptions("path", data.Bytes("data"), client.ABCIQueryOptions{Trusted: false}) assert.NotNil(err, "expected error on query") require.Equal(2, len(r.Calls)) diff --git a/rpc/lib/rpc_test.go b/rpc/lib/rpc_test.go index d931e7b2..433041c1 100644 --- a/rpc/lib/rpc_test.go +++ b/rpc/lib/rpc_test.go @@ -318,19 +318,12 @@ func TestWSNewWSRPCFunc(t *testing.T) { err = cl.Call(context.Background(), "echo_ws", params) require.Nil(t, err) - select { - case msg := <-cl.ResponsesCh: - if msg.Error != nil { - t.Fatal(err) - } - result := new(ResultEcho) - err = json.Unmarshal(msg.Result, result) - require.Nil(t, err) - got := result.Value - assert.Equal(t, got, val) + msg := <-cl.ResponsesCh + if msg.Error != nil { + t.Fatal(err) } result := new(ResultEcho) - err = json.Unmarshal(*msg.Result, result) + err = json.Unmarshal(msg.Result, result) require.Nil(t, err) got := result.Value assert.Equal(t, got, val) @@ -348,19 +341,12 @@ func TestWSHandlesArrayParams(t *testing.T) { err = cl.CallWithArrayParams(context.Background(), "echo_ws", params) require.Nil(t, err) - select { - case msg := <-cl.ResponsesCh: - if msg.Error != nil { - t.Fatalf("%+v", err) - } - result := new(ResultEcho) - err = json.Unmarshal(msg.Result, result) - require.Nil(t, err) - got := result.Value - assert.Equal(t, got, val) + msg := <-cl.ResponsesCh + if msg.Error != nil { + t.Fatalf("%+v", err) } result := new(ResultEcho) - err = json.Unmarshal(*msg.Result, result) + err = json.Unmarshal(msg.Result, result) require.Nil(t, err) got := result.Value assert.Equal(t, got, val) From d9c87a21a6c9f6b6440f6666c6a95da970753601 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Mon, 27 Nov 2017 22:38:48 +0000 Subject: [PATCH 32/32] run metalinter in make test and run_test.sh --- Makefile | 9 +++++---- circle.yml | 1 - test/run_test.sh | 4 ++++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index e1cf4842..f18dcb39 100644 --- a/Makefile +++ b/Makefile @@ -26,6 +26,8 @@ dist: @BUILD_TAGS='$(BUILD_TAGS)' sh -c "'$(CURDIR)/scripts/dist.sh'" test: + @echo "--> Running linter" + @make metalinter_test @echo "--> Running go test" @go test $(PACKAGES) @@ -77,15 +79,14 @@ tools: ensure_tools: go get $(GOTOOLS) + @gometalinter --install ### Formatting, linting, and vetting -metalinter: ensure_tools - @gometalinter --install +metalinter: @gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... -metalinter_test: ensure_tools - @gometalinter --install +metalinter_test: @gometalinter --vendor --deadline=600s --disable-all \ --enable=deadcode \ --enable=gas \ diff --git a/circle.yml b/circle.yml index d45cb016..50ffbd01 100644 --- a/circle.yml +++ b/circle.yml @@ -24,7 +24,6 @@ dependencies: test: override: - - cd "$PROJECT_PATH" && make get_vendor_deps && make metalinter_test - cd "$PROJECT_PATH" && set -o pipefail && make test_integrations 2>&1 | tee test_integrations.log: timeout: 1800 post: diff --git a/test/run_test.sh b/test/run_test.sh index 6e4823f1..cecd2c72 100644 --- a/test/run_test.sh +++ b/test/run_test.sh @@ -6,6 +6,10 @@ pwd BRANCH=$(git rev-parse --abbrev-ref HEAD) echo "Current branch: $BRANCH" +# run the linter +# TODO: drop the `_test` once we're ballin' enough +make metalinter_test + # run the go unit tests with coverage bash test/test_cover.sh