lite2: remove auto update (#4535)

We first introduced auto-update as a separate struct AutoClient, which
was wrapping Client and calling Update periodically.

// AutoClient can auto update itself by fetching headers every N seconds.
type AutoClient struct {
    base         *Client
    updatePeriod time.Duration
    quit         chan struct{}

    trustedHeaders chan *types.SignedHeader
    errs           chan error
}

// NewAutoClient creates a new client and starts a polling goroutine.
func NewAutoClient(base *Client, updatePeriod time.Duration) *AutoClient {
    c := &AutoClient{
        base:           base,
        updatePeriod:   updatePeriod,
        quit:           make(chan struct{}),
        trustedHeaders: make(chan *types.SignedHeader),
        errs:           make(chan error),
    }
    go c.autoUpdate()
    return c
}

// TrustedHeaders returns a channel onto which new trusted headers are posted.
func (c *AutoClient) TrustedHeaders() <-chan *types.SignedHeader {
    return c.trustedHeaders
}

// Err returns a channel onto which errors are posted.
func (c *AutoClient) Errs() <-chan error {
    return c.errs
}

// Stop stops the client.
func (c *AutoClient) Stop() {
    close(c.quit)
}

func (c *AutoClient) autoUpdate() {
    ticker := time.NewTicker(c.updatePeriod)
    defer ticker.Stop()

    for {
        select {
        case <-ticker.C:
            lastTrustedHeight, err := c.base.LastTrustedHeight()
            if err != nil {
                c.errs <- err
                continue
            }

            if lastTrustedHeight == -1 {
                // no headers yet => wait
                continue
            }

            newTrustedHeader, err := c.base.Update(time.Now())
            if err != nil {
                c.errs <- err
                continue
            }

            if newTrustedHeader != nil {
                 c.trustedHeaders <- newTrustedHeader
            }
        case <-c.quit:
            return
        }
    }
}

Later we merged it into the Client itself with the assumption that most clients will want it.

But now I am not sure. Neither IBC nor cosmos/relayer are using it. It increases complexity (Start/Stop methods).

That said, I think it makes sense to remove it until we see a need for it (until we better understand usage behavior). We can always introduce it later 😅. Maybe in the form of AutoClient.

cmd/tendermint/commands/lite.go

@@ -102,7 +102,7 @@ func runProxy(cmd *cobra.Command, args []string) error {
 
 	db, err := dbm.NewGoLevelDB("lite-client-db", home)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "new goleveldb")
 	}
 
 	var c *lite.Client
@@ -129,16 +129,9 @@ func runProxy(cmd *cobra.Command, args []string) error {
 			lite.Logger(logger),
 		)
 	}
-
 	if err != nil {
-		return errors.Wrap(err, "failed to create")
+		return err
 	}
-	logger.Info("Starting client...")
-	err = c.Start()
-	if err != nil {
-		return errors.Wrap(err, "failed to start")
-	}
-	defer c.Stop()
 
 	rpcClient, err := rpcclient.NewHTTP(primaryAddr, "/websocket")
 	if err != nil {

docs/architecture/adr-046-light-client-implementation.md

@@ -8,23 +8,13 @@
 ## Context
 
 A `Client` struct represents a light client, connected to a single blockchain.
-As soon as it's started (via `Start`), it tries to update to the latest header
-(using bisection algorithm by default).
 
-Cleaning routine is also started to remove headers outside of trusting period.
-NOTE: since it's periodic, we still need to check header is not expired in
-`TrustedHeader`, `TrustedValidatorSet` methods (and others which are using the
-latest trusted header).
-
-The user has an option to manually verify headers using `VerifyHeader` and
-`VerifyHeaderAtHeight` methods. To avoid races, `UpdatePeriod(0)` needs to be
-passed when initializing the light client (it turns off the auto update).
+The user has an option to verify headers using `VerifyHeader` or
+`VerifyHeaderAtHeight` or `Update` methods. The latter method downloads the
+latest header from primary and compares it with the currently trusted one.
 
 ```go
 type Client interface {
-	// start and stop updating & cleaning goroutines
-	Start() error
-	Stop()
 	Cleanup() error
 
 	// get trusted headers & validators
@@ -41,6 +31,7 @@ type Client interface {
 	// verify new headers
 	VerifyHeaderAtHeight(height int64, now time.Time) (*types.SignedHeader, error)
 	VerifyHeader(newHeader *types.SignedHeader, newVals *types.ValidatorSet, now time.Time) error
+	Update(now time.Time) error
 }
 ```
 

lite2/client.go

@@ -22,7 +22,6 @@ const (
 	sequential mode = iota + 1
 	skipping
 
-	defaultUpdatePeriod     = 5 * time.Second
 	defaultPruningSize      = 1000
 	defaultMaxRetryAttempts = 10
 )
@@ -55,13 +54,6 @@ func SkippingVerification(trustLevel tmmath.Fraction) Option {
 	}
 }
 
-// UpdatePeriod option can be used to change default polling period (5s).
-func UpdatePeriod(d time.Duration) Option {
-	return func(c *Client) {
-		c.updatePeriod = d
-	}
-}
-
 // PruningSize option sets the maximum amount of headers & validator set pairs
 // that the light client stores. When Prune() is run, all headers (along with
 // the associated validator sets) that are earlier than the h amount of headers
@@ -101,9 +93,6 @@ func MaxRetryAttempts(max uint16) Option {
 // headers from a primary provider, verifies them either sequentially or by
 // skipping some and stores them in a trusted store (usually, a local FS).
 //
-// By default, the client will poll the primary provider for new headers every
-// 5s (UpdatePeriod). If there are any, it will try to advance the state.
-//
 // Default verification: SkippingVerification(DefaultTrustLevel)
 type Client struct {
 	chainID          string
@@ -126,8 +115,6 @@ type Client struct {
 	// Highest validator set from the store (height=H).
 	latestTrustedVals *types.ValidatorSet
 
-	// See UpdatePeriod option
-	updatePeriod time.Duration
 	// See RemoveNoLongerTrustedHeadersPeriod option
 	pruningSize uint16
 	// See ConfirmationFunction option
@@ -202,7 +189,6 @@ func NewClientFromTrustedStore(
 		primary:          primary,
 		witnesses:        witnesses,
 		trustedStore:     trustedStore,
-		updatePeriod:     defaultUpdatePeriod,
 		pruningSize:      defaultPruningSize,
 		confirmationFn:   func(action string) bool { return true },
 		quit:             make(chan struct{}),
@@ -390,27 +376,6 @@ func (c *Client) initializeWithTrustOptions(options TrustOptions) error {
 	return c.updateTrustedHeaderAndVals(h, vals)
 }
 
-// Start starts two processes: 1) auto updating 2) removing outdated headers.
-func (c *Client) Start() error {
-	c.logger.Info("Starting light client")
-
-	if c.updatePeriod > 0 {
-		c.routinesWaitGroup.Add(1)
-		go c.autoUpdateRoutine()
-	}
-
-	return nil
-}
-
-// Stop stops two processes: 1) auto updating 2) removing outdated headers.
-// Stop only returns after both of them are finished running. If you wish to
-// remove all the data, call Cleanup.
-func (c *Client) Stop() {
-	c.logger.Info("Stopping light client")
-	close(c.quit)
-	c.routinesWaitGroup.Wait()
-}
-
 // TrustedHeader returns a trusted header at the given height (0 - the latest).
 //
 // Headers along with validator sets, which can't be trusted anymore, are
@@ -929,30 +894,6 @@ func (c *Client) removeWitness(idx int) {
 	}
 }
 
-func (c *Client) autoUpdateRoutine() {
-	defer c.routinesWaitGroup.Done()
-
-	err := c.Update(time.Now())
-	if err != nil {
-		c.logger.Error("Error during auto update", "err", err)
-	}
-
-	ticker := time.NewTicker(c.updatePeriod)
-	defer ticker.Stop()
-
-	for {
-		select {
-		case <-ticker.C:
-			err := c.Update(time.Now())
-			if err != nil {
-				c.logger.Error("Error during auto update", "err", err)
-			}
-		case <-c.quit:
-			return
-		}
-	}
-}
-
 // Update attempts to advance the state by downloading the latest header and
 // comparing it with the existing one.
 func (c *Client) Update(now time.Time) error {

lite2/client_test.go

@@ -154,7 +154,6 @@ func TestClient_SequentialVerification(t *testing.T) {
 				)},
 				dbs.New(dbm.NewMemDB(), chainID),
 				SequentialVerification(),
-				UpdatePeriod(0),
 			)
 
 			if tc.initErr {
@@ -163,9 +162,6 @@ func TestClient_SequentialVerification(t *testing.T) {
 			}
 
 			require.NoError(t, err)
-			err = c.Start()
-			require.NoError(t, err)
-			defer c.Stop()
 
 			_, err = c.VerifyHeaderAtHeight(3, bTime.Add(3*time.Hour))
 			if tc.verifyErr {
@@ -281,7 +277,6 @@ func TestClient_SkippingVerification(t *testing.T) {
 				)},
 				dbs.New(dbm.NewMemDB(), chainID),
 				SkippingVerification(DefaultTrustLevel),
-				UpdatePeriod(0),
 			)
 			if tc.initErr {
 				require.Error(t, err)
@@ -289,9 +284,6 @@ func TestClient_SkippingVerification(t *testing.T) {
 			}
 
 			require.NoError(t, err)
-			err = c.Start()
-			require.NoError(t, err)
-			defer c.Stop()
 
 			_, err = c.VerifyHeaderAtHeight(3, bTime.Add(3*time.Hour))
 			if tc.verifyErr {
@@ -429,9 +421,6 @@ func TestClientRestoresTrustedHeaderAfterStartup2(t *testing.T) {
 			Logger(log.TestingLogger()),
 		)
 		require.NoError(t, err)
-		err = c.Start()
-		require.NoError(t, err)
-		defer c.Stop()
 
 		// Check we still have the 1st header (+header+).
 		h, err := c.TrustedHeader(1)
@@ -483,9 +472,6 @@ func TestClientRestoresTrustedHeaderAfterStartup2(t *testing.T) {
 			Logger(log.TestingLogger()),
 		)
 		require.NoError(t, err)
-		err = c.Start()
-		require.NoError(t, err)
-		defer c.Stop()
 
 		// Check we no longer have the invalid 1st header (+header+).
 		h, err := c.TrustedHeader(1)
@@ -520,9 +506,6 @@ func TestClientRestoresTrustedHeaderAfterStartup3(t *testing.T) {
 			Logger(log.TestingLogger()),
 		)
 		require.NoError(t, err)
-		err = c.Start()
-		require.NoError(t, err)
-		defer c.Stop()
 
 		// Check we still have the 1st header (+header+).
 		h, err := c.TrustedHeader(1)
@@ -584,9 +567,6 @@ func TestClientRestoresTrustedHeaderAfterStartup3(t *testing.T) {
 			Logger(log.TestingLogger()),
 		)
 		require.NoError(t, err)
-		err = c.Start()
-		require.NoError(t, err)
-		defer c.Stop()
 
 		// Check we have swapped invalid 1st header (+header+) with correct one (+header1+).
 		h, err := c.TrustedHeader(1)
@@ -622,9 +602,6 @@ func TestClient_Update(t *testing.T) {
 		Logger(log.TestingLogger()),
 	)
 	require.NoError(t, err)
-	err = c.Start()
-	require.NoError(t, err)
-	defer c.Stop()
 
 	// should result in downloading & verifying header #3
 	err = c.Update(bTime.Add(2 * time.Hour))
@@ -649,13 +626,9 @@ func TestClient_Concurrency(t *testing.T) {
 		fullNode,
 		[]provider.Provider{fullNode},
 		dbs.New(dbm.NewMemDB(), chainID),
-		UpdatePeriod(0),
 		Logger(log.TestingLogger()),
 	)
 	require.NoError(t, err)
-	err = c.Start()
-	require.NoError(t, err)
-	defer c.Stop()
 
 	_, err = c.VerifyHeaderAtHeight(2, bTime.Add(2*time.Hour))
 	require.NoError(t, err)
@@ -697,7 +670,6 @@ func TestClientReplacesPrimaryWithWitnessIfPrimaryIsUnavailable(t *testing.T) {
 		deadNode,
 		[]provider.Provider{fullNode, fullNode},
 		dbs.New(dbm.NewMemDB(), chainID),
-		UpdatePeriod(0),
 		Logger(log.TestingLogger()),
 		MaxRetryAttempts(1),
 	)
@@ -722,7 +694,6 @@ func TestClient_BackwardsVerification(t *testing.T) {
 			fullNode,
 			[]provider.Provider{fullNode},
 			dbs.New(dbm.NewMemDB(), chainID),
-			UpdatePeriod(0),
 			Logger(log.TestingLogger()),
 		)
 		require.NoError(t, err)
@@ -755,7 +726,6 @@ func TestClient_BackwardsVerification(t *testing.T) {
 			fullNode,
 			[]provider.Provider{fullNode},
 			dbs.New(dbm.NewMemDB(), chainID),
-			UpdatePeriod(0),
 			Logger(log.TestingLogger()),
 		)
 		require.NoError(t, err)
@@ -807,7 +777,6 @@ func TestClient_BackwardsVerification(t *testing.T) {
 				tc.provider,
 				[]provider.Provider{tc.provider},
 				dbs.New(dbm.NewMemDB(), chainID),
-				UpdatePeriod(0),
 				Logger(log.TestingLogger()),
 			)
 			require.NoError(t, err)
@@ -854,7 +823,6 @@ func TestNewClientErrorsIfAllWitnessesUnavailable(t *testing.T) {
 		fullNode,
 		[]provider.Provider{deadNode, deadNode},
 		dbs.New(dbm.NewMemDB(), chainID),
-		UpdatePeriod(0),
 		Logger(log.TestingLogger()),
 		MaxRetryAttempts(1),
 	)
@@ -898,7 +866,6 @@ func TestClientRemovesWitnessIfItSendsUsIncorrectHeader(t *testing.T) {
 		fullNode,
 		[]provider.Provider{badProvider1, badProvider2},
 		dbs.New(dbm.NewMemDB(), chainID),
-		UpdatePeriod(0),
 		Logger(log.TestingLogger()),
 		MaxRetryAttempts(1),
 	)
@@ -926,7 +893,6 @@ func TestClientTrustedValidatorSet(t *testing.T) {
 		fullNode,
 		[]provider.Provider{fullNode},
 		dbs.New(dbm.NewMemDB(), chainID),
-		UpdatePeriod(0),
 		Logger(log.TestingLogger()),
 	)
 

lite2/doc.go

@@ -65,33 +65,29 @@ Example usage:
 
 		db, err := dbm.NewGoLevelDB("lite-client-db", dbDir)
 		if err != nil {
-			// return err
-			t.Fatal(err)
+			// handle error
 		}
-		c, err := NewClient(
+
+		c, err := NewHTTPClient(
 			chainID,
 			TrustOptions{
 				Period: 504 * time.Hour, // 21 days
 				Height: 100,
 				Hash:   header.Hash(),
 			},
-			httpp.New(chainID, "tcp://localhost:26657"),
-			[]provider.Provider{httpp.New(chainID, "tcp://witness1:26657")},
-			dbs.New(db, chainID),
+			"http://localhost:26657",
+			[]string{"http://witness1:26657"},
+			dbs.New(db, ""),
 		)
-
-		err = c.Start()
-		if err != nil {
-			// return err
-			t.Fatal(err)
-		}
-		defer c.Stop()
-
-		h, err := c.TrustedHeader(101)
 		if err != nil {
 			// handle error
 		}
-		fmt.Println("got header", h)
+
+		h, err := c.TrustedHeader(100)
+		if err != nil {
+			// handle error
+		}
+		fmt.Println("header", h)
 
 Check out other examples in example_test.go
 

lite2/example_test.go

@@ -58,18 +58,12 @@ func ExampleClient_Update() {
 		primary,
 		[]provider.Provider{primary}, // NOTE: primary should not be used here
 		dbs.New(db, chainID),
-		UpdatePeriod(0), // NOTE: value should be greater than zero
 		// Logger(log.TestingLogger()),
 	)
 	if err != nil {
 		stdlog.Fatal(err)
 	}
-	err = c.Start()
-	if err != nil {
-		stdlog.Fatal(err)
-	}
 	defer func() {
-		c.Stop()
 		c.Cleanup()
 	}()
 
@@ -78,6 +72,7 @@ func ExampleClient_Update() {
 	// XXX: 30 * time.Minute clock drift is needed because a) Tendermint strips
 	// monotonic component (see types/time/time.go) b) single instance is being
 	// run.
+	// https://github.com/tendermint/tendermint/issues/4489
 	err = c.Update(time.Now().Add(30 * time.Minute))
 	if err != nil {
 		stdlog.Fatal(err)
@@ -137,7 +132,6 @@ func ExampleClient_VerifyHeaderAtHeight() {
 		primary,
 		[]provider.Provider{primary}, // NOTE: primary should not be used here
 		dbs.New(db, chainID),
-		UpdatePeriod(0),
 		// Logger(log.TestingLogger()),
 	)
 	if err != nil {

lite2/rpc/client.go

@@ -322,20 +322,8 @@ func (c *Client) UnsubscribeAll(ctx context.Context, subscriber string) error {
 }
 
 func (c *Client) updateLiteClientIfNeededTo(height int64) (*types.SignedHeader, error) {
-	lastTrustedHeight, err := c.lc.LastTrustedHeight()
-	if err != nil {
-		return nil, errors.Wrap(err, "LastTrustedHeight")
-	}
-
-	if lastTrustedHeight < height {
-		return c.lc.VerifyHeaderAtHeight(height, time.Now())
-	}
-
-	h, err := c.lc.TrustedHeader(height)
-	if err != nil {
-		return nil, errors.Wrapf(err, "TrustedHeader(#%d)", height)
-	}
-	return h, nil
+	h, err := c.lc.VerifyHeaderAtHeight(height, time.Now())
+	return h, errors.Wrapf(err, "failed to update light client to %d", height)
 }
 
 func (c *Client) RegisterOpDecoder(typ string, dec merkle.OpDecoder) {