light: improve timeout functionality (#6145)

cmd/tendermint/commands/light.go

@@ -206,11 +206,6 @@ func runProxy(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	rpcClient, err := rpchttp.New(primaryAddr, "/websocket")
-	if err != nil {
-		return fmt.Errorf("http client for %s: %w", primaryAddr, err)
-	}
-
 	cfg := rpcserver.DefaultConfig()
 	cfg.MaxBodyBytes = config.RPC.MaxBodyBytes
 	cfg.MaxHeaderBytes = config.RPC.MaxHeaderBytes
@@ -222,6 +217,11 @@ func runProxy(cmd *cobra.Command, args []string) error {
 		cfg.WriteTimeout = config.RPC.TimeoutBroadcastTxCommit + 1*time.Second
 	}
 
+	rpcClient, err := rpchttp.NewWithTimeout(primaryAddr, "/websocket", cfg.WriteTimeout)
+	if err != nil {
+		return fmt.Errorf("failed to create http client for %s: %w", primaryAddr, err)
+	}
+
 	p := lproxy.Proxy{
 		Addr:   listenAddr,
 		Config: cfg,

light/provider/http/http.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"math/rand"
+	"net/url"
 	"strings"
 	"time"
 
@@ -17,8 +18,8 @@ import (
 )
 
 var defaultOptions = Options{
-	MaxRetryAttempts: 10,
-	Timeout:          5 * time.Second,
+	MaxRetryAttempts: 5,
+	Timeout:          3 * time.Second,
 }
 
 // http provider uses an RPC client to obtain the necessary information.
@@ -146,29 +147,26 @@ func (p *http) validatorSet(ctx context.Context, height *int64) (*types.Validato
 					}
 				}
 
-			case *rpctypes.RPCError:
-				// Check if we got something other than internal error. This shouldn't happen unless the RPC module
-				// or light client has been tampered with. If we do get this error, stop the connection with the
-				// peer and return an error
-				if e.Code != -32603 {
-					return nil, provider.ErrBadLightBlock{Reason: errors.New(e.Data)}
+			case *url.Error:
+				if e.Timeout() {
+					// request timed out: we wait and try again with exponential backoff
+					time.Sleep(backoffTimeout(uint16(attempt)))
+					continue
 				}
+				return nil, provider.ErrBadLightBlock{Reason: e}
 
+			case *rpctypes.RPCError:
 				// check if the error indicates that the peer doesn't have the block
 				if strings.Contains(e.Data, ctypes.ErrHeightNotAvailable.Error()) ||
 					strings.Contains(e.Data, ctypes.ErrHeightExceedsChainHead.Error()) {
 					return nil, provider.ErrLightBlockNotFound
 				}
-
-				// we wait and try again with exponential backoff
-				// TODO: If we can, we should check if the error is purely because the node failed to respond in
-				// time. If this is the case then we continue, else we should stop straight away
-				time.Sleep(backoffTimeout(uint16(attempt)))
-				continue
+				return nil, provider.ErrBadLightBlock{Reason: e}
 
 			default:
-				// something has happened to the RPC module if we are not receiving errors of type RPCError
-				panic(fmt.Errorf("unexpected error type: %w", err))
+				// If we don't know the error then by default we return a bad light block error and
+				// terminate the connection with the peer.
+				return nil, provider.ErrBadLightBlock{Reason: e}
 			}
 
 			// update the total and increment the page index so we can fetch the
@@ -196,6 +194,14 @@ func (p *http) signedHeader(ctx context.Context, height *int64) (*types.SignedHe
 		case nil: // success!!
 			return &commit.SignedHeader, nil
 
+		case *url.Error:
+			if e.Timeout() {
+				// we wait and try again with exponential backoff
+				time.Sleep(backoffTimeout(uint16(attempt)))
+				continue
+			}
+			return nil, provider.ErrBadLightBlock{Reason: e}
+
 		case *rpctypes.RPCError:
 			// Check if we got something other than internal error. This shouldn't happen unless the RPC module
 			// or light client has been tampered with. If we do get this error, stop the connection with the
@@ -210,15 +216,10 @@ func (p *http) signedHeader(ctx context.Context, height *int64) (*types.SignedHe
 				return nil, provider.ErrLightBlockNotFound
 			}
 
-			// we wait and try again with exponential backoff
-			// TODO: If we can, we should check if the error is purely because the node failed to respond in
-			// time. If this is the case then we continue, else we should stop straight away
-			time.Sleep(backoffTimeout(uint16(attempt)))
-			continue
-
 		default:
-			// something has happened to the RPC module if we are not receiving errors of type RPCError
-			panic(fmt.Errorf("unexpected error type: %w", err))
+			// If we don't know the error then by default we return a bad light block error and
+			// terminate the connection with the peer.
+			return nil, provider.ErrBadLightBlock{Reason: e}
 		}
 	}
 	return nil, provider.ErrNoResponse

light/rpc/client.go

@@ -507,7 +507,7 @@ func (c *Client) updateLightClientIfNeededTo(ctx context.Context, height *int64)
 		l, err = c.lc.VerifyLightBlockAtHeight(ctx, *height, time.Now())
 	}
 	if err != nil {
-		return nil, fmt.Errorf("failed to update light client to %d: %w", height, err)
+		return nil, fmt.Errorf("failed to update light client: %w", err)
 	}
 	return l, nil
 }

rpc/jsonrpc/client/http_json_client.go

@@ -183,11 +183,8 @@ func (c *Client) Call(
 	}
 
 	httpResponse, err := c.client.Do(httpRequest)
-	if e, ok := err.(*url.Error); ok && e.Timeout() {
-		panic("Hello world")
-	}
 	if err != nil {
-		return nil, fmt.Errorf("request failed: %w", err)
+		return nil, err
 	}
 
 	defer httpResponse.Body.Close()