more review comments: require instead assert & wrap errors

This commit is contained in:
Ismail Khoffi
2018-12-03 12:03:50 +01:00
parent 50ac191f9d
commit f17c04c892
5 changed files with 40 additions and 33 deletions

View File

@ -13,6 +13,8 @@ import (
"testing" "testing"
"time" "time"
"github.com/pkg/errors"
abcicli "github.com/tendermint/tendermint/abci/client" abcicli "github.com/tendermint/tendermint/abci/client"
abci "github.com/tendermint/tendermint/abci/types" abci "github.com/tendermint/tendermint/abci/types"
bc "github.com/tendermint/tendermint/blockchain" bc "github.com/tendermint/tendermint/blockchain"
@ -74,7 +76,7 @@ func NewValidatorStub(privValidator types.PrivValidator, valIndex int) *validato
func (vs *validatorStub) signVote(voteType types.SignedMsgType, hash []byte, header types.PartSetHeader) (*types.Vote, error) { func (vs *validatorStub) signVote(voteType types.SignedMsgType, hash []byte, header types.PartSetHeader) (*types.Vote, error) {
addr, err := vs.PrivValidator.GetAddress() addr, err := vs.PrivValidator.GetAddress()
if err != nil { if err != nil {
return nil, err return nil, errors.Wrap(err, "Error while retrieving private validator's address")
} }
vote := &types.Vote{ vote := &types.Vote{
ValidatorIndex: vs.Index, ValidatorIndex: vs.Index,

View File

@ -25,12 +25,12 @@ func TestGenLoadValidator(t *testing.T) {
height := int64(100) height := int64(100)
privVal.LastHeight = height privVal.LastHeight = height
privVal.Save() privVal.Save()
addr, err := privVal.GetAddress() // FilePV doesn't err here addr, err := privVal.GetAddress()
assert.NoError(err) require.NoError(t, err)
privVal = LoadFilePV(tempFile.Name()) privVal = LoadFilePV(tempFile.Name())
loadedAddr, err := privVal.GetAddress() loadedAddr, err := privVal.GetAddress()
assert.NoError(err) require.NoError(t, err)
assert.Equal(addr, loadedAddr, "expected privval addr to be the same") assert.Equal(addr, loadedAddr, "expected privval addr to be the same")
assert.Equal(height, privVal.LastHeight, "expected privval.LastHeight to have been saved") assert.Equal(height, privVal.LastHeight, "expected privval.LastHeight to have been saved")
} }
@ -46,7 +46,7 @@ func TestLoadOrGenValidator(t *testing.T) {
} }
privVal := LoadOrGenFilePV(tempFilePath) privVal := LoadOrGenFilePV(tempFilePath)
addr, err := privVal.GetAddress() addr, err := privVal.GetAddress()
assert.NoError(err) require.NoError(t, err)
privVal = LoadOrGenFilePV(tempFilePath) privVal = LoadOrGenFilePV(tempFilePath)
loadedAddr, err := privVal.GetAddress() loadedAddr, err := privVal.GetAddress()
assert.Equal(addr, loadedAddr, "expected privval addr to be the same") assert.Equal(addr, loadedAddr, "expected privval addr to be the same")
@ -87,10 +87,10 @@ func TestUnmarshalValidator(t *testing.T) {
// make sure the values match // make sure the values match
loadedAddr, err := val.GetAddress() loadedAddr, err := val.GetAddress()
assert.NoError(err) require.NoError(err)
assert.EqualValues(addr, loadedAddr) assert.EqualValues(addr, loadedAddr)
loadedKey, err := val.GetPubKey() loadedKey, err := val.GetPubKey()
assert.NoError(err) require.NoError(err)
assert.EqualValues(pubKey, loadedKey) assert.EqualValues(pubKey, loadedKey)
assert.EqualValues(privKey, val.PrivKey) assert.EqualValues(privKey, val.PrivKey)

View File

@ -6,6 +6,8 @@ import (
"net" "net"
"sync" "sync"
"github.com/pkg/errors"
"github.com/tendermint/go-amino" "github.com/tendermint/go-amino"
"github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto"
cmn "github.com/tendermint/tendermint/libs/common" cmn "github.com/tendermint/tendermint/libs/common"
@ -231,7 +233,9 @@ func handleRequest(req RemoteSignerMsg, chainID string, privVal types.PrivValida
var p crypto.PubKey var p crypto.PubKey
p, err = privVal.GetPubKey() p, err = privVal.GetPubKey()
if err != nil { if err != nil {
// TODO: split up PubKeyMsg into PubKeyRequest / PubKeyResponse and wrap the error
// into the response as done below. For now we just return the error:
return nil, errors.Wrap(err, "Error while retrieving private validator's public key")
} }
res = &PubKeyMsg{p} res = &PubKeyMsg{p}
case *SignVoteRequest: case *SignVoteRequest:

View File

@ -2,9 +2,10 @@ package types
import ( import (
"bytes" "bytes"
"github.com/stretchr/testify/assert"
"testing" "testing"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto"
cmn "github.com/tendermint/tendermint/libs/common" cmn "github.com/tendermint/tendermint/libs/common"
tst "github.com/tendermint/tendermint/libs/test" tst "github.com/tendermint/tendermint/libs/test"
@ -68,7 +69,7 @@ func TestAddVote(t *testing.T) {
// t.Logf(">> %v", voteSet) // t.Logf(">> %v", voteSet)
val0Addr, err := val0.GetAddress() val0Addr, err := val0.GetAddress()
assert.NoError(t, err) require.NoError(t, err)
if voteSet.GetByAddress(val0Addr) != nil { if voteSet.GetByAddress(val0Addr) != nil {
t.Errorf("Expected GetByAddress(val0.Address) to be nil") t.Errorf("Expected GetByAddress(val0.Address) to be nil")
} }
@ -122,7 +123,7 @@ func Test2_3Majority(t *testing.T) {
// 6 out of 10 voted for nil. // 6 out of 10 voted for nil.
for i := 0; i < 6; i++ { for i := 0; i < 6; i++ {
addr, err := privValidators[i].GetAddress() addr, err := privValidators[i].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, i) vote := withValidator(voteProto, addr, i)
_, err = signAddVote(privValidators[i], vote, voteSet) _, err = signAddVote(privValidators[i], vote, voteSet)
if err != nil { if err != nil {
@ -137,7 +138,7 @@ func Test2_3Majority(t *testing.T) {
// 7th validator voted for some blockhash // 7th validator voted for some blockhash
{ {
addr, err := privValidators[6].GetAddress() addr, err := privValidators[6].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 6) vote := withValidator(voteProto, addr, 6)
_, err = signAddVote(privValidators[6], withBlockHash(vote, cmn.RandBytes(32)), voteSet) _, err = signAddVote(privValidators[6], withBlockHash(vote, cmn.RandBytes(32)), voteSet)
if err != nil { if err != nil {
@ -152,7 +153,7 @@ func Test2_3Majority(t *testing.T) {
// 8th validator voted for nil. // 8th validator voted for nil.
{ {
addr, err := privValidators[7].GetAddress() addr, err := privValidators[7].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 7) vote := withValidator(voteProto, addr, 7)
_, err = signAddVote(privValidators[7], vote, voteSet) _, err = signAddVote(privValidators[7], vote, voteSet)
if err != nil { if err != nil {
@ -186,7 +187,7 @@ func Test2_3MajorityRedux(t *testing.T) {
// 66 out of 100 voted for nil. // 66 out of 100 voted for nil.
for i := 0; i < 66; i++ { for i := 0; i < 66; i++ {
addr, err := privValidators[i].GetAddress() addr, err := privValidators[i].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, i) vote := withValidator(voteProto, addr, i)
_, err = signAddVote(privValidators[i], vote, voteSet) _, err = signAddVote(privValidators[i], vote, voteSet)
if err != nil { if err != nil {
@ -202,7 +203,7 @@ func Test2_3MajorityRedux(t *testing.T) {
{ {
adrr, err := privValidators[66].GetAddress() adrr, err := privValidators[66].GetAddress()
vote := withValidator(voteProto, adrr, 66) vote := withValidator(voteProto, adrr, 66)
assert.NoError(t, err) require.NoError(t, err)
_, err = signAddVote(privValidators[66], withBlockHash(vote, nil), voteSet) _, err = signAddVote(privValidators[66], withBlockHash(vote, nil), voteSet)
if err != nil { if err != nil {
t.Error(err) t.Error(err)
@ -216,7 +217,7 @@ func Test2_3MajorityRedux(t *testing.T) {
// 68th validator voted for a different BlockParts PartSetHeader // 68th validator voted for a different BlockParts PartSetHeader
{ {
addr, err := privValidators[67].GetAddress() addr, err := privValidators[67].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 67) vote := withValidator(voteProto, addr, 67)
blockPartsHeader := PartSetHeader{blockPartsTotal, crypto.CRandBytes(32)} blockPartsHeader := PartSetHeader{blockPartsTotal, crypto.CRandBytes(32)}
_, err = signAddVote(privValidators[67], withBlockPartsHeader(vote, blockPartsHeader), voteSet) _, err = signAddVote(privValidators[67], withBlockPartsHeader(vote, blockPartsHeader), voteSet)
@ -232,7 +233,7 @@ func Test2_3MajorityRedux(t *testing.T) {
// 69th validator voted for different BlockParts Total // 69th validator voted for different BlockParts Total
{ {
addr, err := privValidators[68].GetAddress() addr, err := privValidators[68].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 68) vote := withValidator(voteProto, addr, 68)
blockPartsHeader := PartSetHeader{blockPartsTotal + 1, blockPartsHeader.Hash} blockPartsHeader := PartSetHeader{blockPartsTotal + 1, blockPartsHeader.Hash}
_, err = signAddVote(privValidators[68], withBlockPartsHeader(vote, blockPartsHeader), voteSet) _, err = signAddVote(privValidators[68], withBlockPartsHeader(vote, blockPartsHeader), voteSet)
@ -248,7 +249,7 @@ func Test2_3MajorityRedux(t *testing.T) {
// 70th validator voted for different BlockHash // 70th validator voted for different BlockHash
{ {
addr, err := privValidators[69].GetAddress() addr, err := privValidators[69].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 69) vote := withValidator(voteProto, addr, 69)
_, err = signAddVote(privValidators[69], withBlockHash(vote, cmn.RandBytes(32)), voteSet) _, err = signAddVote(privValidators[69], withBlockHash(vote, cmn.RandBytes(32)), voteSet)
if err != nil { if err != nil {
@ -263,7 +264,7 @@ func Test2_3MajorityRedux(t *testing.T) {
// 71st validator voted for the right BlockHash & BlockPartsHeader // 71st validator voted for the right BlockHash & BlockPartsHeader
{ {
addr, err := privValidators[70].GetAddress() addr, err := privValidators[70].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 70) vote := withValidator(voteProto, addr, 70)
_, err = signAddVote(privValidators[70], vote, voteSet) _, err = signAddVote(privValidators[70], vote, voteSet)
if err != nil { if err != nil {
@ -293,7 +294,7 @@ func TestBadVotes(t *testing.T) {
// val0 votes for nil. // val0 votes for nil.
{ {
addr, err := privValidators[0].GetAddress() addr, err := privValidators[0].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 0) vote := withValidator(voteProto, addr, 0)
added, err := signAddVote(privValidators[0], vote, voteSet) added, err := signAddVote(privValidators[0], vote, voteSet)
if !added || err != nil { if !added || err != nil {
@ -304,7 +305,7 @@ func TestBadVotes(t *testing.T) {
// val0 votes again for some block. // val0 votes again for some block.
{ {
addr, err := privValidators[0].GetAddress() addr, err := privValidators[0].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 0) vote := withValidator(voteProto, addr, 0)
added, err := signAddVote(privValidators[0], withBlockHash(vote, cmn.RandBytes(32)), voteSet) added, err := signAddVote(privValidators[0], withBlockHash(vote, cmn.RandBytes(32)), voteSet)
if added || err == nil { if added || err == nil {
@ -315,7 +316,7 @@ func TestBadVotes(t *testing.T) {
// val1 votes on another height // val1 votes on another height
{ {
addr, err := privValidators[1].GetAddress() addr, err := privValidators[1].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 1) vote := withValidator(voteProto, addr, 1)
added, err := signAddVote(privValidators[1], withHeight(vote, height+1), voteSet) added, err := signAddVote(privValidators[1], withHeight(vote, height+1), voteSet)
if added || err == nil { if added || err == nil {
@ -326,7 +327,7 @@ func TestBadVotes(t *testing.T) {
// val2 votes on another round // val2 votes on another round
{ {
addr, err := privValidators[2].GetAddress() addr, err := privValidators[2].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 2) vote := withValidator(voteProto, addr, 2)
added, err := signAddVote(privValidators[2], withRound(vote, round+1), voteSet) added, err := signAddVote(privValidators[2], withRound(vote, round+1), voteSet)
if added || err == nil { if added || err == nil {
@ -337,7 +338,7 @@ func TestBadVotes(t *testing.T) {
// val3 votes of another type. // val3 votes of another type.
{ {
addr, err := privValidators[3].GetAddress() addr, err := privValidators[3].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 3) vote := withValidator(voteProto, addr, 3)
added, err := signAddVote(privValidators[3], withType(vote, byte(PrecommitType)), voteSet) added, err := signAddVote(privValidators[3], withType(vote, byte(PrecommitType)), voteSet)
if added || err == nil { if added || err == nil {
@ -363,7 +364,7 @@ func TestConflicts(t *testing.T) {
} }
val0Addr, err := privValidators[0].GetAddress() val0Addr, err := privValidators[0].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
// val0 votes for nil. // val0 votes for nil.
{ {
vote := withValidator(voteProto, val0Addr, 0) vote := withValidator(voteProto, val0Addr, 0)
@ -418,7 +419,7 @@ func TestConflicts(t *testing.T) {
// val1 votes for blockHash1. // val1 votes for blockHash1.
{ {
addr, err := privValidators[1].GetAddress() addr, err := privValidators[1].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 1) vote := withValidator(voteProto, addr, 1)
added, err := signAddVote(privValidators[1], withBlockHash(vote, blockHash1), voteSet) added, err := signAddVote(privValidators[1], withBlockHash(vote, blockHash1), voteSet)
if !added || err != nil { if !added || err != nil {
@ -437,7 +438,7 @@ func TestConflicts(t *testing.T) {
// val2 votes for blockHash2. // val2 votes for blockHash2.
{ {
addr, err := privValidators[2].GetAddress() addr, err := privValidators[2].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 2) vote := withValidator(voteProto, addr, 2)
added, err := signAddVote(privValidators[2], withBlockHash(vote, blockHash2), voteSet) added, err := signAddVote(privValidators[2], withBlockHash(vote, blockHash2), voteSet)
if !added || err != nil { if !added || err != nil {
@ -459,7 +460,7 @@ func TestConflicts(t *testing.T) {
// val2 votes for blockHash1. // val2 votes for blockHash1.
{ {
addr, err := privValidators[2].GetAddress() addr, err := privValidators[2].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 2) vote := withValidator(voteProto, addr, 2)
added, err := signAddVote(privValidators[2], withBlockHash(vote, blockHash1), voteSet) added, err := signAddVote(privValidators[2], withBlockHash(vote, blockHash1), voteSet)
if !added { if !added {
@ -502,7 +503,7 @@ func TestMakeCommit(t *testing.T) {
// 6 out of 10 voted for some block. // 6 out of 10 voted for some block.
for i := 0; i < 6; i++ { for i := 0; i < 6; i++ {
addr, err := privValidators[i].GetAddress() addr, err := privValidators[i].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, i) vote := withValidator(voteProto, addr, i)
_, err = signAddVote(privValidators[i], vote, voteSet) _, err = signAddVote(privValidators[i], vote, voteSet)
if err != nil { if err != nil {
@ -516,7 +517,7 @@ func TestMakeCommit(t *testing.T) {
// 7th voted for some other block. // 7th voted for some other block.
{ {
addr, err := privValidators[6].GetAddress() addr, err := privValidators[6].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 6) vote := withValidator(voteProto, addr, 6)
vote = withBlockHash(vote, cmn.RandBytes(32)) vote = withBlockHash(vote, cmn.RandBytes(32))
vote = withBlockPartsHeader(vote, PartSetHeader{123, cmn.RandBytes(32)}) vote = withBlockPartsHeader(vote, PartSetHeader{123, cmn.RandBytes(32)})
@ -530,7 +531,7 @@ func TestMakeCommit(t *testing.T) {
// The 8th voted like everyone else. // The 8th voted like everyone else.
{ {
addr, err := privValidators[7].GetAddress() addr, err := privValidators[7].GetAddress()
assert.NoError(t, err) require.NoError(t, err)
vote := withValidator(voteProto, addr, 7) vote := withValidator(voteProto, addr, 7)
_, err = signAddVote(privValidators[7], vote, voteSet) _, err = signAddVote(privValidators[7], vote, voteSet)
if err != nil { if err != nil {

View File

@ -141,7 +141,7 @@ func TestVoteProposalNotEq(t *testing.T) {
func TestVoteVerifySignature(t *testing.T) { func TestVoteVerifySignature(t *testing.T) {
privVal := NewMockPV() privVal := NewMockPV()
pubkey, err := privVal.GetPubKey() pubkey, err := privVal.GetPubKey()
assert.NoError(t, err) require.NoError(t, err)
vote := examplePrecommit() vote := examplePrecommit()
signBytes := vote.SignBytes("test_chain_id") signBytes := vote.SignBytes("test_chain_id")
@ -192,7 +192,7 @@ func TestIsVoteTypeValid(t *testing.T) {
func TestVoteVerify(t *testing.T) { func TestVoteVerify(t *testing.T) {
privVal := NewMockPV() privVal := NewMockPV()
pubkey, err := privVal.GetPubKey() pubkey, err := privVal.GetPubKey()
assert.NoError(t, err) require.NoError(t, err)
vote := examplePrevote() vote := examplePrevote()
vote.ValidatorAddress = pubkey.Address() vote.ValidatorAddress = pubkey.Address()