From 6c2a96e86741426777b636cd4607eca25fc7d02b Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 7 Feb 2019 10:31:36 -0500 Subject: [PATCH 1/6] fix log in unsafe_reset --- cmd/tendermint/commands/reset_priv_validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/tendermint/commands/reset_priv_validator.go b/cmd/tendermint/commands/reset_priv_validator.go index 122c2a72524..055a76c5160 100644 --- a/cmd/tendermint/commands/reset_priv_validator.go +++ b/cmd/tendermint/commands/reset_priv_validator.go @@ -61,7 +61,7 @@ func resetFilePV(privValKeyFile, privValStateFile string, logger log.Logger) { } else { pv := privval.GenFilePV(privValKeyFile, privValStateFile) pv.Save() - logger.Info("Generated private validator file", "file", "keyFile", privValKeyFile, + logger.Info("Generated private validator file", "keyFile", privValKeyFile, "stateFile", privValStateFile) } } From 4d64b6499a9df7d945a1c78a673071b917573fef Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 6 Feb 2019 17:59:43 -0500 Subject: [PATCH 2/6] types.NewCommit --- types/block.go | 8 ++++++++ types/validator_set_test.go | 10 ++-------- types/vote_set.go | 5 +---- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/types/block.go b/types/block.go index ec09fd449fb..103ebebdfc6 100644 --- a/types/block.go +++ b/types/block.go @@ -516,6 +516,14 @@ type Commit struct { bitArray *cmn.BitArray } +// NewCommit returns a new Commit with the given blockID and precommits. +func NewCommit(blockID BlockID, precommits []*CommitSig) *Commit { + return &Commit{ + BlockID: blockID, + Precommits: precommits, + } +} + // VoteSignBytes constructs the SignBytes for the given CommitSig. // The only unique part of the SignBytes is the Timestamp - all other fields // signed over are otherwise the same for all validators. diff --git a/types/validator_set_test.go b/types/validator_set_test.go index 72b2f66133c..ca1f74f0c4e 100644 --- a/types/validator_set_test.go +++ b/types/validator_set_test.go @@ -563,18 +563,12 @@ func TestValidatorSetVerifyCommit(t *testing.T) { sig, err := privKey.Sign(vote.SignBytes(chainID)) assert.NoError(t, err) vote.Signature = sig - commit := &Commit{ - BlockID: blockID, - Precommits: []*CommitSig{vote.CommitSig()}, - } + commit := NewCommit(blockID, []*CommitSig{vote.CommitSig()}) badChainID := "notmychainID" badBlockID := BlockID{Hash: []byte("goodbye")} badHeight := height + 1 - badCommit := &Commit{ - BlockID: blockID, - Precommits: []*CommitSig{nil}, - } + badCommit := NewCommit(blockID, []*CommitSig{nil}) // test some error cases // TODO: test more cases! diff --git a/types/vote_set.go b/types/vote_set.go index 14930da4a42..1cd0f228191 100644 --- a/types/vote_set.go +++ b/types/vote_set.go @@ -545,10 +545,7 @@ func (voteSet *VoteSet) MakeCommit() *Commit { for i, v := range voteSet.votes { commitSigs[i] = v.CommitSig() } - return &Commit{ - BlockID: *voteSet.maj23, - Precommits: commitSigs, - } + return NewCommit(*voteSet.maj23, commitSigs) } //-------------------------------------------------------------------------------- From de308d17861181d157dbfd8b1c1ee9c7e8a9fa6d Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 6 Feb 2019 18:00:14 -0500 Subject: [PATCH 3/6] use types.NewCommit everywhere --- blockchain/reactor_test.go | 4 ++-- blockchain/store_test.go | 7 ++----- consensus/replay_test.go | 6 ++---- consensus/state.go | 2 +- consensus/types/round_state_test.go | 7 ++----- lite/helpers.go | 8 ++------ lite/proxy/validate_test.go | 8 ++++---- node/node_test.go | 2 +- state/execution_test.go | 4 ++-- 9 files changed, 18 insertions(+), 30 deletions(-) diff --git a/blockchain/reactor_test.go b/blockchain/reactor_test.go index 138e16222d3..1e8666f12e3 100644 --- a/blockchain/reactor_test.go +++ b/blockchain/reactor_test.go @@ -95,13 +95,13 @@ func newBlockchainReactor(logger log.Logger, genDoc *types.GenesisDoc, privVals // let's add some blocks in for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ { - lastCommit := &types.Commit{} + lastCommit := types.NewCommit(types.BlockID{}, nil) if blockHeight > 1 { lastBlockMeta := blockStore.LoadBlockMeta(blockHeight - 1) lastBlock := blockStore.LoadBlock(blockHeight - 1) vote := makeVote(&lastBlock.Header, lastBlockMeta.BlockID, state.Validators, privVals[0]).CommitSig() - lastCommit = &types.Commit{Precommits: []*types.CommitSig{vote}, BlockID: lastBlockMeta.BlockID} + lastCommit = types.NewCommit(lastBlockMeta.BlockID, []*types.CommitSig{vote}) } thisBlock := makeBlock(blockHeight, state, lastCommit) diff --git a/blockchain/store_test.go b/blockchain/store_test.go index 9abc210b108..931faf6a719 100644 --- a/blockchain/store_test.go +++ b/blockchain/store_test.go @@ -23,11 +23,8 @@ import ( // make a Commit with a single vote containing just the height and a timestamp func makeTestCommit(height int64, timestamp time.Time) *types.Commit { - return &types.Commit{ - Precommits: []*types.CommitSig{ - {Height: height, Timestamp: timestamp}, - }, - } + commitSigs := []*types.CommitSig{{Height: height, Timestamp: timestamp}} + return types.NewCommit(types.BlockID{}, commitSigs) } func makeStateAndBlockStore(logger log.Logger) (sm.State, *BlockStore) { diff --git a/consensus/replay_test.go b/consensus/replay_test.go index e7269254c49..297c13c3b44 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -537,10 +537,8 @@ func makeBlockchainFromWAL(wal WAL) ([]*types.Block, []*types.Commit, error) { } case *types.Vote: if p.Type == types.PrecommitType { - thisBlockCommit = &types.Commit{ - BlockID: p.BlockID, - Precommits: []*types.CommitSig{p.CommitSig()}, - } + commitSigs := []*types.CommitSig{p.CommitSig()} + thisBlockCommit = types.NewCommit(p.BlockID, commitSigs) } } } diff --git a/consensus/state.go b/consensus/state.go index c8185d46e4d..68ee56839be 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -954,7 +954,7 @@ func (cs *ConsensusState) createProposalBlock() (block *types.Block, blockParts if cs.Height == 1 { // We're creating a proposal for the first block. // The commit is empty, but not nil. - commit = &types.Commit{} + commit = types.NewCommit(types.BlockID{}, nil) } else if cs.LastCommit.HasTwoThirdsMajority() { // Make the commit from LastCommit commit = cs.LastCommit.MakeCommit() diff --git a/consensus/types/round_state_test.go b/consensus/types/round_state_test.go index cb16f939add..a9bc8e14b20 100644 --- a/consensus/types/round_state_test.go +++ b/consensus/types/round_state_test.go @@ -53,11 +53,8 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) { Data: types.Data{ Txs: txs, }, - Evidence: types.EvidenceData{}, - LastCommit: &types.Commit{ - BlockID: blockID, - Precommits: precommits, - }, + Evidence: types.EvidenceData{}, + LastCommit: types.NewCommit(blockID, precommits), } parts := block.MakePartSet(4096) // Random Proposal diff --git a/lite/helpers.go b/lite/helpers.go index 6b18b35141b..119797f36fe 100644 --- a/lite/helpers.go +++ b/lite/helpers.go @@ -80,12 +80,8 @@ func (pkz privKeys) signHeader(header *types.Header, first, last int) *types.Com vote := makeVote(header, vset, pkz[i]) commitSigs[vote.ValidatorIndex] = vote.CommitSig() } - - res := &types.Commit{ - BlockID: types.BlockID{Hash: header.Hash()}, - Precommits: commitSigs, - } - return res + blockID := types.BlockID{Hash: header.Hash()} + return types.NewCommit(blockID, commitSigs) } func makeVote(header *types.Header, valset *types.ValidatorSet, key crypto.PrivKey) *types.Vote { diff --git a/lite/proxy/validate_test.go b/lite/proxy/validate_test.go index 1ce4d667e33..bf61105b468 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.Commit{BlockID: types.BlockID{Hash: []byte("0xDEADBEEF")}}, + Commit: types.NewCommit(types.BlockID{Hash: []byte("0xDEADBEEF")}, nil), }, wantErr: "Data hash doesn't match header", }, @@ -81,7 +81,7 @@ func TestValidateBlock(t *testing.T) { }, signedHeader: types.SignedHeader{ Header: &types.Header{Height: 11}, - Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}}, + Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}), }, }, // End Header.Data hash mismatch test @@ -169,7 +169,7 @@ func TestValidateBlockMeta(t *testing.T) { ValidatorsHash: []byte("Tendermint"), Time: testTime2, }, - Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}}, + Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}), }, wantErr: "Headers don't match", }, @@ -188,7 +188,7 @@ func TestValidateBlockMeta(t *testing.T) { ValidatorsHash: []byte("Tendermint-x"), Time: testTime2, }, - Commit: &types.Commit{BlockID: types.BlockID{Hash: []byte("DEADBEEF")}}, + Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}), }, wantErr: "Headers don't match", }, diff --git a/node/node_test.go b/node/node_test.go index 3218c83273f..ce1c40293fb 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -261,7 +261,7 @@ func TestCreateProposalBlock(t *testing.T) { evidencePool, ) - commit := &types.Commit{} + commit := types.NewCommit(types.BlockID{}, nil) block, _ := blockExec.CreateProposalBlock( height, state, commit, diff --git a/state/execution_test.go b/state/execution_test.go index 041fb558e46..2fddb350716 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -79,7 +79,7 @@ func TestBeginBlockValidators(t *testing.T) { } for _, tc := range testCases { - lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: tc.lastCommitPrecommits} + lastCommit := types.NewCommit(prevBlockID, tc.lastCommitPrecommits) // block for height 2 block, _ := state.MakeBlock(2, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address) @@ -139,7 +139,7 @@ func TestBeginBlockByzantineValidators(t *testing.T) { commitSig0 := (&types.Vote{ValidatorIndex: 0, Timestamp: now, Type: types.PrecommitType}).CommitSig() commitSig1 := (&types.Vote{ValidatorIndex: 1, Timestamp: now}).CommitSig() commitSigs := []*types.CommitSig{commitSig0, commitSig1} - lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: commitSigs} + lastCommit := types.NewCommit(prevBlockID, commitSigs) for _, tc := range testCases { block, _ := state.MakeBlock(10, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address) From d80f819a20fcdf55f83f138e12a359ad62db7807 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 7 Feb 2019 10:31:19 -0500 Subject: [PATCH 4/6] memoize height and round in constructor --- types/block.go | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/types/block.go b/types/block.go index 103ebebdfc6..e51bd6dc6db 100644 --- a/types/block.go +++ b/types/block.go @@ -509,18 +509,26 @@ type Commit struct { BlockID BlockID `json:"block_id"` Precommits []*CommitSig `json:"precommits"` - // Volatile - height int64 - round int + // memoized in constructor from Precommits + height int64 + round int + + // memoized in first call to corresponding method 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 { + height, round := firstHeightRound(precommits) return &Commit{ BlockID: blockID, Precommits: precommits, + + height: height, + round: round, } } @@ -528,25 +536,23 @@ func NewCommit(blockID BlockID, precommits []*CommitSig) *Commit { // The only unique part of the SignBytes is the Timestamp - all other fields // signed over are otherwise the same for all validators. func (commit *Commit) VoteSignBytes(chainID string, cs *CommitSig) []byte { - return cs.toVote().SignBytes(chainID) + return commit.ToVote(cs).SignBytes(chainID) } -// memoizeHeightRound memoizes the height and round of the commit using -// the first non-nil vote. -func (commit *Commit) memoizeHeightRound() { - if len(commit.Precommits) == 0 { +// firstHeightRound returns the height and round from the first non-nil precommit. +func firstHeightRound(precommits []*CommitSig) (height int64, round int) { + if len(precommits) == 0 { return } - if commit.height > 0 { - return - } - for _, precommit := range commit.Precommits { + for _, precommit := range precommits { if precommit != nil { - commit.height = precommit.Height - commit.round = precommit.Round - return + return precommit.Height, precommit.Round } } + // All precommits were nil. + // This should not happen in practice, + // but we try it in tests. + return } // ToVote converts a CommitSig to a Vote. @@ -560,19 +566,11 @@ func (commit *Commit) ToVote(cs *CommitSig) *Vote { // Height returns the height of the commit func (commit *Commit) Height() int64 { - if len(commit.Precommits) == 0 { - return 0 - } - commit.memoizeHeightRound() return commit.height } // Round returns the round of the commit func (commit *Commit) Round() int { - if len(commit.Precommits) == 0 { - return 0 - } - commit.memoizeHeightRound() return commit.round } From 4758ce9ff29762cd4d73fbd351473b442b7da144 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 7 Feb 2019 18:08:07 -0500 Subject: [PATCH 5/6] notes about deprecating toVote --- types/block.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/types/block.go b/types/block.go index e51bd6dc6db..8c300cbdb1c 100644 --- a/types/block.go +++ b/types/block.go @@ -490,8 +490,8 @@ func (cs *CommitSig) String() string { } // toVote converts the CommitSig to a vote. -// Once CommitSig has fewer fields than vote, -// converting to a Vote will require more information. +// TODO: deprecate for #1648. Converting to Vote will require +// access to ValidatorSet. func (cs *CommitSig) toVote() *Vote { if cs == nil { return nil @@ -557,10 +557,9 @@ func firstHeightRound(precommits []*CommitSig) (height int64, round int) { // ToVote converts a CommitSig to a Vote. // If the CommitSig is nil, the Vote will be nil. -// When CommitSig is reduced to contain fewer fields, -// this will need access to the ValidatorSet to properly -// reconstruct the vote. func (commit *Commit) ToVote(cs *CommitSig) *Vote { + // TODO: use commit.validatorSet to reconstruct vote + // and deprecate .toVote return cs.toVote() } @@ -601,9 +600,10 @@ func (commit *Commit) BitArray() *cmn.BitArray { } // GetByIndex returns the vote corresponding to a given validator index. +// Panics if `index >= commit.Size()`. // Implements VoteSetReader. func (commit *Commit) GetByIndex(index int) *Vote { - return commit.Precommits[index].toVote() + return commit.ToVote(commit.Precommits[index]) } // IsCommit returns true if there is at least one vote. From 866dd4365c845b5b669dd8ed5d9c0291f6dc1d15 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 7 Feb 2019 18:31:29 -0500 Subject: [PATCH 6/6] bring back memoizeHeightRound --- lite/proxy/validate_test.go | 6 +++--- types/block.go | 34 +++++++++++++++++----------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lite/proxy/validate_test.go b/lite/proxy/validate_test.go index bf61105b468..dce177d7ba2 100644 --- a/lite/proxy/validate_test.go +++ b/lite/proxy/validate_test.go @@ -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")}), + Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil), }, }, // End Header.Data hash mismatch test @@ -169,7 +169,7 @@ func TestValidateBlockMeta(t *testing.T) { ValidatorsHash: []byte("Tendermint"), Time: testTime2, }, - Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}), + Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil), }, wantErr: "Headers don't match", }, @@ -188,7 +188,7 @@ func TestValidateBlockMeta(t *testing.T) { ValidatorsHash: []byte("Tendermint-x"), Time: testTime2, }, - Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}), + Commit: types.NewCommit(types.BlockID{Hash: []byte("DEADBEEF")}, nil), }, wantErr: "Headers don't match", }, diff --git a/types/block.go b/types/block.go index 8c300cbdb1c..bcaf0725c78 100644 --- a/types/block.go +++ b/types/block.go @@ -509,11 +509,11 @@ type Commit struct { BlockID BlockID `json:"block_id"` Precommits []*CommitSig `json:"precommits"` - // memoized in constructor from Precommits - height int64 - round int - // memoized in first call to corresponding method + // NOTE: can't memoize in constructor because constructor + // isn't used for unmarshaling + height int64 + round int hash cmn.HexBytes bitArray *cmn.BitArray } @@ -522,13 +522,9 @@ type Commit struct { // TODO: memoize ValidatorSet in constructor so votes can be easily reconstructed // from CommitSig after #1648. func NewCommit(blockID BlockID, precommits []*CommitSig) *Commit { - height, round := firstHeightRound(precommits) return &Commit{ BlockID: blockID, Precommits: precommits, - - height: height, - round: round, } } @@ -539,20 +535,22 @@ func (commit *Commit) VoteSignBytes(chainID string, cs *CommitSig) []byte { return commit.ToVote(cs).SignBytes(chainID) } -// firstHeightRound returns the height and round from the first non-nil precommit. -func firstHeightRound(precommits []*CommitSig) (height int64, round int) { - if len(precommits) == 0 { +// memoizeHeightRound memoizes the height and round of the commit using +// the first non-nil vote. +func (commit *Commit) memoizeHeightRound() { + if len(commit.Precommits) == 0 { return } - for _, precommit := range precommits { + if commit.height > 0 { + return + } + for _, precommit := range commit.Precommits { if precommit != nil { - return precommit.Height, precommit.Round + commit.height = precommit.Height + commit.round = precommit.Round + return } } - // All precommits were nil. - // This should not happen in practice, - // but we try it in tests. - return } // ToVote converts a CommitSig to a Vote. @@ -565,11 +563,13 @@ func (commit *Commit) ToVote(cs *CommitSig) *Vote { // Height returns the height of the commit func (commit *Commit) Height() int64 { + commit.memoizeHeightRound() return commit.height } // Round returns the round of the commit func (commit *Commit) Round() int { + commit.memoizeHeightRound() return commit.round }