wip: compiles but test fails

internal/consensus/state.go

@@ -2308,6 +2308,7 @@ func (cs *State) addVote(
 			return
 		}
 
+		// late votes still _must_ have extensions.
 		added, err = cs.LastCommit.AddVote(vote)
 		if !added {
 			return
@@ -2339,11 +2340,31 @@ func (cs *State) addVote(
 
 	// Verify VoteExtension if precommit and not nil
 	// https://github.com/tendermint/tendermint/issues/8487
-	if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() {
-		err := cs.blockExec.VerifyVoteExtension(ctx, vote)
-		cs.metrics.MarkVoteExtensionReceived(err == nil)
-		if err != nil {
-			return false, err
+	if vote.Type == tmproto.PrecommitType && !vote.BlockID.IsNil() &&
+		!bytes.Equal(vote.ValidatorAddress, cs.privValidatorPubKey.Address()) {
+		// The core fields of the vote message were already validated in the
+		// consensus reactor when the vote was received.
+		// Here, we valdiate 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.ValidateExtension()
+		if err == nil {
+			_, val := cs.state.Validators.GetByIndex(vote.ValidatorIndex)
+			err = vote.VerifyWithExtension(cs.state.ChainID, val.PubKey)
+		}
+		if err == nil {
+			err := cs.blockExec.VerifyVoteExtension(ctx, vote)
+			cs.metrics.MarkVoteExtensionReceived(err == nil)
+		} else {
+			if !errors.Is(err, types.ErrVoteExtensionAbsent) {
+				return false, err
+			}
+			if cs.requireVoteExtension() {
+				return false, err
+			}
 		}
 	}
 
@@ -2741,6 +2762,14 @@ func (cs *State) calculateProposalTimestampDifferenceMetric() {
 	}
 }
 
+func (cs *State) requireVoteExtension() bool {
+	requireHeight := cs.state.ConsensusParams.Vote.ExtensionRequireHeight
+	if requireHeight < cs.Height {
+		return false
+	}
+	return true
+}
+
 // proposerWaitTime determines how long the proposer should wait to propose its next block.
 // If the result is zero, a block can be proposed immediately.
 //

internal/consensus/state_test.go

@@ -2266,6 +2266,87 @@ func TestPrepareProposalReceivesVoteExtensions(t *testing.T) {
 	}
 }
 
+func TestVerifyVoteExtensionCalled(t *testing.T) {
+	for _, testCase := range []struct {
+		name                 string
+		hasExtension         bool
+		initialRequireHeight int64
+	}{
+		{
+			name:         "called when extension present",
+			hasExtension: true,
+		},
+		{
+			name:         "not called when extension absent",
+			hasExtension: false,
+		},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			config := configSetup(t)
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			numValidators := 3
+			m := abcimocks.NewApplication(t)
+			m.On("ProcessProposal", mock.Anything, mock.Anything).Return(&abci.ResponseProcessProposal{
+				Status: abci.ResponseProcessProposal_ACCEPT,
+			}, 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 {
+				m.On("VerifyVoteExtension", mock.Anything, mock.Anything).Return(&abci.ResponseVerifyVoteExtension{
+					Status: abci.ResponseVerifyVoteExtension_ACCEPT,
+				}, nil).Times(numValidators)
+			}
+			m.On("FinalizeBlock", mock.Anything, mock.Anything).Return(&abci.ResponseFinalizeBlock{}, nil).Maybe()
+			m.On("Commit", mock.Anything).Return(&abci.ResponseCommit{}, nil).Maybe()
+			l := log.NewTestingLogger(t)
+			cs1, vss := makeState(ctx, t, makeStateArgs{config: config, application: m, logger: l, validators: numValidators})
+			height, round := cs1.Height, cs1.Round
+
+			proposalCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryCompleteProposal)
+			newRoundCh := subscribe(ctx, t, cs1.eventBus, types.EventQueryNewRound)
+			pv1, err := cs1.privValidator.GetPubKey(ctx)
+			require.NoError(t, err)
+			addr := pv1.Address()
+			voteCh := subscribeToVoter(ctx, t, cs1, addr)
+
+			startTestRound(ctx, cs1, cs1.Height, round)
+			ensureNewRound(t, newRoundCh, height, round)
+			ensureNewProposal(t, proposalCh, height, round)
+			rs := cs1.GetRoundState()
+
+			blockID := types.BlockID{
+				Hash:          rs.ProposalBlock.Hash(),
+				PartSetHeader: rs.ProposalBlockParts.Header(),
+			}
+
+			// sign all of the votes
+			signAddVotes(ctx, t, cs1, tmproto.PrevoteType, config.ChainID(), blockID, vss[1:]...)
+			ensurePrevoteMatch(t, voteCh, height, round, rs.ProposalBlock.Hash())
+
+			var ext []byte
+			if testCase.hasExtension {
+				ext = []byte("extension")
+			}
+			// sign all of the precommits
+			for _, vs := range vss[1:] {
+				vote, err := vs.signVote(ctx, tmproto.PrecommitType, config.ChainID(), blockID, ext)
+				if !testCase.hasExtension {
+					vote.ExtensionSignature = nil
+				}
+				require.NoError(t, err)
+				addVotes(cs1, vote)
+			}
+			ensurePrecommit(t, voteCh, height, round)
+
+			height++
+			ensureNewRound(t, newRoundCh, height, round)
+			m.AssertExpectations(t)
+		})
+	}
+}
+
 // 4 vals, 3 Nil Precommits at P0
 // What we want:
 // P0 waits for timeoutPrecommit before starting next round

internal/state/execution.go

@@ -3,6 +3,7 @@ package state
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -102,13 +103,12 @@ func (blockExec *BlockExecutor) CreateProposalBlock(
 	txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGas)
 	commit := lastExtCommit.StripExtensions()
 	block := state.MakeBlock(height, txs, commit, evidence, proposerAddr)
-
 	rpp, err := blockExec.appClient.PrepareProposal(
 		ctx,
 		&abci.RequestPrepareProposal{
 			MaxTxBytes:          maxDataBytes,
 			Txs:                 block.Txs.ToSliceOfBytes(),
-			LocalLastCommit:     buildExtendedCommitInfo(lastExtCommit, blockExec.store, state.InitialHeight),
+			LocalLastCommit:     buildExtendedCommitInfo(lastExtCommit, blockExec.store, state.InitialHeight, state.ConsensusParams.Vote.ExtensionRequireHeight),
 			ByzantineValidators: block.Evidence.ToABCI(),
 			Height:              block.Height,
 			Time:                block.Time,
@@ -321,7 +321,7 @@ func (blockExec *BlockExecutor) VerifyVoteExtension(ctx context.Context, vote *t
 	}
 
 	if !resp.IsOK() {
-		return types.ErrVoteInvalidExtension
+		return errors.New("invalid vote extension")
 	}
 
 	return nil
@@ -428,7 +428,7 @@ func buildLastCommitInfo(block *types.Block, store Store, initialHeight int64) a
 // data, it returns an empty record.
 //
 // Assumes that the commit signatures are sorted according to validator index.
-func buildExtendedCommitInfo(ec *types.ExtendedCommit, store Store, initialHeight int64) abci.ExtendedCommitInfo {
+func buildExtendedCommitInfo(ec *types.ExtendedCommit, store Store, initialHeight, extensionRequireHeight int64) abci.ExtendedCommitInfo {
 	if ec.Height < initialHeight {
 		// There are no extended commits for heights below the initial height.
 		return abci.ExtendedCommitInfo{}
@@ -469,6 +469,10 @@ func buildExtendedCommitInfo(ec *types.ExtendedCommit, store Store, initialHeigh
 		if ecs.BlockIDFlag == types.BlockIDFlagCommit {
 			// We only care about vote extensions if a validator has voted to
 			// commit.
+			if ecs.Extension == nil && ecs.ExtensionSignature == nil && ec.Height > extensionRequireHeight {
+				// TODO: this error is akward, make it better
+				panic(fmt.Errorf("commit received with missing vote extension data"))
+			}
 			ext = ecs.Extension
 		}
 

types/vote.go

@@ -27,7 +27,7 @@ var (
 	ErrVoteInvalidBlockHash          = errors.New("invalid block hash")
 	ErrVoteNonDeterministicSignature = errors.New("non-deterministic signature")
 	ErrVoteNil                       = errors.New("nil vote")
-	ErrVoteInvalidExtension          = errors.New("invalid vote extension")
+	ErrVoteExtensionAbsent           = errors.New("vote extensions absent")
 )
 
 type ErrVoteConflictingVotes struct {
@@ -317,16 +317,24 @@ func (vote *Vote) ValidateWithExtension() error {
 		return err
 	}
 
+	if err := vote.ValidateExtension(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+//
+func (vote *Vote) ValidateExtension() 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 errors.New("vote extension signature is missing")
+			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

@@ -194,7 +194,7 @@ func (voteSet *VoteSet) addVote(vote *Vote) (added bool, err error) {
 	}
 
 	// Check signature.
-	if err := vote.VerifyWithExtension(voteSet.chainID, val.PubKey); err != nil {
+	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)
 	}