diff --git a/x/finality/keeper/tallying.go b/x/finality/keeper/tallying.go index 33f6eb6dd..13eece9e1 100644 --- a/x/finality/keeper/tallying.go +++ b/x/finality/keeper/tallying.go @@ -16,11 +16,6 @@ import ( // - non-finalisable blocks (i.e., block with no active validator) // but without block that has validator set AND does not receive QC func (k Keeper) TallyBlocks(ctx sdk.Context) { - // blocksToFinalize is the set of blocks to finalise within this tallying attempt - blocksToFinalize := []*types.IndexedBlock{} - // valSets is the BTC validator set at each height with a non-finalised block - valSets := map[uint64]map[string]uint64{} - activatedHeight, err := k.BTCStakingKeeper.GetBTCStakingActivatedHeight(ctx) if err != nil { // invoking TallyBlocks when BTC staking protocol is not activated is a programming error @@ -28,56 +23,53 @@ func (k Keeper) TallyBlocks(ctx sdk.Context) { ctx.BlockHeight(), activatedHeight)) } - // find all blocks that are non-finalised AND have validator set, from latest to the earliest activated height + // start finalising blocks since max(activatedHeight, nextHeightToFinalize) + startHeight := k.getNextHeightToFinalize(ctx) + if startHeight < activatedHeight { + startHeight = activatedHeight + } + + // find all blocks that are non-finalised AND have validator set since max(activatedHeight, lastFinalizedHeight+1) // There are 4 different scenarios as follows - // - has validators, non-finalised: can finalise, add to blocksToFinalize - // - does not have validators, non-finalised: non-finalisable, skip - // - has validators, finalised, break here + // - has validators, non-finalised: tally and try to finalise + // - does not have validators, non-finalised: non-finalisable, continue + // - has validators, finalised: impossible to happen, panic // - does not have validators, finalised: impossible to happen, panic // After this for loop, the blocks since earliest activated height are either finalised or non-finalisable - blockRevIter := k.blockStore(ctx).ReverseIterator(sdk.Uint64ToBigEndian(uint64(activatedHeight)), nil) - for ; blockRevIter.Valid(); blockRevIter.Next() { - // get the indexed block - ibBytes := blockRevIter.Value() - var ib types.IndexedBlock - k.cdc.MustUnmarshal(ibBytes, &ib) + for i := startHeight; i <= uint64(ctx.BlockHeight()); i++ { + ib, err := k.GetBlock(ctx, i) + if err != nil { + panic(err) // failing to get an existing block is a programming error + } + // get the validator set of this block valSet := k.BTCStakingKeeper.GetVotingPowerTable(ctx, ib.Height) if valSet != nil && !ib.Finalized { - // has validators, non-finalised: can finalise, add block and valset - blocksToFinalize = append(blocksToFinalize, &ib) - valSets[ib.Height] = valSet + // has validators, non-finalised: tally and try to finalise the block + voterBTCPKs := k.GetVoters(ctx, ib.Height) + if tally(valSet, voterBTCPKs) { + // if this block gets >2/3 votes, finalise it + k.finalizeBlock(ctx, ib, voterBTCPKs) + } else { + // if not, then this block and all subsequent blocks should not be finalised + // thus, we need to break here + break + } } else if valSet == nil && !ib.Finalized { - // does not have validators, non-finalised: not finalisable, skip + // does not have validators, non-finalised: not finalisable, + // increment the next height to finalise and continue + k.setNextHeightToFinalize(ctx, ib.Height+1) continue } else if valSet != nil && ib.Finalized { // has validators and the block has finalised - // this means that the entire prefix has been finalised, break here - break + // this can only be a programming error + panic(fmt.Errorf("block %d is finalized, but last finalized height in DB does not reach here", ib.Height)) } else if valSet == nil && ib.Finalized { // does not have validators, finalised: impossible to happen, panic panic(fmt.Errorf("block %d is finalized, but does not have a validator set", ib.Height)) } } - // closing the iterator right now before finalising the finalisable blocks - // this is to follow the contract at https://github.com/cosmos/cosmos-sdk/blob/v0.47.4/store/types/store.go#L239-L240 - blockRevIter.Close() - - // for each of these blocks from earliest to latest, tally the block w.r.t. existing votes - for i := len(blocksToFinalize) - 1; i >= 0; i-- { - blockToFinalize := blocksToFinalize[i] - voterBTCPKs := k.GetVoters(ctx, blockToFinalize.Height) - valSet := valSets[blockToFinalize.Height] - if tally(valSet, voterBTCPKs) { - // if this block gets >2/3 votes, finalise it - k.finalizeBlock(ctx, blockToFinalize, voterBTCPKs) - } else { - // if not, then this block and all subsequent blocks should not be finalised - // thus, we need to break here - break - } - } } // finalizeBlock sets a block to be finalised in KVStore and distributes rewards to @@ -86,6 +78,8 @@ func (k Keeper) finalizeBlock(ctx sdk.Context, block *types.IndexedBlock, voterB // set block to be finalised in KVStore block.Finalized = true k.SetBlock(ctx, block) + // set next height to finalise as height+1 + k.setNextHeightToFinalize(ctx, block.Height+1) // distribute rewards to BTC staking stakeholders w.r.t. the reward distribution cache rdc, err := k.BTCStakingKeeper.GetRewardDistCache(ctx, block.Height) if err != nil { @@ -110,5 +104,23 @@ func tally(valSet map[string]uint64, voterBTCPKs map[string]struct{}) bool { votedPower += power } } - return votedPower > totalPower*2/3 + return votedPower*3 > totalPower*2 +} + +// setNextHeightToFinalize sets the next height to finalise as the given height +func (k Keeper) setNextHeightToFinalize(ctx sdk.Context, height uint64) { + store := ctx.KVStore(k.storeKey) + heightBytes := sdk.Uint64ToBigEndian(height) + store.Set(types.NextHeightToFinalizeKey, heightBytes) +} + +// getNextHeightToFinalize gets the next height to finalise +func (k Keeper) getNextHeightToFinalize(ctx sdk.Context) uint64 { + store := ctx.KVStore(k.storeKey) + bz := store.Get(types.NextHeightToFinalizeKey) + if bz == nil { + return 0 + } + height := sdk.BigEndianToUint64(bz) + return height } diff --git a/x/finality/keeper/tallying_test.go b/x/finality/keeper/tallying_test.go index 12ba86bc9..d6135d83d 100644 --- a/x/finality/keeper/tallying_test.go +++ b/x/finality/keeper/tallying_test.go @@ -16,7 +16,7 @@ import ( "github.com/stretchr/testify/require" ) -func FuzzTallying(f *testing.F) { +func FuzzTallying_PanicCases(f *testing.F) { datagen.AddRandomSeedsToFuzzer(f, 10) f.Fuzz(func(t *testing.T, seed int64) { @@ -39,14 +39,29 @@ func FuzzTallying(f *testing.F) { Finalized: true, }) // activate BTC staking protocol at height 1 + ctx = ctx.WithBlockHeight(1) bsKeeper.EXPECT().GetBTCStakingActivatedHeight(gomock.Any()).Return(uint64(1), nil).Times(1) bsKeeper.EXPECT().GetVotingPowerTable(gomock.Any(), gomock.Eq(uint64(1))).Return(nil).Times(1) require.Panics(t, func() { fKeeper.TallyBlocks(ctx) }) + }) +} + +func FuzzTallying_FinalizingNoBlock(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) + fKeeper, ctx := keepertest.FinalityKeeper(t, bsKeeper, iKeeper) // activate BTC staking protocol at a random height activatedHeight := datagen.RandomInt(r, 10) + 1 - // Case 3: index a list of blocks, don't give them QCs, and tally them + // index a list of blocks, don't give them QCs, and tally them // Expect they are not finalised for i := activatedHeight; i < activatedHeight+10; i++ { // index blocks @@ -55,22 +70,12 @@ func FuzzTallying(f *testing.F) { LastCommitHash: datagen.GenRandomByteArray(r, 32), Finalized: false, }) - // 1 vote - votedValPK, err := datagen.GenRandomBIP340PubKey(r) + // this block does not have QC + err := giveNoQCToHeight(r, ctx, bsKeeper, fKeeper, i) require.NoError(t, err) - votedSig, err := bbn.NewSchnorrEOTSSig(datagen.GenRandomByteArray(r, 32)) - require.NoError(t, err) - fKeeper.SetSig(ctx, i, votedValPK, votedSig) - // 4 BTC vals - valSet := map[string]uint64{ - hex.EncodeToString(votedValPK.MustMarshal()): 1, - hex.EncodeToString(datagen.GenRandomByteArray(r, 32)): 1, - hex.EncodeToString(datagen.GenRandomByteArray(r, 32)): 1, - hex.EncodeToString(datagen.GenRandomByteArray(r, 32)): 1, - } - bsKeeper.EXPECT().GetVotingPowerTable(gomock.Any(), gomock.Eq(i)).Return(valSet).Times(1) } // add mock queries to GetBTCStakingActivatedHeight + ctx = ctx.WithBlockHeight(int64(activatedHeight) + 10 - 1) bsKeeper.EXPECT().GetBTCStakingActivatedHeight(gomock.Any()).Return(activatedHeight, nil).Times(1) // tally blocks and none of them should be finalised fKeeper.TallyBlocks(ctx) @@ -79,8 +84,26 @@ func FuzzTallying(f *testing.F) { require.NoError(t, err) require.False(t, ib.Finalized) } + }) + +} + +func FuzzTallying_FinalizingSomeBlocks(f *testing.F) { + datagen.AddRandomSeedsToFuzzer(f, 10) - // Case 4: index a list of blocks, give some of them QCs, and tally them. + 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) + fKeeper, ctx := keepertest.FinalityKeeper(t, bsKeeper, iKeeper) + + // activate BTC staking protocol at a random height + activatedHeight := datagen.RandomInt(r, 10) + 1 + + // index a list of blocks, give some of them QCs, and tally them. // Expect they are all finalised numWithQCs := datagen.RandomInt(r, 5) + 1 for i := activatedHeight; i < activatedHeight+10; i++ { @@ -105,6 +128,7 @@ func FuzzTallying(f *testing.F) { iKeeper.EXPECT().RewardBTCStaking(gomock.Any(), gomock.Any(), gomock.Any()).Return().Times(int(numWithQCs)) bsKeeper.EXPECT().RemoveRewardDistCache(gomock.Any(), gomock.Any()).Return().Times(int(numWithQCs)) // add mock queries to GetBTCStakingActivatedHeight + ctx = ctx.WithBlockHeight(int64(activatedHeight) + 10 - 1) bsKeeper.EXPECT().GetBTCStakingActivatedHeight(gomock.Any()).Return(activatedHeight, nil).Times(1) // tally blocks and none of them should be finalised fKeeper.TallyBlocks(ctx) @@ -118,6 +142,7 @@ func FuzzTallying(f *testing.F) { } } }) + } func giveQCToHeight(r *rand.Rand, ctx sdk.Context, bsKeeper *types.MockBTCStakingKeeper, fKeeper *keeper.Keeper, height uint64) error { @@ -162,7 +187,7 @@ func giveNoQCToHeight(r *rand.Rand, ctx sdk.Context, bsKeeper *types.MockBTCStak hex.EncodeToString(datagen.GenRandomByteArray(r, 32)): 1, hex.EncodeToString(datagen.GenRandomByteArray(r, 32)): 1, } - bsKeeper.EXPECT().GetVotingPowerTable(gomock.Any(), gomock.Eq(height)).Return(valSet).Times(1) + bsKeeper.EXPECT().GetVotingPowerTable(gomock.Any(), gomock.Eq(height)).Return(valSet).MaxTimes(1) return nil } diff --git a/x/finality/types/keys.go b/x/finality/types/keys.go index b949e2271..0a5ce2e40 100644 --- a/x/finality/types/keys.go +++ b/x/finality/types/keys.go @@ -15,11 +15,12 @@ const ( ) var ( - BlockKey = []byte{0x01} // key prefix for blocks - VoteKey = []byte{0x02} // key prefix for votes - PubRandKey = []byte{0x03} // key prefix for public randomness - ParamsKey = []byte{0x04} // key prefix for the parameters - EvidenceKey = []byte{0x05} // key prefix for evidences + BlockKey = []byte{0x01} // key prefix for blocks + VoteKey = []byte{0x02} // key prefix for votes + PubRandKey = []byte{0x03} // key prefix for public randomness + ParamsKey = []byte{0x04} // key prefix for the parameters + EvidenceKey = []byte{0x05} // key prefix for evidences + NextHeightToFinalizeKey = []byte{0x06} // key prefix for next height to finalise ) func KeyPrefix(p string) []byte {