Browse Source

lite2: remove expiration checks on functions that don't require them (#4477)

closes: #4455

Verifying backwards checks that the trustedHeader hasn't expired both before and after the loop in case of verifying many headers (a longer operation), but not during the loop itself.

TrustedHeader() no longer checks whether the header saved in the store has expired.

Tests have been updated to reflect the changes

## Commits:

* verify headers backwards out of trust period

* removed expiration check in trusted header func

* modified tests to reflect changes

* wrote new tests for backwards verification

* modified TrustedHeader and TrustedValSet functions

* condensed test functions

* condensed test functions further

* fix build error

* update doc

* add comments

* remove unnecessary declaration

* extract latestHeight check into a separate func

Co-authored-by: Callum Waters <cmwaters19@gmail.com>
pull/4480/head
Anton Kaliaev 4 years ago
committed by GitHub
parent
commit
6daea31f50
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 202 additions and 176 deletions
  1. +2
    -2
      docs/architecture/adr-046-light-client-implementation.md
  2. +58
    -63
      lite2/client.go
  3. +138
    -107
      lite2/client_test.go
  4. +2
    -2
      lite2/example_test.go
  5. +2
    -2
      lite2/rpc/client.go

+ 2
- 2
docs/architecture/adr-046-light-client-implementation.md View File

@ -27,8 +27,8 @@ type Client interface {
Cleanup() error
// get trusted headers & validators
TrustedHeader(height int64, now time.Time) (*types.SignedHeader, error)
TrustedValidatorSet(height int64, now time.Time) (*types.ValidatorSet, error)
TrustedHeader(height int64) (*types.SignedHeader, error)
TrustedValidatorSet(height int64) (*types.ValidatorSet, error)
LastTrustedHeight() (int64, error)
FirstTrustedHeight() (int64, error)


+ 58
- 63
lite2/client.go View File

@ -411,8 +411,6 @@ func (c *Client) Stop() {
}
// TrustedHeader returns a trusted header at the given height (0 - the latest).
// If a header is missing in trustedStore (e.g. it was skipped during
// bisection), it will be downloaded from primary.
//
// Headers along with validator sets, which can't be trusted anymore, are
// removed once a day (can be changed with RemoveNoLongerTrustedHeadersPeriod
@ -421,77 +419,65 @@ func (c *Client) Stop() {
// height must be >= 0.
//
// It returns an error if:
// - header expired, therefore can't be trusted (ErrOldHeaderExpired);
// - there are some issues with the trusted store, although that should not
// happen normally;
// - negative height is passed;
// - header has not been verified yet
// - header has not been verified yet and is therefore not in the store
//
// Safe for concurrent use by multiple goroutines.
func (c *Client) TrustedHeader(height int64, now time.Time) (*types.SignedHeader, error) {
if height < 0 {
return nil, errors.New("negative height")
}
// 1) Get latest height.
latestHeight, err := c.LastTrustedHeight()
if err != nil {
return nil, err
}
if latestHeight == -1 {
return nil, errors.New("no headers exist")
}
if height > latestHeight {
return nil, errors.Errorf("unverified header requested (latest: %d)", latestHeight)
}
if height == 0 {
height = latestHeight
}
// 2) Get header from store.
h, err := c.trustedStore.SignedHeader(height)
func (c *Client) TrustedHeader(height int64) (*types.SignedHeader, error) {
height, err := c.compareWithLatestHeight(height)
if err != nil {
return nil, err
}
// 3) Ensure header can still be trusted.
if HeaderExpired(h, c.trustingPeriod, now) {
return nil, ErrOldHeaderExpired{h.Time.Add(c.trustingPeriod), now}
}
return h, nil
return c.trustedStore.SignedHeader(height)
}
// TrustedValidatorSet returns a trusted validator set at the given height. If
// a validator set is missing in trustedStore (e.g. the associated header was
// skipped during bisection), it will be downloaded from primary.
// TrustedValidatorSet returns a trusted validator set at the given height (0 -
// latest).
//
// height must be >= 0.
//
// Headers along with validator sets, which can't be trusted anymore, are
// Headers along with validator sets are
// removed once a day (can be changed with RemoveNoLongerTrustedHeadersPeriod
// option).
//
// It returns an error if:
// - header signed by that validator set expired (ErrOldHeaderExpired)
// Function returns an error if:
// - there are some issues with the trusted store, although that should not
// happen normally;
// - negative height is passed;
// - header signed by that validator set has not been verified yet
//
// Safe for concurrent use by multiple goroutines.
func (c *Client) TrustedValidatorSet(height int64, now time.Time) (*types.ValidatorSet, error) {
// Checks height is positive and header is not expired.
// Additionally, it fetches validator set from primary if it's missing in
// store.
_, err := c.TrustedHeader(height, now)
func (c *Client) TrustedValidatorSet(height int64) (*types.ValidatorSet, error) {
height, err := c.compareWithLatestHeight(height)
if err != nil {
return nil, err
}
return c.trustedStore.ValidatorSet(height)
}
func (c *Client) compareWithLatestHeight(height int64) (int64, error) {
latestHeight, err := c.LastTrustedHeight()
if err != nil {
return 0, err
}
if latestHeight == -1 {
return 0, errors.New("no headers exist")
}
switch {
case height > latestHeight:
return 0, errors.Errorf("unverified header/valset requested (latest: %d)", latestHeight)
case height == 0:
return latestHeight, nil
case height < 0:
return 0, errors.New("negative height")
}
return height, nil
}
// LastTrustedHeight returns a last trusted height. -1 and nil are returned if
// there are no trusted headers.
//
@ -528,14 +514,11 @@ func (c *Client) VerifyHeaderAtHeight(height int64, now time.Time) (*types.Signe
}
// Check if header already verified.
h, err := c.TrustedHeader(height, now)
switch err.(type) {
case nil:
h, err := c.TrustedHeader(height)
if err == nil {
c.logger.Info("Header has already been verified", "height", height, "hash", hash2str(h.Hash()))
// Return already trusted header
return h, nil
case ErrOldHeaderExpired:
return nil, err
}
// Request the header and the vals.
@ -567,19 +550,19 @@ func (c *Client) VerifyHeaderAtHeight(height int64, now time.Time) (*types.Signe
// provider, provider.ErrSignedHeaderNotFound /
// provider.ErrValidatorSetNotFound error is returned.
func (c *Client) VerifyHeader(newHeader *types.SignedHeader, newVals *types.ValidatorSet, now time.Time) error {
if newHeader.Height <= 0 {
return errors.New("negative or zero height")
}
// Check if newHeader already verified.
h, err := c.TrustedHeader(newHeader.Height, now)
switch err.(type) {
case nil:
// Make sure it's the same header.
h, err := c.TrustedHeader(newHeader.Height)
if err == nil {
if !bytes.Equal(h.Hash(), newHeader.Hash()) {
return errors.Errorf("existing trusted header %X does not match newHeader %X", h.Hash(), newHeader.Hash())
}
c.logger.Info("Header has already been verified",
"height", newHeader.Height, "hash", hash2str(newHeader.Hash()))
return nil
case ErrOldHeaderExpired:
return err
}
return c.verifyHeader(newHeader, newVals, now)
@ -793,8 +776,11 @@ func (c *Client) updateTrustedHeaderAndVals(h *types.SignedHeader, vals *types.V
return errors.Wrap(err, "failed to save trusted header")
}
c.latestTrustedHeader = h
c.latestTrustedVals = vals
// Only update latestTrustedHeader if we move forward (not backwards).
if c.latestTrustedHeader == nil || h.Height > c.latestTrustedHeader.Height {
c.latestTrustedHeader = h
c.latestTrustedVals = vals
}
return nil
}
@ -814,17 +800,21 @@ func (c *Client) fetchHeaderAndValsAtHeight(height int64) (*types.SignedHeader,
}
// Backwards verification (see VerifyHeaderBackwards func in the spec)
func (c *Client) backwards(trustedHeader *types.SignedHeader, newHeader *types.SignedHeader,
func (c *Client) backwards(
initiallyTrustedHeader *types.SignedHeader,
newHeader *types.SignedHeader,
now time.Time) error {
if HeaderExpired(initiallyTrustedHeader, c.trustingPeriod, now) {
return ErrOldHeaderExpired{initiallyTrustedHeader.Time.Add(c.trustingPeriod), now}
}
var (
trustedHeader = initiallyTrustedHeader
interimHeader *types.SignedHeader
err error
)
if HeaderExpired(newHeader, c.trustingPeriod, now) {
return ErrOldHeaderExpired{newHeader.Time.Add(c.trustingPeriod), now}
}
for trustedHeader.Height > newHeader.Height {
interimHeader, err = c.signedHeaderFromPrimary(trustedHeader.Height - 1)
if err != nil {
@ -850,6 +840,11 @@ func (c *Client) backwards(trustedHeader *types.SignedHeader, newHeader *types.S
trustedHeader = interimHeader
}
// Initially trusted header might have expired at this point.
if HeaderExpired(initiallyTrustedHeader, c.trustingPeriod, now) {
return ErrOldHeaderExpired{initiallyTrustedHeader.Time.Add(c.trustingPeriod), now}
}
return nil
}


+ 138
- 107
lite2/client_test.go View File

@ -39,18 +39,21 @@ var (
Height: 1,
Hash: h1.Hash(),
}
valSet = map[int64]*types.ValidatorSet{
1: vals,
2: vals,
3: vals,
4: vals,
}
headerSet = map[int64]*types.SignedHeader{
1: h1,
2: h2,
3: h3,
}
fullNode = mockp.New(
chainID,
map[int64]*types.SignedHeader{
1: h1,
2: h2,
3: h3,
},
map[int64]*types.ValidatorSet{
1: vals,
2: vals,
3: vals,
},
headerSet,
valSet,
)
deadNode = mockp.NewDeadMock(chainID)
)
@ -76,11 +79,7 @@ func TestClient_SequentialVerification(t *testing.T) {
// last header (3/3 signed)
3: h3,
},
map[int64]*types.ValidatorSet{
1: vals,
2: vals,
3: vals,
},
valSet,
false,
false,
},
@ -109,11 +108,7 @@ func TestClient_SequentialVerification(t *testing.T) {
3: keys.GenSignedHeader(chainID, 3, bTime.Add(2*time.Hour), nil, vals, vals,
[]byte("app_hash"), []byte("cons_hash"), []byte("results_hash"), 0, len(keys)),
},
map[int64]*types.ValidatorSet{
1: vals,
2: vals,
3: vals,
},
valSet,
false,
true,
},
@ -129,24 +124,13 @@ func TestClient_SequentialVerification(t *testing.T) {
3: keys.GenSignedHeader(chainID, 3, bTime.Add(2*time.Hour), nil, vals, vals,
[]byte("app_hash"), []byte("cons_hash"), []byte("results_hash"), len(keys)-1, len(keys)),
},
map[int64]*types.ValidatorSet{
1: vals,
2: vals,
3: vals,
},
valSet,
false,
true,
},
{
"bad: different validator set at height 3",
map[int64]*types.SignedHeader{
// trusted header
1: h1,
// interim header (3/3 signed)
2: h2,
// last header (3/3 signed)
3: h3,
},
headerSet,
map[int64]*types.ValidatorSet{
1: vals,
2: vals,
@ -221,11 +205,7 @@ func TestClient_SkippingVerification(t *testing.T) {
// last header (3/3 signed)
3: h3,
},
map[int64]*types.ValidatorSet{
1: vals,
2: vals,
3: vals,
},
valSet,
false,
false,
},
@ -357,12 +337,12 @@ func TestClientRemovesNoLongerTrustedHeaders(t *testing.T) {
c.RemoveNoLongerTrustedHeaders(now)
// Check expired headers are no longer available.
h, err := c.TrustedHeader(1, now)
h, err := c.TrustedHeader(1)
assert.Error(t, err)
assert.Nil(t, h)
// Check not expired headers are available.
h, err = c.TrustedHeader(2, now)
h, err = c.TrustedHeader(2)
assert.NoError(t, err)
assert.NotNil(t, h)
}
@ -385,7 +365,7 @@ func TestClient_Cleanup(t *testing.T) {
require.NoError(t, err)
// Check no headers exist after Cleanup.
h, err := c.TrustedHeader(1, bTime.Add(1*time.Second))
h, err := c.TrustedHeader(1)
assert.Error(t, err)
assert.Nil(t, h)
}
@ -411,7 +391,7 @@ func TestClientRestoresTrustedHeaderAfterStartup1(t *testing.T) {
require.NoError(t, err)
defer c.Stop()
h, err := c.TrustedHeader(1, bTime.Add(1*time.Second))
h, err := c.TrustedHeader(1)
assert.NoError(t, err)
assert.NotNil(t, h)
assert.Equal(t, h.Hash(), h1.Hash())
@ -433,9 +413,7 @@ func TestClientRestoresTrustedHeaderAfterStartup1(t *testing.T) {
// trusted header
1: header1,
},
map[int64]*types.ValidatorSet{
1: vals,
},
valSet,
)
c, err := NewClient(
@ -455,7 +433,7 @@ func TestClientRestoresTrustedHeaderAfterStartup1(t *testing.T) {
require.NoError(t, err)
defer c.Stop()
h, err := c.TrustedHeader(1, bTime.Add(1*time.Second))
h, err := c.TrustedHeader(1)
assert.NoError(t, err)
assert.NotNil(t, h)
assert.Equal(t, h.Hash(), header1.Hash())
@ -488,7 +466,7 @@ func TestClientRestoresTrustedHeaderAfterStartup2(t *testing.T) {
defer c.Stop()
// Check we still have the 1st header (+header+).
h, err := c.TrustedHeader(1, bTime.Add(2*time.Hour).Add(1*time.Second))
h, err := c.TrustedHeader(1)
assert.NoError(t, err)
assert.NotNil(t, h)
assert.Equal(t, h.Hash(), h1.Hash())
@ -514,10 +492,7 @@ func TestClientRestoresTrustedHeaderAfterStartup2(t *testing.T) {
1: diffHeader1,
2: diffHeader2,
},
map[int64]*types.ValidatorSet{
1: vals,
2: vals,
},
valSet,
)
c, err := NewClient(
@ -538,7 +513,7 @@ func TestClientRestoresTrustedHeaderAfterStartup2(t *testing.T) {
defer c.Stop()
// Check we no longer have the invalid 1st header (+header+).
h, err := c.TrustedHeader(1, bTime.Add(2*time.Hour).Add(1*time.Second))
h, err := c.TrustedHeader(1)
assert.Error(t, err)
assert.Nil(t, h)
}
@ -557,23 +532,11 @@ func TestClientRestoresTrustedHeaderAfterStartup3(t *testing.T) {
err = trustedStore.SaveSignedHeaderAndValidatorSet(h2, vals)
require.NoError(t, err)
primary := mockp.New(
chainID,
map[int64]*types.SignedHeader{
1: h1,
2: h2,
},
map[int64]*types.ValidatorSet{
1: vals,
2: vals,
},
)
c, err := NewClient(
chainID,
trustOptions,
primary,
[]provider.Provider{primary},
fullNode,
[]provider.Provider{fullNode},
trustedStore,
Logger(log.TestingLogger()),
)
@ -583,13 +546,13 @@ func TestClientRestoresTrustedHeaderAfterStartup3(t *testing.T) {
defer c.Stop()
// Check we still have the 1st header (+header+).
h, err := c.TrustedHeader(1, bTime.Add(2*time.Hour).Add(1*time.Second))
h, err := c.TrustedHeader(1)
assert.NoError(t, err)
assert.NotNil(t, h)
assert.Equal(t, h.Hash(), h1.Hash())
// Check we no longer have 2nd header (+header2+).
h, err = c.TrustedHeader(2, bTime.Add(2*time.Hour).Add(1*time.Second))
h, err = c.TrustedHeader(2)
assert.Error(t, err)
assert.Nil(t, h)
}
@ -615,9 +578,7 @@ func TestClientRestoresTrustedHeaderAfterStartup3(t *testing.T) {
map[int64]*types.SignedHeader{
1: header1,
},
map[int64]*types.ValidatorSet{
1: vals,
},
valSet,
)
c, err := NewClient(
@ -638,13 +599,13 @@ func TestClientRestoresTrustedHeaderAfterStartup3(t *testing.T) {
defer c.Stop()
// Check we have swapped invalid 1st header (+header+) with correct one (+header1+).
h, err := c.TrustedHeader(1, bTime.Add(2*time.Hour).Add(1*time.Second))
h, err := c.TrustedHeader(1)
assert.NoError(t, err)
assert.NotNil(t, h)
assert.Equal(t, h.Hash(), header1.Hash())
// Check we no longer have invalid 2nd header (+header2+).
h, err = c.TrustedHeader(2, bTime.Add(2*time.Hour).Add(1*time.Second))
h, err = c.TrustedHeader(2)
assert.Error(t, err)
assert.Nil(t, h)
}
@ -668,7 +629,7 @@ func TestClient_Update(t *testing.T) {
err = c.Update(bTime.Add(2 * time.Hour))
require.NoError(t, err)
h, err := c.TrustedHeader(3, bTime.Add(2*time.Hour))
h, err := c.TrustedHeader(3)
assert.NoError(t, err)
require.NotNil(t, h)
assert.EqualValues(t, 3, h.Height)
@ -709,11 +670,11 @@ func TestClient_Concurrency(t *testing.T) {
_, err = c.FirstTrustedHeight()
assert.NoError(t, err)
h, err := c.TrustedHeader(1, bTime.Add(2*time.Hour))
h, err := c.TrustedHeader(1)
assert.NoError(t, err)
assert.NotNil(t, h)
vals, err := c.TrustedValidatorSet(2, bTime.Add(2*time.Hour))
vals, err := c.TrustedValidatorSet(2)
assert.NoError(t, err)
assert.NotNil(t, vals)
}()
@ -743,41 +704,111 @@ func TestClientReplacesPrimaryWithWitnessIfPrimaryIsUnavailable(t *testing.T) {
}
func TestClient_BackwardsVerification(t *testing.T) {
c, err := NewClient(
chainID,
TrustOptions{
Period: 1 * time.Hour,
Height: 3,
Hash: h3.Hash(),
},
fullNode,
[]provider.Provider{fullNode},
dbs.New(dbm.NewMemDB(), chainID),
UpdatePeriod(0),
Logger(log.TestingLogger()),
)
require.NoError(t, err)
{
c, err := NewClient(
chainID,
TrustOptions{
Period: 1 * time.Hour,
Height: 3,
Hash: h3.Hash(),
},
fullNode,
[]provider.Provider{fullNode},
dbs.New(dbm.NewMemDB(), chainID),
UpdatePeriod(0),
Logger(log.TestingLogger()),
)
require.NoError(t, err)
// 1) header is missing => expect no error
h, err := c.VerifyHeaderAtHeight(2, bTime.Add(1*time.Hour).Add(1*time.Second))
require.NoError(t, err)
if assert.NotNil(t, h) {
assert.EqualValues(t, 2, h.Height)
// 1) header is missing => expect no error
h, err := c.VerifyHeaderAtHeight(2, bTime.Add(1*time.Hour).Add(1*time.Second))
require.NoError(t, err)
if assert.NotNil(t, h) {
assert.EqualValues(t, 2, h.Height)
}
// 2) untrusted header is expired but trusted header is not => expect no error
h, err = c.VerifyHeaderAtHeight(1, bTime.Add(1*time.Hour).Add(1*time.Second))
assert.NoError(t, err)
assert.NotNil(t, h)
// 3) already stored headers should return the header without error
h, err = c.VerifyHeaderAtHeight(2, bTime.Add(1*time.Hour).Add(1*time.Second))
assert.NoError(t, err)
assert.NotNil(t, h)
}
{
c, err := NewClient(
chainID,
TrustOptions{
Period: 1 * time.Hour,
Height: 3,
Hash: h3.Hash(),
},
fullNode,
[]provider.Provider{fullNode},
dbs.New(dbm.NewMemDB(), chainID),
UpdatePeriod(0),
Logger(log.TestingLogger()),
)
require.NoError(t, err)
// 2) header is missing, but it's expired => expect error
h, err = c.VerifyHeaderAtHeight(1, bTime.Add(1*time.Hour).Add(1*time.Second))
assert.Error(t, err)
assert.NotNil(t, h)
// 3) trusted header has expired => expect error
_, err = c.VerifyHeaderAtHeight(1, bTime.Add(4*time.Hour).Add(1*time.Second))
assert.Error(t, err)
}
{
testCases := []struct {
provider provider.Provider
}{
{
// provides incorrect height
mockp.New(
chainID,
map[int64]*types.SignedHeader{
1: h1,
2: keys.GenSignedHeader(chainID, 1, bTime.Add(1*time.Hour), nil, vals, vals,
[]byte("app_hash"), []byte("cons_hash"), []byte("results_hash"), 0, len(keys)),
3: h3,
},
valSet,
),
},
{
// provides incorrect hash
mockp.New(
chainID,
map[int64]*types.SignedHeader{
1: h1,
2: keys.GenSignedHeader(chainID, 2, bTime.Add(30*time.Minute), nil, vals, vals,
[]byte("app_hash"), []byte("cons_hash"), []byte("results_hash"), 0, len(keys)),
3: h3,
},
valSet,
),
},
}
// 3) already stored headers should return the header without error
h, err = c.VerifyHeaderAtHeight(3, bTime.Add(1*time.Hour).Add(1*time.Second))
assert.NoError(t, err)
assert.NotNil(t, h)
for _, tc := range testCases {
c, err := NewClient(
chainID,
TrustOptions{
Period: 1 * time.Hour,
Height: 3,
Hash: h3.Hash(),
},
tc.provider,
[]provider.Provider{tc.provider},
dbs.New(dbm.NewMemDB(), chainID),
UpdatePeriod(0),
Logger(log.TestingLogger()),
)
require.NoError(t, err)
// 4) cannot verify a header in the future
_, err = c.VerifyHeaderAtHeight(4, bTime.Add(1*time.Hour).Add(1*time.Second))
assert.Error(t, err)
_, err = c.VerifyHeaderAtHeight(2, bTime.Add(1*time.Hour).Add(1*time.Second))
assert.Error(t, err)
}
}
}
func TestClient_NewClientFromTrustedStore(t *testing.T) {
@ -797,7 +828,7 @@ func TestClient_NewClientFromTrustedStore(t *testing.T) {
// 2) Check header exists (deadNode is being used to ensure we're not getting
// it from primary)
h, err := c.TrustedHeader(1, bTime.Add(1*time.Second))
h, err := c.TrustedHeader(1)
assert.NoError(t, err)
assert.EqualValues(t, 1, h.Height)
}


+ 2
- 2
lite2/example_test.go View File

@ -76,7 +76,7 @@ func TestExample_Client_AutoUpdate(t *testing.T) {
time.Sleep(2 * time.Second)
h, err := c.TrustedHeader(0, time.Now())
h, err := c.TrustedHeader(0)
if err != nil {
stdlog.Fatal(err)
}
@ -146,7 +146,7 @@ func TestExample_Client_ManualUpdate(t *testing.T) {
stdlog.Fatal(err)
}
h, err := c.TrustedHeader(3, time.Now())
h, err := c.TrustedHeader(3)
if err != nil {
stdlog.Fatal(err)
}


+ 2
- 2
lite2/rpc/client.go View File

@ -191,7 +191,7 @@ func (c *Client) BlockchainInfo(minHeight, maxHeight int64) (*ctypes.ResultBlock
// Verify each of the BlockMetas.
for _, meta := range res.BlockMetas {
h, err := c.lc.TrustedHeader(meta.Header.Height, time.Now())
h, err := c.lc.TrustedHeader(meta.Header.Height)
if err != nil {
return nil, errors.Wrapf(err, "TrustedHeader(%d)", meta.Header.Height)
}
@ -331,7 +331,7 @@ func (c *Client) updateLiteClientIfNeededTo(height int64) (*types.SignedHeader,
return c.lc.VerifyHeaderAtHeight(height, time.Now())
}
h, err := c.lc.TrustedHeader(height, time.Now())
h, err := c.lc.TrustedHeader(height)
if err != nil {
return nil, errors.Wrapf(err, "TrustedHeader(#%d)", height)
}


Loading…
Cancel
Save