diff --git a/light/client.go b/light/client.go index 33afee2e3..529bb7907 100644 --- a/light/client.go +++ b/light/client.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "math/rand" "time" "github.com/tendermint/tendermint/libs/log" @@ -681,7 +680,7 @@ func (c *Client) verifySkipping( if depth == len(blockCache)-1 { pivotHeight := verifiedBlock.Height + (blockCache[depth].Height-verifiedBlock. Height)*verifySkippingNumerator/verifySkippingDenominator - interimBlock, err := c.lightBlockFrom(pivotHeight, source) + interimBlock, err := source.LightBlock(pivotHeight) if err != nil { return ErrVerificationFailed{From: verifiedBlock.Height, To: pivotHeight, Reason: err} } @@ -855,36 +854,6 @@ func (c *Client) updateTrustedLightBlock(l *types.LightBlock) error { return nil } -// 0 - latest header -// Note it does not do retries nor swapping. -func (c *Client) lightBlockFromWitness(height int64, - witness provider.Provider) (*types.LightBlock, *errBadWitness) { - - l, err := witness.LightBlock(height) - if err != nil { - return nil, &errBadWitness{err, noResponse, -1} - } - err = c.validateLightBlock(l, height) - if err != nil { - return nil, &errBadWitness{err, invalidLightBlock, -1} - } - - return l, nil -} - -func (c *Client) lightBlockFrom(height int64, - source provider.Provider) (*types.LightBlock, error) { - - c.providerMutex.Lock() - sourceIsPrimary := (c.primary == source) - c.providerMutex.Unlock() - - if sourceIsPrimary { - return c.lightBlockFromPrimary(height) - } - return c.lightBlockFromWitness(height, source) -} - // backwards verification (see VerifyHeaderBackwards func in the spec) verifies // headers before a trusted header. If a sent header is invalid the primary is // replaced with another provider and the operation is repeated. @@ -932,46 +901,44 @@ func (c *Client) compareNewHeaderWithWitnesses(l *types.LightBlock, now time.Tim // 1. Make sure AT LEAST ONE witness returns the same header. var headerMatched bool - for attempt := uint16(1); attempt <= c.maxRetryAttempts; attempt++ { - if len(c.witnesses) == 0 { - return errNoWitnesses{} - } - // launch one goroutine per witness - errc := make(chan error, len(c.witnesses)) - for i, witness := range c.witnesses { - go c.compareNewHeaderWithWitness(errc, l, witness, i, now) - } + if len(c.witnesses) == 0 { + return errNoWitnesses{} + } - witnessesToRemove := make([]int, 0) - - // handle errors as they come - for i := 0; i < cap(errc); i++ { - err := <-errc - - switch e := err.(type) { - case nil: // at least one header matched - headerMatched = true - case errBadWitness: - c.logger.Info("Bad witness", "witness", c.witnesses[e.WitnessIndex], "err", err) - // if witness sent us invalid header / vals, remove it - if e.Code == invalidLightBlock { - c.logger.Info("Witness sent us invalid header / vals -> removing it", "witness", c.witnesses[e.WitnessIndex]) - witnessesToRemove = append(witnessesToRemove, e.WitnessIndex) - } - } - } + // launch one goroutine per witness + errc := make(chan error, len(c.witnesses)) + for i, witness := range c.witnesses { + go c.compareNewHeaderWithWitness(errc, l, witness, i, now) + } - for _, idx := range witnessesToRemove { - c.removeWitness(idx) - } + witnessesToRemove := make([]int, 0) - if headerMatched { - return nil + // handle errors as they come + for i := 0; i < cap(errc); i++ { + err := <-errc + + switch e := err.(type) { + case nil: // at least one header matched + headerMatched = true + case errBadWitness: + c.logger.Info("Requested light block from bad witness", "witness", c.witnesses[e.WitnessIndex], "err", err) + // if witness sent us invalid header / vals, remove it + if e.Code == invalidLightBlock { + c.logger.Info("Witness sent us invalid header / vals -> removing it", "witness", c.witnesses[e.WitnessIndex]) + witnessesToRemove = append(witnessesToRemove, e.WitnessIndex) + } + default: + c.logger.Info("Requested light block from witness but got error", "error", err) } + } - // 2. Otherwise, sleep - time.Sleep(backoffTimeout(attempt)) + for _, idx := range witnessesToRemove { + c.removeWitness(idx) + } + + if headerMatched { + return nil } return errors.New("awaiting response from all witnesses exceeded dropout time") @@ -980,16 +947,21 @@ func (c *Client) compareNewHeaderWithWitnesses(l *types.LightBlock, now time.Tim func (c *Client) compareNewHeaderWithWitness(errc chan error, l *types.LightBlock, witness provider.Provider, witnessIndex int, now time.Time) { - altBlock, err := c.lightBlockFromWitness(l.Height, witness) + altBlock, err := witness.LightBlock(l.Height) if err != nil { - err.WitnessIndex = witnessIndex - errc <- err + if _, ok := err.(provider.ErrBadLightBlock); ok { + errc <- errBadWitness{Reason: err, Code: invalidLightBlock, WitnessIndex: witnessIndex} + } else { + errc <- err + } + // if not a bad light block, then the witness has either not responded or + // doesn't have the block -> we ignore return } if !bytes.Equal(l.Hash(), altBlock.Hash()) { if bsErr := c.verifySkipping(witness, c.latestTrustedBlock, altBlock, now); bsErr != nil { - errc <- errBadWitness{bsErr, invalidLightBlock, witnessIndex} + errc <- errBadWitness{Reason: bsErr, Code: invalidLightBlock, WitnessIndex: witnessIndex} return } } @@ -1061,58 +1033,19 @@ func (c *Client) replacePrimaryProvider() error { // at the specified height. Handles dropout by the primary provider by swapping // with an alternative provider. func (c *Client) lightBlockFromPrimary(height int64) (*types.LightBlock, error) { - for attempt := uint16(1); attempt <= c.maxRetryAttempts; attempt++ { - c.providerMutex.Lock() - l, providerErr := c.primary.LightBlock(height) - c.providerMutex.Unlock() - if providerErr == nil { - err := c.validateLightBlock(l, height) - if err != nil { - replaceErr := c.replacePrimaryProvider() - if replaceErr != nil { - return nil, fmt.Errorf("%v. Tried to replace primary but: %w", err.Error(), replaceErr) - } - // replace primary and request a light block again - return c.lightBlockFromPrimary(height) - } - // valid light block has been received - return l, nil - } - if providerErr == provider.ErrLightBlockNotFound { - return nil, providerErr - } - c.logger.Error("Failed to get signed header from primary", "attempt", attempt, "err", providerErr) - time.Sleep(backoffTimeout(attempt)) - } - - err := c.replacePrimaryProvider() - if err != nil { - return nil, fmt.Errorf("primary dropped out. Tried to replace but: %w", err) - } - - return c.lightBlockFromPrimary(height) -} - -func (c *Client) validateLightBlock(l *types.LightBlock, expectedHeight int64) error { - if l == nil { - return errors.New("nil header") - } - err := l.ValidateBasic(c.chainID) + c.providerMutex.Lock() + l, err := c.primary.LightBlock(height) + c.providerMutex.Unlock() if err != nil { - return err - } - - if expectedHeight > 0 && l.Height != expectedHeight { - return fmt.Errorf("height mismatch, got: %d, expected: %d", l.Height, expectedHeight) + c.logger.Debug("Error on light block request from primary", "error", err) + replaceErr := c.replacePrimaryProvider() + if replaceErr != nil { + return nil, fmt.Errorf("%v. Tried to replace primary but: %w", err.Error(), replaceErr) + } + // replace primary and request a light block again + return c.lightBlockFromPrimary(height) } - return nil -} - -// exponential backoff (with jitter) -// 0.5s -> 2s -> 4.5s -> 8s -> 12.5 with 1s variation -func backoffTimeout(attempt uint16) time.Duration { - // nolint:gosec // G404: Use of weak random number generator - return time.Duration(500*attempt*attempt)*time.Millisecond + time.Duration(rand.Intn(1000))*time.Millisecond + return l, err } func hash2str(hash []byte) string { diff --git a/light/client_test.go b/light/client_test.go index c9765c885..d4f17023d 100644 --- a/light/client_test.go +++ b/light/client_test.go @@ -889,7 +889,6 @@ func TestClientRemovesWitnessIfItSendsUsIncorrectHeader(t *testing.T) { map[int64]*types.SignedHeader{ 1: h1, 2: h2, - 3: {Header: nil, Commit: nil}, }, map[int64]*types.ValidatorSet{ 1: vals, @@ -927,18 +926,7 @@ func TestClientRemovesWitnessIfItSendsUsIncorrectHeader(t *testing.T) { } func TestClient_TrustedValidatorSet(t *testing.T) { - noValSetNode := mockp.New( - chainID, - headerSet, - map[int64]*types.ValidatorSet{ - 1: nil, - 2: nil, - 3: nil, - }, - ) - differentVals, _ := types.RandValidatorSet(10, 100) - badValSetNode := mockp.New( chainID, map[int64]*types.SignedHeader{ @@ -960,8 +948,8 @@ func TestClient_TrustedValidatorSet(t *testing.T) { c, err := light.NewClient( chainID, trustOptions, - noValSetNode, - []provider.Provider{badValSetNode, fullNode, fullNode}, + fullNode, + []provider.Provider{badValSetNode, fullNode}, dbs.New(dbm.NewMemDB(), chainID), light.Logger(log.TestingLogger()), ) diff --git a/light/provider/errors.go b/light/provider/errors.go index 0dea0dd1d..5d24efd73 100644 --- a/light/provider/errors.go +++ b/light/provider/errors.go @@ -1,9 +1,25 @@ package provider -import "errors" +import ( + "errors" + "fmt" +) var ( // ErrLightBlockNotFound is returned when a provider can't find the // requested header. ErrLightBlockNotFound = errors.New("light block not found") + // ErrNoResponse is returned if the provider doesn't respond to the + // request in a gieven time + ErrNoResponse = errors.New("client failed to respond") ) + +// ErrBadLightBlock is returned when a provider returns an invalid +// light block. +type ErrBadLightBlock struct { + Reason error +} + +func (e ErrBadLightBlock) Error() string { + return fmt.Sprintf("client provided bad signed header: %s", e.Reason.Error()) +} diff --git a/light/provider/http/http.go b/light/provider/http/http.go index b996c5059..a46e961ff 100644 --- a/light/provider/http/http.go +++ b/light/provider/http/http.go @@ -1,10 +1,11 @@ package http import ( - "errors" "fmt" + "math/rand" "regexp" "strings" + "time" "github.com/tendermint/tendermint/light/provider" rpcclient "github.com/tendermint/tendermint/rpc/client" @@ -13,7 +14,10 @@ import ( ) // 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)`) +var ( + regexpMissingHeight = regexp.MustCompile(`height \d+ (must be less than or equal to|is not available)`) + maxRetryAttempts = 10 +) // http provider uses an RPC client to obtain the necessary information. type http struct { @@ -60,66 +64,95 @@ func (p *http) String() string { func (p *http) LightBlock(height int64) (*types.LightBlock, error) { h, err := validateHeight(height) if err != nil { - return nil, err + return nil, provider.ErrBadLightBlock{Reason: err} } - commit, err := p.client.Commit(h) + sh, err := p.signedHeader(h) if err != nil { - // TODO: standardize errors on the RPC side - if regexpMissingHeight.MatchString(err.Error()) { - return nil, provider.ErrLightBlockNotFound - } return nil, err } - if commit.Header == nil { - return nil, errors.New("header is nil") + vs, err := p.validatorSet(h) + if err != nil { + return nil, err } - // Verify we're still on the same chain. - if p.chainID != commit.Header.ChainID { - return nil, fmt.Errorf("expected chainID %s, got %s", p.chainID, commit.Header.ChainID) + lb := &types.LightBlock{ + SignedHeader: sh, + ValidatorSet: vs, } - maxPerPage := 100 - res, err := p.client.Validators(h, nil, &maxPerPage) + err = lb.ValidateBasic(p.chainID) if err != nil { - // TODO: standardize errors on the RPC side - if regexpMissingHeight.MatchString(err.Error()) { - return nil, provider.ErrLightBlockNotFound - } - return nil, err + return nil, provider.ErrBadLightBlock{Reason: err} } + return lb, nil +} + +// ReportEvidence calls `/broadcast_evidence` endpoint. +func (p *http) ReportEvidence(ev types.Evidence) error { + _, err := p.client.BroadcastEvidence(ev) + return err +} + +func (p *http) validatorSet(height *int64) (*types.ValidatorSet, error) { var ( - vals = res.Validators - page = 1 + maxPerPage = 100 + vals = []*types.Validator{} + page = 1 ) - // Check if there are more validators. - for len(res.Validators) == maxPerPage { - res, err = p.client.Validators(h, &page, &maxPerPage) - if err != nil { - return nil, err - } - if len(res.Validators) > 0 { + for len(vals)%maxPerPage == 0 { + for attempt := 1; attempt <= maxRetryAttempts; attempt++ { + res, err := p.client.Validators(height, &page, &maxPerPage) + if err != nil { + // TODO: standardize errors on the RPC side + if regexpMissingHeight.MatchString(err.Error()) { + return nil, provider.ErrLightBlockNotFound + } + // if we have exceeded retry attempts then return no response error + if attempt == maxRetryAttempts { + return nil, provider.ErrNoResponse + } + // else we wait and try again with exponential backoff + 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} + } + return valSet, nil + } vals = append(vals, res.Validators...) + page++ + break } - page++ } - - valset := types.NewValidatorSet(vals) - - return &types.LightBlock{ - SignedHeader: &commit.SignedHeader, - ValidatorSet: valset, - }, nil + valSet, err := types.ValidatorSetFromExistingValidators(vals) + if err != nil { + return nil, provider.ErrBadLightBlock{Reason: err} + } + return valSet, nil } -// ReportEvidence calls `/broadcast_evidence` endpoint. -func (p *http) ReportEvidence(ev types.Evidence) error { - _, err := p.client.BroadcastEvidence(ev) - return err +func (p *http) signedHeader(height *int64) (*types.SignedHeader, error) { + for attempt := 1; attempt <= maxRetryAttempts; attempt++ { + commit, err := p.client.Commit(height) + if err != nil { + // TODO: standardize errors on the RPC side + if regexpMissingHeight.MatchString(err.Error()) { + return nil, provider.ErrLightBlockNotFound + } + // we wait and try again with exponential backoff + time.Sleep(backoffTimeout(uint16(attempt))) + continue + } + return &commit.SignedHeader, nil + } + return nil, provider.ErrNoResponse } func validateHeight(height int64) (*int64, error) { @@ -133,3 +166,10 @@ func validateHeight(height int64) (*int64, error) { } return h, nil } + +// exponential backoff (with jitter) +// 0.5s -> 2s -> 4.5s -> 8s -> 12.5 with 1s variation +func backoffTimeout(attempt uint16) time.Duration { + // nolint:gosec // G404: Use of weak random number generator + return time.Duration(500*attempt*attempt)*time.Millisecond + time.Duration(rand.Intn(1000))*time.Millisecond +} diff --git a/light/provider/http/http_test.go b/light/provider/http/http_test.go index 6952763db..082de01bb 100644 --- a/light/provider/http/http_test.go +++ b/light/provider/http/http_test.go @@ -66,7 +66,6 @@ func TestProvider(t *testing.T) { // let's get the highest block sh, err := p.LightBlock(0) - require.NoError(t, err) assert.True(t, sh.Height < 1000) diff --git a/light/provider/mock/mock.go b/light/provider/mock/mock.go index 23f8f3821..176bf392b 100644 --- a/light/provider/mock/mock.go +++ b/light/provider/mock/mock.go @@ -1,6 +1,7 @@ package mock import ( + "errors" "fmt" "strings" @@ -48,23 +49,34 @@ func (p *Mock) String() string { } func (p *Mock) LightBlock(height int64) (*types.LightBlock, error) { + var lb *types.LightBlock if height == 0 && len(p.headers) > 0 { sh := p.headers[int64(len(p.headers))] vals := p.vals[int64(len(p.vals))] - return &types.LightBlock{ + lb = &types.LightBlock{ SignedHeader: sh, ValidatorSet: vals, - }, nil + } + } if _, ok := p.headers[height]; ok { sh := p.headers[height] vals := p.vals[height] - return &types.LightBlock{ + lb = &types.LightBlock{ SignedHeader: sh, ValidatorSet: vals, - }, nil + } + } + if lb == nil { + return nil, provider.ErrLightBlockNotFound + } + if lb.SignedHeader == nil || lb.ValidatorSet == nil { + return nil, provider.ErrBadLightBlock{Reason: errors.New("nil header or vals")} + } + if err := lb.ValidateBasic(lb.ChainID); err != nil { + return nil, provider.ErrBadLightBlock{Reason: err} } - return nil, provider.ErrLightBlockNotFound + return lb, nil } func (p *Mock) ReportEvidence(ev types.Evidence) error { diff --git a/types/validator_set.go b/types/validator_set.go index e1bfd31f7..cdd748476 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -986,9 +986,13 @@ func ValidatorSetFromProto(vp *tmproto.ValidatorSet) (*ValidatorSet, error) { // 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("no validators given") + 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, diff --git a/types/validator_set_test.go b/types/validator_set_test.go index 6745b20e0..9ce5b6025 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -1436,9 +1436,10 @@ func TestNewValidatorSetFromExistingValidators(t *testing.T) { valSet.IncrementProposerPriority(5) newValSet := NewValidatorSet(valSet.Validators) - existingValSet, err := ValidatorSetFromExistingValidators(valSet.Validators) - require.NoError(t, err) assert.NotEqual(t, valSet, newValSet) + + existingValSet, err := ValidatorSetFromExistingValidators(valSet.Validators) + assert.NoError(t, err) assert.Equal(t, valSet, existingValSet) assert.Equal(t, valSet.CopyIncrementProposerPriority(3), existingValSet.CopyIncrementProposerPriority(3)) }