privval: if remote signer errors, don't retry

Refs #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

@@ -60,6 +60,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 errors.New("exhausted all attempts to sign vote")
@@ -71,6 +75,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 errors.New("exhausted all attempts to sign proposal")

privval/signer_client.go

@@ -50,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
@@ -58,7 +57,6 @@ func (sc *SignerClient) Ping() error {
 
 	_, ok := response.(*PingResponse)
 	if !ok {
-		sc.endpoint.Logger.Error("SignerClient::Ping", "err", "response != PingResponse")
 		return err
 	}
 
@@ -91,16 +89,13 @@ func (sc *SignerClient) GetPubKey() crypto.PubKey {
 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
 	}
@@ -113,13 +108,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

@@ -250,8 +250,7 @@ func TestSignerUnexpectedResponse(t *testing.T) {
 
 		ts := time.Now()
 		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")
 	}
 }