diff --git a/CHANGELOG.md b/CHANGELOG.md index 75ca299cf..236b70723 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/UPGRADING.md b/UPGRADING.md index 3e2d1f699..edd50d9e1 100644 --- a/UPGRADING.md +++ b/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, diff --git a/cmd/priv_val_server/main.go b/cmd/priv_val_server/main.go index 6949e8781..768b9cf63 100644 --- a/cmd/priv_val_server/main.go +++ b/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) diff --git a/node/node.go b/node/node.go index 46cec300c..0c38fc116 100644 --- a/node/node.go +++ b/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 diff --git a/privval/client.go b/privval/client.go index 4d4395fdf..1ad104d8d 100644 --- a/privval/client.go +++ b/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 - } -} diff --git a/privval/client_test.go b/privval/client_test.go index 7fae6bf8b..3c3270649 100644 --- a/privval/client_test.go +++ b/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() diff --git a/privval/remote_signer_test.go b/privval/remote_signer_test.go new file mode 100644 index 000000000..8927e2242 --- /dev/null +++ b/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") + } +} diff --git a/privval/socket.go b/privval/socket.go index 96fa6c8e8..bd9cd9209 100644 --- a/privval/socket.go +++ b/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 { diff --git a/privval/socket_test.go b/privval/socket_test.go index 0c05fa3a0..b411b7f3a 100644 --- a/privval/socket_test.go +++ b/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 }