fix handling covent message when quorum is already reached (#141)

Pr: #130 allowed for
accepting covenant signatures after quorum is reached.

This change introduces regression i.e covenant signatures accepted after
quorum is reached also generated voting power events, this could lead to
weird results in processing voting power events.

This pr fixes that by making sure that events of any kind are generated
only if this is first time quorum is reached.

CHANGELOG.md

@@ -59,6 +59,11 @@ events
 * [#113](https://github.com/babylonlabs-io/babylon/pull/113) Add multibuild binary
 for upgrade handler `testnet` and `mainnet`.
 
+### Bug Fixes
+
+* [#141](https://github.com/babylonlabs-io/babylon/pull/141) Generate voting
+power events only once when reaching covenant committee quorum
+
 ## v0.11.0
 
 ### State Machine Breaking

x/btcstaking/keeper/btc_delegations.go

@@ -94,6 +94,7 @@ func (k Keeper) addCovenantSigsToBTCDelegation(
 	parsedUnbondingSlashingAdaptorSignatures []asig.AdaptorSignature,
 	params *types.Params,
 ) {
+	hadQuorum := btcDel.HasCovenantQuorums(params.CovenantQuorum)
 
 	// All is fine add received signatures to the BTC delegation and BtcUndelegation
 	btcDel.AddCovenantSigs(
@@ -115,7 +116,9 @@ func (k Keeper) addCovenantSigsToBTCDelegation(
 
 	// If reaching the covenant quorum after this msg, the BTC delegation becomes
 	// active. Then, record and emit this event
-	if btcDel.HasCovenantQuorums(params.CovenantQuorum) {
+	// We only emit power distribution events, and external quorum events if it
+	// is the first time the quorum is reached
+	if !hadQuorum && btcDel.HasCovenantQuorums(params.CovenantQuorum) {
 		if btcDel.HasInclusionProof() {
 			quorumReachedEvent := types.NewCovenantQuorumReachedEvent(
 				btcDel,

x/btcstaking/keeper/keeper_test.go

@@ -401,14 +401,12 @@ func (h *Helper) CreateCovenantSigs(
 	del *types.BTCDelegation,
 ) {
 	stakingTx, err := bbn.NewBTCTxFromBytes(del.StakingTx)
+	h.NoError(err)
 	stakingTxHash := stakingTx.TxHash().String()
 
-	bsParams := h.BTCStakingKeeper.GetParams(h.Ctx)
-
-	h.NoError(err)
 	covenantMsgs := h.GenerateCovenantSignaturesMessages(r, covenantSKs, msgCreateBTCDel, del)
-	for i := 0; i < int(bsParams.CovenantQuorum); i++ {
-		msgCopy := covenantMsgs[i]
+	for _, m := range covenantMsgs {
+		msgCopy := m
 		_, err := h.MsgServer.AddCovenantSigs(h.Ctx, msgCopy)
 		h.NoError(err)
 	}
@@ -417,14 +415,14 @@ func (h *Helper) CreateCovenantSigs(
 	*/
 	actualDelWithCovenantSigs, err := h.BTCStakingKeeper.GetBTCDelegation(h.Ctx, stakingTxHash)
 	h.NoError(err)
-	require.Equal(h.t, len(actualDelWithCovenantSigs.CovenantSigs), int(bsParams.CovenantQuorum))
+	require.Equal(h.t, len(actualDelWithCovenantSigs.CovenantSigs), len(covenantMsgs))
 	require.True(h.t, actualDelWithCovenantSigs.HasCovenantQuorums(h.BTCStakingKeeper.GetParams(h.Ctx).CovenantQuorum))
 
 	require.NotNil(h.t, actualDelWithCovenantSigs.BtcUndelegation)
 	require.NotNil(h.t, actualDelWithCovenantSigs.BtcUndelegation.CovenantSlashingSigs)
 	require.NotNil(h.t, actualDelWithCovenantSigs.BtcUndelegation.CovenantUnbondingSigList)
-	require.Len(h.t, actualDelWithCovenantSigs.BtcUndelegation.CovenantUnbondingSigList, int(bsParams.CovenantQuorum))
-	require.Len(h.t, actualDelWithCovenantSigs.BtcUndelegation.CovenantSlashingSigs, int(bsParams.CovenantQuorum))
+	require.Len(h.t, actualDelWithCovenantSigs.BtcUndelegation.CovenantUnbondingSigList, len(covenantMsgs))
+	require.Len(h.t, actualDelWithCovenantSigs.BtcUndelegation.CovenantSlashingSigs, len(covenantMsgs))
 	require.Len(h.t, actualDelWithCovenantSigs.BtcUndelegation.CovenantSlashingSigs[0].AdaptorSigs, 1)
 
 }

x/btcstaking/keeper/power_dist_change_test.go

@@ -3,6 +3,7 @@ package keeper_test
 import (
 	"math/rand"
 	"testing"
+	"time"
 
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/golang/mock/gomock"
@@ -493,3 +494,76 @@ func FuzzBTCDelegationEvents(f *testing.F) {
 		require.Len(t, events, 0)
 	})
 }
+
+func TestDoNotGenerateDuplicateEventsAfterHavingCovenantQuorum(t *testing.T) {
+	r := rand.New(rand.NewSource(time.Now().UnixNano()))
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	// mock BTC light client and BTC checkpoint modules
+	btclcKeeper := types.NewMockBTCLightClientKeeper(ctrl)
+	btccKeeper := types.NewMockBtcCheckpointKeeper(ctrl)
+	finalityKeeper := types.NewMockFinalityKeeper(ctrl)
+	h := NewHelper(t, btclcKeeper, btccKeeper, finalityKeeper)
+
+	// set all parameters
+	covenantSKs, _ := h.GenAndApplyParams(r)
+	changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net)
+	require.NoError(t, err)
+
+	// generate and insert new finality provider
+	_, fpPK, fp := h.CreateFinalityProvider(r)
+
+	// generate and insert new BTC delegation
+	stakingValue := int64(2 * 10e8)
+	expectedStakingTxHash, _, _, msgCreateBTCDel, actualDel := h.CreateDelegation(
+		r,
+		fpPK,
+		changeAddress.EncodeAddress(),
+		stakingValue,
+		1000,
+	)
+
+	/*
+		at this point, there should be 1 event that BTC delegation
+		will become expired at end height - w
+	*/
+	// there exists no event at the current BTC tip
+	btcTip := btclcKeeper.GetTipInfo(h.Ctx)
+	events := h.BTCStakingKeeper.GetAllPowerDistUpdateEvents(h.Ctx, btcTip.Height, btcTip.Height)
+	require.Len(t, events, 0)
+	// the BTC delegation will be unbonded at end height - w
+	unbondedHeight := actualDel.EndHeight - btccKeeper.GetParams(h.Ctx).CheckpointFinalizationTimeout
+	events = h.BTCStakingKeeper.GetAllPowerDistUpdateEvents(h.Ctx, unbondedHeight, unbondedHeight)
+	require.Len(t, events, 1)
+	btcDelStateUpdate := events[0].GetBtcDelStateUpdate()
+	require.NotNil(t, btcDelStateUpdate)
+	require.Equal(t, expectedStakingTxHash, btcDelStateUpdate.StakingTxHash)
+	require.Equal(t, types.BTCDelegationStatus_UNBONDED, btcDelStateUpdate.NewState)
+
+	// ensure this finality provider does not have voting power at the current height
+	babylonHeight := datagen.RandomInt(r, 10) + 1
+	h.SetCtxHeight(babylonHeight)
+	h.BTCLightClientKeeper.EXPECT().GetTipInfo(gomock.Eq(h.Ctx)).Return(btcTip).AnyTimes()
+	err = h.BTCStakingKeeper.BeginBlocker(h.Ctx)
+	h.NoError(err)
+	require.Zero(t, h.BTCStakingKeeper.GetVotingPower(h.Ctx, *fp.BtcPk, babylonHeight))
+
+	msgs := h.GenerateCovenantSignaturesMessages(r, covenantSKs, msgCreateBTCDel, actualDel)
+
+	// Generate and report covenant signatures from all covenant members.
+	for _, m := range msgs {
+		mCopy := m
+		_, err = h.MsgServer.AddCovenantSigs(h.Ctx, mCopy)
+		h.NoError(err)
+	}
+
+	// event though all covenant signatures are reported, only one event should be generated
+	events = h.BTCStakingKeeper.GetAllPowerDistUpdateEvents(h.Ctx, btcTip.Height, btcTip.Height)
+	// we should only have one event that the BTC delegation becomes active
+	require.Len(t, events, 1)
+	btcDelStateUpdate = events[0].GetBtcDelStateUpdate()
+	require.NotNil(t, btcDelStateUpdate)
+	require.Equal(t, expectedStakingTxHash, btcDelStateUpdate.StakingTxHash)
+	require.Equal(t, types.BTCDelegationStatus_ACTIVE, btcDelStateUpdate.NewState)
+}