do not verify extension if not enabled

internal/consensus/state.go

@@ -2376,34 +2376,35 @@ func (cs *State) addVote(
 		return
 	}
 
-	var myAddr []byte
-	if cs.privValidatorPubKey != nil {
-		myAddr = cs.privValidatorPubKey.Address()
-	}
-	// Verify VoteExtension if precommit and not nil
-	// https://github.com/tendermint/tendermint/issues/8487
-	if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() &&
-		!bytes.Equal(vote.ValidatorAddress, myAddr) {
-		// The core fields of the vote message were already validated in the
-		// consensus reactor when the vote was received.
-		// Here, we validate that the vote extension was included in the vote
-		// message.
-		// Chains that are not configured to require vote extensions
-		// 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.EnsureExtension()
-		if err == nil {
-			_, val := cs.state.Validators.GetByIndex(vote.ValidatorIndex)
-			err = vote.VerifyExtension(cs.state.ChainID, val.PubKey)
+	// Check to see if the chain is configured to extend votes.
+	if cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(cs.Height) {
+		// The chain is configured to extend votes, check that the vote is
+		// not for a nil block and verify the extensions signature against the
+		// corresponding public key.
+
+		var myAddr []byte
+		if cs.privValidatorPubKey != nil {
+			myAddr = cs.privValidatorPubKey.Address()
 		}
-		if err == nil {
+		// Verify VoteExtension if precommit and not nil
+		// https://github.com/tendermint/tendermint/issues/8487
+		if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() &&
+			!bytes.Equal(vote.ValidatorAddress, myAddr) { // Skip the VerifyVoteExtension call if the vote was issued by this validator.
+
+			// The core fields of the vote message were already validated in the
+			// consensus reactor when the vote was received.
+			// Here, we verify the signature of the vote extension included in the vote
+			// message.
+			_, val := cs.state.Validators.GetByIndex(vote.ValidatorIndex)
+			if err := vote.VerifyExtension(cs.state.ChainID, val.PubKey); err != nil {
+				return false, err
+			}
+
 			err := cs.blockExec.VerifyVoteExtension(ctx, vote)
 			cs.metrics.MarkVoteExtensionReceived(err == nil)
-		} else if !errors.Is(err, types.ErrVoteExtensionAbsent) {
-			return false, err
-		} else if cs.state.ConsensusParams.ABCI.VoteExtensionsEnabled(cs.Height) {
-			return false, err
+			if err != nil {
+				return false, err
+			}
 		}
 	}
 

internal/consensus/state_test.go

@@ -2115,6 +2115,7 @@ func TestVerifyVoteExtensionNotCalledOnAbsentPrecommit(t *testing.T) {
 	m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&abci.ResponseFinalizeBlock{}, nil).Maybe()
 	cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m})
 	height, round := cs1.Height, cs1.Round
+	cs1.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = cs1.Height
 
 	proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal)
 	newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound)
@@ -2253,44 +2254,50 @@ func TestPrepareProposalReceivesVoteExtensions(t *testing.T) {
 	}
 }
 
-// TestVoteExtensionRequiredHeight tests that 'ExtensionRequireHeight' correctly
+// TestVoteExtensionEnableHeight tests that 'ExtensionRequireHeight' correctly
 // enforces that vote extensions be present in consensus for heights greater than
 // or equal to the configured value.
-func TestVoteExtensionRequiredHeight(t *testing.T) {
+func TestVoteExtensionEnableHeight(t *testing.T) {
 	for _, testCase := range []struct {
 		name                  string
-		initialRequiredHeight int64
+		enableHeight          int64
 		hasExtension          bool
+		expectVerifyCalled    bool
 		expectSuccessfulRound bool
 	}{
 		{
-			name:                  "extension present but not required",
+			name:                  "extension present but not enabled",
 			hasExtension:          true,
-			initialRequiredHeight: 0,
+			enableHeight:          0,
+			expectVerifyCalled:    false,
 			expectSuccessfulRound: true,
 		},
 		{
 			name:                  "extension absent but not required",
 			hasExtension:          false,
-			initialRequiredHeight: 0,
+			enableHeight:          0,
+			expectVerifyCalled:    false,
 			expectSuccessfulRound: true,
 		},
 		{
 			name:                  "extension present and required",
 			hasExtension:          true,
-			initialRequiredHeight: 1,
+			enableHeight:          1,
+			expectVerifyCalled:    true,
 			expectSuccessfulRound: true,
 		},
 		{
 			name:                  "extension absent but required",
 			hasExtension:          false,
-			initialRequiredHeight: 1,
+			enableHeight:          1,
+			expectVerifyCalled:    false,
 			expectSuccessfulRound: false,
 		},
 		{
 			name:                  "extension absent but required in future height",
 			hasExtension:          false,
-			initialRequiredHeight: 2,
+			enableHeight:          2,
+			expectVerifyCalled:    false,
 			expectSuccessfulRound: true,
 		},
 	} {
@@ -2306,7 +2313,7 @@ func TestVoteExtensionRequiredHeight(t *testing.T) {
 			}, nil)
 			m.On("PrepareProposal", mock.Anything, mock.Anything).Return(&abci.ResponsePrepareProposal{}, nil)
 			m.On("ExtendVote", mock.Anything, mock.Anything).Return(&abci.ResponseExtendVote{}, nil)
-			if testCase.hasExtension {
+			if testCase.expectVerifyCalled {
 				m.On("VerifyVoteExtension", mock.Anything, mock.Anything).Return(&abci.ResponseVerifyVoteExtension{
 					Status: abci.ResponseVerifyVoteExtension_ACCEPT,
 				}, nil).Times(numValidators - 1)
@@ -2314,7 +2321,7 @@ func TestVoteExtensionRequiredHeight(t *testing.T) {
 			m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&abci.ResponseFinalizeBlock{}, nil).Maybe()
 			m.On("Commit", mock.Anything).Return(&abci.ResponseCommit{}, nil).Maybe()
 			cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m, validators: numValidators})
-			cs1.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = testCase.initialRequiredHeight
+			cs1.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = testCase.enableHeight
 			height, round := cs1.Height, cs1.Round
 
 			timeoutCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryTimeoutPropose)