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

Closes #5112

privval/errors.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 )
 
+// EndpointTimeoutError occurs when endpoint times out.
 type EndpointTimeoutError struct{}
 
 // Implement the net.Error interface.
@@ -14,15 +15,15 @@ func (e EndpointTimeoutError) Temporary() bool { return true }
 
 // Socket errors.
 var (
-	ErrUnexpectedResponse = errors.New("received unexpected response")
-	ErrNoConnection       = errors.New("endpoint is not connected")
 	ErrConnectionTimeout  = EndpointTimeoutError{}
-
-	ErrReadTimeout  = errors.New("endpoint read timed out")
-	ErrWriteTimeout = errors.New("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

@@ -54,6 +54,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)
@@ -66,6 +70,10 @@ func (sc *RetrySignerClient) SignVote(chainID string, vote *tmproto.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)
@@ -78,6 +86,10 @@ func (sc *RetrySignerClient) SignProposal(chainID string, proposal *tmproto.Prop
 		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 (
-	"errors"
 	"fmt"
 	"time"
 
@@ -53,7 +52,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(mustWrapMsg(&privvalproto.PingRequest{}))
-
 	if err != nil {
 		sc.endpoint.Logger.Error("SignerClient::Ping", "err", err)
 		return nil
@@ -61,7 +59,6 @@ func (sc *SignerClient) Ping() error {
 
 	pb := response.GetPingResponse()
 	if pb == nil {
-		sc.endpoint.Logger.Error("SignerClient::Ping", "err", "response != PingResponse")
 		return err
 	}
 
@@ -73,22 +70,18 @@ func (sc *SignerClient) Ping() error {
 func (sc *SignerClient) GetPubKey() (crypto.PubKey, error) {
 	response, err := sc.endpoint.SendRequest(mustWrapMsg(&privvalproto.PubKeyRequest{}))
 	if err != nil {
-		sc.endpoint.Logger.Error("SignerClient::GetPubKey", "err", err)
 		return nil, fmt.Errorf("send: %w", err)
 	}
 
-	pubKeyResp := response.GetPubKeyResponse()
-	if pubKeyResp == nil {
-		sc.endpoint.Logger.Error("SignerClient::GetPubKey", "err", "response != PubKeyResponse")
-		return nil, fmt.Errorf("unexpected response type %T", response)
+	resp := response.GetPubKeyResponse()
+	if resp == nil {
+		return nil, ErrUnexpectedResponse
+	}
+	if resp.Error != nil {
+		return nil, &RemoteSignerError{Code: int(resp.Error.Code), Description: resp.Error.Description}
 	}
 
-	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", errors.New(pubKeyResp.Error.Description))
-	}
-
-	pk, err := cryptoenc.PubKeyFromProto(*pubKeyResp.PubKey)
+	pk, err := cryptoenc.PubKeyFromProto(*resp.PubKey)
 	if err != nil {
 		return nil, err
 	}
@@ -98,19 +91,15 @@ func (sc *SignerClient) GetPubKey() (crypto.PubKey, error) {
 
 // SignVote requests a remote signer to sign a vote
 func (sc *SignerClient) SignVote(chainID string, vote *tmproto.Vote) error {
-
 	response, err := sc.endpoint.SendRequest(mustWrapMsg(&privvalproto.SignVoteRequest{Vote: vote}))
 	if err != nil {
-		sc.endpoint.Logger.Error("SignerClient::SignVote", "err", err)
 		return err
 	}
 
 	resp := response.GetSignedVoteResponse()
 	if resp == nil {
-		sc.endpoint.Logger.Error("SignerClient::GetPubKey", "err", "response != SignedVoteResponse")
 		return ErrUnexpectedResponse
 	}
-
 	if resp.Error != nil {
 		return &RemoteSignerError{Code: int(resp.Error.Code), Description: resp.Error.Description}
 	}
@@ -122,16 +111,13 @@ func (sc *SignerClient) SignVote(chainID string, vote *tmproto.Vote) error {
 
 // SignProposal requests a remote signer to sign a proposal
 func (sc *SignerClient) SignProposal(chainID string, proposal *tmproto.Proposal) error {
-
 	response, err := sc.endpoint.SendRequest(mustWrapMsg(&privvalproto.SignProposalRequest{Proposal: *proposal}))
 	if err != nil {
-		sc.endpoint.Logger.Error("SignerClient::SignProposal", "err", err)
 		return err
 	}
 
 	resp := response.GetSignedProposalResponse()
 	if resp == nil {
-		sc.endpoint.Logger.Error("SignerClient::SignProposal", "err", "response != SignedProposalResponse")
 		return ErrUnexpectedResponse
 	}
 	if resp.Error != nil {

privval/signer_client_test.go

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