Browse Source

light: add case to catch cancelled contexts within the detector (backport #6701) (#6720)

pull/6783/head
mergify[bot] 3 years ago
committed by GitHub
parent
commit
b69ac23fd2
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 105 additions and 24 deletions
  1. +4
    -1
      CHANGELOG_PENDING.md
  2. +12
    -7
      light/client.go
  3. +60
    -1
      light/client_test.go
  4. +17
    -9
      light/detector.go
  5. +2
    -5
      light/provider/mock/deadmock.go
  6. +10
    -1
      light/provider/mock/mock.go

+ 4
- 1
CHANGELOG_PENDING.md View File

@ -20,10 +20,13 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
### FEATURES
- [rpc] [\#6717](https://github.com/tendermint/tendermint/pull/6717) introduce
`/genesis_chunked` rpc endpoint for handling large genesis files by chunking them
### IMPROVEMENTS
### BUG FIXES
- [light] [\#6685](https://github.com/tendermint/tendermint/pull/6685) fix bug
with incorrecly handling contexts which occasionally froze state sync.
with incorrectly handling contexts that would occasionally freeze state sync. (@cmwaters)

+ 12
- 7
light/client.go View File

@ -1152,14 +1152,19 @@ func (c *Client) compareFirstHeaderWithWitnesses(ctx context.Context, h *types.S
and remove witness. Otherwise, use the different primary`, e.WitnessIndex), "witness", c.witnesses[e.WitnessIndex])
return err
case errBadWitness:
// If witness sent us an invalid header, then remove it. If it didn't
// respond or couldn't find the block, then we ignore it and move on to
// the next witness.
if _, ok := e.Reason.(provider.ErrBadLightBlock); ok {
c.logger.Info("Witness sent us invalid header / vals -> removing it",
"witness", c.witnesses[e.WitnessIndex], "err", err)
witnessesToRemove = append(witnessesToRemove, e.WitnessIndex)
// If witness sent us an invalid header, then remove it
c.logger.Info("witness sent an invalid light block, removing...",
"witness", c.witnesses[e.WitnessIndex],
"err", err)
witnessesToRemove = append(witnessesToRemove, e.WitnessIndex)
default: // benign errors can be ignored with the exception of context errors
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
return err
}
// the witness either didn't respond or didn't have the block. We ignore it.
c.logger.Info("error comparing first header with witness. You may want to consider removing the witness",
"err", err)
}
}


+ 60
- 1
light/client_test.go View File

@ -2,6 +2,7 @@ package light_test
import (
"context"
"errors"
"sync"
"testing"
"time"
@ -776,7 +777,7 @@ func TestClientReplacesPrimaryWithWitnessIfPrimaryIsUnavailable(t *testing.T) {
require.NoError(t, err)
assert.NotEqual(t, c.Primary(), deadNode)
assert.Equal(t, 1, len(c.Witnesses()))
assert.Equal(t, 2, len(c.Witnesses()))
}
func TestClient_BackwardsVerification(t *testing.T) {
@ -1099,3 +1100,61 @@ func TestClientEnsureValidHeadersAndValSets(t *testing.T) {
}
}
func TestClientHandlesContexts(t *testing.T) {
p := mockp.New(genMockNode(chainID, 100, 10, 1, bTime))
genBlock, err := p.LightBlock(ctx, 1)
require.NoError(t, err)
// instantiate the light client with a timeout
ctxTimeOut, cancel := context.WithTimeout(ctx, 10*time.Millisecond)
defer cancel()
_, err = light.NewClient(
ctxTimeOut,
chainID,
light.TrustOptions{
Period: 24 * time.Hour,
Height: 1,
Hash: genBlock.Hash(),
},
p,
[]provider.Provider{p, p},
dbs.New(dbm.NewMemDB(), chainID),
)
require.Error(t, ctxTimeOut.Err())
require.Error(t, err)
require.True(t, errors.Is(err, context.DeadlineExceeded))
// instantiate the client for real
c, err := light.NewClient(
ctx,
chainID,
light.TrustOptions{
Period: 24 * time.Hour,
Height: 1,
Hash: genBlock.Hash(),
},
p,
[]provider.Provider{p, p},
dbs.New(dbm.NewMemDB(), chainID),
)
require.NoError(t, err)
// verify a block with a timeout
ctxTimeOutBlock, cancel := context.WithTimeout(ctx, 10*time.Millisecond)
defer cancel()
_, err = c.VerifyLightBlockAtHeight(ctxTimeOutBlock, 100, bTime.Add(100*time.Minute))
require.Error(t, ctxTimeOutBlock.Err())
require.Error(t, err)
require.True(t, errors.Is(err, context.DeadlineExceeded))
// verify a block with a cancel
ctxCancel, cancel := context.WithCancel(ctx)
defer cancel()
time.AfterFunc(10*time.Millisecond, cancel)
_, err = c.VerifyLightBlockAtHeight(ctxCancel, 100, bTime.Add(100*time.Minute))
require.Error(t, ctxCancel.Err())
require.Error(t, err)
require.True(t, errors.Is(err, context.Canceled))
}

+ 17
- 9
light/detector.go View File

@ -74,14 +74,18 @@ func (c *Client) detectDivergence(ctx context.Context, primaryTrace []*types.Lig
witnessesToRemove = append(witnessesToRemove, e.WitnessIndex)
case errBadWitness:
c.logger.Info("Witness returned an error during header comparison", "witness", c.witnesses[e.WitnessIndex],
"err", err)
// if witness sent us an invalid header, then remove it. If it didn't respond or couldn't find the block, then we
// ignore it and move on to the next witness
if _, ok := e.Reason.(provider.ErrBadLightBlock); ok {
c.logger.Info("Witness sent us invalid header / vals -> removing it", "witness", c.witnesses[e.WitnessIndex])
witnessesToRemove = append(witnessesToRemove, e.WitnessIndex)
// these are all melevolent errors and should result in removing the
// witness
c.logger.Info("witness returned an error during header comparison, removing...",
"witness", c.witnesses[e.WitnessIndex], "err", err)
witnessesToRemove = append(witnessesToRemove, e.WitnessIndex)
default:
// Benign errors which can be ignored unless there was a context
// canceled
if errors.Is(e, context.Canceled) || errors.Is(e, context.DeadlineExceeded) {
return e
}
c.logger.Info("error in light block request to witness", "err", err)
}
}
@ -118,7 +122,7 @@ func (c *Client) compareNewHeaderWithWitness(ctx context.Context, errc chan erro
// the witness hasn't been helpful in comparing headers, we mark the response and continue
// comparing with the rest of the witnesses
case provider.ErrNoResponse, provider.ErrLightBlockNotFound:
case provider.ErrNoResponse, provider.ErrLightBlockNotFound, context.DeadlineExceeded, context.Canceled:
errc <- err
return
@ -155,7 +159,11 @@ func (c *Client) compareNewHeaderWithWitness(ctx context.Context, errc chan erro
time.Sleep(2*c.maxClockDrift + c.maxBlockLag)
isTargetHeight, lightBlock, err = c.getTargetBlockOrLatest(ctx, h.Height, witness)
if err != nil {
errc <- errBadWitness{Reason: err, WitnessIndex: witnessIndex}
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
errc <- err
} else {
errc <- errBadWitness{Reason: err, WitnessIndex: witnessIndex}
}
return
}
if isTargetHeight {


+ 2
- 5
light/provider/mock/deadmock.go View File

@ -2,14 +2,11 @@ package mock
import (
"context"
"errors"
"github.com/tendermint/tendermint/light/provider"
"github.com/tendermint/tendermint/types"
)
var errNoResp = errors.New("no response from provider")
type deadMock struct {
chainID string
}
@ -24,9 +21,9 @@ func (p *deadMock) ChainID() string { return p.chainID }
func (p *deadMock) String() string { return "deadMock" }
func (p *deadMock) LightBlock(_ context.Context, height int64) (*types.LightBlock, error) {
return nil, errNoResp
return nil, provider.ErrNoResponse
}
func (p *deadMock) ReportEvidence(_ context.Context, ev types.Evidence) error {
return errNoResp
return provider.ErrNoResponse
}

+ 10
- 1
light/provider/mock/mock.go View File

@ -6,6 +6,7 @@ import (
"fmt"
"strings"
"sync"
"time"
"github.com/tendermint/tendermint/light/provider"
"github.com/tendermint/tendermint/types"
@ -60,9 +61,17 @@ func (p *Mock) String() string {
return fmt.Sprintf("Mock{headers: %s, vals: %v}", headers.String(), vals.String())
}
func (p *Mock) LightBlock(_ context.Context, height int64) (*types.LightBlock, error) {
func (p *Mock) LightBlock(ctx context.Context, height int64) (*types.LightBlock, error) {
p.mtx.Lock()
defer p.mtx.Unlock()
// allocate a window of time for contexts to be canceled
select {
case <-ctx.Done():
return nil, ctx.Err()
case <-time.After(10 * time.Millisecond):
}
var lb *types.LightBlock
if height > p.latestHeight {


Loading…
Cancel
Save