From d3f965ba68fa39af56793bc78a9d4bcdd6c57551 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 6 Mar 2020 17:10:28 +0400 Subject: [PATCH] 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 | 9 ++++----- .../adr-046-light-client-implementation.md | 10 +++++----- lite2/client.go | 17 +++++++++-------- lite2/client_test.go | 12 +++++------- lite2/example_test.go | 9 ++------- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/docs/architecture/adr-042-state-sync.md b/docs/architecture/adr-042-state-sync.md index d525a4974..89d95f2e4 100644 --- a/docs/architecture/adr-042-state-sync.md +++ b/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 diff --git a/docs/architecture/adr-046-light-client-implementation.md b/docs/architecture/adr-046-light-client-implementation.md index aa7df3450..37a7c83c5 100644 --- a/docs/architecture/adr-046-light-client-implementation.md +++ b/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 } ``` diff --git a/lite2/client.go b/lite2/client.go index b353f70d8..2eefd472d 100644 --- a/lite2/client.go +++ b/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 diff --git a/lite2/client_test.go b/lite2/client_test.go index e9023c3ad..65ea55122 100644 --- a/lite2/client_test.go +++ b/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) diff --git a/lite2/example_test.go b/lite2/example_test.go index 34889ef98..e8c3b8bb3 100644 --- a/lite2/example_test.go +++ b/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")