chore(adr-39): Reject out-dated finality votes (#365)

CHANGELOG.md

@@ -65,6 +65,7 @@ for rewards
 - [#341](https://github.com/babylonlabs-io/babylon/pull/341) Select parameters
 for pre-approval flow based on BTC LC tip height
 - [#360](https://github.com/babylonlabs-io/babylon/pull/360) Refactor rewarding
+- [#365](https://github.com/babylonlabs-io/babylon/pull/365) Reject outdated finality votes
 
 ## v0.18.2
 

x/checkpointing/keeper/keeper.go

@@ -433,6 +433,10 @@ func (k Keeper) GetLastFinalizedEpoch(ctx context.Context) uint64 {
 	return sdk.BigEndianToUint64(epochNumberBytes)
 }
 
+func (k Keeper) GetEpochByHeight(ctx context.Context, height uint64) uint64 {
+	return k.epochingKeeper.GetEpochNumByHeight(ctx, height)
+}
+
 // SetLastFinalizedEpoch sets the last finalised epoch
 func (k Keeper) SetLastFinalizedEpoch(ctx context.Context, epochNumber uint64) {
 	store := k.storeService.OpenKVStore(ctx)

x/checkpointing/types/expected_keepers.go

@@ -13,6 +13,7 @@ import (
 // EpochingKeeper defines the expected interface needed to retrieve epoch info
 type EpochingKeeper interface {
 	GetEpoch(ctx context.Context) *epochingtypes.Epoch
+	GetEpochNumByHeight(ctx context.Context, height uint64) uint64
 	EnqueueMsg(ctx context.Context, msg epochingtypes.QueuedMessage)
 	GetValidatorSet(ctx context.Context, epochNumer uint64) epochingtypes.ValidatorSet
 	GetTotalVotingPower(ctx context.Context, epochNumber uint64) int64

x/epoching/keeper/epochs.go

@@ -55,6 +55,27 @@ func (k Keeper) GetEpoch(ctx context.Context) *types.Epoch {
 	return &epoch
 }
 
+func (k Keeper) GetEpochNumByHeight(ctx context.Context, height uint64) uint64 {
+	return CalculateEpochNumber(height, k.GetParams(ctx).EpochInterval)
+}
+
+// CalculateEpochNumber returns the epoch number for a given height
+// For height 0, it returns epoch 0
+// For all other heights, it calculates based on the epoch interval
+// Example with interval 5:
+// Height: 0  | 1  2  3  4  5 | 6  7  8  9  10 | 11 12 13 14 15 |
+// Epoch:  0  |       1       |        2        |        3        |
+func CalculateEpochNumber(height uint64, epochInterval uint64) uint64 {
+	if height == 0 {
+		return 0
+	}
+
+	// Subtract 1 from height since epoch 1 starts at height 1
+	height--
+	// Add interval to ensure we round up for partial epochs
+	return (height / epochInterval) + 1
+}
+
 func (k Keeper) GetHistoricalEpoch(ctx context.Context, epochNumber uint64) (*types.Epoch, error) {
 	epoch, err := k.getEpochInfo(ctx, epochNumber)
 	return epoch, err

x/finality/keeper/msg_server.go

@@ -73,6 +73,18 @@ func (ms msgServer) AddFinalitySig(goCtx context.Context, req *types.MsgAddFinal
 		return nil, errMod.Wrapf("finality block height: %d is lower than activation height %d", req.BlockHeight, activationHeight)
 	}
 
+	indexedBlock, err := ms.GetBlock(ctx, req.BlockHeight)
+	if err != nil {
+		return nil, err
+	}
+	should, err := ms.ShouldAcceptSigForHeight(ctx, indexedBlock)
+	if err != nil {
+		return nil, err
+	}
+	if !should {
+		return nil, types.ErrSigHeightOutdated.Wrapf("height: %d", req.BlockHeight)
+	}
+
 	fpPK := req.FpBtcPk
 
 	// ensure the finality provider exists
@@ -133,10 +145,6 @@ func (ms msgServer) AddFinalitySig(goCtx context.Context, req *types.MsgAddFinal
 	ms.SetPubRand(ctx, req.FpBtcPk, req.BlockHeight, *req.PubRand)
 
 	// verify whether the voted block is a fork or not
-	indexedBlock, err := ms.GetBlock(ctx, req.BlockHeight)
-	if err != nil {
-		return nil, err
-	}
 	if !bytes.Equal(indexedBlock.AppHash, req.BlockAppHash) {
 		// the finality provider votes for a fork!
 
@@ -210,6 +218,18 @@ func (ms msgServer) AddFinalitySig(goCtx context.Context, req *types.MsgAddFinal
 	return &types.MsgAddFinalitySigResponse{}, nil
 }
 
+func (ms msgServer) ShouldAcceptSigForHeight(ctx context.Context, block *types.IndexedBlock) (bool, error) {
+	epochNum := ms.CheckpointingKeeper.GetEpochByHeight(ctx, block.Height)
+	lastFinalizedEpoch := ms.GetLastFinalizedEpoch(ctx)
+	timestamped := lastFinalizedEpoch >= epochNum
+
+	// should NOT accept sig for height is the block is already and finalized by the BTC-timestamping
+	// protocol
+	should := !(block.Finalized && timestamped)
+
+	return should, nil
+}
+
 // CommitPubRandList commits a list of EOTS public randomness
 func (ms msgServer) CommitPubRandList(goCtx context.Context, req *types.MsgCommitPubRandList) (*types.MsgCommitPubRandListResponse, error) {
 	defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), types.MetricsKeyCommitPubRandList)

x/finality/keeper/msg_server_test.go

@@ -153,12 +153,15 @@ func FuzzAddFinalitySig(f *testing.F) {
 		signer := datagen.GenRandomAccount().Address
 		msg, err := datagen.NewMsgAddFinalitySig(signer, btcSK, startHeight, blockHeight, randListInfo, blockAppHash)
 		require.NoError(t, err)
+		ctx = ctx.WithHeaderInfo(header.Info{Height: int64(blockHeight)})
+		fKeeper.IndexBlock(ctx)
 
 		// Case 0: fail if the committed epoch is not finalized
 		lastFinalizedEpoch := datagen.RandomInt(r, int(committedEpochNum))
-		o1 := cKeeper.EXPECT().GetLastFinalizedEpoch(gomock.Any()).Return(lastFinalizedEpoch).Times(1)
+		o1 := cKeeper.EXPECT().GetLastFinalizedEpoch(gomock.Any()).Return(lastFinalizedEpoch).Times(2)
 		fKeeper.SetVotingPower(ctx, fpBTCPKBytes, blockHeight, 1)
 		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(1)
+		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), msg.BlockHeight).Return(uint64(1)).Times(1)
 		_, err = ms.AddFinalitySig(ctx, msg)
 		require.ErrorIs(t, err, types.ErrPubRandCommitNotBTCTimestamped)
 
@@ -169,6 +172,7 @@ func FuzzAddFinalitySig(f *testing.F) {
 		// Case 1: fail if the finality provider does not have voting power
 		fKeeper.SetVotingPower(ctx, fpBTCPKBytes, blockHeight, 0)
 		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(1)
+		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), msg.BlockHeight).Return(uint64(1)).Times(1)
 		_, err = ms.AddFinalitySig(ctx, msg)
 		require.Error(t, err)
 
@@ -178,7 +182,6 @@ func FuzzAddFinalitySig(f *testing.F) {
 		// Case 2: fail if the finality provider has not committed public randomness at that height
 		blockHeight2 := startHeight + numPubRand + 1
 		fKeeper.SetVotingPower(ctx, fpBTCPKBytes, blockHeight, 1)
-		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(1)
 		msg.BlockHeight = blockHeight2
 		_, err = ms.AddFinalitySig(ctx, msg)
 		require.Error(t, err)
@@ -191,6 +194,7 @@ func FuzzAddFinalitySig(f *testing.F) {
 		fKeeper.IndexBlock(ctx)
 		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(1)
 		// add vote and it should work
+		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), msg.BlockHeight).Return(uint64(1)).Times(1)
 		_, err = ms.AddFinalitySig(ctx, msg)
 		require.NoError(t, err)
 		// query this vote and assert
@@ -200,6 +204,7 @@ func FuzzAddFinalitySig(f *testing.F) {
 
 		// Case 4: In case of duplicate vote return success
 		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(1)
+		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), msg.BlockHeight).Return(uint64(1)).Times(1)
 		_, err = ms.AddFinalitySig(ctx, msg)
 		require.Error(t, err)
 
@@ -212,6 +217,7 @@ func FuzzAddFinalitySig(f *testing.F) {
 		bsKeeper.EXPECT().SlashFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(nil).Times(1)
 		// NOTE: even though this finality provider is slashed, the msg should be successful
 		// Otherwise the saved evidence will be rolled back
+		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), msg.BlockHeight).Return(uint64(1)).Times(1)
 		_, err = ms.AddFinalitySig(ctx, msg2)
 		require.NoError(t, err)
 		// ensure the evidence has been stored
@@ -235,15 +241,23 @@ func FuzzAddFinalitySig(f *testing.F) {
 		// Case 6: slashed finality provider cannot vote
 		fp.SlashedBabylonHeight = blockHeight
 		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(1)
+		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), msg.BlockHeight).Return(uint64(1)).Times(1)
 		_, err = ms.AddFinalitySig(ctx, msg)
 		require.ErrorIs(t, err, bstypes.ErrFpAlreadySlashed)
 
 		// Case 7: jailed finality provider cannot vote
 		fp.Jailed = true
 		fp.SlashedBabylonHeight = 0
 		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(1)
+		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), msg.BlockHeight).Return(uint64(1)).Times(1)
 		_, err = ms.AddFinalitySig(ctx, msg)
 		require.ErrorIs(t, err, bstypes.ErrFpAlreadyJailed)
+
+		// Case 8: vote rejected due to the block is finalized and timestamped
+		fKeeper.SetBlock(ctx, &types.IndexedBlock{Height: blockHeight, Finalized: true})
+		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), msg.BlockHeight).Return(uint64(1)).Times(1)
+		_, err = ms.AddFinalitySig(ctx, msg)
+		require.ErrorIs(t, err, types.ErrSigHeightOutdated)
 	})
 }
 
@@ -362,6 +376,7 @@ func TestVoteForConflictingHashShouldRetrieveEvidenceAndSlash(t *testing.T) {
 	msg1, err := datagen.NewMsgAddFinalitySig(signer, btcSK, startHeight, blockHeight, randListInfo, forkHash)
 	require.NoError(t, err)
 	fKeeper.SetVotingPower(ctx, fpBTCPKBytes, blockHeight, 1)
+	cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), gomock.Any()).Return(uint64(1)).AnyTimes()
 	bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(),
 		gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(1)
 	_, err = ms.AddFinalitySig(ctx, msg1)
@@ -444,6 +459,7 @@ func TestDoNotPanicOnNilProof(t *testing.T) {
 	// set the committed epoch finalized for the rest of the cases
 	lastFinalizedEpoch := datagen.GenRandomEpochNum(r) + committedEpochNum
 	cKeeper.EXPECT().GetLastFinalizedEpoch(gomock.Any()).Return(lastFinalizedEpoch).AnyTimes()
+	cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), gomock.Any()).Return(lastFinalizedEpoch).AnyTimes()
 
 	// add vote and it should work
 	_, err = ms.AddFinalitySig(ctx, msg)

x/finality/types/errors.go

@@ -22,4 +22,5 @@ var (
 	ErrVotingPowerTableNotUpdated     = errorsmod.Register(ModuleName, 1113, "voting power table has not been updated")
 	ErrBTCStakingNotActivated         = errorsmod.Register(ModuleName, 1114, "the BTC staking protocol is not activated yet")
 	ErrFinalityNotActivated           = errorsmod.Register(ModuleName, 1115, "finality is not active yet")
+	ErrSigHeightOutdated              = errorsmod.Register(ModuleName, 1116, "the voting block is already finalized and timestamped")
 )

x/finality/types/expected_keepers.go

@@ -25,6 +25,7 @@ type BTCStakingKeeper interface {
 }
 
 type CheckpointingKeeper interface {
+	GetEpochByHeight(ctx context.Context, height uint64) uint64
 	GetEpoch(ctx context.Context) *etypes.Epoch
 	GetLastFinalizedEpoch(ctx context.Context) uint64
 }