lite2: indicate success/failure of Update (#4536)

error itself is not enough since it only signals if there were any
errors. Either (types.SignedHeader) or (success bool) is needed to
indicate the status of the operation. Returning a header is optimal
since most of the clients will want to get a newly verified header
anyway.

docs/architecture/adr-042-state-sync.md

@@ -34,7 +34,7 @@ across different criteria:
 
 ### Implementation Question
 * What is the format of a snapshot
-    * Complete snapshot 
+    * Complete snapshot
     * Ordered IAVL key ranges
     * Compressed individually chunks which can be validated
 * How is data validated
@@ -58,7 +58,7 @@ request time. This solution would create an auxiliary data structure
 optimized for batch read/writes.
 
 Additionally the propsosals tend to vary on how they provide safety
-properties. 
+properties.
 
 **LightClient** Where a client can aquire the merkle root from the block
 headers synchronized from a trusted validator set. Subsets of the application state,
@@ -70,7 +70,7 @@ downloaded and compared against versions provided by a majority of
 peers.
 
 #### Lazy StateSync
-An [initial specification](https://docs.google.com/document/d/15MFsQtNA0MGBv7F096FFWRDzQ1vR6_dics5Y49vF8JU/edit?ts=5a0f3629) was published by Alexis Sellier.
+An initial specification was published by Alexis Sellier.
 In this design, the state has a given `size` of primitive elements (like
 keys or nodes), each element is assigned a number from 0 to `size-1`,
 and chunks consists of a range of such elements.  Ackratos raised
@@ -104,7 +104,7 @@ chunks and snappy compressed. Hashes of snappy compressed chunks are stored in a
 manifest file which co-ordinates the state-sync. Obtaining a correct manifest
 file seems to require an honest majority of peers. This means you may not find
 out the state is incorrect until you download the whole thing and compare it
-with a verified block header. 
+with a verified block header.
 
 A similar solution was implemented by Binance in
 [#3594](https://github.com/tendermint/tendermint/pull/3594)
@@ -229,7 +229,6 @@ Proposed
 
 ## References
 [sync: Sync current state without full replay for Applications](https://github.com/tendermint/tendermint/issues/828) - original issue
-[tendermint state sync proposal](https://docs.google.com/document/d/15MFsQtNA0MGBv7F096FFWRDzQ1vR6_dics5Y49vF8JU/edit?ts=5a0f3629) - Cloudhead proposal
 [tendermint state sync proposal 2](https://docs.google.com/document/d/1npGTAa1qxe8EQZ1wG0a0Sip9t5oX2vYZNUDwr_LVRR4/edit) - ackratos proposal
 [proposal 2 implementation](https://github.com/tendermint/tendermint/pull/3243)  - ackratos implementation
 [WIP General/Lazy State-Sync pseudo-spec](https://github.com/tendermint/tendermint/issues/3639) - Jae Proposal

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

@@ -15,7 +15,10 @@ latest header from primary and compares it with the currently trusted one.
 
 ```go
 type Client interface {
-	Cleanup() error
+	// 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) (*types.SignedHeader, error)
 
 	// get trusted headers & validators
 	TrustedHeader(height int64) (*types.SignedHeader, error)
@@ -28,10 +31,7 @@ type Client interface {
 	Primary() provider.Provider
 	Witnesses() []provider.Provider
 
-	// 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
+	Cleanup() error
 }
 ```
 

lite2/client.go

@@ -895,33 +895,34 @@ func (c *Client) removeWitness(idx int) {
 }
 
 // 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 {
+// comparing it with the existing one. It returns a new header on a successful
+// update. Otherwise, it returns nil (plus an error, if any).
+func (c *Client) Update(now time.Time) (*types.SignedHeader, error) {
 	lastTrustedHeight, err := c.LastTrustedHeight()
 	if err != nil {
-		return errors.Wrap(err, "can't get last trusted height")
+		return nil, errors.Wrap(err, "can't get last trusted height")
 	}
 
 	if lastTrustedHeight == -1 {
 		// no headers yet => wait
-		return nil
+		return nil, nil
 	}
 
 	latestHeader, latestVals, err := c.fetchHeaderAndValsAtHeight(0)
 	if err != nil {
-		return errors.Wrapf(err, "can't get latest header and vals")
+		return nil, errors.Wrapf(err, "can't get latest header and vals")
 	}
 
 	if latestHeader.Height > lastTrustedHeight {
 		err = c.VerifyHeader(latestHeader, latestVals, now)
 		if err != nil {
-			return err
+			return nil, err
 		}
-
 		c.logger.Info("Advanced to new state", "height", latestHeader.Height, "hash", hash2str(latestHeader.Hash()))
+		return latestHeader, nil
 	}
 
-	return nil
+	return nil, nil
 }
 
 // replaceProvider takes the first alternative provider and promotes it as the

lite2/client_test.go

@@ -604,13 +604,11 @@ func TestClient_Update(t *testing.T) {
 	require.NoError(t, err)
 
 	// should result in downloading & verifying header #3
-	err = c.Update(bTime.Add(2 * time.Hour))
-	require.NoError(t, err)
-
-	h, err := c.TrustedHeader(3)
+	h, err := c.Update(bTime.Add(2 * time.Hour))
 	assert.NoError(t, err)
-	require.NotNil(t, h)
-	assert.EqualValues(t, 3, h.Height)
+	if assert.NotNil(t, h) {
+		assert.EqualValues(t, 3, h.Height)
+	}
 
 	valSet, _, err := c.TrustedValidatorSet(3)
 	assert.NoError(t, err)
@@ -675,7 +673,7 @@ func TestClientReplacesPrimaryWithWitnessIfPrimaryIsUnavailable(t *testing.T) {
 	)
 
 	require.NoError(t, err)
-	err = c.Update(bTime.Add(2 * time.Hour))
+	_, err = c.Update(bTime.Add(2 * time.Hour))
 	require.NoError(t, err)
 
 	assert.NotEqual(t, c.Primary(), deadNode)

lite2/example_test.go

@@ -73,17 +73,12 @@ func ExampleClient_Update() {
 	// 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))
+	h, err := c.Update(time.Now().Add(30 * time.Minute))
 	if err != nil {
 		stdlog.Fatal(err)
 	}
 
-	h, err := c.TrustedHeader(0)
-	if err != nil {
-		stdlog.Fatal(err)
-	}
-
-	if h.Height > 2 {
+	if h != nil && h.Height > 2 {
 		fmt.Println("successful update")
 	} else {
 		fmt.Println("update failed")