diff --git a/Makefile b/Makefile index 2271abebf..f18dcb392 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 @@ -25,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) @@ -76,11 +79,40 @@ tools: ensure_tools: go get $(GOTOOLS) + @gometalinter --install ### Formatting, linting, and vetting -megacheck: - @for pkg in ${PACKAGES}; do megacheck "$$pkg"; done - +metalinter: + @gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... + +metalinter_test: + @gometalinter --vendor --deadline=600s --disable-all \ + --enable=deadcode \ + --enable=gas \ + --enable=misspell \ + --enable=safesql \ + ./... + + #--enable=maligned \ + #--enable=dupl \ + #--enable=errcheck \ + #--enable=goconst \ + #--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 \ .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/codec_test.go b/benchmarks/codec_test.go index 7162e63d0..3650d281e 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/benchmarks/os_test.go b/benchmarks/os_test.go index 9c8fae656..dfadc3128 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/benchmarks/proto/test.pb.go b/benchmarks/proto/test.pb.go index 6539cae32..dc21a2a82 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/blockchain/pool.go b/blockchain/pool.go index 47e59711e..1c5a78565 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() @@ -311,7 +311,10 @@ func (pool *BlockPool) makeNextRequester() { pool.requesters[nextHeight] = request pool.numPending++ - request.Start() + _, err := request.Start() + if err != nil { + request.Logger.Error("Error starting request", "err", err) + } } func (pool *BlockPool) sendRequest(height int, peerID string) { diff --git a/blockchain/pool_test.go b/blockchain/pool_test.go index a1fce2da8..5c4c8aa3b 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 64e5e9377..4d20e777e 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 { @@ -108,7 +110,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, @@ -226,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/blockchain/store.go b/blockchain/store.go index 5bf854775..bcd10856f 100644 --- a/blockchain/store.go +++ b/blockchain/store.go @@ -7,9 +7,9 @@ import ( "io" "sync" - wire "github.com/tendermint/go-wire" + "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/gen_validator.go b/cmd/tendermint/commands/gen_validator.go index 984176d2a..59fe30128 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/cmd/tendermint/commands/init.go b/cmd/tendermint/commands/init.go index cbafac3ef..e8f22eb1c 100644 --- a/cmd/tendermint/commands/init.go +++ b/cmd/tendermint/commands/init.go @@ -28,12 +28,14 @@ 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, }} - 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 b9c08715f..513365237 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) } diff --git a/cmd/tendermint/commands/root_test.go b/cmd/tendermint/commands/root_test.go index 7c3bf801b..b4e30d980 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 ac6f337a9..2c859df2b 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 86ca1531d..a46f227c5 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 5dcbe5332..ec70ab75d 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 { + cmn.PanicSanity(err.Error()) + } + if err := cmn.EnsureDir(rootDir+"/data", 0700); err != nil { + cmn.PanicSanity(err.Error()) + } 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 { + cmn.PanicSanity(err.Error()) + } + if err := cmn.EnsureDir(rootDir+"/data", 0700); err != nil { + cmn.PanicSanity(err.Error()) + } configFilePath := path.Join(rootDir, "config.toml") genesisFilePath := path.Join(rootDir, "genesis.json") diff --git a/config/toml_test.go b/config/toml_test.go index d8f372aee..bf3bf58f7 100644 --- a/config/toml_test.go +++ b/config/toml_test.go @@ -24,7 +24,7 @@ func TestEnsureRoot(t *testing.T) { // setup temp dir for test tmpDir, err := ioutil.TempDir("", "config-test") require.Nil(err) - defer os.RemoveAll(tmpDir) + defer os.RemoveAll(tmpDir) // nolint: errcheck // create root dir EnsureRoot(tmpDir) diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go index 6bd7bdd4a..9ac163eb9 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, gosimple conRI = conR if i == 0 { @@ -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() @@ -289,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/common_test.go b/consensus/common_test.go index 50793e651..8528b0a95 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/mempool_test.go b/consensus/mempool_test.go index 3314caad1..73f676342 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" @@ -118,8 +120,12 @@ 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) + assert.False(t, resDeliver.IsErr(), cmn.Fmt("expected no error. got %v", resDeliver)) + + resCommit := app.Commit() + 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.go b/consensus/reactor.go index 11cd07501..e873ddde9 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 { @@ -95,31 +97,34 @@ 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 conS", "err", err) + } } // GetChannels implements Reactor 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/consensus/reactor_test.go b/consensus/reactor_test.go index 45e94a121..458ff2c44 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 49aa5e7fe..349eade04 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" @@ -99,8 +99,13 @@ 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 { - gr.Close() + if err := gr.Close(); err != nil { + return err + } } if found { return fmt.Errorf("WAL should not contain #ENDHEIGHT %d.", csHeight) @@ -116,7 +121,7 @@ func (cs *ConsensusState) catchupReplay(csHeight int) error { 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) @@ -145,6 +150,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") @@ -164,7 +170,7 @@ func makeHeightSearchFunc(height int) auto.SearchFunc { return -1, nil } } -} +}*/ //---------------------------------------------- // Recover from failure during block processing @@ -230,7 +236,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 +371,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 6b52b5b09..ba7b12654 100644 --- a/consensus/replay_file.go +++ b/consensus/replay_file.go @@ -59,13 +59,13 @@ 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 } pb := newPlayback(file, fp, cs, cs.state.Copy()) - defer pb.fp.Close() + defer pb.fp.Close() // nolint: errcheck var nextN int // apply N msgs in a row var msg *TimedWALMessage @@ -127,8 +127,10 @@ func (pb *playback) replayReset(count int, newStepCh chan interface{}) error { newCS.SetEventBus(pb.cs.eventBus) newCS.startForReplay() - pb.fp.Close() - fp, err := os.OpenFile(pb.fileName, os.O_RDONLY, 0666) + if err := pb.fp.Close(); err != nil { + return err + } + fp, err := os.OpenFile(pb.fileName, os.O_RDONLY, 0600) if err != nil { return err } @@ -220,7 +222,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 { + pb.cs.Logger.Error("Replay reset error", "err", err) + } } else { i, err := strconv.Atoi(tokens[1]) if err != nil { @@ -228,7 +232,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 { + pb.cs.Logger.Error("Replay reset error", "err", err) + } } } diff --git a/consensus/replay_test.go b/consensus/replay_test.go index a5d3f0883..0403b8a48 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,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/consensus/state.go b/consensus/state.go index c65976d79..a535a1017 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,11 @@ 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 { + cs.Logger.Error("Error starting timeout ticker", "err", err) + return + } go cs.receiveRoutine(maxSteps) } @@ -338,12 +345,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 } //------------------------------------------------------------ @@ -361,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()) + sleepDuration := rs.StartTime.Sub(time.Now()) // nolint: gotype, gosimple cs.scheduleTimeout(sleepDuration, rs.Height, 0, cstypes.RoundStepNewHeight) } @@ -692,10 +703,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/consensus/state_test.go b/consensus/state_test.go index 49ec1185c..ecccafed4 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/consensus/wal.go b/consensus/wal.go index 3f85f7da6..109f5f3f2 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() } @@ -273,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 c2f570a76..faa68dd9b 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/mempool/mempool.go b/mempool/mempool.go index caaa034e9..d781500c2 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 { + mem.logger.Error("Error writing to WAL", "err", err) + } + _, err = mem.wal.Write([]byte("\n")) + if err != nil { + mem.logger.Error("Error writing to WAL", "err", err) + } } // END WAL @@ -331,10 +337,10 @@ 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) { - // TODO: check err ? - mem.proxyAppConn.FlushSync() // To flush async resCb calls e.g. from CheckTx - +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 + return err + } // First, create a lookup map of txns in new txs. txsMap := make(map[string]struct{}) for _, tx := range txs { @@ -357,6 +363,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/mempool/mempool_test.go b/mempool/mempool_test.go index 46401e88b..7773d9d74 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 @@ -49,9 +52,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) } } @@ -78,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) @@ -88,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 @@ -146,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/mempool/reactor.go b/mempool/reactor.go index 6a8765200..9e51d2df5 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/node/node.go b/node/node.go index c25c01027..def313943 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() { @@ -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/node/node_test.go b/node/node_test.go index 01099459a..645bd2f21 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/addrbook.go b/p2p/addrbook.go index 0b3301060..8f924d129 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 28b136c77..ad73b68e8 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, @@ -569,7 +571,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/connection_test.go b/p2p/connection_test.go index d74deabfa..11b036dcc 100644 --- a/p2p/connection_test.go +++ b/p2p/connection_test.go @@ -32,8 +32,8 @@ func TestMConnectionSend(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer server.Close() - defer client.Close() + defer server.Close() // nolint: errcheck + defer client.Close() // nolint: errcheck mconn := createTestMConnection(client) _, err := mconn.Start() @@ -44,12 +44,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 +65,8 @@ func TestMConnectionReceive(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer server.Close() - defer client.Close() + defer server.Close() // nolint: errcheck + defer client.Close() // nolint: errcheck receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -97,8 +103,8 @@ func TestMConnectionStatus(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer server.Close() - defer client.Close() + defer server.Close() // nolint: errcheck + defer client.Close() // nolint: errcheck mconn := createTestMConnection(client) _, err := mconn.Start() @@ -114,8 +120,8 @@ func TestMConnectionStopsAndReturnsError(t *testing.T) { assert, require := assert.New(t), require.New(t) server, client := netPipe() - defer server.Close() - defer client.Close() + defer server.Close() // nolint: errcheck + defer client.Close() // nolint: errcheck receivedCh := make(chan []byte) errorsCh := make(chan interface{}) @@ -130,7 +136,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 aefac986a..fa16e4a26 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() + 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/listener.go b/p2p/listener.go index 971390974..32a608d6c 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 c3d33a9ae..92018e0aa 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 ec8349551..b0247d376 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. @@ -230,7 +232,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 e37455256..694300523 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 ba52b22a4..b2a01493d 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)) @@ -78,7 +82,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() @@ -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 fd70198f4..73bb9e75e 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 @@ -82,7 +87,7 @@ func (r *PEXReactor) OnStop() { // GetChannels implements Reactor func (r *PEXReactor) GetChannels() []*ChannelDescriptor { return []*ChannelDescriptor{ - &ChannelDescriptor{ + { ID: PexChannel, Priority: 1, SendQueueCapacity: 10, @@ -278,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/pex_reactor_test.go b/p2p/pex_reactor_test.go index 3efc3c643..e79c73a86 100644 --- a/p2p/pex_reactor_test.go +++ b/p2p/pex_reactor_test.go @@ -20,7 +20,7 @@ func TestPEXReactorBasic(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) @@ -36,7 +36,7 @@ func TestPEXReactorAddRemovePeer(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) @@ -69,7 +69,7 @@ func TestPEXReactorRunning(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", false) book.SetLogger(log.TestingLogger()) @@ -139,7 +139,7 @@ func TestPEXReactorReceive(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", false) book.SetLogger(log.TestingLogger()) @@ -164,7 +164,7 @@ func TestPEXReactorAbuseFromPeer(t *testing.T) { dir, err := ioutil.TempDir("", "pex_reactor") require.Nil(err) - defer os.RemoveAll(dir) + defer os.RemoveAll(dir) // nolint: errcheck book := NewAddrBook(dir+"addrbook.json", true) book.SetLogger(log.TestingLogger()) diff --git a/p2p/secret_connection.go b/p2p/secret_connection.go index 0e107ea59..aec0a7519 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) // 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) // does not error + hasher.Write(input) // nolint: errcheck, gas 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 d0d008529..8b58fb417 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 c19931550..bea2ca1bf 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,12 @@ 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 { + // Should never happen + sw.Logger.Error("Error starting peer", "peer", peer, "err", err) + } + for _, reactor := range sw.reactors { reactor.AddPeer(peer) } @@ -511,7 +518,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] @@ -568,7 +575,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 +592,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 1ea79efef..58ef3e5f0 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 ( @@ -100,12 +101,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 } @@ -171,10 +172,12 @@ func TestConnAddrFilter(t *testing.T) { // connect to good peer go func() { - s1.addPeerWithConnection(c1) + err := s1.addPeerWithConnection(c1) + assert.NotNil(t, err, "expected err") }() go func() { - s2.addPeerWithConnection(c2) + err := s2.addPeerWithConnection(c2) + assert.NotNil(t, err, "expected err") }() assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond) @@ -206,10 +209,12 @@ func TestConnPubKeyFilter(t *testing.T) { // connect to good peer go func() { - s1.addPeerWithConnection(c1) + err := s1.addPeerWithConnection(c1) + assert.NotNil(t, err, "expected error") }() go func() { - s2.addPeerWithConnection(c2) + err := s2.addPeerWithConnection(c2) + assert.NotNil(t, err, "expected error") }() assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond) @@ -220,7 +225,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 +252,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 @@ -295,12 +306,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/p2p/trust/trustmetric.go b/p2p/trust/trustmetric.go index cbc2db7d5..c6740c0da 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 1d3770b57..4e0994b71 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/upnp/probe.go b/p2p/upnp/probe.go index 74d4d4c51..d2338b95e 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 { + logger.Error(cmn.Fmt("Listener closing error: %v", err)) + } }() supportsHairpin := testHairpin(listener, fmt.Sprintf("%v:%v", ext, extPort), logger) diff --git a/p2p/upnp/upnp.go b/p2p/upnp/upnp.go index 7d44d1e31..cac67a730 100644 --- a/p2p/upnp/upnp.go +++ b/p2p/upnp/upnp.go @@ -40,11 +40,10 @@ func Discover() (nat NAT, err error) { return } socket := conn.(*net.UDPConn) - defer socket.Close() + defer socket.Close() // nolint: errcheck - 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" @@ -64,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 { @@ -198,7 +200,8 @@ 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)) return @@ -296,15 +299,21 @@ 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 } 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 +348,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 @@ -365,7 +374,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/p2p/util.go b/p2p/util.go index 2be320263..a4c3ad58b 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) + hasher.Write(b) // nolint: errcheck, gas sum := hasher.Sum(nil) hasher.Reset() - hasher.Write(sum) + hasher.Write(sum) // nolint: errcheck, gas return hasher.Sum(nil) } diff --git a/proxy/app_conn_test.go b/proxy/app_conn_test.go index 0c700140b..bb56d7213 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.go b/rpc/client/mock/abci.go index 2ed012e44..e935a2824 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/client/mock/abci_test.go b/rpc/client/mock/abci_test.go index a7afa0894..36a457918 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,8 +94,11 @@ 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() + 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] @@ -118,11 +123,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}} - r.BroadcastTxCommit(txs[0]) - r.BroadcastTxSync(txs[1]) - r.BroadcastTxAsync(txs[2]) + _, err = r.BroadcastTxCommit(txs[0]) + assert.NotNil(err, "expected err on broadcast") + _, err = r.BroadcastTxSync(txs[1]) + assert.NotNil(err, "expected err on broadcast") + _, err = r.BroadcastTxAsync(txs[2]) + assert.NotNil(err, "expected err on broadcast") require.Equal(5, len(r.Calls)) diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index f2626f84e..c68276354 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 a3c970d48..0b5154769 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/grpc/client_server.go b/rpc/grpc/client_server.go index 1c6498df7..80d736f57 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 f19c2e941..a1b23a258 100644 --- a/rpc/lib/client/http_client.go +++ b/rpc/lib/client/http_client.go @@ -93,7 +93,8 @@ 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 { return nil, err @@ -128,7 +129,8 @@ 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 { return nil, err diff --git a/rpc/lib/client/ws_client.go b/rpc/lib/client/ws_client.go index bfe2272e4..573964328 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 { + c.Logger.Error("failed to set write deadline", "err", 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,10 @@ func (c *WSClient) writeRoutine() { defer func() { ticker.Stop() - c.conn.Close() + if err := c.conn.Close(); err != nil { + // ignore error; it will trigger in tests + // likely because it's closing an already closed connection + } c.wg.Done() }() @@ -360,10 +363,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 { + c.Logger.Error("failed to set write deadline", "err", 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 +376,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 { + c.Logger.Error("failed to set write deadline", "err", 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 +392,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 { + c.Logger.Error("failed to write message", "err", err) + } return } } @@ -397,7 +404,10 @@ 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 { + // ignore error; it will trigger in tests + // likely because it's closing an already closed connection + } c.wg.Done() }() @@ -415,7 +425,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 { + c.Logger.Error("failed to set read deadline", "err", 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 3a0632e38..8552a4ee1 100644 --- a/rpc/lib/client/ws_client_test.go +++ b/rpc/lib/client/ws_client_test.go @@ -34,7 +34,7 @@ func (h *myHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if err != nil { panic(err) } - defer conn.Close() + defer conn.Close() // nolint: errcheck for { messageType, _, err := conn.ReadMessage() if err != nil { @@ -43,7 +43,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 +104,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 { + t.Error(err) + } // results in WS write error, the client should resend on reconnect call(t, "a", c) @@ -135,14 +139,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/rpc_test.go b/rpc/lib/rpc_test.go index b5af0e43a..433041c1f 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) { @@ -322,17 +318,15 @@ 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) + require.Nil(t, err) + got := result.Value + assert.Equal(t, got, val) } func TestWSHandlesArrayParams(t *testing.T) { @@ -347,17 +341,15 @@ 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) + require.Nil(t, err) + got := result.Value + assert.Equal(t, got, val) } // TestWSClientPingPong checks that a client & server exchange pings diff --git a/rpc/lib/server/handlers.go b/rpc/lib/server/handlers.go index 1f2907001..2e24195dc 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 @@ -529,7 +533,7 @@ func (wsc *wsConnection) readRoutine() { wsc.WriteRPCResponse(types.RPCInternalError("unknown", err)) go wsc.readRoutine() } else { - wsc.baseConn.Close() + wsc.baseConn.Close() // nolint: errcheck } }() @@ -543,7 +547,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 { + wsc.Logger.Error("failed to set read deadline", "err", err) + } var in []byte _, in, err := wsc.baseConn.ReadMessage() if err != nil { @@ -615,7 +621,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 @@ -662,7 +670,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) } @@ -713,7 +723,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 { + wm.logger.Error("Error starting connection", "err", err) + } } // rpc.websocket @@ -770,5 +783,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()) + w.Write(buf.Bytes()) // nolint: errcheck } diff --git a/rpc/lib/server/http_server.go b/rpc/lib/server/http_server.go index 7623337db..515baf5dd 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) + 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) + w.Write(jsonBytes) // nolint: errcheck, gas } //----------------------------------------------------------------------------- diff --git a/rpc/lib/server/parse_test.go b/rpc/lib/server/parse_test.go index 3c6d6edde..a86226f2c 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/rpc/test/helpers.go b/rpc/test/helpers.go index 03538b512..d7e5f82ca 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/scripts/wal2json/main.go b/scripts/wal2json/main.go index 2cf40c57d..e44ed4b17 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/state/execution.go b/state/execution.go index 76205d0fd..6c74f7a9e 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 @@ -271,9 +273,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) { @@ -282,14 +282,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 { + 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) } - s.TxIndexer.AddBatch(batch) } // ExecCommitBlock executes and commits a block on the proxyApp without validating or mutating the state. diff --git a/state/execution_test.go b/state/execution_test.go index 8fcdcf1c5..626b2ecde 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/indexer.go b/state/txindex/indexer.go index 66897905c..039460a16 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_test.go b/state/txindex/kv/kv_test.go index 903189c2b..673674b30 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,16 @@ func benchmarkTxIndex(txsCount int, b *testing.B) { if err != nil { b.Fatal(err) } - defer os.RemoveAll(dir) + defer os.RemoveAll(dir) // nolint: errcheck 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/test/run_test.sh b/test/run_test.sh index 6e4823f10..cecd2c72b 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 diff --git a/types/heartbeat_test.go b/types/heartbeat_test.go index 8a0967128..660ccd0f9 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") }) } diff --git a/types/part_set.go b/types/part_set.go index e15d2cab6..e8a0997c0 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) // doesn't err + hasher.Write(part.Bytes) // nolint: errcheck, gas part.hash = hasher.Sum(nil) return part.hash } diff --git a/types/priv_validator_test.go b/types/priv_validator_test.go index ac91de861..cd2dfc137 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) diff --git a/types/proposal_test.go b/types/proposal_test.go index d1c991849..352ba8de1 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) + } } } diff --git a/types/services.go b/types/services.go index e34d846b5..f025de79b 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() {} diff --git a/types/validator.go b/types/validator.go index 7b167b273..c5d064e01 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 diff --git a/types/validator_set_test.go b/types/validator_set_test.go index a285adeeb..572b7b007 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" ) diff --git a/types/vote_set_test.go b/types/vote_set_test.go index 5a757a00b..ebead3eea 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,10 @@ func Test2_3MajorityRedux(t *testing.T) { // 70th validator voted for different BlockHash { vote := withValidator(voteProto, privValidators[69].GetAddress(), 69) - signAddVote(privValidators[69], withBlockHash(vote, cmn.RandBytes(32)), voteSet) + _, err := signAddVote(privValidators[69], 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: last vote added had different BlockHash") @@ -226,7 +250,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 +466,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. @@ -450,13 +480,20 @@ func TestMakeCommit(t *testing.T) { vote := withValidator(voteProto, privValidators[6].GetAddress(), 6) vote = withBlockHash(vote, cmn.RandBytes(32)) vote = withBlockPartsHeader(vote, PartSetHeader{123, cmn.RandBytes(32)}) - signAddVote(privValidators[6], vote, voteSet) + + _, err := signAddVote(privValidators[6], vote, voteSet) + if err != nil { + t.Error(err) + } } // 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()