Return error for all PrivValidator methoods

As calls to the private validator can involve side-effects like network
communication it is desirable for all methods returning an error to not
break the control flow of the caller.

* adjust PrivValidator interface

types/priv_validator.go

@@ -46,8 +46,8 @@ type ValidatorID struct {
 // PrivValidator defines the functionality of a local Tendermint validator
 // that signs votes, proposals, and heartbeats, and never double signs.
 type PrivValidator2 interface {
-	Address() Address // redundant since .PubKey().Address()
+	Address() (Address, error) // redundant since .PubKey().Address()
-	PubKey() crypto.PubKey
+	PubKey() (crypto.PubKey, error)
 
 	SignVote(chainID string, vote *Vote) error
 	SignProposal(chainID string, proposal *Proposal) error

types/priv_validator/json.go

@@ -76,7 +76,12 @@ func (pvj *PrivValidatorJSON) SignProposal(chainID string, proposal *types.Propo
 
 // String returns a string representation of the PrivValidatorJSON.
 func (pvj *PrivValidatorJSON) String() string {
-	return fmt.Sprintf("PrivValidator{%v %v}", pvj.Address(), pvj.PrivValidatorUnencrypted.String())
+	addr, err := pvj.Address()
+	if err != nil {
+		panic(err)
+	}
+
+	return fmt.Sprintf("PrivValidator{%v %v}", addr, pvj.PrivValidatorUnencrypted.String())
 }
 
 func (pvj *PrivValidatorJSON) Save() {
@@ -170,7 +175,17 @@ func (pvs PrivValidatorsByAddress) Len() int {
 }
 
 func (pvs PrivValidatorsByAddress) Less(i, j int) bool {
-	return bytes.Compare(pvs[i].Address(), pvs[j].Address()) == -1
+	iaddr, err := pvs[j].Address()
+	if err != nil {
+		panic(err)
+	}
+
+	jaddr, err := pvs[i].Address()
+	if err != nil {
+		panic(err)
+	}
+
+	return bytes.Compare(iaddr, jaddr) == -1
 }
 
 func (pvs PrivValidatorsByAddress) Swap(i, j int) {

types/priv_validator/priv_validator_test.go

@@ -17,7 +17,7 @@ import (
 )
 
 func TestGenLoadValidator(t *testing.T) {
-	assert := assert.New(t)
+	assert, require := assert.New(t), require.New(t)
 
 	_, tempFilePath := cmn.Tempfile("priv_validator_")
 	privVal := GenPrivValidatorJSON(tempFilePath)
@@ -25,24 +25,33 @@ func TestGenLoadValidator(t *testing.T) {
 	height := int64(100)
 	privVal.LastSignedInfo.Height = height
 	privVal.Save()
-	addr := privVal.Address()
+	addr, err := privVal.Address()
+	require.Nil(err)
 
 	privVal = LoadPrivValidatorJSON(tempFilePath)
-	assert.Equal(addr, privVal.Address(), "expected privval addr to be the same")
+	pAddr, err := privVal.Address()
+	require.Nil(err)
+
+	assert.Equal(addr, pAddr, "expected privval addr to be the same")
 	assert.Equal(height, privVal.LastSignedInfo.Height, "expected privval.LastHeight to have been saved")
 }
 
 func TestLoadOrGenValidator(t *testing.T) {
-	assert := assert.New(t)
+	assert, require := assert.New(t), require.New(t)
 
 	_, tempFilePath := cmn.Tempfile("priv_validator_")
 	if err := os.Remove(tempFilePath); err != nil {
 		t.Error(err)
 	}
 	privVal := LoadOrGenPrivValidatorJSON(tempFilePath)
-	addr := privVal.Address()
+	addr, err := privVal.Address()
+	require.Nil(err)
+
 	privVal = LoadOrGenPrivValidatorJSON(tempFilePath)
-	assert.Equal(addr, privVal.Address(), "expected privval addr to be the same")
+	pAddr, err := privVal.Address()
+	require.Nil(err)
+
+	assert.Equal(addr, pAddr, "expected privval addr to be the same")
 }
 
 func TestUnmarshalValidator(t *testing.T) {
@@ -87,8 +96,14 @@ func TestUnmarshalValidator(t *testing.T) {
 	require.Nil(err, "%+v", err)
 
 	// make sure the values match
-	assert.EqualValues(addrBytes, val.Address())
+	vAddr, err := val.Address()
-	assert.EqualValues(pubKey, val.PubKey())
+	require.Nil(err)
+
+	pKey, err := val.PubKey()
+	require.Nil(err)
+
+	assert.EqualValues(addrBytes, vAddr)
+	assert.EqualValues(pubKey, pKey)
 	assert.EqualValues(privKey, val.PrivKey)
 
 	// export it and make sure it is the same
@@ -98,7 +113,7 @@ func TestUnmarshalValidator(t *testing.T) {
 }
 
 func TestSignVote(t *testing.T) {
-	assert := assert.New(t)
+	assert, require := assert.New(t), require.New(t)
 
 	_, tempFilePath := cmn.Tempfile("priv_validator_")
 	privVal := GenPrivValidatorJSON(tempFilePath)
@@ -109,8 +124,11 @@ func TestSignVote(t *testing.T) {
 	voteType := types.VoteTypePrevote
 
 	// sign a vote for first time
-	vote := newVote(privVal.Address(), 0, height, round, voteType, block1)
+	addr, err := privVal.Address()
-	err := privVal.SignVote("mychainid", vote)
+	require.Nil(err)
+
+	vote := newVote(addr, 0, height, round, voteType, block1)
+	err = privVal.SignVote("mychainid", vote)
 	assert.NoError(err, "expected no error signing vote")
 
 	// try to sign the same vote again; should be fine
@@ -119,10 +137,10 @@ func TestSignVote(t *testing.T) {
 
 	// now try some bad votes
 	cases := []*types.Vote{
-		newVote(privVal.Address(), 0, height, round-1, voteType, block1),   // round regression
+		newVote(addr, 0, height, round-1, voteType, block1),   // round regression
-		newVote(privVal.Address(), 0, height-1, round, voteType, block1),   // height regression
+		newVote(addr, 0, height-1, round, voteType, block1),   // height regression
-		newVote(privVal.Address(), 0, height-2, round+4, voteType, block1), // height regression and different round
+		newVote(addr, 0, height-2, round+4, voteType, block1), // height regression and different round
-		newVote(privVal.Address(), 0, height, round, voteType, block2),     // different block
+		newVote(addr, 0, height, round, voteType, block2),     // different block
 	}
 
 	for _, c := range cases {
@@ -179,6 +197,8 @@ func TestSignProposal(t *testing.T) {
 }
 
 func TestDifferByTimestamp(t *testing.T) {
+	require := require.New(t)
+
 	_, tempFilePath := cmn.Tempfile("priv_validator_")
 	privVal := GenPrivValidatorJSON(tempFilePath)
 
@@ -208,10 +228,13 @@ func TestDifferByTimestamp(t *testing.T) {
 
 	// test vote
 	{
+		addr, err := privVal.Address()
+		require.Nil(err)
+
 		voteType := types.VoteTypePrevote
 		blockID := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{}}
-		vote := newVote(privVal.Address(), 0, height, round, voteType, blockID)
+		vote := newVote(addr, 0, height, round, voteType, blockID)
-		err := privVal.SignVote("mychainid", vote)
+		err = privVal.SignVote("mychainid", vote)
 		assert.NoError(t, err, "expected no error signing vote")
 
 		signBytes := types.SignBytes(chainID, vote)

types/priv_validator/socket.go

@@ -86,23 +86,28 @@ func (pvsc *PrivValidatorSocketClient) OnStop() {
 }
 
 // Address is an alias for PubKey().Address().
-func (pvsc *PrivValidatorSocketClient) Address() cmn.HexBytes {
+func (pvsc *PrivValidatorSocketClient) Address() (cmn.HexBytes, error) {
-	return pvsc.PubKey().Address()
+	p, err := pvsc.PubKey()
+	if err != nil {
+		return nil, err
+	}
+
+	return p.Address(), nil
 }
 
 // PubKey implements PrivValidator.
-func (pvsc *PrivValidatorSocketClient) PubKey() crypto.PubKey {
+func (pvsc *PrivValidatorSocketClient) PubKey() (crypto.PubKey, error) {
 	err := writeMsg(pvsc.conn, &PubKeyMsg{})
 	if err != nil {
-		panic(err)
+		return crypto.PubKey{}, err
 	}
 
 	res, err := readMsg(pvsc.conn)
 	if err != nil {
-		panic(err)
+		return crypto.PubKey{}, err
 	}
 
-	return res.(*PubKeyMsg).PubKey
+	return res.(*PubKeyMsg).PubKey, nil
 }
 
 // SignVote implements PrivValidator.
@@ -316,7 +321,10 @@ func (pvss *PrivValidatorSocketServer) handleConnection(conn net.Conn) {
 
 		switch r := req.(type) {
 		case *PubKeyMsg:
-			res = &PubKeyMsg{pvss.privVal.PubKey()}
+			var p crypto.PubKey
+
+			p, err = pvss.privVal.PubKey()
+			res = &PubKeyMsg{p}
 		case *SignVoteMsg:
 			err = pvss.privVal.SignVote(pvss.chainID, r.Vote)
 			res = &SignVoteMsg{r.Vote}

types/priv_validator/socket_test.go

@@ -8,7 +8,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	crypto "github.com/tendermint/go-crypto"
-	cmn "github.com/tendermint/tmlibs/common"
 	"github.com/tendermint/tmlibs/log"
 
 	"github.com/tendermint/tendermint/types"
@@ -51,8 +50,21 @@ func TestPrivValidatorSocketServer(t *testing.T) {
 
 	assert.True(pvsc.IsRunning())
 
-	assert.Equal(pvsc.Address(), cmn.HexBytes(pvss.privVal.PubKey().Address()))
+	cAddr, err := pvsc.Address()
-	assert.Equal(pvsc.PubKey(), pvss.privVal.PubKey())
+	require.Nil(err)
+
+	sAddr, err := pvss.privVal.Address()
+	require.Nil(err)
+
+	assert.Equal(cAddr, sAddr)
+
+	cKey, err := pvsc.PubKey()
+	require.Nil(err)
+
+	sKey, err := pvss.privVal.PubKey()
+	require.Nil(err)
+
+	assert.Equal(cKey, sKey)
 
 	err = pvsc.SignProposal(chainID, &types.Proposal{
 		Timestamp: time.Now(),
@@ -104,8 +116,21 @@ func TestPrivValidatorSocketServerWithoutSecret(t *testing.T) {
 
 	assert.True(pvsc.IsRunning())
 
-	assert.Equal(pvsc.Address(), cmn.HexBytes(pvss.privVal.PubKey().Address()))
+	cAddr, err := pvsc.Address()
-	assert.Equal(pvsc.PubKey(), pvss.privVal.PubKey())
+	require.Nil(err)
+
+	sAddr, err := pvss.privVal.Address()
+	require.Nil(err)
+
+	assert.Equal(cAddr, sAddr)
+
+	cKey, err := pvsc.PubKey()
+	require.Nil(err)
+
+	sKey, err := pvss.privVal.PubKey()
+	require.Nil(err)
+
+	assert.Equal(cKey, sKey)
 
 	err = pvsc.SignProposal(chainID, &types.Proposal{
 		Timestamp: time.Now(),

types/priv_validator/unencrypted.go

@@ -35,15 +35,20 @@ func NewPrivValidatorUnencrypted(priv crypto.PrivKey) *PrivValidatorUnencrypted
 
 // String returns a string representation of the PrivValidatorUnencrypted
 func (upv *PrivValidatorUnencrypted) String() string {
-	return fmt.Sprintf("PrivValidator{%v %v}", upv.Address(), upv.LastSignedInfo.String())
+	addr, err := upv.Address()
+	if err != nil {
+		panic(err)
+	}
+
+	return fmt.Sprintf("PrivValidator{%v %v}", addr, upv.LastSignedInfo.String())
 }
 
-func (upv *PrivValidatorUnencrypted) Address() cmn.HexBytes {
+func (upv *PrivValidatorUnencrypted) Address() (cmn.HexBytes, error) {
-	return upv.PrivKey.PubKey().Address()
+	return upv.PrivKey.PubKey().Address(), nil
 }
 
-func (upv *PrivValidatorUnencrypted) PubKey() crypto.PubKey {
+func (upv *PrivValidatorUnencrypted) PubKey() (crypto.PubKey, error) {
-	return upv.PrivKey.PubKey()
+	return upv.PrivKey.PubKey(), nil
 }
 
 func (upv *PrivValidatorUnencrypted) SignVote(chainID string, vote *types.Vote) error {