privval: if remote signer errors, don't retry (#5140)

Closes #5112

privval/errors.go

@@ -1,9 +1,11 @@
 package privval
 
 import (
+	"errors"
 	"fmt"
 )
 
+// EndpointTimeoutError occurs when endpoint times out.
 type EndpointTimeoutError struct{}
 
 // Implement the net.Error interface.
@@ -13,15 +15,15 @@ func (e EndpointTimeoutError) Temporary() bool { return true }
 
 // Socket errors.
 var (
-	ErrUnexpectedResponse = fmt.Errorf("received unexpected response")
-	ErrNoConnection       = fmt.Errorf("endpoint is not connected")
 	ErrConnectionTimeout  = EndpointTimeoutError{}
-
-	ErrReadTimeout  = fmt.Errorf("endpoint read timed out")
-	ErrWriteTimeout = fmt.Errorf("endpoint write timed out")
+	ErrNoConnection       = errors.New("endpoint is not connected")
+	ErrReadTimeout        = errors.New("endpoint read timed out")
+	ErrUnexpectedResponse = errors.New("empty response")
+	ErrWriteTimeout       = errors.New("endpoint write timed out")
 )
 
-// RemoteSignerError allows (remote) validators to include meaningful error descriptions in their reply.
+// RemoteSignerError allows (remote) validators to include meaningful error
+// descriptions in their reply.
 type RemoteSignerError struct {
 	// TODO(ismail): create an enum of known errors
 	Code        int

privval/retry_signer_client.go

@@ -53,6 +53,10 @@ func (sc *RetrySignerClient) GetPubKey() (crypto.PubKey, error) {
 		if err == nil {
 			return pk, nil
 		}
+		// If remote signer errors, we don't retry.
+		if _, ok := err.(*RemoteSignerError); ok {
+			return nil, err
+		}
 		time.Sleep(sc.timeout)
 	}
 	return nil, fmt.Errorf("exhausted all attempts to get pubkey: %w", err)
@@ -65,6 +69,10 @@ func (sc *RetrySignerClient) SignVote(chainID string, vote *types.Vote) error {
 		if err == nil {
 			return nil
 		}
+		// If remote signer errors, we don't retry.
+		if _, ok := err.(*RemoteSignerError); ok {
+			return err
+		}
 		time.Sleep(sc.timeout)
 	}
 	return fmt.Errorf("exhausted all attempts to sign vote: %w", err)
@@ -77,6 +85,10 @@ func (sc *RetrySignerClient) SignProposal(chainID string, proposal *types.Propos
 		if err == nil {
 			return nil
 		}
+		// If remote signer errors, we don't retry.
+		if _, ok := err.(*RemoteSignerError); ok {
+			return err
+		}
 		time.Sleep(sc.timeout)
 	}
 	return fmt.Errorf("exhausted all attempts to sign proposal: %w", err)

privval/signer_client.go

@@ -1,7 +1,6 @@
 package privval
 
 import (
-	"fmt"
 	"time"
 
 	"github.com/pkg/errors"
@@ -51,7 +50,6 @@ func (sc *SignerClient) WaitForConnection(maxWait time.Duration) error {
 // Ping sends a ping request to the remote signer
 func (sc *SignerClient) Ping() error {
 	response, err := sc.endpoint.SendRequest(&PingRequest{})
-
 	if err != nil {
 		sc.endpoint.Logger.Error("SignerClient::Ping", "err", err)
 		return nil
@@ -59,8 +57,7 @@ func (sc *SignerClient) Ping() error {
 
 	_, ok := response.(*PingResponse)
 	if !ok {
-		sc.endpoint.Logger.Error("SignerClient::Ping", "err", "response != PingResponse")
-		return err
+		return ErrUnexpectedResponse
 	}
 
 	return nil
@@ -71,19 +68,16 @@ func (sc *SignerClient) Ping() error {
 func (sc *SignerClient) GetPubKey() (crypto.PubKey, error) {
 	response, err := sc.endpoint.SendRequest(&PubKeyRequest{})
 	if err != nil {
-		sc.endpoint.Logger.Error("SignerClient::GetPubKey", "err", err)
-		return nil, errors.Wrap(err, "send")
+		return nil, err
 	}
 
 	pubKeyResp, ok := response.(*PubKeyResponse)
 	if !ok {
-		sc.endpoint.Logger.Error("SignerClient::GetPubKey", "err", "response != PubKeyResponse")
-		return nil, errors.Errorf("unexpected response type %T", response)
+		return nil, ErrUnexpectedResponse
 	}
 
 	if pubKeyResp.Error != nil {
-		sc.endpoint.Logger.Error("failed to get private validator's public key", "err", pubKeyResp.Error)
-		return nil, fmt.Errorf("remote error: %w", pubKeyResp.Error)
+		return nil, pubKeyResp.Error
 	}
 
 	return pubKeyResp.PubKey, nil
@@ -93,16 +87,13 @@ func (sc *SignerClient) GetPubKey() (crypto.PubKey, error) {
 func (sc *SignerClient) SignVote(chainID string, vote *types.Vote) error {
 	response, err := sc.endpoint.SendRequest(&SignVoteRequest{Vote: vote})
 	if err != nil {
-		sc.endpoint.Logger.Error("SignerClient::SignVote", "err", err)
 		return err
 	}
 
 	resp, ok := response.(*SignedVoteResponse)
 	if !ok {
-		sc.endpoint.Logger.Error("SignerClient::GetPubKey", "err", "response != SignedVoteResponse")
 		return ErrUnexpectedResponse
 	}
-
 	if resp.Error != nil {
 		return resp.Error
 	}
@@ -115,13 +106,11 @@ func (sc *SignerClient) SignVote(chainID string, vote *types.Vote) error {
 func (sc *SignerClient) SignProposal(chainID string, proposal *types.Proposal) error {
 	response, err := sc.endpoint.SendRequest(&SignProposalRequest{Proposal: proposal})
 	if err != nil {
-		sc.endpoint.Logger.Error("SignerClient::SignProposal", "err", err)
 		return err
 	}
 
 	resp, ok := response.(*SignedProposalResponse)
 	if !ok {
-		sc.endpoint.Logger.Error("SignerClient::SignProposal", "err", "response != SignedProposalResponse")
 		return ErrUnexpectedResponse
 	}
 	if resp.Error != nil {

privval/signer_client_test.go

@@ -257,6 +257,6 @@ func TestSignerUnexpectedResponse(t *testing.T) {
 		want := &types.Vote{Timestamp: ts, Type: types.PrecommitType}
 
 		e := tc.signerClient.SignVote(tc.chainID, want)
-		assert.EqualError(t, e, "received unexpected response")
+		assert.EqualError(t, e, "empty response")
 	}
 }