privval: retry GetPubKey/SignVote/SignProposal N times before

returning an error

Closes #4707

CHANGELOG.md

@@ -42,7 +42,6 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 ### BUG FIXES:
 
 - [rpc] [\#4568](https://github.com/tendermint/tendermint/issues/4568) Fix panic when `Subscribe` is called, but HTTP client is not running. `Subscribe`, `Unsubscribe(All)` methods return an error now (@melekes).
-  
 
 ## v0.33.3
 
@@ -339,6 +338,12 @@ subjectivity interface. Refer to the [spec](https://github.com/tendermint/spec/b
 - [consensus/types] [\#4243](https://github.com/tendermint/tendermint/issues/4243) fix BenchmarkRoundStateDeepCopy panics (@cuonglm)
 - [rpc] [\#4256](https://github.com/tendermint/tendermint/issues/4256) Pass `outCapacity` to `eventBus#Subscribe` when subscribing using a local client
 
+## v0.32.11
+
+### BUG FIXES:
+
+- [privval] [\#4275](https://github.com/tendermint/tendermint/issues/4275) Fix consensus failure when remote signer drops (@melekes)
+
 ## v0.32.10
 
 *April 6, 2020*
@@ -421,7 +426,7 @@ program](https://hackerone.com/tendermint).
 
 ### BUG FIXES:
 
-- [rpc/lib] [\#4051](https://github.com/tendermint/tendermint/pull/4131) Fix RPC client, which was previously resolving https protocol to http (@yenkhoon)
+- [rpc/lib] [\#4131](https://github.com/tendermint/tendermint/pull/4131) Fix RPC client, which was previously resolving https protocol to http (@yenkhoon)
 - [cs] [\#4069](https://github.com/tendermint/tendermint/issues/4069) Don't panic when block meta is not found in store (@gregzaitsev)
 
 ## v0.32.8

CHANGELOG_PENDING.md

@@ -43,3 +43,4 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 - [light] [\#4741](https://github.com/tendermint/tendermint/pull/4741) Correctly return  `ErrSignedHeaderNotFound` and `ErrValidatorSetNotFound` on corresponding RPC errors (@erikgrinaker)
 - [rpc] \#4805 Attempt to handle panics during panic recovery (@erikgrinaker)
 - [types] [\#4764](https://github.com/tendermint/tendermint/pull/4764) Return an error if voting power overflows in `VerifyCommitTrusting` (@melekes)
+- [privval] [\#4812](https://github.com/tendermint/tendermint/pull/4812) Retry `GetPubKey/SignVote/SignProposal` a few times before returning an error (@melekes)

node/node.go

@@ -1194,15 +1194,27 @@ func createAndStartPrivValidatorSocketClient(
 ) (types.PrivValidator, error) {
 	pve, err := privval.NewSignerListener(listenAddr, logger)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to start private validator")
+		return nil, fmt.Errorf("failed to start private validator: %w", err)
 	}
 
 	pvsc, err := privval.NewSignerClient(pve)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to start private validator")
+		return nil, fmt.Errorf("failed to start private validator: %w", err)
 	}
 
-	return pvsc, nil
+	// try to get a pubkey from private validate first time
+	_, err = pvsc.GetPubKey()
+	if err != nil {
+		return nil, fmt.Errorf("can't get pubkey: %w", err)
+	}
+
+	const (
+		retries = 50 // 50 * 100ms = 5s total
+		timeout = 100 * time.Millisecond
+	)
+	pvscWithRetries := privval.NewRetrySignerClient(pvsc, retries, timeout)
+
+	return pvscWithRetries, nil
 }
 
 // splitAndTrimEmpty slices s into all subslices separated by sep and returns a

node/node_test.go

@@ -159,7 +159,7 @@ func TestNodeSetPrivValTCP(t *testing.T) {
 
 	n, err := DefaultNewNode(config, log.TestingLogger())
 	require.NoError(t, err)
-	assert.IsType(t, &privval.SignerClient{}, n.PrivValidator())
+	assert.IsType(t, &privval.RetrySignerClient{}, n.PrivValidator())
 }
 
 // address without a protocol must result in error
@@ -203,7 +203,7 @@ func TestNodeSetPrivValIPC(t *testing.T) {
 
 	n, err := DefaultNewNode(config, log.TestingLogger())
 	require.NoError(t, err)
-	assert.IsType(t, &privval.SignerClient{}, n.PrivValidator())
+	assert.IsType(t, &privval.RetrySignerClient{}, n.PrivValidator())
 }
 
 // testFreeAddr claims a free port so we don't block on listener being ready.

privval/doc.go

@@ -19,5 +19,11 @@ SignerDialerEndpoint
 
 SignerDialerEndpoint is a simple wrapper around a net.Conn. It's used by both IPCVal and TCPVal.
 
+SignerClient
+
+SignerClient handles remote validator connections that provide signing services.
+In production, it's recommended to wrap it with RetrySignerClient to avoid
+termination in case of temporary errors.
+
 */
 package privval

privval/retry_signer_client.go

@@ -0,0 +1,83 @@
+package privval
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/tendermint/tendermint/crypto"
+	"github.com/tendermint/tendermint/types"
+)
+
+// RetrySignerClient wraps SignerClient adding retry for each operation (except
+// Ping) w/ a timeout.
+type RetrySignerClient struct {
+	next    *SignerClient
+	retries int
+	timeout time.Duration
+}
+
+// NewRetrySignerClient returns RetrySignerClient. If +retries+ is 0, the
+// client will be retrying each operation indefinitely.
+func NewRetrySignerClient(sc *SignerClient, retries int, timeout time.Duration) *RetrySignerClient {
+	return &RetrySignerClient{sc, retries, timeout}
+}
+
+var _ types.PrivValidator = (*RetrySignerClient)(nil)
+
+func (sc *RetrySignerClient) Close() error {
+	return sc.next.Close()
+}
+
+func (sc *RetrySignerClient) IsConnected() bool {
+	return sc.next.IsConnected()
+}
+
+func (sc *RetrySignerClient) WaitForConnection(maxWait time.Duration) error {
+	return sc.next.WaitForConnection(maxWait)
+}
+
+//--------------------------------------------------------
+// Implement PrivValidator
+
+func (sc *RetrySignerClient) Ping() error {
+	return sc.next.Ping()
+}
+
+func (sc *RetrySignerClient) GetPubKey() (crypto.PubKey, error) {
+	var (
+		pk  crypto.PubKey
+		err error
+	)
+	for i := 0; i < sc.retries || sc.retries == 0; i++ {
+		pk, err = sc.next.GetPubKey()
+		if err == nil {
+			return pk, nil
+		}
+		time.Sleep(sc.timeout)
+	}
+	return nil, fmt.Errorf("exhausted all attempts to get pubkey: %w", err)
+}
+
+func (sc *RetrySignerClient) SignVote(chainID string, vote *types.Vote) error {
+	var err error
+	for i := 0; i < sc.retries || sc.retries == 0; i++ {
+		err = sc.next.SignVote(chainID, vote)
+		if err == nil {
+			return nil
+		}
+		time.Sleep(sc.timeout)
+	}
+	return fmt.Errorf("exhausted all attempts to sign vote: %w", err)
+}
+
+func (sc *RetrySignerClient) SignProposal(chainID string, proposal *types.Proposal) error {
+	var err error
+	for i := 0; i < sc.retries || sc.retries == 0; i++ {
+		err = sc.next.SignProposal(chainID, proposal)
+		if err == nil {
+			return nil
+		}
+		time.Sleep(sc.timeout)
+	}
+	return fmt.Errorf("exhausted all attempts to sign proposal: %w", err)
+}