From 8026fd3cc90451474562548430140241591a8904 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 22 Jul 2020 10:12:37 +0400 Subject: [PATCH 1/2] privval: if remote signer errors, don't retry (#5140) Closes #5112 --- privval/errors.go | 14 ++++++++------ privval/retry_signer_client.go | 12 ++++++++++++ privval/signer_client.go | 19 ++++--------------- privval/signer_client_test.go | 2 +- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/privval/errors.go b/privval/errors.go index 9f151f11d..297d5dca2 100644 --- a/privval/errors.go +++ b/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 diff --git a/privval/retry_signer_client.go b/privval/retry_signer_client.go index d2cde8741..d1a2409c6 100644 --- a/privval/retry_signer_client.go +++ b/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) diff --git a/privval/signer_client.go b/privval/signer_client.go index 3e69c6c08..2003c3f68 100644 --- a/privval/signer_client.go +++ b/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 { diff --git a/privval/signer_client_test.go b/privval/signer_client_test.go index 2156a8cf2..975628a90 100644 --- a/privval/signer_client_test.go +++ b/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") } } From 1b1fe4d7cc7124013d2be9b7f3e5c5db25511b01 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 4 Aug 2020 12:20:35 +0200 Subject: [PATCH 2/2] prepare 0.33.7 release (#5202) --- CHANGELOG.md | 9 +++++++++ CHANGELOG_PENDING.md | 2 +- version/version.go | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdb3e1c6d..3184eb8bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## v0.33.7 + +*August 4, 2020* + +### BUG FIXES: + +- [go] Build release binary using Go 1.14.4, to avoid halt caused by Go 1.14.1 (https://github.com/golang/go/issues/38223) +- [privval] [\#5140](https://github.com/tendermint/tendermint/pull/5140) `RemoteSignerError` from remote signers are no longer retried (@melekes) + ## v0.33.6 *July 2, 2020* diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index ca710ba8c..fd11672d7 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,4 +1,4 @@ -## v0.33.7 +## v0.33.8 \*\* diff --git a/version/version.go b/version/version.go index 06012e15f..f394a2446 100644 --- a/version/version.go +++ b/version/version.go @@ -21,7 +21,7 @@ const ( // XXX: Don't change the name of this variable or you will break // automation :) - TMCoreSemVer = "0.33.6" + TMCoreSemVer = "0.33.7" // ABCISemVer is the semantic version of the ABCI library ABCISemVer = "0.16.2"