From d8f0bc3e60eaddef11fe7e83328a22a149de5a01 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 6 Feb 2019 14:11:35 +0400 Subject: [PATCH 1/8] Revert "quick fix for CircleCI (#2279)" This reverts commit 1cf6712a36e8ecc843a68aa373748e89e0afecba. --- .circleci/config.yml | 156 ++++++------------------------------------- 1 file changed, 22 insertions(+), 134 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ecc7c0ac7..826793370 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -48,10 +48,10 @@ jobs: key: v3-pkg-cache paths: - /go/pkg - # - save_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - # paths: - # - /go/src/github.com/tendermint/tendermint + - save_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} + paths: + - /go/src/github.com/tendermint/tendermint build_slate: <<: *defaults @@ -60,23 +60,8 @@ jobs: at: /tmp/workspace - restore_cache: key: v3-pkg-cache - # https://discuss.circleci.com/t/saving-cache-stopped-working-warning-skipping-this-step-disabled-in-configuration/24423/2 - # - restore_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - - checkout - - run: - name: tools - command: | - export PATH="$GOBIN:$PATH" - make get_tools - - run: - name: dependencies - command: | - export PATH="$GOBIN:$PATH" - make get_vendor_deps - - run: mkdir -p $GOPATH/src/github.com/tendermint - - run: ln -sf /home/circleci/project $GOPATH/src/github.com/tendermint/tendermint - + - restore_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - run: name: slate docs command: | @@ -91,23 +76,8 @@ jobs: at: /tmp/workspace - restore_cache: key: v3-pkg-cache - # - restore_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - - checkout - - run: - name: tools - command: | - export PATH="$GOBIN:$PATH" - make get_tools - make get_dev_tools - - run: - name: dependencies - command: | - export PATH="$GOBIN:$PATH" - make get_vendor_deps - - run: mkdir -p $GOPATH/src/github.com/tendermint - - run: ln -sf /home/circleci/project $GOPATH/src/github.com/tendermint/tendermint - + - restore_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - run: name: metalinter command: | @@ -128,22 +98,8 @@ jobs: at: /tmp/workspace - restore_cache: key: v3-pkg-cache - # - restore_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - - checkout - - run: - name: tools - command: | - export PATH="$GOBIN:$PATH" - make get_tools - - run: - name: dependencies - command: | - export PATH="$GOBIN:$PATH" - make get_vendor_deps - - run: mkdir -p $GOPATH/src/github.com/tendermint - - run: ln -sf /home/circleci/project $GOPATH/src/github.com/tendermint/tendermint - + - restore_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - run: name: Run abci apps tests command: | @@ -159,22 +115,8 @@ jobs: at: /tmp/workspace - restore_cache: key: v3-pkg-cache - # - restore_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - - checkout - - run: - name: tools - command: | - export PATH="$GOBIN:$PATH" - make get_tools - - run: - name: dependencies - command: | - export PATH="$GOBIN:$PATH" - make get_vendor_deps - - run: mkdir -p $GOPATH/src/github.com/tendermint - - run: ln -sf /home/circleci/project $GOPATH/src/github.com/tendermint/tendermint - + - restore_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - run: name: Run abci-cli tests command: | @@ -188,22 +130,8 @@ jobs: at: /tmp/workspace - restore_cache: key: v3-pkg-cache - # - restore_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - - checkout - - run: - name: tools - command: | - export PATH="$GOBIN:$PATH" - make get_tools - - run: - name: dependencies - command: | - export PATH="$GOBIN:$PATH" - make get_vendor_deps - - run: mkdir -p $GOPATH/src/github.com/tendermint - - run: ln -sf /home/circleci/project $GOPATH/src/github.com/tendermint/tendermint - + - restore_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - run: sudo apt-get update && sudo apt-get install -y --no-install-recommends bsdmainutils - run: name: Run tests @@ -217,22 +145,8 @@ jobs: at: /tmp/workspace - restore_cache: key: v3-pkg-cache - # - restore_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - - checkout - - run: - name: tools - command: | - export PATH="$GOBIN:$PATH" - make get_tools - - run: - name: dependencies - command: | - export PATH="$GOBIN:$PATH" - make get_vendor_deps - - run: mkdir -p $GOPATH/src/github.com/tendermint - - run: ln -sf /home/circleci/project $GOPATH/src/github.com/tendermint/tendermint - + - restore_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - run: mkdir -p /tmp/logs - run: name: Run tests @@ -256,22 +170,8 @@ jobs: at: /tmp/workspace - restore_cache: key: v3-pkg-cache - # - restore_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - - checkout - - run: - name: tools - command: | - export PATH="$GOBIN:$PATH" - make get_tools - - run: - name: dependencies - command: | - export PATH="$GOBIN:$PATH" - make get_vendor_deps - - run: mkdir -p $GOPATH/src/github.com/tendermint - - run: ln -sf /home/circleci/project $GOPATH/src/github.com/tendermint/tendermint - + - restore_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - run: name: Run tests command: bash test/persist/test_failure_indices.sh @@ -317,22 +217,10 @@ jobs: steps: - attach_workspace: at: /tmp/workspace - # - restore_cache: - # key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - - checkout - - run: - name: tools - command: | - export PATH="$GOBIN:$PATH" - make get_tools - - run: - name: dependencies - command: | - export PATH="$GOBIN:$PATH" - make get_vendor_deps - - run: mkdir -p $GOPATH/src/github.com/tendermint - - run: ln -sf /home/circleci/project $GOPATH/src/github.com/tendermint/tendermint - + - restore_cache: + key: v3-pkg-cache + - restore_cache: + key: v3-tree-{{ .Environment.CIRCLE_SHA1 }} - run: name: gather command: | From da33dd04cc90f60f339afa8182f59294e67d151b Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 6 Feb 2019 15:00:55 +0400 Subject: [PATCH 2/8] switch to golangci-lint from gometalinter :speedboat: --- .circleci/config.yml | 2 +- .golangci.yml | 65 ++++++++++++++++++++++++++++++++++++++++++++ Makefile | 43 +++-------------------------- Vagrantfile | 2 +- libs/test.sh | 2 +- scripts/get_tools.sh | 21 ++++++++++---- 6 files changed, 88 insertions(+), 47 deletions(-) create mode 100644 .golangci.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index 826793370..d29680945 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -83,7 +83,7 @@ jobs: command: | set -ex export PATH="$GOBIN:$PATH" - make metalinter + make lint - run: name: check_dep command: | diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..ed2f6ab3e --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,65 @@ +run: + deadline: 1m + +linters: + enable-all: true + disable: + - gocyclo + - golint + - maligned + - errcheck + - staticcheck + - dupl + - ineffassign + - interfacer + - unconvert + - goconst + - unparam + - nakedret + - lll + - gochecknoglobals + - govet + - gocritic + - gosec + - gochecknoinits + - scopelint + - stylecheck + - deadcode + - prealloc + - unused + - gosimple + +# linters-settings: +# govet: +# check-shadowing: true +# golint: +# min-confidence: 0 +# gocyclo: +# min-complexity: 10 +# maligned: +# suggest-new: true +# dupl: +# threshold: 100 +# goconst: +# min-len: 2 +# min-occurrences: 2 +# depguard: +# list-type: blacklist +# packages: +# # logging is allowed only by logutils.Log, logrus +# # is allowed to use only in logutils package +# - github.com/sirupsen/logrus +# misspell: +# locale: US +# lll: +# line-length: 140 +# goimports: +# local-prefixes: github.com/golangci/golangci-lint +# gocritic: +# enabled-tags: +# - performance +# - style +# - experimental +# disabled-checks: +# - wrapperFunc +# - commentFormatting # https://github.com/go-critic/go-critic/issues/755 diff --git a/Makefile b/Makefile index 8c0928d04..ac9700d65 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ GOTOOLS = \ github.com/mitchellh/gox \ github.com/golang/dep/cmd/dep \ - github.com/alecthomas/gometalinter \ + github.com/golangci/golangci-lint/cmd/golangci-lint \ github.com/gogo/protobuf/protoc-gen-gogo \ github.com/square/certstrap GOBIN?=${GOPATH}/bin @@ -11,8 +11,6 @@ INCLUDE = -I=. -I=${GOPATH}/src -I=${GOPATH}/src/github.com/gogo/protobuf/protob BUILD_TAGS?='tendermint' BUILD_FLAGS = -ldflags "-X github.com/tendermint/tendermint/version.GitCommit=`git rev-parse --short=8 HEAD`" -LINT_FLAGS = --exclude '.*\.pb\.go' --exclude 'vendor/*' --vendor --deadline=600s - all: check build test install check: check_tools get_vendor_deps @@ -82,10 +80,6 @@ get_tools: @echo "--> Installing tools" ./scripts/get_tools.sh -get_dev_tools: - @echo "--> Downloading linters (this may take awhile)" - $(GOPATH)/src/github.com/alecthomas/gometalinter/scripts/install.sh -b $(GOBIN) - update_tools: @echo "--> Updating tools" ./scripts/get_tools.sh @@ -246,38 +240,9 @@ cleanup_after_test_with_deadlock: fmt: @go fmt ./... -metalinter: +lint: @echo "--> Running linter" - @gometalinter $(LINT_FLAGS) --disable-all \ - --enable=vet \ - --enable=vetshadow \ - --enable=deadcode \ - --enable=varcheck \ - --enable=structcheck \ - --enable=misspell \ - --enable=safesql \ - --enable=gosec \ - --enable=goimports \ - --enable=gofmt \ - ./... - #--enable=gotype \ - #--enable=gotypex \ - #--enable=gocyclo \ - #--enable=golint \ - #--enable=maligned \ - #--enable=errcheck \ - #--enable=staticcheck \ - #--enable=dupl \ - #--enable=ineffassign \ - #--enable=interfacer \ - #--enable=unconvert \ - #--enable=goconst \ - #--enable=unparam \ - #--enable=nakedret \ - -metalinter_all: - @echo "--> Running linter (all)" - gometalinter $(LINT_FLAGS) --enable-all --disable=lll ./... + @golangci-lint run DESTINATION = ./index.html.md @@ -343,4 +308,4 @@ build-slate: # To avoid unintended conflicts with file names, always add to .PHONY # unless there is a reason not to. # https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html -.PHONY: check build build_race build_abci dist install install_abci check_dep check_tools get_tools get_dev_tools update_tools get_vendor_deps draw_deps get_protoc protoc_abci protoc_libs gen_certs clean_certs grpc_dbserver test_cover test_apps test_persistence test_p2p test test_race test_integrations test_release test100 vagrant_test fmt rpc-docs build-linux localnet-start localnet-stop build-docker build-docker-localnode sentry-start sentry-config sentry-stop build-slate protoc_grpc protoc_all build_c install_c test_with_deadlock cleanup_after_test_with_deadlock metalinter metalinter_all +.PHONY: check build build_race build_abci dist install install_abci check_dep check_tools get_tools update_tools get_vendor_deps draw_deps get_protoc protoc_abci protoc_libs gen_certs clean_certs grpc_dbserver test_cover test_apps test_persistence test_p2p test test_race test_integrations test_release test100 vagrant_test fmt rpc-docs build-linux localnet-start localnet-stop build-docker build-docker-localnode sentry-start sentry-config sentry-stop build-slate protoc_grpc protoc_all build_c install_c test_with_deadlock cleanup_after_test_with_deadlock lint diff --git a/Vagrantfile b/Vagrantfile index f058d78e7..320f3b1c3 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -53,6 +53,6 @@ Vagrant.configure("2") do |config| # get all deps and tools, ready to install/test su - vagrant -c 'source /home/vagrant/.bash_profile' - su - vagrant -c 'cd /home/vagrant/go/src/github.com/tendermint/tendermint && make get_tools && make get_dev_tools && make get_vendor_deps' + su - vagrant -c 'cd /home/vagrant/go/src/github.com/tendermint/tendermint && make get_tools && make get_vendor_deps' SHELL end diff --git a/libs/test.sh b/libs/test.sh index ecf17fc45..64898b0d2 100755 --- a/libs/test.sh +++ b/libs/test.sh @@ -2,7 +2,7 @@ set -e # run the linter -# make metalinter_test +# make lint # setup certs make gen_certs diff --git a/scripts/get_tools.sh b/scripts/get_tools.sh index 87e30a3d9..dd9566917 100755 --- a/scripts/get_tools.sh +++ b/scripts/get_tools.sh @@ -5,11 +5,14 @@ set -e # specific git hash. # # repos it installs: -# github.com/mitchellh/gox # github.com/golang/dep/cmd/dep -# gopkg.in/alecthomas/gometalinter.v2 # github.com/gogo/protobuf/protoc-gen-gogo # github.com/square/certstrap +# github.com/mitchellh/gox +# github.com/golangci/golangci-lint +# github.com/petermattis/goid +# github.com/sasha-s/go-deadlock +# goimports ## check if GOPATH is set if [ -z ${GOPATH+x} ]; then @@ -45,14 +48,22 @@ installFromGithub() { echo "" } -installFromGithub mitchellh/gox 51ed453898ca5579fea9ad1f08dff6b121d9f2e8 +######################## COMMON TOOLS ######################################## installFromGithub golang/dep 22125cfaa6ddc71e145b1535d4b7ee9744fefff2 cmd/dep -## gometalinter v3.0.0 -installFromGithub alecthomas/gometalinter df395bfa67c5d0630d936c0044cf07ff05086655 + +######################## DEVELOPER TOOLS ##################################### installFromGithub gogo/protobuf 61dbc136cf5d2f08d68a011382652244990a53a9 protoc-gen-gogo + installFromGithub square/certstrap e27060a3643e814151e65b9807b6b06d169580a7 +# used to build tm-monitor & tm-bench binaries +installFromGithub mitchellh/gox 51ed453898ca5579fea9ad1f08dff6b121d9f2e8 + +## golangci-lint v1.13.2 +installFromGithub golangci/golangci-lint 7b2421d55194c9dc385eff7720a037aa9244ca3c cmd/golangci-lint + ## make test_with_deadlock +## XXX: https://github.com/tendermint/tendermint/issues/3242 installFromGithub petermattis/goid b0b1615b78e5ee59739545bb38426383b2cda4c9 installFromGithub sasha-s/go-deadlock d68e2bc52ae3291765881b9056f2c1527f245f1e go get golang.org/x/tools/cmd/goimports From ffd3bf8448d5ca5fd36e1056a97586d7bbbf8fa4 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 6 Feb 2019 15:16:38 +0400 Subject: [PATCH 3/8] remove or comment out unused code --- .golangci.yml | 2 - blockchain/pool.go | 34 +++---- consensus/common_test.go | 58 +++++------ consensus/state_test.go | 4 - crypto/merkle/proof_test.go | 22 ++-- lite/proxy/query_test.go | 157 ++++++++++++++--------------- p2p/conn/secret_connection_test.go | 9 -- p2p/switch.go | 8 +- p2p/trust/metric_test.go | 82 +++++++-------- state/state_test.go | 4 - 10 files changed, 179 insertions(+), 201 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ed2f6ab3e..6175fc90b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -26,8 +26,6 @@ linters: - stylecheck - deadcode - prealloc - - unused - - gosimple # linters-settings: # govet: diff --git a/blockchain/pool.go b/blockchain/pool.go index e6be36012..236bf7e09 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -363,23 +363,23 @@ func (pool *BlockPool) sendError(err error, peerID p2p.ID) { pool.errorsCh <- peerError{err, peerID} } -// unused by tendermint; left for debugging purposes -func (pool *BlockPool) debug() string { - pool.mtx.Lock() - defer pool.mtx.Unlock() - - str := "" - nextHeight := pool.height + pool.requestersLen() - for h := pool.height; h < nextHeight; h++ { - if pool.requesters[h] == nil { - str += fmt.Sprintf("H(%v):X ", h) - } else { - str += fmt.Sprintf("H(%v):", h) - str += fmt.Sprintf("B?(%v) ", pool.requesters[h].block != nil) - } - } - return str -} +// for debugging purposes +// func (pool *BlockPool) debug() string { +// pool.mtx.Lock() +// defer pool.mtx.Unlock() + +// str := "" +// nextHeight := pool.height + pool.requestersLen() +// for h := pool.height; h < nextHeight; h++ { +// if pool.requesters[h] == nil { +// str += fmt.Sprintf("H(%v):X ", h) +// } else { +// str += fmt.Sprintf("H(%v):", h) +// str += fmt.Sprintf("B?(%v) ", pool.requesters[h].block != nil) +// } +// } +// return str +// } //------------------------------------- diff --git a/consensus/common_test.go b/consensus/common_test.go index a975b2b60..4e3d60e1d 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -378,35 +378,35 @@ func ensureNewEvent( } } -func ensureNewRoundStep(stepCh <-chan interface{}, height int64, round int) { - ensureNewEvent( - stepCh, - height, - round, - ensureTimeout, - "Timeout expired while waiting for NewStep event") -} - -func ensureNewVote(voteCh <-chan interface{}, height int64, round int) { - select { - case <-time.After(ensureTimeout): - break - case v := <-voteCh: - edv, ok := v.(types.EventDataVote) - if !ok { - panic(fmt.Sprintf("expected a *types.Vote, "+ - "got %v. wrong subscription channel?", - reflect.TypeOf(v))) - } - vote := edv.Vote - if vote.Height != height { - panic(fmt.Sprintf("expected height %v, got %v", height, vote.Height)) - } - if vote.Round != round { - panic(fmt.Sprintf("expected round %v, got %v", round, vote.Round)) - } - } -} +// func ensureNewRoundStep(stepCh <-chan interface{}, height int64, round int) { +// ensureNewEvent( +// stepCh, +// height, +// round, +// ensureTimeout, +// "Timeout expired while waiting for NewStep event") +// } + +// func ensureNewVote(voteCh <-chan interface{}, height int64, round int) { +// select { +// case <-time.After(ensureTimeout): +// break +// case v := <-voteCh: +// edv, ok := v.(types.EventDataVote) +// if !ok { +// panic(fmt.Sprintf("expected a *types.Vote, "+ +// "got %v. wrong subscription channel?", +// reflect.TypeOf(v))) +// } +// vote := edv.Vote +// if vote.Height != height { +// panic(fmt.Sprintf("expected height %v, got %v", height, vote.Height)) +// } +// if vote.Round != round { +// panic(fmt.Sprintf("expected round %v, got %v", round, vote.Round)) +// } +// } +// } func ensureNewRound(roundCh <-chan interface{}, height int64, round int) { select { diff --git a/consensus/state_test.go b/consensus/state_test.go index 10c04fbc5..153f51e16 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -22,10 +22,6 @@ func init() { config = ResetConfig("consensus_state_test") } -func ensureProposeTimeout(timeoutPropose time.Duration) time.Duration { - return time.Duration(timeoutPropose.Nanoseconds()*2) * time.Nanosecond -} - /* ProposeSuite diff --git a/crypto/merkle/proof_test.go b/crypto/merkle/proof_test.go index 2a0bdccfc..504156246 100644 --- a/crypto/merkle/proof_test.go +++ b/crypto/merkle/proof_test.go @@ -26,17 +26,17 @@ func NewDominoOp(key, input, output string) DominoOp { } } -func DominoOpDecoder(pop ProofOp) (ProofOperator, error) { - if pop.Type != ProofOpDomino { - panic("unexpected proof op type") - } - var op DominoOp // a bit strange as we'll discard this, but it works. - err := amino.UnmarshalBinaryLengthPrefixed(pop.Data, &op) - if err != nil { - return nil, cmn.ErrorWrap(err, "decoding ProofOp.Data into SimpleValueOp") - } - return NewDominoOp(string(pop.Key), op.Input, op.Output), nil -} +// func DominoOpDecoder(pop ProofOp) (ProofOperator, error) { +// if pop.Type != ProofOpDomino { +// panic("unexpected proof op type") +// } +// var op DominoOp // a bit strange as we'll discard this, but it works. +// err := amino.UnmarshalBinaryLengthPrefixed(pop.Data, &op) +// if err != nil { +// return nil, cmn.ErrorWrap(err, "decoding ProofOp.Data into SimpleValueOp") +// } +// return NewDominoOp(string(pop.Key), op.Input, op.Output), nil +// } func (dop DominoOp) ProofOp() ProofOp { bz := amino.MustMarshalBinaryLengthPrefixed(dop) diff --git a/lite/proxy/query_test.go b/lite/proxy/query_test.go index d8d45df3b..707430b61 100644 --- a/lite/proxy/query_test.go +++ b/lite/proxy/query_test.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -21,7 +20,7 @@ import ( var node *nm.Node var chainID = "tendermint_test" // TODO use from config. -var waitForEventTimeout = 5 * time.Second +// var waitForEventTimeout = 5 * time.Second // TODO fix tests!! @@ -42,83 +41,83 @@ func kvstoreTx(k, v []byte) []byte { // TODO: enable it after general proof format has been adapted // in abci/examples/kvstore.go -func _TestAppProofs(t *testing.T) { - assert, require := assert.New(t), require.New(t) - - prt := defaultProofRuntime() - cl := client.NewLocal(node) - client.WaitForHeight(cl, 1, nil) - - // This sets up our trust on the node based on some past point. - source := certclient.NewProvider(chainID, cl) - seed, err := source.LatestFullCommit(chainID, 1, 1) - require.NoError(err, "%#v", err) - cert := lite.NewBaseVerifier(chainID, seed.Height(), seed.Validators) - - // Wait for tx confirmation. - done := make(chan int64) - go func() { - evtTyp := types.EventTx - _, err = client.WaitForOneEvent(cl, evtTyp, waitForEventTimeout) - require.Nil(err, "%#v", err) - close(done) - }() - - // Submit a transaction. - k := []byte("my-key") - v := []byte("my-value") - tx := kvstoreTx(k, v) - br, err := cl.BroadcastTxCommit(tx) - require.NoError(err, "%#v", err) - require.EqualValues(0, br.CheckTx.Code, "%#v", br.CheckTx) - require.EqualValues(0, br.DeliverTx.Code) - brh := br.Height - - // Fetch latest after tx commit. - <-done - latest, err := source.LatestFullCommit(chainID, 1, 1<<63-1) - require.NoError(err, "%#v", err) - rootHash := latest.SignedHeader.AppHash - if rootHash == nil { - // Fetch one block later, AppHash hasn't been committed yet. - // TODO find a way to avoid doing this. - client.WaitForHeight(cl, latest.SignedHeader.Height+1, nil) - latest, err = source.LatestFullCommit(chainID, latest.SignedHeader.Height+1, 1<<63-1) - require.NoError(err, "%#v", err) - rootHash = latest.SignedHeader.AppHash - } - require.NotNil(rootHash) - - // verify a query before the tx block has no data (and valid non-exist proof) - bs, height, proof, err := GetWithProof(prt, k, brh-1, cl, cert) - require.NoError(err, "%#v", err) - // require.NotNil(proof) - // TODO: Ensure that *some* keys will be there, ensuring that proof is nil, - // (currently there's a race condition) - // and ensure that proof proves absence of k. - require.Nil(bs) - - // but given that block it is good - bs, height, proof, err = GetWithProof(prt, k, brh, cl, cert) - require.NoError(err, "%#v", err) - require.NotNil(proof) - require.Equal(height, brh) - - assert.EqualValues(v, bs) - err = prt.VerifyValue(proof, rootHash, string(k), bs) // XXX key encoding - assert.NoError(err, "%#v", err) - - // Test non-existing key. - missing := []byte("my-missing-key") - bs, _, proof, err = GetWithProof(prt, missing, 0, cl, cert) - require.NoError(err) - require.Nil(bs) - require.NotNil(proof) - err = prt.VerifyAbsence(proof, rootHash, string(missing)) // XXX VerifyAbsence(), keyencoding - assert.NoError(err, "%#v", err) - err = prt.VerifyAbsence(proof, rootHash, string(k)) // XXX VerifyAbsence(), keyencoding - assert.Error(err, "%#v", err) -} +// func TestAppProofs(t *testing.T) { +// assert, require := assert.New(t), require.New(t) + +// prt := defaultProofRuntime() +// cl := client.NewLocal(node) +// client.WaitForHeight(cl, 1, nil) + +// // This sets up our trust on the node based on some past point. +// source := certclient.NewProvider(chainID, cl) +// seed, err := source.LatestFullCommit(chainID, 1, 1) +// require.NoError(err, "%#v", err) +// cert := lite.NewBaseVerifier(chainID, seed.Height(), seed.Validators) + +// // Wait for tx confirmation. +// done := make(chan int64) +// go func() { +// evtTyp := types.EventTx +// _, err = client.WaitForOneEvent(cl, evtTyp, waitForEventTimeout) +// require.Nil(err, "%#v", err) +// close(done) +// }() + +// // Submit a transaction. +// k := []byte("my-key") +// v := []byte("my-value") +// tx := kvstoreTx(k, v) +// br, err := cl.BroadcastTxCommit(tx) +// require.NoError(err, "%#v", err) +// require.EqualValues(0, br.CheckTx.Code, "%#v", br.CheckTx) +// require.EqualValues(0, br.DeliverTx.Code) +// brh := br.Height + +// // Fetch latest after tx commit. +// <-done +// latest, err := source.LatestFullCommit(chainID, 1, 1<<63-1) +// require.NoError(err, "%#v", err) +// rootHash := latest.SignedHeader.AppHash +// if rootHash == nil { +// // Fetch one block later, AppHash hasn't been committed yet. +// // TODO find a way to avoid doing this. +// client.WaitForHeight(cl, latest.SignedHeader.Height+1, nil) +// latest, err = source.LatestFullCommit(chainID, latest.SignedHeader.Height+1, 1<<63-1) +// require.NoError(err, "%#v", err) +// rootHash = latest.SignedHeader.AppHash +// } +// require.NotNil(rootHash) + +// // verify a query before the tx block has no data (and valid non-exist proof) +// bs, height, proof, err := GetWithProof(prt, k, brh-1, cl, cert) +// require.NoError(err, "%#v", err) +// // require.NotNil(proof) +// // TODO: Ensure that *some* keys will be there, ensuring that proof is nil, +// // (currently there's a race condition) +// // and ensure that proof proves absence of k. +// require.Nil(bs) + +// // but given that block it is good +// bs, height, proof, err = GetWithProof(prt, k, brh, cl, cert) +// require.NoError(err, "%#v", err) +// require.NotNil(proof) +// require.Equal(height, brh) + +// assert.EqualValues(v, bs) +// err = prt.VerifyValue(proof, rootHash, string(k), bs) // XXX key encoding +// assert.NoError(err, "%#v", err) + +// // Test non-existing key. +// missing := []byte("my-missing-key") +// bs, _, proof, err = GetWithProof(prt, missing, 0, cl, cert) +// require.NoError(err) +// require.Nil(bs) +// require.NotNil(proof) +// err = prt.VerifyAbsence(proof, rootHash, string(missing)) // XXX VerifyAbsence(), keyencoding +// assert.NoError(err, "%#v", err) +// err = prt.VerifyAbsence(proof, rootHash, string(k)) // XXX VerifyAbsence(), keyencoding +// assert.Error(err, "%#v", err) +// } func TestTxProofs(t *testing.T) { assert, require := assert.New(t), require.New(t) diff --git a/p2p/conn/secret_connection_test.go b/p2p/conn/secret_connection_test.go index 69d9c09fe..6b2854767 100644 --- a/p2p/conn/secret_connection_test.go +++ b/p2p/conn/secret_connection_test.go @@ -398,12 +398,3 @@ func BenchmarkSecretConnection(b *testing.B) { } //barSecConn.Close() race condition } - -func fingerprint(bz []byte) []byte { - const fbsize = 40 - if len(bz) < fbsize { - return bz - } else { - return bz[:fbsize] - } -} diff --git a/p2p/switch.go b/p2p/switch.go index dbd9c2a60..7d2e6c3f3 100644 --- a/p2p/switch.go +++ b/p2p/switch.go @@ -480,14 +480,12 @@ func (sw *Switch) acceptRoutine() { metrics: sw.metrics, }) if err != nil { - switch err.(type) { + switch err := err.(type) { case ErrRejected: - rErr := err.(ErrRejected) - - if rErr.IsSelf() { + if err.IsSelf() { // Remove the given address from the address book and add to our addresses // to avoid dialing in the future. - addr := rErr.Addr() + addr := err.Addr() sw.addrBook.RemoveAddress(&addr) sw.addrBook.AddOurAddress(&addr) } diff --git a/p2p/trust/metric_test.go b/p2p/trust/metric_test.go index f690ce557..89327c0e1 100644 --- a/p2p/trust/metric_test.go +++ b/p2p/trust/metric_test.go @@ -65,44 +65,44 @@ func TestTrustMetricCopyNilPointer(t *testing.T) { } // XXX: This test fails non-deterministically -func _TestTrustMetricStopPause(t *testing.T) { - // The TestTicker will provide manual control over - // the passing of time within the metric - tt := NewTestTicker() - tm := NewMetric() - tm.SetTicker(tt) - tm.Start() - // Allow some time intervals to pass and pause - tt.NextTick() - tt.NextTick() - tm.Pause() - - // could be 1 or 2 because Pause and NextTick race - first := tm.Copy().numIntervals - - // Allow more time to pass and check the intervals are unchanged - tt.NextTick() - tt.NextTick() - assert.Equal(t, first, tm.Copy().numIntervals) - - // Get the trust metric activated again - tm.GoodEvents(5) - // Allow some time intervals to pass and stop - tt.NextTick() - tt.NextTick() - tm.Stop() - tm.Wait() - - second := tm.Copy().numIntervals - // Allow more intervals to pass while the metric is stopped - // and check that the number of intervals match - tm.NextTimeInterval() - tm.NextTimeInterval() - // XXX: fails non-deterministically: - // expected 5, got 6 - assert.Equal(t, second+2, tm.Copy().numIntervals) - - if first > second { - t.Fatalf("numIntervals should always increase or stay the same over time") - } -} +// func _TestTrustMetricStopPause(t *testing.T) { +// // The TestTicker will provide manual control over +// // the passing of time within the metric +// tt := NewTestTicker() +// tm := NewMetric() +// tm.SetTicker(tt) +// tm.Start() +// // Allow some time intervals to pass and pause +// tt.NextTick() +// tt.NextTick() +// tm.Pause() + +// // could be 1 or 2 because Pause and NextTick race +// first := tm.Copy().numIntervals + +// // Allow more time to pass and check the intervals are unchanged +// tt.NextTick() +// tt.NextTick() +// assert.Equal(t, first, tm.Copy().numIntervals) + +// // Get the trust metric activated again +// tm.GoodEvents(5) +// // Allow some time intervals to pass and stop +// tt.NextTick() +// tt.NextTick() +// tm.Stop() +// tm.Wait() + +// second := tm.Copy().numIntervals +// // Allow more intervals to pass while the metric is stopped +// // and check that the number of intervals match +// tm.NextTimeInterval() +// tm.NextTimeInterval() +// // XXX: fails non-deterministically: +// // expected 5, got 6 +// assert.Equal(t, second+2, tm.Copy().numIntervals) + +// if first > second { +// t.Fatalf("numIntervals should always increase or stay the same over time") +// } +// } diff --git a/state/state_test.go b/state/state_test.go index 9ab0de135..904d7a10b 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -938,10 +938,6 @@ func makeParams(blockBytes, blockGas, evidenceAge int64) types.ConsensusParams { } } -func pk() []byte { - return ed25519.GenPrivKey().PubKey().Bytes() -} - func TestApplyUpdates(t *testing.T) { initParams := makeParams(1, 2, 3) From 3c8156a55a7a13188c7c8fd6e54433b465212920 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 6 Feb 2019 15:24:54 +0400 Subject: [PATCH 4/8] preallocating memory when we can --- .golangci.yml | 1 - crypto/merkle/proof.go | 2 +- libs/common/colors.go | 2 +- libs/events/events.go | 2 +- p2p/pex/pex_reactor.go | 3 +-- 5 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 6175fc90b..ca86e6d5b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,7 +25,6 @@ linters: - scopelint - stylecheck - deadcode - - prealloc # linters-settings: # govet: diff --git a/crypto/merkle/proof.go b/crypto/merkle/proof.go index 8f8b460c9..5e2a3ab12 100644 --- a/crypto/merkle/proof.go +++ b/crypto/merkle/proof.go @@ -98,7 +98,7 @@ func (prt *ProofRuntime) Decode(pop ProofOp) (ProofOperator, error) { } func (prt *ProofRuntime) DecodeProof(proof *Proof) (ProofOperators, error) { - var poz ProofOperators + poz := make(ProofOperators, 0, len(proof.Ops)) for _, pop := range proof.Ops { operator, err := prt.Decode(pop) if err != nil { diff --git a/libs/common/colors.go b/libs/common/colors.go index 4837f97b4..89dda2c97 100644 --- a/libs/common/colors.go +++ b/libs/common/colors.go @@ -43,7 +43,7 @@ func treat(s string, color string) string { } func treatAll(color string, args ...interface{}) string { - var parts []string + parts := make([]string, 0, len(args)) for _, arg := range args { parts = append(parts, treat(fmt.Sprintf("%v", arg), color)) } diff --git a/libs/events/events.go b/libs/events/events.go index fb90bbea6..34333a068 100644 --- a/libs/events/events.go +++ b/libs/events/events.go @@ -188,7 +188,7 @@ func (cell *eventCell) RemoveListener(listenerID string) int { func (cell *eventCell) FireEvent(data EventData) { cell.mtx.RLock() - var eventCallbacks []EventCallback + eventCallbacks := make([]EventCallback, 0, len(cell.listeners)) for _, cb := range cell.listeners { eventCallbacks = append(eventCallbacks, cb) } diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go index 0b043ca83..01d1d8db5 100644 --- a/p2p/pex/pex_reactor.go +++ b/p2p/pex/pex_reactor.go @@ -616,10 +616,9 @@ func (of oldestFirst) Less(i, j int) bool { return of[i].LastAttempt.Before(of[j // getPeersToCrawl returns addresses of potential peers that we wish to validate. // NOTE: The status information is ordered as described above. func (r *PEXReactor) getPeersToCrawl() []crawlPeerInfo { - var of oldestFirst - // TODO: be more selective addrs := r.book.ListOfKnownAddresses() + of := make(oldestFirst, 0, len(addrs)) for _, addr := range addrs { if len(addr.ID()) == 0 { continue // dont use peers without id From 23314daee4c1d3c2cb85c67f1debbdf2d5e9bd86 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 6 Feb 2019 15:38:02 +0400 Subject: [PATCH 5/8] comment out clist tests w/ depend on runtime.SetFinalizer --- .golangci.yml | 1 - libs/clist/clist_test.go | 190 +++++++++++++++++++-------------------- 2 files changed, 94 insertions(+), 97 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ca86e6d5b..4cae2f76a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -24,7 +24,6 @@ linters: - gochecknoinits - scopelint - stylecheck - - deadcode # linters-settings: # govet: diff --git a/libs/clist/clist_test.go b/libs/clist/clist_test.go index 4ded6177a..edf013752 100644 --- a/libs/clist/clist_test.go +++ b/libs/clist/clist_test.go @@ -2,8 +2,6 @@ package clist import ( "fmt" - "runtime" - "sync/atomic" "testing" "time" @@ -65,100 +63,100 @@ func TestSmall(t *testing.T) { } -/* -This test is quite hacky because it relies on SetFinalizer -which isn't guaranteed to run at all. -*/ -// nolint: megacheck -func _TestGCFifo(t *testing.T) { - - const numElements = 1000000 - l := New() - gcCount := new(uint64) - - // SetFinalizer doesn't work well with circular structures, - // so we construct a trivial non-circular structure to - // track. - type value struct { - Int int - } - done := make(chan struct{}) - - for i := 0; i < numElements; i++ { - v := new(value) - v.Int = i - l.PushBack(v) - runtime.SetFinalizer(v, func(v *value) { - atomic.AddUint64(gcCount, 1) - }) - } - - for el := l.Front(); el != nil; { - l.Remove(el) - //oldEl := el - el = el.Next() - //oldEl.DetachPrev() - //oldEl.DetachNext() - } - - runtime.GC() - time.Sleep(time.Second * 3) - runtime.GC() - time.Sleep(time.Second * 3) - _ = done - - if *gcCount != numElements { - t.Errorf("Expected gcCount to be %v, got %v", numElements, - *gcCount) - } -} - -/* -This test is quite hacky because it relies on SetFinalizer -which isn't guaranteed to run at all. -*/ -// nolint: megacheck -func _TestGCRandom(t *testing.T) { - - const numElements = 1000000 - l := New() - gcCount := 0 - - // SetFinalizer doesn't work well with circular structures, - // so we construct a trivial non-circular structure to - // track. - type value struct { - Int int - } - - for i := 0; i < numElements; i++ { - v := new(value) - v.Int = i - l.PushBack(v) - runtime.SetFinalizer(v, func(v *value) { - gcCount++ - }) - } - - els := make([]*CElement, 0, numElements) - for el := l.Front(); el != nil; el = el.Next() { - els = append(els, el) - } - - for _, i := range cmn.RandPerm(numElements) { - el := els[i] - l.Remove(el) - _ = el.Next() - } - - runtime.GC() - time.Sleep(time.Second * 3) - - if gcCount != numElements { - t.Errorf("Expected gcCount to be %v, got %v", numElements, - gcCount) - } -} +// This test is quite hacky because it relies on SetFinalizer +// which isn't guaranteed to run at all. +//func TestGCFifo(t *testing.T) { +// if runtime.GOARCH != "amd64" { +// t.Skipf("Skipping on non-amd64 machine") +// } + +// const numElements = 1000000 +// l := New() +// gcCount := new(uint64) + +// // SetFinalizer doesn't work well with circular structures, +// // so we construct a trivial non-circular structure to +// // track. +// type value struct { +// Int int +// } +// done := make(chan struct{}) + +// for i := 0; i < numElements; i++ { +// v := new(value) +// v.Int = i +// l.PushBack(v) +// runtime.SetFinalizer(v, func(v *value) { +// atomic.AddUint64(gcCount, 1) +// }) +// } + +// for el := l.Front(); el != nil; { +// l.Remove(el) +// //oldEl := el +// el = el.Next() +// //oldEl.DetachPrev() +// //oldEl.DetachNext() +// } + +// runtime.GC() +// time.Sleep(time.Second * 3) +// runtime.GC() +// time.Sleep(time.Second * 3) +// _ = done + +// if *gcCount != numElements { +// t.Errorf("Expected gcCount to be %v, got %v", numElements, +// *gcCount) +// } +//} + +// This test is quite hacky because it relies on SetFinalizer +// which isn't guaranteed to run at all. +// func TestGCRandom(t *testing.T) { +// if runtime.GOARCH != "amd64" { +// t.Skipf("Skipping on non-amd64 machine") +// } + +// const numElements = 1000000 +// l := New() +// gcCount := 0 + +// // SetFinalizer doesn't work well with circular structures, +// // so we construct a trivial non-circular structure to +// // track. +// type value struct { +// Int int +// } + +// for i := 0; i < numElements; i++ { +// v := new(value) +// v.Int = i +// l.PushBack(v) +// runtime.SetFinalizer(v, func(v *value) { +// gcCount++ +// }) +// } + +// els := make([]*CElement, 0, numElements) +// for el := l.Front(); el != nil; el = el.Next() { +// els = append(els, el) +// } + +// for _, i := range cmn.RandPerm(numElements) { +// el := els[i] +// l.Remove(el) +// _ = el.Next() +// } + +// runtime.GC() +// time.Sleep(time.Second * 3) + +// if gcCount != numElements { +// t.Errorf("Expected gcCount to be %v, got %v", numElements, +// gcCount) +// } +// } func TestScanRightDeleteRandom(t *testing.T) { From 6941d1bb35a04a1d32a027af38d85a0d78374063 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 6 Feb 2019 18:20:10 +0400 Subject: [PATCH 6/8] use nolint label instead of commenting --- blockchain/pool.go | 33 +++---- consensus/common_test.go | 30 ------ crypto/merkle/proof_test.go | 23 ++--- libs/clist/clist_test.go | 182 ++++++++++++++++++------------------ lite/proxy/query_test.go | 159 +++++++++++++++---------------- p2p/trust/metric_test.go | 83 ++++++++-------- 6 files changed, 245 insertions(+), 265 deletions(-) diff --git a/blockchain/pool.go b/blockchain/pool.go index 236bf7e09..804a43250 100644 --- a/blockchain/pool.go +++ b/blockchain/pool.go @@ -364,22 +364,23 @@ func (pool *BlockPool) sendError(err error, peerID p2p.ID) { } // for debugging purposes -// func (pool *BlockPool) debug() string { -// pool.mtx.Lock() -// defer pool.mtx.Unlock() - -// str := "" -// nextHeight := pool.height + pool.requestersLen() -// for h := pool.height; h < nextHeight; h++ { -// if pool.requesters[h] == nil { -// str += fmt.Sprintf("H(%v):X ", h) -// } else { -// str += fmt.Sprintf("H(%v):", h) -// str += fmt.Sprintf("B?(%v) ", pool.requesters[h].block != nil) -// } -// } -// return str -// } +//nolint:unused +func (pool *BlockPool) debug() string { + pool.mtx.Lock() + defer pool.mtx.Unlock() + + str := "" + nextHeight := pool.height + pool.requestersLen() + for h := pool.height; h < nextHeight; h++ { + if pool.requesters[h] == nil { + str += fmt.Sprintf("H(%v):X ", h) + } else { + str += fmt.Sprintf("H(%v):", h) + str += fmt.Sprintf("B?(%v) ", pool.requesters[h].block != nil) + } + } + return str +} //------------------------------------- diff --git a/consensus/common_test.go b/consensus/common_test.go index 4e3d60e1d..e6e64c252 100644 --- a/consensus/common_test.go +++ b/consensus/common_test.go @@ -378,36 +378,6 @@ func ensureNewEvent( } } -// func ensureNewRoundStep(stepCh <-chan interface{}, height int64, round int) { -// ensureNewEvent( -// stepCh, -// height, -// round, -// ensureTimeout, -// "Timeout expired while waiting for NewStep event") -// } - -// func ensureNewVote(voteCh <-chan interface{}, height int64, round int) { -// select { -// case <-time.After(ensureTimeout): -// break -// case v := <-voteCh: -// edv, ok := v.(types.EventDataVote) -// if !ok { -// panic(fmt.Sprintf("expected a *types.Vote, "+ -// "got %v. wrong subscription channel?", -// reflect.TypeOf(v))) -// } -// vote := edv.Vote -// if vote.Height != height { -// panic(fmt.Sprintf("expected height %v, got %v", height, vote.Height)) -// } -// if vote.Round != round { -// panic(fmt.Sprintf("expected round %v, got %v", round, vote.Round)) -// } -// } -// } - func ensureNewRound(roundCh <-chan interface{}, height int64, round int) { select { case <-time.After(ensureTimeout): diff --git a/crypto/merkle/proof_test.go b/crypto/merkle/proof_test.go index 504156246..4de3246f1 100644 --- a/crypto/merkle/proof_test.go +++ b/crypto/merkle/proof_test.go @@ -26,17 +26,18 @@ func NewDominoOp(key, input, output string) DominoOp { } } -// func DominoOpDecoder(pop ProofOp) (ProofOperator, error) { -// if pop.Type != ProofOpDomino { -// panic("unexpected proof op type") -// } -// var op DominoOp // a bit strange as we'll discard this, but it works. -// err := amino.UnmarshalBinaryLengthPrefixed(pop.Data, &op) -// if err != nil { -// return nil, cmn.ErrorWrap(err, "decoding ProofOp.Data into SimpleValueOp") -// } -// return NewDominoOp(string(pop.Key), op.Input, op.Output), nil -// } +//nolint:unused +func DominoOpDecoder(pop ProofOp) (ProofOperator, error) { + if pop.Type != ProofOpDomino { + panic("unexpected proof op type") + } + var op DominoOp // a bit strange as we'll discard this, but it works. + err := amino.UnmarshalBinaryLengthPrefixed(pop.Data, &op) + if err != nil { + return nil, cmn.ErrorWrap(err, "decoding ProofOp.Data into SimpleValueOp") + } + return NewDominoOp(string(pop.Key), op.Input, op.Output), nil +} func (dop DominoOp) ProofOp() ProofOp { bz := amino.MustMarshalBinaryLengthPrefixed(dop) diff --git a/libs/clist/clist_test.go b/libs/clist/clist_test.go index edf013752..7fb7db4d2 100644 --- a/libs/clist/clist_test.go +++ b/libs/clist/clist_test.go @@ -2,6 +2,8 @@ package clist import ( "fmt" + "runtime" + "sync/atomic" "testing" "time" @@ -65,98 +67,100 @@ func TestSmall(t *testing.T) { // This test is quite hacky because it relies on SetFinalizer // which isn't guaranteed to run at all. -//func TestGCFifo(t *testing.T) { -// if runtime.GOARCH != "amd64" { -// t.Skipf("Skipping on non-amd64 machine") -// } - -// const numElements = 1000000 -// l := New() -// gcCount := new(uint64) - -// // SetFinalizer doesn't work well with circular structures, -// // so we construct a trivial non-circular structure to -// // track. -// type value struct { -// Int int -// } -// done := make(chan struct{}) - -// for i := 0; i < numElements; i++ { -// v := new(value) -// v.Int = i -// l.PushBack(v) -// runtime.SetFinalizer(v, func(v *value) { -// atomic.AddUint64(gcCount, 1) -// }) -// } - -// for el := l.Front(); el != nil; { -// l.Remove(el) -// //oldEl := el -// el = el.Next() -// //oldEl.DetachPrev() -// //oldEl.DetachNext() -// } - -// runtime.GC() -// time.Sleep(time.Second * 3) -// runtime.GC() -// time.Sleep(time.Second * 3) -// _ = done - -// if *gcCount != numElements { -// t.Errorf("Expected gcCount to be %v, got %v", numElements, -// *gcCount) -// } -//} +//nolint:unused,deadcode +func _TestGCFifo(t *testing.T) { + if runtime.GOARCH != "amd64" { + t.Skipf("Skipping on non-amd64 machine") + } + + const numElements = 1000000 + l := New() + gcCount := new(uint64) + + // SetFinalizer doesn't work well with circular structures, + // so we construct a trivial non-circular structure to + // track. + type value struct { + Int int + } + done := make(chan struct{}) + + for i := 0; i < numElements; i++ { + v := new(value) + v.Int = i + l.PushBack(v) + runtime.SetFinalizer(v, func(v *value) { + atomic.AddUint64(gcCount, 1) + }) + } + + for el := l.Front(); el != nil; { + l.Remove(el) + //oldEl := el + el = el.Next() + //oldEl.DetachPrev() + //oldEl.DetachNext() + } + + runtime.GC() + time.Sleep(time.Second * 3) + runtime.GC() + time.Sleep(time.Second * 3) + _ = done + + if *gcCount != numElements { + t.Errorf("Expected gcCount to be %v, got %v", numElements, + *gcCount) + } +} // This test is quite hacky because it relies on SetFinalizer // which isn't guaranteed to run at all. -// func TestGCRandom(t *testing.T) { -// if runtime.GOARCH != "amd64" { -// t.Skipf("Skipping on non-amd64 machine") -// } - -// const numElements = 1000000 -// l := New() -// gcCount := 0 - -// // SetFinalizer doesn't work well with circular structures, -// // so we construct a trivial non-circular structure to -// // track. -// type value struct { -// Int int -// } - -// for i := 0; i < numElements; i++ { -// v := new(value) -// v.Int = i -// l.PushBack(v) -// runtime.SetFinalizer(v, func(v *value) { -// gcCount++ -// }) -// } - -// els := make([]*CElement, 0, numElements) -// for el := l.Front(); el != nil; el = el.Next() { -// els = append(els, el) -// } - -// for _, i := range cmn.RandPerm(numElements) { -// el := els[i] -// l.Remove(el) -// _ = el.Next() -// } - -// runtime.GC() -// time.Sleep(time.Second * 3) - -// if gcCount != numElements { -// t.Errorf("Expected gcCount to be %v, got %v", numElements, -// gcCount) -// } -// } +//nolint:unused,deadcode +func TestGCRandom(t *testing.T) { + if runtime.GOARCH != "amd64" { + t.Skipf("Skipping on non-amd64 machine") + } + + const numElements = 1000000 + l := New() + gcCount := 0 + + // SetFinalizer doesn't work well with circular structures, + // so we construct a trivial non-circular structure to + // track. + type value struct { + Int int + } + + for i := 0; i < numElements; i++ { + v := new(value) + v.Int = i + l.PushBack(v) + runtime.SetFinalizer(v, func(v *value) { + gcCount++ + }) + } + + els := make([]*CElement, 0, numElements) + for el := l.Front(); el != nil; el = el.Next() { + els = append(els, el) + } + + for _, i := range cmn.RandPerm(numElements) { + el := els[i] + l.Remove(el) + _ = el.Next() + } + + runtime.GC() + time.Sleep(time.Second * 3) + + if gcCount != numElements { + t.Errorf("Expected gcCount to be %v, got %v", numElements, + gcCount) + } +} func TestScanRightDeleteRandom(t *testing.T) { diff --git a/lite/proxy/query_test.go b/lite/proxy/query_test.go index 707430b61..9547f7713 100644 --- a/lite/proxy/query_test.go +++ b/lite/proxy/query_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -20,7 +21,8 @@ import ( var node *nm.Node var chainID = "tendermint_test" // TODO use from config. -// var waitForEventTimeout = 5 * time.Second +//nolint:unused +var waitForEventTimeout = 5 * time.Second // TODO fix tests!! @@ -41,83 +43,84 @@ func kvstoreTx(k, v []byte) []byte { // TODO: enable it after general proof format has been adapted // in abci/examples/kvstore.go -// func TestAppProofs(t *testing.T) { -// assert, require := assert.New(t), require.New(t) - -// prt := defaultProofRuntime() -// cl := client.NewLocal(node) -// client.WaitForHeight(cl, 1, nil) - -// // This sets up our trust on the node based on some past point. -// source := certclient.NewProvider(chainID, cl) -// seed, err := source.LatestFullCommit(chainID, 1, 1) -// require.NoError(err, "%#v", err) -// cert := lite.NewBaseVerifier(chainID, seed.Height(), seed.Validators) - -// // Wait for tx confirmation. -// done := make(chan int64) -// go func() { -// evtTyp := types.EventTx -// _, err = client.WaitForOneEvent(cl, evtTyp, waitForEventTimeout) -// require.Nil(err, "%#v", err) -// close(done) -// }() - -// // Submit a transaction. -// k := []byte("my-key") -// v := []byte("my-value") -// tx := kvstoreTx(k, v) -// br, err := cl.BroadcastTxCommit(tx) -// require.NoError(err, "%#v", err) -// require.EqualValues(0, br.CheckTx.Code, "%#v", br.CheckTx) -// require.EqualValues(0, br.DeliverTx.Code) -// brh := br.Height - -// // Fetch latest after tx commit. -// <-done -// latest, err := source.LatestFullCommit(chainID, 1, 1<<63-1) -// require.NoError(err, "%#v", err) -// rootHash := latest.SignedHeader.AppHash -// if rootHash == nil { -// // Fetch one block later, AppHash hasn't been committed yet. -// // TODO find a way to avoid doing this. -// client.WaitForHeight(cl, latest.SignedHeader.Height+1, nil) -// latest, err = source.LatestFullCommit(chainID, latest.SignedHeader.Height+1, 1<<63-1) -// require.NoError(err, "%#v", err) -// rootHash = latest.SignedHeader.AppHash -// } -// require.NotNil(rootHash) - -// // verify a query before the tx block has no data (and valid non-exist proof) -// bs, height, proof, err := GetWithProof(prt, k, brh-1, cl, cert) -// require.NoError(err, "%#v", err) -// // require.NotNil(proof) -// // TODO: Ensure that *some* keys will be there, ensuring that proof is nil, -// // (currently there's a race condition) -// // and ensure that proof proves absence of k. -// require.Nil(bs) - -// // but given that block it is good -// bs, height, proof, err = GetWithProof(prt, k, brh, cl, cert) -// require.NoError(err, "%#v", err) -// require.NotNil(proof) -// require.Equal(height, brh) - -// assert.EqualValues(v, bs) -// err = prt.VerifyValue(proof, rootHash, string(k), bs) // XXX key encoding -// assert.NoError(err, "%#v", err) - -// // Test non-existing key. -// missing := []byte("my-missing-key") -// bs, _, proof, err = GetWithProof(prt, missing, 0, cl, cert) -// require.NoError(err) -// require.Nil(bs) -// require.NotNil(proof) -// err = prt.VerifyAbsence(proof, rootHash, string(missing)) // XXX VerifyAbsence(), keyencoding -// assert.NoError(err, "%#v", err) -// err = prt.VerifyAbsence(proof, rootHash, string(k)) // XXX VerifyAbsence(), keyencoding -// assert.Error(err, "%#v", err) -// } +//nolint:unused,deadcode +func _TestAppProofs(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + prt := defaultProofRuntime() + cl := client.NewLocal(node) + client.WaitForHeight(cl, 1, nil) + + // This sets up our trust on the node based on some past point. + source := certclient.NewProvider(chainID, cl) + seed, err := source.LatestFullCommit(chainID, 1, 1) + require.NoError(err, "%#v", err) + cert := lite.NewBaseVerifier(chainID, seed.Height(), seed.Validators) + + // Wait for tx confirmation. + done := make(chan int64) + go func() { + evtTyp := types.EventTx + _, err = client.WaitForOneEvent(cl, evtTyp, waitForEventTimeout) + require.Nil(err, "%#v", err) + close(done) + }() + + // Submit a transaction. + k := []byte("my-key") + v := []byte("my-value") + tx := kvstoreTx(k, v) + br, err := cl.BroadcastTxCommit(tx) + require.NoError(err, "%#v", err) + require.EqualValues(0, br.CheckTx.Code, "%#v", br.CheckTx) + require.EqualValues(0, br.DeliverTx.Code) + brh := br.Height + + // Fetch latest after tx commit. + <-done + latest, err := source.LatestFullCommit(chainID, 1, 1<<63-1) + require.NoError(err, "%#v", err) + rootHash := latest.SignedHeader.AppHash + if rootHash == nil { + // Fetch one block later, AppHash hasn't been committed yet. + // TODO find a way to avoid doing this. + client.WaitForHeight(cl, latest.SignedHeader.Height+1, nil) + latest, err = source.LatestFullCommit(chainID, latest.SignedHeader.Height+1, 1<<63-1) + require.NoError(err, "%#v", err) + rootHash = latest.SignedHeader.AppHash + } + require.NotNil(rootHash) + + // verify a query before the tx block has no data (and valid non-exist proof) + bs, height, proof, err := GetWithProof(prt, k, brh-1, cl, cert) + require.NoError(err, "%#v", err) + // require.NotNil(proof) + // TODO: Ensure that *some* keys will be there, ensuring that proof is nil, + // (currently there's a race condition) + // and ensure that proof proves absence of k. + require.Nil(bs) + + // but given that block it is good + bs, height, proof, err = GetWithProof(prt, k, brh, cl, cert) + require.NoError(err, "%#v", err) + require.NotNil(proof) + require.Equal(height, brh) + + assert.EqualValues(v, bs) + err = prt.VerifyValue(proof, rootHash, string(k), bs) // XXX key encoding + assert.NoError(err, "%#v", err) + + // Test non-existing key. + missing := []byte("my-missing-key") + bs, _, proof, err = GetWithProof(prt, missing, 0, cl, cert) + require.NoError(err) + require.Nil(bs) + require.NotNil(proof) + err = prt.VerifyAbsence(proof, rootHash, string(missing)) // XXX VerifyAbsence(), keyencoding + assert.NoError(err, "%#v", err) + err = prt.VerifyAbsence(proof, rootHash, string(k)) // XXX VerifyAbsence(), keyencoding + assert.Error(err, "%#v", err) +} func TestTxProofs(t *testing.T) { assert, require := assert.New(t), require.New(t) diff --git a/p2p/trust/metric_test.go b/p2p/trust/metric_test.go index 89327c0e1..0273dad69 100644 --- a/p2p/trust/metric_test.go +++ b/p2p/trust/metric_test.go @@ -65,44 +65,45 @@ func TestTrustMetricCopyNilPointer(t *testing.T) { } // XXX: This test fails non-deterministically -// func _TestTrustMetricStopPause(t *testing.T) { -// // The TestTicker will provide manual control over -// // the passing of time within the metric -// tt := NewTestTicker() -// tm := NewMetric() -// tm.SetTicker(tt) -// tm.Start() -// // Allow some time intervals to pass and pause -// tt.NextTick() -// tt.NextTick() -// tm.Pause() - -// // could be 1 or 2 because Pause and NextTick race -// first := tm.Copy().numIntervals - -// // Allow more time to pass and check the intervals are unchanged -// tt.NextTick() -// tt.NextTick() -// assert.Equal(t, first, tm.Copy().numIntervals) - -// // Get the trust metric activated again -// tm.GoodEvents(5) -// // Allow some time intervals to pass and stop -// tt.NextTick() -// tt.NextTick() -// tm.Stop() -// tm.Wait() - -// second := tm.Copy().numIntervals -// // Allow more intervals to pass while the metric is stopped -// // and check that the number of intervals match -// tm.NextTimeInterval() -// tm.NextTimeInterval() -// // XXX: fails non-deterministically: -// // expected 5, got 6 -// assert.Equal(t, second+2, tm.Copy().numIntervals) - -// if first > second { -// t.Fatalf("numIntervals should always increase or stay the same over time") -// } -// } +//nolint:unused,deadcode +func _TestTrustMetricStopPause(t *testing.T) { + // The TestTicker will provide manual control over + // the passing of time within the metric + tt := NewTestTicker() + tm := NewMetric() + tm.SetTicker(tt) + tm.Start() + // Allow some time intervals to pass and pause + tt.NextTick() + tt.NextTick() + tm.Pause() + + // could be 1 or 2 because Pause and NextTick race + first := tm.Copy().numIntervals + + // Allow more time to pass and check the intervals are unchanged + tt.NextTick() + tt.NextTick() + assert.Equal(t, first, tm.Copy().numIntervals) + + // Get the trust metric activated again + tm.GoodEvents(5) + // Allow some time intervals to pass and stop + tt.NextTick() + tt.NextTick() + tm.Stop() + tm.Wait() + + second := tm.Copy().numIntervals + // Allow more intervals to pass while the metric is stopped + // and check that the number of intervals match + tm.NextTimeInterval() + tm.NextTimeInterval() + // XXX: fails non-deterministically: + // expected 5, got 6 + assert.Equal(t, second+2, tm.Copy().numIntervals) + + if first > second { + t.Fatalf("numIntervals should always increase or stay the same over time") + } +} From 1a35895ac80caa50418ed1172ec80e71a0a6b692 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 6 Feb 2019 18:23:25 +0400 Subject: [PATCH 7/8] remove gotype linter WARN [runner/nolint] Found unknown linters in //nolint directives: gotype --- consensus/byzantine_test.go | 3 +-- consensus/state.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go index 6f46c04d3..ba69d0cc1 100644 --- a/consensus/byzantine_test.go +++ b/consensus/byzantine_test.go @@ -76,8 +76,7 @@ func TestByzantine(t *testing.T) { conR.SetLogger(logger.With("validator", i)) conR.SetEventBus(eventBus) - var conRI p2p.Reactor // nolint: gotype, gosimple - conRI = conR + var conRI p2p.Reactor = conR // make first val byzantine if i == 0 { diff --git a/consensus/state.go b/consensus/state.go index c8185d46e..74165801b 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -454,7 +454,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", tmtime.Now(), "startTime", cs.StartTime) - sleepDuration := rs.StartTime.Sub(tmtime.Now()) // nolint: gotype, gosimple + sleepDuration := rs.StartTime.Sub(tmtime.Now()) cs.scheduleTimeout(sleepDuration, rs.Height, 0, cstypes.RoundStepNewHeight) } From 1386707ceb922f8cd390105c86c6a598c044f748 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 6 Feb 2019 18:36:54 +0400 Subject: [PATCH 8/8] rename TestGCRandom to _TestGCRandom --- libs/clist/clist_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/clist/clist_test.go b/libs/clist/clist_test.go index 7fb7db4d2..13aca3577 100644 --- a/libs/clist/clist_test.go +++ b/libs/clist/clist_test.go @@ -117,7 +117,7 @@ func _TestGCFifo(t *testing.T) { // This test is quite hacky because it relies on SetFinalizer // which isn't guaranteed to run at all. //nolint:unused,deadcode -func TestGCRandom(t *testing.T) { +func _TestGCRandom(t *testing.T) { if runtime.GOARCH != "amd64" { t.Skipf("Skipping on non-amd64 machine") }