From ad715fe9666ef8c8a9e35ffaae1e2862f54aa905 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 26 Nov 2019 14:10:38 +0400 Subject: [PATCH] types: change `Commit` to consist of just signatures (#4146) * types: change `Commit` to consist of just signatures These are final changes towards removing votes from commit and leaving only signatures (see ADR-25) Fixes #1648 * bring back TestCommitToVoteSetWithVotesForAnotherBlockOrNilBlock + add absent flag to Vote to indicate that it's for another block * encode nil votes as CommitSig with BlockIDFlagAbsent + make Commit#Precommits array of non-pointers because precommit will never be nil * add NewCommitSigAbsent and Absent() funcs * uncomment validation in CommitSig#ValidateBasic * add comments to ValidatorSet funcs * add a changelog entry * break instead of continue continue does not make sense in these cases * types: rename Commit#Precommits to Signatures * swagger: fix /commit response * swagger: change block_id_flag type * fix merge conflicts --- CHANGELOG_PENDING.md | 6 + blockchain/v0/reactor_test.go | 6 +- blockchain/v1/reactor_test.go | 6 +- consensus/mempool_test.go | 22 +- consensus/reactor.go | 26 +-- consensus/reactor_test.go | 6 +- consensus/replay_test.go | 14 +- consensus/state.go | 10 +- consensus/types/round_state_test.go | 6 +- lite/helpers.go | 20 +- lite/proxy/validate_test.go | 8 +- lite2/test_helpers.go | 22 +- node/node_test.go | 2 +- rpc/swagger/swagger.yaml | 54 ++--- state/execution.go | 21 +- state/execution_test.go | 41 ++-- state/helpers_test.go | 11 +- state/state.go | 15 +- state/validation.go | 8 +- state/validation_test.go | 39 ++-- store/store_test.go | 9 +- types/block.go | 308 ++++++++++++++++------------ types/block_test.go | 60 +++--- types/errors.go | 12 +- types/validator_set.go | 124 +++++------ types/validator_set_test.go | 24 ++- types/vote.go | 25 ++- types/vote_set.go | 20 +- types/vote_set_test.go | 19 +- types/vote_test.go | 14 -- 30 files changed, 532 insertions(+), 426 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 2703dcfb0..d9fe0c751 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -12,6 +12,11 @@ different modules: - Go API: the `Header` broke - P2P: since blocks go over the wire, technically the P2P protocol broke +Also, blocks are significantly smaller 🔥 because we got rid of the redundant +information in `Block#LastCommit`. `Commit` now mainly consists of a signature +and a validator address plus a timestamp. Note we may remove the validator +address & timestamp fields in the future (see ADR-25). + Special thanks to external contributors on this release: @erikgrinaker, @PSalant726, @gchaincl, @gregzaitsev @@ -66,6 +71,7 @@ program](https://hackerone.com/tendermint). - [abci] \#2521 Remove `TotalTxs` and `NumTxs` from `Header` - [types][\#4151](https://github.com/tendermint/tendermint/pull/4151) Enforce ordering of votes in DuplicateVoteEvidence to be lexicographically sorted on BlockID + - [types] \#1648 Change `Commit` to consist of just signatures - P2P Protocol diff --git a/blockchain/v0/reactor_test.go b/blockchain/v0/reactor_test.go index 6c4c8333d..08ff10a93 100644 --- a/blockchain/v0/reactor_test.go +++ b/blockchain/v0/reactor_test.go @@ -87,7 +87,7 @@ func newBlockchainReactor( // let's add some blocks in for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ { - lastCommit := types.NewCommit(types.BlockID{}, nil) + lastCommit := types.NewCommit(blockHeight-1, 0, types.BlockID{}, nil) if blockHeight > 1 { lastBlockMeta := blockStore.LoadBlockMeta(blockHeight - 1) lastBlock := blockStore.LoadBlock(blockHeight - 1) @@ -101,8 +101,8 @@ func newBlockchainReactor( if err != nil { panic(err) } - voteCommitSig := vote.CommitSig() - lastCommit = types.NewCommit(lastBlockMeta.BlockID, []*types.CommitSig{voteCommitSig}) + lastCommit = types.NewCommit(vote.Height, vote.Round, + lastBlockMeta.BlockID, []types.CommitSig{vote.CommitSig()}) } thisBlock := makeBlock(blockHeight, state, lastCommit) diff --git a/blockchain/v1/reactor_test.go b/blockchain/v1/reactor_test.go index d9cc99af6..2add11df1 100644 --- a/blockchain/v1/reactor_test.go +++ b/blockchain/v1/reactor_test.go @@ -109,13 +109,13 @@ func newBlockchainReactor( // let's add some blocks in for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ { - lastCommit := types.NewCommit(types.BlockID{}, nil) + lastCommit := types.NewCommit(blockHeight-1, 1, 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.NewCommit(lastBlockMeta.BlockID, []*types.CommitSig{vote}) + vote := makeVote(&lastBlock.Header, lastBlockMeta.BlockID, state.Validators, privVals[0]) + lastCommit = types.NewCommit(vote.Height, vote.Round, lastBlockMeta.BlockID, []types.CommitSig{vote.CommitSig()}) } thisBlock := makeBlock(blockHeight, state, lastCommit) diff --git a/consensus/mempool_test.go b/consensus/mempool_test.go index d13148c14..8cb01cb12 100644 --- a/consensus/mempool_test.go +++ b/consensus/mempool_test.go @@ -111,21 +111,19 @@ func TestMempoolTxConcurrentWithCommit(t *testing.T) { blockDB := dbm.NewMemDB() cs := newConsensusStateWithConfigAndBlockStore(config, state, privVals[0], NewCounterApplication(), blockDB) sm.SaveState(blockDB, state) - height, round := cs.Height, cs.Round - newBlockCh := subscribe(cs.eventBus, types.EventQueryNewBlock) + newBlockHeaderCh := subscribe(cs.eventBus, types.EventQueryNewBlockHeader) - NTxs := 3000 - go deliverTxsRange(cs, 0, NTxs) + const numTxs int64 = 3000 + go deliverTxsRange(cs, 0, int(numTxs)) - startTestRound(cs, height, round) - for nTxs := 0; nTxs < NTxs; { - ticker := time.NewTicker(time.Second * 30) + startTestRound(cs, cs.Height, cs.Round) + for n := int64(0); n < numTxs; { select { - case msg := <-newBlockCh: - blockEvent := msg.Data().(types.EventDataNewBlock) - nTxs += len(blockEvent.Block.Txs) - case <-ticker.C: - panic("Timed out waiting to commit blocks with transactions") + case msg := <-newBlockHeaderCh: + headerEvent := msg.Data().(types.EventDataNewBlockHeader) + n += headerEvent.NumTxs + case <-time.After(30 * time.Second): + t.Fatal("Timed out waiting 30s to commit blocks with transactions") } } } diff --git a/consensus/reactor.go b/consensus/reactor.go index 072453771..36c3185e4 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -452,7 +452,7 @@ func makeRoundStepMessage(rs *cstypes.RoundState) (nrsMsg *NewRoundStepMessage) Round: rs.Round, Step: rs.Step, SecondsSinceStartTime: int(time.Since(rs.StartTime).Seconds()), - LastCommitRound: rs.LastCommit.Round(), + LastCommitRound: rs.LastCommit.GetRound(), } return } @@ -805,7 +805,7 @@ OUTER_LOOP: commit := conR.conS.LoadCommit(prs.Height) peer.TrySend(StateChannel, cdc.MustMarshalBinaryBare(&VoteSetMaj23Message{ Height: prs.Height, - Round: commit.Round(), + Round: commit.Round, Type: types.PrecommitType, BlockID: commit.BlockID, })) @@ -1053,7 +1053,7 @@ func (ps *PeerState) PickVoteToSend(votes types.VoteSetReader) (vote *types.Vote return nil, false } - height, round, type_, size := votes.Height(), votes.Round(), types.SignedMsgType(votes.Type()), votes.Size() + height, round, votesType, size := votes.GetHeight(), votes.GetRound(), types.SignedMsgType(votes.Type()), votes.Size() // Lazily set data using 'votes'. if votes.IsCommit() { @@ -1061,7 +1061,7 @@ func (ps *PeerState) PickVoteToSend(votes types.VoteSetReader) (vote *types.Vote } ps.ensureVoteBitArrays(height, size) - psVotes := ps.getVoteBitArray(height, round, type_) + psVotes := ps.getVoteBitArray(height, round, votesType) if psVotes == nil { return nil, false // Not something worth sending } @@ -1071,14 +1071,14 @@ func (ps *PeerState) PickVoteToSend(votes types.VoteSetReader) (vote *types.Vote return nil, false } -func (ps *PeerState) getVoteBitArray(height int64, round int, type_ types.SignedMsgType) *cmn.BitArray { - if !types.IsVoteTypeValid(type_) { +func (ps *PeerState) getVoteBitArray(height int64, round int, votesType types.SignedMsgType) *cmn.BitArray { + if !types.IsVoteTypeValid(votesType) { return nil } if ps.PRS.Height == height { if ps.PRS.Round == round { - switch type_ { + switch votesType { case types.PrevoteType: return ps.PRS.Prevotes case types.PrecommitType: @@ -1086,7 +1086,7 @@ func (ps *PeerState) getVoteBitArray(height int64, round int, type_ types.Signed } } if ps.PRS.CatchupCommitRound == round { - switch type_ { + switch votesType { case types.PrevoteType: return nil case types.PrecommitType: @@ -1094,7 +1094,7 @@ func (ps *PeerState) getVoteBitArray(height int64, round int, type_ types.Signed } } if ps.PRS.ProposalPOLRound == round { - switch type_ { + switch votesType { case types.PrevoteType: return ps.PRS.ProposalPOL case types.PrecommitType: @@ -1105,7 +1105,7 @@ func (ps *PeerState) getVoteBitArray(height int64, round int, type_ types.Signed } if ps.PRS.Height == height+1 { if ps.PRS.LastCommitRound == round { - switch type_ { + switch votesType { case types.PrevoteType: return nil case types.PrecommitType: @@ -1223,16 +1223,16 @@ func (ps *PeerState) SetHasVote(vote *types.Vote) { ps.setHasVote(vote.Height, vote.Round, vote.Type, vote.ValidatorIndex) } -func (ps *PeerState) setHasVote(height int64, round int, type_ types.SignedMsgType, index int) { +func (ps *PeerState) setHasVote(height int64, round int, voteType types.SignedMsgType, index int) { logger := ps.logger.With( "peerH/R", fmt.Sprintf("%d/%d", ps.PRS.Height, ps.PRS.Round), "H/R", fmt.Sprintf("%d/%d", height, round)) - logger.Debug("setHasVote", "type", type_, "index", index) + logger.Debug("setHasVote", "type", voteType, "index", index) // NOTE: some may be nil BitArrays -> no side effects. - psVotes := ps.getVoteBitArray(height, round, type_) + psVotes := ps.getVoteBitArray(height, round, voteType) if psVotes != nil { psVotes.SetIndex(index, true) } diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index 476eeddfb..23469bf00 100644 --- a/consensus/reactor_test.go +++ b/consensus/reactor_test.go @@ -613,9 +613,9 @@ func validateBlock(block *types.Block, activeVals map[string]struct{}) error { 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) + for _, commitSig := range block.LastCommit.Signatures { + if _, ok := activeVals[string(commitSig.ValidatorAddress)]; !ok { + return fmt.Errorf("found vote for inactive validator %X", commitSig.ValidatorAddress) } } return nil diff --git a/consensus/replay_test.go b/consensus/replay_test.go index da96be39c..87c150fe8 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -883,7 +883,7 @@ func makeBlocks(n int, state *sm.State, privVal types.PrivValidator) []*types.Bl func makeBlock(state sm.State, lastBlock *types.Block, lastBlockMeta *types.BlockMeta, privVal types.PrivValidator, height int64) (*types.Block, *types.PartSet) { - lastCommit := types.NewCommit(types.BlockID{}, nil) + lastCommit := types.NewCommit(height-1, 0, types.BlockID{}, nil) if height > 1 { vote, _ := types.MakeVote( lastBlock.Header.Height, @@ -891,8 +891,8 @@ func makeBlock(state sm.State, lastBlock *types.Block, lastBlockMeta *types.Bloc state.Validators, privVal, lastBlock.Header.ChainID) - voteCommitSig := vote.CommitSig() - lastCommit = types.NewCommit(lastBlockMeta.BlockID, []*types.CommitSig{voteCommitSig}) + lastCommit = types.NewCommit(vote.Height, vote.Round, + lastBlockMeta.BlockID, []types.CommitSig{vote.CommitSig()}) } return state.MakeBlock(height, []types.Tx{}, lastCommit, nil, state.Validators.GetProposer().Address) @@ -971,7 +971,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)) } @@ -988,8 +988,8 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) { } case *types.Vote: if p.Type == types.PrecommitType { - commitSigs := []*types.CommitSig{p.CommitSig()} - thisBlockCommit = types.NewCommit(p.BlockID, commitSigs) + thisBlockCommit = types.NewCommit(p.Height, p.Round, + p.BlockID, []types.CommitSig{p.CommitSig()}) } } } @@ -1002,7 +1002,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)) } diff --git a/consensus/state.go b/consensus/state.go index 68ea31d26..015c3b513 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -1013,7 +1013,7 @@ func (cs *ConsensusState) createProposalBlock() (block *types.Block, blockParts case cs.Height == 1: // We're creating a proposal for the first block. // The commit is empty, but not nil. - commit = types.NewCommit(types.BlockID{}, nil) + commit = types.NewCommit(0, 0, types.BlockID{}, nil) case cs.LastCommit.HasTwoThirdsMajority(): // Make the commit from LastCommit commit = cs.LastCommit.MakeCommit() @@ -1464,11 +1464,11 @@ func (cs *ConsensusState) recordMetrics(height int64, block *types.Block) { missingValidators := 0 missingValidatorsPower := int64(0) for i, val := range cs.Validators.Validators { - var vote *types.CommitSig - if i < len(block.LastCommit.Precommits) { - vote = block.LastCommit.Precommits[i] + if i >= len(block.LastCommit.Signatures) { + break } - if vote == nil { + commitSig := block.LastCommit.Signatures[i] + if commitSig.Absent() { missingValidators++ missingValidatorsPower += val.VotingPower } diff --git a/consensus/types/round_state_test.go b/consensus/types/round_state_test.go index a9bc8e14b..d7861b313 100644 --- a/consensus/types/round_state_test.go +++ b/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.CommitSig, nval) + commitSigs := make([]types.CommitSig, nval) blockID := types.BlockID{ Hash: cmn.RandBytes(20), PartsHeader: types.PartSetHeader{ @@ -25,7 +25,7 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) { } sig := make([]byte, ed25519.SignatureSize) for i := 0; i < nval; i++ { - precommits[i] = (&types.Vote{ + commitSigs[i] = (&types.Vote{ ValidatorAddress: types.Address(cmn.RandBytes(20)), Timestamp: tmtime.Now(), BlockID: blockID, @@ -54,7 +54,7 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) { Txs: txs, }, Evidence: types.EvidenceData{}, - LastCommit: types.NewCommit(blockID, precommits), + LastCommit: types.NewCommit(1, 0, blockID, commitSigs), } parts := block.MakePartSet(4096) // Random Proposal diff --git a/lite/helpers.go b/lite/helpers.go index 115f7c2c5..5535722b0 100644 --- a/lite/helpers.go +++ b/lite/helpers.go @@ -70,21 +70,29 @@ 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 { - commitSigs := make([]*types.CommitSig, len(pkz)) + commitSigs := make([]types.CommitSig, len(pkz)) + for i := 0; i < len(pkz); i++ { + commitSigs[i] = types.NewCommitSigAbsent() + } // We need this list to keep the ordering. vset := pkz.ToValidators(1, 0) + blockID := types.BlockID{ + Hash: header.Hash(), + PartsHeader: types.PartSetHeader{Total: 1, Hash: crypto.CRandBytes(32)}, + } + // Fill in the votes we want. for i := first; i < last && i < len(pkz); i++ { - vote := makeVote(header, vset, pkz[i]) + vote := makeVote(header, vset, pkz[i], blockID) commitSigs[vote.ValidatorIndex] = vote.CommitSig() } - blockID := types.BlockID{Hash: header.Hash()} - return types.NewCommit(blockID, commitSigs) + + return types.NewCommit(header.Height, 1, blockID, commitSigs) } -func makeVote(header *types.Header, valset *types.ValidatorSet, key crypto.PrivKey) *types.Vote { +func makeVote(header *types.Header, valset *types.ValidatorSet, key crypto.PrivKey, blockID types.BlockID) *types.Vote { addr := key.PubKey().Address() idx, _ := valset.GetByAddress(addr) vote := &types.Vote{ @@ -94,7 +102,7 @@ func makeVote(header *types.Header, valset *types.ValidatorSet, key crypto.PrivK Round: 1, Timestamp: tmtime.Now(), Type: types.PrecommitType, - BlockID: types.BlockID{Hash: header.Hash()}, + BlockID: blockID, } // Sign it signBytes := vote.SignBytes(header.ChainID) diff --git a/lite/proxy/validate_test.go b/lite/proxy/validate_test.go index e55fc666e..cf9a0de6b 100644 --- a/lite/proxy/validate_test.go +++ b/lite/proxy/validate_test.go @@ -70,7 +70,7 @@ func TestValidateBlock(t *testing.T) { }, signedHeader: types.SignedHeader{ Header: &types.Header{Height: 11}, - Commit: types.NewCommit(types.BlockID{Hash: []byte("0xDEADBEEF")}, nil), + Commit: types.NewCommit(11, 0, 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.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil), + Commit: types.NewCommit(11, 0, 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.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil), + Commit: types.NewCommit(11, 0, 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.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil), + Commit: types.NewCommit(11, 0, types.BlockID{Hash: []byte("DEADBEEF")}, nil), }, wantErr: "headers don't match", }, diff --git a/lite2/test_helpers.go b/lite2/test_helpers.go index 4a03bf836..912a407b2 100644 --- a/lite2/test_helpers.go +++ b/lite2/test_helpers.go @@ -72,21 +72,31 @@ 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 { - commitSigs := make([]*types.CommitSig, len(pkz)) + commitSigs := make([]types.CommitSig, len(pkz)) + for i := 0; i < len(pkz); i++ { + commitSigs[i] = types.NewCommitSigAbsent() + } // We need this list to keep the ordering. vset := pkz.ToValidators(1, 0) + blockID := types.BlockID{ + Hash: header.Hash(), + PartsHeader: types.PartSetHeader{Total: 1, Hash: crypto.CRandBytes(32)}, + } + // Fill in the votes we want. for i := first; i < last && i < len(pkz); i++ { - vote := makeVote(header, vset, pkz[i]) + vote := makeVote(header, vset, pkz[i], blockID) commitSigs[vote.ValidatorIndex] = vote.CommitSig() } - blockID := types.BlockID{Hash: header.Hash()} - return types.NewCommit(blockID, commitSigs) + + return types.NewCommit(header.Height, 1, blockID, commitSigs) } -func makeVote(header *types.Header, valset *types.ValidatorSet, key crypto.PrivKey) *types.Vote { +func makeVote(header *types.Header, valset *types.ValidatorSet, + key crypto.PrivKey, blockID types.BlockID) *types.Vote { + addr := key.PubKey().Address() idx, _ := valset.GetByAddress(addr) vote := &types.Vote{ @@ -96,7 +106,7 @@ func makeVote(header *types.Header, valset *types.ValidatorSet, key crypto.PrivK Round: 1, Timestamp: tmtime.Now(), Type: types.PrecommitType, - BlockID: types.BlockID{Hash: header.Hash()}, + BlockID: blockID, } // Sign it signBytes := vote.SignBytes(header.ChainID) diff --git a/node/node_test.go b/node/node_test.go index f93fcd2b0..c1a4cdb84 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -279,7 +279,7 @@ func TestCreateProposalBlock(t *testing.T) { evidencePool, ) - commit := types.NewCommit(types.BlockID{}, nil) + commit := types.NewCommit(height-1, 0, types.BlockID{}, nil) block, _ := blockExec.CreateProposalBlock( height, state, commit, diff --git a/rpc/swagger/swagger.yaml b/rpc/swagger/swagger.yaml index 5b4766ca2..f23af419b 100644 --- a/rpc/swagger/swagger.yaml +++ b/rpc/swagger/swagger.yaml @@ -1350,9 +1350,13 @@ definitions: last_commit: type: object properties: + height: + type: number + round: + type: number block_id: $ref: "#/definitions/BlockID" - precommits: + signatures: type: array items: $ref: "#/definitions/Commit" @@ -1691,9 +1695,17 @@ definitions: type: "object" commit: required: + - "height" + - "round" - "block_id" - - "precommits" + - "signatures" properties: + height: + type: "string" + example: "1311801" + round: + type: "string" + example: "0" block_id: required: - "hash" @@ -1715,50 +1727,20 @@ definitions: example: "38D4B26B5B725C4F13571EFE022C030390E4C33C8CF6F88EDD142EA769642DBD" type: "object" type: "object" - precommits: + signatures: type: "array" items: type: "object" properties: - type: + block_id_flag: type: "number" example: 2 - height: - type: "string" - example: "12" - round: - type: "string" - example: "0" - block_id: - required: - - "hash" - - "parts" - properties: - hash: - type: "string" - example: "112BC173FD838FB68EB43476816CD7B4C6661B6884A9E357B417EE957E1CF8F7" - parts: - required: - - "total" - - "hash" - properties: - total: - type: "string" - example: "1" - hash: - type: "string" - example: "38D4B26B5B725C4F13571EFE022C030390E4C33C8CF6F88EDD142EA769642DBD" - type: "object" - type: "object" - timestamp: - type: "string" - example: "2019-04-22T17:01:58.376629719Z" validator_address: type: "string" example: "000001E443FD237E4B616E2FA69DF4EE3D49A94F" - validator_index: + timestamp: type: "string" - example: "0" + example: "2019-04-22T17:01:58.376629719Z" signature: type: "string" example: "14jaTQXYRt8kbLKEhdHq7AXycrFImiLuZx50uOjs2+Zv+2i7RTG/jnObD07Jo2ubZ8xd7bNBJMqkgtkd0oQHAw==" diff --git a/state/execution.go b/state/execution.go index 8a3af7362..1db515909 100644 --- a/state/execution.go +++ b/state/execution.go @@ -319,28 +319,27 @@ func getBeginBlockValidatorInfo(block *types.Block, stateDB dbm.DB) (abci.LastCo panic(err) // shouldn't happen } - // Sanity check that commit length matches validator set size - + // Sanity check that commit size matches validator set size - // only applies after first block - - precommitLen := block.LastCommit.Size() + commitSize := block.LastCommit.Size() valSetLen := len(lastValSet.Validators) - if precommitLen != valSetLen { + if commitSize != valSetLen { // sanity check - panic(fmt.Sprintf("precommit length (%d) doesn't match valset length (%d) at height %d\n\n%v\n\n%v", - precommitLen, valSetLen, block.Height, block.LastCommit.Precommits, lastValSet.Validators)) + panic(fmt.Sprintf("commit size (%d) doesn't match valset length (%d) at height %d\n\n%v\n\n%v", + commitSize, valSetLen, block.Height, block.LastCommit.Signatures, lastValSet.Validators)) } } else { lastValSet = types.NewValidatorSet(nil) } for i, val := range lastValSet.Validators { - var vote *types.CommitSig - if i < len(block.LastCommit.Precommits) { - vote = block.LastCommit.Precommits[i] + if i >= len(block.LastCommit.Signatures) { + break } + commitSig := block.LastCommit.Signatures[i] voteInfo := abci.VoteInfo{ Validator: types.TM2PB.Validator(val), - SignedLastBlock: vote != nil, + SignedLastBlock: !commitSig.Absent(), } voteInfos[i] = voteInfo } @@ -357,7 +356,7 @@ func getBeginBlockValidatorInfo(block *types.Block, stateDB dbm.DB) (abci.LastCo } commitInfo := abci.LastCommitInfo{ - Round: int32(block.LastCommit.Round()), + Round: int32(block.LastCommit.Round), Votes: voteInfos, } return commitInfo, byzVals diff --git a/state/execution_test.go b/state/execution_test.go index 40c075e3a..71b412d0f 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -62,22 +62,31 @@ func TestBeginBlockValidators(t *testing.T) { prevParts := types.PartSetHeader{} prevBlockID := types.BlockID{Hash: prevHash, PartsHeader: prevParts} - now := tmtime.Now() - commitSig0 := (&types.Vote{ValidatorIndex: 0, Timestamp: now, Type: types.PrecommitType}).CommitSig() - commitSig1 := (&types.Vote{ValidatorIndex: 1, Timestamp: now}).CommitSig() + var ( + now = tmtime.Now() + commitSig0 = types.NewCommitSigForBlock( + []byte("Signature1"), + state.Validators.Validators[0].Address, + now) + commitSig1 = types.NewCommitSigForBlock( + []byte("Signature2"), + state.Validators.Validators[1].Address, + now) + absentSig = types.NewCommitSigAbsent() + ) testCases := []struct { desc string - lastCommitPrecommits []*types.CommitSig + lastCommitSigs []types.CommitSig expectedAbsentValidators []int }{ - {"none absent", []*types.CommitSig{commitSig0, commitSig1}, []int{}}, - {"one absent", []*types.CommitSig{commitSig0, nil}, []int{1}}, - {"multiple absent", []*types.CommitSig{nil, nil}, []int{0, 1}}, + {"none absent", []types.CommitSig{commitSig0, commitSig1}, []int{}}, + {"one absent", []types.CommitSig{commitSig0, absentSig}, []int{1}}, + {"multiple absent", []types.CommitSig{absentSig, absentSig}, []int{0, 1}}, } for _, tc := range testCases { - lastCommit := types.NewCommit(prevBlockID, tc.lastCommitPrecommits) + lastCommit := types.NewCommit(1, 0, prevBlockID, tc.lastCommitSigs) // block for height 2 block, _ := state.MakeBlock(2, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address) @@ -134,10 +143,18 @@ func TestBeginBlockByzantineValidators(t *testing.T) { types.TM2PB.Evidence(ev2, valSet, now)}}, } - 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.NewCommit(prevBlockID, commitSigs) + var ( + commitSig0 = types.NewCommitSigForBlock( + []byte("Signature1"), + state.Validators.Validators[0].Address, + now) + commitSig1 = types.NewCommitSigForBlock( + []byte("Signature2"), + state.Validators.Validators[1].Address, + now) + ) + commitSigs := []types.CommitSig{commitSig0, commitSig1} + lastCommit := types.NewCommit(9, 0, prevBlockID, commitSigs) for _, tc := range testCases { block, _ := state.MakeBlock(10, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address) diff --git a/state/helpers_test.go b/state/helpers_test.go index 8a8dd9b13..00f30ed24 100644 --- a/state/helpers_test.go +++ b/state/helpers_test.go @@ -4,14 +4,16 @@ import ( "bytes" "fmt" + dbm "github.com/tendermint/tm-db" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" + cmn "github.com/tendermint/tendermint/libs/common" "github.com/tendermint/tendermint/proxy" sm "github.com/tendermint/tendermint/state" "github.com/tendermint/tendermint/types" tmtime "github.com/tendermint/tendermint/types/time" - dbm "github.com/tendermint/tm-db" ) type paramsChangeTestCase struct { @@ -61,7 +63,8 @@ func makeAndApplyGoodBlock(state sm.State, height int64, lastCommit *types.Commi if err := blockExec.ValidateBlock(state, block); err != nil { return state, types.BlockID{}, err } - blockID := types.BlockID{Hash: block.Hash(), PartsHeader: types.PartSetHeader{}} + blockID := types.BlockID{Hash: block.Hash(), + PartsHeader: types.PartSetHeader{Total: 3, Hash: cmn.RandBytes(32)}} state, err := blockExec.ApplyBlock(state, blockID, block) if err != nil { return state, types.BlockID{}, err @@ -75,7 +78,7 @@ func makeValidCommit( vals *types.ValidatorSet, privVals map[string]types.PrivValidator, ) (*types.Commit, error) { - sigs := make([]*types.CommitSig, 0) + sigs := make([]types.CommitSig, 0) for i := 0; i < vals.Size(); i++ { _, val := vals.GetByIndex(i) vote, err := types.MakeVote(height, blockID, vals, privVals[val.Address.String()], chainID) @@ -84,7 +87,7 @@ func makeValidCommit( } sigs = append(sigs, vote.CommitSig()) } - return types.NewCommit(blockID, sigs), nil + return types.NewCommit(height, 0, blockID, sigs), nil } // make some bogus txs diff --git a/state/state.go b/state/state.go index 8cdff90e4..e0612576a 100644 --- a/state/state.go +++ b/state/state.go @@ -164,15 +164,18 @@ func (state State) MakeBlock( // the votes sent by honest processes, i.e., a faulty processes can not arbitrarily increase or decrease the // computed value. func MedianTime(commit *types.Commit, validators *types.ValidatorSet) time.Time { - - weightedTimes := make([]*tmtime.WeightedTime, len(commit.Precommits)) + weightedTimes := make([]*tmtime.WeightedTime, len(commit.Signatures)) totalVotingPower := int64(0) - for i, vote := range commit.Precommits { - if vote != nil { - _, validator := validators.GetByIndex(vote.ValidatorIndex) + for i, commitSig := range commit.Signatures { + if commitSig.Absent() { + continue + } + _, validator := validators.GetByAddress(commitSig.ValidatorAddress) + // If there's no condition, TestValidateBlockCommit panics; not needed normally. + if validator != nil { totalVotingPower += validator.VotingPower - weightedTimes[i] = tmtime.NewWeightedTime(vote.Timestamp, validator.VotingPower) + weightedTimes[i] = tmtime.NewWeightedTime(commitSig.Timestamp, validator.VotingPower) } } diff --git a/state/validation.go b/state/validation.go index 409059901..2faee80a2 100644 --- a/state/validation.go +++ b/state/validation.go @@ -81,12 +81,12 @@ func validateBlock(evidencePool EvidencePool, stateDB dbm.DB, state State, block // Validate block LastCommit. if block.Height == 1 { - if len(block.LastCommit.Precommits) != 0 { - return errors.New("block at height 1 can't have LastCommit precommits") + if len(block.LastCommit.Signatures) != 0 { + return errors.New("block at height 1 can't have LastCommit signatures") } } else { - if len(block.LastCommit.Precommits) != state.LastValidators.Size() { - return types.NewErrInvalidCommitPrecommits(state.LastValidators.Size(), len(block.LastCommit.Precommits)) + if len(block.LastCommit.Signatures) != state.LastValidators.Size() { + return types.NewErrInvalidCommitSignatures(state.LastValidators.Size(), len(block.LastCommit.Signatures)) } err := state.LastValidators.VerifyCommit( state.ChainID, state.LastBlockID, block.Height-1, block.LastCommit) diff --git a/state/validation_test.go b/state/validation_test.go index 4c0bf1247..00918010d 100644 --- a/state/validation_test.go +++ b/state/validation_test.go @@ -30,14 +30,14 @@ func TestValidateBlockHeader(t *testing.T) { mock.Mempool{}, sm.MockEvidencePool{}, ) - lastCommit := types.NewCommit(types.BlockID{}, nil) + lastCommit := types.NewCommit(0, 0, types.BlockID{}, nil) // some bad values wrongHash := tmhash.Sum([]byte("this hash is wrong")) wrongVersion1 := state.Version.Consensus - wrongVersion1.Block += 1 + wrongVersion1.Block++ wrongVersion2 := state.Version.Consensus - wrongVersion2.App += 1 + wrongVersion2.App++ // Manipulation of any header field causes failure. testCases := []struct { @@ -100,8 +100,8 @@ func TestValidateBlockCommit(t *testing.T) { mock.Mempool{}, sm.MockEvidencePool{}, ) - lastCommit := types.NewCommit(types.BlockID{}, nil) - wrongPrecommitsCommit := types.NewCommit(types.BlockID{}, nil) + lastCommit := types.NewCommit(0, 0, types.BlockID{}, nil) + wrongSigsCommit := types.NewCommit(1, 0, types.BlockID{}, nil) badPrivVal := types.NewMockPV() for height := int64(1); height < validationTestsStopHeight; height++ { @@ -119,22 +119,25 @@ func TestValidateBlockCommit(t *testing.T) { chainID, ) require.NoError(t, err, "height %d", height) - wrongHeightCommit := types.NewCommit(state.LastBlockID, []*types.CommitSig{wrongHeightVote.CommitSig()}) + wrongHeightCommit := types.NewCommit( + wrongHeightVote.Height, + wrongHeightVote.Round, + state.LastBlockID, + []types.CommitSig{wrongHeightVote.CommitSig()}, + ) block, _ := state.MakeBlock(height, makeTxs(height), wrongHeightCommit, nil, proposerAddr) err = blockExec.ValidateBlock(state, block) _, isErrInvalidCommitHeight := err.(types.ErrInvalidCommitHeight) require.True(t, isErrInvalidCommitHeight, "expected ErrInvalidCommitHeight at height %d but got: %v", height, err) /* - #2589: test len(block.LastCommit.Precommits) == state.LastValidators.Size() + #2589: test len(block.LastCommit.Signatures) == state.LastValidators.Size() */ - block, _ = state.MakeBlock(height, makeTxs(height), wrongPrecommitsCommit, nil, proposerAddr) + block, _ = state.MakeBlock(height, makeTxs(height), wrongSigsCommit, nil, proposerAddr) err = blockExec.ValidateBlock(state, block) - _, isErrInvalidCommitPrecommits := err.(types.ErrInvalidCommitPrecommits) - require.True( - t, - isErrInvalidCommitPrecommits, - "expected ErrInvalidCommitPrecommits at height %d but got: %v", + _, isErrInvalidCommitSignatures := err.(types.ErrInvalidCommitSignatures) + require.True(t, isErrInvalidCommitSignatures, + "expected ErrInvalidCommitSignatures at height %d, but got: %v", height, err, ) @@ -157,7 +160,7 @@ func TestValidateBlockCommit(t *testing.T) { require.NoError(t, err, "height %d", height) /* - wrongPrecommitsCommit is fine except for the extra bad precommit + wrongSigsCommit is fine except for the extra bad precommit */ goodVote, err := types.MakeVote(height, blockID, state.Validators, privVals[proposerAddr.String()], chainID) require.NoError(t, err, "height %d", height) @@ -172,7 +175,11 @@ func TestValidateBlockCommit(t *testing.T) { } err = badPrivVal.SignVote(chainID, goodVote) require.NoError(t, err, "height %d", height) - wrongPrecommitsCommit = types.NewCommit(blockID, []*types.CommitSig{goodVote.CommitSig(), badVote.CommitSig()}) + err = badPrivVal.SignVote(chainID, badVote) + require.NoError(t, err, "height %d", height) + + wrongSigsCommit = types.NewCommit(goodVote.Height, goodVote.Round, + blockID, []types.CommitSig{goodVote.CommitSig(), badVote.CommitSig()}) } } @@ -189,7 +196,7 @@ func TestValidateBlockEvidence(t *testing.T) { mock.Mempool{}, sm.MockEvidencePool{}, ) - lastCommit := types.NewCommit(types.BlockID{}, nil) + lastCommit := types.NewCommit(0, 0, types.BlockID{}, nil) for height := int64(1); height < validationTestsStopHeight; height++ { proposerAddr := state.Validators.GetProposer().Address diff --git a/store/store_test.go b/store/store_test.go index 63c05023d..70d7e1c46 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -29,8 +29,13 @@ type cleanupFunc func() // make a Commit with a single vote containing just the height and a timestamp func makeTestCommit(height int64, timestamp time.Time) *types.Commit { - commitSigs := []*types.CommitSig{{Height: height, Timestamp: timestamp}} - return types.NewCommit(types.BlockID{}, commitSigs) + commitSigs := []types.CommitSig{{ + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: []byte("ValidatorAddress"), + Timestamp: timestamp, + Signature: []byte("Signature"), + }} + return types.NewCommit(height, 0, types.BlockID{}, commitSigs) } func makeTxs(height int64) (txs []types.Tx) { diff --git a/types/block.go b/types/block.go index c9a69a45a..b2fca5f09 100644 --- a/types/block.go +++ b/types/block.go @@ -73,7 +73,7 @@ func (b *Block) ValidateBasic() error { return errors.New("nil LastCommit") } if err := b.LastCommit.ValidateBasic(); err != nil { - return fmt.Errorf("wrong LastCommit") + return fmt.Errorf("wrong LastCommit: %v", err) } } if err := ValidateHash(b.LastCommitHash); err != nil { @@ -434,27 +434,112 @@ func (h *Header) StringIndented(indent string) string { //------------------------------------- -// CommitSig is a vote included in a Commit. -// For now, it is identical to a vote, -// but in the future it will contain fewer fields -// to eliminate the redundancy in commits. -// See https://github.com/tendermint/tendermint/issues/1648. -type CommitSig Vote +// BlockIDFlag indicates which BlockID the signature is for. +type BlockIDFlag byte -// String returns the underlying Vote.String() -func (cs *CommitSig) String() string { - return cs.toVote().String() +const ( + // BlockIDFlagAbsent - no vote was received from a validator. + BlockIDFlagAbsent BlockIDFlag = iota + 1 + // BlockIDFlagCommit - voted for the Commit.BlockID. + BlockIDFlagCommit + // BlockIDFlagNil - voted for nil. + BlockIDFlagNil +) + +// CommitSig is a part of the Vote included in a Commit. +type CommitSig struct { + BlockIDFlag BlockIDFlag `json:"block_id_flag"` + ValidatorAddress Address `json:"validator_address"` + Timestamp time.Time `json:"timestamp"` + Signature []byte `json:"signature"` } -// toVote converts the CommitSig to a vote. -// TODO: deprecate for #1648. Converting to Vote will require -// access to ValidatorSet. -func (cs *CommitSig) toVote() *Vote { - if cs == nil { - return nil +// NewCommitSigForBlock returns new CommitSig with BlockIDFlagCommit. +func NewCommitSigForBlock(signature []byte, valAddr Address, ts time.Time) CommitSig { + return CommitSig{ + BlockIDFlag: BlockIDFlagCommit, + ValidatorAddress: valAddr, + Timestamp: ts, + Signature: signature, } - v := Vote(*cs) - return &v +} + +// NewCommitSigAbsent returns new CommitSig with BlockIDFlagAbsent. Other +// fields are all empty. +func NewCommitSigAbsent() CommitSig { + return CommitSig{ + BlockIDFlag: BlockIDFlagAbsent, + } +} + +// Absent returns true if CommitSig is absent. +func (cs CommitSig) Absent() bool { + return cs.BlockIDFlag == BlockIDFlagAbsent +} + +func (cs CommitSig) String() string { + return fmt.Sprintf("CommitSig{%X by %X on %v @ %s}", + cmn.Fingerprint(cs.Signature), + cmn.Fingerprint(cs.ValidatorAddress), + cs.BlockIDFlag, + CanonicalTime(cs.Timestamp)) +} + +// BlockID returns the Commit's BlockID if CommitSig indicates signing, +// otherwise - empty BlockID. +func (cs CommitSig) BlockID(commitBlockID BlockID) BlockID { + var blockID BlockID + switch cs.BlockIDFlag { + case BlockIDFlagAbsent: + blockID = BlockID{} + case BlockIDFlagCommit: + blockID = commitBlockID + case BlockIDFlagNil: + blockID = BlockID{} + default: + panic(fmt.Sprintf("Unknown BlockIDFlag: %v", cs.BlockIDFlag)) + } + return blockID +} + +// ValidateBasic performs basic validation. +func (cs CommitSig) ValidateBasic() error { + switch cs.BlockIDFlag { + case BlockIDFlagAbsent: + case BlockIDFlagCommit: + case BlockIDFlagNil: + default: + return fmt.Errorf("unknown BlockIDFlag: %v", cs.BlockIDFlag) + } + + switch cs.BlockIDFlag { + case BlockIDFlagAbsent: + if len(cs.ValidatorAddress) != 0 { + return errors.New("validator address is present") + } + if !cs.Timestamp.IsZero() { + return errors.New("time is present") + } + if len(cs.Signature) != 0 { + return errors.New("signature is present") + } + default: + if len(cs.ValidatorAddress) != crypto.AddressSize { + return fmt.Errorf("expected ValidatorAddress size to be %d bytes, got %d bytes", + crypto.AddressSize, + len(cs.ValidatorAddress), + ) + } + // NOTE: Timestamp validation is subtle and handled elsewhere. + if len(cs.Signature) == 0 { + return errors.New("signature is missing") + } + if len(cs.Signature) > MaxSignatureSize { + return fmt.Errorf("signature is too big (max: %d)", MaxSignatureSize) + } + } + + return nil } //------------------------------------- @@ -462,40 +547,40 @@ func (cs *CommitSig) toVote() *Vote { // Commit contains the evidence that a block was committed by a set of validators. // NOTE: Commit is empty for height 1, but never nil. 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 []*CommitSig `json:"precommits"` - - // memoized in first call to corresponding method - // NOTE: can't memoize in constructor because constructor - // isn't used for unmarshaling - height int64 - round int + // NOTE: The signatures are in order of address to preserve the bonded + // ValidatorSet order. + // Any peer with a block can gossip signatures by index with a peer without + // recalculating the active ValidatorSet. + Height int64 `json:"height"` + Round int `json:"round"` + BlockID BlockID `json:"block_id"` + Signatures []CommitSig `json:"signatures"` + + // Memoized in first call to corresponding method. + // NOTE: can't memoize in constructor because constructor isn't used for + // unmarshaling. 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 { +// NewCommit returns a new Commit. +func NewCommit(height int64, round int, blockID BlockID, commitSigs []CommitSig) *Commit { return &Commit{ + Height: height, + Round: round, BlockID: blockID, - Precommits: precommits, + Signatures: commitSigs, } } -// Construct a VoteSet from the Commit and validator set. Panics -// if precommits from the commit can't be added to the voteset. +// CommitToVoteSet constructs a VoteSet from the Commit and validator set. +// Panics if signatures from the commit can't be added to the voteset. // Inverse of VoteSet.MakeCommit(). func CommitToVoteSet(chainID string, commit *Commit, vals *ValidatorSet) *VoteSet { - height, round, typ := commit.Height(), commit.Round(), PrecommitType - voteSet := NewVoteSet(chainID, height, round, typ, vals) - for idx, precommit := range commit.Precommits { - if precommit == nil { - continue + voteSet := NewVoteSet(chainID, commit.Height, commit.Round, PrecommitType, vals) + for idx, commitSig := range commit.Signatures { + if commitSig.Absent() { + continue // OK, some precommits can be missing. } added, err := voteSet.AddVote(commit.GetVote(idx)) if !added || err != nil { @@ -509,21 +594,12 @@ func CommitToVoteSet(chainID string, commit *Commit, vals *ValidatorSet) *VoteSe // Returns nil if the precommit at valIdx is nil. // Panics if valIdx >= commit.Size(). func (commit *Commit) GetVote(valIdx int) *Vote { - commitSig := commit.Precommits[valIdx] - if commitSig == nil { - return nil - } - - // NOTE: this commitSig might be for a nil blockID, - // so we can't just use commit.BlockID here. - // For #1648, CommitSig will need to indicate what BlockID it's for ! - blockID := commitSig.BlockID - commit.memoizeHeightRound() + commitSig := commit.Signatures[valIdx] return &Vote{ Type: PrecommitType, - Height: commit.height, - Round: commit.round, - BlockID: blockID, + Height: commit.Height, + Round: commit.Round, + BlockID: commitSig.BlockID(commit.BlockID), Timestamp: commitSig.Timestamp, ValidatorAddress: commitSig.ValidatorAddress, ValidatorIndex: valIdx, @@ -539,58 +615,42 @@ func (commit *Commit) VoteSignBytes(chainID string, valIdx int) []byte { return commit.GetVote(valIdx).SignBytes(chainID) } -// memoizeHeightRound memoizes the height and round of the commit using -// the first non-nil vote. -// Should be called before any attempt to access `commit.height` or `commit.round`. -func (commit *Commit) memoizeHeightRound() { - if len(commit.Precommits) == 0 { - return - } - if commit.height > 0 { - return - } - for _, precommit := range commit.Precommits { - if precommit != nil { - commit.height = precommit.Height - commit.round = precommit.Round - return - } - } -} - -// Height returns the height of the commit -func (commit *Commit) Height() int64 { - commit.memoizeHeightRound() - return commit.height +// Type returns the vote type of the commit, which is always VoteTypePrecommit +// Implements VoteSetReader. +func (commit *Commit) Type() byte { + return byte(PrecommitType) } -// Round returns the round of the commit -func (commit *Commit) Round() int { - commit.memoizeHeightRound() - return commit.round +// GetHeight returns height of the commit. +// Implements VoteSetReader. +func (commit *Commit) GetHeight() int64 { + return commit.Height } -// Type returns the vote type of the commit, which is always VoteTypePrecommit -func (commit *Commit) Type() byte { - return byte(PrecommitType) +// GetRound returns height of the commit. +// Implements VoteSetReader. +func (commit *Commit) GetRound() int { + return commit.Round } -// Size returns the number of votes in the commit +// Size returns the number of signatures in the commit. +// Implements VoteSetReader. func (commit *Commit) Size() int { if commit == nil { return 0 } - return len(commit.Precommits) + return len(commit.Signatures) } -// BitArray returns a BitArray of which validators voted in this commit +// BitArray returns a BitArray of which validators voted for BlockID or nil in this commit. +// Implements VoteSetReader. func (commit *Commit) BitArray() *cmn.BitArray { if commit.bitArray == nil { - commit.bitArray = cmn.NewBitArray(len(commit.Precommits)) - for i, precommit := range commit.Precommits { + commit.bitArray = cmn.NewBitArray(len(commit.Signatures)) + for i, commitSig := range commit.Signatures { // TODO: need to check the BlockID otherwise we could be counting conflicts, // not just the one with +2/3 ! - commit.bitArray.SetIndex(i, precommit != nil) + commit.bitArray.SetIndex(i, !commitSig.Absent()) } } return commit.bitArray @@ -603,44 +663,35 @@ func (commit *Commit) GetByIndex(valIdx int) *Vote { return commit.GetVote(valIdx) } -// IsCommit returns true if there is at least one vote. +// IsCommit returns true if there is at least one signature. +// Implements VoteSetReader. func (commit *Commit) IsCommit() bool { - return len(commit.Precommits) != 0 + return len(commit.Signatures) != 0 } // ValidateBasic performs basic validation that doesn't involve state data. // Does not actually check the cryptographic signatures. func (commit *Commit) ValidateBasic() error { + if commit.Height < 0 { + return errors.New("negative Height") + } + if commit.Round < 0 { + return errors.New("negative Round") + } + if commit.BlockID.IsZero() { return errors.New("commit cannot be for nil block") } - 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 != PrecommitType { - 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) + if len(commit.Signatures) == 0 { + return errors.New("no signatures in commit") + } + for i, commitSig := range commit.Signatures { + if err := commitSig.ValidateBasic(); err != nil { + return fmt.Errorf("wrong CommitSig #%d: %v", i, err) } } + return nil } @@ -650,9 +701,9 @@ func (commit *Commit) Hash() cmn.HexBytes { return nil } if commit.hash == nil { - bs := make([][]byte, len(commit.Precommits)) - for i, precommit := range commit.Precommits { - bs[i] = cdcEncode(precommit) + bs := make([][]byte, len(commit.Signatures)) + for i, commitSig := range commit.Signatures { + bs[i] = cdcEncode(commitSig) } commit.hash = merkle.SimpleHashFromByteSlices(bs) } @@ -664,18 +715,22 @@ func (commit *Commit) StringIndented(indent string) string { if commit == nil { return "nil-Commit" } - precommitStrings := make([]string, len(commit.Precommits)) - for i, precommit := range commit.Precommits { - precommitStrings[i] = precommit.String() + commitSigStrings := make([]string, len(commit.Signatures)) + for i, commitSig := range commit.Signatures { + commitSigStrings[i] = commitSig.String() } return fmt.Sprintf(`Commit{ +%s Height: %d +%s Round: %d %s BlockID: %v -%s Precommits: +%s Signatures: %s %v %s}#%v`, + indent, commit.Height, + indent, commit.Round, indent, commit.BlockID, indent, - indent, strings.Join(precommitStrings, "\n"+indent+" "), + indent, strings.Join(commitSigStrings, "\n"+indent+" "), indent, commit.hash) } @@ -695,7 +750,6 @@ type SignedHeader struct { // sure to use a Verifier to validate the signatures actually provide a // significantly strong proof for this header's validity. func (sh SignedHeader) ValidateBasic(chainID string) error { - // Make sure the header is consistent with the commit. if sh.Header == nil { return errors.New("signedHeader missing header") @@ -710,9 +764,9 @@ func (sh SignedHeader) ValidateBasic(chainID string) error { sh.ChainID, chainID) } // Check Height. - if sh.Commit.Height() != sh.Height { + if sh.Commit.Height != sh.Height { return fmt.Errorf("signedHeader header and commit height mismatch: %v vs %v", - sh.Height, sh.Commit.Height()) + sh.Height, sh.Commit.Height) } // Check Hash. hhash := sh.Hash() diff --git a/types/block_test.go b/types/block_test.go index 1c5671f86..daac8cdec 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -70,7 +70,7 @@ func TestBlockValidateBasic(t *testing.T) { {"Make Block w/ proposer Addr", func(blk *Block) { blk.ProposerAddress = valSet.GetProposer().Address }, false}, {"Negative Height", func(blk *Block) { blk.Height = -1 }, true}, {"Remove 1/2 the commits", func(blk *Block) { - blk.LastCommit.Precommits = commit.Precommits[:commit.Size()/2] + blk.LastCommit.Signatures = commit.Signatures[:commit.Size()/2] blk.LastCommit.hash = nil // clear hash or change wont be noticed }, true}, {"Remove LastCommitHash", func(blk *Block) { blk.LastCommitHash = []byte("something else") }, true}, @@ -124,7 +124,7 @@ func TestBlockMakePartSetWithEvidence(t *testing.T) { ev := NewMockGoodEvidence(h, 0, valSet.Validators[0].Address) evList := []Evidence{ev} - partSet := MakeBlock(h, []Tx{Tx("Hello World")}, commit, evList).MakePartSet(1024) + partSet := MakeBlock(h, []Tx{Tx("Hello World")}, commit, evList).MakePartSet(512) assert.NotNil(t, partSet) assert.Equal(t, 3, partSet.Total()) } @@ -167,23 +167,29 @@ func TestBlockString(t *testing.T) { } func makeBlockIDRandom() BlockID { - blockHash := make([]byte, tmhash.Size) - partSetHash := make([]byte, tmhash.Size) + var ( + blockHash = make([]byte, tmhash.Size) + partSetHash = make([]byte, tmhash.Size) + ) rand.Read(blockHash) //nolint: gosec rand.Read(partSetHash) //nolint: gosec - blockPartsHeader := PartSetHeader{123, partSetHash} - return BlockID{blockHash, blockPartsHeader} + return BlockID{blockHash, PartSetHeader{123, partSetHash}} } func makeBlockID(hash []byte, partSetSize int, partSetHash []byte) BlockID { + var ( + h = make([]byte, tmhash.Size) + psH = make([]byte, tmhash.Size) + ) + copy(h, hash) + copy(psH, partSetHash) return BlockID{ - Hash: hash, + Hash: h, PartsHeader: PartSetHeader{ Total: partSetSize, - Hash: partSetHash, + Hash: psH, }, } - } var nilBytes []byte @@ -205,8 +211,8 @@ func TestCommit(t *testing.T) { commit, err := MakeCommit(lastID, h-1, 1, voteSet, vals) require.NoError(t, err) - assert.Equal(t, h-1, commit.Height()) - assert.Equal(t, 1, commit.Round()) + assert.Equal(t, h-1, commit.Height) + assert.Equal(t, 1, commit.Round) assert.Equal(t, PrecommitType, SignedMsgType(commit.Type())) if commit.Size() <= 0 { t.Fatalf("commit %v has a zero or negative size: %d", commit, commit.Size()) @@ -226,11 +232,9 @@ func TestCommitValidateBasic(t *testing.T) { expectErr bool }{ {"Random Commit", func(com *Commit) {}, false}, - {"Nil precommit", func(com *Commit) { com.Precommits[0] = nil }, false}, - {"Incorrect signature", func(com *Commit) { com.Precommits[0].Signature = []byte{0} }, false}, - {"Incorrect type", func(com *Commit) { com.Precommits[0].Type = PrevoteType }, true}, - {"Incorrect height", func(com *Commit) { com.Precommits[0].Height = int64(100) }, true}, - {"Incorrect round", func(com *Commit) { com.Precommits[0].Round = 100 }, true}, + {"Incorrect signature", func(com *Commit) { com.Signatures[0].Signature = []byte{0} }, false}, + {"Incorrect height", func(com *Commit) { com.Height = int64(-100) }, true}, + {"Incorrect round", func(com *Commit) { com.Round = -100 }, true}, } for _, tc := range testCases { tc := tc @@ -444,13 +448,13 @@ func TestCommitToVoteSet(t *testing.T) { } } -func TestCommitToVoteSetWithVotesForAnotherBlockOrNilBlock(t *testing.T) { +func TestCommitToVoteSetWithVotesForNilBlock(t *testing.T) { blockID := makeBlockID([]byte("blockhash"), 1000, []byte("partshash")) - blockID2 := makeBlockID([]byte("blockhash2"), 1000, []byte("partshash")) - blockID3 := makeBlockID([]byte("blockhash3"), 10000, []byte("partshash")) - height := int64(3) - round := 1 + const ( + height = int64(3) + round = 0 + ) type commitVoteTest struct { blockIDs []BlockID @@ -460,16 +464,11 @@ func TestCommitToVoteSetWithVotesForAnotherBlockOrNilBlock(t *testing.T) { } testCases := []commitVoteTest{ - {[]BlockID{blockID, blockID2, blockID3}, []int{8, 1, 1}, 10, true}, - {[]BlockID{blockID, blockID2, blockID3}, []int{67, 20, 13}, 100, true}, - {[]BlockID{blockID, blockID2, blockID3}, []int{1, 1, 1}, 3, false}, - {[]BlockID{blockID, blockID2, blockID3}, []int{3, 1, 1}, 5, false}, {[]BlockID{blockID, {}}, []int{67, 33}, 100, true}, - {[]BlockID{blockID, blockID2, {}}, []int{10, 5, 5}, 20, false}, } for _, tc := range testCases { - voteSet, valSet, vals := randVoteSet(height-1, 1, PrecommitType, tc.numValidators, 1) + voteSet, valSet, vals := randVoteSet(height-1, round, PrecommitType, tc.numValidators, 1) vi := 0 for n := range tc.blockIDs { @@ -485,11 +484,14 @@ func TestCommitToVoteSetWithVotesForAnotherBlockOrNilBlock(t *testing.T) { Timestamp: tmtime.Now(), } - _, err := signAddVote(vals[vi], vote, voteSet) + added, err := signAddVote(vals[vi], vote, voteSet) assert.NoError(t, err) + assert.True(t, added) + vi++ } } + if tc.valid { commit := voteSet.MakeCommit() // panics without > 2/3 valid votes assert.NotNil(t, commit) @@ -508,7 +510,7 @@ func TestSignedHeaderValidateBasic(t *testing.T) { h := Header{ Version: version.Consensus{Block: math.MaxInt64, App: math.MaxInt64}, ChainID: chainID, - Height: commit.Height(), + Height: commit.Height, Time: timestamp, LastBlockID: commit.BlockID, LastCommitHash: commit.Hash(), diff --git a/types/errors.go b/types/errors.go index 603ac51d7..d828f6387 100644 --- a/types/errors.go +++ b/types/errors.go @@ -10,9 +10,9 @@ type ( Actual int64 } - // ErrInvalidCommitPrecommits is returned when we encounter a commit where - // the number of precommits doesn't match the number of validators. - ErrInvalidCommitPrecommits struct { + // ErrInvalidCommitSignatures is returned when we encounter a commit where + // the number of signatures doesn't match the number of validators. + ErrInvalidCommitSignatures struct { Expected int Actual int } @@ -29,13 +29,13 @@ func (e ErrInvalidCommitHeight) Error() string { return fmt.Sprintf("Invalid commit -- wrong height: %v vs %v", e.Expected, e.Actual) } -func NewErrInvalidCommitPrecommits(expected, actual int) ErrInvalidCommitPrecommits { - return ErrInvalidCommitPrecommits{ +func NewErrInvalidCommitSignatures(expected, actual int) ErrInvalidCommitSignatures { + return ErrInvalidCommitSignatures{ Expected: expected, Actual: actual, } } -func (e ErrInvalidCommitPrecommits) Error() string { +func (e ErrInvalidCommitSignatures) Error() string { return fmt.Sprintf("Invalid commit -- wrong set size: %v vs %v", e.Expected, e.Actual) } diff --git a/types/validator_set.go b/types/validator_set.go index 4e74ab2e1..d32250744 100644 --- a/types/validator_set.go +++ b/types/validator_set.go @@ -14,18 +14,17 @@ import ( cmn "github.com/tendermint/tendermint/libs/common" ) -// MaxTotalVotingPower - the maximum allowed total voting power. -// It needs to be sufficiently small to, in all cases: -// 1. prevent clipping in incrementProposerPriority() -// 2. let (diff+diffMax-1) not overflow in IncrementProposerPriority() -// (Proof of 1 is tricky, left to the reader). -// It could be higher, but this is sufficiently large for our purposes, -// and leaves room for defensive purposes. -// PriorityWindowSizeFactor - is a constant that when multiplied with the total voting power gives -// the maximum allowed distance between validator priorities. - const ( - MaxTotalVotingPower = int64(math.MaxInt64) / 8 + // MaxTotalVotingPower - the maximum allowed total voting power. + // It needs to be sufficiently small to, in all cases: + // 1. prevent clipping in incrementProposerPriority() + // 2. let (diff+diffMax-1) not overflow in IncrementProposerPriority() + // (Proof of 1 is tricky, left to the reader). + // It could be higher, but this is sufficiently large for our purposes, + // and leaves room for defensive purposes. + MaxTotalVotingPower = int64(math.MaxInt64) / 8 + // PriorityWindowSizeFactor - is a constant that when multiplied with the total voting power gives + // the maximum allowed distance between validator priorities. PriorityWindowSizeFactor = 2 ) @@ -67,12 +66,13 @@ func NewValidatorSet(valz []*Validator) *ValidatorSet { return vals } -// Nil or empty validator sets are invalid. +// IsNilOrEmpty returns true if validator set is nil or empty. func (vals *ValidatorSet) IsNilOrEmpty() bool { return vals == nil || len(vals.Validators) == 0 } -// Increment ProposerPriority and update the proposer on a copy, and return it. +// CopyIncrementProposerPriority increments ProposerPriority and update the +// proposer on a copy, and return it. func (vals *ValidatorSet) CopyIncrementProposerPriority(times int) *ValidatorSet { copy := vals.Copy() copy.IncrementProposerPriority(times) @@ -106,6 +106,7 @@ func (vals *ValidatorSet) IncrementProposerPriority(times int) { vals.Proposer = proposer } +// RescalePriorities ... func (vals *ValidatorSet) RescalePriorities(diffMax int64) { if vals.IsNilOrEmpty() { panic("empty validator set") @@ -177,9 +178,8 @@ func computeMaxMinPriorityDiff(vals *ValidatorSet) int64 { diff := max - min if diff < 0 { return -1 * diff - } else { - return diff } + return diff } func (vals *ValidatorSet) getValWithMostPriority() *Validator { @@ -594,22 +594,21 @@ func (vals *ValidatorSet) UpdateWithChangeSet(changes []*Validator) error { return vals.updateWithChangeSet(changes, true) } -// VerifyCommit verifies that +2/3 of the validator set signed this commit. -func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, height int64, commit *Commit) error { - if vals.Size() != len(commit.Precommits) { - return NewErrInvalidCommitPrecommits(vals.Size(), len(commit.Precommits)) +// VerifyCommit verifies +2/3 of the set had signed the given commit. +func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, + height int64, commit *Commit) error { + + if vals.Size() != len(commit.Signatures) { + return NewErrInvalidCommitSignatures(vals.Size(), len(commit.Signatures)) } if err := vals.verifyCommitBasic(commit, height, blockID); err != nil { return err } talliedVotingPower := int64(0) - for idx, precommit := range commit.Precommits { - // skip absent and nil votes - // NOTE: do we want to check the validity of votes - // for nil? - if precommit == nil { - continue // OK, some precommits can be missing. + for idx, commitSig := range commit.Signatures { + if commitSig.Absent() { + continue // OK, some signatures can be absent. } // The vals and commit have a 1-to-1 correspondance. @@ -617,17 +616,17 @@ func (vals *ValidatorSet) VerifyCommit(chainID string, blockID BlockID, height i val := vals.Validators[idx] // Validate signature. - precommitSignBytes := commit.VoteSignBytes(chainID, idx) - if !val.PubKey.VerifyBytes(precommitSignBytes, precommit.Signature) { - return fmt.Errorf("invalid commit -- invalid signature: %v", precommit) + voteSignBytes := commit.VoteSignBytes(chainID, idx) + if !val.PubKey.VerifyBytes(voteSignBytes, commitSig.Signature) { + return fmt.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature) } - // Good precommit! - if blockID.Equals(precommit.BlockID) { + // Good! + if blockID.Equals(commitSig.BlockID(commit.BlockID)) { talliedVotingPower += val.VotingPower } // else { // It's OK that the BlockID doesn't match. We include stray - // precommits to measure validator availability. + // signatures (~votes for nil) to measure validator availability. // } } @@ -680,40 +679,31 @@ 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 errors.Errorf("blocks don't match - %d vs %d", round, precommit.Round) - } - if precommit.Round != round { - return errors.Errorf("invalid commit -- wrong round: %v vs %v", round, precommit.Round) - } - if precommit.Type != PrecommitType { - return errors.Errorf("invalid commit -- not precommit @ index %v", idx) + for idx, commitSig := range commit.Signatures { + if commitSig.Absent() { + continue // OK, some signatures can be absent. } + // See if this validator is in oldVals. - oldIdx, val := oldVals.GetByAddress(precommit.ValidatorAddress) + oldIdx, val := oldVals.GetByAddress(commitSig.ValidatorAddress) if val == nil || seen[oldIdx] { continue // missing or double vote... } seen[oldIdx] = true // Validate signature. - precommitSignBytes := commit.VoteSignBytes(chainID, idx) - if !val.PubKey.VerifyBytes(precommitSignBytes, precommit.Signature) { - return errors.Errorf("invalid commit -- invalid signature: %v", precommit) + voteSignBytes := commit.VoteSignBytes(chainID, idx) + if !val.PubKey.VerifyBytes(voteSignBytes, commitSig.Signature) { + return errors.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature) } - // Good precommit! - if blockID.Equals(precommit.BlockID) { + // Good! + if blockID.Equals(commitSig.BlockID(commit.BlockID)) { oldVotingPower += val.VotingPower } // else { // It's OK that the BlockID doesn't match. We include stray - // precommits to measure validator availability. + // signatures (~votes for nil) to measure validator availability. // } } @@ -740,31 +730,28 @@ func (vals *ValidatorSet) VerifyCommitTrusting(chainID string, blockID BlockID, } talliedVotingPower := int64(0) - for idx, precommit := range commit.Precommits { - // skip absent and nil votes - // NOTE: do we want to check the validity of votes - // for nil? - if precommit == nil { - continue + for idx, commitSig := range commit.Signatures { + if commitSig.Absent() { + continue // OK, some signatures can be absent. } // We don't know the validators that committed this block, so we have to // check for each vote if its validator is already known. - _, val := vals.GetByAddress(precommit.ValidatorAddress) + _, val := vals.GetByAddress(commitSig.ValidatorAddress) if val != nil { // Validate signature. - precommitSignBytes := commit.VoteSignBytes(chainID, idx) - if !val.PubKey.VerifyBytes(precommitSignBytes, precommit.Signature) { - return fmt.Errorf("invalid commit -- invalid signature: %v", precommit) + voteSignBytes := commit.VoteSignBytes(chainID, idx) + if !val.PubKey.VerifyBytes(voteSignBytes, commitSig.Signature) { + return errors.Errorf("wrong signature (#%d): %X", idx, commitSig.Signature) } - // Good precommit! - if blockID.Equals(precommit.BlockID) { + // Good! + if blockID.Equals(commitSig.BlockID(commit.BlockID)) { talliedVotingPower += val.VotingPower } // else { // It's OK that the BlockID doesn't match. We include stray - // precommits to measure validator availability. + // signatures (~votes for nil) to measure validator availability. // } } } @@ -782,8 +769,8 @@ func (vals *ValidatorSet) verifyCommitBasic(commit *Commit, height int64, blockI if err := commit.ValidateBasic(); err != nil { return err } - if height != commit.Height() { - return NewErrInvalidCommitHeight(height, commit.Height()) + if height != commit.Height { + return NewErrInvalidCommitHeight(height, commit.Height) } if !blockID.Equals(commit.BlockID) { return fmt.Errorf("invalid commit -- wrong block ID: want %v, got %v", @@ -797,7 +784,6 @@ func (vals *ValidatorSet) verifyCommitBasic(commit *Commit, height int64, blockI // IsErrTooMuchChange returns true if err is related to changes in validator // set exceeding max limit. -// TODO: remove func IsErrTooMuchChange(err error) bool { _, ok := errors.Cause(err).(ErrTooMuchChange) return ok @@ -819,7 +805,7 @@ func (vals *ValidatorSet) String() string { return vals.StringIndented("") } -// String +// StringIndented returns an intended string representation of ValidatorSet. func (vals *ValidatorSet) StringIndented(indent string) string { if vals == nil { return "nil-ValidatorSet" @@ -844,7 +830,7 @@ func (vals *ValidatorSet) StringIndented(indent string) string { //------------------------------------- // Implements sort for sorting validators by address. -// Sort validators by address. +// ValidatorsByAddress is used to sort validators by address. type ValidatorsByAddress []*Validator func (valz ValidatorsByAddress) Len() int { diff --git a/types/validator_set_test.go b/types/validator_set_test.go index 455e15801..dab9dde18 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -602,9 +602,12 @@ func TestValidatorSetVerifyCommit(t *testing.T) { v1 := NewValidator(pubKey, 1000) vset := NewValidatorSet([]*Validator{v1}) - chainID := "mychainID" - blockID := BlockID{Hash: []byte("hello")} - height := int64(5) + // good + var ( + chainID = "mychainID" + blockID = makeBlockIDRandom() + height = int64(5) + ) vote := &Vote{ ValidatorAddress: v1.Address, ValidatorIndex: 0, @@ -617,12 +620,15 @@ func TestValidatorSetVerifyCommit(t *testing.T) { sig, err := privKey.Sign(vote.SignBytes(chainID)) assert.NoError(t, err) vote.Signature = sig - commit := NewCommit(blockID, []*CommitSig{vote.CommitSig()}) - - badChainID := "notmychainID" - badBlockID := BlockID{Hash: []byte("goodbye")} - badHeight := height + 1 - badCommit := NewCommit(blockID, []*CommitSig{nil}) + commit := NewCommit(vote.Height, vote.Round, blockID, []CommitSig{vote.CommitSig()}) + + // bad + var ( + badChainID = "notmychainID" + badBlockID = BlockID{Hash: []byte("goodbye")} + badHeight = height + 1 + badCommit = NewCommit(badHeight, 0, blockID, []CommitSig{{BlockIDFlag: BlockIDFlagAbsent}}) + ) // test some error cases // TODO: test more cases! diff --git a/types/vote.go b/types/vote.go index 9e19841a6..98a5fbf19 100644 --- a/types/vote.go +++ b/types/vote.go @@ -57,13 +57,27 @@ type Vote struct { } // CommitSig converts the Vote to a CommitSig. -// If the Vote is nil, the CommitSig will be nil. -func (vote *Vote) CommitSig() *CommitSig { +func (vote *Vote) CommitSig() CommitSig { if vote == nil { - return nil + return NewCommitSigAbsent() + } + + var blockIDFlag BlockIDFlag + switch { + case vote.BlockID.IsComplete(): + blockIDFlag = BlockIDFlagCommit + case vote.BlockID.IsZero(): + blockIDFlag = BlockIDFlagNil + default: + panic(fmt.Sprintf("Invalid vote %v - expected BlockID to be either empty or complete", vote)) + } + + return CommitSig{ + BlockIDFlag: blockIDFlag, + ValidatorAddress: vote.ValidatorAddress, + Timestamp: vote.Timestamp, + Signature: vote.Signature, } - cs := CommitSig(*vote) - return &cs } func (vote *Vote) SignBytes(chainID string) []byte { @@ -83,6 +97,7 @@ func (vote *Vote) String() string { if vote == nil { return nilVoteStr } + var typeString string switch vote.Type { case PrevoteType: diff --git a/types/vote_set.go b/types/vote_set.go index f296e82e1..af5c1cf3f 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -98,20 +98,23 @@ func (voteSet *VoteSet) ChainID() string { return voteSet.chainID } -func (voteSet *VoteSet) Height() int64 { +// Implements VoteSetReader. +func (voteSet *VoteSet) GetHeight() int64 { if voteSet == nil { return 0 } return voteSet.height } -func (voteSet *VoteSet) Round() int { +// Implements VoteSetReader. +func (voteSet *VoteSet) GetRound() int { if voteSet == nil { return -1 } return voteSet.round } +// Implements VoteSetReader. func (voteSet *VoteSet) Type() byte { if voteSet == nil { return 0x00 @@ -119,6 +122,7 @@ func (voteSet *VoteSet) Type() byte { return byte(voteSet.type_) } +// Implements VoteSetReader. func (voteSet *VoteSet) Size() int { if voteSet == nil { return 0 @@ -335,6 +339,7 @@ func (voteSet *VoteSet) SetPeerMaj23(peerID P2PID, blockID BlockID) error { return nil } +// Implements VoteSetReader. func (voteSet *VoteSet) BitArray() *cmn.BitArray { if voteSet == nil { return nil @@ -358,6 +363,7 @@ func (voteSet *VoteSet) BitArrayByBlockID(blockID BlockID) *cmn.BitArray { } // NOTE: if validator has conflicting votes, returns "canonical" vote +// Implements VoteSetReader. func (voteSet *VoteSet) GetByIndex(valIndex int) *Vote { if voteSet == nil { return nil @@ -389,6 +395,7 @@ func (voteSet *VoteSet) HasTwoThirdsMajority() bool { return voteSet.maj23 != nil } +// Implements VoteSetReader. func (voteSet *VoteSet) IsCommit() bool { if voteSet == nil { return false @@ -556,11 +563,12 @@ func (voteSet *VoteSet) MakeCommit() *Commit { } // For every validator, get the precommit - commitSigs := make([]*CommitSig, len(voteSet.votes)) + commitSigs := make([]CommitSig, len(voteSet.votes)) for i, v := range voteSet.votes { commitSigs[i] = v.CommitSig() } - return NewCommit(*voteSet.maj23, commitSigs) + + return NewCommit(voteSet.GetHeight(), voteSet.GetRound(), *voteSet.maj23, commitSigs) } //-------------------------------------------------------------------------------- @@ -607,8 +615,8 @@ func (vs *blockVotes) getByIndex(index int) *Vote { // Common interface between *consensus.VoteSet and types.Commit type VoteSetReader interface { - Height() int64 - Round() int + GetHeight() int64 + GetRound() int Type() byte Size() int BitArray() *cmn.BitArray diff --git a/types/vote_set_test.go b/types/vote_set_test.go index d43482020..45ad80f36 100644 --- a/types/vote_set_test.go +++ b/types/vote_set_test.go @@ -522,16 +522,27 @@ func TestMakeCommit(t *testing.T) { } } + // The 9th voted for nil. + { + addr := privValidators[8].GetPubKey().Address() + vote := withValidator(voteProto, addr, 8) + vote.BlockID = BlockID{} + + _, err := signAddVote(privValidators[8], vote, voteSet) + if err != nil { + t.Error(err) + } + } + commit := voteSet.MakeCommit() // Commit should have 10 elements - if len(commit.Precommits) != 10 { - t.Errorf("commit Precommits should have the same number of precommits as validators") + if len(commit.Signatures) != 10 { + t.Errorf("expected commit to include %d elems, got %d", 10, len(commit.Signatures)) } - // Ensure that Commit precommits are ordered. + // Ensure that Commit is good. if err := commit.ValidateBasic(); err != nil { t.Errorf("error in Commit.ValidateBasic(): %v", err) } - } diff --git a/types/vote_test.go b/types/vote_test.go index 6c2379feb..40a9d650a 100644 --- a/types/vote_test.go +++ b/types/vote_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - amino "github.com/tendermint/go-amino" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/tmhash" @@ -45,19 +44,6 @@ func exampleVote(t byte) *Vote { } } -// Ensure that Vote and CommitSig have the same encoding. -// This ensures using CommitSig isn't a breaking change. -// This test will fail and can be removed once CommitSig contains only sigs and -// timestamps. -func TestVoteEncoding(t *testing.T) { - vote := examplePrecommit() - commitSig := vote.CommitSig() - cdc := amino.NewCodec() - bz1 := cdc.MustMarshalBinaryBare(vote) - bz2 := cdc.MustMarshalBinaryBare(commitSig) - assert.Equal(t, bz1, bz2) -} - func TestVoteSignable(t *testing.T) { vote := examplePrecommit() signBytes := vote.SignBytes("test_chain_id")