lite2: make witnesses mandatory (#4358)

* lite2: make witnesses mandatory

at least one witness is required

* lite2: return an error if there are no witnesses

tendermint/tendermint#4358 (review)

* cmd/lite: add witnesses flag

* fix linter errors

cmd/tendermint/commands/lite.go

@@ -2,6 +2,7 @@ package commands
 
 import (
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -12,6 +13,7 @@ import (
 
 	tmos "github.com/tendermint/tendermint/libs/os"
 	lite "github.com/tendermint/tendermint/lite2"
+	"github.com/tendermint/tendermint/lite2/provider"
 	httpp "github.com/tendermint/tendermint/lite2/provider/http"
 	lproxy "github.com/tendermint/tendermint/lite2/proxy"
 	lrpc "github.com/tendermint/tendermint/lite2/rpc"
@@ -36,9 +38,10 @@ just with added trust and running locally.`,
 
 var (
 	listenAddr         string
-	nodeAddr           string
+	primaryAddr        string
 	chainID            string
 	home               string
+	witnessesAddrs     string
 	maxOpenConnections int
 
 	trustingPeriod time.Duration
@@ -47,29 +50,51 @@ var (
 )
 
 func init() {
-	LiteCmd.Flags().StringVar(&listenAddr, "laddr", "tcp://localhost:8888", "Serve the proxy on the given address")
-	LiteCmd.Flags().StringVar(&nodeAddr, "node", "tcp://localhost:26657", "Connect to a Tendermint node at this address")
+	LiteCmd.Flags().StringVar(&listenAddr, "laddr", "tcp://localhost:8888",
+		"Serve the proxy on the given address")
+
+	LiteCmd.Flags().StringVar(&primaryAddr, "primary", "tcp://localhost:26657",
+		"Connect to a Tendermint node at this address")
+
+	LiteCmd.Flags().StringVar(&witnessesAddrs, "witnesses", "",
+		"Tendermint nodes to cross-check the primary node, comma-separated")
+
 	LiteCmd.Flags().StringVar(&chainID, "chain-id", "tendermint", "Specify the Tendermint chain ID")
+
 	LiteCmd.Flags().StringVar(&home, "home-dir", ".tendermint-lite", "Specify the home directory")
 	LiteCmd.Flags().IntVar(
 		&maxOpenConnections,
 		"max-open-connections",
 		900,
 		"Maximum number of simultaneous connections (including WebSocket).")
 
-	LiteCmd.Flags().DurationVar(&trustingPeriod, "trusting-period", 168*time.Hour, "Trusting period. Should be significantly less than the unbonding period")
+	LiteCmd.Flags().DurationVar(&trustingPeriod, "trusting-period", 168*time.Hour,
+		"Trusting period. Should be significantly less than the unbonding period")
+
 	LiteCmd.Flags().Int64Var(&trustedHeight, "trusted-height", 1, "Trusted header's height")
+
 	LiteCmd.Flags().BytesHexVar(&trustedHash, "trusted-hash", []byte{}, "Trusted header's hash")
 }
 
 func runProxy(cmd *cobra.Command, args []string) error {
 	liteLogger := logger.With("module", "lite")
 
-	logger.Info("Connecting to Tendermint node...")
-	// First, connect a client
-	node, err := rpcclient.NewHTTP(nodeAddr, "/websocket")
+	logger.Info("Connecting to the primary node...")
+	rpcClient, err := rpcclient.NewHTTP(chainID, primaryAddr)
 	if err != nil {
-		return errors.Wrap(err, "new HTTP client")
+		return errors.Wrapf(err, "http client for %s", primaryAddr)
+	}
+	primary := httpp.NewWithClient(chainID, rpcClient)
+
+	logger.Info("Connecting to the witness nodes...")
+	addrs := strings.Split(witnessesAddrs, ",")
+	witnesses := make([]provider.Provider, len(addrs))
+	for i, addr := range addrs {
+		p, err := httpp.New(chainID, addr)
+		if err != nil {
+			return errors.Wrapf(err, "http provider for %s", addr)
+		}
+		witnesses[i] = p
 	}
 
 	logger.Info("Creating client...")
@@ -84,7 +109,8 @@ func runProxy(cmd *cobra.Command, args []string) error {
 			Height: trustedHeight,
 			Hash:   trustedHash,
 		},
-		httpp.NewWithClient(chainID, node),
+		primary,
+		witnesses,
 		dbs.New(db, chainID),
 		lite.Logger(liteLogger),
 	)
@@ -96,7 +122,7 @@ func runProxy(cmd *cobra.Command, args []string) error {
 		Addr:   listenAddr,
 		Config: &rpcserver.Config{MaxOpenConnections: maxOpenConnections},
 		Codec:  amino.NewCodec(),
-		Client: lrpc.NewClient(node, c),
+		Client: lrpc.NewClient(rpcClient, c),
 		Logger: liteLogger,
 	}
 	// Stop upon receiving SIGTERM or CTRL-C.

lite2/client.go

@@ -75,29 +75,12 @@ func SequentialVerification() Option {
 // applies to non-adjacent headers. For adjacent headers, sequential
 // verification is used.
 func SkippingVerification(trustLevel tmmath.Fraction) Option {
-	if err := ValidateTrustLevel(trustLevel); err != nil {
-		panic(err)
-	}
 	return func(c *Client) {
 		c.verificationMode = skipping
 		c.trustLevel = trustLevel
 	}
 }
 
-// Witnesses option can be used to supply providers, which will be used for
-// cross-checking the primary provider. A witness can become a primary iff the
-// current primary is unavailable.
-func Witnesses(providers []provider.Provider) Option {
-	return func(c *Client) {
-		for _, witness := range providers {
-			if witness.ChainID() != c.ChainID() {
-				panic("Witness chainID is not equal to the Lite Client chainID")
-			}
-		}
-		c.witnesses = providers
-	}
-}
-
 // UpdatePeriod option can be used to change default polling period (5s).
 func UpdatePeriod(d time.Duration) Option {
 	return func(c *Client) {
@@ -175,11 +158,16 @@ type Client struct {
 // obtain the header & vals from the primary or they are invalid (e.g. trust
 // hash does not match with the one from the header).
 //
+// Witnesses are providers, which will be used for cross-checking the primary
+// provider. At least one witness must be given. A witness can become a primary
+// iff the current primary is unavailable.
+//
 // See all Option(s) for the additional configuration.
 func NewClient(
 	chainID string,
 	trustOptions TrustOptions,
 	primary provider.Provider,
+	witnesses []provider.Provider,
 	trustedStore store.Store,
 	options ...Option) (*Client, error) {
 
@@ -189,6 +177,7 @@ func NewClient(
 		verificationMode:                   skipping,
 		trustLevel:                         DefaultTrustLevel,
 		primary:                            primary,
+		witnesses:                          witnesses,
 		trustedStore:                       trustedStore,
 		updatePeriod:                       defaultUpdatePeriod,
 		removeNoLongerTrustedHeadersPeriod: defaultRemoveNoLongerTrustedHeadersPeriod,
@@ -201,6 +190,24 @@ func NewClient(
 		o(c)
 	}
 
+	// Validate the number of witnesses.
+	if len(c.witnesses) < 1 {
+		return nil, errors.New("expected at least one witness")
+	}
+
+	// Verify witnesses are all on the same chain.
+	for i, w := range witnesses {
+		if w.ChainID() != chainID {
+			return nil, errors.Errorf("witness #%d: %v is on another chain %s, expected %s",
+				i, w, w.ChainID(), chainID)
+		}
+	}
+
+	// Validate trust level.
+	if err := ValidateTrustLevel(c.trustLevel); err != nil {
+		return nil, err
+	}
+
 	if err := c.restoreTrustedHeaderAndNextVals(); err != nil {
 		return nil, err
 	}
@@ -523,11 +530,9 @@ func (c *Client) VerifyHeader(newHeader *types.SignedHeader, newVals *types.Vali
 		return errors.Errorf("header at more recent height #%d exists", c.trustedHeader.Height)
 	}
 
-	if len(c.witnesses) > 0 {
-		if err := c.compareNewHeaderWithRandomWitness(newHeader); err != nil {
-			c.logger.Error("Error when comparing new header with one from a witness", "err", err)
-			return err
-		}
+	if err := c.compareNewHeaderWithRandomWitness(newHeader); err != nil {
+		c.logger.Error("Error when comparing new header with one from a witness", "err", err)
+		return err
 	}
 
 	var err error
@@ -733,6 +738,10 @@ func (c *Client) fetchHeaderAndValsAtHeight(height int64) (*types.SignedHeader,
 
 // compare header with one from a random witness.
 func (c *Client) compareNewHeaderWithRandomWitness(h *types.SignedHeader) error {
+	if len(c.witnesses) == 0 {
+		return errors.New("could not find any witnesses")
+	}
+
 	// 1. Pick a witness.
 	witness := c.witnesses[tmrand.Intn(len(c.witnesses))]