Consolidates deadline tests for privval Unix/TCP (#3143)

* Consolidates deadline tests for privval Unix/TCP

Following on from #3115 and #3132, this converts fundamental timeout
errors from the client's `acceptConnection()` method so that these can
be detected by the test for the TCP connection.

Timeout deadlines are now tested for both TCP and Unix domain socket
connections.

There is also no need for the additional secret connection code: the
connection will time out at the `acceptConnection()` phase for TCP
connections, and will time out when attempting to obtain the
`RemoteSigner`'s public key for Unix domain socket connections.

* Removes extraneous logging

* Adds IsConnTimeout helper function

This commit adds a helper function to detect whether an error is either
a fundamental networking timeout error, or an `ErrConnTimeout` error
specific to the `RemoteSigner` class.

* Adds a test for the IsConnTimeout() helper function

* Separates tests logically for IsConnTimeout

privval/client_test.go

@@ -13,7 +13,6 @@ import (
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/libs/log"
 
-	p2pconn "github.com/tendermint/tendermint/p2p/conn"
 	"github.com/tendermint/tendermint/types"
 )
 
@@ -186,40 +185,36 @@ func TestSocketPVVoteKeepalive(t *testing.T) {
 	}
 }
 
-// TestSocketPVDeadlineTCPOnly is not relevant to Unix domain sockets, since the
-// OS knows instantaneously the state of both sides of the connection.
-func TestSocketPVDeadlineTCPOnly(t *testing.T) {
-	var (
-		addr            = testFreeTCPAddr(t)
-		listenc         = make(chan struct{})
-		thisConnTimeout = 100 * time.Millisecond
-		sc              = newSocketVal(log.TestingLogger(), addr, thisConnTimeout)
-	)
+func TestSocketPVDeadline(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				listenc         = make(chan struct{})
+				thisConnTimeout = 100 * time.Millisecond
+				sc              = newSocketVal(log.TestingLogger(), tc.addr, thisConnTimeout)
+			)
 
-	go func(sc *SocketVal) {
-		defer close(listenc)
+			go func(sc *SocketVal) {
+				defer close(listenc)
 
-		assert.Equal(t, sc.Start().(cmn.Error).Data(), ErrConnTimeout)
+				// Note: the TCP connection times out at the accept() phase,
+				// whereas the Unix domain sockets connection times out while
+				// attempting to fetch the remote signer's public key.
+				assert.True(t, IsConnTimeout(sc.Start()))
 
-		assert.False(t, sc.IsRunning())
-	}(sc)
+				assert.False(t, sc.IsRunning())
+			}(sc)
 
-	for {
-		conn, err := cmn.Connect(addr)
-		if err != nil {
-			continue
-		}
+			for {
+				_, err := cmn.Connect(tc.addr)
+				if err == nil {
+					break
+				}
+			}
 
-		_, err = p2pconn.MakeSecretConnection(
-			conn,
-			ed25519.GenPrivKey(),
-		)
-		if err == nil {
-			break
-		}
+			<-listenc
+		}()
 	}
-
-	<-listenc
 }
 
 func TestRemoteSignVoteErrors(t *testing.T) {

privval/remote_signer.go

@@ -258,3 +258,18 @@ func handleRequest(req RemoteSignerMsg, chainID string, privVal types.PrivValida
 
 	return res, err
 }
+
+// IsConnTimeout returns a boolean indicating whether the error is known to
+// report that a connection timeout occurred. This detects both fundamental
+// network timeouts, as well as ErrConnTimeout errors.
+func IsConnTimeout(err error) bool {
+	if cmnErr, ok := err.(cmn.Error); ok {
+		if cmnErr.Data() == ErrConnTimeout {
+			return true
+		}
+	}
+	if _, ok := err.(timeoutError); ok {
+		return true
+	}
+	return false
+}

privval/remote_signer_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/tendermint/tendermint/crypto/ed25519"
@@ -66,3 +67,24 @@ func TestRemoteSignerRetryTCPOnly(t *testing.T) {
 		t.Error("expected remote to observe connection attempts")
 	}
 }
+
+func TestIsConnTimeoutForFundamentalTimeouts(t *testing.T) {
+	// Generate a networking timeout
+	dialer := DialTCPFn(testFreeTCPAddr(t), time.Millisecond, ed25519.GenPrivKey())
+	_, err := dialer()
+	assert.Error(t, err)
+	assert.True(t, IsConnTimeout(err))
+}
+
+func TestIsConnTimeoutForWrappedConnTimeouts(t *testing.T) {
+	dialer := DialTCPFn(testFreeTCPAddr(t), time.Millisecond, ed25519.GenPrivKey())
+	_, err := dialer()
+	assert.Error(t, err)
+	err = cmn.ErrorWrap(ErrConnTimeout, err.Error())
+	assert.True(t, IsConnTimeout(err))
+}
+
+func TestIsConnTimeoutForNonTimeoutErrors(t *testing.T) {
+	assert.False(t, IsConnTimeout(cmn.ErrorWrap(ErrDialRetryMax, "max retries exceeded")))
+	assert.False(t, IsConnTimeout(errors.New("completely irrelevant error")))
+}