types.NewCommit (#3275)

* types.NewCommit

* use types.NewCommit everywhere

* fix log in unsafe_reset

* memoize height and round in constructor

* notes about deprecating toVote

* bring back memoizeHeightRound

blockchain/reactor_test.go

@@ -95,13 +95,13 @@ func newBlockchainReactor(logger log.Logger, genDoc *types.GenesisDoc, privVals
 
 	// let's add some blocks in
 	for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ {
-		lastCommit := &types.Commit{}
+		lastCommit := types.NewCommit(types.BlockID{}, nil)
 		if blockHeight > 1 {
 			lastBlockMeta := blockStore.LoadBlockMeta(blockHeight - 1)
 			lastBlock := blockStore.LoadBlock(blockHeight - 1)
 
 			vote := makeVote(&lastBlock.Header, lastBlockMeta.BlockID, state.Validators, privVals[0]).CommitSig()
-			lastCommit = &types.Commit{Precommits: []*types.CommitSig{vote}, BlockID: lastBlockMeta.BlockID}
+			lastCommit = types.NewCommit(lastBlockMeta.BlockID, []*types.CommitSig{vote})
 		}
 
 		thisBlock := makeBlock(blockHeight, state, lastCommit)

blockchain/store_test.go

@@ -23,11 +23,8 @@ import (
 
 // make a Commit with a single vote containing just the height and a timestamp
 func makeTestCommit(height int64, timestamp time.Time) *types.Commit {
-	return &types.Commit{
-		Precommits: []*types.CommitSig{
-			{Height: height, Timestamp: timestamp},
-		},
-	}
+	commitSigs := []*types.CommitSig{{Height: height, Timestamp: timestamp}}
+	return types.NewCommit(types.BlockID{}, commitSigs)
 }
 
 func makeStateAndBlockStore(logger log.Logger) (sm.State, *BlockStore) {

cmd/tendermint/commands/reset_priv_validator.go

@@ -61,7 +61,7 @@ func resetFilePV(privValKeyFile, privValStateFile string, logger log.Logger) {
 	} else {
 		pv := privval.GenFilePV(privValKeyFile, privValStateFile)
 		pv.Save()
-		logger.Info("Generated private validator file", "file", "keyFile", privValKeyFile,
+		logger.Info("Generated private validator file", "keyFile", privValKeyFile,
 			"stateFile", privValStateFile)
 	}
 }

consensus/replay_test.go

@@ -537,10 +537,8 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) {
 			}
 		case *types.Vote:
 			if p.Type == types.PrecommitType {
-				thisBlockCommit = &types.Commit{
-					BlockID:    p.BlockID,
-					Precommits: []*types.CommitSig{p.CommitSig()},
-				}
+				commitSigs := []*types.CommitSig{p.CommitSig()}
+				thisBlockCommit = types.NewCommit(p.BlockID, commitSigs)
 			}
 		}
 	}

consensus/state.go

@@ -954,7 +954,7 @@ func (cs *ConsensusState) createProposalBlock() (block *types.Block, blockParts
 	if cs.Height == 1 {
 		// We're creating a proposal for the first block.
 		// The commit is empty, but not nil.
-		commit = &types.Commit{}
+		commit = types.NewCommit(types.BlockID{}, nil)
 	} else if cs.LastCommit.HasTwoThirdsMajority() {
 		// Make the commit from LastCommit
 		commit = cs.LastCommit.MakeCommit()

consensus/types/round_state_test.go

@@ -53,11 +53,8 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) {
 		Data: types.Data{
 			Txs: txs,
 		},
-		Evidence: types.EvidenceData{},
-		LastCommit: &types.Commit{
-			BlockID:    blockID,
-			Precommits: precommits,
-		},
+		Evidence:   types.EvidenceData{},
+		LastCommit: types.NewCommit(blockID, precommits),
 	}
 	parts := block.MakePartSet(4096)
 	// Random Proposal

lite/helpers.go

@@ -80,12 +80,8 @@ func (pkz privKeys) signHeader(header *types.Header, first, last int) *types.Com
 		vote := makeVote(header, vset, pkz[i])
 		commitSigs[vote.ValidatorIndex] = vote.CommitSig()
 	}
-
-	res := &types.Commit{
-		BlockID:    types.BlockID{Hash: header.Hash()},
-		Precommits: commitSigs,
-	}
-	return res
+	blockID := types.BlockID{Hash: header.Hash()}
+	return types.NewCommit(blockID, commitSigs)
 }
 
 func makeVote(header *types.Header, valset *types.ValidatorSet, key crypto.PrivKey) *types.Vote {

lite/proxy/validate_test.go

@@ -70,7 +70,7 @@ func TestValidateBlock(t *testing.T) {
 			},
 			signedHeader: types.SignedHeader{
 				Header: &types.Header{Height: 11},
-				Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("0xDEADBEEF")}},
+				Commit: types.NewCommit(types.BlockID{Hash: []byte("0xDEADBEEF")}, nil),
 			},
 			wantErr: "Data hash doesn't match header",
 		},
@@ -81,7 +81,7 @@ func TestValidateBlock(t *testing.T) {
 			},
 			signedHeader: types.SignedHeader{
 				Header: &types.Header{Height: 11},
-				Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}},
+				Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil),
 			},
 		},
 		// End Header.Data hash mismatch test
@@ -169,7 +169,7 @@ func TestValidateBlockMeta(t *testing.T) {
 					ValidatorsHash: []byte("Tendermint"),
 					Time:           testTime2,
 				},
-				Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}},
+				Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil),
 			},
 			wantErr: "Headers don't match",
 		},
@@ -188,7 +188,7 @@ func TestValidateBlockMeta(t *testing.T) {
 					ValidatorsHash: []byte("Tendermint-x"),
 					Time:           testTime2,
 				},
-				Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}},
+				Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil),
 			},
 			wantErr: "Headers don't match",
 		},

node/node_test.go

@@ -260,7 +260,7 @@ func TestCreateProposalBlock(t *testing.T) {
 		evidencePool,
 	)
 
-	commit := &types.Commit{}
+	commit := types.NewCommit(types.BlockID{}, nil)
 	block, _ := blockExec.CreateProposalBlock(
 		height,
 		state, commit,

state/execution_test.go

@@ -79,7 +79,7 @@ func TestBeginBlockValidators(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: tc.lastCommitPrecommits}
+		lastCommit := types.NewCommit(prevBlockID, tc.lastCommitPrecommits)
 
 		// block for height 2
 		block, _ := state.MakeBlock(2, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address)
@@ -139,7 +139,7 @@ func TestBeginBlockByzantineValidators(t *testing.T) {
 	commitSig0 := (&types.Vote{ValidatorIndex: 0, Timestamp: now, Type: types.PrecommitType}).CommitSig()
 	commitSig1 := (&types.Vote{ValidatorIndex: 1, Timestamp: now}).CommitSig()
 	commitSigs := []*types.CommitSig{commitSig0, commitSig1}
-	lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: commitSigs}
+	lastCommit := types.NewCommit(prevBlockID, commitSigs)
 	for _, tc := range testCases {
 
 		block, _ := state.MakeBlock(10, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address)

types/block.go

@@ -490,8 +490,8 @@ func (cs *CommitSig) String() string {
 }
 
 // toVote converts the CommitSig to a vote.
-// Once CommitSig has fewer fields than vote,
-// converting to a Vote will require more information.
+// TODO: deprecate for #1648. Converting to Vote will require
+// access to ValidatorSet.
 func (cs *CommitSig) toVote() *Vote {
 	if cs == nil {
 		return nil
@@ -509,18 +509,30 @@ type Commit struct {
 	BlockID    BlockID      `json:"block_id"`
 	Precommits []*CommitSig `json:"precommits"`
 
-	// Volatile
+	// memoized in first call to corresponding method
+	// NOTE: can't memoize in constructor because constructor
+	// isn't used for unmarshaling
 	height   int64
 	round    int
 	hash     cmn.HexBytes
 	bitArray *cmn.BitArray
 }
 
+// NewCommit returns a new Commit with the given blockID and precommits.
+// TODO: memoize ValidatorSet in constructor so votes can be easily reconstructed
+// from CommitSig after #1648.
+func NewCommit(blockID BlockID, precommits []*CommitSig) *Commit {
+	return &Commit{
+		BlockID:    blockID,
+		Precommits: precommits,
+	}
+}
+
 // VoteSignBytes constructs the SignBytes for the given CommitSig.
 // The only unique part of the SignBytes is the Timestamp - all other fields
 // signed over are otherwise the same for all validators.
 func (commit *Commit) VoteSignBytes(chainID string, cs *CommitSig) []byte {
-	return cs.toVote().SignBytes(chainID)
+	return commit.ToVote(cs).SignBytes(chainID)
 }
 
 // memoizeHeightRound memoizes the height and round of the commit using
@@ -543,27 +555,20 @@ func (commit *Commit) memoizeHeightRound() {
 
 // ToVote converts a CommitSig to a Vote.
 // If the CommitSig is nil, the Vote will be nil.
-// When CommitSig is reduced to contain fewer fields,
-// this will need access to the ValidatorSet to properly
-// reconstruct the vote.
 func (commit *Commit) ToVote(cs *CommitSig) *Vote {
+	// TODO: use commit.validatorSet to reconstruct vote
+	// and deprecate .toVote
 	return cs.toVote()
 }
 
 // Height returns the height of the commit
 func (commit *Commit) Height() int64 {
-	if len(commit.Precommits) == 0 {
-		return 0
-	}
 	commit.memoizeHeightRound()
 	return commit.height
 }
 
 // Round returns the round of the commit
 func (commit *Commit) Round() int {
-	if len(commit.Precommits) == 0 {
-		return 0
-	}
 	commit.memoizeHeightRound()
 	return commit.round
 }
@@ -595,9 +600,10 @@ func (commit *Commit) BitArray() *cmn.BitArray {
 }
 
 // GetByIndex returns the vote corresponding to a given validator index.
+// Panics if `index >= commit.Size()`.
 // Implements VoteSetReader.
 func (commit *Commit) GetByIndex(index int) *Vote {
-	return commit.Precommits[index].toVote()
+	return commit.ToVote(commit.Precommits[index])
 }
 
 // IsCommit returns true if there is at least one vote.

types/validator_set_test.go

@@ -563,18 +563,12 @@ func TestValidatorSetVerifyCommit(t *testing.T) {
 	sig, err := privKey.Sign(vote.SignBytes(chainID))
 	assert.NoError(t, err)
 	vote.Signature = sig
-	commit := &Commit{
-		BlockID:    blockID,
-		Precommits: []*CommitSig{vote.CommitSig()},
-	}
+	commit := NewCommit(blockID, []*CommitSig{vote.CommitSig()})
 
 	badChainID := "notmychainID"
 	badBlockID := BlockID{Hash: []byte("goodbye")}
 	badHeight := height + 1
-	badCommit := &Commit{
-		BlockID:    blockID,
-		Precommits: []*CommitSig{nil},
-	}
+	badCommit := NewCommit(blockID, []*CommitSig{nil})
 
 	// test some error cases
 	// TODO: test more cases!

types/vote_set.go

@@ -545,10 +545,7 @@ func (voteSet *VoteSet) MakeCommit() *Commit {
 	for i, v := range voteSet.votes {
 		commitSigs[i] = v.CommitSig()
 	}
-	return &Commit{
-		BlockID:    *voteSet.maj23,
-		Precommits: commitSigs,
-	}
+	return NewCommit(*voteSet.maj23, commitSigs)
 }
 
 //--------------------------------------------------------------------------------