Addressing review comments

CHANGELOG_PENDING.md

@@ -1,6 +1,7 @@
 # Pending
 
 BREAKING CHANGES:
+- [types] Reduce Commit votes to signatures and timestamps
 - [types] Header ...
 - [state] Add NextValidatorSet, changes on-disk representation of state
 - [state] Validator set changes are delayed by one block (!)

blockchain/store_test.go

@@ -69,8 +69,11 @@ var (
 	partSet     = block.MakePartSet(2)
 	part1       = partSet.GetPart(0)
 	part2       = partSet.GetPart(1)
-	seenCommit1 = &types.Commit{Precommits: []*types.Vote{{Height: 10,
-		Timestamp: time.Now().UTC()}}}
+	seenCommit1 = &types.Commit{
+		Precommits: []*types.CommitSig{{
+			Timestamp: time.Now().UTC()}},
+		HeightNum: 10,
+	}
 )
 
 // TODO: This test should be simplified ...
@@ -90,8 +93,11 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
 	// save a block
 	block := makeBlock(bs.Height()+1, state)
 	validPartSet := block.MakePartSet(2)
-	seenCommit := &types.Commit{Precommits: []*types.Vote{{Height: 10,
-		Timestamp: time.Now().UTC()}}}
+	seenCommit := &types.Commit{
+		Precommits: []*types.CommitSig{{
+			Timestamp: time.Now().UTC()}},
+		HeightNum: 10,
+	}
 	bs.SaveBlock(block, partSet, seenCommit)
 	require.Equal(t, bs.Height(), block.Header.Height, "expecting the new height to be changed")
 
@@ -110,8 +116,11 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) {
 
 	// End of setup, test data
 
-	commitAtH10 := &types.Commit{Precommits: []*types.Vote{{Height: 10,
-		Timestamp: time.Now().UTC()}}}
+	commitAtH10 := &types.Commit{
+		Precommits: []*types.CommitSig{{
+			Timestamp: time.Now().UTC()}},
+		HeightNum: 10,
+	}
 	tuples := []struct {
 		block      *types.Block
 		parts      *types.PartSet
@@ -334,8 +343,11 @@ func TestBlockFetchAtHeight(t *testing.T) {
 	block := makeBlock(bs.Height()+1, state)
 
 	partSet := block.MakePartSet(2)
-	seenCommit := &types.Commit{Precommits: []*types.Vote{{Height: 10,
-		Timestamp: time.Now().UTC()}}}
+	seenCommit := &types.Commit{
+		Precommits: []*types.CommitSig{{
+			Timestamp: time.Now().UTC()}},
+		HeightNum: 10,
+	}
 
 	bs.SaveBlock(block, partSet, seenCommit)
 	require.Equal(t, bs.Height(), block.Header.Height, "expecting the new height to be changed")

consensus/reactor.go

@@ -629,6 +629,7 @@ OUTER_LOOP:
 			// Load the block commit for prs.Height,
 			// which contains precommit signatures for prs.Height.
 			commit := conR.conS.blockStore.LoadBlockCommit(prs.Height)
+			commit.SetAddresses(conR.conS.Validators.GetAddresses())
 			if ps.PickSendVote(commit) {
 				logger.Debug("Picked Catchup commit to send", "height", prs.Height)
 				continue OUTER_LOOP

consensus/reactor_test.go

@@ -607,12 +607,6 @@ func validateBlock(block *types.Block, activeVals map[string]struct{}) error {
 	if block.LastCommit.Size() != len(activeVals) {
 		return fmt.Errorf("Commit size doesn't match number of active validators. Got %d, expected %d", block.LastCommit.Size(), len(activeVals))
 	}
-
-	for _, vote := range block.LastCommit.Precommits {
-		if _, ok := activeVals[string(vote.ValidatorAddress)]; !ok {
-			return fmt.Errorf("Found vote for unactive validator %X", vote.ValidatorAddress)
-		}
-	}
 	return nil
 }
 

consensus/replay_test.go

@@ -531,7 +531,7 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) {
 				if block.Height != height+1 {
 					panic(fmt.Sprintf("read bad block from wal. got height %d, expected %d", block.Height, height+1))
 				}
-				commitHeight := thisBlockCommit.Precommits[0].Height
+				commitHeight := thisBlockCommit.Height()
 				if commitHeight != height+1 {
 					panic(fmt.Sprintf("commit doesnt match. got height %d, expected %d", commitHeight, height+1))
 				}
@@ -549,8 +549,13 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) {
 		case *types.Vote:
 			if p.Type == types.VoteTypePrecommit {
 				thisBlockCommit = &types.Commit{
-					BlockID:    p.BlockID,
-					Precommits: []*types.Vote{p},
+					BlockID: p.BlockID,
+					Precommits: []*types.CommitSig{{
+						Signature: p.Signature,
+						Timestamp: p.Timestamp,
+					}},
+					HeightNum: p.Height,
+					RoundNum:  p.Round,
 				}
 			}
 		}
@@ -564,7 +569,7 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) {
 	if block.Height != height+1 {
 		panic(fmt.Sprintf("read bad block from wal. got height %d, expected %d", block.Height, height+1))
 	}
-	commitHeight := thisBlockCommit.Precommits[0].Height
+	commitHeight := thisBlockCommit.Height()
 	if commitHeight != height+1 {
 		panic(fmt.Sprintf("commit doesnt match. got height %d, expected %d", commitHeight, height+1))
 	}

consensus/state.go

@@ -452,12 +452,14 @@ func (cs *ConsensusState) reconstructLastCommit(state sm.State) {
 		return
 	}
 	seenCommit := cs.blockStore.LoadSeenCommit(state.LastBlockHeight)
+	seenCommit.SetAddresses(cs.Validators.GetAddresses())
 	lastPrecommits := types.NewVoteSet(state.ChainID, state.LastBlockHeight, seenCommit.Round(), types.VoteTypePrecommit, state.LastValidators)
-	for _, precommit := range seenCommit.Precommits {
-		if precommit == nil {
+	for idx := 0; idx < len(seenCommit.Precommits); idx++ {
+		if seenCommit.Precommits[idx] == nil {
 			continue
 		}
-		added, err := lastPrecommits.AddVote(precommit)
+		vote := seenCommit.GetByIndex(idx)
+		added, err := lastPrecommits.AddVote(vote)
 		if !added || err != nil {
 			cmn.PanicCrisis(fmt.Sprintf("Failed to reconstruct LastCommit: %v", err))
 		}
@@ -1344,7 +1346,7 @@ func (cs *ConsensusState) recordMetrics(height int64, block *types.Block) {
 	missingValidators := 0
 	missingValidatorsPower := int64(0)
 	for i, val := range cs.Validators.Validators {
-		var vote *types.Vote
+		var vote *types.CommitSig
 		if i < len(block.LastCommit.Precommits) {
 			vote = block.LastCommit.Precommits[i]
 		}

consensus/state_test.go

@@ -7,7 +7,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/abci/example/counter"
 	cstypes "github.com/tendermint/tendermint/consensus/types"
+	dbm "github.com/tendermint/tendermint/libs/db"
 	"github.com/tendermint/tendermint/libs/log"
 	tmpubsub "github.com/tendermint/tendermint/libs/pubsub"
 	"github.com/tendermint/tendermint/types"
@@ -291,6 +294,51 @@ func TestStateFullRoundNil(t *testing.T) {
 	validatePrevoteAndPrecommit(t, cs, round, 0, vss[0], nil, nil)
 }
 
+// Tests that consensus state can be saved and reloaded from DB after committing a block.
+func TestStateFullRound1Reload(t *testing.T) {
+	blockDB := dbm.NewMemDB()
+	state, privVals := randGenesisState(1, false, 10)
+	vss := make([]*validatorStub, 1)
+	cs := newConsensusStateWithConfigAndBlockStore(config, state, privVals[0], counter.NewCounterApplication(true), blockDB)
+	vss[0] = NewValidatorStub(privVals[0], 0)
+	incrementHeight(vss[1:]...)
+	height, round := cs.Height, cs.Round
+
+	// NOTE: buffer capacity of 0 ensures we can validate prevote and last commit
+	// before consensus can move to the next height (and cause a race condition)
+	cs.eventBus.Stop()
+	eventBus := types.NewEventBusWithBufferCapacity(0)
+	eventBus.SetLogger(log.TestingLogger().With("module", "events"))
+	cs.SetEventBus(eventBus)
+	eventBus.Start()
+
+	voteCh := subscribe(cs.eventBus, types.EventQueryVote)
+	propCh := subscribe(cs.eventBus, types.EventQueryCompleteProposal)
+	newRoundCh := subscribe(cs.eventBus, types.EventQueryNewRound)
+
+	startTestRound(cs, height, round)
+
+	<-newRoundCh
+
+	// grab proposal
+	re := <-propCh
+	propBlockHash := re.(types.EventDataRoundState).RoundState.(*cstypes.RoundState).ProposalBlock.Hash()
+
+	<-voteCh // wait for prevote
+	validatePrevote(t, cs, round, vss[0], propBlockHash)
+
+	<-voteCh // wait for precommit
+
+	// we're going to roll right into new height
+	<-newRoundCh
+
+	validateLastPrecommit(t, cs, vss[0], propBlockHash)
+
+	// try reloading the consensus state from the DB
+	ncs := newConsensusStateWithConfigAndBlockStore(config, cs.state, nil, counter.NewCounterApplication(true), blockDB)
+	require.NotNil(t, ncs, "newConsensusState should not fail")
+}
+
 // run through propose, prevote, precommit commit with two validators
 // where the first validator has to wait for votes from the second
 func TestStateFullRound2(t *testing.T) {

consensus/types/round_state_test.go

@@ -16,7 +16,7 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) {
 	// Random validators
 	nval, ntxs := 100, 100
 	vset, _ := types.RandValidatorSet(nval, 1)
-	precommits := make([]*types.Vote, nval)
+	precommits := make([]*types.CommitSig, nval)
 	blockID := types.BlockID{
 		Hash: cmn.RandBytes(20),
 		PartsHeader: types.PartSetHeader{
@@ -25,11 +25,9 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) {
 	}
 	sig := make([]byte, ed25519.SignatureSize)
 	for i := 0; i < nval; i++ {
-		precommits[i] = &types.Vote{
-			ValidatorAddress: types.Address(cmn.RandBytes(20)),
-			Timestamp:        time.Now(),
-			BlockID:          blockID,
-			Signature:        sig,
+		precommits[i] = &types.CommitSig{
+			Timestamp: time.Now().UTC(),
+			Signature: sig,
 		}
 	}
 	txs := make([]types.Tx, ntxs)
@@ -40,7 +38,7 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) {
 	block := &types.Block{
 		Header: types.Header{
 			ChainID:         cmn.RandStr(12),
-			Time:            time.Now(),
+			Time:            time.Now().UTC(),
 			LastBlockID:     blockID,
 			LastCommitHash:  cmn.RandBytes(20),
 			DataHash:        cmn.RandBytes(20),
@@ -62,7 +60,7 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) {
 	parts := block.MakePartSet(4096)
 	// Random Proposal
 	proposal := &types.Proposal{
-		Timestamp: time.Now(),
+		Timestamp: time.Now().UTC(),
 		BlockPartsHeader: types.PartSetHeader{
 			Hash: cmn.RandBytes(20),
 		},
@@ -73,8 +71,8 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) {
 	// TODO: hvs :=
 
 	rs := &RoundState{
-		StartTime:          time.Now(),
-		CommitTime:         time.Now(),
+		StartTime:          time.Now().UTC(),
+		CommitTime:         time.Now().UTC(),
 		Validators:         vset,
 		Proposal:           proposal,
 		ProposalBlock:      block,

docs/spec/blockchain/blockchain.md

@@ -10,6 +10,7 @@ The Tendermint blockchains consists of a short list of basic data types:
 - `Header`
 - `Vote`
 - `BlockID`
+- `Commit`
 - `Signature`
 - `Evidence`
 
@@ -22,7 +23,7 @@ and a list of evidence of malfeasance (ie. signing conflicting votes).
 type Block struct {
     Header      Header
     Txs         [][]byte
-    LastCommit  []Vote
+    LastCommit  Commit
     Evidence    []Evidence
 }
 ```
@@ -107,6 +108,29 @@ There are two types of votes:
 a *prevote* has `vote.Type == 1` and
 a *precommit* has `vote.Type == 2`.
 
+## Commit
+
+A Commit contains enough data to verify that a BlockID was committed to at a certain
+height and round by a sufficient set of validators.
+
+```go
+type Commit struct {
+    HeightNum  int64
+    RoundNum   int
+    BlockID    BlockID
+    Precommits []*CommitSig
+}
+```
+
+The `CommitSig` values of the `Precommits` array contain the data for each validator needed to reconstruct and verify their precommit vote. For each entry, if the validator did not vote for the committed `BlockID`, their `CommitSig` value is `nil`. The only data unique to each vote is the `Timestamp` and `Signature`, all other data can be stored once in the `Commit` struct.
+
+```go
+type CommitSig struct {
+	Signature []byte
+	Timestamp time.Time
+}
+```
+
 ## Signature
 
 Tendermint allows for multiple signature schemes to be used by prepending a single type-byte

docs/tendermint-core/block-structure.md

@@ -73,7 +73,7 @@ they are commited to the chain.
 
 The
 [Commit](https://godoc.org/github.com/tendermint/tendermint/types#Commit)
-contains a set of
+contains the information needed to reconstruct the set of
 [Votes](https://godoc.org/github.com/tendermint/tendermint/types#Vote)
 that were made by the validator set to reach consensus on this block.
 This is the key to the security in any PoS system, and actually no data

lite/client/provider.go

@@ -129,6 +129,7 @@ func (p *provider) fillFullCommit(signedHeader types.SignedHeader) (fc lite.Full
 	if err != nil {
 		return lite.FullCommit{}, err
 	}
+	signedHeader.Commit.SetAddresses(valset.GetAddresses())
 
 	return lite.NewFullCommit(signedHeader, valset, nextValset), nil
 }

lite/dbprovider.go

@@ -177,6 +177,7 @@ func (dbp *DBProvider) fillFullCommit(sh types.SignedHeader) (FullCommit, error)
 	if err != nil {
 		return FullCommit{}, err
 	}
+	sh.Commit.SetAddresses(valset.GetAddresses())
 	// Return filled FullCommit.
 	return FullCommit{
 		SignedHeader:   sh,

lite/helpers.go

@@ -71,7 +71,7 @@ func (pkz privKeys) ToValidators(init, inc int64) *types.ValidatorSet {
 
 // signHeader properly signs the header with all keys from first to last exclusive.
 func (pkz privKeys) signHeader(header *types.Header, first, last int) *types.Commit {
-	votes := make([]*types.Vote, len(pkz))
+	commitSigs := make([]*types.CommitSig, len(pkz))
 
 	// We need this list to keep the ordering.
 	vset := pkz.ToValidators(1, 0)
@@ -79,14 +79,11 @@ func (pkz privKeys) signHeader(header *types.Header, first, last int) *types.Com
 	// Fill in the votes we want.
 	for i := first; i < last && i < len(pkz); i++ {
 		vote := makeVote(header, vset, pkz[i])
-		votes[vote.ValidatorIndex] = vote
+		commitSigs[vote.ValidatorIndex] = vote.ToCommitSig()
 	}
 
-	res := &types.Commit{
-		BlockID:    types.BlockID{Hash: header.Hash()},
-		Precommits: votes,
-	}
-	return res
+	blockID := types.BlockID{Hash: header.Hash()}
+	return types.NewCommit(header.Height, 1, blockID, commitSigs, vset.GetAddresses())
 }
 
 func makeVote(header *types.Header, valset *types.ValidatorSet, key crypto.PrivKey) *types.Vote {

state/execution.go

@@ -241,7 +241,7 @@ func getBeginBlockValidatorInfo(block *types.Block, lastValSet *types.ValidatorS
 	// Collect the vote info (list of validators and whether or not they signed).
 	voteInfos := make([]abci.VoteInfo, len(lastValSet.Validators))
 	for i, val := range lastValSet.Validators {
-		var vote *types.Vote
+		var vote *types.CommitSig
 		if i < len(block.LastCommit.Precommits) {
 			vote = block.LastCommit.Precommits[i]
 		}

state/execution_test.go

@@ -63,17 +63,17 @@ func TestBeginBlockValidators(t *testing.T) {
 	prevBlockID := types.BlockID{prevHash, prevParts}
 
 	now := time.Now().UTC()
-	vote0 := &types.Vote{ValidatorIndex: 0, Timestamp: now, Type: types.VoteTypePrecommit}
-	vote1 := &types.Vote{ValidatorIndex: 1, Timestamp: now}
+	vote0 := &types.CommitSig{Timestamp: now}
+	vote1 := &types.CommitSig{Timestamp: now}
 
 	testCases := []struct {
 		desc                     string
-		lastCommitPrecommits     []*types.Vote
+		lastCommitPrecommits     []*types.CommitSig
 		expectedAbsentValidators []int
 	}{
-		{"none absent", []*types.Vote{vote0, vote1}, []int{}},
-		{"one absent", []*types.Vote{vote0, nil}, []int{1}},
-		{"multiple absent", []*types.Vote{nil, nil}, []int{0, 1}},
+		{"none absent", []*types.CommitSig{vote0, vote1}, []int{}},
+		{"one absent", []*types.CommitSig{vote0, nil}, []int{1}},
+		{"multiple absent", []*types.CommitSig{nil, nil}, []int{0, 1}},
 	}
 
 	for _, tc := range testCases {
@@ -133,9 +133,9 @@ func TestBeginBlockByzantineValidators(t *testing.T) {
 			types.TM2PB.Evidence(ev2, valSet, now)}},
 	}
 
-	vote0 := &types.Vote{ValidatorIndex: 0, Timestamp: now, Type: types.VoteTypePrecommit}
-	vote1 := &types.Vote{ValidatorIndex: 1, Timestamp: now}
-	votes := []*types.Vote{vote0, vote1}
+	vote0 := &types.CommitSig{Timestamp: now}
+	vote1 := &types.CommitSig{Timestamp: now}
+	votes := []*types.CommitSig{vote0, vote1}
 	lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: votes}
 	for _, tc := range testCases {
 

types/block.go

@@ -296,49 +296,66 @@ type Commit struct {
 	// NOTE: The Precommits are in order of address to preserve the bonded ValidatorSet order.
 	// Any peer with a block can gossip precommits by index with a peer without recalculating the
 	// active ValidatorSet.
-	BlockID    BlockID `json:"block_id"`
-	Precommits []*Vote `json:"precommits"`
+	HeightNum  int64
+	RoundNum   int
+	BlockID    BlockID      `json:"block_id"`
+	Precommits []*CommitSig `json:"precommits"`
+	addresses  []Address
 
 	// Volatile
-	firstPrecommit *Vote
-	hash           cmn.HexBytes
-	bitArray       *cmn.BitArray
+	hash     cmn.HexBytes
+	bitArray *cmn.BitArray
 }
 
-// FirstPrecommit returns the first non-nil precommit in the commit.
-// If all precommits are nil, it returns an empty precommit with height 0.
-func (commit *Commit) FirstPrecommit() *Vote {
-	if len(commit.Precommits) == 0 {
-		return nil
+func NewCommit(height int64, round int, blockID BlockID, precommits []*CommitSig, addresses []Address) *Commit {
+	return &Commit{
+		HeightNum:  height,
+		RoundNum:   round,
+		BlockID:    blockID,
+		Precommits: precommits,
+		addresses:  addresses,
 	}
-	if commit.firstPrecommit != nil {
-		return commit.firstPrecommit
-	}
-	for _, precommit := range commit.Precommits {
-		if precommit != nil {
-			commit.firstPrecommit = precommit
-			return precommit
-		}
+}
+
+type CommitSig struct {
+	Signature []byte
+	Timestamp time.Time
+}
+
+func NewCommitSig(signature []byte, timestamp time.Time) *CommitSig {
+	return &CommitSig{
+		signature,
+		timestamp,
 	}
+}
+
+func (commitSig *CommitSig) String() string {
+	return fmt.Sprintf("CommitSig{%X @ %s}",
+		cmn.Fingerprint(commitSig.Signature),
+		CanonicalTime(commitSig.Timestamp))
+}
+
+func (commitSig *CommitSig) ToVote(index int, address Address, height int64, round int, blockID BlockID) *Vote {
 	return &Vote{
-		Type: VoteTypePrecommit,
+		ValidatorIndex:   index,
+		ValidatorAddress: address,
+		Height:           height,
+		Round:            round,
+		Timestamp:        commitSig.Timestamp,
+		Type:             VoteTypePrecommit,
+		BlockID:          blockID,
+		Signature:        commitSig.Signature,
 	}
 }
 
 // Height returns the height of the commit
 func (commit *Commit) Height() int64 {
-	if len(commit.Precommits) == 0 {
-		return 0
-	}
-	return commit.FirstPrecommit().Height
+	return commit.HeightNum
 }
 
 // Round returns the round of the commit
 func (commit *Commit) Round() int {
-	if len(commit.Precommits) == 0 {
-		return 0
-	}
-	return commit.FirstPrecommit().Round
+	return commit.RoundNum
 }
 
 // Type returns the vote type of the commit, which is always VoteTypePrecommit
@@ -367,9 +384,13 @@ func (commit *Commit) BitArray() *cmn.BitArray {
 	return commit.bitArray
 }
 
+func (commit *Commit) SetAddresses(addresses []Address) {
+	commit.addresses = addresses
+}
+
 // GetByIndex returns the vote corresponding to a given validator index
 func (commit *Commit) GetByIndex(index int) *Vote {
-	return commit.Precommits[index]
+	return commit.Precommits[index].ToVote(index, commit.addresses[index], commit.Height(), commit.Round(), commit.BlockID)
 }
 
 // IsCommit returns true if there is at least one vote
@@ -386,30 +407,6 @@ func (commit *Commit) ValidateBasic() error {
 	if len(commit.Precommits) == 0 {
 		return errors.New("No precommits in commit")
 	}
-	height, round := commit.Height(), commit.Round()
-
-	// Validate the precommits.
-	for _, precommit := range commit.Precommits {
-		// It's OK for precommits to be missing.
-		if precommit == nil {
-			continue
-		}
-		// Ensure that all votes are precommits.
-		if precommit.Type != VoteTypePrecommit {
-			return fmt.Errorf("Invalid commit vote. Expected precommit, got %v",
-				precommit.Type)
-		}
-		// Ensure that all heights are the same.
-		if precommit.Height != height {
-			return fmt.Errorf("Invalid commit precommit height. Expected %v, got %v",
-				height, precommit.Height)
-		}
-		// Ensure that all rounds are the same.
-		if precommit.Round != round {
-			return fmt.Errorf("Invalid commit precommit round. Expected %v, got %v",
-				round, precommit.Round)
-		}
-	}
 	return nil
 }
 
@@ -434,8 +431,8 @@ func (commit *Commit) StringIndented(indent string) string {
 		return "nil-Commit"
 	}
 	precommitStrings := make([]string, len(commit.Precommits))
-	for i, precommit := range commit.Precommits {
-		precommitStrings[i] = precommit.String()
+	for i := 0; i < len(commit.Precommits); i++ {
+		precommitStrings[i] = commit.GetByIndex(i).String()
 	}
 	return fmt.Sprintf(`Commit{
 %s  BlockID:    %v

types/block_test.go

@@ -118,7 +118,7 @@ func TestBlockMakePartSetWithEvidence(t *testing.T) {
 
 	partSet := MakeBlock(h, txs, commit, evList).MakePartSet(1024)
 	assert.NotNil(t, partSet)
-	assert.Equal(t, 3, partSet.Total())
+	assert.Equal(t, 2, partSet.Total())
 }
 
 func TestBlockHashesTo(t *testing.T) {
@@ -193,7 +193,6 @@ func TestCommit(t *testing.T) {
 	commit, err := MakeCommit(lastID, h-1, 1, voteSet, vals)
 	require.NoError(t, err)
 
-	assert.NotNil(t, commit.FirstPrecommit())
 	assert.Equal(t, h-1, commit.Height())
 	assert.Equal(t, 1, commit.Round())
 	assert.Equal(t, VoteTypePrecommit, commit.Type())
@@ -216,21 +215,6 @@ func TestCommitValidateBasic(t *testing.T) {
 	commit = randCommit()
 	commit.Precommits[0] = nil
 	assert.NoError(t, commit.ValidateBasic())
-
-	// tamper with types
-	commit = randCommit()
-	commit.Precommits[0].Type = VoteTypePrevote
-	assert.Error(t, commit.ValidateBasic())
-
-	// tamper with height
-	commit = randCommit()
-	commit.Precommits[0].Height = int64(100)
-	assert.Error(t, commit.ValidateBasic())
-
-	// tamper with round
-	commit = randCommit()
-	commit.Precommits[0].Round = 100
-	assert.Error(t, commit.ValidateBasic())
 }
 
 func randCommit() *Commit {

types/validator_set.go

@@ -121,6 +121,14 @@ func (vals *ValidatorSet) GetByAddress(address []byte) (index int, val *Validato
 	return -1, nil
 }
 
+func (vals *ValidatorSet) GetAddresses() []Address {
+	addresses := make([]Address, len(vals.Validators))
+	for i := 0; i < len(vals.Validators); i++ {
+		addresses[i] = vals.Validators[i].Address
+	}
+	return addresses
+}
+
 // GetByIndex returns the validator's address and validator itself by index.
 // It returns nil values if index is less than 0 or greater or equal to
 // len(ValidatorSet.Validators).
@@ -270,34 +278,21 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, height i
 	}
 
 	talliedVotingPower := int64(0)
-	round := commit.Round()
+	commit.SetAddresses(vals.GetAddresses())
 
 	for idx, precommit := range commit.Precommits {
 		if precommit == nil {
 			continue // OK, some precommits can be missing.
 		}
-		if precommit.Height != height {
-			return fmt.Errorf("Invalid commit -- wrong height: want %v got %v", height, precommit.Height)
-		}
-		if precommit.Round != round {
-			return fmt.Errorf("Invalid commit -- wrong round: want %v got %v", round, precommit.Round)
-		}
-		if precommit.Type != VoteTypePrecommit {
-			return fmt.Errorf("Invalid commit -- not precommit @ index %v", idx)
-		}
 		_, val := vals.GetByIndex(idx)
+		vote := commit.GetByIndex(idx)
 		// Validate signature.
-		precommitSignBytes := precommit.SignBytes(chainID)
+		precommitSignBytes := vote.SignBytes(chainID)
 		if !val.PubKey.VerifyBytes(precommitSignBytes, precommit.Signature) {
 			return fmt.Errorf("Invalid commit -- invalid signature: %v", precommit)
 		}
 		// Good precommit!
-		if blockID.Equals(precommit.BlockID) {
-			talliedVotingPower += val.VotingPower
-		} else {
-			// It's OK that the BlockID doesn't match.  We include stray
-			// precommits to measure validator availability.
-		}
+		talliedVotingPower += val.VotingPower
 	}
 
 	if talliedVotingPower > vals.TotalVotingPower()*2/3 {
@@ -349,40 +344,27 @@ func (vals *ValidatorSet) VerifyFutureCommit(newSet *ValidatorSet, chainID strin
 	// Check old voting power.
 	oldVotingPower := int64(0)
 	seen := map[int]bool{}
-	round := commit.Round()
 
 	for idx, precommit := range commit.Precommits {
 		if precommit == nil {
 			continue
 		}
-		if precommit.Height != height {
-			return cmn.NewError("Blocks don't match - %d vs %d", round, precommit.Round)
-		}
-		if precommit.Round != round {
-			return cmn.NewError("Invalid commit -- wrong round: %v vs %v", round, precommit.Round)
-		}
-		if precommit.Type != VoteTypePrecommit {
-			return cmn.NewError("Invalid commit -- not precommit @ index %v", idx)
-		}
+		vote := commit.GetByIndex(idx)
 		// See if this validator is in oldVals.
-		idx, val := oldVals.GetByAddress(precommit.ValidatorAddress)
+		adr, _ := newSet.GetByIndex(idx)
+		idx, val := oldVals.GetByAddress(adr)
 		if val == nil || seen[idx] {
 			continue // missing or double vote...
 		}
 		seen[idx] = true
 
 		// Validate signature.
-		precommitSignBytes := precommit.SignBytes(chainID)
+		precommitSignBytes := vote.SignBytes(chainID)
 		if !val.PubKey.VerifyBytes(precommitSignBytes, precommit.Signature) {
 			return cmn.NewError("Invalid commit -- invalid signature: %v", precommit)
 		}
 		// Good precommit!
-		if blockID.Equals(precommit.BlockID) {
-			oldVotingPower += val.VotingPower
-		} else {
-			// It's OK that the BlockID doesn't match.  We include stray
-			// precommits to measure validator availability.
-		}
+		oldVotingPower += val.VotingPower
 	}
 
 	if oldVotingPower <= oldVals.TotalVotingPower()*2/3 {

types/validator_set_test.go

@@ -393,7 +393,10 @@ func TestValidatorSetVerifyCommit(t *testing.T) {
 	vote.Signature = sig
 	commit := &Commit{
 		BlockID:    blockID,
-		Precommits: []*Vote{vote},
+		Precommits: []*CommitSig{NewCommitSig(sig, vote.Timestamp)},
+		HeightNum:  height,
+		RoundNum:   0,
+		addresses:  []Address{v1.Address},
 	}
 
 	badChainID := "notmychainID"
@@ -401,7 +404,9 @@ func TestValidatorSetVerifyCommit(t *testing.T) {
 	badHeight := height + 1
 	badCommit := &Commit{
 		BlockID:    blockID,
-		Precommits: []*Vote{nil},
+		Precommits: []*CommitSig{nil},
+		HeightNum:  height,
+		RoundNum:   0,
 	}
 
 	// test some error cases

types/vote.go

@@ -116,3 +116,7 @@ func (vote *Vote) Verify(chainID string, pubKey crypto.PubKey) error {
 	}
 	return nil
 }
+
+func (vote *Vote) ToCommitSig() *CommitSig {
+	return NewCommitSig(vote.Signature, vote.Timestamp)
+}

types/vote_set.go

@@ -539,14 +539,18 @@ func (voteSet *VoteSet) MakeCommit() *Commit {
 	if voteSet.maj23 == nil {
 		cmn.PanicSanity("Cannot MakeCommit() unless a blockhash has +2/3")
 	}
+	blockID := *voteSet.maj23
 
 	// For every validator, get the precommit
 	votesCopy := make([]*Vote, len(voteSet.votes))
 	copy(votesCopy, voteSet.votes)
-	return &Commit{
-		BlockID:    *voteSet.maj23,
-		Precommits: votesCopy,
+	precommits := make([]*CommitSig, len(voteSet.votes))
+	for i, v := range votesCopy {
+		if v != nil && v.BlockID.Equals(blockID) {
+			precommits[i] = v.ToCommitSig()
+		}
 	}
+	return NewCommit(voteSet.height, voteSet.round, blockID, precommits, voteSet.valSet.GetAddresses())
 }
 
 //--------------------------------------------------------------------------------

types/vote_set_test.go

@@ -447,7 +447,7 @@ func TestConflicts(t *testing.T) {
 
 func TestMakeCommit(t *testing.T) {
 	height, round := int64(1), 0
-	voteSet, _, privValidators := randVoteSet(height, round, VoteTypePrecommit, 10, 1)
+	voteSet, valz, privValidators := randVoteSet(height, round, VoteTypePrecommit, 10, 1)
 	blockHash, blockPartsHeader := crypto.CRandBytes(32), PartSetHeader{123, crypto.CRandBytes(32)}
 
 	voteProto := &Vote{
@@ -493,6 +493,20 @@ func TestMakeCommit(t *testing.T) {
 		}
 	}
 
+	// The 9th voted for a different BlockID
+	{
+		voteProtoDiff := voteProto.Copy()
+		voteProtoDiff.BlockID = BlockID{
+			crypto.CRandBytes(32),
+			PartSetHeader{123, crypto.CRandBytes(32)},
+		}
+		vote := withValidator(voteProtoDiff, privValidators[8].GetAddress(), 8)
+		_, err := signAddVote(privValidators[8], vote, voteSet)
+		if err != nil {
+			t.Error(err)
+		}
+	}
+
 	commit := voteSet.MakeCommit()
 
 	// Commit should have 10 elements
@@ -505,4 +519,8 @@ func TestMakeCommit(t *testing.T) {
 		t.Errorf("Error in Commit.ValidateBasic(): %v", err)
 	}
 
+	err := valz.VerifyCommit("test_chain_id", BlockID{blockHash, blockPartsHeader}, int64(1), commit)
+	if err != nil {
+		t.Errorf("Error in VerifyCommit: %v", err)
+	}
 }