Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,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

Expand Down
14 changes: 14 additions & 0 deletions testutil/mocks/checkpointing_expected_keepers.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions x/checkpointing/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions x/checkpointing/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions x/epoching/keeper/epochs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 24 additions & 4 deletions x/finality/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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!

Expand Down Expand Up @@ -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)
Expand Down
20 changes: 18 additions & 2 deletions x/finality/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)

Expand All @@ -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
Expand All @@ -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)
})
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions x/finality/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
1 change: 1 addition & 0 deletions x/finality/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
14 changes: 14 additions & 0 deletions x/finality/types/mocked_keepers.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading