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
2026-01-06 05:25:35 +00:00 · 2019-11-26 14:10:38 +04:00
parent fb8b00f1d8
commit ad715fe966
30 changed files with 531 additions and 425 deletions
--- 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
--- 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)
--- 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
--- 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)
 		}
 	}

--- 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)
--- 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