Skip to content

Commit

Permalink
privval: if remote signer errors, don't retry
Browse files Browse the repository at this point in the history
Refs #5112
  • Loading branch information
melekes authored and tessr committed Aug 5, 2020
1 parent f11eae0 commit 9da308f
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 15 deletions.
14 changes: 8 additions & 6 deletions privval/errors.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package privval

import (
"errors"
"fmt"
)

// EndpointTimeoutError occurs when endpoint times out.
type EndpointTimeoutError struct{}

// Implement the net.Error interface.
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions privval/retry_signer_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down
7 changes: 0 additions & 7 deletions privval/signer_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,13 @@ 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
}

_, ok := response.(*PingResponse)
if !ok {
sc.endpoint.Logger.Error("SignerClient::Ping", "err", "response != PingResponse")
return err
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions privval/signer_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

0 comments on commit 9da308f

Please sign in to comment.