From 3e1c88fda83b357a52fff745e5469f32291d9715 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Sat, 25 Apr 2020 14:34:01 +0200 Subject: [PATCH 1/2] lite: fix HTTP provider error handling Fixes #4739, kind of. See #4740 for the proper fix. --- For contributor use: - [x] Wrote tests - [x] Updated CHANGELOG_PENDING.md - [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [x] Updated relevant documentation (`docs/`) and code comments - [x] Re-reviewed `Files changed` in the Github PR explorer - [x] Applied Appropriate Labels --- CHANGELOG_PENDING.md | 2 ++ lite2/provider/http/http.go | 8 ++++++-- lite2/provider/http/http_test.go | 21 ++++++++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 52ee5ab2b..095582b04 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -28,3 +28,5 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [txindex] [\#4466](https://github.com/tendermint/tendermint/pull/4466) Allow to index an event at runtime (@favadi) ### BUG FIXES: + +- [light] [\#4741](https://github.com/tendermint/tendermint/pull/4741) Correctly return `ErrSignedHeaderNotFound` and `ErrValidatorSetNotFound` on corresponding RPC errors (@erikgrinaker) diff --git a/lite2/provider/http/http.go b/lite2/provider/http/http.go index 30fdaf2e8..360338f75 100644 --- a/lite2/provider/http/http.go +++ b/lite2/provider/http/http.go @@ -3,6 +3,7 @@ package http import ( "errors" "fmt" + "regexp" "strings" "github.com/tendermint/tendermint/lite2/provider" @@ -11,6 +12,9 @@ import ( "github.com/tendermint/tendermint/types" ) +// This is very brittle, see: https://github.com/tendermint/tendermint/issues/4740 +var regexpMissingHeight = regexp.MustCompile(`height \d+ (must be less than or equal to|is not available)`) + // http provider uses an RPC client to obtain the necessary information. type http struct { chainID string @@ -62,7 +66,7 @@ func (p *http) SignedHeader(height int64) (*types.SignedHeader, error) { commit, err := p.client.Commit(h) if err != nil { // TODO: standartise errors on the RPC side - if strings.Contains(err.Error(), "height must be less than or equal") { + if regexpMissingHeight.MatchString(err.Error()) { return nil, provider.ErrSignedHeaderNotFound } return nil, err @@ -92,7 +96,7 @@ func (p *http) ValidatorSet(height int64) (*types.ValidatorSet, error) { res, err := p.client.Validators(h, 0, maxPerPage) if err != nil { // TODO: standartise errors on the RPC side - if strings.Contains(err.Error(), "height must be less than or equal") { + if regexpMissingHeight.MatchString(err.Error()) { return nil, provider.ErrValidatorSetNotFound } return nil, err diff --git a/lite2/provider/http/http_test.go b/lite2/provider/http/http_test.go index 49c3d3622..33ab63a37 100644 --- a/lite2/provider/http/http_test.go +++ b/lite2/provider/http/http_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/abci/example/kvstore" + "github.com/tendermint/tendermint/lite2/provider" "github.com/tendermint/tendermint/lite2/provider/http" litehttp "github.com/tendermint/tendermint/lite2/provider/http" rpcclient "github.com/tendermint/tendermint/rpc/client" @@ -33,6 +34,7 @@ func TestNewProvider(t *testing.T) { func TestMain(m *testing.M) { app := kvstore.NewApplication() + app.RetainBlocks = 5 node := rpctest.StartTendermint(app) code := m.Run() @@ -73,8 +75,25 @@ func TestProvider(t *testing.T) { assert.Nil(t, sh.ValidateBasic(chainID)) // historical queries now work :) - lower := sh.Height - 5 + lower := sh.Height - 3 sh, err = p.SignedHeader(lower) assert.Nil(t, err, "%+v", err) assert.Equal(t, lower, sh.Height) + + // fetching missing heights (both future and pruned) should return appropriate errors + _, err = p.SignedHeader(1000) + require.Error(t, err) + assert.Equal(t, provider.ErrSignedHeaderNotFound, err) + + _, err = p.ValidatorSet(1000) + require.Error(t, err) + assert.Equal(t, provider.ErrValidatorSetNotFound, err) + + _, err = p.SignedHeader(1) + require.Error(t, err) + assert.Equal(t, provider.ErrSignedHeaderNotFound, err) + + _, err = p.ValidatorSet(1) + require.Error(t, err) + assert.Equal(t, provider.ErrValidatorSetNotFound, err) } From 8c6d1e669fcbb44f3db2cc97c25e0eea72332964 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Sat, 25 Apr 2020 19:16:58 +0100 Subject: [PATCH 2/2] Makefile: parse TENDERMINT_BUILD_OPTIONS (#4738) Reduce the number of targets and make the buildsystem more flexible by parsing the TENDERMINT_BUILD_OPTIONS command line variable (a-la Debian, inspired by dpkg-buildpackage's DEB_BUILD_OPTIONS), e.g: $ make install TENDERMINT_BUILD_OPTIONS='cleveldb' replaces the old: $ make install_c Options can be mix&match'd, e.g.: $ make install TENDERMINT_BUILD_OPTIONS='cleveldb race nostrip' Three options are available: - nostrip: don't strip debugging symbols nor DWARF tables. - cleveldb: use cleveldb as db backend instead of goleveldb; it switches on the CGO_ENABLED Go environment variale. - race: pass -race to go build and enable data race detection. This changeset is a port of gaia pull request: cosmos/gaia#363. --- CHANGELOG_PENDING.md | 4 ++ DOCKER/Dockerfile.build_c-amazonlinux | 2 +- Makefile | 48 +++++++++++-------- docs/introduction/install.md | 4 +- docs/tendermint-core/running-in-production.md | 2 +- 5 files changed, 37 insertions(+), 23 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 095582b04..6b289bbd1 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -26,6 +26,10 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [abci/server] [\#4719](https://github.com/tendermint/tendermint/pull/4719) Print panic & stack trace to STDERR if logger is not set (@melekes) - [types] [\#4638](https://github.com/tendermint/tendermint/pull/4638) Implement `Header#ValidateBasic` (@alexanderbez) - [txindex] [\#4466](https://github.com/tendermint/tendermint/pull/4466) Allow to index an event at runtime (@favadi) +- [buildsystem] [\#4378](https://github.com/tendermint/tendermint/pull/4738) Replace build_c and install_c with TENDERMINT_BUILD_OPTIONS parsing. The following options are available: + - nostrip: don't strip debugging symbols nor DWARF tables. + - cleveldb: use cleveldb as db backend instead of goleveldb. + - race: pass -race to go build and enable data race detection. ### BUG FIXES: diff --git a/DOCKER/Dockerfile.build_c-amazonlinux b/DOCKER/Dockerfile.build_c-amazonlinux index 64babe3ae..05bc7e265 100644 --- a/DOCKER/Dockerfile.build_c-amazonlinux +++ b/DOCKER/Dockerfile.build_c-amazonlinux @@ -24,5 +24,5 @@ ENV GOPATH=/go/src RUN mkdir -p /tendermint WORKDIR /tendermint -CMD ["/usr/bin/make", "build_c"] +CMD ["/usr/bin/make", "build", "TENDERMINT_BUILD_OPTIONS=cleveldb"] diff --git a/Makefile b/Makefile index eb9626bf4..c7a389476 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,33 @@ PACKAGES=$(shell go list ./...) OUTPUT?=build/tendermint -BUILD_TAGS?='tendermint' -LD_FLAGS = -X github.com/tendermint/tendermint/version.GitCommit=`git rev-parse --short=8 HEAD` -s -w +BUILD_TAGS?=tendermint +LD_FLAGS = -X github.com/tendermint/tendermint/version.GitCommit=`git rev-parse --short=8 HEAD` BUILD_FLAGS = -mod=readonly -ldflags "$(LD_FLAGS)" HTTPS_GIT := https://github.com/tendermint/tendermint.git DOCKER_BUF := docker run -v $(shell pwd):/workspace --workdir /workspace bufbuild/buf +CGO_ENABLED ?= 0 + +# handle nostrip +ifeq (,$(findstring nostrip,$(TENDERMINT_BUILD_OPTIONS))) + BUILD_FLAGS += -trimpath + LD_FLAGS += -s -w +endif + +# handle race +ifeq (race,$(findstring race,$(TENDERMINT_BUILD_OPTIONS))) + CGO_ENABLED=1 + BUILD_FLAGS += -race +endif + +# handle cleveldb +ifeq (cleveldb,$(findstring cleveldb,$(TENDERMINT_BUILD_OPTIONS))) + CGO_ENABLED=1 + BUILD_TAGS += cleveldb +endif + +# allow users to pass additional flags via the conventional LDFLAGS variable +LD_FLAGS += $(LDFLAGS) all: check build test install .PHONY: all @@ -19,25 +41,13 @@ include tests.mk ############################################################################### build: - CGO_ENABLED=0 go build $(BUILD_FLAGS) -tags $(BUILD_TAGS) -o $(OUTPUT) ./cmd/tendermint/ + CGO_ENABLED=$(CGO_ENABLED) go build $(BUILD_FLAGS) -tags '$(BUILD_TAGS)' -o $(OUTPUT) ./cmd/tendermint/ .PHONY: build -build_c: - CGO_ENABLED=1 go build $(BUILD_FLAGS) -tags "$(BUILD_TAGS) cleveldb" -o $(OUTPUT) ./cmd/tendermint/ -.PHONY: build_c - -build_race: - CGO_ENABLED=1 go build -race $(BUILD_FLAGS) -tags $(BUILD_TAGS) -o $(OUTPUT) ./cmd/tendermint -.PHONY: build_race - install: - CGO_ENABLED=0 go install $(BUILD_FLAGS) -tags $(BUILD_TAGS) ./cmd/tendermint + CGO_ENABLED=$(CGO_ENABLED) go install $(BUILD_FLAGS) -tags $(BUILD_TAGS) ./cmd/tendermint .PHONY: install -install_c: - CGO_ENABLED=1 go install $(BUILD_FLAGS) -tags "$(BUILD_TAGS) cleveldb" ./cmd/tendermint -.PHONY: install_c - ############################################################################### ### Protobuf ### ############################################################################### @@ -197,9 +207,9 @@ build-docker-localnode: @cd networks/local && make .PHONY: build-docker-localnode -# Runs `make build_c` from within an Amazon Linux (v2)-based Docker build -# container in order to build an Amazon Linux-compatible binary. Produces a -# compatible binary at ./build/tendermint +# Runs `make build TENDERMINT_BUILD_OPTIONS=cleveldb` from within an Amazon +# Linux (v2)-based Docker build container in order to build an Amazon +# Linux-compatible binary. Produces a compatible binary at ./build/tendermint build_c-amazonlinux: $(MAKE) -C ./DOCKER build_amazonlinux_buildimage docker run --rm -it -v `pwd`:/tendermint tendermint/tendermint:build_c-amazonlinux diff --git a/docs/introduction/install.md b/docs/introduction/install.md index b94230cc1..878f6d043 100644 --- a/docs/introduction/install.md +++ b/docs/introduction/install.md @@ -119,13 +119,13 @@ db_backend = "cleveldb" To install Tendermint, run: ``` -CGO_LDFLAGS="-lsnappy" make install_c +CGO_LDFLAGS="-lsnappy" make install TENDERMINT_BUILD_OPTIONS=cleveldb ``` or run: ``` -CGO_LDFLAGS="-lsnappy" make build_c +CGO_LDFLAGS="-lsnappy" make build TENDERMINT_BUILD_OPTIONS=cleveldb ``` which puts the binary in `./build`. diff --git a/docs/tendermint-core/running-in-production.md b/docs/tendermint-core/running-in-production.md index d386308de..20eb8910d 100644 --- a/docs/tendermint-core/running-in-production.md +++ b/docs/tendermint-core/running-in-production.md @@ -10,7 +10,7 @@ By default, Tendermint uses the `syndtr/goleveldb` package for its in-process key-value database. Unfortunately, this implementation of LevelDB seems to suffer under heavy load (see [#226](https://github.com/syndtr/goleveldb/issues/226)). It may be best to install the real C-implementation of LevelDB and compile Tendermint to use -that using `make build_c`. See the [install instructions](../introduction/install.md) for details. +that using `make build TENDERMINT_BUILD_OPTIONS=cleveldb`. See the [install instructions](../introduction/install.md) for details. Tendermint keeps multiple distinct databases in the `$TMROOT/data`: