lite2: compare header with witnesses in parallel (#4935)

Closes #4801

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:

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
-			}
+			go c.compareNewHeaderWithWitness(errc, h, witness, i)
+		}
 
-			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
-			}
+		witnessesToRemove := make([]int, 0)
 
-			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
+		// 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) {

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)
+	}
+}