Merge pull request #3138 from tendermint/master

Merge master back to develop

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## v0.28.0
 
-*January 14th, 2019*
+*January 16th, 2019*
 
 Special thanks to external contributors on this release:
 @fmauricios, @gianfelipe93, @husio, @needkane, @srmo, @yutianwu
@@ -11,23 +11,25 @@ This release is primarily about upgrades to the `privval` system -
 separating the `priv_validator.json` into distinct config and data files, and
 refactoring the socket validator to support reconnections.
 
+**Note:** Please backup your existing `priv_validator.json` before using this
+version.
+
 See [UPGRADING.md](UPGRADING.md) for more details.
 
 ### BREAKING CHANGES:
 
 * CLI/RPC/Config
-- [cli] Removed `node` `--proxy_app=dummy` option. Use `kvstore` (`persistent_kvstore`) instead.
-- [cli] Renamed `node` `--proxy_app=nilapp` to `--proxy_app=noop`.
-- [config] [\#2992](https://github.com/tendermint/tendermint/issues/2992) `allow_duplicate_ip` is now set to false
-- [privval] [\#1181](https://github.com/tendermint/tendermint/issues/1181) Split immutable and mutable parts of `priv_validator.json`
-  (@yutianwu)
-- [privval] [\#2926](https://github.com/tendermint/tendermint/issues/2926) Split up `PubKeyMsg` into `PubKeyRequest` and `PubKeyResponse` to be consistent with other message types
-- [privval] [\#2923](https://github.com/tendermint/tendermint/issues/2923) Listen for unix socket connections instead of dialing them
+  - [cli] Removed `--proxy_app=dummy` option. Use `kvstore` (`persistent_kvstore`) instead.
+  - [cli] Renamed `--proxy_app=nilapp` to `--proxy_app=noop`.
+  - [config] [\#2992](https://github.com/tendermint/tendermint/issues/2992) `allow_duplicate_ip` is now set to false
+  - [privval] [\#1181](https://github.com/tendermint/tendermint/issues/1181) Split `priv_validator.json` into immutable (`config/priv_validator_key.json`) and mutable (`data/priv_validator_state.json`) parts (@yutianwu)
+  - [privval] [\#2926](https://github.com/tendermint/tendermint/issues/2926) Split up `PubKeyMsg` into `PubKeyRequest` and `PubKeyResponse` to be consistent with other message types
+  - [privval] [\#2923](https://github.com/tendermint/tendermint/issues/2923) Listen for unix socket connections instead of dialing them
 
 * Apps
 
 * Go API
-- [types] [\#2981](https://github.com/tendermint/tendermint/issues/2981) Remove `PrivValidator.GetAddress()`
+  - [types] [\#2981](https://github.com/tendermint/tendermint/issues/2981) Remove `PrivValidator.GetAddress()`
 
 * Blockchain Protocol
 
@@ -38,20 +40,20 @@ See [UPGRADING.md](UPGRADING.md) for more details.
 
 ### IMPROVEMENTS:
 - [consensus] [\#3086](https://github.com/tendermint/tendermint/issues/3086) Log peerID on ignored votes (@srmo)
-- [docs] [\#3061](https://github.com/tendermint/tendermint/issues/3061) Added spec on signing consensus msgs at
+- [docs] [\#3061](https://github.com/tendermint/tendermint/issues/3061) Added specification for signing consensus msgs at
   ./docs/spec/consensus/signing.md
 - [privval] [\#2948](https://github.com/tendermint/tendermint/issues/2948) Memoize pubkey so it's only requested once on startup
 - [privval] [\#2923](https://github.com/tendermint/tendermint/issues/2923) Retry RemoteSigner connections on error
 
 ### BUG FIXES:
 
-- [types] [\#2926](https://github.com/tendermint/tendermint/issues/2926) Do not panic if retrieving the private validator's public key fails
-- [rpc] [\#3053](https://github.com/tendermint/tendermint/issues/3053) Fix internal error in `/tx_search` when results are empty
-  (@gianfelipe93)
+- [build] [\#3085](https://github.com/tendermint/tendermint/issues/3085) Fix `Version` field in build scripts (@husio)
 - [crypto/multisig] [\#3102](https://github.com/tendermint/tendermint/issues/3102) Fix multisig keys address length
 - [crypto/encoding] [\#3101](https://github.com/tendermint/tendermint/issues/3101) Fix `PubKeyMultisigThreshold` unmarshalling into `crypto.PubKey` interface
-- [build] [\#3085](https://github.com/tendermint/tendermint/issues/3085) Fix `Version` field in build scripts (@husio)
 - [p2p/conn] [\#3111](https://github.com/tendermint/tendermint/issues/3111) Make SecretConnection thread safe
+- [rpc] [\#3053](https://github.com/tendermint/tendermint/issues/3053) Fix internal error in `/tx_search` when results are empty
+  (@gianfelipe93)
+- [types] [\#2926](https://github.com/tendermint/tendermint/issues/2926) Do not panic if retrieving the privval's public key fails
 
 ## v0.27.4
 
@@ -70,9 +72,8 @@ See [UPGRADING.md](UPGRADING.md) for more details.
 ### BREAKING CHANGES:
 
 * Go API
-
-- [dep] [\#3027](https://github.com/tendermint/tendermint/issues/3027) Revert to mainline Go crypto library, eliminating the modified
-  `bcrypt.GenerateFromPassword`
+  - [dep] [\#3027](https://github.com/tendermint/tendermint/issues/3027) Revert to mainline Go crypto library, eliminating the modified
+    `bcrypt.GenerateFromPassword`
 
 ## v0.27.2
 

UPGRADING.md

@@ -7,12 +7,12 @@ a newer version of Tendermint Core.
 
 This release breaks the format for the `priv_validator.json` file
 and the protocol used for the external validator process.
-It is compatible with v0.27.0 blockchains (neither the BlockProtocol or the
+It is compatible with v0.27.0 blockchains (neither the BlockProtocol nor the
 P2PProtocol have changed).
 
 Please read carefully for details about upgrading.
 
-XXX: Backup your `config/priv_validator.json`
+**Note:** Backup your `config/priv_validator.json`
 before proceeding.
 
 ### `priv_validator.json`
@@ -20,7 +20,7 @@ before proceeding.
 The `config/priv_validator.json` is now two files:
 `config/priv_validator_key.json` and `data/priv_validator_state.json`.
 The former contains the key material, the later contains the details on the last
-thing signed.
+message signed.
 
 When running v0.28.0 for the first time, it will back up any pre-existing
 `priv_validator.json` file and proceed to split it into the two new files.
@@ -43,8 +43,8 @@ Thus in both cases, the external process is expected to dial
 Tendermint. This is different from how Unix sockets used to work, where
 Tendermint dialed the external process.
 
-The `PubKeyMsg` was also split into two for consistency with other message
-types.
+The `PubKeyMsg` was also split into separate `Request` and `Response` types
+for consistency with other messages.
 
 Note that the TCP sockets don't yet use a persistent key,
 so while they're encrypted, they can't yet be properly authenticated.
@@ -52,7 +52,6 @@ See [#3105](https://github.com/tendermint/tendermint/issues/3105).
 Note the Unix socket has neither encryption nor authentication, but will
 add a shared-secret in [#3099](https://github.com/tendermint/tendermint/issues/3099).
 
-
 ## v0.27.0
 
 This release contains some breaking changes to the block and p2p protocols,

cmd/priv_val_server/main.go

@@ -45,7 +45,7 @@ func main() {
 		dialer = privval.DialTCPFn(address, connTimeout, ed25519.GenPrivKey())
 	default:
 		logger.Error("Unknown protocol", "protocol", protocol)
-		return
+		os.Exit(1)
 	}
 
 	rs := privval.NewRemoteSigner(logger, *chainID, pv, dialer)

node/node.go

@@ -903,7 +903,7 @@ func createAndStartPrivValidatorSocketClient(
 
 	pvsc := privval.NewSocketVal(logger.With("module", "privval"), listener)
 	if err := pvsc.Start(); err != nil {
-		return nil, errors.Wrap(err, "failed to start")
+		return nil, errors.Wrap(err, "failed to start private validator")
 	}
 
 	return pvsc, nil

privval/client.go

@@ -191,19 +191,19 @@ func (sc *SocketVal) OnStop() {
 // connection is closed in OnStop.
 // returns true if the listener is closed
 // (ie. it returns a nil conn).
-func (sc *SocketVal) reset() (bool, error) {
+func (sc *SocketVal) reset() (closed bool, err error) {
 	sc.mtx.Lock()
 	defer sc.mtx.Unlock()
 
 	// first check if the conn already exists and close it.
 	if sc.signer != nil {
 		if err := sc.signer.Close(); err != nil {
-			sc.Logger.Error("error closing connection", "err", err)
+			sc.Logger.Error("error closing socket val connection during reset", "err", err)
 		}
 	}
 
 	// wait for a new conn
-	conn, err := sc.waitConnection()
+	conn, err := sc.acceptConnection()
 	if err != nil {
 		return false, err
 	}
@@ -224,6 +224,8 @@ func (sc *SocketVal) reset() (bool, error) {
 	return false, nil
 }
 
+// Attempt to accept a connection.
+// Times out after the listener's acceptDeadline
 func (sc *SocketVal) acceptConnection() (net.Conn, error) {
 	conn, err := sc.listener.Accept()
 	if err != nil {
@@ -231,33 +233,6 @@ func (sc *SocketVal) acceptConnection() (net.Conn, error) {
 			return nil, nil // Ignore error from listener closing.
 		}
 		return nil, err
-
 	}
 	return conn, nil
 }
-
-// waitConnection uses the configured wait timeout to error if no external
-// process connects in the time period.
-func (sc *SocketVal) waitConnection() (net.Conn, error) {
-	var (
-		connc = make(chan net.Conn, 1)
-		errc  = make(chan error, 1)
-	)
-
-	go func(connc chan<- net.Conn, errc chan<- error) {
-		conn, err := sc.acceptConnection()
-		if err != nil {
-			errc <- err
-			return
-		}
-
-		connc <- conn
-	}(connc, errc)
-
-	select {
-	case conn := <-connc:
-		return conn, nil
-	case err := <-errc:
-		return nil, err
-	}
-}

privval/client_test.go

@@ -27,120 +27,170 @@ var (
 	testHeartbeatTimeout3o2 = 6 * time.Millisecond // 3/2 of the other one
 )
 
-func TestSocketPVAddress(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-	)
-	defer sc.Stop()
-	defer rs.Stop()
+type socketTestCase struct {
+	addr   string
+	dialer Dialer
+}
 
-	serverAddr := rs.privVal.GetPubKey().Address()
-	clientAddr := sc.GetPubKey().Address()
+func socketTestCases(t *testing.T) []socketTestCase {
+	tcpAddr := fmt.Sprintf("tcp://%s", testFreeTCPAddr(t))
+	unixFilePath, err := testUnixAddr()
+	require.NoError(t, err)
+	unixAddr := fmt.Sprintf("unix://%s", unixFilePath)
+	return []socketTestCase{
+		socketTestCase{
+			addr:   tcpAddr,
+			dialer: DialTCPFn(tcpAddr, testConnDeadline, ed25519.GenPrivKey()),
+		},
+		socketTestCase{
+			addr:   unixAddr,
+			dialer: DialUnixFn(unixFilePath),
+		},
+	}
+}
 
-	assert.Equal(t, serverAddr, clientAddr)
+func TestSocketPVAddress(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		// Execute the test within a closure to ensure the deferred statements
+		// are called between each for loop iteration, for isolated test cases.
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			serverAddr := rs.privVal.GetPubKey().Address()
+			clientAddr := sc.GetPubKey().Address()
+
+			assert.Equal(t, serverAddr, clientAddr)
+		}()
+	}
 }
 
 func TestSocketPVPubKey(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-	)
-	defer sc.Stop()
-	defer rs.Stop()
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+			)
+			defer sc.Stop()
+			defer rs.Stop()
 
-	clientKey := sc.GetPubKey()
+			clientKey := sc.GetPubKey()
 
-	privvalPubKey := rs.privVal.GetPubKey()
+			privvalPubKey := rs.privVal.GetPubKey()
 
-	assert.Equal(t, privvalPubKey, clientKey)
+			assert.Equal(t, privvalPubKey, clientKey)
+		}()
+	}
 }
 
 func TestSocketPVProposal(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-
-		ts             = time.Now()
-		privProposal   = &types.Proposal{Timestamp: ts}
-		clientProposal = &types.Proposal{Timestamp: ts}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	require.NoError(t, rs.privVal.SignProposal(chainID, privProposal))
-	require.NoError(t, sc.SignProposal(chainID, clientProposal))
-	assert.Equal(t, privProposal.Signature, clientProposal.Signature)
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+
+				ts             = time.Now()
+				privProposal   = &types.Proposal{Timestamp: ts}
+				clientProposal = &types.Proposal{Timestamp: ts}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			require.NoError(t, rs.privVal.SignProposal(chainID, privProposal))
+			require.NoError(t, sc.SignProposal(chainID, clientProposal))
+			assert.Equal(t, privProposal.Signature, clientProposal.Signature)
+		}()
+	}
 }
 
 func TestSocketPVVote(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+
+				ts    = time.Now()
+				vType = types.PrecommitType
+				want  = &types.Vote{Timestamp: ts, Type: vType}
+				have  = &types.Vote{Timestamp: ts, Type: vType}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			require.NoError(t, rs.privVal.SignVote(chainID, want))
+			require.NoError(t, sc.SignVote(chainID, have))
+			assert.Equal(t, want.Signature, have.Signature)
+		}()
+	}
 }
 
 func TestSocketPVVoteResetDeadline(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	time.Sleep(testConnDeadline2o3)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
-
-	// This would exceed the deadline if it was not extended by the previous message
-	time.Sleep(testConnDeadline2o3)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+
+				ts    = time.Now()
+				vType = types.PrecommitType
+				want  = &types.Vote{Timestamp: ts, Type: vType}
+				have  = &types.Vote{Timestamp: ts, Type: vType}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			time.Sleep(testConnDeadline2o3)
+
+			require.NoError(t, rs.privVal.SignVote(chainID, want))
+			require.NoError(t, sc.SignVote(chainID, have))
+			assert.Equal(t, want.Signature, have.Signature)
+
+			// This would exceed the deadline if it was not extended by the previous message
+			time.Sleep(testConnDeadline2o3)
+
+			require.NoError(t, rs.privVal.SignVote(chainID, want))
+			require.NoError(t, sc.SignVote(chainID, have))
+			assert.Equal(t, want.Signature, have.Signature)
+		}()
+	}
 }
 
 func TestSocketPVVoteKeepalive(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		want  = &types.Vote{Timestamp: ts, Type: vType}
-		have  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	time.Sleep(testConnDeadline * 2)
-
-	require.NoError(t, rs.privVal.SignVote(chainID, want))
-	require.NoError(t, sc.SignVote(chainID, have))
-	assert.Equal(t, want.Signature, have.Signature)
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewMockPV(), tc.addr, tc.dialer)
+
+				ts    = time.Now()
+				vType = types.PrecommitType
+				want  = &types.Vote{Timestamp: ts, Type: vType}
+				have  = &types.Vote{Timestamp: ts, Type: vType}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			time.Sleep(testConnDeadline * 2)
+
+			require.NoError(t, rs.privVal.SignVote(chainID, want))
+			require.NoError(t, sc.SignVote(chainID, have))
+			assert.Equal(t, want.Signature, have.Signature)
+		}()
+	}
 }
 
-func TestSocketPVDeadline(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            = testFreeAddr(t)
+		addr            = testFreeTCPAddr(t)
 		listenc         = make(chan struct{})
 		thisConnTimeout = 100 * time.Millisecond
 		sc              = newSocketVal(log.TestingLogger(), addr, thisConnTimeout)
@@ -172,218 +222,195 @@ func TestSocketPVDeadline(t *testing.T) {
 	<-listenc
 }
 
-func TestRemoteSignerRetry(t *testing.T) {
-	var (
-		attemptc = make(chan int)
-		retries  = 2
-	)
-
-	ln, err := net.Listen("tcp", "127.0.0.1:0")
-	require.NoError(t, err)
-
-	go func(ln net.Listener, attemptc chan<- int) {
-		attempts := 0
-
-		for {
-			conn, err := ln.Accept()
-			require.NoError(t, err)
-
-			err = conn.Close()
-			require.NoError(t, err)
-
-			attempts++
-
-			if attempts == retries {
-				attemptc <- attempts
-				break
-			}
-		}
-	}(ln, attemptc)
-
-	rs := NewRemoteSigner(
-		log.TestingLogger(),
-		cmn.RandStr(12),
-		types.NewMockPV(),
-		DialTCPFn(ln.Addr().String(), testConnDeadline, ed25519.GenPrivKey()),
-	)
-	defer rs.Stop()
-
-	RemoteSignerConnDeadline(time.Millisecond)(rs)
-	RemoteSignerConnRetries(retries)(rs)
-
-	assert.Equal(t, rs.Start(), ErrDialRetryMax)
-
-	select {
-	case attempts := <-attemptc:
-		assert.Equal(t, retries, attempts)
-	case <-time.After(100 * time.Millisecond):
-		t.Error("expected remote to observe connection attempts")
-	}
-}
-
 func TestRemoteSignVoteErrors(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewErroringMockPV())
-
-		ts    = time.Now()
-		vType = types.PrecommitType
-		vote  = &types.Vote{Timestamp: ts, Type: vType}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	err := sc.SignVote("", vote)
-	require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error())
-
-	err = rs.privVal.SignVote(chainID, vote)
-	require.Error(t, err)
-	err = sc.SignVote(chainID, vote)
-	require.Error(t, err)
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewErroringMockPV(), tc.addr, tc.dialer)
+
+				ts    = time.Now()
+				vType = types.PrecommitType
+				vote  = &types.Vote{Timestamp: ts, Type: vType}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			err := sc.SignVote("", vote)
+			require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error())
+
+			err = rs.privVal.SignVote(chainID, vote)
+			require.Error(t, err)
+			err = sc.SignVote(chainID, vote)
+			require.Error(t, err)
+		}()
+	}
 }
 
 func TestRemoteSignProposalErrors(t *testing.T) {
-	var (
-		chainID = cmn.RandStr(12)
-		sc, rs  = testSetupSocketPair(t, chainID, types.NewErroringMockPV())
-
-		ts       = time.Now()
-		proposal = &types.Proposal{Timestamp: ts}
-	)
-	defer sc.Stop()
-	defer rs.Stop()
-
-	err := sc.SignProposal("", proposal)
-	require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error())
-
-	err = rs.privVal.SignProposal(chainID, proposal)
-	require.Error(t, err)
-
-	err = sc.SignProposal(chainID, proposal)
-	require.Error(t, err)
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				chainID = cmn.RandStr(12)
+				sc, rs  = testSetupSocketPair(t, chainID, types.NewErroringMockPV(), tc.addr, tc.dialer)
+
+				ts       = time.Now()
+				proposal = &types.Proposal{Timestamp: ts}
+			)
+			defer sc.Stop()
+			defer rs.Stop()
+
+			err := sc.SignProposal("", proposal)
+			require.Equal(t, err.(*RemoteSignerError).Description, types.ErroringMockPVErr.Error())
+
+			err = rs.privVal.SignProposal(chainID, proposal)
+			require.Error(t, err)
+
+			err = sc.SignProposal(chainID, proposal)
+			require.Error(t, err)
+		}()
+	}
 }
 
 func TestErrUnexpectedResponse(t *testing.T) {
-	var (
-		addr    = testFreeAddr(t)
-		logger  = log.TestingLogger()
-		chainID = cmn.RandStr(12)
-		readyc  = make(chan struct{})
-		errc    = make(chan error, 1)
-
-		rs = NewRemoteSigner(
-			logger,
-			chainID,
-			types.NewMockPV(),
-			DialTCPFn(addr, testConnDeadline, ed25519.GenPrivKey()),
-		)
-		sc = newSocketVal(logger, addr, testConnDeadline)
-	)
-
-	testStartSocketPV(t, readyc, sc)
-	defer sc.Stop()
-	RemoteSignerConnDeadline(time.Millisecond)(rs)
-	RemoteSignerConnRetries(100)(rs)
-	// we do not want to Start() the remote signer here and instead use the connection to
-	// reply with intentionally wrong replies below:
-	rsConn, err := rs.connect()
-	defer rsConn.Close()
-	require.NoError(t, err)
-	require.NotNil(t, rsConn)
-	// send over public key to get the remote signer running:
-	go testReadWriteResponse(t, &PubKeyResponse{}, rsConn)
-	<-readyc
-
-	// Proposal:
-	go func(errc chan error) {
-		errc <- sc.SignProposal(chainID, &types.Proposal{})
-	}(errc)
-	// read request and write wrong response:
-	go testReadWriteResponse(t, &SignedVoteResponse{}, rsConn)
-	err = <-errc
-	require.Error(t, err)
-	require.Equal(t, err, ErrUnexpectedResponse)
-
-	// Vote:
-	go func(errc chan error) {
-		errc <- sc.SignVote(chainID, &types.Vote{})
-	}(errc)
-	// read request and write wrong response:
-	go testReadWriteResponse(t, &SignedProposalResponse{}, rsConn)
-	err = <-errc
-	require.Error(t, err)
-	require.Equal(t, err, ErrUnexpectedResponse)
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				logger  = log.TestingLogger()
+				chainID = cmn.RandStr(12)
+				readyc  = make(chan struct{})
+				errc    = make(chan error, 1)
+
+				rs = NewRemoteSigner(
+					logger,
+					chainID,
+					types.NewMockPV(),
+					tc.dialer,
+				)
+				sc = newSocketVal(logger, tc.addr, testConnDeadline)
+			)
+
+			testStartSocketPV(t, readyc, sc)
+			defer sc.Stop()
+			RemoteSignerConnDeadline(time.Millisecond)(rs)
+			RemoteSignerConnRetries(100)(rs)
+			// we do not want to Start() the remote signer here and instead use the connection to
+			// reply with intentionally wrong replies below:
+			rsConn, err := rs.connect()
+			defer rsConn.Close()
+			require.NoError(t, err)
+			require.NotNil(t, rsConn)
+			// send over public key to get the remote signer running:
+			go testReadWriteResponse(t, &PubKeyResponse{}, rsConn)
+			<-readyc
+
+			// Proposal:
+			go func(errc chan error) {
+				errc <- sc.SignProposal(chainID, &types.Proposal{})
+			}(errc)
+			// read request and write wrong response:
+			go testReadWriteResponse(t, &SignedVoteResponse{}, rsConn)
+			err = <-errc
+			require.Error(t, err)
+			require.Equal(t, err, ErrUnexpectedResponse)
+
+			// Vote:
+			go func(errc chan error) {
+				errc <- sc.SignVote(chainID, &types.Vote{})
+			}(errc)
+			// read request and write wrong response:
+			go testReadWriteResponse(t, &SignedProposalResponse{}, rsConn)
+			err = <-errc
+			require.Error(t, err)
+			require.Equal(t, err, ErrUnexpectedResponse)
+		}()
+	}
 }
 
-func TestRetryTCPConnToRemoteSigner(t *testing.T) {
-	var (
-		addr    = testFreeAddr(t)
-		logger  = log.TestingLogger()
-		chainID = cmn.RandStr(12)
-		readyc  = make(chan struct{})
-
-		rs = NewRemoteSigner(
-			logger,
-			chainID,
-			types.NewMockPV(),
-			DialTCPFn(addr, testConnDeadline, ed25519.GenPrivKey()),
-		)
-		thisConnTimeout = testConnDeadline
-		sc              = newSocketVal(logger, addr, thisConnTimeout)
-	)
-	// Ping every:
-	SocketValHeartbeat(testHeartbeatTimeout)(sc)
-
-	RemoteSignerConnDeadline(testConnDeadline)(rs)
-	RemoteSignerConnRetries(10)(rs)
-
-	testStartSocketPV(t, readyc, sc)
-	defer sc.Stop()
-	require.NoError(t, rs.Start())
-	assert.True(t, rs.IsRunning())
-
-	<-readyc
-	time.Sleep(testHeartbeatTimeout * 2)
-
-	rs.Stop()
-	rs2 := NewRemoteSigner(
-		logger,
-		chainID,
-		types.NewMockPV(),
-		DialTCPFn(addr, testConnDeadline, ed25519.GenPrivKey()),
-	)
-	// let some pings pass
-	time.Sleep(testHeartbeatTimeout3o2)
-	require.NoError(t, rs2.Start())
-	assert.True(t, rs2.IsRunning())
-	defer rs2.Stop()
-
-	// give the client some time to re-establish the conn to the remote signer
-	// should see sth like this in the logs:
-	//
-	// E[10016-01-10|17:12:46.128] Ping                                         err="remote signer timed out"
-	// I[10016-01-10|17:16:42.447] Re-created connection to remote signer       impl=SocketVal
-	time.Sleep(testConnDeadline * 2)
+func TestRetryConnToRemoteSigner(t *testing.T) {
+	for _, tc := range socketTestCases(t) {
+		func() {
+			var (
+				logger  = log.TestingLogger()
+				chainID = cmn.RandStr(12)
+				readyc  = make(chan struct{})
+
+				rs = NewRemoteSigner(
+					logger,
+					chainID,
+					types.NewMockPV(),
+					tc.dialer,
+				)
+				thisConnTimeout = testConnDeadline
+				sc              = newSocketVal(logger, tc.addr, thisConnTimeout)
+			)
+			// Ping every:
+			SocketValHeartbeat(testHeartbeatTimeout)(sc)
+
+			RemoteSignerConnDeadline(testConnDeadline)(rs)
+			RemoteSignerConnRetries(10)(rs)
+
+			testStartSocketPV(t, readyc, sc)
+			defer sc.Stop()
+			require.NoError(t, rs.Start())
+			assert.True(t, rs.IsRunning())
+
+			<-readyc
+			time.Sleep(testHeartbeatTimeout * 2)
+
+			rs.Stop()
+			rs2 := NewRemoteSigner(
+				logger,
+				chainID,
+				types.NewMockPV(),
+				tc.dialer,
+			)
+			// let some pings pass
+			time.Sleep(testHeartbeatTimeout3o2)
+			require.NoError(t, rs2.Start())
+			assert.True(t, rs2.IsRunning())
+			defer rs2.Stop()
+
+			// give the client some time to re-establish the conn to the remote signer
+			// should see sth like this in the logs:
+			//
+			// E[10016-01-10|17:12:46.128] Ping                                         err="remote signer timed out"
+			// I[10016-01-10|17:16:42.447] Re-created connection to remote signer       impl=SocketVal
+			time.Sleep(testConnDeadline * 2)
+		}()
+	}
 }
 
 func newSocketVal(logger log.Logger, addr string, connDeadline time.Duration) *SocketVal {
-	ln, err := net.Listen(cmn.ProtocolAndAddress(addr))
+	proto, address := cmn.ProtocolAndAddress(addr)
+	ln, err := net.Listen(proto, address)
+	logger.Info("Listening at", "proto", proto, "address", address)
 	if err != nil {
 		panic(err)
 	}
-	tcpLn := NewTCPListener(ln, ed25519.GenPrivKey())
-	TCPListenerAcceptDeadline(testAcceptDeadline)(tcpLn)
-	TCPListenerConnDeadline(testConnDeadline)(tcpLn)
-	return NewSocketVal(logger, tcpLn)
+	var svln net.Listener
+	if proto == "unix" {
+		unixLn := NewUnixListener(ln)
+		UnixListenerAcceptDeadline(testAcceptDeadline)(unixLn)
+		UnixListenerConnDeadline(connDeadline)(unixLn)
+		svln = unixLn
+	} else {
+		tcpLn := NewTCPListener(ln, ed25519.GenPrivKey())
+		TCPListenerAcceptDeadline(testAcceptDeadline)(tcpLn)
+		TCPListenerConnDeadline(connDeadline)(tcpLn)
+		svln = tcpLn
+	}
+	return NewSocketVal(logger, svln)
 }
 
 func testSetupSocketPair(
 	t *testing.T,
 	chainID string,
 	privValidator types.PrivValidator,
+	addr string,
+	dialer Dialer,
 ) (*SocketVal, *RemoteSigner) {
 	var (
-		addr    = testFreeAddr(t)
 		logger  = log.TestingLogger()
 		privVal = privValidator
 		readyc  = make(chan struct{})
@@ -391,7 +418,7 @@ func testSetupSocketPair(
 			logger,
 			chainID,
 			privVal,
-			DialTCPFn(addr, testConnDeadline, ed25519.GenPrivKey()),
+			dialer,
 		)
 
 		thisConnTimeout = testConnDeadline
@@ -429,8 +456,8 @@ func testStartSocketPV(t *testing.T, readyc chan struct{}, sc *SocketVal) {
 	}(sc)
 }
 
-// testFreeAddr claims a free port so we don't block on listener being ready.
-func testFreeAddr(t *testing.T) string {
+// testFreeTCPAddr claims a free port so we don't block on listener being ready.
+func testFreeTCPAddr(t *testing.T) string {
 	ln, err := net.Listen("tcp", "127.0.0.1:0")
 	require.NoError(t, err)
 	defer ln.Close()

privval/remote_signer_test.go

@@ -0,0 +1,68 @@
+package privval
+
+import (
+	"net"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto/ed25519"
+	cmn "github.com/tendermint/tendermint/libs/common"
+	"github.com/tendermint/tendermint/libs/log"
+	"github.com/tendermint/tendermint/types"
+)
+
+// TestRemoteSignerRetryTCPOnly will test connection retry attempts over TCP. We
+// don't need this for Unix sockets because the OS instantly knows the state of
+// both ends of the socket connection. This basically causes the
+// RemoteSigner.dialer() call inside RemoteSigner.connect() to return
+// successfully immediately, putting an instant stop to any retry attempts.
+func TestRemoteSignerRetryTCPOnly(t *testing.T) {
+	var (
+		attemptc = make(chan int)
+		retries  = 2
+	)
+
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+
+	go func(ln net.Listener, attemptc chan<- int) {
+		attempts := 0
+
+		for {
+			conn, err := ln.Accept()
+			require.NoError(t, err)
+
+			err = conn.Close()
+			require.NoError(t, err)
+
+			attempts++
+
+			if attempts == retries {
+				attemptc <- attempts
+				break
+			}
+		}
+	}(ln, attemptc)
+
+	rs := NewRemoteSigner(
+		log.TestingLogger(),
+		cmn.RandStr(12),
+		types.NewMockPV(),
+		DialTCPFn(ln.Addr().String(), testConnDeadline, ed25519.GenPrivKey()),
+	)
+	defer rs.Stop()
+
+	RemoteSignerConnDeadline(time.Millisecond)(rs)
+	RemoteSignerConnRetries(retries)(rs)
+
+	assert.Equal(t, rs.Start(), ErrDialRetryMax)
+
+	select {
+	case attempts := <-attemptc:
+		assert.Equal(t, retries, attempts)
+	case <-time.After(100 * time.Millisecond):
+		t.Error("expected remote to observe connection attempts")
+	}
+}

privval/socket.go

@@ -157,7 +157,7 @@ type timeoutConn struct {
 	connDeadline time.Duration
 }
 
-// newTimeoutConn returns an instance of newTCPTimeoutConn.
+// newTimeoutConn returns an instance of timeoutConn.
 func newTimeoutConn(
 	conn net.Conn,
 	connDeadline time.Duration) *timeoutConn {

privval/socket_test.go

@@ -1,7 +1,9 @@
 package privval
 
 import (
+	"io/ioutil"
 	"net"
+	"os"
 	"testing"
 	"time"
 
@@ -18,67 +20,114 @@ func newPrivKey() ed25519.PrivKeyEd25519 {
 //-------------------------------------------
 // tests
 
-func TestTCPListenerAcceptDeadline(t *testing.T) {
+type listenerTestCase struct {
+	description string // For test reporting purposes.
+	listener    net.Listener
+	dialer      Dialer
+}
+
+// testUnixAddr will attempt to obtain a platform-independent temporary file
+// name for a Unix socket
+func testUnixAddr() (string, error) {
+	f, err := ioutil.TempFile("", "tendermint-privval-test-*")
+	if err != nil {
+		return "", err
+	}
+	addr := f.Name()
+	f.Close()
+	os.Remove(addr)
+	return addr, nil
+}
+
+func tcpListenerTestCase(t *testing.T, acceptDeadline, connectDeadline time.Duration) listenerTestCase {
 	ln, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	tcpLn := NewTCPListener(ln, newPrivKey())
-	TCPListenerAcceptDeadline(time.Millisecond)(tcpLn)
-	TCPListenerConnDeadline(time.Second)(tcpLn)
-
-	_, err = tcpLn.Accept()
-	opErr, ok := err.(*net.OpError)
-	if !ok {
-		t.Fatalf("have %v, want *net.OpError", err)
-	}
-
-	if have, want := opErr.Op, "accept"; have != want {
-		t.Errorf("have %v, want %v", have, want)
+	TCPListenerAcceptDeadline(acceptDeadline)(tcpLn)
+	TCPListenerConnDeadline(connectDeadline)(tcpLn)
+	return listenerTestCase{
+		description: "TCP",
+		listener:    tcpLn,
+		dialer:      DialTCPFn(ln.Addr().String(), testConnDeadline, newPrivKey()),
 	}
 }
 
-func TestTCPListenerConnDeadline(t *testing.T) {
-	ln, err := net.Listen("tcp", "127.0.0.1:0")
+func unixListenerTestCase(t *testing.T, acceptDeadline, connectDeadline time.Duration) listenerTestCase {
+	addr, err := testUnixAddr()
+	if err != nil {
+		t.Fatal(err)
+	}
+	ln, err := net.Listen("unix", addr)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	tcpLn := NewTCPListener(ln, newPrivKey())
-	TCPListenerAcceptDeadline(time.Second)(tcpLn)
-	TCPListenerConnDeadline(time.Millisecond)(tcpLn)
-
-	readyc := make(chan struct{})
-	donec := make(chan struct{})
-	go func(ln net.Listener) {
-		defer close(donec)
-
-		c, err := ln.Accept()
-		if err != nil {
-			t.Fatal(err)
-		}
-		<-readyc
+	unixLn := NewUnixListener(ln)
+	UnixListenerAcceptDeadline(acceptDeadline)(unixLn)
+	UnixListenerConnDeadline(connectDeadline)(unixLn)
+	return listenerTestCase{
+		description: "Unix",
+		listener:    unixLn,
+		dialer:      DialUnixFn(addr),
+	}
+}
 
-		time.Sleep(2 * time.Millisecond)
+func listenerTestCases(t *testing.T, acceptDeadline, connectDeadline time.Duration) []listenerTestCase {
+	return []listenerTestCase{
+		tcpListenerTestCase(t, acceptDeadline, connectDeadline),
+		unixListenerTestCase(t, acceptDeadline, connectDeadline),
+	}
+}
 
-		msg := make([]byte, 200)
-		_, err = c.Read(msg)
+func TestListenerAcceptDeadlines(t *testing.T) {
+	for _, tc := range listenerTestCases(t, time.Millisecond, time.Second) {
+		_, err := tc.listener.Accept()
 		opErr, ok := err.(*net.OpError)
 		if !ok {
-			t.Fatalf("have %v, want *net.OpError", err)
+			t.Fatalf("for %s listener, have %v, want *net.OpError", tc.description, err)
 		}
 
-		if have, want := opErr.Op, "read"; have != want {
-			t.Errorf("have %v, want %v", have, want)
+		if have, want := opErr.Op, "accept"; have != want {
+			t.Errorf("for %s listener,  have %v, want %v", tc.description, have, want)
 		}
-	}(tcpLn)
+	}
+}
 
-	dialer := DialTCPFn(ln.Addr().String(), testConnDeadline, newPrivKey())
-	_, err = dialer()
-	if err != nil {
-		t.Fatal(err)
+func TestListenerConnectDeadlines(t *testing.T) {
+	for _, tc := range listenerTestCases(t, time.Second, time.Millisecond) {
+		readyc := make(chan struct{})
+		donec := make(chan struct{})
+		go func(ln net.Listener) {
+			defer close(donec)
+
+			c, err := ln.Accept()
+			if err != nil {
+				t.Fatal(err)
+			}
+			<-readyc
+
+			time.Sleep(2 * time.Millisecond)
+
+			msg := make([]byte, 200)
+			_, err = c.Read(msg)
+			opErr, ok := err.(*net.OpError)
+			if !ok {
+				t.Fatalf("for %s listener, have %v, want *net.OpError", tc.description, err)
+			}
+
+			if have, want := opErr.Op, "read"; have != want {
+				t.Errorf("for %s listener, have %v, want %v", tc.description, have, want)
+			}
+		}(tc.listener)
+
+		_, err := tc.dialer()
+		if err != nil {
+			t.Fatal(err)
+		}
+		close(readyc)
+		<-donec
 	}
-	close(readyc)
-	<-donec
 }