diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 184df82cc..401d1d8b6 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -28,10 +28,13 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [proto/p2p] Renamed `DefaultNodeInfo` and `DefaultNodeInfoOther` to `NodeInfo` and `NodeInfoOther` (@erikgrinaker) - [proto/p2p] Rename `NodeInfo.default_node_id` to `node_id` (@erikgrinaker) - [libs/os] Kill() and {Must,}{Read,Write}File() functions have been removed. (@alessio) - - [store] \#5848 Remove block store state in favor of using the db iterators directly (@cmwaters) - [state] \#5864 Use an iterator when pruning state (@cmwaters) + - [store] \#5848 Remove block store state in favor of using the db iterators directly (@cmwaters) + - [state] \#5864 Use an iterator when pruning state (@cmwaters) - [types] \#6023 Remove `tm2pb.Header`, `tm2pb.BlockID`, `tm2pb.PartSetHeader` and `tm2pb.NewValidatorUpdate`. - Each of the above types has a `ToProto` and `FromProto` method or function which replaced this logic. - [rpc/client/http] \#6022 Change `timeout` type to `time.Duration` in `NewWithTimeout` + - [light] \#6054 Move `MaxRetryAttempt` option from client to provider. + - `NewWithOptions` now sets the max retry attempts and timeouts (@cmwaters) - Blockchain Protocol diff --git a/light/client.go b/light/client.go index 04f9653fc..0b0ee748e 100644 --- a/light/client.go +++ b/light/client.go @@ -21,8 +21,7 @@ const ( sequential mode = iota + 1 skipping - defaultPruningSize = 1000 - defaultMaxRetryAttempts = 10 + defaultPruningSize = 1000 // For verifySkipping, when using the cache of headers from the previous batch, // they will always be at a height greater than 1/2 (normal verifySkipping) so to // find something in between the range, 9/16 is used. @@ -90,14 +89,6 @@ func Logger(l log.Logger) Option { } } -// MaxRetryAttempts option can be used to set max attempts before replacing -// primary with a witness. -func MaxRetryAttempts(max uint16) Option { - return func(c *Client) { - c.maxRetryAttempts = max - } -} - // MaxClockDrift defines how much new header's time can drift into // the future. Default: 10s. func MaxClockDrift(d time.Duration) Option { @@ -116,7 +107,6 @@ type Client struct { trustingPeriod time.Duration // see TrustOptions.Period verificationMode mode trustLevel tmmath.Fraction - maxRetryAttempts uint16 // see MaxRetryAttempts option maxClockDrift time.Duration // Mutex for locking during changes of the light clients providers @@ -202,7 +192,6 @@ func NewClientFromTrustedStore( trustingPeriod: trustingPeriod, verificationMode: skipping, trustLevel: DefaultTrustLevel, - maxRetryAttempts: defaultMaxRetryAttempts, maxClockDrift: defaultMaxClockDrift, primary: primary, witnesses: witnesses, diff --git a/light/client_test.go b/light/client_test.go index 7e375ade3..2489b23c8 100644 --- a/light/client_test.go +++ b/light/client_test.go @@ -768,7 +768,6 @@ func TestClientReplacesPrimaryWithWitnessIfPrimaryIsUnavailable(t *testing.T) { []provider.Provider{fullNode, fullNode}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), - light.MaxRetryAttempts(1), ) require.NoError(t, err) @@ -946,7 +945,6 @@ func TestClientRemovesWitnessIfItSendsUsIncorrectHeader(t *testing.T) { []provider.Provider{badProvider1, badProvider2}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), - light.MaxRetryAttempts(1), ) // witness should have behaved properly -> no error require.NoError(t, err) @@ -1086,7 +1084,6 @@ func TestClientEnsureValidHeadersAndValSets(t *testing.T) { badNode, []provider.Provider{badNode, badNode}, dbs.New(dbm.NewMemDB()), - light.MaxRetryAttempts(1), ) require.NoError(t, err) diff --git a/light/detector_test.go b/light/detector_test.go index 04d3c3377..d81467f9b 100644 --- a/light/detector_test.go +++ b/light/detector_test.go @@ -56,7 +56,6 @@ func TestLightClientAttackEvidence_Lunatic(t *testing.T) { []provider.Provider{witness}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), - light.MaxRetryAttempts(1), ) require.NoError(t, err) @@ -138,7 +137,6 @@ func TestLightClientAttackEvidence_Equivocation(t *testing.T) { []provider.Provider{witness}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), - light.MaxRetryAttempts(1), verificationOption, ) require.NoError(t, err) @@ -193,7 +191,6 @@ func TestClientDivergentTraces1(t *testing.T) { []provider.Provider{witness}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), - light.MaxRetryAttempts(1), ) require.Error(t, err) assert.Contains(t, err.Error(), "does not match primary") @@ -217,7 +214,6 @@ func TestClientDivergentTraces2(t *testing.T) { []provider.Provider{deadNode, deadNode, primary}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), - light.MaxRetryAttempts(1), ) require.NoError(t, err) @@ -252,7 +248,6 @@ func TestClientDivergentTraces3(t *testing.T) { []provider.Provider{witness}, dbs.New(dbm.NewMemDB()), light.Logger(log.TestingLogger()), - light.MaxRetryAttempts(1), ) require.NoError(t, err) diff --git a/light/provider/http/http.go b/light/provider/http/http.go index 009542971..a48cce33b 100644 --- a/light/provider/http/http.go +++ b/light/provider/http/http.go @@ -18,20 +18,28 @@ var ( // This is very brittle, see: https://github.com/tendermint/tendermint/issues/4740 regexpMissingHeight = regexp.MustCompile(`height \d+ (must be less than or equal to|is not available)`) - maxRetryAttempts = 10 - timeout = 5 * time.Second + defaultMaxRetryAttempts = 10 + defaultTimeout = 5 * time.Second ) // http provider uses an RPC client to obtain the necessary information. type http struct { chainID string client rpcclient.RemoteClient + + maxRetryAttempts int } // New creates a HTTP provider, which is using the rpchttp.HTTP client under // the hood. If no scheme is provided in the remote URL, http will be used by // default. The 5s timeout is used for all requests. func New(chainID, remote string) (provider.Provider, error) { + return NewWithOptions(chainID, remote, defaultMaxRetryAttempts, defaultTimeout) +} + +// NewWithOptions is an extension to creating a new http provider that allows the addition +// of a specified timeout and maxRetryAttempts +func NewWithOptions(chainID, remote string, maxRetryAttempts int, timeout time.Duration) (provider.Provider, error) { // Ensure URL scheme is set (default HTTP) when not provided. if !strings.Contains(remote, "://") { remote = "http://" + remote @@ -42,14 +50,19 @@ func New(chainID, remote string) (provider.Provider, error) { return nil, err } - return NewWithClient(chainID, httpClient), nil + return NewWithClientAndOptions(chainID, httpClient, maxRetryAttempts), nil } -// NewWithClient allows you to provide a custom client. func NewWithClient(chainID string, client rpcclient.RemoteClient) provider.Provider { + return NewWithClientAndOptions(chainID, client, defaultMaxRetryAttempts) +} + +// NewWithClient allows you to provide a custom client. +func NewWithClientAndOptions(chainID string, client rpcclient.RemoteClient, maxRetryAttempts int) provider.Provider { return &http{ - client: client, - chainID: chainID, + client: client, + chainID: chainID, + maxRetryAttempts: maxRetryAttempts, } } @@ -108,7 +121,7 @@ func (p *http) validatorSet(ctx context.Context, height *int64) (*types.Validato ) for len(vals) != total && page <= maxPages { - for attempt := 1; attempt <= maxRetryAttempts; attempt++ { + for attempt := 1; attempt <= p.maxRetryAttempts; attempt++ { res, err := p.client.Validators(ctx, height, &page, &perPage) if err != nil { // TODO: standardize errors on the RPC side @@ -116,7 +129,7 @@ func (p *http) validatorSet(ctx context.Context, height *int64) (*types.Validato return nil, provider.ErrLightBlockNotFound } // if we have exceeded retry attempts then return no response error - if attempt == maxRetryAttempts { + if attempt == p.maxRetryAttempts { return nil, provider.ErrNoResponse } // else we wait and try again with exponential backoff @@ -153,7 +166,7 @@ func (p *http) validatorSet(ctx context.Context, height *int64) (*types.Validato } func (p *http) signedHeader(ctx context.Context, height *int64) (*types.SignedHeader, error) { - for attempt := 1; attempt <= maxRetryAttempts; attempt++ { + for attempt := 1; attempt <= p.maxRetryAttempts; attempt++ { commit, err := p.client.Commit(ctx, height) if err != nil { // TODO: standardize errors on the RPC side diff --git a/statesync/stateprovider.go b/statesync/stateprovider.go index 5ecbeb166..cd833578e 100644 --- a/statesync/stateprovider.go +++ b/statesync/stateprovider.go @@ -71,7 +71,7 @@ func NewLightClientStateProvider( } lc, err := light.NewClient(ctx, chainID, trustOptions, providers[0], providers[1:], - lightdb.New(dbm.NewMemDB()), light.Logger(logger), light.MaxRetryAttempts(5)) + lightdb.New(dbm.NewMemDB()), light.Logger(logger)) if err != nil { return nil, err }