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

Backport / fix block tally in finality module (#374) #375

Merged
merged 1 commit into from
Jan 2, 2025
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

## Unreleased

### Bug fixes

- [#374](https://github.com/babylonlabs-io/babylon/pull/374) Fix non-consecutive finalization
of the block in `TallyBlocks` function

## v1.0.0-rc2

### Bug fixes
Expand Down
3 changes: 2 additions & 1 deletion x/finality/keeper/tallying.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func (k Keeper) TallyBlocks(ctx context.Context) {
// - has finality providers, finalised: impossible to happen, panic
// - does not have finality providers, finalised: impossible to happen, panic
// After this for loop, the blocks since earliest activated height are either finalised or non-finalisable
finalizationLoop:
for i := startHeight; i <= uint64(sdkCtx.HeaderInfo().Height); i++ {
ib, err := k.GetBlock(ctx, i)
if err != nil {
Expand All @@ -58,7 +59,7 @@ func (k Keeper) TallyBlocks(ctx context.Context) {
} else {
// if not, then this block and all subsequent blocks should not be finalised
// thus, we need to break here
break
break finalizationLoop
}
case fpSet == nil && !ib.Finalized:
// does not have finality providers, non-finalised: not finalisable,
Expand Down
58 changes: 58 additions & 0 deletions x/finality/keeper/tallying_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,61 @@ func giveNoQCToHeight(r *rand.Rand, ctx sdk.Context, fKeeper *keeper.Keeper, hei

return nil
}

func FuzzConsecutiveFinalization(f *testing.F) {
datagen.AddRandomSeedsToFuzzer(f, 10)

f.Fuzz(func(t *testing.T, seed int64) {
r := rand.New(rand.NewSource(seed))
ctrl := gomock.NewController(t)
defer ctrl.Finish()

bsKeeper := types.NewMockBTCStakingKeeper(ctrl)
iKeeper := types.NewMockIncentiveKeeper(ctrl)
cKeeper := types.NewMockCheckpointingKeeper(ctrl)
fKeeper, ctx := keepertest.FinalityKeeper(t, bsKeeper, iKeeper, cKeeper)

// activate BTC staking protocol at a random height
activatedHeight := datagen.RandomInt(r, 10) + 1
numBlockToInspect := uint64(30)
// There will be a block in between activatedHeight and activatedHeight + numBlockToInspect
// that woud not get necessary votes to be finalised
firstNonFinalizedBlock := activatedHeight + 1 + datagen.RandomInt(r, 20)

for i := activatedHeight; i < activatedHeight+numBlockToInspect; i++ {
// index blocks
fKeeper.SetBlock(ctx, &types.IndexedBlock{
Height: i,
AppHash: datagen.GenRandomByteArray(r, 32),
Finalized: false,
})

if i == firstNonFinalizedBlock {
// this block does not have QC
err := giveNoQCToHeight(r, ctx, fKeeper, i)
require.NoError(t, err)
} else {
// this block has QC
err := giveQCToHeight(r, ctx, fKeeper, i)
require.NoError(t, err)
}
}

ctx = datagen.WithCtxHeight(ctx, activatedHeight+numBlockToInspect-1)
fKeeper.TallyBlocks(ctx)

// all blocks up to firstNonFinalizedBlock must be finalised
for i := activatedHeight; i < firstNonFinalizedBlock; i++ {
ib, err := fKeeper.GetBlock(ctx, i)
require.NoError(t, err)
require.True(t, ib.Finalized)
}

// all blocks from the firstNonFinalizedBlock must not be finalised
for i := firstNonFinalizedBlock; i < activatedHeight+numBlockToInspect; i++ {
ib, err := fKeeper.GetBlock(ctx, i)
require.NoError(t, err)
require.False(t, ib.Finalized)
}
})
}
Loading