diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index c933d0062..a9f14a9e0 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -23,3 +23,5 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi ### IMPROVEMENTS ### BUG FIXES + +- [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 fd42d75d7..3f4ee842a 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 uint = 5 // sec. ) // 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 } @@ -98,15 +100,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()) { @@ -120,18 +128,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/types/validator_set.go b/types/validator_set.go index af7b4034e..c808adb0c 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -982,16 +982,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, }