abci++: Propagate vote extensions (RFC 017) (#8433)

* Add protos for ExtendedCommit Cherry-pick from e73f017 just the changes to the .proto files, since we have deleted the .intermediate files. Signed-off-by: Thane Thomson <connect@thanethomson.com> * make proto-gen Signed-off-by: Thane Thomson <connect@thanethomson.com> * BlockStore holds extended commit Cherry-pick 8d504d4 and fix conflicts. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Reshuffle ExtendedCommit and ExtendedCommitSig Separate the data structures and functions from their Commit-oriented counterparts to adhere to the current coding style. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix exit condition in blocksync * Add note to remove TxResult proto As Sergio pointed out in 3e31aa6, this proto message can probably be removed. We should do this in a separate PR. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Lift termination condition into for loop Signed-off-by: Thane Thomson <connect@thanethomson.com> * Enforce vote extension signature requirement Signed-off-by: Thane Thomson <connect@thanethomson.com> * Expand on comment for PeekTwoBlocks for posterity Signed-off-by: Thane Thomson <connect@thanethomson.com> * Isolate TODO more clearly Signed-off-by: Thane Thomson <connect@thanethomson.com> * make mockery Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix comment Signed-off-by: Thane Thomson <connect@thanethomson.com> * Make panic output from BlockStore.SaveBlock more readable Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add helper methods to ExtendedCommitSig and ExtendedCommit Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix most tests except TestHandshake* Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix store prefix collision Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix TestBlockFetchAtHeight Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove global state from store tests Signed-off-by: Thane Thomson <connect@thanethomson.com> * Apply suggestions from code review Co-authored-by: M. J. Fromberger <fromberger@interchain.io> Co-authored-by: Sergio Mena <sergio@informal.systems> * blocksync: Just return error Signed-off-by: Thane Thomson <connect@thanethomson.com> * make format Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Remove unused/commented-out code Signed-off-by: Thane Thomson <connect@thanethomson.com> * blocksync: Change pool AddBlock function signature to return errors Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Improve legibility of switch statements Signed-off-by: Thane Thomson <connect@thanethomson.com> * blocksync: Expand on extended commit requirement in AddBlock description Signed-off-by: Thane Thomson <connect@thanethomson.com> * blocksync: Return error without also logging it Signed-off-by: Thane Thomson <connect@thanethomson.com> * consensus: Rename short-lived local variable Signed-off-by: Thane Thomson <connect@thanethomson.com> * consensus: Allocate TODO to Sergio Signed-off-by: Thane Thomson <connect@thanethomson.com> * evidence/pool_test: Inline slice construction Signed-off-by: Thane Thomson <connect@thanethomson.com> * state: Rename LoadBlockExtCommit to LoadBlockExtendedCommit Signed-off-by: Thane Thomson <connect@thanethomson.com> * proto: Remove TODO on TxResult Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Minor format Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Reformat ExtendedCommitSig.BlockID Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Remove NewExtendedCommit constructor Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Remove NewCommit constructor Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Shorten receiver names for ExtendedCommit Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Convert ExtendedCommit.Copy to a deep clone Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Assign TODO to Sergio Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Fix legibility nits Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Improve legibility Signed-off-by: Thane Thomson <connect@thanethomson.com> * store/state: Add TODO to move prefixes to common package Signed-off-by: Thane Thomson <connect@thanethomson.com> * Propagate validator info to PrepareProposal In order to propagate validator voting power through to PrepareProposal, we need to load the validator set info from the height corresponding to the extended commit that we're passing through to PrepareProposal as the "LocalLastCommit". Signed-off-by: Thane Thomson <connect@thanethomson.com> * Rename local var for clarity Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix TestMaxProposalBlockSize Signed-off-by: Thane Thomson <connect@thanethomson.com> * Rename local var for clarity Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove debug log Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove CommigSig.ForBlock helper Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove CommigSig.Absent helper Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove ExtendedCommitSig.ForBlock helper Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove ExtendedCommitSig.Absent helper Signed-off-by: Thane Thomson <connect@thanethomson.com> * There are no extended commits below the initial height Signed-off-by: Thane Thomson <connect@thanethomson.com> * Fix comment grammar Signed-off-by: Thane Thomson <connect@thanethomson.com> * Remove JSON encoding from ExtendedCommit Signed-off-by: Thane Thomson <connect@thanethomson.com> * Embed CommitSig into ExtendedCommitSig instead of duplicating fields Signed-off-by: Thane Thomson <connect@thanethomson.com> * Rename ExtendedCommit vote_extension field to extension for consistency with domain types Signed-off-by: Thane Thomson <connect@thanethomson.com> * blocksync: Panic if we peek a block without an extended commit Signed-off-by: Thane Thomson <connect@thanethomson.com> * Apply suggestions from code review Co-authored-by: M. J. Fromberger <fromberger@interchain.io> * Remove Sergio from TODO Signed-off-by: Thane Thomson <connect@thanethomson.com> * Increase hard-coded vote extension max size to 1MB Signed-off-by: Thane Thomson <connect@thanethomson.com> * state: Remove unnecessary comment Signed-off-by: Thane Thomson <connect@thanethomson.com> * state: Ensure no of commit sigs equals validator set length Signed-off-by: Thane Thomson <connect@thanethomson.com> * make format Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Minor legibility improvements Signed-off-by: Thane Thomson <connect@thanethomson.com> * Improve legibility Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Remove unused GetVotes function on VoteSet Signed-off-by: Thane Thomson <connect@thanethomson.com> * Refactor TestMaxProposalBlockSize to construct more realistic extended commit Signed-off-by: Thane Thomson <connect@thanethomson.com> * Refactor buildExtendedCommitInfo to resemble buildLastCommitInfo Signed-off-by: Thane Thomson <connect@thanethomson.com> * Apply suggestions from code review Co-authored-by: M. J. Fromberger <fromberger@interchain.io> * abci++: Disable VerifyVoteExtension call on nil precommits (#8491) Signed-off-by: Thane Thomson <connect@thanethomson.com> * types: Require vote extensions on non-nil precommits and not otherwise Signed-off-by: Thane Thomson <connect@thanethomson.com> * Disable lint Signed-off-by: Thane Thomson <connect@thanethomson.com> * Increase timeout for TestReactorVotingPowerChange to counter flakiness Signed-off-by: Thane Thomson <connect@thanethomson.com> * Only sign and verify vote extensions in non-nil precommits Signed-off-by: Thane Thomson <connect@thanethomson.com> * Revert "Disable lint" This reverts commit 6fffbf9. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Add missing non-nil check uncovered non-deterministically in TestHandshakeReplayAll Signed-off-by: Thane Thomson <connect@thanethomson.com> * Expand error message for accuracy Signed-off-by: Thane Thomson <connect@thanethomson.com> * Only call ExtendVote when we make non-nil precommits Signed-off-by: Thane Thomson <connect@thanethomson.com> * Revert "Increase timeout for TestReactorVotingPowerChange to counter flakiness" This reverts commit af51493. Signed-off-by: Thane Thomson <connect@thanethomson.com> * Refactor ValidateBasic for ExtendedCommitSig for legibility Signed-off-by: Thane Thomson <connect@thanethomson.com> Co-authored-by: Sergio Mena <sergio@informal.systems> Co-authored-by: M. J. Fromberger <fromberger@interchain.io>
tendermint · Nov 12, 2022 · 7db70fb · 7db70fb
1 parent 860d610
commit 7db70fb
Show file tree

Hide file tree

Showing 40 changed files with 1,006 additions and 509 deletions.
diff --git a/blocksync/pool.go b/blocksync/pool.go
@@ -185,16 +185,20 @@ func (pool *BlockPool) IsCaughtUp() bool {
 	return isCaughtUp
 }
 
-// PeekTwoBlocks returns blocks at pool.height and pool.height+1.
-// We need to see the second block's Commit to validate the first block.
-// So we peek two blocks at a time.
+// PeekTwoBlocks returns blocks at pool.height and pool.height+1. We need to
+// see the second block's Commit to validate the first block. So we peek two
+// blocks at a time. We return an extended commit, containing vote extensions
+// and their associated signatures, as this is critical to consensus in ABCI++
+// as we switch from block sync to consensus mode.
+//
 // The caller will verify the commit.
-func (pool *BlockPool) PeekTwoBlocks() (first *types.Block, second *types.Block) {
+func (pool *BlockPool) PeekTwoBlocks() (first, second *types.Block, firstExtCommit *types.ExtendedCommit) {
 	pool.mtx.Lock()
 	defer pool.mtx.Unlock()
 
 	if r := pool.requesters[pool.height]; r != nil {
 		first = r.getBlock()
+		firstExtCommit = r.getExtendedCommit()
 	}
 	if r := pool.requesters[pool.height+1]; r != nil {
 		second = r.getBlock()
@@ -203,7 +207,8 @@ func (pool *BlockPool) PeekTwoBlocks() (first *types.Block, second *types.Block)
 }
 
 // PopRequest pops the first block at pool.height.
-// It must have been validated by 'second'.Commit from PeekTwoBlocks().
+// It must have been validated by the second Commit from PeekTwoBlocks.
+// TODO(thane): (?) and its corresponding ExtendedCommit.
 func (pool *BlockPool) PopRequest() {
 	pool.mtx.Lock()
 	defer pool.mtx.Unlock()
@@ -240,12 +245,23 @@ func (pool *BlockPool) RedoRequest(height int64) p2p.ID {
 	return peerID
 }
 
-// AddBlock validates that the block comes from the peer it was expected from and calls the requester to store it.
+// AddBlock validates that the block comes from the peer it was expected from
+// and calls the requester to store it.
+//
+// This requires an extended commit at the same height as the supplied block -
+// the block contains the last commit, but we need the latest commit in case we
+// need to switch over from block sync to consensus at this height. If the
+// height of the extended commit and the height of the block do not match, we
+// do not add the block and return an error.
 // TODO: ensure that blocks come in order for each peer.
-func (pool *BlockPool) AddBlock(peerID p2p.ID, block *types.Block, blockSize int) {
+func (pool *BlockPool) AddBlock(peerID p2p.ID, block *types.Block, extCommit *types.ExtendedCommit, blockSize int) {
 	pool.mtx.Lock()
 	defer pool.mtx.Unlock()
 
+	if block.Height != extCommit.Height {
+		return fmt.Errorf("heights don't match, not adding block (block height: %d, commit height: %d)", block.Height, extCommit.Height)
+	}
+
 	requester := pool.requesters[block.Height]
 	if requester == nil {
 		pool.Logger.Info(
@@ -263,19 +279,22 @@ func (pool *BlockPool) AddBlock(peerID p2p.ID, block *types.Block, blockSize int
 		if diff > maxDiffBetweenCurrentAndReceivedBlockHeight {
 			pool.sendError(errors.New("peer sent us a block we didn't expect with a height too far ahead/behind"), peerID)
 		}
-		return
+		return fmt.Errorf("peer sent us a block we didn't expect (peer: %s, current height: %d, block height: %d)", peerID, pool.height, block.Height)
 	}
 
-	if requester.setBlock(block, peerID) {
+	if requester.setBlock(block, extCommit, peerID) {
 		atomic.AddInt32(&pool.numPending, -1)
 		peer := pool.peers[peerID]
 		if peer != nil {
 			peer.decrPending(blockSize)
 		}
 	} else {
-		pool.Logger.Info("invalid peer", "peer", peerID, "blockHeight", block.Height)
-		pool.sendError(errors.New("invalid peer"), peerID)
+		err := errors.New("requester is different or block already exists")
+		pool.sendError(err, peerID)
+		return fmt.Errorf("%w (peer: %s, requester: %s, block height: %d)", err, peerID, requester.getPeerID(), block.Height)
 	}
+
+	return nil
 }
 
 // MaxPeerHeight returns the highest reported height.
@@ -424,6 +443,7 @@ func (pool *BlockPool) debug() string {
 		} else {
 			str += fmt.Sprintf("H(%v):", h)
 			str += fmt.Sprintf("B?(%v) ", pool.requesters[h].block != nil)
+			str += fmt.Sprintf("C?(%v) ", pool.requesters[h].extCommit != nil)
 		}
 	}
 	return str
@@ -512,9 +532,10 @@ type bpRequester struct {
 	gotBlockCh chan struct{}
 	redoCh     chan p2p.ID // redo may send multitime, add peerId to identify repeat
 
-	mtx    tmsync.Mutex
-	peerID p2p.ID
-	block  *types.Block
+	mtx       tmsync.Mutex
+	peerID    p2p.ID
+	block     *types.Block
+	extCommit *types.ExtendedCommit
 }
 
 func newBPRequester(pool *BlockPool, height int64) *bpRequester {
@@ -537,13 +558,14 @@ func (bpr *bpRequester) OnStart() error {
 }
 
 // Returns true if the peer matches and block doesn't already exist.
-func (bpr *bpRequester) setBlock(block *types.Block, peerID p2p.ID) bool {
+func (bpr *bpRequester) setBlock(block *types.Block, extCommit *types.ExtendedCommit, peerID p2p.ID) bool {
 	bpr.mtx.Lock()
 	if bpr.block != nil || bpr.peerID != peerID {
 		bpr.mtx.Unlock()
 		return false
 	}
 	bpr.block = block
+	bpr.extCommit = extCommit
 	bpr.mtx.Unlock()
 
 	select {
@@ -559,6 +581,12 @@ func (bpr *bpRequester) getBlock() *types.Block {
 	return bpr.block
 }
 
+func (bpr *bpRequester) getExtendedCommit() *types.ExtendedCommit {
+	bpr.mtx.Lock()
+	defer bpr.mtx.Unlock()
+	return bpr.extCommit
+}
+
 func (bpr *bpRequester) getPeerID() p2p.ID {
 	bpr.mtx.Lock()
 	defer bpr.mtx.Unlock()
@@ -576,6 +604,7 @@ func (bpr *bpRequester) reset() {
 
 	bpr.peerID = ""
 	bpr.block = nil
+	bpr.extCommit = nil
 }
 
 // Tells bpRequester to pick another peer and try again.

diff --git a/blocksync/pool_test.go b/blocksync/pool_test.go
@@ -42,7 +42,10 @@ func (p testPeer) runInputRoutine() {
 // Request desired, pretend like we got the block immediately.
 func (p testPeer) simulateInput(input inputData) {
 	block := &types.Block{Header: types.Header{Height: input.request.Height}}
-	input.pool.AddBlock(input.request.PeerID, block, 123)
+	extCommit := &types.ExtendedCommit{
+		Height: input.request.Height,
+	}
+	_ = input.pool.AddBlock(input.request.PeerID, block, extCommit, 123)
 	// TODO: uncommenting this creates a race which is detected by:
 	// https://github.com/golang/go/blob/2bd767b1022dd3254bcec469f0ee164024726486/src/testing/testing.go#L854-L856
 	// see: https://github.com/tendermint/tendermint/issues/3390#issue-418379890
@@ -112,7 +115,7 @@ func TestBlockPoolBasic(t *testing.T) {
 			if !pool.IsRunning() {
 				return
 			}
-			first, second := pool.PeekTwoBlocks()
+			first, second, _ := pool.PeekTwoBlocks()
 			if first != nil && second != nil {
 				pool.PopRequest()
 			} else {
@@ -171,7 +174,7 @@ func TestBlockPoolTimeout(t *testing.T) {
 			if !pool.IsRunning() {
 				return
 			}
-			first, second := pool.PeekTwoBlocks()
+			first, second, _ := pool.PeekTwoBlocks()
 			if first != nil && second != nil {
 				pool.PopRequest()
 			} else {

diff --git a/blocksync/reactor.go b/blocksync/reactor.go
@@ -52,7 +52,7 @@ type Reactor struct {
 	initialState sm.State
 
 	blockExec *sm.BlockExecutor
-	store     *store.BlockStore
+	store     sm.BlockStore
 	pool      *BlockPool
 	blockSync bool
 
@@ -176,13 +176,21 @@ func (bcR *Reactor) respondToPeer(msg *bcproto.BlockRequest,
 
 	block := bcR.store.LoadBlock(msg.Height)
 	if block != nil {
+		extCommit := bcR.store.LoadBlockExtendedCommit(msg.Height)
+		if extCommit == nil {
+			bcR.Logger.Error("found block in store without extended commit", "block", block)
+			return false
+		}
 		bl, err := block.ToProto()
 		if err != nil {
 			bcR.Logger.Error("could not convert msg to protobuf", "err", err)
 			return false
 		}
 
-		msgBytes, err := EncodeMsg(&bcproto.BlockResponse{Block: bl})
+		msgBytes, err := EncodeMsg(&bcproto.BlockResponse{
+			Block:     bl,
+			ExtCommit: extCommit.ToProto()},
+		)
 		if err != nil {
 			bcR.Logger.Error("could not marshal msg", "err", err)
 			return false
@@ -228,7 +236,17 @@ func (bcR *Reactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) {
 			bcR.Logger.Error("Block content is invalid", "err", err)
 			return
 		}
-		bcR.pool.AddBlock(src.ID(), bi, len(msgBytes))
+		extCommit, err := types.ExtendedCommitFromProto(msg.ExtCommit)
+		if err != nil {
+			bcR.Logger.Error("failed to convert extended commit from proto",
+				"peer", src,
+				"err", err)
+			return
+		}
+
+		if err := bcR.pool.AddBlock(src.ID(), bi, extCommit, len(msgBytes)); err != nil {
+			bcR.Logger.Error("failed to add block", "err", err)
+		}
 	case *bcproto.StatusRequest:
 		// Send peer our state.
 		msgBytes, err := EncodeMsg(&bcproto.StatusResponse{
@@ -317,6 +335,19 @@ FOR_LOOP:
 			outbound, inbound, _ := bcR.Switch.NumPeers()
 			bcR.Logger.Debug("Consensus ticker", "numPending", numPending, "total", lenRequesters,
 				"outbound", outbound, "inbound", inbound)
+
+			// TODO(sergio) Might be needed for implementing the upgrading solution. Remove after that
+			if state.LastBlockHeight > 0 && blocksSynced == 0 {
+				// Having state-synced, we need to blocksync at least one block
+				bcR.Logger.Info(
+					"no seen commit yet",
+					"height", height,
+					"last_block_height", state.LastBlockHeight,
+					"initial_height", state.InitialHeight,
+					"max_peer_height", bcR.pool.MaxPeerHeight(),
+				)
+				continue FOR_LOOP
+			}
 			if bcR.pool.IsCaughtUp() {
 				bcR.Logger.Info("Time to switch to consensus reactor!", "height", height)
 				if err := bcR.pool.Stop(); err != nil {
@@ -349,10 +380,14 @@ FOR_LOOP:
 			// routine.
 
 			// See if there are any blocks to sync.
-			first, second := bcR.pool.PeekTwoBlocks()
+			first, second, extCommit := bcR.pool.PeekTwoBlocks()
 			// bcR.Logger.Info("TrySync peeked", "first", first, "second", second)
-			if first == nil || second == nil {
-				// We need both to sync the first block.
+			if first == nil || second == nil || extCommit == nil {
+				if first != nil && extCommit == nil {
+					// See https://github.com/tendermint/tendermint/pull/8433#discussion_r866790631
+					panic(fmt.Errorf("peeked first block without extended commit at height %d - possible node store corruption", first.Height))
+				}
+				// we need all to sync the first block
 				continue FOR_LOOP
 			} else {
 				// Try again quickly next loop.
@@ -372,6 +407,7 @@ FOR_LOOP:
 			// NOTE: we can probably make this more efficient, but note that calling
 			// first.Hash() doesn't verify the tx contents, so MakePartSet() is
 			// currently necessary.
+			// TODO(sergio): Should we also validate against the extended commit?
 			err = state.Validators.VerifyCommitLight(
 				chainID, firstID, first.Height, second.LastCommit)
 
@@ -402,7 +438,7 @@ FOR_LOOP:
 			bcR.pool.PopRequest()
 
 			// TODO: batch saves so we dont persist to disk every block
-			bcR.store.SaveBlock(first, firstParts, second.LastCommit)
+			bcR.store.SaveBlock(first, firstParts, extCommit)
 
 			// TODO: same thing for app - but we would need a way to
 			// get the hash without persisting the state

diff --git a/blocksync/reactor_test.go b/blocksync/reactor_test.go
@@ -108,40 +108,46 @@ func newReactor(
 		panic(err)
 	}
 
+	var lastExtCommit *types.ExtendedCommit
+
+	// The commit we are building for the current height.
+	seenExtCommit := &types.ExtendedCommit{}
+
 	// let's add some blocks in
 	for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ {
-		lastCommit := types.NewCommit(blockHeight-1, 0, types.BlockID{}, nil)
-		if blockHeight > 1 {
-			lastBlockMeta := blockStore.LoadBlockMeta(blockHeight - 1)
-			lastBlock := blockStore.LoadBlock(blockHeight - 1)
-
-			vote, err := types.MakeVote(
-				lastBlock.Header.Height,
-				lastBlockMeta.BlockID,
-				state.Validators,
-				privVals[0],
-				lastBlock.Header.ChainID,
-				time.Now(),
-			)
-			if err != nil {
-				panic(err)
-			}
-			lastCommit = types.NewCommit(vote.Height, vote.Round,
-				lastBlockMeta.BlockID, []types.CommitSig{vote.CommitSig()})
-		}
+		lastExtCommit = seenExtCommit.Clone()
 
-		thisBlock := state.MakeBlock(blockHeight, nil, lastCommit, nil, state.Validators.Proposer.Address)
+		thisBlock := state.MakeBlock(blockHeight, nil, lastExtCommit.StripExtensions(), nil, state.Validators.Proposer.Address)
 
 		thisParts, err := thisBlock.MakePartSet(types.BlockPartSizeBytes)
 		require.NoError(t, err)
 		blockID := types.BlockID{Hash: thisBlock.Hash(), PartSetHeader: thisParts.Header()}
 
+		// Simulate a commit for the current height
+		vote, err := types.MakeVote(
+			thisBlock.Header.Height,
+			blockID,
+			state.Validators,
+			privVals[0],
+			thisBlock.Header.ChainID,
+			time.Now(),
+		)
+		if err != nil {
+			panic(err)
+		}
+		seenExtCommit = &types.ExtendedCommit{
+			Height:             vote.Height,
+			Round:              vote.Round,
+			BlockID:            blockID,
+			ExtendedSignatures: []types.ExtendedCommitSig{vote.ExtendedCommitSig()},
+		}
+
 		state, _, err = blockExec.ApplyBlock(state, blockID, thisBlock)
 		if err != nil {
 			panic(fmt.Errorf("error apply block: %w", err))
 		}
 
-		blockStore.SaveBlock(thisBlock, thisParts, lastCommit)
+		blockStore.SaveBlock(thisBlock, thisParts, seenExtCommiqt)
 	}
 
 	bcReactor := NewReactor(state.Copy(), blockExec, blockStore, fastSync)

diff --git a/consensus/byzantine_test.go b/consensus/byzantine_test.go
@@ -188,24 +188,22 @@ func TestByzantinePrevoteEquivocation(t *testing.T) {
 			panic("entered createProposalBlock with privValidator being nil")
 		}
 
-		var commit *types.Commit
-		var votes []*types.Vote
+		var extCommit *types.ExtendedCommit
 		switch {
 		case lazyProposer.Height == lazyProposer.state.InitialHeight:
 			// We're creating a proposal for the first block.
 			// The commit is empty, but not nil.
-			commit = types.NewCommit(0, 0, types.BlockID{}, nil)
+			extCommit = &types.ExtendedCommit{}
 		case lazyProposer.LastCommit.HasTwoThirdsMajority():
 			// Make the commit from LastCommit
-			commit = lazyProposer.LastCommit.MakeCommit()
-			votes = lazyProposer.LastCommit.GetVotes()
+			extCommit = lazyProposer.LastCommit.MakeExtendedCommit()
 		default: // This shouldn't happen.
 			lazyProposer.Logger.Error("enterPropose: Cannot propose anything: No commit for the previous block")
 			return
 		}
 
 		// omit the last signature in the commit
-		commit.Signatures[len(commit.Signatures)-1] = types.NewCommitSigAbsent()
+		extCommit.ExtendedSignatures[len(extCommit.ExtendedSignatures)-1] = types.NewExtendedCommitSigAbsent()
 
 		if lazyProposer.privValidatorPubKey == nil {
 			// If this node is a validator & proposer in the current round, it will
@@ -216,7 +214,7 @@ func TestByzantinePrevoteEquivocation(t *testing.T) {
 		proposerAddr := lazyProposer.privValidatorPubKey.Address()
 
 		block, err := lazyProposer.blockExec.CreateProposalBlock(
-			lazyProposer.Height, lazyProposer.state, commit, proposerAddr, votes)
+			lazyProposer.Height, lazyProposer.state, extCommit, proposerAddr)
 		require.NoError(t, err)
 		blockParts, err := block.MakePartSet(types.BlockPartSizeBytes)
 		require.NoError(t, err)

diff --git a/consensus/common_test.go b/consensus/common_test.go
@@ -156,7 +156,8 @@ func (vs *validatorStub) signVote(
 // Sign vote for type/hash/header
 func signVote(vs *validatorStub, voteType tmproto.SignedMsgType, hash []byte, header types.PartSetHeader) *types.Vote {
 	var ext []byte
-	if voteType == tmproto.PrecommitType {
+	// Only non-nil precommits are allowed to carry vote extensions.
+	if voteType == tmproto.PrecommitType && !blockID.IsNil() {
 		ext = []byte("extension")
 	}
 	v, err := vs.signVote(voteType, hash, header, ext)