validation shimmed into place for switch to consensus

internal/consensus/reactor_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/tendermint/tendermint/internal/test/factory"
 	"github.com/tendermint/tendermint/libs/log"
 	tmcons "github.com/tendermint/tendermint/proto/tendermint/consensus"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	"github.com/tendermint/tendermint/types"
 )
 
@@ -600,6 +601,106 @@ func TestReactorCreatesBlockWhenEmptyBlocksFalse(t *testing.T) {
 	wg.Wait()
 }
 
+func TestSwitchToConsensusVoteExtensions(t *testing.T) {
+	for _, testCase := range []struct {
+		name                  string
+		storedHeight          int64
+		initialRequiredHeight int64
+		includeExtensions     bool
+		shouldPanic           bool
+	}{
+		{
+			name:                  "no vote extensions but not required",
+			initialRequiredHeight: 0,
+			storedHeight:          2,
+			includeExtensions:     false,
+			shouldPanic:           false,
+		},
+		{
+			name:                  "no vote extensions but required this height",
+			initialRequiredHeight: 2,
+			storedHeight:          2,
+			includeExtensions:     false,
+			shouldPanic:           true,
+		},
+		{
+			name:                  "no vote extensions and required in future",
+			initialRequiredHeight: 3,
+			storedHeight:          2,
+			includeExtensions:     false,
+			shouldPanic:           false,
+		},
+		{
+			name:                  "no vote extensions and required previous height",
+			initialRequiredHeight: 1,
+			storedHeight:          2,
+			includeExtensions:     false,
+			shouldPanic:           true,
+		},
+		{
+			name:                  "vote extensions and required previous height",
+			initialRequiredHeight: 1,
+			storedHeight:          2,
+			includeExtensions:     true,
+			shouldPanic:           false,
+		},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+			defer cancel()
+			cs, vs := makeState(ctx, t, makeStateArgs{validators: 1})
+			validator := vs[0]
+			validator.Height = testCase.storedHeight
+
+			cs.state.LastBlockHeight = testCase.storedHeight
+			cs.state.LastValidators = cs.state.Validators.Copy()
+			cs.state.ConsensusParams.Vote.ExtensionRequireHeight = testCase.initialRequiredHeight
+
+			propBlock, err := cs.createProposalBlock(ctx)
+			require.NoError(t, err)
+
+			// Consensus is preparing to do the next height after the stored height.
+			cs.Height = testCase.storedHeight + 1
+			propBlock.Height = testCase.storedHeight
+			blockParts, err := propBlock.MakePartSet(types.BlockPartSizeBytes)
+			require.NoError(t, err)
+
+			voteSet := types.NewVoteSet(cs.state.ChainID, testCase.storedHeight, 0, tmproto.PrecommitType, cs.state.Validators)
+			signedVote := signVote(ctx, t, validator, tmproto.PrecommitType, cs.state.ChainID, types.BlockID{
+				Hash:          propBlock.Hash(),
+				PartSetHeader: blockParts.Header(),
+			})
+
+			if !testCase.includeExtensions {
+				signedVote.Extension = nil
+				signedVote.ExtensionSignature = nil
+			}
+
+			added, err := voteSet.AddVote(signedVote)
+			require.NoError(t, err)
+			require.True(t, added)
+			cs.blockStore.SaveBlock(propBlock, blockParts, voteSet.MakeExtendedCommit())
+			reactor := NewReactor(
+				log.NewNopLogger(),
+				cs,
+				nil,
+				nil,
+				cs.eventBus,
+				true,
+				NopMetrics(),
+			)
+
+			if testCase.shouldPanic {
+				assert.Panics(t, func() {
+					reactor.SwitchToConsensus(ctx, cs.state, false)
+				})
+			} else {
+				reactor.SwitchToConsensus(ctx, cs.state, false)
+			}
+		})
+	}
+}
+
 func TestReactorRecordsVotesAndBlockParts(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()

internal/consensus/state.go

@@ -703,6 +703,13 @@ func (cs *State) reconstructLastCommit(state sm.State) {
 		))
 	}
 
+	requireHeight := cs.state.ConsensusParams.Vote.ExtensionRequireHeight
+	if requireHeight != 0 && extCommit.Height >= requireHeight {
+		if err := extCommit.EnsureExtensions(); err != nil {
+			panic(fmt.Sprintf("failed to reconstruct last commit; invalid vote extensions: %v", err))
+		}
+	}
+
 	lastPrecommits := extCommit.ToVoteSet(state.ChainID, state.LastValidators)
 	if !lastPrecommits.HasTwoThirdsMajority() {
 		panic("failed to reconstruct last commit; does not have +2/3 maj")
@@ -2350,7 +2357,7 @@ func (cs *State) addVote(
 		// will consider the vote valid even if the extension is absent.
 		// VerifyVoteExtension will not be called in this case if the extension
 		// is absent.
-		err := vote.ValidateExtension()
+		err := vote.EnsureExtension()
 		if err == nil {
 			_, val := cs.state.Validators.GetByIndex(vote.ValidatorIndex)
 			err = vote.VerifyWithExtension(cs.state.ChainID, val.PubKey)

types/block.go

@@ -757,9 +757,6 @@ func (ecs ExtendedCommitSig) ValidateBasic() error {
 		if len(ecs.Extension) > MaxVoteExtensionSize {
 			return fmt.Errorf("vote extension is too big (max: %d)", MaxVoteExtensionSize)
 		}
-		if len(ecs.ExtensionSignature) == 0 {
-			return errors.New("vote extension signature is missing")
-		}
 		if len(ecs.ExtensionSignature) > MaxSignatureSize {
 			return fmt.Errorf("vote extension signature is too big (max: %d)", MaxSignatureSize)
 		}
@@ -777,6 +774,13 @@ func (ecs ExtendedCommitSig) ValidateBasic() error {
 	return nil
 }
 
+func (ecs ExtendedCommitSig) ValidateExtension() error {
+	if len(ecs.ExtensionSignature) == 0 {
+		return errors.New("vote extension signature is missing")
+	}
+	return nil
+}
+
 // ToProto converts the ExtendedCommitSig to its Protobuf representation.
 func (ecs *ExtendedCommitSig) ToProto() *tmproto.ExtendedCommitSig {
 	if ecs == nil {
@@ -1017,7 +1021,6 @@ func (ec *ExtendedCommit) Clone() *ExtendedCommit {
 // Panics if signatures from the commit can't be added to the voteset.
 // Inverse of VoteSet.MakeExtendedCommit().
 func (ec *ExtendedCommit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet {
-	requireExtensions := false
 	voteSet := NewVoteSet(chainID, ec.Height, ec.Round, tmproto.PrecommitType, vals)
 	for idx, ecs := range ec.ExtendedSignatures {
 		if ecs.BlockIDFlag == BlockIDFlagAbsent {
@@ -1027,11 +1030,6 @@ func (ec *ExtendedCommit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet
 		if err := vote.ValidateBasic(); err != nil {
 			panic(fmt.Errorf("failed to validate vote reconstructed from LastCommit: %w", err))
 		}
-		if requireExtensions {
-			if err := vote.ValidateExtension(); err != nil {
-				panic(fmt.Errorf("failed to validate vote extensions reconstructed from LastCommit: %w", err))
-			}
-		}
 		added, err := voteSet.AddVote(vote)
 		if !added || err != nil {
 			panic(fmt.Errorf("failed to reconstruct vote set from extended commit: %w", err))
@@ -1040,6 +1038,22 @@ func (ec *ExtendedCommit) ToVoteSet(chainID string, vals *ValidatorSet) *VoteSet
 	return voteSet
 }
 
+// TODO Comment
+// this should probably also verify the signature
+// probably want to change to just verify when present.
+func (ec *ExtendedCommit) EnsureExtensions() error {
+	for idx, ecs := range ec.ExtendedSignatures {
+		if ecs.BlockIDFlag == BlockIDFlagAbsent {
+			continue
+		}
+		vote := ec.GetExtendedVote(int32(idx))
+		if err := vote.EnsureExtension(); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // StripExtensions converts an ExtendedCommit to a Commit by removing all vote
 // extension-related fields.
 func (ec *ExtendedCommit) StripExtensions() *Commit {

types/vote.go

@@ -313,7 +313,7 @@ func (vote *Vote) ValidateWithExtension() error {
 		return err
 	}
 
-	if err := vote.ValidateExtension(); err != nil {
+	if err := vote.EnsureExtension(); err != nil {
 		return err
 	}
 
@@ -321,15 +321,12 @@ func (vote *Vote) ValidateWithExtension() error {
 }
 
 //
-func (vote *Vote) ValidateExtension() error {
+func (vote *Vote) EnsureExtension() error {
 	// We should always see vote extension signatures in non-nil precommits
 	if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() {
 		if len(vote.ExtensionSignature) == 0 {
 			return ErrVoteExtensionAbsent
 		}
-		if len(vote.ExtensionSignature) > MaxSignatureSize {
-			return fmt.Errorf("vote extension signature is too big (max: %d)", MaxSignatureSize)
-		}
 	}
 	return nil
 }

types/vote_set.go

@@ -201,8 +201,14 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) {
 			return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err)
 		}
 	} else {
-		if err := vote.Verify(voteSet.chainID, val.PubKey); err != nil {
-			return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err)
+		if len(vote.ExtensionSignature) != 0 {
+			if err := vote.VerifyWithExtension(voteSet.chainID, val.PubKey); err != nil {
+				return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err)
+			}
+		} else {
+			if err := vote.Verify(voteSet.chainID, val.PubKey); err != nil {
+				return false, fmt.Errorf("failed to verify vote with ChainID %s and PubKey %s: %w", voteSet.chainID, val.PubKey, err)
+			}
 		}
 	}