merge develop to main

app/abci.go

@@ -10,7 +10,9 @@ import (
 	abci "github.com/cometbft/cometbft/abci/types"
 	cmtTypes "github.com/cometbft/cometbft/types"
 	"github.com/cosmos/cosmos-sdk/codec/address"
+	"github.com/cosmos/cosmos-sdk/codec/unknownproto"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/gogoproto/jsonpb"
 	"github.com/cosmos/gogoproto/proto"
 	"github.com/ethereum/go-ethereum/common"
 
@@ -339,11 +341,20 @@ func (app *HeimdallApp) ExtendVoteHandler() sdk.ExtendVoteHandler {
 			// We still want to participate in the consensus even if we fail to generate the milestone proposition
 		} else if milestoneProp != nil {
 			if err := milestoneAbci.ValidateMilestoneProposition(ctx, &app.MilestoneKeeper, milestoneProp); err != nil {
-				logger.Error("Invalid milestone proposition", "error", err, "height", req.Height, "milestoneProp", milestoneProp)
+				logger.Error("Invalid milestone proposition generated",
+					"startBlock", milestoneProp.StartBlockNumber,
+					"endBlock", milestoneProp.StartBlockNumber+uint64(len(milestoneProp.BlockHashes)-1),
+					"blockHashes", strutil.HashesToString(milestoneProp.BlockHashes),
+					"error", err,
+				)
 				// We don't want to halt consensus because of invalid milestone proposition
 			} else {
 				vt.MilestoneProposition = milestoneProp
-				logger.Debug("Proposed milestone", "hash", strutil.HashesToString(milestoneProp.BlockHashes), "startBlock", milestoneProp.StartBlockNumber, "endBlock", milestoneProp.StartBlockNumber+uint64(len(milestoneProp.BlockHashes)))
+				logger.Info("Generated a new milestone proposition",
+					"startBlock", milestoneProp.StartBlockNumber,
+					"endBlock", milestoneProp.StartBlockNumber+uint64(len(milestoneProp.BlockHashes)-1),
+					"blockHashes", strutil.HashesToString(milestoneProp.BlockHashes),
+				)
 			}
 		}
 
@@ -382,6 +393,11 @@ func (app *HeimdallApp) VerifyVoteExtensionHandler() sdk.VerifyVoteExtensionHand
 			return nil, err
 		}
 
+		if err := rejectUnknownVoteExtFields(req.VoteExtension); err != nil {
+			logger.Error("ALERT, VOTE EXTENSION REJECTED. THIS SHOULD NOT HAPPEN; THE VALIDATOR COULD BE MALICIOUS! Error while checking unknown fields in VoteExtension", "validator", valAddr, "error", err)
+			return &abci.ResponseVerifyVoteExtension{Status: abci.ResponseVerifyVoteExtension_REJECT}, nil
+		}
+
 		var voteExtension sidetxs.VoteExtension
 		if err := proto.Unmarshal(req.VoteExtension, &voteExtension); err != nil {
 			logger.Error("ALERT, VOTE EXTENSION REJECTED. THIS SHOULD NOT HAPPEN; THE VALIDATOR COULD BE MALICIOUS! Error while unmarshalling VoteExtension", "validator", valAddr, "error", err)
@@ -437,7 +453,13 @@ func (app *HeimdallApp) checkAndAddFutureSpan(ctx sdk.Context, majorityMilestone
 	logger := app.Logger()
 
 	if majorityMilestone.StartBlockNumber+uint64(len(majorityMilestone.BlockHashes)-1) >= lastSpan.StartBlock && helper.IsRio(lastSpan.EndBlock+1) {
-		logger.Info("New milestone is greater than the last span, creating a new veblop span", "lastSpan", lastSpan, "newMilestone", majorityMilestone)
+		logger.Info("New milestone's end block reached or exceeded the last span's start block, creating a new veblop span",
+			"lastSpanId", lastSpan.Id,
+			"lastSpanStartBlock", lastSpan.StartBlock,
+			"lastSpanEndBlock", lastSpan.EndBlock,
+			"milestoneStartBlock", majorityMilestone.StartBlockNumber,
+			"milestoneEndBlock", majorityMilestone.StartBlockNumber+uint64(len(majorityMilestone.BlockHashes)-1),
+		)
 
 		params, err := app.BorKeeper.GetParams(ctx)
 		if err != nil {
@@ -502,7 +524,13 @@ func (app *HeimdallApp) checkAndRotateCurrentSpan(ctx sdk.Context) error {
 	diff := ctx.BlockHeight() - int64(lastMilestoneBlock)
 
 	if lastMilestone != nil && lastMilestoneBlock != 0 && diff > helper.GetChangeProducerThreshold(ctx) && helper.IsRio(lastMilestone.EndBlock+1) {
-		logger.Info("Block finalization time is greater than change producer threshold, creating a new veblop span", "lastMilestone", lastMilestone, "lastMilestoneBlock", lastMilestoneBlock, "diff", diff, "currentBlock", ctx.BlockHeight())
+		logger.Info("Block finalization time is greater than the change producer threshold, creating a new veblop span",
+			"lastMilestoneStartBlock", lastMilestone.StartBlock,
+			"lastMilestoneEndBlock", lastMilestone.EndBlock,
+			"lastMilestoneHeimdallBlock", lastMilestoneBlock,
+			"currentBlock", ctx.BlockHeight(),
+			"diff", diff,
+		)
 
 		addSpanCtx, spanCache := app.cacheTxContext(ctx)
 
@@ -571,6 +599,8 @@ func (app *HeimdallApp) checkAndRotateCurrentSpan(ctx sdk.Context) error {
 				logger.Error("Error occurred while adding latest failed producer", "error", err)
 				return err
 			}
+
+			logger.Info("Span rotated due to the current producer's ineffectiveness", "currentProducerID", currentProducer)
 		}
 
 		if err == nil {
@@ -706,6 +736,11 @@ func (app *HeimdallApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlo
 		} else if helper.IsRio(majorityMilestone.StartBlockNumber) && ctx.BlockHeight() == int64(lastSpanHeimdallBlock)+1 {
 			logger.Info("Last span was created in the previous block, skipping milestone addition", "lastSpanHeimdallBlock", lastSpanHeimdallBlock, "currentBlock", ctx.BlockHeight())
 		} else {
+			logger.Info("2/3rd majority reached on milestone proposition",
+				"startBlock", majorityMilestone.StartBlockNumber,
+				"endBlock", majorityMilestone.StartBlockNumber+uint64(len(majorityMilestone.BlockHashes)-1),
+				strutil.HashesToString(majorityMilestone.BlockHashes),
+			)
 			isValidMilestone = true
 		}
 	}
@@ -719,14 +754,6 @@ func (app *HeimdallApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlo
 
 		addMilestoneCtx, msCache := app.cacheTxContext(ctx)
 
-		logger.Debug("Adding milestone", "hashes",
-			strutil.HashesToString(majorityMilestone.BlockHashes),
-			"startBlock", majorityMilestone.StartBlockNumber,
-			"endBlock", majorityMilestone.StartBlockNumber+uint64(len(majorityMilestone.BlockHashes)-1),
-			"proposer", proposer,
-			"totalDifficulty", majorityMilestone.BlockTds[len(majorityMilestone.BlockHashes)-1],
-		)
-
 		if err := app.MilestoneKeeper.AddMilestone(addMilestoneCtx, milestoneTypes.Milestone{
 			Proposer:        proposer,
 			Hash:            majorityMilestone.BlockHashes[len(majorityMilestone.BlockHashes)-1],
@@ -789,11 +816,16 @@ func (app *HeimdallApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlo
 		}
 
 		if pendingMilestone == nil {
+			logger.Debug("No milestone proposition majority found, checking for span rotation")
 			if err := app.checkAndRotateCurrentSpan(ctx); err != nil {
 				return nil, err
 			}
 		} else {
-			logger.Debug("33% majority milestone proposition found, skipping span rotation")
+			logger.Info("1/3rd voting power found on milestone proposition, skipping span rotation",
+				"startBlock", pendingMilestone.StartBlockNumber,
+				"endBlock", pendingMilestone.StartBlockNumber+uint64(len(pendingMilestone.BlockHashes)-1),
+				strutil.HashesToString(pendingMilestone.BlockHashes),
+			)
 		}
 	}
 
@@ -871,7 +903,7 @@ func (app *HeimdallApp) PreBlocker(ctx sdk.Context, req *abci.RequestFinalizeBlo
 
 				// make sure only one post handler is executed
 				if executedPostHandlers > 0 {
-					logger.Info("One post handler already executed, skipping others", "msg", msg)
+					logger.Info("One post handler already executed, skipping others", "msg", msg.String())
 					break
 				}
 			}
@@ -921,3 +953,16 @@ func (app *HeimdallApp) getValidatorSetForHeight(ctx sdk.Context, height int64)
 	}
 	return validatorSet, nil
 }
+
+// rejectUnknownVoteExtFields checks for unknown fields in the VoteExtension proto message
+func rejectUnknownVoteExtFields(bz []byte) error {
+	msg := new(sidetxs.VoteExtension)
+
+	var resolver jsonpb.AnyResolver = unknownproto.DefaultAnyResolver{}
+
+	if err := unknownproto.RejectUnknownFieldsStrict(bz, msg, resolver); err != nil {
+		return fmt.Errorf("vote extension contains unknown fields/extra bytes: %w", err)
+	}
+
+	return nil
+}

app/abci_test.go

@@ -777,6 +777,48 @@ func TestVerifyVoteExtensionHandler(t *testing.T) {
 	}
 }
 
+func TestVerifyVoteExtensionHandler_RejectsUnknownFieldsPadding(t *testing.T) {
+	setupAppResult := SetupApp(t, 1)
+	hApp := setupAppResult.App
+	validatorPrivKeys := setupAppResult.ValidatorKeys
+
+	ctx := hApp.BaseApp.NewContext(true)
+	ctx = setupContextWithVoteExtensionsEnableHeight(ctx, 1)
+
+	vals := hApp.StakeKeeper.GetAllValidators(ctx)
+	require.NotEmpty(t, vals)
+
+	valAddrBytes := common.FromHex(vals[0].Signer)
+
+	cometVal := abci.Validator{
+		Address: valAddrBytes,
+		Power:   vals[0].VotingPower,
+	}
+
+	ext := setupExtendedVoteInfo(
+		t,
+		cmtproto.BlockIDFlagCommit,
+		common.FromHex(TxHash1),
+		common.FromHex(TxHash2),
+		cometVal,
+		validatorPrivKeys[0],
+	)
+
+	// padding
+	paddedVE := appendProtobufPadding(ext.VoteExtension, 64*1024)
+
+	req := &abci.RequestVerifyVoteExtension{
+		ValidatorAddress: valAddrBytes,
+		Height:           CurrentHeight,
+		VoteExtension:    paddedVE,
+	}
+
+	resp, err := hApp.VerifyVoteExtensionHandler()(ctx, req)
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+	require.Equal(t, abci.ResponseVerifyVoteExtension_REJECT, resp.Status)
+}
+
 func TestPreBlocker(t *testing.T) {
 	priv, app, ctx, validatorPrivKeys := SetupAppWithABCIctx(t)
 	validators := app.StakeKeeper.GetAllValidators(ctx)

app/vote_ext_utils.go

@@ -60,6 +60,10 @@ func ValidateVoteExtensions(ctx sdk.Context, reqHeight int64, extVoteInfo []abci
 	ac := address.HexCodec{}
 
 	for _, vote := range extVoteInfo {
+		// reject unknown fields
+		if err := rejectUnknownVoteExtFields(vote.VoteExtension); err != nil {
+			return fmt.Errorf("unknown fields detected in vote extensions at height %d: %w", reqHeight, err)
+		}
 
 		// make sure the BlockIdFlag is valid
 		if !isBlockIdFlagValid(vote.BlockIdFlag) {
@@ -113,7 +117,7 @@ func ValidateVoteExtensions(ctx sdk.Context, reqHeight int64, extVoteInfo []abci
 		if _, found := seenValidators[valAddrStr]; found {
 			return fmt.Errorf("duplicate vote detected from validator %s at height %d", valAddrStr, reqHeight)
 		}
-		// Add validator address to the map
+		// Add the validator address to the map
 		seenValidators[valAddrStr] = struct{}{}
 
 		_, validator := validatorSet.GetByAddress(valAddrStr)
@@ -201,6 +205,12 @@ func FilterVoteExtensions(ctx sdk.Context, reqHeight int64, extVoteInfo []abciTy
 	ac := address.HexCodec{}
 
 	for _, vote := range extVoteInfo {
+		// reject unknown fields and skip invalid ones
+		if err := rejectUnknownVoteExtFields(vote.VoteExtension); err != nil {
+			logger.Error("unknown fields detected in vote extensions, skipping",
+				"height", reqHeight, "error", err)
+			continue
+		}
 
 		// make sure the BlockIdFlag is valid
 		if !isBlockIdFlagValid(vote.BlockIdFlag) {
@@ -252,7 +262,7 @@ func FilterVoteExtensions(ctx sdk.Context, reqHeight int64, extVoteInfo []abciTy
 		if _, found := seenValidators[valAddrStr]; found {
 			return nil, fmt.Errorf("duplicate vote detected from validator %s at height %d", valAddrStr, reqHeight)
 		}
-		// Add validator address to the map
+		// Add the validator address to the map
 		seenValidators[valAddrStr] = struct{}{}
 
 		_, validator := validatorSet.GetByAddress(valAddrStr)
@@ -614,12 +624,16 @@ func ValidateNonRpVoteExtensions(
 		return nil
 	}
 
-	// Check if there is vote extension with majority voting power
+	// Check if there is a vote extension with majority voting power
 	majorityExt, err := getMajorityNonRpVoteExtension(ctx, extVoteInfo, validatorSet, logger)
 	if err != nil {
 		return err
 	}
 
+	// Not running rejectUnknownVoteExtFields() here, because
+	// NonRpVoteExtension is not a protobuf-encoded sidetxs.VoteExtension and
+	// it would incorrectly reject valid non-rp VEs.
+
 	if err := ValidateNonRpVoteExtension(ctx, height-1, majorityExt, chainManagerKeeper, checkpointKeeper, contractCaller); err != nil {
 		return fmt.Errorf("failed to validate majority non rp vote extension: %w", err)
 	}