diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e8d6d5e2..0134ded90 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/testutil/mocks/checkpointing_expected_keepers.go b/testutil/mocks/checkpointing_expected_keepers.go index c4975364a..6c84cd2c8 100644 --- a/testutil/mocks/checkpointing_expected_keepers.go +++ b/testutil/mocks/checkpointing_expected_keepers.go @@ -79,6 +79,20 @@ func (mr *MockEpochingKeeperMockRecorder) GetEpoch(ctx interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEpoch", reflect.TypeOf((*MockEpochingKeeper)(nil).GetEpoch), ctx) } +// GetEpochNumByHeight mocks base method. +func (m *MockEpochingKeeper) GetEpochNumByHeight(ctx context.Context, height uint64) uint64 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetEpochNumByHeight", ctx, height) + ret0, _ := ret[0].(uint64) + return ret0 +} + +// GetEpochNumByHeight indicates an expected call of GetEpochNumByHeight. +func (mr *MockEpochingKeeperMockRecorder) GetEpochNumByHeight(ctx, height interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEpochNumByHeight", reflect.TypeOf((*MockEpochingKeeper)(nil).GetEpochNumByHeight), ctx, height) +} + // GetPubKeyByConsAddr mocks base method. func (m *MockEpochingKeeper) GetPubKeyByConsAddr(ctx context.Context, consAddr types1.ConsAddress) (crypto.PublicKey, error) { m.ctrl.T.Helper() diff --git a/x/checkpointing/keeper/keeper.go b/x/checkpointing/keeper/keeper.go index b5c0d677f..b521e8ada 100644 --- a/x/checkpointing/keeper/keeper.go +++ b/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) diff --git a/x/checkpointing/types/expected_keepers.go b/x/checkpointing/types/expected_keepers.go index 53f15112c..4a3f9d25d 100644 --- a/x/checkpointing/types/expected_keepers.go +++ b/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 diff --git a/x/epoching/keeper/epochs.go b/x/epoching/keeper/epochs.go index d8967bb0b..52b6c7456 100644 --- a/x/epoching/keeper/epochs.go +++ b/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 diff --git a/x/finality/keeper/msg_server.go b/x/finality/keeper/msg_server.go index 6b10ad7d9..ad9213307 100644 --- a/x/finality/keeper/msg_server.go +++ b/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) diff --git a/x/finality/keeper/msg_server_test.go b/x/finality/keeper/msg_server_test.go index 6dc2ae3cb..570bbd426 100644 --- a/x/finality/keeper/msg_server_test.go +++ b/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,6 +241,7 @@ 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) @@ -242,8 +249,15 @@ func FuzzAddFinalitySig(f *testing.F) { 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) diff --git a/x/finality/types/errors.go b/x/finality/types/errors.go index 1b026b6dc..5930fabdc 100644 --- a/x/finality/types/errors.go +++ b/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") ) diff --git a/x/finality/types/expected_keepers.go b/x/finality/types/expected_keepers.go index d56afbb4f..bc7eebea3 100644 --- a/x/finality/types/expected_keepers.go +++ b/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 } diff --git a/x/finality/types/mocked_keepers.go b/x/finality/types/mocked_keepers.go index c7098e7fd..7786eb1ca 100644 --- a/x/finality/types/mocked_keepers.go +++ b/x/finality/types/mocked_keepers.go @@ -242,6 +242,20 @@ func (mr *MockCheckpointingKeeperMockRecorder) GetEpoch(ctx interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEpoch", reflect.TypeOf((*MockCheckpointingKeeper)(nil).GetEpoch), ctx) } +// GetEpochByHeight mocks base method. +func (m *MockCheckpointingKeeper) GetEpochByHeight(ctx context.Context, height uint64) uint64 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetEpochByHeight", ctx, height) + ret0, _ := ret[0].(uint64) + return ret0 +} + +// GetEpochByHeight indicates an expected call of GetEpochByHeight. +func (mr *MockCheckpointingKeeperMockRecorder) GetEpochByHeight(ctx, height interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEpochByHeight", reflect.TypeOf((*MockCheckpointingKeeper)(nil).GetEpochByHeight), ctx, height) +} + // GetLastFinalizedEpoch mocks base method. func (m *MockCheckpointingKeeper) GetLastFinalizedEpoch(ctx context.Context) uint64 { m.ctrl.T.Helper()