From 1cd9bdb80bc97ca0e90a78103d2640b844625637 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 1 Feb 2021 15:32:37 +0400 Subject: [PATCH] light/provider/http: fix Validators (#6022) Closes #6010 --- CHANGELOG_PENDING.md | 4 +++- light/provider/http/http.go | 46 ++++++++++++++++++++++++++----------- rpc/client/http/http.go | 4 ++-- rpc/client/rpc_test.go | 5 ++-- types/validator_set.go | 11 ++++++--- 5 files changed, 47 insertions(+), 23 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 4cc96be70..c27201962 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -30,6 +30,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [libs/os] Kill() and {Must,}{Read,Write}File() functions have been removed. (@alessio) - [store] \#5848 Remove block store state in favor of using the db iterators directly (@cmwaters) - [state] \#5864 Use an iterator when pruning state (@cmwaters) + - [rpc/client/http] \#6022 Change `timeout` type to `time.Duration` in `NewWithTimeout` - Blockchain Protocol @@ -42,7 +43,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [crypto/ed25519] \#5632 Adopt zip215 `ed25519` verification. (@marbar3778) - [privval] \#5603 Add `--key` to `init`, `gen_validator`, `testnet` & `unsafe_reset_priv_validator` for use in generating `secp256k1` keys. -- [privval] \#5725 Add gRPC support to private validator. +- [privval] \#5725 Add gRPC support to private validator. - [privval] \#5876 `tendermint show-validator` will query the remote signer if gRPC is being used (@marbar3778) - [abci/client] \#5673 `Async` requests return an error if queue is full (@melekes) - [mempool] \#5673 Cancel `CheckTx` requests if RPC client disconnects or times out (@melekes) @@ -62,3 +63,4 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [privval] \#5638 Increase read/write timeout to 5s and calculate ping interval based on it (@JoeKash) - [blockchain/v1] [\#5701](https://github.com/tendermint/tendermint/pull/5701) Handle peers without blocks (@melekes) - [blockchain/v1] \#5711 Fix deadlock (@melekes) +- [light] \#6022 Fix a bug when the number of validators equals 100 (@melekes) diff --git a/light/provider/http/http.go b/light/provider/http/http.go index f6b616cdd..c28d732ca 100644 --- a/light/provider/http/http.go +++ b/light/provider/http/http.go @@ -14,10 +14,12 @@ import ( "github.com/tendermint/tendermint/types" ) -// This is very brittle, see: https://github.com/tendermint/tendermint/issues/4740 var ( + // This is very brittle, see: https://github.com/tendermint/tendermint/issues/4740 regexpMissingHeight = regexp.MustCompile(`height \d+ (must be less than or equal to|is not available)`) - maxRetryAttempts = 10 + + maxRetryAttempts = 10 + timeout = 5 * time.Second ) // http provider uses an RPC client to obtain the necessary information. @@ -28,14 +30,14 @@ type http struct { // New creates a HTTP provider, which is using the rpchttp.HTTP client under // the hood. If no scheme is provided in the remote URL, http will be used by -// default. +// default. The 5s timeout is used for all requests. func New(chainID, remote string) (provider.Provider, error) { // Ensure URL scheme is set (default HTTP) when not provided. if !strings.Contains(remote, "://") { remote = "http://" + remote } - httpClient, err := rpchttp.New(remote, "/websocket") + httpClient, err := rpchttp.NewWithTimeout(remote, "/websocket", timeout) if err != nil { return nil, err } @@ -93,15 +95,21 @@ func (p *http) ReportEvidence(ctx context.Context, ev types.Evidence) error { } func (p *http) validatorSet(ctx context.Context, height *int64) (*types.ValidatorSet, error) { + // Since the malicious node could report a massive number of pages, making us + // spend a considerable time iterating, we restrict the number of pages here. + // => 10000 validators max + const maxPages = 100 + var ( - maxPerPage = 100 - vals = []*types.Validator{} - page = 1 + perPage = 100 + vals = []*types.Validator{} + page = 1 + total = -1 ) - for len(vals)%maxPerPage == 0 { + for len(vals) != total && page <= maxPages { for attempt := 1; attempt <= maxRetryAttempts; attempt++ { - res, err := p.client.Validators(ctx, height, &page, &maxPerPage) + res, err := p.client.Validators(ctx, height, &page, &perPage) if err != nil { // TODO: standardize errors on the RPC side if regexpMissingHeight.MatchString(err.Error()) { @@ -115,18 +123,28 @@ func (p *http) validatorSet(ctx context.Context, height *int64) (*types.Validato time.Sleep(backoffTimeout(uint16(attempt))) continue } - if len(res.Validators) == 0 { // no more validators left - valSet, err := types.ValidatorSetFromExistingValidators(vals) - if err != nil { - return nil, provider.ErrBadLightBlock{Reason: err} + + // Validate response. + if len(res.Validators) == 0 { + return nil, provider.ErrBadLightBlock{ + Reason: fmt.Errorf("validator set is empty (height: %d, page: %d, per_page: %d)", + height, page, perPage), } - return valSet, nil } + if res.Total <= 0 { + return nil, provider.ErrBadLightBlock{ + Reason: fmt.Errorf("total number of vals is <= 0: %d (height: %d, page: %d, per_page: %d)", + res.Total, height, page, perPage), + } + } + + total = res.Total vals = append(vals, res.Validators...) page++ break } } + valSet, err := types.ValidatorSetFromExistingValidators(vals) if err != nil { return nil, provider.ErrBadLightBlock{Reason: err} diff --git a/rpc/client/http/http.go b/rpc/client/http/http.go index 8fc0c7a91..e92a3884c 100644 --- a/rpc/client/http/http.go +++ b/rpc/client/http/http.go @@ -118,12 +118,12 @@ func New(remote, wsEndpoint string) (*HTTP, error) { // NewWithTimeout does the same thing as New, except you can set a Timeout for // http.Client. A Timeout of zero means no timeout. -func NewWithTimeout(remote, wsEndpoint string, timeout uint) (*HTTP, error) { +func NewWithTimeout(remote, wsEndpoint string, timeout time.Duration) (*HTTP, error) { httpClient, err := jsonrpcclient.DefaultHTTPClient(remote) if err != nil { return nil, err } - httpClient.Timeout = time.Duration(timeout) * time.Second + httpClient.Timeout = timeout return NewWithClient(remote, wsEndpoint, httpClient) } diff --git a/rpc/client/rpc_test.go b/rpc/client/rpc_test.go index ed719bed0..7232ed9dd 100644 --- a/rpc/client/rpc_test.go +++ b/rpc/client/rpc_test.go @@ -40,7 +40,7 @@ func getHTTPClient() *rpchttp.HTTP { return c } -func getHTTPClientWithTimeout(timeout uint) *rpchttp.HTTP { +func getHTTPClientWithTimeout(timeout time.Duration) *rpchttp.HTTP { rpcAddr := rpctest.GetConfig().RPC.ListenAddress c, err := rpchttp.NewWithTimeout(rpcAddr, "/websocket", timeout) if err != nil { @@ -497,8 +497,7 @@ func TestTx(t *testing.T) { } func TestTxSearchWithTimeout(t *testing.T) { - // Get a client with a time-out of 10 secs. - timeoutClient := getHTTPClientWithTimeout(10) + timeoutClient := getHTTPClientWithTimeout(10 * time.Second) _, _, tx := MakeTxKV() _, err := timeoutClient.BroadcastTxCommit(context.Background(), tx) diff --git a/types/validator_set.go b/types/validator_set.go index 7cff0405d..3c797db76 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -991,16 +991,21 @@ func ValidatorSetFromProto(vp *tmproto.ValidatorSet) (*ValidatorSet, error) { return vals, vals.ValidateBasic() } -// ValidatorSetFromExistingValidators takes an existing array of validators and rebuilds -// the exact same validator set that corresponds to it without changing the proposer priority or power -// if any of the validators fail validate basic then an empty set is returned. +// ValidatorSetFromExistingValidators takes an existing array of validators and +// rebuilds the exact same validator set that corresponds to it without +// changing the proposer priority or power if any of the validators fail +// validate basic then an empty set is returned. func ValidatorSetFromExistingValidators(valz []*Validator) (*ValidatorSet, error) { + if len(valz) == 0 { + return nil, errors.New("validator set is empty") + } for _, val := range valz { err := val.ValidateBasic() if err != nil { return nil, fmt.Errorf("can't create validator set: %w", err) } } + vals := &ValidatorSet{ Validators: valz, }