From 123beeadc423db6e287c07a975be39b852374ec4 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 3 Jun 2020 11:32:51 +0400 Subject: [PATCH] lite2: compare header with witnesses in parallel (#4935) Closes #4801 --- CHANGELOG_PENDING.md | 3 +- lite2/client.go | 85 ++++++++++++++++++++++++++++++-------------- lite2/errors.go | 26 ++++++++++++++ 3 files changed, 87 insertions(+), 27 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index b87e58daf..567f4cee2 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -37,7 +37,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [types] [\#4792](https://github.com/tendermint/tendermint/pull/4792) Sort validators by voting power to enable faster commit verification (@melekes) - [evidence] [\#4780](https://github.com/tendermint/tendermint/pull/4780) Cap evidence to an absolute number (@cmwaters) Add `max_num` to consensus evidence parameters (default: 50 items). - - [mempool] \#4940 Migrate mempool from amino binary encoding to Protobuf + - [mempool] \#4940 Migrate mempool from amino binary encoding to Protobuf ### FEATURES: @@ -56,6 +56,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [evidence] [\#4839](https://github.com/tendermint/tendermint/pull/4839) Reject duplicate evidence from being proposed (@cmwaters) - [evidence] [\#4892](https://github.com/tendermint/tendermint/pull/4892) Remove redundant header from phantom validator evidence (@cmwaters) - [types] [\#4905](https://github.com/tendermint/tendermint/pull/4905) Add ValidateBasic to validator and validator set (@cmwaters) +- [lite2] [\#4935](https://github.com/tendermint/tendermint/pull/4935) Fetch and compare a new header with witnesses in parallel (@melekes) - [lite2] [\#4929](https://github.com/tendermint/tendermint/pull/4929) compare header w/ witnesses only when doing bisection (@melekes) ### BUG FIXES: diff --git a/lite2/client.go b/lite2/client.go index 673b7e042..1b8913bb9 100644 --- a/lite2/client.go +++ b/lite2/client.go @@ -937,47 +937,52 @@ func (c *Client) compareNewHeaderWithWitnesses(h *types.SignedHeader) error { defer c.providerMutex.Unlock() // 1. Make sure AT LEAST ONE witness returns the same header. - headerMatched := false - witnessesToRemove := make([]int, 0) + var ( + headerMatched bool + lastErrConfHeaders error + ) 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 { - altH, err := witness.SignedHeader(h.Height) - if err != nil { - c.logger.Error("Failed to get a header from witness", "height", h.Height, "witness", witness) - continue - } - - if err = altH.ValidateBasic(c.chainID); err != nil { - c.logger.Error("Witness sent us incorrect header", "err", err, "witness", witness) - witnessesToRemove = append(witnessesToRemove, i) - continue - } + go c.compareNewHeaderWithWitness(errc, h, witness, i) + } - if !bytes.Equal(h.Hash(), altH.Hash()) { - if err = c.latestTrustedVals.VerifyCommitTrusting(c.chainID, altH.Commit, c.trustLevel); err != nil { - c.logger.Error("Witness sent us incorrect header", "err", err, "witness", witness) - witnessesToRemove = append(witnessesToRemove, i) - continue + 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 ErrConflictingHeaders: // potential fork + c.logger.Error(err.Error(), "witness", e.Witness) + c.sendConflictingHeadersEvidence(types.ConflictingHeadersEvidence{H1: h, H2: e.H2}) + lastErrConfHeaders = e + case errBadWitness: + c.logger.Error(err.Error(), "witness", c.witnesses[e.WitnessIndex]) + // if witness sent us invalid header, remove it + if e.Code == invalidHeader { + witnessesToRemove = append(witnessesToRemove, e.WitnessIndex) } - - c.sendConflictingHeadersEvidence(types.ConflictingHeadersEvidence{H1: h, H2: altH}) - - return ErrConflictingHeaders{H1: h, Primary: c.primary, H2: altH, Witness: witness} } - - headerMatched = true } for _, idx := range witnessesToRemove { c.removeWitness(idx) } - witnessesToRemove = make([]int, 0) - if headerMatched { + if lastErrConfHeaders != nil { + // NOTE: all of the potential forks will be reported, but we only return + // the last ErrConflictingHeaders error here. + return lastErrConfHeaders + } else if headerMatched { return nil } @@ -988,6 +993,34 @@ func (c *Client) compareNewHeaderWithWitnesses(h *types.SignedHeader) error { return errors.New("awaiting response from all witnesses exceeded dropout time") } +func (c *Client) compareNewHeaderWithWitness(errc chan error, h *types.SignedHeader, + witness provider.Provider, witnessIndex int) { + + altH, err := witness.SignedHeader(h.Height) + if err != nil { + errc <- errBadWitness{err, noResponse, witnessIndex} + return + } + + if err = altH.ValidateBasic(c.chainID); err != nil { + errc <- errBadWitness{err, invalidHeader, witnessIndex} + return + } + + if !bytes.Equal(h.Hash(), altH.Hash()) { + // FIXME: call bisection instead of VerifyCommitTrusting + // https://github.com/tendermint/tendermint/issues/4934 + if err = c.latestTrustedVals.VerifyCommitTrusting(c.chainID, altH.Commit, c.trustLevel); err != nil { + errc <- errBadWitness{err, invalidHeader, witnessIndex} + return + } + + errc <- ErrConflictingHeaders{H1: h, Primary: c.primary, H2: altH, Witness: witness} + } + + errc <- nil +} + // NOTE: requires a providerMutex locked. func (c *Client) removeWitness(idx int) { switch len(c.witnesses) { diff --git a/lite2/errors.go b/lite2/errors.go index 4779a46ec..e6bb39359 100644 --- a/lite2/errors.go +++ b/lite2/errors.go @@ -63,3 +63,29 @@ type errNoWitnesses struct{} func (e errNoWitnesses) Error() string { return "no witnesses connected. please reset light client" } + +type badWitnessCode int + +const ( + noResponse badWitnessCode = iota + 1 + invalidHeader +) + +// errBadWitness is returned when the witness either does not respond or +// responds with an invalid header. +type errBadWitness struct { + Reason error + Code badWitnessCode + WitnessIndex int +} + +func (e errBadWitness) Error() string { + switch e.Code { + case noResponse: + return fmt.Sprintf("failed to get a header from witness: %v", e.Reason) + case invalidHeader: + return fmt.Sprintf("witness sent us invalid header: %v", e.Reason) + default: + return fmt.Sprintf("unknown code: %d", e.Code) + } +}