hotfix: nil check for resilience against empty vote extensions (#480)

testutil/helper/gen_blocks.go

@@ -198,13 +198,16 @@ func (h *Helper) ApplyEmptyBlockWithValSet(r *rand.Rand, valSetWithKeys *datagen
 	if len(ppRes.Txs) > 0 {
 		blockTxs = ppRes.Txs
 	}
-	_, err = h.App.ProcessProposal(&abci.RequestProcessProposal{
+	processRes, err := h.App.ProcessProposal(&abci.RequestProcessProposal{
 		Txs:    ppRes.Txs,
 		Height: newHeight,
 	})
 	if err != nil {
 		return emptyCtx, err
 	}
+	if processRes.Status == abci.ResponseProcessProposal_REJECT {
+		return emptyCtx, fmt.Errorf("rejected proposal")
+	}
 
 	// 4. finalize block
 	resp, err := h.App.FinalizeBlock(&abci.RequestFinalizeBlock{
@@ -293,13 +296,113 @@ func (h *Helper) ApplyEmptyBlockWithInvalidBLSSig(r *rand.Rand) (sdk.Context, er
 	if len(ppRes.Txs) > 0 {
 		blockTxs = ppRes.Txs
 	}
-	_, err = h.App.ProcessProposal(&abci.RequestProcessProposal{
+	processRes, err := h.App.ProcessProposal(&abci.RequestProcessProposal{
 		Txs:    ppRes.Txs,
 		Height: newHeight,
 	})
 	if err != nil {
 		return emptyCtx, err
 	}
+	if processRes.Status == abci.ResponseProcessProposal_REJECT {
+		return emptyCtx, fmt.Errorf("rejected proposal")
+	}
+
+	// 4. finalize block
+	resp, err := h.App.FinalizeBlock(&abci.RequestFinalizeBlock{
+		Txs:                blockTxs,
+		Height:             newHeader.Height,
+		NextValidatorsHash: newHeader.NextValidatorsHash,
+		Hash:               newHeader.Hash(),
+	})
+	if err != nil {
+		return emptyCtx, err
+	}
+
+	newHeader.AppHash = resp.AppHash
+	h.Ctx = h.Ctx.WithHeaderInfo(header.Info{
+		Height:  newHeader.Height,
+		AppHash: resp.AppHash,
+		Hash:    newHeader.Hash(),
+	}).WithBlockHeader(*newHeader.ToProto())
+
+	_, err = h.App.Commit()
+	if err != nil {
+		return emptyCtx, err
+	}
+
+	return h.Ctx, nil
+}
+
+func (h *Helper) ApplyEmptyBlockWithSomeEmptyVoteExtensions(r *rand.Rand) (sdk.Context, error) {
+	emptyCtx := sdk.Context{}
+	if h.App.LastBlockHeight() == 0 {
+		if err := h.genAndApplyEmptyBlock(); err != nil {
+			return emptyCtx, err
+		}
+	}
+	valSetWithKeys := h.GenValidators
+	prevHeight := h.App.LastBlockHeight()
+	epoch := h.App.EpochingKeeper.GetEpoch(h.Ctx)
+	newHeight := prevHeight + 1
+
+	// 1. get previous vote extensions
+	prevEpoch := epoch.EpochNumber
+	blockHash := datagen.GenRandomBlockHash(r)
+	extendedVotes, err := h.getExtendedVotesFromValSet(prevEpoch, uint64(prevHeight), blockHash, valSetWithKeys)
+	if err != nil {
+		return emptyCtx, err
+	}
+
+	// 2. create new header
+	valSet, err := h.App.StakingKeeper.GetLastValidators(h.Ctx)
+	if err != nil {
+		return emptyCtx, err
+	}
+	valhash := CalculateValHash(valSet)
+	newHeader := cmttypes.Header{
+		Height:             newHeight,
+		ValidatorsHash:     valhash,
+		NextValidatorsHash: valhash,
+		LastBlockID: cmttypes.BlockID{
+			Hash: datagen.GenRandomByteArray(r, 32),
+		},
+	}
+	h.Ctx = h.Ctx.WithHeaderInfo(header.Info{
+		Height: newHeader.Height,
+		Hash:   newHeader.Hash(),
+	}).WithBlockHeader(*newHeader.ToProto())
+
+	// 3. prepare proposal with previous BLS sigs
+	var blockTxs [][]byte
+	if epoch.IsVoteExtensionProposal(h.Ctx) {
+		// nullifies a subset of extended votes
+		numEmptyVoteExts := len(extendedVotes)/3 - 1
+		for i := 0; i < numEmptyVoteExts; i++ {
+			extendedVotes[i] = abci.ExtendedVoteInfo{
+				// generate random vote extension including empty one
+				VoteExtension: datagen.GenRandomByteArray(r, uint64(r.Intn(10))),
+			}
+		}
+	}
+	ppRes, err := h.App.PrepareProposal(&abci.RequestPrepareProposal{
+		LocalLastCommit: abci.ExtendedCommitInfo{Votes: extendedVotes},
+		Height:          newHeight,
+	})
+	if err != nil {
+		return emptyCtx, err
+	}
+	blockTxs = ppRes.Txs
+
+	processRes, err := h.App.ProcessProposal(&abci.RequestProcessProposal{
+		Txs:    blockTxs,
+		Height: newHeight,
+	})
+	if err != nil {
+		return emptyCtx, err
+	}
+	if processRes.Status == abci.ResponseProcessProposal_REJECT {
+		return emptyCtx, fmt.Errorf("rejected proposal")
+	}
 
 	// 4. finalize block
 	resp, err := h.App.FinalizeBlock(&abci.RequestFinalizeBlock{

x/checkpointing/proposal.go

@@ -192,17 +192,29 @@ func (h *ProposalHandler) findLastBlockHash(extendedVotes []abci.ExtendedVoteInf
 	blockHashes := make(map[string]int64, 0)
 	// Iterate over vote extensions and if they have a valid structure
 	// increase the voting power of the block hash they commit to
+	var totalPower int64 = 0
 	for _, vote := range extendedVotes {
+		// accumulate voting power from all the votes
+		totalPower += vote.Validator.Power
 		var ve ckpttypes.VoteExtension
+		if len(vote.VoteExtension) == 0 {
+			continue
+		}
 		if err := ve.Unmarshal(vote.VoteExtension); err != nil {
 			continue
 		}
+		if ve.BlockHash == nil {
+			continue
+		}
+		bHash, err := ve.BlockHash.Marshal()
+		if err != nil {
+			continue
+		}
 		// Encode the block hash using hex
-		blockHashes[hex.EncodeToString(ve.BlockHash.MustMarshal())] += vote.Validator.Power
+		blockHashes[hex.EncodeToString(bHash)] += vote.Validator.Power
 	}
 	var (
 		maxPower     int64 = 0
-		totalPower   int64 = 0
 		resBlockHash string
 	)
 	// Find the block hash that has the maximum voting power committed to it
@@ -211,7 +223,6 @@ func (h *ProposalHandler) findLastBlockHash(extendedVotes []abci.ExtendedVoteInf
 			resBlockHash = blockHash
 			maxPower = power
 		}
-		totalPower += power
 	}
 	if len(resBlockHash) == 0 {
 		return nil, fmt.Errorf("could not find the block hash")
@@ -249,7 +260,8 @@ func (h *ProposalHandler) ProcessProposal() sdk.ProcessProposalHandler {
 			// 1. extract the special tx containing the checkpoint
 			injectedCkpt, err := extractInjectedCheckpoint(req.Txs)
 			if err != nil {
-				h.logger.Error("cannot get injected checkpoint", "err", err)
+				h.logger.Error(
+					"processProposal: failed to extract injected checkpoint from the tx set", "err", err)
 				// should not return error here as error will cause panic
 				return resReject, nil
 			}
@@ -325,7 +337,8 @@ func (h *ProposalHandler) PreBlocker() sdk.PreBlocker {
 		// 1. extract the special tx containing BLS sigs
 		injectedCkpt, err := extractInjectedCheckpoint(req.Txs)
 		if err != nil {
-			return res, fmt.Errorf("failed to get extract injected checkpoint from the tx set: %w", err)
+			return res, fmt.Errorf(
+				"preblocker: failed to extract injected checkpoint from the tx set: %w", err)
 		}
 
 		// 2. update checkpoint
@@ -346,7 +359,7 @@ func extractInjectedCheckpoint(txs [][]byte) (*ckpttypes.InjectedCheckpoint, err
 	injectedTx := txs[defaultInjectedTxIndex]
 
 	if len(injectedTx) == 0 {
-		return nil, fmt.Errorf("err in PreBlocker: the injected vote extensions tx is empty")
+		return nil, fmt.Errorf("the injected vote extensions tx is empty")
 	}
 
 	var injectedCkpt ckpttypes.InjectedCheckpoint

x/checkpointing/types/types.go

@@ -5,6 +5,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"errors"
+	"fmt"
 
 	"github.com/boljen/go-bitmap"
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -126,7 +127,9 @@ func (cm *RawCheckpointWithMeta) RecordStateUpdate(ctx context.Context, status C
 
 func (bh *BlockHash) Unmarshal(bz []byte) error {
 	if len(bz) != HashSize {
-		return errors.New("invalid appHash length")
+		return fmt.Errorf(
+			"invalid block hash length, expected: %d, got: %d",
+			HashSize, len(bz))
 	}
 	*bh = bz
 	return nil

x/checkpointing/vote_ext_test.go

@@ -14,7 +14,7 @@ import (
 // FuzzAddBLSSigVoteExtension_MultipleVals tests adding BLS signatures via VoteExtension
 // with multiple validators
 func FuzzAddBLSSigVoteExtension_MultipleVals(f *testing.F) {
-	datagen.AddRandomSeedsToFuzzer(f, 4)
+	datagen.AddRandomSeedsToFuzzer(f, 10)
 
 	f.Fuzz(func(t *testing.T, seed int64) {
 		r := rand.New(rand.NewSource(seed))
@@ -47,7 +47,7 @@ func FuzzAddBLSSigVoteExtension_MultipleVals(f *testing.F) {
 // FuzzAddBLSSigVoteExtension_InsufficientVotingPower tests adding BLS signatures
 // with insufficient voting power
 func FuzzAddBLSSigVoteExtension_InsufficientVotingPower(f *testing.F) {
-	datagen.AddRandomSeedsToFuzzer(f, 4)
+	datagen.AddRandomSeedsToFuzzer(f, 10)
 
 	f.Fuzz(func(t *testing.T, seed int64) {
 		r := rand.New(rand.NewSource(seed))
@@ -78,7 +78,7 @@ func FuzzAddBLSSigVoteExtension_InsufficientVotingPower(f *testing.F) {
 // FuzzAddBLSSigVoteExtension_InvalidBLSSig tests adding BLS signatures
 // with invalid BLS signature
 func FuzzAddBLSSigVoteExtension_InvalidBLSSig(f *testing.F) {
-	datagen.AddRandomSeedsToFuzzer(f, 4)
+	datagen.AddRandomSeedsToFuzzer(f, 10)
 
 	f.Fuzz(func(t *testing.T, seed int64) {
 		r := rand.New(rand.NewSource(seed))
@@ -99,3 +99,39 @@ func FuzzAddBLSSigVoteExtension_InvalidBLSSig(f *testing.F) {
 		}
 	})
 }
+
+// FuzzAddBLSSigVoteExtension_EmptyVoteExtensions tests resilience against
+// empty vote extensions
+func FuzzAddBLSSigVoteExtension_EmptyVoteExtensions(f *testing.F) {
+	datagen.AddRandomSeedsToFuzzer(f, 10)
+
+	f.Fuzz(func(t *testing.T, seed int64) {
+		r := rand.New(rand.NewSource(seed))
+		// generate the validator set with 10 validators as genesis
+		genesisValSet, privSigner, err := datagen.GenesisValidatorSetWithPrivSigner(10)
+		require.NoError(t, err)
+		helper := testhelper.NewHelperWithValSet(t, genesisValSet, privSigner)
+		ek := helper.App.EpochingKeeper
+		ck := helper.App.CheckpointingKeeper
+
+		epoch := ek.GetEpoch(helper.Ctx)
+		require.Equal(t, uint64(1), epoch.EpochNumber)
+
+		// go to block 10, ensure the checkpoint is finalized
+		interval := ek.GetParams(helper.Ctx).EpochInterval
+		for i := uint64(0); i < interval-2; i++ {
+			_, err := helper.ApplyEmptyBlockWithSomeEmptyVoteExtensions(r)
+			require.NoError(t, err)
+		}
+		// height 11, i.e., 1st block of next epoch
+		_, err = helper.ApplyEmptyBlockWithSomeEmptyVoteExtensions(r)
+		require.NoError(t, err)
+
+		epoch = ek.GetEpoch(helper.Ctx)
+		require.Equal(t, uint64(2), epoch.EpochNumber)
+
+		ckpt, err := ck.GetRawCheckpoint(helper.Ctx, epoch.EpochNumber-1)
+		require.NoError(t, err)
+		require.Equal(t, types.Sealed, ckpt.Status)
+	})
+}